[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20240806105448.7177f6b1@imammedo.users.ipa.redhat.com>
Date: Tue, 6 Aug 2024 10:54:48 +0200
From: Igor Mammedov <imammedo@...hat.com>
To: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
Cc: Jonathan Cameron <Jonathan.Cameron@...wei.com>, Shiju Jose
<shiju.jose@...wei.com>, "Michael S. Tsirkin" <mst@...hat.com>, Ani Sinha
<anisinha@...hat.com>, linux-kernel@...r.kernel.org, qemu-devel@...gnu.org
Subject: Re: [PATCH v5 2/7] acpi/generic_event_device: add an APEI error
device
On Fri, 2 Aug 2024 23:43:57 +0200
Mauro Carvalho Chehab <mchehab+huawei@...nel.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@...nel.org>
> Co-authored-by: Jonathan Cameron <Jonathan.Cameron@...wei.com>
> Cc: Jonathan Cameron <Jonathan.Cameron@...wei.com>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@...nel.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
Powered by blists - more mailing lists