| Message ID | 5dfb5fb31afa249e06e0f849b37a7cb525f81215.1722634602.git.mchehab+huawei@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | Add ACPI CPER firmware first error injection on ARM emulation | expand |
Em Mon, 5 Aug 2024 17:39:46 +0100 Jonathan Cameron <Jonathan.Cameron@Huawei.com> escreveu: > On Fri, 2 Aug 2024 23:43:57 +0200 > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: > > > Adds a Generic Event Device to handle generic hardware error > > events, supporting General Purpose Event (GPE) 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 bios 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> > > Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > Much nicer with a GED event. > Happy to give SoB on this as you requested due to changes. > > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > One minor comment though. > The pnp0c33 device isn't technically coupled to the generic_event_device. > Perhaps that should be in aml_build.h/.c instead of where you > have it here? > > Maybe we can move it later though if anyone implements non GED signalling? I opted to place it there at hw/acpi/generic_event_device.c, just after PNP0C0C, e. g.: void acpi_dsdt_add_power_button(Aml *scope) { Aml *dev = aml_device(ACPI_POWER_BUTTON_DEVICE); aml_append(dev, aml_name_decl("_HID", aml_string("PNP0C0C"))); aml_append(dev, aml_name_decl("_UID", aml_int(0))); aml_append(scope, dev); } void acpi_dsdt_add_error_device(Aml *scope) { 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))); aml_append(dev, aml_name_decl("_STA", aml_int(0xF))); aml_append(scope, dev); } IMO this way it will be kept closer to other PNP devices. If this starts to grow, then some later cleanup could move those to some separate file, but, as now there are just two, I would just keep both there at GED file. > > Jonathan > > > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > > --- > > hw/acpi/generic_event_device.c | 17 +++++++++++++++++ > > include/hw/acpi/acpi_dev_interface.h | 1 + > > include/hw/acpi/generic_event_device.h | 3 +++ > > 3 files changed, 21 insertions(+) > > > > diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c > > index 15b4c3ebbf24..b9ad05e98c05 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"), > > @@ -153,6 +159,15 @@ void acpi_dsdt_add_power_button(Aml *scope) > > aml_append(scope, dev); > > } > > > > +void acpi_dsdt_add_error_device(Aml *scope) > > +{ > > + 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))); > > + aml_append(dev, aml_name_decl("_STA", aml_int(0xF))); > > + aml_append(scope, dev); > > +} > > + > > /* Memory read by the GED _EVT AML dynamic method */ > > static uint64_t ged_evt_read(void *opaque, hwaddr addr, unsigned size) > > { > > @@ -295,6 +310,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/generic_event_device.h b/include/hw/acpi/generic_event_device.h > > index 40af3550b56d..b8f2f1328e0c 100644 > > --- a/include/hw/acpi/generic_event_device.h > > +++ b/include/hw/acpi/generic_event_device.h > > @@ -66,6 +66,7 @@ > > #include "qom/object.h" > > > > #define ACPI_POWER_BUTTON_DEVICE "PWRB" > > +#define ACPI_APEI_ERROR_DEVICE "GEDD" > > > > #define TYPE_ACPI_GED "acpi-ged" > > OBJECT_DECLARE_SIMPLE_TYPE(AcpiGedState, ACPI_GED) > > @@ -98,6 +99,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; > > @@ -120,5 +122,6 @@ struct AcpiGedState { > > void build_ged_aml(Aml *table, const char* name, HotplugHandler *hotplug_dev, > > uint32_t ged_irq, AmlRegionSpace rs, hwaddr ged_base); > > void acpi_dsdt_add_power_button(Aml *scope); > > +void acpi_dsdt_add_error_device(Aml *scope); > > > > #endif > Thanks, Mauro
On Fri, 2 Aug 2024 23:43:57 +0200 Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: subj: s/APEI/error/ to match spec > Adds a Generic Event Device to handle generic hardware error ^^^^^ Did you want to say: Error ? > events, supporting General Purpose Event (GPE) as specified at even though GPE can be used (for example with non hw-reduced pc/q35 machines), it's not what you are doing here. s/General Purpose Event (GPE)/generic event device/ > 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 bios via ACPI APEI Generic Hardware Error Source (GHES). event is sent not to 'bios' but to the guest, OSPM in spec language > > Co-authored-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > Co-authored-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > --- > hw/acpi/generic_event_device.c | 17 +++++++++++++++++ > include/hw/acpi/acpi_dev_interface.h | 1 + > include/hw/acpi/generic_event_device.h | 3 +++ > 3 files changed, 21 insertions(+) > > diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c > index 15b4c3ebbf24..b9ad05e98c05 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"), > @@ -153,6 +159,15 @@ void acpi_dsdt_add_power_button(Aml *scope) > aml_append(scope, dev); > } put mandatory comment here, in format: earliest spec rev + chapter > +void acpi_dsdt_add_error_device(Aml *scope) s/void acpi_dsdt_add_error_device/Aml* aml_error_device()/ > +{ > + 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))); > + aml_append(dev, aml_name_decl("_STA", aml_int(0xF))); not necessary unless you want set it anything else beside 0xF > + aml_append(scope, dev); > +} and maybe move the function to aml-build.c > /* Memory read by the GED _EVT AML dynamic method */ > static uint64_t ged_evt_read(void *opaque, hwaddr addr, unsigned size) > { > @@ -295,6 +310,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/generic_event_device.h b/include/hw/acpi/generic_event_device.h > index 40af3550b56d..b8f2f1328e0c 100644 > --- a/include/hw/acpi/generic_event_device.h > +++ b/include/hw/acpi/generic_event_device.h > @@ -66,6 +66,7 @@ > #include "qom/object.h" > > #define ACPI_POWER_BUTTON_DEVICE "PWRB" > +#define ACPI_APEI_ERROR_DEVICE "GEDD" perhaps aml_build.h would be a better place (if you consider using it with pc/q35 machines) > #define TYPE_ACPI_GED "acpi-ged" > OBJECT_DECLARE_SIMPLE_TYPE(AcpiGedState, ACPI_GED) > @@ -98,6 +99,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; > @@ -120,5 +122,6 @@ struct AcpiGedState { > void build_ged_aml(Aml *table, const char* name, HotplugHandler *hotplug_dev, > uint32_t ged_irq, AmlRegionSpace rs, hwaddr ged_base); > void acpi_dsdt_add_power_button(Aml *scope); > +void acpi_dsdt_add_error_device(Aml *scope); > > #endif
diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c index 15b4c3ebbf24..b9ad05e98c05 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"), @@ -153,6 +159,15 @@ void acpi_dsdt_add_power_button(Aml *scope) aml_append(scope, dev); } +void acpi_dsdt_add_error_device(Aml *scope) +{ + 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))); + aml_append(dev, aml_name_decl("_STA", aml_int(0xF))); + aml_append(scope, dev); +} + /* Memory read by the GED _EVT AML dynamic method */ static uint64_t ged_evt_read(void *opaque, hwaddr addr, unsigned size) { @@ -295,6 +310,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/generic_event_device.h b/include/hw/acpi/generic_event_device.h index 40af3550b56d..b8f2f1328e0c 100644 --- a/include/hw/acpi/generic_event_device.h +++ b/include/hw/acpi/generic_event_device.h @@ -66,6 +66,7 @@ #include "qom/object.h" #define ACPI_POWER_BUTTON_DEVICE "PWRB" +#define ACPI_APEI_ERROR_DEVICE "GEDD" #define TYPE_ACPI_GED "acpi-ged" OBJECT_DECLARE_SIMPLE_TYPE(AcpiGedState, ACPI_GED) @@ -98,6 +99,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; @@ -120,5 +122,6 @@ struct AcpiGedState { void build_ged_aml(Aml *table, const char* name, HotplugHandler *hotplug_dev, uint32_t ged_irq, AmlRegionSpace rs, hwaddr ged_base); void acpi_dsdt_add_power_button(Aml *scope); +void acpi_dsdt_add_error_device(Aml *scope); #endif
Adds a Generic Event Device to handle generic hardware error events, supporting General Purpose Event (GPE) 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 bios 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> Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> --- hw/acpi/generic_event_device.c | 17 +++++++++++++++++ include/hw/acpi/acpi_dev_interface.h | 1 + include/hw/acpi/generic_event_device.h | 3 +++ 3 files changed, 21 insertions(+)