Re: [PATCH 10/11] qapi/acpi-hest: add an interface to do generic CPER error injection

[Igor Mammedov]

On Thu, 23 Jan 2025 11:00:32 +0000
Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:

> On Wed, 22 Jan 2025 16:46:27 +0100
> Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote:
> 
> > Creates a QMP command to be used for generic ACPI APEI hardware error
> > injection (HEST) via GHESv2, and add support for it for ARM guests.
> > 
> > Error injection uses ACPI_HEST_SRC_ID_QMP source ID to be platform
> > independent. This is mapped at arch virt bindings, depending on the
> > types supported by QEMU and by the BIOS. So, on ARM, this is supported
> > via ACPI_GHES_NOTIFY_GPIO notification type.
> > 
> > This patch is co-authored:
> >     - original ghes logic to inject a simple ARM record by Shiju Jose;
> >     - generic logic to handle block addresses by Jonathan Cameron;
> >     - generic GHESv2 error inject by Mauro Carvalho Chehab;
> > 
> > Co-authored-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > Co-authored-by: Shiju Jose <shiju.jose@huawei.com>
> > Co-authored-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> > 
> > ---
> > 
> > Changes since v9:
> > - ARM source IDs renamed to reflect SYNC/ASYNC;
> > - command name changed to better reflect what it does;
> > - some improvements at JSON documentation;
> > - add a check for QMP source at the notification logic.
> > 
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>  
> Another bonus.
> 
> Few trivial formatting comments, otherwise looks fine to me.
> 
> Jonathan
> 
> > diff --git a/hw/acpi/ghes.c b/hw/acpi/ghes.c
> > index 5d29db3918dd..cf83c959b5ef 100644
> > --- a/hw/acpi/ghes.c
> > +++ b/hw/acpi/ghes.c
> > @@ -547,7 +547,7 @@ void ghes_record_cper_errors(const void *cper, size_t 
> > len,
> >      /* Write the generic error data entry into guest memory */
> >      cpu_physical_memory_write(cper_addr, cper, len);
> >  
> > -    notifier_list_notify(&acpi_generic_error_notifiers, NULL);
> > +    notifier_list_notify(&acpi_generic_error_notifiers, &source_id);
> >  }
> >  
> >  int acpi_ghes_memory_errors(uint16_t source_id, uint64_t physical_address)
> > diff --git a/hw/acpi/ghes_cper.c b/hw/acpi/ghes_cper.c
> > new file mode 100644
> > index 000000000000..02c47b41b990
> > --- /dev/null
> > +++ b/hw/acpi/ghes_cper.c
> > @@ -0,0 +1,32 @@
> > +/*
> > + * CPER payload parser for error injection
> > + *
> > + * Copyright(C) 2024 Huawei LTD.

2025

> > + *
> > + * This code is licensed under the GPL version 2 or later. See the
> > + * COPYING file in the top-level directory.
> > + *
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +
> > +#include "qemu/base64.h"
> > +#include "qemu/error-report.h"
> > +#include "qemu/uuid.h"
> > +#include "qapi/qapi-commands-acpi-hest.h"
> > +#include "hw/acpi/ghes.h"
> > +
> > +void qmp_inject_ghes_error(const char *qmp_cper, Error **errp)
> > +{
> > +  
> Odd blank line that can go.
> 
> > +    uint8_t *cper;
> > +    size_t  len;
> > +
> > +    cper = qbase64_decode(qmp_cper, -1, &len, errp);
> > +    if (!cper) {
> > +        error_setg(errp, "missing GHES CPER payload");
> > +        return;
> > +    }
> > +
> > +    ghes_record_cper_errors(cper, len, ACPI_HEST_SRC_ID_QMP, errp);
> > +}
> > diff --git a/hw/acpi/ghes_cper_stub.c b/hw/acpi/ghes_cper_stub.c
> > new file mode 100644
> > index 000000000000..8782e2c02fa8
> > --- /dev/null
> > +++ b/hw/acpi/ghes_cper_stub.c
> > @@ -0,0 +1,19 @@
> > +/*
> > + * Stub interface for CPER payload parser for error injection
> > + *
> > + * Copyright(C) 2024 Huawei LTD.

2025

> > + *
> > + * This code is licensed under the GPL version 2 or later. See the
> > + * COPYING file in the top-level directory.
> > + *  
> Trivial but I'd drop these trailing blank lines as they don't add
> anything other than making people scroll further.
> 
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "qapi/error.h"
> > +#include "qapi/qapi-commands-acpi-hest.h"
> > +#include "hw/acpi/ghes.h"  
> 
> Trivial but doe we need ghes.h?
> 
> > +
> > +void qmp_inject_ghes_error(const char *cper, Error **errp)
> > +{
> > +    error_setg(errp, "GHES QMP error inject is not compiled in");
> > +}  
>