Re: [RFC PATCH v2 1/3] hw/cxl: QMP based poison injection support

[Markus Armbruster]

Jonathan Cameron via <qemu-devel@nongnu.org> 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.
>
> 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.
>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

[...]

> diff --git a/qapi/cxl.json b/qapi/cxl.json
> index 3c18556ee8..5b995db255 100644
> --- a/qapi/cxl.json
> +++ b/qapi/cxl.json

There is no qapi/cxl.json in current master.  So this must be based on
some other patch(es).  Please point to it in the cover letter.  I like
to point both in human-readable and machine-readable form, e.g. like
this:

    Based on my "[PATCH 00/22] qapi: Remove simple unions from the schema
    language".

    Based-on: Message-Id: <20210913123932.3306639-1-armbru@redhat.com>

> @@ -5,6 +5,17 @@
>  # = CXL devices
>  ##
>  
> +##
> +# @cxl-inject-poison:
> +#
> +# @path: CXL type 3 device canonical QOM path
> +#
> +# @start: Start address
> +# @length: Length of poison to inject

Either separate all the arguments with blank lines, or none.

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

Both commit message and doc comment are rather terse.

The commit message should make the case for the feature: why do we want
it?  This typically involves explaining the problem(s) it solves.

The doc comment ideally explains intended use.