| Message ID | 034a7e86761e09996001394c98ffb8201ac52cd2.1721630625.git.mchehab+huawei@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add ACPI CPER firmware first error injection for Arm Processor | expand |
On Mon, 22 Jul 2024 08:45:54 +0200 Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: > From: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > Creates a GED - Generic Event Device and set a GPIO to The wonder of confusing names in ACPI. I thought I'd fixed this but clearly not. This GED isn't a Generic Event Device, it's a Generic Error (Device) as per 18.3.2.7.2 Event Notification for Generic Error Sources in ACPI 6.5 PNP0C33 vs Generic Event Device which is unrelated :( and has ID ACPI0013 This one is a bit of ACPI glue logic. https://lore.kernel.org/lkml/1272350481-27951-8-git-send-email-ying.huang@intel.com/ which is the kernel patch provides a bit more info. In kernel world this is called Hardware Error Device I guess to avoid tripping over GED naming? Maybe we are better using that here - so HED_* > be used or error injection. This should probably shout a bit more about the set_error callback added to struct MachineClass Whilst that works I suspect it will be controversial and I'd like to hear other suggestions on how to provide a convenient hook to signal that we've put an event in the firmware buffer where that signaling can come from pretty much any hw device emulation in QEMU. Jonathan > > [mchehab: use a define for the generic event pin number and do some cleanups] > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > --- > hw/arm/virt-acpi-build.c | 30 ++++++++++++++++++++++++++---- > hw/arm/virt.c | 14 ++++++++++++-- > include/hw/arm/virt.h | 1 + > include/hw/boards.h | 1 + > 4 files changed, 40 insertions(+), 6 deletions(-) > > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c > index f76fb117adff..c502ccf40909 100644 > --- a/hw/arm/virt-acpi-build.c > +++ b/hw/arm/virt-acpi-build.c > @@ -63,6 +63,7 @@ > > #define ARM_SPI_BASE 32 > > +#define ACPI_GENERIC_EVENT_DEVICE "GEDD" > #define ACPI_BUILD_TABLE_SIZE 0x20000 > > static void acpi_dsdt_add_cpus(Aml *scope, VirtMachineState *vms) > @@ -142,6 +143,8 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, > static void acpi_dsdt_add_gpio(Aml *scope, const MemMapEntry *gpio_memmap, > uint32_t gpio_irq) > { > + uint32_t pin; > + > Aml *dev = aml_device("GPO0"); > aml_append(dev, aml_name_decl("_HID", aml_string("ARMH0061"))); > aml_append(dev, aml_name_decl("_UID", aml_int(0))); > @@ -155,7 +158,12 @@ static void acpi_dsdt_add_gpio(Aml *scope, const MemMapEntry *gpio_memmap, > > Aml *aei = aml_resource_template(); > > - const uint32_t pin = GPIO_PIN_POWER_BUTTON; > + pin = GPIO_PIN_POWER_BUTTON; > + aml_append(aei, aml_gpio_int(AML_CONSUMER, AML_EDGE, AML_ACTIVE_HIGH, > + AML_EXCLUSIVE, AML_PULL_UP, 0, &pin, 1, > + "GPO0", NULL, 0)); > + /* Pin for generic error */ > + pin = GPIO_PIN_GENERIC_ERROR; > aml_append(aei, aml_gpio_int(AML_CONSUMER, AML_EDGE, AML_ACTIVE_HIGH, > AML_EXCLUSIVE, AML_PULL_UP, 0, &pin, 1, > "GPO0", NULL, 0)); > @@ -166,6 +174,11 @@ static void acpi_dsdt_add_gpio(Aml *scope, const MemMapEntry *gpio_memmap, > aml_append(method, aml_notify(aml_name(ACPI_POWER_BUTTON_DEVICE), > aml_int(0x80))); > aml_append(dev, method); > + method = aml_method("_E06", 0, AML_NOTSERIALIZED); > + aml_append(method, aml_notify(aml_name(ACPI_GENERIC_EVENT_DEVICE), > + aml_int(0x80))); > + aml_append(dev, method); > + > aml_append(scope, dev); > } > > @@ -800,6 +813,15 @@ static void build_fadt_rev6(GArray *table_data, BIOSLinker *linker, > build_fadt(table_data, linker, &fadt, vms->oem_id, vms->oem_table_id); > } > > +static void acpi_dsdt_add_generic_event_device(Aml *scope) > +{ > + Aml *dev = aml_device(ACPI_GENERIC_EVENT_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); > +} > + > /* DSDT */ > static void > build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > @@ -841,10 +863,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > HOTPLUG_HANDLER(vms->acpi_dev), > irqmap[VIRT_ACPI_GED] + ARM_SPI_BASE, AML_SYSTEM_MEMORY, > memmap[VIRT_ACPI_GED].base); > - } else { > - acpi_dsdt_add_gpio(scope, &memmap[VIRT_GPIO], > - (irqmap[VIRT_GPIO] + ARM_SPI_BASE)); > } > + acpi_dsdt_add_gpio(scope, &memmap[VIRT_GPIO], > + (irqmap[VIRT_GPIO] + ARM_SPI_BASE)); > > if (vms->acpi_dev) { > uint32_t event = object_property_get_uint(OBJECT(vms->acpi_dev), > @@ -858,6 +879,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > } > > acpi_dsdt_add_power_button(scope); > + acpi_dsdt_add_generic_event_device(scope); > #ifdef CONFIG_TPM > acpi_dsdt_add_tpm(scope, vms); > #endif > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index c99c8b1713c6..f81cf3a69961 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -997,6 +997,13 @@ static void create_rtc(const VirtMachineState *vms) > } > > static DeviceState *gpio_key_dev; > + > +static DeviceState *gpio_error_dev; > +static void virt_set_error(void) > +{ > + qemu_set_irq(qdev_get_gpio_in(gpio_error_dev, 0), 1); > +} > + > static void virt_powerdown_req(Notifier *n, void *opaque) > { > VirtMachineState *s = container_of(n, VirtMachineState, powerdown_notifier); > @@ -1015,6 +1022,9 @@ static void create_gpio_keys(char *fdt, DeviceState *pl061_dev, > gpio_key_dev = sysbus_create_simple("gpio-key", -1, > qdev_get_gpio_in(pl061_dev, > GPIO_PIN_POWER_BUTTON)); > + gpio_error_dev = sysbus_create_simple("gpio-key", -1, > + qdev_get_gpio_in(pl061_dev, > + GPIO_PIN_GENERIC_ERROR)); > > qemu_fdt_add_subnode(fdt, "/gpio-keys"); > qemu_fdt_setprop_string(fdt, "/gpio-keys", "compatible", "gpio-keys"); > @@ -2385,9 +2395,8 @@ static void machvirt_init(MachineState *machine) > > if (has_ged && aarch64 && firmware_loaded && virt_is_acpi_enabled(vms)) { > vms->acpi_dev = create_acpi_ged(vms); > - } else { > - create_gpio_devices(vms, VIRT_GPIO, sysmem); > } > + create_gpio_devices(vms, VIRT_GPIO, sysmem); > > if (vms->secure && !vmc->no_secure_gpio) { > create_gpio_devices(vms, VIRT_SECURE_GPIO, secure_sysmem); > @@ -3101,6 +3110,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data) > mc->default_ram_id = "mach-virt.ram"; > mc->default_nic = "virtio-net-pci"; > > + mc->set_error = virt_set_error; > object_class_property_add(oc, "acpi", "OnOffAuto", > virt_get_acpi, virt_set_acpi, > NULL, NULL); > diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h > index a4d937ed45ac..c9769d7d4d7f 100644 > --- a/include/hw/arm/virt.h > +++ b/include/hw/arm/virt.h > @@ -49,6 +49,7 @@ > > /* GPIO pins */ > #define GPIO_PIN_POWER_BUTTON 3 > +#define GPIO_PIN_GENERIC_ERROR 6 > > enum { > VIRT_FLASH, > diff --git a/include/hw/boards.h b/include/hw/boards.h > index ef6f18f2c1a7..6cf01f3934ae 100644 > --- a/include/hw/boards.h > +++ b/include/hw/boards.h > @@ -304,6 +304,7 @@ struct MachineClass { > const CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine); > int64_t (*get_default_cpu_node_id)(const MachineState *ms, int idx); > ram_addr_t (*fixup_ram_size)(ram_addr_t size); > + void (*set_error)(void); > }; > > /**
On Mon, 22 Jul 2024 08:45:54 +0200 Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: > From: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > Creates a GED - Generic Event Device and set a GPIO to > be used or error injection. QEMU already has GED device, so question is why it wasn't used for event delivery? I nutshell, I'd really prefer this series being rewritten to reuse exiting GED instead of adding ad hoc GPIO and ACPI plumbing. PS: as side effect of that, error injection could be used no only for ARM but other machines that use GED (providing they implement GHES) Also CCing Shameer wrt touched power button code > [mchehab: use a define for the generic event pin number and do some cleanups] > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > --- > hw/arm/virt-acpi-build.c | 30 ++++++++++++++++++++++++++---- > hw/arm/virt.c | 14 ++++++++++++-- > include/hw/arm/virt.h | 1 + > include/hw/boards.h | 1 + > 4 files changed, 40 insertions(+), 6 deletions(-) > > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c > index f76fb117adff..c502ccf40909 100644 > --- a/hw/arm/virt-acpi-build.c > +++ b/hw/arm/virt-acpi-build.c > @@ -63,6 +63,7 @@ > > #define ARM_SPI_BASE 32 > > +#define ACPI_GENERIC_EVENT_DEVICE "GEDD" > #define ACPI_BUILD_TABLE_SIZE 0x20000 > > static void acpi_dsdt_add_cpus(Aml *scope, VirtMachineState *vms) > @@ -142,6 +143,8 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, > static void acpi_dsdt_add_gpio(Aml *scope, const MemMapEntry *gpio_memmap, > uint32_t gpio_irq) this function supposed to be called when acpi_dev is not present (exiting GED device) and run on old machines only, so it should not be called for recent machine types. I'd avoid adding anything to it. see more comment about it below > { > + uint32_t pin; > + > Aml *dev = aml_device("GPO0"); > aml_append(dev, aml_name_decl("_HID", aml_string("ARMH0061"))); > aml_append(dev, aml_name_decl("_UID", aml_int(0))); > @@ -155,7 +158,12 @@ static void acpi_dsdt_add_gpio(Aml *scope, const MemMapEntry *gpio_memmap, > > Aml *aei = aml_resource_template(); > > - const uint32_t pin = GPIO_PIN_POWER_BUTTON; > + pin = GPIO_PIN_POWER_BUTTON; > + aml_append(aei, aml_gpio_int(AML_CONSUMER, AML_EDGE, AML_ACTIVE_HIGH, > + AML_EXCLUSIVE, AML_PULL_UP, 0, &pin, 1, > + "GPO0", NULL, 0)); > + /* Pin for generic error */ > + pin = GPIO_PIN_GENERIC_ERROR; > aml_append(aei, aml_gpio_int(AML_CONSUMER, AML_EDGE, AML_ACTIVE_HIGH, > AML_EXCLUSIVE, AML_PULL_UP, 0, &pin, 1, > "GPO0", NULL, 0)); > @@ -166,6 +174,11 @@ static void acpi_dsdt_add_gpio(Aml *scope, const MemMapEntry *gpio_memmap, > aml_append(method, aml_notify(aml_name(ACPI_POWER_BUTTON_DEVICE), > aml_int(0x80))); > aml_append(dev, method); > + method = aml_method("_E06", 0, AML_NOTSERIALIZED); > + aml_append(method, aml_notify(aml_name(ACPI_GENERIC_EVENT_DEVICE), > + aml_int(0x80))); > + aml_append(dev, method); > + > aml_append(scope, dev); > } > > @@ -800,6 +813,15 @@ static void build_fadt_rev6(GArray *table_data, BIOSLinker *linker, > build_fadt(table_data, linker, &fadt, vms->oem_id, vms->oem_table_id); > } > > +static void acpi_dsdt_add_generic_event_device(Aml *scope) > +{ > + Aml *dev = aml_device(ACPI_GENERIC_EVENT_DEVICE); > + aml_append(dev, aml_name_decl("_HID", aml_string("PNP0C33"))); this is not _event_ device, it's referred as _error_ device in spec. PS: please properly document new ACPI primitives/devices, see comment above aml_notify() for example. Use earliest APIC spec where the device was defined for the 1st time. > + aml_append(dev, aml_name_decl("_UID", aml_int(0))); > + aml_append(dev, aml_name_decl("_STA", aml_int(0xF))); > + aml_append(scope, dev); > +} > + > /* DSDT */ > static void > build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > @@ -841,10 +863,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > HOTPLUG_HANDLER(vms->acpi_dev), > irqmap[VIRT_ACPI_GED] + ARM_SPI_BASE, AML_SYSTEM_MEMORY, > memmap[VIRT_ACPI_GED].base); > - } else { > - acpi_dsdt_add_gpio(scope, &memmap[VIRT_GPIO], > - (irqmap[VIRT_GPIO] + ARM_SPI_BASE)); > } > + acpi_dsdt_add_gpio(scope, &memmap[VIRT_GPIO], > + (irqmap[VIRT_GPIO] + ARM_SPI_BASE)); wouldn't that create double/conflicting power button handlers (GPIO and GED one), on recent machine types GED should be used and power button in acpi_dsdt_add_gpio() is used only if machine doesn't have GED. > > if (vms->acpi_dev) { > uint32_t event = object_property_get_uint(OBJECT(vms->acpi_dev), > @@ -858,6 +879,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > } > > acpi_dsdt_add_power_button(scope); > + acpi_dsdt_add_generic_event_device(scope); > #ifdef CONFIG_TPM > acpi_dsdt_add_tpm(scope, vms); > #endif > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index c99c8b1713c6..f81cf3a69961 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -997,6 +997,13 @@ static void create_rtc(const VirtMachineState *vms) > } > > static DeviceState *gpio_key_dev; > + > +static DeviceState *gpio_error_dev; > +static void virt_set_error(void) > +{ > + qemu_set_irq(qdev_get_gpio_in(gpio_error_dev, 0), 1); > +} > + > static void virt_powerdown_req(Notifier *n, void *opaque) > { > VirtMachineState *s = container_of(n, VirtMachineState, powerdown_notifier); > @@ -1015,6 +1022,9 @@ static void create_gpio_keys(char *fdt, DeviceState *pl061_dev, > gpio_key_dev = sysbus_create_simple("gpio-key", -1, > qdev_get_gpio_in(pl061_dev, > GPIO_PIN_POWER_BUTTON)); > + gpio_error_dev = sysbus_create_simple("gpio-key", -1, > + qdev_get_gpio_in(pl061_dev, > + GPIO_PIN_GENERIC_ERROR)); > > qemu_fdt_add_subnode(fdt, "/gpio-keys"); > qemu_fdt_setprop_string(fdt, "/gpio-keys", "compatible", "gpio-keys"); > @@ -2385,9 +2395,8 @@ static void machvirt_init(MachineState *machine) > > if (has_ged && aarch64 && firmware_loaded && virt_is_acpi_enabled(vms)) { > vms->acpi_dev = create_acpi_ged(vms); > - } else { > - create_gpio_devices(vms, VIRT_GPIO, sysmem); > } > + create_gpio_devices(vms, VIRT_GPIO, sysmem); again, this create duplicate/conflicting power button source > > if (vms->secure && !vmc->no_secure_gpio) { > create_gpio_devices(vms, VIRT_SECURE_GPIO, secure_sysmem); > @@ -3101,6 +3110,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data) > mc->default_ram_id = "mach-virt.ram"; > mc->default_nic = "virtio-net-pci"; > > + mc->set_error = virt_set_error; > object_class_property_add(oc, "acpi", "OnOffAuto", > virt_get_acpi, virt_set_acpi, > NULL, NULL); > diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h > index a4d937ed45ac..c9769d7d4d7f 100644 > --- a/include/hw/arm/virt.h > +++ b/include/hw/arm/virt.h > @@ -49,6 +49,7 @@ > > /* GPIO pins */ > #define GPIO_PIN_POWER_BUTTON 3 > +#define GPIO_PIN_GENERIC_ERROR 6 > > enum { > VIRT_FLASH, > diff --git a/include/hw/boards.h b/include/hw/boards.h > index ef6f18f2c1a7..6cf01f3934ae 100644 > --- a/include/hw/boards.h > +++ b/include/hw/boards.h > @@ -304,6 +304,7 @@ struct MachineClass { > const CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine); > int64_t (*get_default_cpu_node_id)(const MachineState *ms, int idx); > ram_addr_t (*fixup_ram_size)(ram_addr_t size); > + void (*set_error)(void); > }; > > /**
Em Tue, 30 Jul 2024 10:36:15 +0200 Igor Mammedov <imammedo@redhat.com> escreveu: > On Mon, 22 Jul 2024 08:45:54 +0200 > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote: > > > From: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > > > Creates a GED - Generic Event Device and set a GPIO to > > be used or error injection. > > QEMU already has GED device, so question is why it wasn't > used for event delivery? > I nutshell, I'd really prefer this series being rewritten > to reuse exiting GED instead of adding ad hoc GPIO and ACPI > plumbing. Makes sense. I'll split this one on two patches, the first one adding the error device PNP to acpi/generic_event_device, and the second one with ghes and arm virt changes to support it, using a notifier list inside ghes to signalize the error events. Jonathan, As the logic will be different, I'm placing you as co-author, and adding you as Cc on the patches. If you're ok with that, please reply with your SoB to them when I submit the next patch series. > PS: > as side effect of that, error injection could be used no only for > ARM but other machines that use GED (providing they implement GHES) > > Also CCing Shameer wrt touched power button code > > > [mchehab: use a define for the generic event pin number and do some cleanups] > > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > > --- > > hw/arm/virt-acpi-build.c | 30 ++++++++++++++++++++++++++---- > > hw/arm/virt.c | 14 ++++++++++++-- > > include/hw/arm/virt.h | 1 + > > include/hw/boards.h | 1 + > > 4 files changed, 40 insertions(+), 6 deletions(-) > > > > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c > > index f76fb117adff..c502ccf40909 100644 > > --- a/hw/arm/virt-acpi-build.c > > +++ b/hw/arm/virt-acpi-build.c > > @@ -63,6 +63,7 @@ > > > > #define ARM_SPI_BASE 32 > > > > +#define ACPI_GENERIC_EVENT_DEVICE "GEDD" > > #define ACPI_BUILD_TABLE_SIZE 0x20000 > > > > static void acpi_dsdt_add_cpus(Aml *scope, VirtMachineState *vms) > > @@ -142,6 +143,8 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, > > static void acpi_dsdt_add_gpio(Aml *scope, const MemMapEntry *gpio_memmap, > > uint32_t gpio_irq) > > this function supposed to be called when acpi_dev is not present (exiting GED device) > and run on old machines only, so it should not be called for recent machine types. > I'd avoid adding anything to it. > > see more comment about it below > > > { > > + uint32_t pin; > > + > > Aml *dev = aml_device("GPO0"); > > aml_append(dev, aml_name_decl("_HID", aml_string("ARMH0061"))); > > aml_append(dev, aml_name_decl("_UID", aml_int(0))); > > @@ -155,7 +158,12 @@ static void acpi_dsdt_add_gpio(Aml *scope, const MemMapEntry *gpio_memmap, > > > > Aml *aei = aml_resource_template(); > > > > - const uint32_t pin = GPIO_PIN_POWER_BUTTON; > > + pin = GPIO_PIN_POWER_BUTTON; > > + aml_append(aei, aml_gpio_int(AML_CONSUMER, AML_EDGE, AML_ACTIVE_HIGH, > > + AML_EXCLUSIVE, AML_PULL_UP, 0, &pin, 1, > > + "GPO0", NULL, 0)); > > + /* Pin for generic error */ > > + pin = GPIO_PIN_GENERIC_ERROR; > > aml_append(aei, aml_gpio_int(AML_CONSUMER, AML_EDGE, AML_ACTIVE_HIGH, > > AML_EXCLUSIVE, AML_PULL_UP, 0, &pin, 1, > > "GPO0", NULL, 0)); > > @@ -166,6 +174,11 @@ static void acpi_dsdt_add_gpio(Aml *scope, const MemMapEntry *gpio_memmap, > > aml_append(method, aml_notify(aml_name(ACPI_POWER_BUTTON_DEVICE), > > aml_int(0x80))); > > aml_append(dev, method); > > + method = aml_method("_E06", 0, AML_NOTSERIALIZED); > > + aml_append(method, aml_notify(aml_name(ACPI_GENERIC_EVENT_DEVICE), > > + aml_int(0x80))); > > + aml_append(dev, method); > > + > > aml_append(scope, dev); > > } > > > > @@ -800,6 +813,15 @@ static void build_fadt_rev6(GArray *table_data, BIOSLinker *linker, > > build_fadt(table_data, linker, &fadt, vms->oem_id, vms->oem_table_id); > > } > > > > +static void acpi_dsdt_add_generic_event_device(Aml *scope) > > +{ > > + Aml *dev = aml_device(ACPI_GENERIC_EVENT_DEVICE); > > + aml_append(dev, aml_name_decl("_HID", aml_string("PNP0C33"))); > this is not _event_ device, it's referred as _error_ device in spec. > > PS: > please properly document new ACPI primitives/devices, > see comment above aml_notify() for example. > Use earliest APIC spec where the device was defined for the 1st time. > > > + aml_append(dev, aml_name_decl("_UID", aml_int(0))); > > + aml_append(dev, aml_name_decl("_STA", aml_int(0xF))); > > + aml_append(scope, dev); > > +} > > + > > /* DSDT */ > > static void > > build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > > @@ -841,10 +863,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > > HOTPLUG_HANDLER(vms->acpi_dev), > > irqmap[VIRT_ACPI_GED] + ARM_SPI_BASE, AML_SYSTEM_MEMORY, > > memmap[VIRT_ACPI_GED].base); > > - } else { > > - acpi_dsdt_add_gpio(scope, &memmap[VIRT_GPIO], > > - (irqmap[VIRT_GPIO] + ARM_SPI_BASE)); > > } > > + acpi_dsdt_add_gpio(scope, &memmap[VIRT_GPIO], > > + (irqmap[VIRT_GPIO] + ARM_SPI_BASE)); > > wouldn't that create double/conflicting power button handlers > (GPIO and GED one), on recent machine types GED should be used > and power button in acpi_dsdt_add_gpio() is used only if > machine doesn't have GED. > > > > > if (vms->acpi_dev) { > > uint32_t event = object_property_get_uint(OBJECT(vms->acpi_dev), > > @@ -858,6 +879,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > > } > > > > acpi_dsdt_add_power_button(scope); > > + acpi_dsdt_add_generic_event_device(scope); > > #ifdef CONFIG_TPM > > acpi_dsdt_add_tpm(scope, vms); > > #endif > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > > index c99c8b1713c6..f81cf3a69961 100644 > > --- a/hw/arm/virt.c > > +++ b/hw/arm/virt.c > > @@ -997,6 +997,13 @@ static void create_rtc(const VirtMachineState *vms) > > } > > > > static DeviceState *gpio_key_dev; > > + > > +static DeviceState *gpio_error_dev; > > +static void virt_set_error(void) > > +{ > > + qemu_set_irq(qdev_get_gpio_in(gpio_error_dev, 0), 1); > > +} > > + > > static void virt_powerdown_req(Notifier *n, void *opaque) > > { > > VirtMachineState *s = container_of(n, VirtMachineState, powerdown_notifier); > > @@ -1015,6 +1022,9 @@ static void create_gpio_keys(char *fdt, DeviceState *pl061_dev, > > gpio_key_dev = sysbus_create_simple("gpio-key", -1, > > qdev_get_gpio_in(pl061_dev, > > GPIO_PIN_POWER_BUTTON)); > > + gpio_error_dev = sysbus_create_simple("gpio-key", -1, > > + qdev_get_gpio_in(pl061_dev, > > + GPIO_PIN_GENERIC_ERROR)); > > > > qemu_fdt_add_subnode(fdt, "/gpio-keys"); > > qemu_fdt_setprop_string(fdt, "/gpio-keys", "compatible", "gpio-keys"); > > @@ -2385,9 +2395,8 @@ static void machvirt_init(MachineState *machine) > > > > if (has_ged && aarch64 && firmware_loaded && virt_is_acpi_enabled(vms)) { > > vms->acpi_dev = create_acpi_ged(vms); > > - } else { > > - create_gpio_devices(vms, VIRT_GPIO, sysmem); > > } > > + create_gpio_devices(vms, VIRT_GPIO, sysmem); > > again, this create duplicate/conflicting power button source > > > > > if (vms->secure && !vmc->no_secure_gpio) { > > create_gpio_devices(vms, VIRT_SECURE_GPIO, secure_sysmem); > > @@ -3101,6 +3110,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data) > > mc->default_ram_id = "mach-virt.ram"; > > mc->default_nic = "virtio-net-pci"; > > > > + mc->set_error = virt_set_error; > > object_class_property_add(oc, "acpi", "OnOffAuto", > > virt_get_acpi, virt_set_acpi, > > NULL, NULL); > > diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h > > index a4d937ed45ac..c9769d7d4d7f 100644 > > --- a/include/hw/arm/virt.h > > +++ b/include/hw/arm/virt.h > > @@ -49,6 +49,7 @@ > > > > /* GPIO pins */ > > #define GPIO_PIN_POWER_BUTTON 3 > > +#define GPIO_PIN_GENERIC_ERROR 6 > > > > enum { > > VIRT_FLASH, > > diff --git a/include/hw/boards.h b/include/hw/boards.h > > index ef6f18f2c1a7..6cf01f3934ae 100644 > > --- a/include/hw/boards.h > > +++ b/include/hw/boards.h > > @@ -304,6 +304,7 @@ struct MachineClass { > > const CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine); > > int64_t (*get_default_cpu_node_id)(const MachineState *ms, int idx); > > ram_addr_t (*fixup_ram_size)(ram_addr_t size); > > + void (*set_error)(void); > > }; > > > > /** > Thanks, Mauro
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index f76fb117adff..c502ccf40909 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -63,6 +63,7 @@ #define ARM_SPI_BASE 32 +#define ACPI_GENERIC_EVENT_DEVICE "GEDD" #define ACPI_BUILD_TABLE_SIZE 0x20000 static void acpi_dsdt_add_cpus(Aml *scope, VirtMachineState *vms) @@ -142,6 +143,8 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, static void acpi_dsdt_add_gpio(Aml *scope, const MemMapEntry *gpio_memmap, uint32_t gpio_irq) { + uint32_t pin; + Aml *dev = aml_device("GPO0"); aml_append(dev, aml_name_decl("_HID", aml_string("ARMH0061"))); aml_append(dev, aml_name_decl("_UID", aml_int(0))); @@ -155,7 +158,12 @@ static void acpi_dsdt_add_gpio(Aml *scope, const MemMapEntry *gpio_memmap, Aml *aei = aml_resource_template(); - const uint32_t pin = GPIO_PIN_POWER_BUTTON; + pin = GPIO_PIN_POWER_BUTTON; + aml_append(aei, aml_gpio_int(AML_CONSUMER, AML_EDGE, AML_ACTIVE_HIGH, + AML_EXCLUSIVE, AML_PULL_UP, 0, &pin, 1, + "GPO0", NULL, 0)); + /* Pin for generic error */ + pin = GPIO_PIN_GENERIC_ERROR; aml_append(aei, aml_gpio_int(AML_CONSUMER, AML_EDGE, AML_ACTIVE_HIGH, AML_EXCLUSIVE, AML_PULL_UP, 0, &pin, 1, "GPO0", NULL, 0)); @@ -166,6 +174,11 @@ static void acpi_dsdt_add_gpio(Aml *scope, const MemMapEntry *gpio_memmap, aml_append(method, aml_notify(aml_name(ACPI_POWER_BUTTON_DEVICE), aml_int(0x80))); aml_append(dev, method); + method = aml_method("_E06", 0, AML_NOTSERIALIZED); + aml_append(method, aml_notify(aml_name(ACPI_GENERIC_EVENT_DEVICE), + aml_int(0x80))); + aml_append(dev, method); + aml_append(scope, dev); } @@ -800,6 +813,15 @@ static void build_fadt_rev6(GArray *table_data, BIOSLinker *linker, build_fadt(table_data, linker, &fadt, vms->oem_id, vms->oem_table_id); } +static void acpi_dsdt_add_generic_event_device(Aml *scope) +{ + Aml *dev = aml_device(ACPI_GENERIC_EVENT_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); +} + /* DSDT */ static void build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) @@ -841,10 +863,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) HOTPLUG_HANDLER(vms->acpi_dev), irqmap[VIRT_ACPI_GED] + ARM_SPI_BASE, AML_SYSTEM_MEMORY, memmap[VIRT_ACPI_GED].base); - } else { - acpi_dsdt_add_gpio(scope, &memmap[VIRT_GPIO], - (irqmap[VIRT_GPIO] + ARM_SPI_BASE)); } + acpi_dsdt_add_gpio(scope, &memmap[VIRT_GPIO], + (irqmap[VIRT_GPIO] + ARM_SPI_BASE)); if (vms->acpi_dev) { uint32_t event = object_property_get_uint(OBJECT(vms->acpi_dev), @@ -858,6 +879,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) } acpi_dsdt_add_power_button(scope); + acpi_dsdt_add_generic_event_device(scope); #ifdef CONFIG_TPM acpi_dsdt_add_tpm(scope, vms); #endif diff --git a/hw/arm/virt.c b/hw/arm/virt.c index c99c8b1713c6..f81cf3a69961 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -997,6 +997,13 @@ static void create_rtc(const VirtMachineState *vms) } static DeviceState *gpio_key_dev; + +static DeviceState *gpio_error_dev; +static void virt_set_error(void) +{ + qemu_set_irq(qdev_get_gpio_in(gpio_error_dev, 0), 1); +} + static void virt_powerdown_req(Notifier *n, void *opaque) { VirtMachineState *s = container_of(n, VirtMachineState, powerdown_notifier); @@ -1015,6 +1022,9 @@ static void create_gpio_keys(char *fdt, DeviceState *pl061_dev, gpio_key_dev = sysbus_create_simple("gpio-key", -1, qdev_get_gpio_in(pl061_dev, GPIO_PIN_POWER_BUTTON)); + gpio_error_dev = sysbus_create_simple("gpio-key", -1, + qdev_get_gpio_in(pl061_dev, + GPIO_PIN_GENERIC_ERROR)); qemu_fdt_add_subnode(fdt, "/gpio-keys"); qemu_fdt_setprop_string(fdt, "/gpio-keys", "compatible", "gpio-keys"); @@ -2385,9 +2395,8 @@ static void machvirt_init(MachineState *machine) if (has_ged && aarch64 && firmware_loaded && virt_is_acpi_enabled(vms)) { vms->acpi_dev = create_acpi_ged(vms); - } else { - create_gpio_devices(vms, VIRT_GPIO, sysmem); } + create_gpio_devices(vms, VIRT_GPIO, sysmem); if (vms->secure && !vmc->no_secure_gpio) { create_gpio_devices(vms, VIRT_SECURE_GPIO, secure_sysmem); @@ -3101,6 +3110,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data) mc->default_ram_id = "mach-virt.ram"; mc->default_nic = "virtio-net-pci"; + mc->set_error = virt_set_error; object_class_property_add(oc, "acpi", "OnOffAuto", virt_get_acpi, virt_set_acpi, NULL, NULL); diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h index a4d937ed45ac..c9769d7d4d7f 100644 --- a/include/hw/arm/virt.h +++ b/include/hw/arm/virt.h @@ -49,6 +49,7 @@ /* GPIO pins */ #define GPIO_PIN_POWER_BUTTON 3 +#define GPIO_PIN_GENERIC_ERROR 6 enum { VIRT_FLASH, diff --git a/include/hw/boards.h b/include/hw/boards.h index ef6f18f2c1a7..6cf01f3934ae 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -304,6 +304,7 @@ struct MachineClass { const CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine); int64_t (*get_default_cpu_node_id)(const MachineState *ms, int idx); ram_addr_t (*fixup_ram_size)(ram_addr_t size); + void (*set_error)(void); }; /**