Re: [PATCH v5 4/6] hw/cxl: QMP based poison injection support

[Markus Armbruster]

Jonathan Cameron <Jonathan.Cameron@huawei.com> writes:

> Inject poison using qmp command cxl-inject-poison to add an entry to the
> poison list.
>
> For now, the poison is not returned CXL.mem reads, but only via the
> mailbox command Get Poison List. So a normal memory read to an address
> that is on the poison list will not yet result in a synchronous exception
> (and similar for partial cacheline writes).
> That is left for a future patch.
>
> See CXL rev 3.0, sec 8.2.9.8.4.1 Get Poison list (Opcode 4300h)
>
> Kernel patches to use this interface here:
> https://lore.kernel.org/linux-cxl/cover.1665606782.git.alison.schofield@intel.com/
>
> To inject poison using qmp (telnet to the qmp port)
> { "execute": "qmp_capabilities" }
>
> { "execute": "cxl-inject-poison",
>     "arguments": {
>          "path": "/machine/peripheral/cxl-pmem0",
>          "start": 2048,
>          "length": 256
>     }
> }
>
> Adjusted to select a device on your machine.
>
> Note that the poison list supported is kept short enough to avoid the
> complexity of state machine that is needed to handle the MORE flag.
>
> Reviewed-by: Fan Ni <fan.ni@samsung.com>
> Reviewed-by: Ira Weiny <ira.weiny@intel.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> ---
> v5:
>   - Picked up Fan Ni's tag
>   - Use ROUND_DOWN() as suggestion by Philippe
>   - Update qapi docs to 8.1
>   - Added comment to clarify the lack of synchronous poison exceptions
>     that has been left for a future patch set.
> ---
>  hw/cxl/cxl-mailbox-utils.c  | 90 +++++++++++++++++++++++++++++++++++++
>  hw/mem/cxl_type3.c          | 56 +++++++++++++++++++++++
>  hw/mem/cxl_type3_stubs.c    |  6 +++
>  include/hw/cxl/cxl.h        |  1 +
>  include/hw/cxl/cxl_device.h | 20 +++++++++
>  qapi/cxl.json               | 18 ++++++++
>  6 files changed, 191 insertions(+)

Please add

    [diff]
            orderFile = scripts/git.orderfile

to your .git/config, so that QAPI schema changes come first in diffs.

[...]

> diff --git a/qapi/cxl.json b/qapi/cxl.json
> index 4be7d46041..ca3af3f0b2 100644
> --- a/qapi/cxl.json
> +++ b/qapi/cxl.json
> @@ -5,6 +5,24 @@
>  # = CXL devices
>  ##
>  
> +##
> +# @cxl-inject-poison:
> +#
> +# Poison records indicate that a CXL memory device knows that a particular
> +# memory region may be corrupted. This may be because of locally detected
> +# errors (e.g. ECC failure) or poisoned writes received from other components
> +# in the system. This injection mechanism enables testing of the OS handling
> +# of poison records which may be queried via the CXL mailbox.
> +#
> +# @path: CXL type 3 device canonical QOM path
> +# @start: Start address - must be 64 byte aligned.
> +# @length: Length of poison to inject - must be a multiple of 64 bytes.
> +#
> +# Since: 8.1
> +##
> +{ 'command': 'cxl-inject-poison',
> +  'data': { 'path': 'str', 'start': 'uint64', 'length': 'uint64' }}
> +
>  ##
>  # @CxlUncorErrorType:
>  #

Please format like

   ##
   # @cxl-inject-poison:
   #
   # Poison records indicate that a CXL memory device knows that a
   # particular memory region may be corrupted.  This may be because of
   # locally detected errors (e.g. ECC failure) or poisoned writes
   # received from other components in the system.  This injection
   # mechanism enables testing of the OS handling of poison records which
   # may be queried via the CXL mailbox.
   #
   # @path: CXL type 3 device canonical QOM path
   #
   # @start: Start address - must be 64 byte aligned.
   #
   # @length: Length of poison to inject - must be a multiple of 64
   #     bytes.
   #
   # Since: 8.1
   ##

to blend in with recent commit a937b6aa739 (qapi: Reformat doc comments
to conform to current conventions).

The blank lines help with catching certain errors.  rST loves to
surprise...

> +{ 'command': 'cxl-inject-poison',
> +  'data': { 'path': 'str', 'start': 'uint64', 'length': 'uint64' }}
> +

Make this 'length': 'size'.  Just to signal intent; it generates the
same code.

>  ##
>  # @CxlUncorErrorType:
>  #

With that
Acked-by: Markus Armbruster <armbru@redhat.com>