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.
> + *
> + * 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.
> + *
> + * 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");
> +}
