Re: [PATCH v4 5/7] qapi/ghes-cper: add an interface to do generic CPER error injection

Markus Armbruster
2024-08-02T09:44:38Z

Mauro Carvalho Chehab <mchehab+huawei@kernel.org> writes:

> Creates a QAPI to be used for generic ACPI APEI hardware error
> injection (HEST) via GHESv2.

Creates a QMP command

> The actual GHES code will be added at the followup patch.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
> MAINTAINERS | 7 ++++++
> hw/acpi/Kconfig | 5 ++++
> hw/acpi/ghes_cper.c | 53 +++++++++++++++++++++++++++++++++++++++++
> hw/acpi/meson.build | 2 ++
> hw/arm/Kconfig | 5 ++++
> include/hw/acpi/ghes.h | 6 +++++
> qapi/ghes-cper.json | 54 ++++++++++++++++++++++++++++++++++++++++++
> qapi/meson.build | 1 +
> qapi/qapi-schema.json | 1 +
> 9 files changed, 134 insertions(+)
> create mode 100644 hw/acpi/ghes_cper.c
> create mode 100644 qapi/ghes-cper.json
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 98eddf7ae155..655edcb6688c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2075,6 +2075,13 @@ F: hw/acpi/ghes.c
> F: include/hw/acpi/ghes.h
> F: docs/specs/acpi_hest_ghes.rst
>
> +ACPI/HEST/GHES/ARM processor CPER
> +R: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> +S: Maintained
> +F: hw/arm/ghes_cper.c
> +F: hw/acpi/ghes_cper_stub.c
> +F: qapi/ghes-cper.json
> +
> ppc4xx
> L: qemu-ppc@nongnu.org
> S: Orphan

[...]

> diff --git a/hw/acpi/meson.build b/hw/acpi/meson.build
> index fa5c07db9068..6cbf430eb66d 100644
> --- a/hw/acpi/meson.build
> +++ b/hw/acpi/meson.build
> @@ -34,4 +34,6 @@ endif
> system_ss.add(when: 'CONFIG_ACPI', if_false: files('acpi-stub.c', 'aml-build-stub.c', 'ghes-stub.c', 'acpi_interface.c'))
> system_ss.add(when: 'CONFIG_ACPI_PCI_BRIDGE', if_false: files('pci-bridge-stub.c'))
> system_ss.add_all(when: 'CONFIG_ACPI', if_true: acpi_ss)
> +system_ss.add(when: 'CONFIG_GHES_CPER', if_true: files('ghes_cper.c'))
> +system_ss.add(when: 'CONFIG_GHES_CPER', if_false: files('ghes_cper_stub.c'))

../hw/acpi/meson.build:38:50: ERROR: File ghes_cper_stub.c does not exist.

> system_ss.add(files('acpi-qmp-cmds.c'))

[...]

> diff --git a/qapi/ghes-cper.json b/qapi/ghes-cper.json
> new file mode 100644
> index 000000000000..2b0438431054
> --- /dev/null
> +++ b/qapi/ghes-cper.json

Would this fit into acpi.json?

If yes: would the maintainers of acpi.json be happy to take it?
Michael, Igor?

> @@ -0,0 +1,54 @@
> +# -*- Mode: Python -*-
> +# vim: filetype=python
> +
> +##
> +# = GHESv2 CPER Error Injection
> +#
> +# These are defined at
> +# ACPI 6.2: 18.3.2.8 Generic Hardware Error Source version 2
> +# (GHESv2 - Type 10)
> +##
> +
> +##
> +# @CommonPlatformErrorRecord:
> +#
> +# Common Platform Error Record - CPER - as defined at the UEFI
> +# specification. See

Separate sentences with two spaces for consistency, please.

> +# https://uefi.org/specs/UEFI/2.10/Apx_N_Common_Platform_Error_Record.html#record-header
> +# for more details.
> +#
> +# @notification-type: pre-assigned GUID value indicating the record
> +# association with an error event notification type, as defined
> +# at https://uefi.org/specs/UEFI/2.10/Apx_N_Common_Platform_Error_Record.html#record-header
> +#
> +# @raw-data: Contains the payload of the CPER.
> +#
> +# Since: 9.2
> +##
> +{ 'struct': 'CommonPlatformErrorRecord',
> + 'data': {
> + 'notification-type': ['uint8'],
> + 'raw-data': ['uint8']
> + }
> +}

You use ['uint8'] for binary. We commonly represent binary as
base64-encoded strings in QAPI.

If @notification-type is a UUID, we should use the usual text
representation. qemu/uuid.h has functions to convert between binary and
text.

> +
> +##
> +# @ghes-cper:
> +#
> +# Inject ARM Processor error with data to be filled according with
> +# ACPI 6.2 GHESv2 spec.
> +#
> +# @cper: a single CPER record to be sent to the guest OS.
> +#
> +# Features:
> +#
> +# @unstable: This command is experimental.
> +#
> +# Since: 9.2
> +##
> +{ 'command': 'ghes-cper',
> + 'data': {
> + 'cper': 'CommonPlatformErrorRecord'
> + },
> + 'features': [ 'unstable' ]
> +}
> diff --git a/qapi/meson.build b/qapi/meson.build
> index e7bc54e5d047..bd13cd7d40c9 100644
> --- a/qapi/meson.build
> +++ b/qapi/meson.build
> @@ -35,6 +35,7 @@ qapi_all_modules = [
> 'dump',
> 'ebpf',
> 'error',
> + 'ghes-cper',
> 'introspect',
> 'job',
> 'machine-common',
> diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
> index b1581988e4eb..54165c8d6655 100644
> --- a/qapi/qapi-schema.json
> +++ b/qapi/qapi-schema.json
> @@ -81,3 +81,4 @@
> { 'include': 'vfio.json' }
> { 'include': 'cryptodev.json' }
> { 'include': 'cxl.json' }
> +{ 'include': 'ghes-cper.json' }

Put it next to acpi.json, perhaps?