1. Patchew
  2. QEMU
  3. [PATCH v5 0/7] Add ACPI CPER firmware first error injection on ARM emulation
  4. View patch

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

Mauro Carvalho Chehab posted 7 patches 3 months, 3 weeks ago
There is a newer version of this series
[PATCH v5 5/7] qapi/ghes-cper: add an interface to do generic CPER error injection
Posted by Mauro Carvalho Chehab 3 months, 3 weeks ago 
Creates a QMP command to be used for generic ACPI APEI hardware error
injection (HEST) via GHESv2.

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      | 45 ++++++++++++++++++++++++++++++++
 hw/acpi/ghes_cper_stub.c | 18 +++++++++++++
 hw/acpi/meson.build      |  2 ++
 hw/arm/Kconfig           |  5 ++++
 include/hw/acpi/ghes.h   |  7 +++++
 qapi/ghes-cper.json      | 55 ++++++++++++++++++++++++++++++++++++++++
 qapi/meson.build         |  1 +
 qapi/qapi-schema.json    |  1 +
 10 files changed, 146 insertions(+)
 create mode 100644 hw/acpi/ghes_cper.c
 create mode 100644 hw/acpi/ghes_cper_stub.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/Kconfig b/hw/acpi/Kconfig
index e07d3204eb36..73ffbb82c150 100644
--- a/hw/acpi/Kconfig
+++ b/hw/acpi/Kconfig
@@ -51,6 +51,11 @@ config ACPI_APEI
     bool
     depends on ACPI
 
+config GHES_CPER
+    bool
+    depends on ACPI_APEI
+    default y
+
 config ACPI_PCI
     bool
     depends on ACPI && PCI
diff --git a/hw/acpi/ghes_cper.c b/hw/acpi/ghes_cper.c
new file mode 100644
index 000000000000..7aa7e71e90dc
--- /dev/null
+++ b/hw/acpi/ghes_cper.c
@@ -0,0 +1,45 @@
+/*
+ * ARM Processor 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-ghes-cper.h"
+#include "hw/acpi/ghes.h"
+
+void qmp_ghes_cper(CommonPlatformErrorRecord *qmp_cper,
+                   Error **errp)
+{
+    int rc;
+    AcpiGhesCper cper;
+    QemuUUID be_uuid, le_uuid;
+
+    rc = qemu_uuid_parse(qmp_cper->notification_type, &be_uuid);
+    if (rc) {
+        error_setg(errp, "GHES: Invalid UUID: %s",
+                   qmp_cper->notification_type);
+        return;
+    }
+
+    le_uuid = qemu_uuid_bswap(be_uuid);
+    cper.guid = le_uuid.data;
+
+    cper.data = qbase64_decode(qmp_cper->raw_data, -1,
+                               &cper.data_len, errp);
+    if (!cper.data) {
+        return;
+    }
+
+    /* TODO: call a function at ghes */
+
+    g_free(cper.data);
+}
diff --git a/hw/acpi/ghes_cper_stub.c b/hw/acpi/ghes_cper_stub.c
new file mode 100644
index 000000000000..7ce6ed70a265
--- /dev/null
+++ b/hw/acpi/ghes_cper_stub.c
@@ -0,0 +1,18 @@
+/*
+ * ARM Processor 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 "qapi/error.h"
+#include "qapi/qapi-commands-ghes-cper.h"
+#include "hw/acpi/ghes.h"
+
+void qmp_ghes_cper(CommonPlatformErrorRecord *cper, Error **errp)
+{
+}
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'))
 system_ss.add(files('acpi-qmp-cmds.c'))
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 1ad60da7aa2d..bed6ba27d715 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -712,3 +712,8 @@ config ARMSSE
     select UNIMP
     select SSE_COUNTER
     select SSE_TIMER
+
+config GHES_CPER
+    bool
+    depends on ARM
+    default y if AARCH64
diff --git a/include/hw/acpi/ghes.h b/include/hw/acpi/ghes.h
index 33be1eb5acf4..06a5b8820cd5 100644
--- a/include/hw/acpi/ghes.h
+++ b/include/hw/acpi/ghes.h
@@ -23,6 +23,7 @@
 #define ACPI_GHES_H
 
 #include "hw/acpi/bios-linker-loader.h"
+#include "qapi/error.h"
 #include "qemu/notify.h"
 
 extern NotifierList generic_error_notifiers;
@@ -78,6 +79,12 @@ void acpi_ghes_add_fw_cfg(AcpiGhesState *vms, FWCfgState *s,
                           GArray *hardware_errors);
 int acpi_ghes_record_errors(uint8_t notify, uint64_t error_physical_addr);
 
+typedef struct AcpiGhesCper {
+    uint8_t *guid;
+    uint8_t *data;
+    size_t data_len;
+} AcpiGhesCper;
+
 /**
  * acpi_ghes_present: Report whether ACPI GHES table is present
  *
diff --git a/qapi/ghes-cper.json b/qapi/ghes-cper.json
new file mode 100644
index 000000000000..3cc4f9f2aaa9
--- /dev/null
+++ b/qapi/ghes-cper.json
@@ -0,0 +1,55 @@
+# -*- 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
+# https://uefi.org/specs/UEFI/2.10/Apx_N_Common_Platform_Error_Record.html#record-header
+# for more details.
+#
+# @notification-type: pre-assigned GUID string 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 a base64 encoded string with the payload of
+#   the CPER.
+#
+# Since: 9.2
+##
+{ 'struct': 'CommonPlatformErrorRecord',
+  'data': {
+      'notification-type': 'str',
+      'raw-data': 'str'
+  }
+}
+
+##
+# @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..c1a267399fe5 100644
--- a/qapi/qapi-schema.json
+++ b/qapi/qapi-schema.json
@@ -75,6 +75,7 @@
 { 'include': 'misc-target.json' }
 { 'include': 'audio.json' }
 { 'include': 'acpi.json' }
+{ 'include': 'ghes-cper.json' }
 { 'include': 'pci.json' }
 { 'include': 'stats.json' }
 { 'include': 'virtio.json' }
-- 
2.45.2
Re: [PATCH v5 5/7] qapi/ghes-cper: add an interface to do generic CPER error injection
Posted by Markus Armbruster 3 months, 2 weeks ago 
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> writes:

> Creates a QMP command to be used for generic ACPI APEI hardware error
> injection (HEST) via GHESv2.
>
> 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      | 45 ++++++++++++++++++++++++++++++++
>  hw/acpi/ghes_cper_stub.c | 18 +++++++++++++
>  hw/acpi/meson.build      |  2 ++
>  hw/arm/Kconfig           |  5 ++++
>  include/hw/acpi/ghes.h   |  7 +++++
>  qapi/ghes-cper.json      | 55 ++++++++++++++++++++++++++++++++++++++++
>  qapi/meson.build         |  1 +
>  qapi/qapi-schema.json    |  1 +
>  10 files changed, 146 insertions(+)
>  create mode 100644 hw/acpi/ghes_cper.c
>  create mode 100644 hw/acpi/ghes_cper_stub.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
> +

Here's the reason for creating a new QAPI module instead of adding to
existing module acpi.json: different maintainers.

Hypothetical question: if we didn't care for that, would this go into
qapi/acpi.json?

If yes, then should we call it acpi-ghes-cper.json or acpi-ghes.json
instead?

>  ppc4xx
>  L: qemu-ppc@nongnu.org
>  S: Orphan

[...]

> diff --git a/qapi/ghes-cper.json b/qapi/ghes-cper.json
> new file mode 100644
> index 000000000000..3cc4f9f2aaa9
> --- /dev/null
> +++ b/qapi/ghes-cper.json
> @@ -0,0 +1,55 @@
> +# -*- 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)
> +##

Feels a bit terse.  These what?

The reference could be clearer: "defined in the ACPI Specification 6.2,
section 18.3.2.8 Generic Hardware Error Source version 2".  A link would
be nice, if it's stable.

> +
> +##
> +# @CommonPlatformErrorRecord:
> +#
> +# Common Platform Error Record - CPER - as defined at the UEFI
> +# specification.  See
> +# https://uefi.org/specs/UEFI/2.10/Apx_N_Common_Platform_Error_Record.html#record-header
> +# for more details.
> +#
> +# @notification-type: pre-assigned GUID string 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

Please indent four spaces for consistency, like this:

   # @notification-type: pre-assigned GUID string 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 a base64 encoded string with the payload of
> +#   the CPER.

Suggest

  # @raw-data: payload of the CPER encoded in base64

Have you considered naming this @payload instead?

> +#
> +# Since: 9.2
> +##
> +{ 'struct': 'CommonPlatformErrorRecord',
> +  'data': {
> +      'notification-type': 'str',
> +      'raw-data': 'str'
> +  }
> +}
> +
> +##
> +# @ghes-cper:
> +#
> +# Inject ARM Processor error with data to be filled according with
> +# ACPI 6.2 GHESv2 spec.

according to

(Beware, I'm not a native speaker)

> +#
> +# @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..c1a267399fe5 100644
> --- a/qapi/qapi-schema.json
> +++ b/qapi/qapi-schema.json
> @@ -75,6 +75,7 @@
>  { 'include': 'misc-target.json' }
>  { 'include': 'audio.json' }
>  { 'include': 'acpi.json' }
> +{ 'include': 'ghes-cper.json' }
>  { 'include': 'pci.json' }
>  { 'include': 'stats.json' }
>  { 'include': 'virtio.json' }
Re: [PATCH v5 5/7] qapi/ghes-cper: add an interface to do generic CPER error injection
Posted by Mauro Carvalho Chehab 3 months, 2 weeks ago 
Em Thu, 08 Aug 2024 10:50:33 +0200
Markus Armbruster <armbru@redhat.com> escreveu:

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

> > 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
> > +  
> 
> Here's the reason for creating a new QAPI module instead of adding to
> existing module acpi.json: different maintainers.
> 
> Hypothetical question: if we didn't care for that, would this go into
> qapi/acpi.json?

Independently of maintainers, GHES is part of ACPI APEI HEST, meaning
to report hardware errors. Such hardware errors are typically handled by 
the host OS, so quest doesn't need to be aware of that[1].

So, IMO the best would be to keep APEI/HEST/GHES in a separate file.

[1] still, I can foresee some scenarios were passing some errors to the
    guest could make sense.

> 
> If yes, then should we call it acpi-ghes-cper.json or acpi-ghes.json
> instead?

Naming it as acpi-ghes,acpi-hest or acpi-ghes-cper would equally work
from my side.

> 
> >  ppc4xx
> >  L: qemu-ppc@nongnu.org
> >  S: Orphan  
> 
> [...]
> 
> > diff --git a/qapi/ghes-cper.json b/qapi/ghes-cper.json
> > new file mode 100644
> > index 000000000000..3cc4f9f2aaa9
> > --- /dev/null
> > +++ b/qapi/ghes-cper.json
> > @@ -0,0 +1,55 @@
> > +# -*- 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)
> > +##  
> 
> Feels a bit terse.  These what?
> 
> The reference could be clearer: "defined in the ACPI Specification 6.2,
> section 18.3.2.8 Generic Hardware Error Source version 2".  A link would
> be nice, if it's stable.

I can add a link, but only newer ACPI versions are hosted in html format
(e. g. only versions 6.4 and 6.5 are available as html at uefi.org).

Can I place something like:

	Defined since ACPI Specification 6.2,
	section 18.3.2.8 Generic Hardware Error Source version 2. See:

	https://uefi.org/specs/ACPI/6.5/18_Platform_Error_Interfaces.html#generic-hardware-error-source-version-2-ghesv2-type-10

e. g. having the link pointing to ACPI 6.4 or 6.5, instead of 6.2?

>   # @raw-data: payload of the CPER encoded in base64
> 
> Have you considered naming this @payload instead?

Works for me.

Thanks,
Mauro
Re: [PATCH v5 5/7] qapi/ghes-cper: add an interface to do generic CPER error injection
Posted by Igor Mammedov 3 months, 2 weeks ago 
On Thu, 8 Aug 2024 16:11:41 +0200
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote:

> Em Thu, 08 Aug 2024 10:50:33 +0200
> Markus Armbruster <armbru@redhat.com> escreveu:
> 
> > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> writes:  
> 
> > > 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
> > > +    
> > 
> > Here's the reason for creating a new QAPI module instead of adding to
> > existing module acpi.json: different maintainers.
> > 
> > Hypothetical question: if we didn't care for that, would this go into
> > qapi/acpi.json?  
> 
> Independently of maintainers, GHES is part of ACPI APEI HEST, meaning
> to report hardware errors. Such hardware errors are typically handled by 
> the host OS, so quest doesn't need to be aware of that[1].
> 
> So, IMO the best would be to keep APEI/HEST/GHES in a separate file.
> 
> [1] still, I can foresee some scenarios were passing some errors to the
>     guest could make sense.
> 
> > 
> > If yes, then should we call it acpi-ghes-cper.json or acpi-ghes.json
> > instead?  
> 
> Naming it as acpi-ghes,acpi-hest or acpi-ghes-cper would equally work
> from my side.

if we going to keep it generic, acpi-hest would do

> 
> >   
> > >  ppc4xx
> > >  L: qemu-ppc@nongnu.org
> > >  S: Orphan    
> > 
> > [...]
> >   
> > > diff --git a/qapi/ghes-cper.json b/qapi/ghes-cper.json
> > > new file mode 100644
> > > index 000000000000..3cc4f9f2aaa9
> > > --- /dev/null
> > > +++ b/qapi/ghes-cper.json
> > > @@ -0,0 +1,55 @@
> > > +# -*- 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)
> > > +##    
> > 
> > Feels a bit terse.  These what?
> > 
> > The reference could be clearer: "defined in the ACPI Specification 6.2,
> > section 18.3.2.8 Generic Hardware Error Source version 2".  A link would
> > be nice, if it's stable.  
> 
> I can add a link, but only newer ACPI versions are hosted in html format
> (e. g. only versions 6.4 and 6.5 are available as html at uefi.org).

some years earlier it could be said 'stable link' about acpi spec hosted
elsewhere. Not the case anymore after umbrella change.

spec name, rev, chapter worked fine for acpi code (it's easy to find wherever spec is hosted).
Probably the same would work for QAPI, I'm not QAPI maintainer though,
so preffered approach here is absolutely up to you.

> 
> Can I place something like:
> 
> 	Defined since ACPI Specification 6.2,
> 	section 18.3.2.8 Generic Hardware Error Source version 2. See:
> 
> 	https://uefi.org/specs/ACPI/6.5/18_Platform_Error_Interfaces.html#generic-hardware-error-source-version-2-ghesv2-type-10
> 
> e. g. having the link pointing to ACPI 6.4 or 6.5, instead of 6.2?
> 
> >   # @raw-data: payload of the CPER encoded in base64
> > 
> > Have you considered naming this @payload instead?  
> 
> Works for me.
> 
> Thanks,
> Mauro
>
Re: [PATCH v5 5/7] qapi/ghes-cper: add an interface to do generic CPER error injection
Posted by Markus Armbruster 3 months, 2 weeks ago 
Igor Mammedov <imammedo@redhat.com> writes:

> On Thu, 8 Aug 2024 16:11:41 +0200
> Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote:
>
>> Em Thu, 08 Aug 2024 10:50:33 +0200
>> Markus Armbruster <armbru@redhat.com> escreveu:
>> 
>> > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> writes:  
>> 
>> > > 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
>> > > +    
>> > 
>> > Here's the reason for creating a new QAPI module instead of adding to
>> > existing module acpi.json: different maintainers.
>> > 
>> > Hypothetical question: if we didn't care for that, would this go into
>> > qapi/acpi.json?  
>> 
>> Independently of maintainers, GHES is part of ACPI APEI HEST, meaning
>> to report hardware errors. Such hardware errors are typically handled by 
>> the host OS, so quest doesn't need to be aware of that[1].
>> 
>> So, IMO the best would be to keep APEI/HEST/GHES in a separate file.
>> 
>> [1] still, I can foresee some scenarios were passing some errors to the
>>     guest could make sense.
>> 
>> > 
>> > If yes, then should we call it acpi-ghes-cper.json or acpi-ghes.json
>> > instead?  
>> 
>> Naming it as acpi-ghes,acpi-hest or acpi-ghes-cper would equally work
>> from my side.
>
> if we going to keep it generic, acpi-hest would do

Works for me.

>> > >  ppc4xx
>> > >  L: qemu-ppc@nongnu.org
>> > >  S: Orphan    
>> > 
>> > [...]
>> >   
>> > > diff --git a/qapi/ghes-cper.json b/qapi/ghes-cper.json
>> > > new file mode 100644
>> > > index 000000000000..3cc4f9f2aaa9
>> > > --- /dev/null
>> > > +++ b/qapi/ghes-cper.json
>> > > @@ -0,0 +1,55 @@
>> > > +# -*- 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)
>> > > +##    
>> > 
>> > Feels a bit terse.  These what?
>> > 
>> > The reference could be clearer: "defined in the ACPI Specification 6.2,
>> > section 18.3.2.8 Generic Hardware Error Source version 2".  A link would
>> > be nice, if it's stable.  
>> 
>> I can add a link, but only newer ACPI versions are hosted in html format
>> (e. g. only versions 6.4 and 6.5 are available as html at uefi.org).
>
> some years earlier it could be said 'stable link' about acpi spec hosted
> elsewhere. Not the case anymore after umbrella change.
>
> spec name, rev, chapter worked fine for acpi code (it's easy to find wherever spec is hosted).
> Probably the same would work for QAPI, I'm not QAPI maintainer though,
> so preffered approach here is absolutely up to you.

A link is strictly optional.  Stable links are nice, stale links are
annoying.  Mauro, you decide :)

Thanks!

[...]
Re: [PATCH v5 5/7] qapi/ghes-cper: add an interface to do generic CPER error injection
Posted by Mauro Carvalho Chehab 3 months, 2 weeks ago 
Em Thu, 08 Aug 2024 16:45:51 +0200
Markus Armbruster <armbru@redhat.com> escreveu:

> Igor Mammedov <imammedo@redhat.com> writes:
> 
> > On Thu, 8 Aug 2024 16:11:41 +0200
> > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote:
> >  
> >> Em Thu, 08 Aug 2024 10:50:33 +0200
> >> Markus Armbruster <armbru@redhat.com> escreveu:
> >>   
> >> > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> writes:    
> >>   
> >> > > 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
> >> > > +      
> >> > 
> >> > Here's the reason for creating a new QAPI module instead of adding to
> >> > existing module acpi.json: different maintainers.
> >> > 
> >> > Hypothetical question: if we didn't care for that, would this go into
> >> > qapi/acpi.json?    
> >> 
> >> Independently of maintainers, GHES is part of ACPI APEI HEST, meaning
> >> to report hardware errors. Such hardware errors are typically handled by 
> >> the host OS, so quest doesn't need to be aware of that[1].
> >> 
> >> So, IMO the best would be to keep APEI/HEST/GHES in a separate file.
> >> 
> >> [1] still, I can foresee some scenarios were passing some errors to the
> >>     guest could make sense.
> >>   
> >> > 
> >> > If yes, then should we call it acpi-ghes-cper.json or acpi-ghes.json
> >> > instead?    
> >> 
> >> Naming it as acpi-ghes,acpi-hest or acpi-ghes-cper would equally work
> >> from my side.  
> >
> > if we going to keep it generic, acpi-hest would do  
> 
> Works for me.

Ok, I'll do the rename. With regards to the files implementing
support for it: 

	hw/acpi/ghes_cper.c
	hw/acpi/ghes_cper_stub.c

I guess there's no need to rename them, right? IMO such names
are better than acpi/hest.c, specially since the actual implementation
for HEST is inside acpi/ghes.c.

> 
> >> > >  ppc4xx
> >> > >  L: qemu-ppc@nongnu.org
> >> > >  S: Orphan      
> >> > 
> >> > [...]
> >> >     
> >> > > diff --git a/qapi/ghes-cper.json b/qapi/ghes-cper.json
> >> > > new file mode 100644
> >> > > index 000000000000..3cc4f9f2aaa9
> >> > > --- /dev/null
> >> > > +++ b/qapi/ghes-cper.json
> >> > > @@ -0,0 +1,55 @@
> >> > > +# -*- 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)
> >> > > +##      
> >> > 
> >> > Feels a bit terse.  These what?
> >> > 
> >> > The reference could be clearer: "defined in the ACPI Specification 6.2,
> >> > section 18.3.2.8 Generic Hardware Error Source version 2".  A link would
> >> > be nice, if it's stable.    
> >> 
> >> I can add a link, but only newer ACPI versions are hosted in html format
> >> (e. g. only versions 6.4 and 6.5 are available as html at uefi.org).  
> >
> > some years earlier it could be said 'stable link' about acpi spec hosted
> > elsewhere. Not the case anymore after umbrella change.
> >
> > spec name, rev, chapter worked fine for acpi code (it's easy to find wherever spec is hosted).
> > Probably the same would work for QAPI, I'm not QAPI maintainer though,
> > so preffered approach here is absolutely up to you.  
> 
> A link is strictly optional.  Stable links are nice, stale links are
> annoying.  Mauro, you decide :)

Well, I guess I'll add a link then, keeping it in text mode as well.

Changing umbrella is something that doesn't happen too often. Hopefully 
those will stay for a long time, if not forever, under uefi.org. 

If not, we can always drop the link.

Thanks,
Mauro
Re: [PATCH v5 5/7] qapi/ghes-cper: add an interface to do generic CPER error injection
Posted by Igor Mammedov 3 months, 2 weeks ago 
On Fri,  2 Aug 2024 23:44:00 +0200
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.
> 
> 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      | 45 ++++++++++++++++++++++++++++++++
>  hw/acpi/ghes_cper_stub.c | 18 +++++++++++++
>  hw/acpi/meson.build      |  2 ++
>  hw/arm/Kconfig           |  5 ++++
>  include/hw/acpi/ghes.h   |  7 +++++
>  qapi/ghes-cper.json      | 55 ++++++++++++++++++++++++++++++++++++++++
>  qapi/meson.build         |  1 +
>  qapi/qapi-schema.json    |  1 +
>  10 files changed, 146 insertions(+)
>  create mode 100644 hw/acpi/ghes_cper.c
>  create mode 100644 hw/acpi/ghes_cper_stub.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/Kconfig b/hw/acpi/Kconfig
> index e07d3204eb36..73ffbb82c150 100644
> --- a/hw/acpi/Kconfig
> +++ b/hw/acpi/Kconfig
> @@ -51,6 +51,11 @@ config ACPI_APEI
>      bool
>      depends on ACPI
>  
> +config GHES_CPER
> +    bool
> +    depends on ACPI_APEI
> +    default y
> +
>  config ACPI_PCI
>      bool
>      depends on ACPI && PCI
> diff --git a/hw/acpi/ghes_cper.c b/hw/acpi/ghes_cper.c
> new file mode 100644
> index 000000000000..7aa7e71e90dc
> --- /dev/null
> +++ b/hw/acpi/ghes_cper.c
> @@ -0,0 +1,45 @@
> +/*
> + * ARM Processor 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-ghes-cper.h"
> +#include "hw/acpi/ghes.h"
> +
> +void qmp_ghes_cper(CommonPlatformErrorRecord *qmp_cper,
> +                   Error **errp)
> +{
> +    int rc;
> +    AcpiGhesCper cper;
> +    QemuUUID be_uuid, le_uuid;
> +
> +    rc = qemu_uuid_parse(qmp_cper->notification_type, &be_uuid);
> +    if (rc) {
> +        error_setg(errp, "GHES: Invalid UUID: %s",
> +                   qmp_cper->notification_type);
> +        return;
> +    }
> +
> +    le_uuid = qemu_uuid_bswap(be_uuid);
> +    cper.guid = le_uuid.data;
> +
> +    cper.data = qbase64_decode(qmp_cper->raw_data, -1,
> +                               &cper.data_len, errp);
> +    if (!cper.data) {
> +        return;
> +    }
> +
> +    /* TODO: call a function at ghes */
> +
> +    g_free(cper.data);
> +}
> diff --git a/hw/acpi/ghes_cper_stub.c b/hw/acpi/ghes_cper_stub.c
> new file mode 100644
> index 000000000000..7ce6ed70a265
> --- /dev/null
> +++ b/hw/acpi/ghes_cper_stub.c
> @@ -0,0 +1,18 @@
> +/*
> + * ARM Processor 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 "qapi/error.h"
> +#include "qapi/qapi-commands-ghes-cper.h"
> +#include "hw/acpi/ghes.h"
> +
> +void qmp_ghes_cper(CommonPlatformErrorRecord *cper, Error **errp)
> +{
> +}
> 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'))
>  system_ss.add(files('acpi-qmp-cmds.c'))
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 1ad60da7aa2d..bed6ba27d715 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -712,3 +712,8 @@ config ARMSSE
>      select UNIMP
>      select SSE_COUNTER
>      select SSE_TIMER
> +
> +config GHES_CPER
> +    bool
> +    depends on ARM
> +    default y if AARCH64
> diff --git a/include/hw/acpi/ghes.h b/include/hw/acpi/ghes.h
> index 33be1eb5acf4..06a5b8820cd5 100644
> --- a/include/hw/acpi/ghes.h
> +++ b/include/hw/acpi/ghes.h
> @@ -23,6 +23,7 @@
>  #define ACPI_GHES_H
>  
>  #include "hw/acpi/bios-linker-loader.h"
> +#include "qapi/error.h"
>  #include "qemu/notify.h"
>  
>  extern NotifierList generic_error_notifiers;
> @@ -78,6 +79,12 @@ void acpi_ghes_add_fw_cfg(AcpiGhesState *vms, FWCfgState *s,
>                            GArray *hardware_errors);
>  int acpi_ghes_record_errors(uint8_t notify, uint64_t error_physical_addr);
>  
> +typedef struct AcpiGhesCper {
> +    uint8_t *guid;
> +    uint8_t *data;
> +    size_t data_len;
> +} AcpiGhesCper;
> +
>  /**
>   * acpi_ghes_present: Report whether ACPI GHES table is present
>   *
> diff --git a/qapi/ghes-cper.json b/qapi/ghes-cper.json
> new file mode 100644
> index 000000000000..3cc4f9f2aaa9
> --- /dev/null
> +++ b/qapi/ghes-cper.json
> @@ -0,0 +1,55 @@
> +# -*- 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
> +# https://uefi.org/specs/UEFI/2.10/Apx_N_Common_Platform_Error_Record.html#record-header
> +# for more details.
> +#
> +# @notification-type: pre-assigned GUID string 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 a base64 encoded string with the payload of
> +#   the CPER.
> +#
> +# Since: 9.2
> +##
> +{ 'struct': 'CommonPlatformErrorRecord',
> +  'data': {

> +      'notification-type': 'str',

this should be source id (type is just impl. detail of how QEMU delivers
event for given source id)
unless there is no plan to use more sources,
I'd just drop this from API to avoid confusing user.

Since the patch comes before 5/7, it's not clear how it will be used at this point.
I'd move the patch after 5/7.

> +      'raw-data': 'str'
> +  }
> +}
> +
> +##
> +# @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..c1a267399fe5 100644
> --- a/qapi/qapi-schema.json
> +++ b/qapi/qapi-schema.json
> @@ -75,6 +75,7 @@
>  { 'include': 'misc-target.json' }
>  { 'include': 'audio.json' }
>  { 'include': 'acpi.json' }
> +{ 'include': 'ghes-cper.json' }
>  { 'include': 'pci.json' }
>  { 'include': 'stats.json' }
>  { 'include': 'virtio.json' }
Re: [PATCH v5 5/7] qapi/ghes-cper: add an interface to do generic CPER error injection
Posted by Mauro Carvalho Chehab 3 months, 2 weeks ago 
Em Tue, 6 Aug 2024 14:51:53 +0200
Igor Mammedov <imammedo@redhat.com> escreveu:

> > +{ 'struct': 'CommonPlatformErrorRecord',
> > +  'data': {  
> 
> > +      'notification-type': 'str',  
> 
> this should be source id (type is just impl. detail of how QEMU delivers
> event for given source id)
> unless there is no plan to use more sources,
> I'd just drop this from API to avoid confusing user.
> 
> Since the patch comes before 5/7, it's not clear how it will be used at this point.
> I'd move the patch after 5/7.

As described at:

> +# @notification-type: pre-assigned GUID string 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

This is actually GUID of the error to be generated. Perhaps the better would
be to change the above to:

	{ 'struct': 'CommonPlatformErrorRecord',
	  'data': {
		'guid': 'str',
		'raw-data': 'str'
	}

Making it even clearer. In any case, this is mandatory, as otherwise
the interface would be limited to a single type.

Thanks,
Mauro
RE: [PATCH v5 5/7] qapi/ghes-cper: add an interface to do generic CPER error injection
Posted by Shiju Jose 3 months, 2 weeks ago 

>-----Original Message-----
>From: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
>Sent: 02 August 2024 22:44
>Cc: Jonathan Cameron <jonathan.cameron@huawei.com>; Shiju Jose
><shiju.jose@huawei.com>; Mauro Carvalho Chehab
><mchehab+huawei@kernel.org>; Michael S. Tsirkin <mst@redhat.com>; Ani
>Sinha <anisinha@redhat.com>; Dongjiu Geng <gengdongjiu1@gmail.com>; Eric
>Blake <eblake@redhat.com>; Igor Mammedov <imammedo@redhat.com>;
>Markus Armbruster <armbru@redhat.com>; Michael Roth
><michael.roth@amd.com>; Paolo Bonzini <pbonzini@redhat.com>; Peter
>Maydell <peter.maydell@linaro.org>; linux-kernel@vger.kernel.org; qemu-
>arm@nongnu.org; qemu-devel@nongnu.org
>Subject: [PATCH v5 5/7] qapi/ghes-cper: add an interface to do generic CPER
>error injection
>
>Creates a QMP command to be used for generic ACPI APEI hardware error
>injection (HEST) via GHESv2.
>
>The actual GHES code will be added at the followup patch.
>
>Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Few minor comments inline.
Signed-off-by: Shiju Jose <shiju.jose@huawei.com>

>---
> MAINTAINERS              |  7 +++++
> hw/acpi/Kconfig          |  5 ++++
> hw/acpi/ghes_cper.c      | 45 ++++++++++++++++++++++++++++++++
> hw/acpi/ghes_cper_stub.c | 18 +++++++++++++
> hw/acpi/meson.build      |  2 ++
> hw/arm/Kconfig           |  5 ++++
> include/hw/acpi/ghes.h   |  7 +++++
> qapi/ghes-cper.json      | 55 ++++++++++++++++++++++++++++++++++++++++
> qapi/meson.build         |  1 +
> qapi/qapi-schema.json    |  1 +
> 10 files changed, 146 insertions(+)
> create mode 100644 hw/acpi/ghes_cper.c
> create mode 100644 hw/acpi/ghes_cper_stub.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/Kconfig b/hw/acpi/Kconfig index
>e07d3204eb36..73ffbb82c150 100644
>--- a/hw/acpi/Kconfig
>+++ b/hw/acpi/Kconfig
>@@ -51,6 +51,11 @@ config ACPI_APEI
>     bool
>     depends on ACPI
>
>+config GHES_CPER
>+    bool
>+    depends on ACPI_APEI
>+    default y
>+
> config ACPI_PCI
>     bool
>     depends on ACPI && PCI
>diff --git a/hw/acpi/ghes_cper.c b/hw/acpi/ghes_cper.c new file mode 100644
>index 000000000000..7aa7e71e90dc
>--- /dev/null
>+++ b/hw/acpi/ghes_cper.c
>@@ -0,0 +1,45 @@
>+/*
>+ * ARM Processor 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-ghes-cper.h"
>+#include "hw/acpi/ghes.h"
>+
>+void qmp_ghes_cper(CommonPlatformErrorRecord *qmp_cper,
>+                   Error **errp)
>+{
>+    int rc;
>+    AcpiGhesCper cper;
>+    QemuUUID be_uuid, le_uuid;
>+
>+    rc = qemu_uuid_parse(qmp_cper->notification_type, &be_uuid);
>+    if (rc) {
>+        error_setg(errp, "GHES: Invalid UUID: %s",
>+                   qmp_cper->notification_type);
>+        return;
>+    }
>+
>+    le_uuid = qemu_uuid_bswap(be_uuid);
>+    cper.guid = le_uuid.data;
>+
>+    cper.data = qbase64_decode(qmp_cper->raw_data, -1,
>+                               &cper.data_len, errp);
>+    if (!cper.data) {
>+        return;
>+    }
>+
>+    /* TODO: call a function at ghes */
>+
>+    g_free(cper.data);
>+}
>diff --git a/hw/acpi/ghes_cper_stub.c b/hw/acpi/ghes_cper_stub.c new file
>mode 100644 index 000000000000..7ce6ed70a265
>--- /dev/null
>+++ b/hw/acpi/ghes_cper_stub.c
>@@ -0,0 +1,18 @@
>+/*
>+ * ARM Processor 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 "qapi/error.h"
>+#include "qapi/qapi-commands-ghes-cper.h"
>+#include "hw/acpi/ghes.h"
>+
>+void qmp_ghes_cper(CommonPlatformErrorRecord *cper, Error **errp) { }
May be add an unsupported or similar log in the stub function? 

>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'))
> system_ss.add(files('acpi-qmp-cmds.c'))
>diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig index
>1ad60da7aa2d..bed6ba27d715 100644
>--- a/hw/arm/Kconfig
>+++ b/hw/arm/Kconfig
>@@ -712,3 +712,8 @@ config ARMSSE
>     select UNIMP
>     select SSE_COUNTER
>     select SSE_TIMER
>+
>+config GHES_CPER
>+    bool
>+    depends on ARM
>+    default y if AARCH64
>diff --git a/include/hw/acpi/ghes.h b/include/hw/acpi/ghes.h index
>33be1eb5acf4..06a5b8820cd5 100644
>--- a/include/hw/acpi/ghes.h
>+++ b/include/hw/acpi/ghes.h
>@@ -23,6 +23,7 @@
> #define ACPI_GHES_H
>
> #include "hw/acpi/bios-linker-loader.h"
>+#include "qapi/error.h"
> #include "qemu/notify.h"
>
> extern NotifierList generic_error_notifiers; @@ -78,6 +79,12 @@ void
>acpi_ghes_add_fw_cfg(AcpiGhesState *vms, FWCfgState *s,
>                           GArray *hardware_errors);  int
>acpi_ghes_record_errors(uint8_t notify, uint64_t error_physical_addr);
>
>+typedef struct AcpiGhesCper {
>+    uint8_t *guid;
>+    uint8_t *data;
>+    size_t data_len;
>+} AcpiGhesCper;
>+
> /**
>  * acpi_ghes_present: Report whether ACPI GHES table is present
>  *
>diff --git a/qapi/ghes-cper.json b/qapi/ghes-cper.json new file mode 100644
>index 000000000000..3cc4f9f2aaa9
>--- /dev/null
>+++ b/qapi/ghes-cper.json
>@@ -0,0 +1,55 @@
>+# -*- 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 #
>+https://uefi.org/specs/UEFI/2.10/Apx_N_Common_Platform_Error_Record.htm
>+l#record-header
>+# for more details.
>+#
>+# @notification-type: pre-assigned GUID string 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 a base64 encoded string with the payload of
>+#   the CPER.
>+#
>+# Since: 9.2
>+##
>+{ 'struct': 'CommonPlatformErrorRecord',
>+  'data': {
>+      'notification-type': 'str',
>+      'raw-data': 'str'
>+  }
>+}
>+
>+##
>+# @ghes-cper:
>+#
>+# Inject ARM Processor error with data to be filled according with #
>+ACPI 6.2 GHESv2 spec.
 Since ghes-cper is  generic interface, mentioning term "ARM Processor error" here may not be appropriate?  

>+#
>+# @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..c1a267399fe5 100644
>--- a/qapi/qapi-schema.json
>+++ b/qapi/qapi-schema.json
>@@ -75,6 +75,7 @@
> { 'include': 'misc-target.json' }
> { 'include': 'audio.json' }
> { 'include': 'acpi.json' }
>+{ 'include': 'ghes-cper.json' }
> { 'include': 'pci.json' }
> { 'include': 'stats.json' }
> { 'include': 'virtio.json' }
>--
>2.45.2

Thanks,
Shiju
Re: [PATCH v5 5/7] qapi/ghes-cper: add an interface to do generic CPER error injection
Posted by Jonathan Cameron 3 months, 2 weeks ago 
On Fri,  2 Aug 2024 23:44:00 +0200
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.
> 
> The actual GHES code will be added at the followup patch.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Looks good to me.
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.