On Thu, 23 Jan 2025 11:00:32 +0000
Jonathan Cameron <jonathan.came...@huawei.com> wrote:
> On Wed, 22 Jan 2025 16:46:27 +0100
> Mauro Carvalho Chehab <mchehab+hua...@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.came...@huawei.com>
> > Co-authored-by: Shiju Jose <shiju.j...@huawei.com>
> > Co-authored-by: Mauro Carvalho Chehab <mchehab+hua...@kernel.org>
> > Signed-off-by: Jonathan Cameron <jonathan.came...@huawei.com>
> > Signed-off-by: Shiju Jose <shiju.j...@huawei.com>
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+hua...@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+hua...@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");
> > +}
>
