| Message ID | 0be6db8d06b3abab551f24dcc645d46d72d3f668.1723591201.git.mchehab+huawei@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add ACPI CPER firmware first error injection on ARM emulation | expand |
On Wed, Aug 14, 2024 at 01:23:23AM +0200, Mauro Carvalho Chehab wrote: > Adds a generic error device to handle generic hardware error > events as specified at ACPI 6.5 specification at 18.3.2.7.2: > https://uefi.org/specs/ACPI/6.5/18_Platform_Error_Interfaces.html#event-notification-for-generic-error-sources > using HID PNP0C33. > > The PNP0C33 device is used to report hardware errors to > the guest via ACPI APEI Generic Hardware Error Source (GHES). > > Co-authored-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > Co-authored-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > Reviewed-by: Igor Mammedov <imammedo@redhat.com> > --- > hw/acpi/aml-build.c | 10 ++++++++++ > hw/acpi/generic_event_device.c | 8 ++++++++ > include/hw/acpi/acpi_dev_interface.h | 1 + > include/hw/acpi/aml-build.h | 2 ++ > include/hw/acpi/generic_event_device.h | 1 + > 5 files changed, 22 insertions(+) > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c > index 6d4517cfbe3d..cb167523859f 100644 > --- a/hw/acpi/aml-build.c > +++ b/hw/acpi/aml-build.c > @@ -2520,3 +2520,13 @@ Aml *aml_i2c_serial_bus_device(uint16_t address, const char *resource_source) > > return var; > } > + > +/* ACPI 5.0: 18.3.2.6.2 Event Notification For Generic Error Sources */ I could not find Event Notification For Generic Error Sources in ACPI 5.0. Instead, I see 18.3.2.6.2 SCI Notification For Generic Error Sources What did I miss? > +Aml *aml_error_device(void) > +{ > + Aml *dev = aml_device(ACPI_APEI_ERROR_DEVICE); > + aml_append(dev, aml_name_decl("_HID", aml_string("PNP0C33"))); > + aml_append(dev, aml_name_decl("_UID", aml_int(0))); > + comment on why is this here? > + return dev; > +} > diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c > index 15b4c3ebbf24..1673e9695be3 100644 > --- a/hw/acpi/generic_event_device.c > +++ b/hw/acpi/generic_event_device.c > @@ -26,6 +26,7 @@ static const uint32_t ged_supported_events[] = { > ACPI_GED_PWR_DOWN_EVT, > ACPI_GED_NVDIMM_HOTPLUG_EVT, > ACPI_GED_CPU_HOTPLUG_EVT, > + ACPI_GED_ERROR_EVT > }; > > /* > @@ -116,6 +117,11 @@ void build_ged_aml(Aml *table, const char *name, HotplugHandler *hotplug_dev, > aml_notify(aml_name(ACPI_POWER_BUTTON_DEVICE), > aml_int(0x80))); > break; > + case ACPI_GED_ERROR_EVT: > + aml_append(if_ctx, > + aml_notify(aml_name(ACPI_APEI_ERROR_DEVICE), > + aml_int(0x80))); > + break; > case ACPI_GED_NVDIMM_HOTPLUG_EVT: > aml_append(if_ctx, > aml_notify(aml_name("\\_SB.NVDR"), > @@ -295,6 +301,8 @@ static void acpi_ged_send_event(AcpiDeviceIf *adev, AcpiEventStatusBits ev) > sel = ACPI_GED_MEM_HOTPLUG_EVT; > } else if (ev & ACPI_POWER_DOWN_STATUS) { > sel = ACPI_GED_PWR_DOWN_EVT; > + } else if (ev & ACPI_GENERIC_ERROR) { > + sel = ACPI_GED_ERROR_EVT; > } else if (ev & ACPI_NVDIMM_HOTPLUG_STATUS) { > sel = ACPI_GED_NVDIMM_HOTPLUG_EVT; > } else if (ev & ACPI_CPU_HOTPLUG_STATUS) { > diff --git a/include/hw/acpi/acpi_dev_interface.h b/include/hw/acpi/acpi_dev_interface.h > index 68d9d15f50aa..8294f8f0ccca 100644 > --- a/include/hw/acpi/acpi_dev_interface.h > +++ b/include/hw/acpi/acpi_dev_interface.h > @@ -13,6 +13,7 @@ typedef enum { > ACPI_NVDIMM_HOTPLUG_STATUS = 16, > ACPI_VMGENID_CHANGE_STATUS = 32, > ACPI_POWER_DOWN_STATUS = 64, > + ACPI_GENERIC_ERROR = 128, > } AcpiEventStatusBits; > > #define TYPE_ACPI_DEVICE_IF "acpi-device-interface" > diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h > index a3784155cb33..44d1a6af0c69 100644 > --- a/include/hw/acpi/aml-build.h > +++ b/include/hw/acpi/aml-build.h > @@ -252,6 +252,7 @@ struct CrsRangeSet { > /* Consumer/Producer */ > #define AML_SERIAL_BUS_FLAG_CONSUME_ONLY (1 << 1) > > +#define ACPI_APEI_ERROR_DEVICE "GEDD" > /** > * init_aml_allocator: > * > @@ -382,6 +383,7 @@ Aml *aml_dma(AmlDmaType typ, AmlDmaBusMaster bm, AmlTransferSize sz, > uint8_t channel); > Aml *aml_sleep(uint64_t msec); > Aml *aml_i2c_serial_bus_device(uint16_t address, const char *resource_source); > +Aml *aml_error_device(void); > > /* Block AML object primitives */ > Aml *aml_scope(const char *name_format, ...) G_GNUC_PRINTF(1, 2); > diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h > index 40af3550b56d..9ace8fe70328 100644 > --- a/include/hw/acpi/generic_event_device.h > +++ b/include/hw/acpi/generic_event_device.h > @@ -98,6 +98,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(AcpiGedState, ACPI_GED) > #define ACPI_GED_PWR_DOWN_EVT 0x2 > #define ACPI_GED_NVDIMM_HOTPLUG_EVT 0x4 > #define ACPI_GED_CPU_HOTPLUG_EVT 0x8 > +#define ACPI_GED_ERROR_EVT 0x10 > > typedef struct GEDState { > MemoryRegion evt; > -- > 2.46.0
On Wed, 14 Aug 2024 01:23:23 +0200 Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: > Adds a generic error device to handle generic hardware error > events as specified at ACPI 6.5 specification at 18.3.2.7.2: > https://uefi.org/specs/ACPI/6.5/18_Platform_Error_Interfaces.html#event-notification-for-generic-error-sources > using HID PNP0C33. > > The PNP0C33 device is used to report hardware errors to > the guest via ACPI APEI Generic Hardware Error Source (GHES). > > Co-authored-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > Co-authored-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > Reviewed-by: Igor Mammedov <imammedo@redhat.com> > --- > hw/acpi/aml-build.c | 10 ++++++++++ > hw/acpi/generic_event_device.c | 8 ++++++++ > include/hw/acpi/acpi_dev_interface.h | 1 + > include/hw/acpi/aml-build.h | 2 ++ > include/hw/acpi/generic_event_device.h | 1 + > 5 files changed, 22 insertions(+) > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c > index 6d4517cfbe3d..cb167523859f 100644 > --- a/hw/acpi/aml-build.c > +++ b/hw/acpi/aml-build.c > @@ -2520,3 +2520,13 @@ Aml *aml_i2c_serial_bus_device(uint16_t address, const char *resource_source) > > return var; > } > + > +/* ACPI 5.0: 18.3.2.6.2 Event Notification For Generic Error Sources */ Given this section got a rename maybe the comment should mention old name and current name for the section? > +Aml *aml_error_device(void) > +{ > + Aml *dev = aml_device(ACPI_APEI_ERROR_DEVICE); > + aml_append(dev, aml_name_decl("_HID", aml_string("PNP0C33"))); > + aml_append(dev, aml_name_decl("_UID", aml_int(0))); > + > + return dev; > +} > diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c > index 15b4c3ebbf24..1673e9695be3 100644 > --- a/hw/acpi/generic_event_device.c > +++ b/hw/acpi/generic_event_device.c > @@ -26,6 +26,7 @@ static const uint32_t ged_supported_events[] = { > ACPI_GED_PWR_DOWN_EVT, > ACPI_GED_NVDIMM_HOTPLUG_EVT, > ACPI_GED_CPU_HOTPLUG_EVT, > + ACPI_GED_ERROR_EVT trailing comma missing.
Em Wed, 14 Aug 2024 13:33:21 +0100 Jonathan Cameron <Jonathan.Cameron@Huawei.com> escreveu: > On Wed, 14 Aug 2024 01:23:23 +0200 > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: > > > Adds a generic error device to handle generic hardware error > > events as specified at ACPI 6.5 specification at 18.3.2.7.2: > > https://uefi.org/specs/ACPI/6.5/18_Platform_Error_Interfaces.html#event-notification-for-generic-error-sources > > using HID PNP0C33. > > > > The PNP0C33 device is used to report hardware errors to > > the guest via ACPI APEI Generic Hardware Error Source (GHES). > > > > Co-authored-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > > Co-authored-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > > Reviewed-by: Igor Mammedov <imammedo@redhat.com> > > --- > > hw/acpi/aml-build.c | 10 ++++++++++ > > hw/acpi/generic_event_device.c | 8 ++++++++ > > include/hw/acpi/acpi_dev_interface.h | 1 + > > include/hw/acpi/aml-build.h | 2 ++ > > include/hw/acpi/generic_event_device.h | 1 + > > 5 files changed, 22 insertions(+) > > > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c > > index 6d4517cfbe3d..cb167523859f 100644 > > --- a/hw/acpi/aml-build.c > > +++ b/hw/acpi/aml-build.c > > @@ -2520,3 +2520,13 @@ Aml *aml_i2c_serial_bus_device(uint16_t address, const char *resource_source) > > > > return var; > > } > > + > > +/* ACPI 5.0: 18.3.2.6.2 Event Notification For Generic Error Sources */ > > Given this section got a rename maybe the comment should mention old > name and current name for the section? ACPI 6.5 has the same name for the section: 18.3.2.7.2. Event Notification For Generic Error Sources An event notification is recommended for corrected errors where latency in processing error reports is not critical to proper system operation. The implementation of Event notification requires the platform to define a device with PNP ID PNP0C33 in the ACPI namespace, referred to as the error device. Just section number changed. IMO, it is still good enough to seek for it at the docs. Btw, in this specific case, the best is to use the search box of Sphinx html output and seek for PNP0C33 ;-) > > > +Aml *aml_error_device(void) > > +{ > > + Aml *dev = aml_device(ACPI_APEI_ERROR_DEVICE); > > + aml_append(dev, aml_name_decl("_HID", aml_string("PNP0C33"))); > > + aml_append(dev, aml_name_decl("_UID", aml_int(0))); > > + > > + return dev; > > +} > > diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c > > index 15b4c3ebbf24..1673e9695be3 100644 > > --- a/hw/acpi/generic_event_device.c > > +++ b/hw/acpi/generic_event_device.c > > @@ -26,6 +26,7 @@ static const uint32_t ged_supported_events[] = { > > ACPI_GED_PWR_DOWN_EVT, > > ACPI_GED_NVDIMM_HOTPLUG_EVT, > > ACPI_GED_CPU_HOTPLUG_EVT, > > + ACPI_GED_ERROR_EVT > > trailing comma missing. I'll add. Thanks, Mauro
On Fri, 16 Aug 2024 07:53:24 +0200 Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: > Em Wed, 14 Aug 2024 13:33:21 +0100 > Jonathan Cameron <Jonathan.Cameron@Huawei.com> escreveu: > > > On Wed, 14 Aug 2024 01:23:23 +0200 > > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: > > > > > Adds a generic error device to handle generic hardware error > > > events as specified at ACPI 6.5 specification at 18.3.2.7.2: > > > https://uefi.org/specs/ACPI/6.5/18_Platform_Error_Interfaces.html#event-notification-for-generic-error-sources > > > using HID PNP0C33. > > > > > > The PNP0C33 device is used to report hardware errors to > > > the guest via ACPI APEI Generic Hardware Error Source (GHES). > > > > > > Co-authored-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > > > Co-authored-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > > > Reviewed-by: Igor Mammedov <imammedo@redhat.com> > > > --- > > > hw/acpi/aml-build.c | 10 ++++++++++ > > > hw/acpi/generic_event_device.c | 8 ++++++++ > > > include/hw/acpi/acpi_dev_interface.h | 1 + > > > include/hw/acpi/aml-build.h | 2 ++ > > > include/hw/acpi/generic_event_device.h | 1 + > > > 5 files changed, 22 insertions(+) > > > > > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c > > > index 6d4517cfbe3d..cb167523859f 100644 > > > --- a/hw/acpi/aml-build.c > > > +++ b/hw/acpi/aml-build.c > > > @@ -2520,3 +2520,13 @@ Aml *aml_i2c_serial_bus_device(uint16_t address, const char *resource_source) > > > > > > return var; > > > } > > > + > > > +/* ACPI 5.0: 18.3.2.6.2 Event Notification For Generic Error Sources */ > > > > Given this section got a rename maybe the comment should mention old > > name and current name for the section? > > ACPI 6.5 has the same name for the section: We did a bit of digging after an offline discussion. It's only there in Errata B of ACPI 5.0. So need to update this reference to be more specific if we want to avoid confusion. > > 18.3.2.7.2. Event Notification For Generic Error Sources > > An event notification is recommended for corrected errors where latency > in processing error reports is not critical to proper system operation. > The implementation of Event notification requires the platform to define > a device with PNP ID PNP0C33 in the ACPI namespace, referred to as the > error device. > > Just section number changed. IMO, it is still good enough to seek for > it at the docs. > > Btw, in this specific case, the best is to use the search box of > Sphinx html output and seek for PNP0C33 ;-) > > > > > > +Aml *aml_error_device(void) > > > +{ > > > + Aml *dev = aml_device(ACPI_APEI_ERROR_DEVICE); > > > + aml_append(dev, aml_name_decl("_HID", aml_string("PNP0C33"))); > > > + aml_append(dev, aml_name_decl("_UID", aml_int(0))); > > > + > > > + return dev; > > > +} > > > diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c > > > index 15b4c3ebbf24..1673e9695be3 100644 > > > --- a/hw/acpi/generic_event_device.c > > > +++ b/hw/acpi/generic_event_device.c > > > @@ -26,6 +26,7 @@ static const uint32_t ged_supported_events[] = { > > > ACPI_GED_PWR_DOWN_EVT, > > > ACPI_GED_NVDIMM_HOTPLUG_EVT, > > > ACPI_GED_CPU_HOTPLUG_EVT, > > > + ACPI_GED_ERROR_EVT > > > > trailing comma missing. > > I'll add. > > Thanks, > Mauro
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c index 6d4517cfbe3d..cb167523859f 100644 --- a/hw/acpi/aml-build.c +++ b/hw/acpi/aml-build.c @@ -2520,3 +2520,13 @@ Aml *aml_i2c_serial_bus_device(uint16_t address, const char *resource_source) return var; } + +/* ACPI 5.0: 18.3.2.6.2 Event Notification For Generic Error Sources */ +Aml *aml_error_device(void) +{ + Aml *dev = aml_device(ACPI_APEI_ERROR_DEVICE); + aml_append(dev, aml_name_decl("_HID", aml_string("PNP0C33"))); + aml_append(dev, aml_name_decl("_UID", aml_int(0))); + + return dev; +} diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c index 15b4c3ebbf24..1673e9695be3 100644 --- a/hw/acpi/generic_event_device.c +++ b/hw/acpi/generic_event_device.c @@ -26,6 +26,7 @@ static const uint32_t ged_supported_events[] = { ACPI_GED_PWR_DOWN_EVT, ACPI_GED_NVDIMM_HOTPLUG_EVT, ACPI_GED_CPU_HOTPLUG_EVT, + ACPI_GED_ERROR_EVT }; /* @@ -116,6 +117,11 @@ void build_ged_aml(Aml *table, const char *name, HotplugHandler *hotplug_dev, aml_notify(aml_name(ACPI_POWER_BUTTON_DEVICE), aml_int(0x80))); break; + case ACPI_GED_ERROR_EVT: + aml_append(if_ctx, + aml_notify(aml_name(ACPI_APEI_ERROR_DEVICE), + aml_int(0x80))); + break; case ACPI_GED_NVDIMM_HOTPLUG_EVT: aml_append(if_ctx, aml_notify(aml_name("\\_SB.NVDR"), @@ -295,6 +301,8 @@ static void acpi_ged_send_event(AcpiDeviceIf *adev, AcpiEventStatusBits ev) sel = ACPI_GED_MEM_HOTPLUG_EVT; } else if (ev & ACPI_POWER_DOWN_STATUS) { sel = ACPI_GED_PWR_DOWN_EVT; + } else if (ev & ACPI_GENERIC_ERROR) { + sel = ACPI_GED_ERROR_EVT; } else if (ev & ACPI_NVDIMM_HOTPLUG_STATUS) { sel = ACPI_GED_NVDIMM_HOTPLUG_EVT; } else if (ev & ACPI_CPU_HOTPLUG_STATUS) { diff --git a/include/hw/acpi/acpi_dev_interface.h b/include/hw/acpi/acpi_dev_interface.h index 68d9d15f50aa..8294f8f0ccca 100644 --- a/include/hw/acpi/acpi_dev_interface.h +++ b/include/hw/acpi/acpi_dev_interface.h @@ -13,6 +13,7 @@ typedef enum { ACPI_NVDIMM_HOTPLUG_STATUS = 16, ACPI_VMGENID_CHANGE_STATUS = 32, ACPI_POWER_DOWN_STATUS = 64, + ACPI_GENERIC_ERROR = 128, } AcpiEventStatusBits; #define TYPE_ACPI_DEVICE_IF "acpi-device-interface" diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h index a3784155cb33..44d1a6af0c69 100644 --- a/include/hw/acpi/aml-build.h +++ b/include/hw/acpi/aml-build.h @@ -252,6 +252,7 @@ struct CrsRangeSet { /* Consumer/Producer */ #define AML_SERIAL_BUS_FLAG_CONSUME_ONLY (1 << 1) +#define ACPI_APEI_ERROR_DEVICE "GEDD" /** * init_aml_allocator: * @@ -382,6 +383,7 @@ Aml *aml_dma(AmlDmaType typ, AmlDmaBusMaster bm, AmlTransferSize sz, uint8_t channel); Aml *aml_sleep(uint64_t msec); Aml *aml_i2c_serial_bus_device(uint16_t address, const char *resource_source); +Aml *aml_error_device(void); /* Block AML object primitives */ Aml *aml_scope(const char *name_format, ...) G_GNUC_PRINTF(1, 2); diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h index 40af3550b56d..9ace8fe70328 100644 --- a/include/hw/acpi/generic_event_device.h +++ b/include/hw/acpi/generic_event_device.h @@ -98,6 +98,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(AcpiGedState, ACPI_GED) #define ACPI_GED_PWR_DOWN_EVT 0x2 #define ACPI_GED_NVDIMM_HOTPLUG_EVT 0x4 #define ACPI_GED_CPU_HOTPLUG_EVT 0x8 +#define ACPI_GED_ERROR_EVT 0x10 typedef struct GEDState { MemoryRegion evt;