Re: [PATCH v5 8/8] hw/mem/cxl_type3: Add CXL RAS Error Injection Support.

[Markus Armbruster]

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

> On Tue, 21 Feb 2023 23:15:49 +0100
> Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
>> Hi Jonathan,
>> 
>> On 21/2/23 16:21, Jonathan Cameron wrote:
>> > CXL uses PCI AER Internal errors to signal to the host that an error has
>> > occurred. The host can then read more detailed status from the CXL RAS
>> > capability.
>> > 
>> > For uncorrectable errors: support multiple injection in one operation
>> > as this is needed to reliably test multiple header logging support in an
>> > OS. The equivalent feature doesn't exist for correctable errors, so only
>> > one error need be injected at a time.
>> > 
>> > Note:
>> >   - Header content needs to be manually specified in a fashion that
>> >     matches the specification for what can be in the header for each
>> >     error type.
>> > 
>> > Injection via QMP:
>> > { "execute": "qmp_capabilities" }
>> > ...
>> > { "execute": "cxl-inject-uncorrectable-errors",
>> >    "arguments": {
>> >      "path": "/machine/peripheral/cxl-pmem0",
>> >      "errors": [
>> >          {
>> >              "type": "cache-address-parity",
>> >              "header": [ 3, 4]
>> >          },
>> >          {
>> >              "type": "cache-data-parity",
>> >              "header": 
>> > [0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31]
>> >          },
>> >          {
>> >              "type": "internal",
>> >              "header": [ 1, 2, 4]
>> >          }
>> >          ]
>> >    }}
>> > ...
>> > { "execute": "cxl-inject-correctable-error",
>> >      "arguments": {
>> >          "path": "/machine/peripheral/cxl-pmem0",
>> >          "type": "physical"
>> >      } }
>> > 
>> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> Hi Philippe,
>
> Thanks for your review.  One question inline.
>
>> > +#
>> > +# Type of uncorrectable CXL error to inject. These errors are reported via
>> > +# an AER uncorrectable internal error with additional information logged 
>> > at
>> > +# the CXL device.
>> > +#
>> > +# @cache-data-parity: Data error such as data parity or data ECC error 
>> > CXL.cache
>> > +# @cache-address-parity: Address parity or other errors associated with 
>> > the
>> > +#                        address field on CXL.cache
>> > +# @cache-be-parity: Byte enable parity or other byte enable errors on 
>> > CXL.cache
>> > +# @cache-data-ecc: ECC error on CXL.cache
>> > +# @mem-data-parity: Data error such as data parity or data ECC error on 
>> > CXL.mem
>> > +# @mem-address-parity: Address parity or other errors associated with the
>> > +#                      address field on CXL.mem
>> > +# @mem-be-parity: Byte enable parity or other byte enable errors on 
>> > CXL.mem.
>> > +# @mem-data-ecc: Data ECC error on CXL.mem.
>> > +# @reinit-threshold: REINIT threshold hit.
>> > +# @rsvd-encoding: Received unrecognized encoding.
>> > +# @poison-received: Received poison from the peer.
>> > +# @receiver-overflow: Buffer overflows (first 3 bits of header log 
>> > indicate which)
>> > +# @internal: Component specific error
>> > +# @cxl-ide-tx: Integrity and data encryption tx error.
>> > +# @cxl-ide-rx: Integrity and data encryption rx error.
>> > +##
>> > +
>> > +{ 'enum': 'CxlUncorErrorType',  
>> 
>> Doesn't these need
>> 
>>       'if': 'CONFIG_CXL_MEM_DEVICE',
>> 
>> ?
>
> If I make this change I get a bunch of
>
> ./qapi/qapi-types-cxl.h:18:13: error: attempt to use poisoned 
> "CONFIG_CXL_MEM_DEVICE"
>    18 | #if defined(CONFIG_CXL_MEM_DEVICE)

This means you're trying to use target-dependent stuff in
target-independent code.

Have a look at the thread

    Subject: Can we unpoison CONFIG_FOO macros?
    Message-ID: <87lel9o56z.fsf@pond.sub.org>
    https://lists.gnu.org/archive/html/qemu-devel/2023-02/msg01885.html

and if questions remain, ask them right here.

> It's a target specific define (I think) as built alongside PCI_EXPRESS
> Only CXL_ACPI is specifically included by x86 and arm64 (out of tree)
>
> To be honest though I don't fully understand the QEMU build system so the 
> reason
> for the error might be wrong.

[...]