Re: [RFC PATCH 2/2] hw/cxl/type3: send a GMER while injecting poison

[Jonathan Cameron via <qemu-devel@nongnu.org>]

On Fri,  9 Feb 2024 19:54:12 +0800
Shiyang Ruan <ruansy.fnst@fujitsu.com> wrote:

> Send a signal to OS to let it able to handle the poison range.
> 
> TODO: This is an rough draft, will add more parameters for
> qmp_cxl_inject_poison() to set to GMER.

I wonder if that's the best plan or if we should think about
providing some default memory topology and perhaps a means
to override it.   Adding more parameters to the injection
commands works for qmp injection but not for mailbox based
injection from the host for example.

If we have a DPA to channel, rank, etc default mapping then
we should be able to fill them all in based on just the DPA
but allow them to be overridden from the existing GMER
injection (so other test cases can be poked).

One comment inline.

Jonathan

> 
> Signed-off-by: Shiyang Ruan <ruansy.fnst@fujitsu.com>
> ---
>  hw/mem/cxl_type3.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> index d8fb63b1de..813f7f2175 100644
> --- a/hw/mem/cxl_type3.c
> +++ b/hw/mem/cxl_type3.c
> @@ -1116,6 +1116,11 @@ void qmp_cxl_inject_poison(const char *path, uint64_t start, uint64_t length,
>  
>      QLIST_INSERT_HEAD(&ct3d->poison_list, p, node);
>      ct3d->poison_list_cnt++;
> +
> +    /* Emit an GMER event, let os handle it */
> +    qmp_cxl_inject_general_media_event(path, CXL_EVENT_LOG_FAILURE, 0, start,
> +                                       0, 0, 4, false, 0, false, 0,
> +                                       false, 0, NULL, errp);

This results in some slightly messy duplication of effort. I'd like to see the
parts we need factored out of the qmp_cxl_inject_general_media_event()
so that we don't end up looking up the device twice and similar.
Pull the code we need in both places out to
cxl_inject_general_media_event(CXLType3 *ct3d, ...

helper in a precursor patch and reuse that code here.

  }
>  
>  /* For uncorrectable errors include support for multiple header recording */