In ARM platform we implements a notification of error events
via a GPIO pin. For this GPIO-signaled events, we choose GPIO
pin 4 for hardware error device (PNP0C33), So _E04 should be
added to ACPI DSDT table. When GPIO-pin 4 signaled a events,
the guest ACPI driver will receive this notification and
handing the error.
In order to better trigger the GPIO IRQ, we defined a notifier
hardware_error_notifiers. If Qemu wants to deliver a GPIO-Signal
notification, will call it.
Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
---
1. Address discussion result about guest APEI notification type for SIGBUS_MCEERR_AO SIGBUS in [1],
the discussion conclusion is using GPIO-Signal
[1]:
https://lists.gnu.org/archive/html/qemu-devel/2017-10/msg03397.html
https://lists.gnu.org/archive/html/qemu-devel/2017-10/msg03467.html
https://lists.gnu.org/archive/html/qemu-devel/2017-10/msg03601.html
https://lists.gnu.org/archive/html/qemu-devel/2017-10/msg03775.html
2. The ASL dump for the GPIO and hardware error device
................
Device (GPO0)
{
Name (_AEI, ResourceTemplate () // _AEI: ACPI Event Interrupts
{
.............
GpioInt (Edge, ActiveHigh, Exclusive, PullUp, 0x0000,
"GPO0", 0x00, ResourceConsumer, ,
)
{ // Pin list
0x0004
}
})
Method (_E04, 0, NotSerialized) // _Exx: Edge-Triggered GPE
{
Notify (ERRD, 0x80) // Status Change
}
}
Device (ERRD)
{
Name (_HID, EisaId ("PNP0C33") /* Error Device */) // _HID: Hardware ID
Name (_UID, Zero) // _UID: Unique ID
Method (_STA, 0, NotSerialized) // _STA: Status
{
Return (0x0F)
}
}
3. Below is the guest log when Qemu notifies guest using GPIO-signal after record a CPER
[ 504.164899] {1}[Hardware Error]: Hardware error from APEI Generic Hardware Error Source: 7
[ 504.166970] {1}[Hardware Error]: event severity: recoverable
[ 504.251650] {1}[Hardware Error]: Error 0, type: recoverable
[ 504.252974] {1}[Hardware Error]: section_type: memory error
[ 504.254380] {1}[Hardware Error]: physical_address: 0x00000000000003ec
[ 504.255879] {1}[Hardware Error]: error_type: 3, multi-bit ECC
---
hw/arm/virt-acpi-build.c | 31 ++++++++++++++++++++++++++++++-
hw/arm/virt.c | 18 ++++++++++++++++++
include/sysemu/sysemu.h | 3 +++
vl.c | 12 ++++++++++++
4 files changed, 63 insertions(+), 1 deletion(-)
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index b7d45cd..06c14b3 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -49,6 +49,7 @@
#define ARM_SPI_BASE 32
#define ACPI_POWER_BUTTON_DEVICE "PWRB"
+#define ACPI_HARDWARE_ERROR_DEVICE "ERRD"
static void acpi_dsdt_add_cpus(Aml *scope, int smp_cpus)
{
@@ -340,7 +341,13 @@ static void acpi_dsdt_add_gpio(Aml *scope, const MemMapEntry *gpio_memmap,
Aml *aei = aml_resource_template();
/* Pin 3 for power button */
- const uint32_t pin_list[1] = {3};
+ uint32_t pin_list[1] = {3};
+ aml_append(aei, aml_gpio_int(AML_CONSUMER, AML_EDGE, AML_ACTIVE_HIGH,
+ AML_EXCLUSIVE, AML_PULL_UP, 0, pin_list, 1,
+ "GPO0", NULL, 0));
+
+ /* Pin 4 for hardware error device */
+ pin_list[0] = 4;
aml_append(aei, aml_gpio_int(AML_CONSUMER, AML_EDGE, AML_ACTIVE_HIGH,
AML_EXCLUSIVE, AML_PULL_UP, 0, pin_list, 1,
"GPO0", NULL, 0));
@@ -351,6 +358,13 @@ 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);
+
+ /* _E04 is handle for hardware error */
+ method = aml_method("_E04", 0, AML_NOTSERIALIZED);
+ aml_append(method, aml_notify(aml_name(ACPI_HARDWARE_ERROR_DEVICE),
+ aml_int(0x80)));
+ aml_append(dev, method);
+
aml_append(scope, dev);
}
@@ -363,6 +377,20 @@ static void acpi_dsdt_add_power_button(Aml *scope)
aml_append(scope, dev);
}
+static void acpi_dsdt_add_error_device(Aml *scope)
+{
+ Aml *dev = aml_device(ACPI_HARDWARE_ERROR_DEVICE);
+ Aml *method;
+
+ aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0C33")));
+ aml_append(dev, aml_name_decl("_UID", aml_int(0)));
+
+ method = aml_method("_STA", 0, AML_NOTSERIALIZED);
+ aml_append(method, aml_return(aml_int(0x0f)));
+ aml_append(dev, method);
+ aml_append(scope, dev);
+}
+
/* RSDP */
static GArray *
build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned xsdt_tbl_offset)
@@ -716,6 +744,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
acpi_dsdt_add_gpio(scope, &memmap[VIRT_GPIO],
(irqmap[VIRT_GPIO] + ARM_SPI_BASE));
acpi_dsdt_add_power_button(scope);
+ acpi_dsdt_add_error_device(scope);
aml_append(dsdt, scope);
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 6b7a0fe..68495c2 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -701,16 +701,27 @@ static void create_rtc(const VirtMachineState *vms, qemu_irq *pic)
}
static DeviceState *gpio_key_dev;
+static DeviceState *gpio_err_dev;
static void virt_powerdown_req(Notifier *n, void *opaque)
{
/* use gpio Pin 3 for power button event */
qemu_set_irq(qdev_get_gpio_in(gpio_key_dev, 0), 1);
}
+static void virt_error_notify_req(Notifier *n, void *opaque)
+{
+ /* use gpio Pin 4 for hardware error event */
+ qemu_set_irq(qdev_get_gpio_in(gpio_err_dev, 0), 1);
+}
+
static Notifier virt_system_powerdown_notifier = {
.notify = virt_powerdown_req
};
+static Notifier virt_hardware_error_notifier = {
+ .notify = virt_error_notify_req
+};
+
static void create_gpio(const VirtMachineState *vms, qemu_irq *pic)
{
char *nodename;
@@ -739,6 +750,10 @@ static void create_gpio(const VirtMachineState *vms, qemu_irq *pic)
gpio_key_dev = sysbus_create_simple("gpio-key", -1,
qdev_get_gpio_in(pl061_dev, 3));
+
+ gpio_err_dev = sysbus_create_simple("gpio-key", -1,
+ qdev_get_gpio_in(pl061_dev, 4));
+
qemu_fdt_add_subnode(vms->fdt, "/gpio-keys");
qemu_fdt_setprop_string(vms->fdt, "/gpio-keys", "compatible", "gpio-keys");
qemu_fdt_setprop_cell(vms->fdt, "/gpio-keys", "#size-cells", 0);
@@ -755,6 +770,9 @@ static void create_gpio(const VirtMachineState *vms, qemu_irq *pic)
/* connect powerdown request */
qemu_register_powerdown_notifier(&virt_system_powerdown_notifier);
+ /* connect hardware error notify request */
+ qemu_register_hardware_error_notifier(&virt_hardware_error_notifier);
+
g_free(nodename);
}
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index b213696..86931cf 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -75,6 +75,7 @@ void qemu_register_wakeup_notifier(Notifier *notifier);
void qemu_system_shutdown_request(ShutdownCause reason);
void qemu_system_powerdown_request(void);
void qemu_register_powerdown_notifier(Notifier *notifier);
+void qemu_register_hardware_error_notifier(Notifier *notifier);
void qemu_system_debug_request(void);
void qemu_system_vmstop_request(RunState reason);
void qemu_system_vmstop_request_prepare(void);
@@ -93,6 +94,8 @@ void qemu_remove_machine_init_done_notifier(Notifier *notify);
void qemu_announce_self(void);
+void qemu_hardware_error_notify(void);
+
extern int autostart;
typedef enum {
diff --git a/vl.c b/vl.c
index d632693..3552f7d 100644
--- a/vl.c
+++ b/vl.c
@@ -1614,6 +1614,8 @@ static int suspend_requested;
static WakeupReason wakeup_reason;
static NotifierList powerdown_notifiers =
NOTIFIER_LIST_INITIALIZER(powerdown_notifiers);
+static NotifierList hardware_error_notifiers =
+ NOTIFIER_LIST_INITIALIZER(hardware_error_notifiers);
static NotifierList suspend_notifiers =
NOTIFIER_LIST_INITIALIZER(suspend_notifiers);
static NotifierList wakeup_notifiers =
@@ -1850,12 +1852,22 @@ void qemu_register_powerdown_notifier(Notifier *notifier)
notifier_list_add(&powerdown_notifiers, notifier);
}
+void qemu_register_hardware_error_notifier(Notifier *notifier)
+{
+ notifier_list_add(&hardware_error_notifiers, notifier);
+}
+
void qemu_system_debug_request(void)
{
debug_requested = 1;
qemu_notify_event();
}
+void qemu_hardware_error_notify(void)
+{
+ notifier_list_notify(&hardware_error_notifiers, NULL);
+}
+
static bool main_loop_should_exit(void)
{
RunState r;
--
1.8.3.1
On Thu, 28 Dec 2017 13:54:16 +0800
Dongjiu Geng <gengdongjiu@huawei.com> wrote:
> In ARM platform we implements a notification of error events
> via a GPIO pin. For this GPIO-signaled events, we choose GPIO
> pin 4 for hardware error device (PNP0C33), So _E04 should be
> added to ACPI DSDT table. When GPIO-pin 4 signaled a events,
> the guest ACPI driver will receive this notification and
> handing the error.
>
> In order to better trigger the GPIO IRQ, we defined a notifier
> hardware_error_notifiers. If Qemu wants to deliver a GPIO-Signal
> notification, will call it.
>
> Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
> ---
> 1. Address discussion result about guest APEI notification type for SIGBUS_MCEERR_AO SIGBUS in [1],
> the discussion conclusion is using GPIO-Signal
>
> [1]:
> https://lists.gnu.org/archive/html/qemu-devel/2017-10/msg03397.html
> https://lists.gnu.org/archive/html/qemu-devel/2017-10/msg03467.html
> https://lists.gnu.org/archive/html/qemu-devel/2017-10/msg03601.html
> https://lists.gnu.org/archive/html/qemu-devel/2017-10/msg03775.html
>
> 2. The ASL dump for the GPIO and hardware error device
>
> ................
> Device (GPO0)
> {
> Name (_AEI, ResourceTemplate () // _AEI: ACPI Event Interrupts
> {
> .............
> GpioInt (Edge, ActiveHigh, Exclusive, PullUp, 0x0000,
> "GPO0", 0x00, ResourceConsumer, ,
> )
> { // Pin list
> 0x0004
> }
> })
> Method (_E04, 0, NotSerialized) // _Exx: Edge-Triggered GPE
> {
> Notify (ERRD, 0x80) // Status Change
> }
> }
> Device (ERRD)
> {
> Name (_HID, EisaId ("PNP0C33") /* Error Device */) // _HID: Hardware ID
> Name (_UID, Zero) // _UID: Unique ID
> Method (_STA, 0, NotSerialized) // _STA: Status
> {
> Return (0x0F)
> }
> }
>
> 3. Below is the guest log when Qemu notifies guest using GPIO-signal after record a CPER
> [ 504.164899] {1}[Hardware Error]: Hardware error from APEI Generic Hardware Error Source: 7
> [ 504.166970] {1}[Hardware Error]: event severity: recoverable
> [ 504.251650] {1}[Hardware Error]: Error 0, type: recoverable
> [ 504.252974] {1}[Hardware Error]: section_type: memory error
> [ 504.254380] {1}[Hardware Error]: physical_address: 0x00000000000003ec
> [ 504.255879] {1}[Hardware Error]: error_type: 3, multi-bit ECC
> ---
> hw/arm/virt-acpi-build.c | 31 ++++++++++++++++++++++++++++++-
> hw/arm/virt.c | 18 ++++++++++++++++++
> include/sysemu/sysemu.h | 3 +++
> vl.c | 12 ++++++++++++
> 4 files changed, 63 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index b7d45cd..06c14b3 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -49,6 +49,7 @@
>
> #define ARM_SPI_BASE 32
> #define ACPI_POWER_BUTTON_DEVICE "PWRB"
> +#define ACPI_HARDWARE_ERROR_DEVICE "ERRD"
>
> static void acpi_dsdt_add_cpus(Aml *scope, int smp_cpus)
> {
> @@ -340,7 +341,13 @@ static void acpi_dsdt_add_gpio(Aml *scope, const MemMapEntry *gpio_memmap,
>
> Aml *aei = aml_resource_template();
> /* Pin 3 for power button */
> - const uint32_t pin_list[1] = {3};
> + uint32_t pin_list[1] = {3};
> + aml_append(aei, aml_gpio_int(AML_CONSUMER, AML_EDGE, AML_ACTIVE_HIGH,
> + AML_EXCLUSIVE, AML_PULL_UP, 0, pin_list, 1,
> + "GPO0", NULL, 0));
> +
> + /* Pin 4 for hardware error device */
> + pin_list[0] = 4;
> aml_append(aei, aml_gpio_int(AML_CONSUMER, AML_EDGE, AML_ACTIVE_HIGH,
> AML_EXCLUSIVE, AML_PULL_UP, 0, pin_list, 1,
> "GPO0", NULL, 0));
> @@ -351,6 +358,13 @@ 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);
> +
> + /* _E04 is handle for hardware error */
> + method = aml_method("_E04", 0, AML_NOTSERIALIZED);
> + aml_append(method, aml_notify(aml_name(ACPI_HARDWARE_ERROR_DEVICE),
> + aml_int(0x80)));
aml_int(0x80) /* ACPI ... : Notification For Generic Error Sources */
> + aml_append(dev, method);
> +
> aml_append(scope, dev);
> }
>
> @@ -363,6 +377,20 @@ static void acpi_dsdt_add_power_button(Aml *scope)
> aml_append(scope, dev);
> }
>
> +static void acpi_dsdt_add_error_device(Aml *scope)
> +{
> + Aml *dev = aml_device(ACPI_HARDWARE_ERROR_DEVICE);
> + Aml *method;
> +
> + aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0C33")));
> + aml_append(dev, aml_name_decl("_UID", aml_int(0)));
> +
> + method = aml_method("_STA", 0, AML_NOTSERIALIZED);
> + aml_append(method, aml_return(aml_int(0x0f)));
no need for dummy _STA method, device is assumed to be present if there is no _STA
> + aml_append(dev, method);
> + aml_append(scope, dev);
> +}
> +
> /* RSDP */
> static GArray *
> build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned xsdt_tbl_offset)
> @@ -716,6 +744,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> acpi_dsdt_add_gpio(scope, &memmap[VIRT_GPIO],
> (irqmap[VIRT_GPIO] + ARM_SPI_BASE));
> acpi_dsdt_add_power_button(scope);
> + acpi_dsdt_add_error_device(scope);
>
> aml_append(dsdt, scope);
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 6b7a0fe..68495c2 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -701,16 +701,27 @@ static void create_rtc(const VirtMachineState *vms, qemu_irq *pic)
> }
>
> static DeviceState *gpio_key_dev;
> +static DeviceState *gpio_err_dev;
> static void virt_powerdown_req(Notifier *n, void *opaque)
> {
> /* use gpio Pin 3 for power button event */
> qemu_set_irq(qdev_get_gpio_in(gpio_key_dev, 0), 1);
> }
>
> +static void virt_error_notify_req(Notifier *n, void *opaque)
> +{
> + /* use gpio Pin 4 for hardware error event */
> + qemu_set_irq(qdev_get_gpio_in(gpio_err_dev, 0), 1);
> +}
> +
> static Notifier virt_system_powerdown_notifier = {
> .notify = virt_powerdown_req
> };
>
> +static Notifier virt_hardware_error_notifier = {
> + .notify = virt_error_notify_req
> +};
> +
> static void create_gpio(const VirtMachineState *vms, qemu_irq *pic)
> {
> char *nodename;
> @@ -739,6 +750,10 @@ static void create_gpio(const VirtMachineState *vms, qemu_irq *pic)
>
> gpio_key_dev = sysbus_create_simple("gpio-key", -1,
> qdev_get_gpio_in(pl061_dev, 3));
> +
> + gpio_err_dev = sysbus_create_simple("gpio-key", -1,
> + qdev_get_gpio_in(pl061_dev, 4));
> +
> qemu_fdt_add_subnode(vms->fdt, "/gpio-keys");
> qemu_fdt_setprop_string(vms->fdt, "/gpio-keys", "compatible", "gpio-keys");
> qemu_fdt_setprop_cell(vms->fdt, "/gpio-keys", "#size-cells", 0);
> @@ -755,6 +770,9 @@ static void create_gpio(const VirtMachineState *vms, qemu_irq *pic)
> /* connect powerdown request */
> qemu_register_powerdown_notifier(&virt_system_powerdown_notifier);
>
> + /* connect hardware error notify request */
> + qemu_register_hardware_error_notifier(&virt_hardware_error_notifier);
> +
> g_free(nodename);
> }
>
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index b213696..86931cf 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -75,6 +75,7 @@ void qemu_register_wakeup_notifier(Notifier *notifier);
> void qemu_system_shutdown_request(ShutdownCause reason);
> void qemu_system_powerdown_request(void);
> void qemu_register_powerdown_notifier(Notifier *notifier);
> +void qemu_register_hardware_error_notifier(Notifier *notifier);
> void qemu_system_debug_request(void);
> void qemu_system_vmstop_request(RunState reason);
> void qemu_system_vmstop_request_prepare(void);
> @@ -93,6 +94,8 @@ void qemu_remove_machine_init_done_notifier(Notifier *notify);
>
> void qemu_announce_self(void);
>
> +void qemu_hardware_error_notify(void);
> +
> extern int autostart;
>
> typedef enum {
> diff --git a/vl.c b/vl.c
> index d632693..3552f7d 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1614,6 +1614,8 @@ static int suspend_requested;
> static WakeupReason wakeup_reason;
> static NotifierList powerdown_notifiers =
> NOTIFIER_LIST_INITIALIZER(powerdown_notifiers);
> +static NotifierList hardware_error_notifiers =
> + NOTIFIER_LIST_INITIALIZER(hardware_error_notifiers);
> static NotifierList suspend_notifiers =
> NOTIFIER_LIST_INITIALIZER(suspend_notifiers);
> static NotifierList wakeup_notifiers =
> @@ -1850,12 +1852,22 @@ void qemu_register_powerdown_notifier(Notifier *notifier)
> notifier_list_add(&powerdown_notifiers, notifier);
> }
>
> +void qemu_register_hardware_error_notifier(Notifier *notifier)
> +{
> + notifier_list_add(&hardware_error_notifiers, notifier);
> +}
> +
> void qemu_system_debug_request(void)
> {
> debug_requested = 1;
> qemu_notify_event();
> }
>
> +void qemu_hardware_error_notify(void)
> +{
> + notifier_list_notify(&hardware_error_notifiers, NULL);
> +}
I'd prefer if you'd replace all this Notifier code with a machine callback
and call it when needed.
> static bool main_loop_should_exit(void)
> {
> RunState r;
On 2017/12/28 22:53, Igor Mammedov wrote:
> On Thu, 28 Dec 2017 13:54:16 +0800
> Dongjiu Geng <gengdongjiu@huawei.com> wrote:
>
>> In ARM platform we implements a notification of error events
>> via a GPIO pin. For this GPIO-signaled events, we choose GPIO
>> pin 4 for hardware error device (PNP0C33), So _E04 should be
>> added to ACPI DSDT table. When GPIO-pin 4 signaled a events,
>> the guest ACPI driver will receive this notification and
>> handing the error.
>>
>> In order to better trigger the GPIO IRQ, we defined a notifier
>> hardware_error_notifiers. If Qemu wants to deliver a GPIO-Signal
>> notification, will call it.
>>
>> Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
>> ---
>> 1. Address discussion result about guest APEI notification type for SIGBUS_MCEERR_AO SIGBUS in [1],
>> the discussion conclusion is using GPIO-Signal
>>
>> [1]:
>> https://lists.gnu.org/archive/html/qemu-devel/2017-10/msg03397.html
>> https://lists.gnu.org/archive/html/qemu-devel/2017-10/msg03467.html
>> https://lists.gnu.org/archive/html/qemu-devel/2017-10/msg03601.html
>> https://lists.gnu.org/archive/html/qemu-devel/2017-10/msg03775.html
>>
>> 2. The ASL dump for the GPIO and hardware error device
>>
>> ................
>> Device (GPO0)
>> {
>> Name (_AEI, ResourceTemplate () // _AEI: ACPI Event Interrupts
>> {
>> .............
>> GpioInt (Edge, ActiveHigh, Exclusive, PullUp, 0x0000,
>> "GPO0", 0x00, ResourceConsumer, ,
>> )
>> { // Pin list
>> 0x0004
>> }
>> })
>> Method (_E04, 0, NotSerialized) // _Exx: Edge-Triggered GPE
>> {
>> Notify (ERRD, 0x80) // Status Change
>> }
>> }
>> Device (ERRD)
>> {
>> Name (_HID, EisaId ("PNP0C33") /* Error Device */) // _HID: Hardware ID
>> Name (_UID, Zero) // _UID: Unique ID
>> Method (_STA, 0, NotSerialized) // _STA: Status
>> {
>> Return (0x0F)
>> }
>> }
>>
>> 3. Below is the guest log when Qemu notifies guest using GPIO-signal after record a CPER
>> [ 504.164899] {1}[Hardware Error]: Hardware error from APEI Generic Hardware Error Source: 7
>> [ 504.166970] {1}[Hardware Error]: event severity: recoverable
>> [ 504.251650] {1}[Hardware Error]: Error 0, type: recoverable
>> [ 504.252974] {1}[Hardware Error]: section_type: memory error
>> [ 504.254380] {1}[Hardware Error]: physical_address: 0x00000000000003ec
>> [ 504.255879] {1}[Hardware Error]: error_type: 3, multi-bit ECC
>> ---
>> hw/arm/virt-acpi-build.c | 31 ++++++++++++++++++++++++++++++-
>> hw/arm/virt.c | 18 ++++++++++++++++++
>> include/sysemu/sysemu.h | 3 +++
>> vl.c | 12 ++++++++++++
>> 4 files changed, 63 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>> index b7d45cd..06c14b3 100644
>> --- a/hw/arm/virt-acpi-build.c
>> +++ b/hw/arm/virt-acpi-build.c
>> @@ -49,6 +49,7 @@
>>
>> #define ARM_SPI_BASE 32
>> #define ACPI_POWER_BUTTON_DEVICE "PWRB"
>> +#define ACPI_HARDWARE_ERROR_DEVICE "ERRD"
>>
>> static void acpi_dsdt_add_cpus(Aml *scope, int smp_cpus)
>> {
>> @@ -340,7 +341,13 @@ static void acpi_dsdt_add_gpio(Aml *scope, const MemMapEntry *gpio_memmap,
>>
>> Aml *aei = aml_resource_template();
>> /* Pin 3 for power button */
>> - const uint32_t pin_list[1] = {3};
>> + uint32_t pin_list[1] = {3};
>> + aml_append(aei, aml_gpio_int(AML_CONSUMER, AML_EDGE, AML_ACTIVE_HIGH,
>> + AML_EXCLUSIVE, AML_PULL_UP, 0, pin_list, 1,
>> + "GPO0", NULL, 0));
>> +
>> + /* Pin 4 for hardware error device */
>> + pin_list[0] = 4;
>> aml_append(aei, aml_gpio_int(AML_CONSUMER, AML_EDGE, AML_ACTIVE_HIGH,
>> AML_EXCLUSIVE, AML_PULL_UP, 0, pin_list, 1,
>> "GPO0", NULL, 0));
>> @@ -351,6 +358,13 @@ 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);
>> +
>> + /* _E04 is handle for hardware error */
>> + method = aml_method("_E04", 0, AML_NOTSERIALIZED);
>> + aml_append(method, aml_notify(aml_name(ACPI_HARDWARE_ERROR_DEVICE),
>> + aml_int(0x80)));
> aml_int(0x80) /* ACPI ... : Notification For Generic Error Sources */
sure. thanks
>
>> + aml_append(dev, method);
>> +
>> aml_append(scope, dev);
>> }
>>
>> @@ -363,6 +377,20 @@ static void acpi_dsdt_add_power_button(Aml *scope)
>> aml_append(scope, dev);
>> }
>>
>> +static void acpi_dsdt_add_error_device(Aml *scope)
>> +{
>> + Aml *dev = aml_device(ACPI_HARDWARE_ERROR_DEVICE);
>> + Aml *method;
>> +
>> + aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0C33")));
>> + aml_append(dev, aml_name_decl("_UID", aml_int(0)));
>> +
>> + method = aml_method("_STA", 0, AML_NOTSERIALIZED);
>> + aml_append(method, aml_return(aml_int(0x0f)));
> no need for dummy _STA method, device is assumed to be present if there is no _STA
Igor,
do you mean remove above two line code as shown in [1]?
I dump the DSDT table in my host Ubuntu PC for the error device (PNP0C33), it has the _STA, as shown in [2].
do we not want to add the _STA for guest?
[1]
+ method = aml_method("_STA", 0, AML_NOTSERIALIZED);
+ aml_append(method, aml_return(aml_int(0x0f)));
[2]:
Device (WERR)
{
Name (_HID, EisaId ("PNP0C33")) // _HID: Hardware ID
Method (_STA, 0, NotSerialized) // _STA: Status
{
If (LGreaterEqual (OSYS, 0x07D9))
{
Return (0x0F)
}
Else
{
Return (Zero)
}
}
}
>
>> + aml_append(dev, method);
>> + aml_append(scope, dev);
>> +}
>> +
>> /* RSDP */
>> static GArray *
>> build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned xsdt_tbl_offset)
>> @@ -716,6 +744,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>> acpi_dsdt_add_gpio(scope, &memmap[VIRT_GPIO],
>> (irqmap[VIRT_GPIO] + ARM_SPI_BASE));
>> acpi_dsdt_add_power_button(scope);
>> + acpi_dsdt_add_error_device(scope);
>>
>> aml_append(dsdt, scope);
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index 6b7a0fe..68495c2 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -701,16 +701,27 @@ static void create_rtc(const VirtMachineState *vms, qemu_irq *pic)
>> }
>>
>> static DeviceState *gpio_key_dev;
>> +static DeviceState *gpio_err_dev;
>> static void virt_powerdown_req(Notifier *n, void *opaque)
>> {
>> /* use gpio Pin 3 for power button event */
>> qemu_set_irq(qdev_get_gpio_in(gpio_key_dev, 0), 1);
>> }
>>
>> +static void virt_error_notify_req(Notifier *n, void *opaque)
>> +{
>> + /* use gpio Pin 4 for hardware error event */
>> + qemu_set_irq(qdev_get_gpio_in(gpio_err_dev, 0), 1);
>> +}
>> +
>> static Notifier virt_system_powerdown_notifier = {
>> .notify = virt_powerdown_req
>> };
>>
>> +static Notifier virt_hardware_error_notifier = {
>> + .notify = virt_error_notify_req
>> +};
>> +
>> static void create_gpio(const VirtMachineState *vms, qemu_irq *pic)
>> {
>> char *nodename;
>> @@ -739,6 +750,10 @@ static void create_gpio(const VirtMachineState *vms, qemu_irq *pic)
>>
>> gpio_key_dev = sysbus_create_simple("gpio-key", -1,
>> qdev_get_gpio_in(pl061_dev, 3));
>> +
>> + gpio_err_dev = sysbus_create_simple("gpio-key", -1,
>> + qdev_get_gpio_in(pl061_dev, 4));
>> +
>> qemu_fdt_add_subnode(vms->fdt, "/gpio-keys");
>> qemu_fdt_setprop_string(vms->fdt, "/gpio-keys", "compatible", "gpio-keys");
>> qemu_fdt_setprop_cell(vms->fdt, "/gpio-keys", "#size-cells", 0);
>> @@ -755,6 +770,9 @@ static void create_gpio(const VirtMachineState *vms, qemu_irq *pic)
>> /* connect powerdown request */
>> qemu_register_powerdown_notifier(&virt_system_powerdown_notifier);
>>
>> + /* connect hardware error notify request */
>> + qemu_register_hardware_error_notifier(&virt_hardware_error_notifier);
>> +
>> g_free(nodename);
>> }
>>
>> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
>> index b213696..86931cf 100644
>> --- a/include/sysemu/sysemu.h
>> +++ b/include/sysemu/sysemu.h
>> @@ -75,6 +75,7 @@ void qemu_register_wakeup_notifier(Notifier *notifier);
>> void qemu_system_shutdown_request(ShutdownCause reason);
>> void qemu_system_powerdown_request(void);
>> void qemu_register_powerdown_notifier(Notifier *notifier);
>> +void qemu_register_hardware_error_notifier(Notifier *notifier);
>> void qemu_system_debug_request(void);
>> void qemu_system_vmstop_request(RunState reason);
>> void qemu_system_vmstop_request_prepare(void);
>> @@ -93,6 +94,8 @@ void qemu_remove_machine_init_done_notifier(Notifier *notify);
>>
>> void qemu_announce_self(void);
>>
>> +void qemu_hardware_error_notify(void);
>> +
>> extern int autostart;
>>
>> typedef enum {
>> diff --git a/vl.c b/vl.c
>> index d632693..3552f7d 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -1614,6 +1614,8 @@ static int suspend_requested;
>> static WakeupReason wakeup_reason;
>> static NotifierList powerdown_notifiers =
>> NOTIFIER_LIST_INITIALIZER(powerdown_notifiers);
>> +static NotifierList hardware_error_notifiers =
>> + NOTIFIER_LIST_INITIALIZER(hardware_error_notifiers);
>> static NotifierList suspend_notifiers =
>> NOTIFIER_LIST_INITIALIZER(suspend_notifiers);
>> static NotifierList wakeup_notifiers =
>> @@ -1850,12 +1852,22 @@ void qemu_register_powerdown_notifier(Notifier *notifier)
>> notifier_list_add(&powerdown_notifiers, notifier);
>> }
>>
>> +void qemu_register_hardware_error_notifier(Notifier *notifier)
>> +{
>> + notifier_list_add(&hardware_error_notifiers, notifier);
>> +}
>> +
>> void qemu_system_debug_request(void)
>> {
>> debug_requested = 1;
>> qemu_notify_event();
>> }
>>
>> +void qemu_hardware_error_notify(void)
>> +{
>> + notifier_list_notify(&hardware_error_notifiers, NULL);
>> +}
>
> I'd prefer if you'd replace all this Notifier code with a machine callback
> and call it when needed.
Ok, thanks for this suggestion. I will check the code and see how to add a machine callback.
>
>> static bool main_loop_should_exit(void)
>> {
>> RunState r;
>
>
> .
>
On Wed, 3 Jan 2018 11:48:30 +0800
gengdongjiu <gengdongjiu@huawei.com> wrote:
> On 2017/12/28 22:53, Igor Mammedov wrote:
> > On Thu, 28 Dec 2017 13:54:16 +0800
> > Dongjiu Geng <gengdongjiu@huawei.com> wrote:
[...]
> >> +static void acpi_dsdt_add_error_device(Aml *scope)
> >> +{
> >> + Aml *dev = aml_device(ACPI_HARDWARE_ERROR_DEVICE);
> >> + Aml *method;
> >> +
> >> + aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0C33")));
> >> + aml_append(dev, aml_name_decl("_UID", aml_int(0)));
> >> +
> >> + method = aml_method("_STA", 0, AML_NOTSERIALIZED);
> >> + aml_append(method, aml_return(aml_int(0x0f)));
> > no need for dummy _STA method, device is assumed to be present if there is no _STA
> Igor,
> do you mean remove above two line code as shown in [1]?
> I dump the DSDT table in my host Ubuntu PC for the error device (PNP0C33), it has the _STA, as shown in [2].
> do we not want to add the _STA for guest?
>
> [1]
> + method = aml_method("_STA", 0, AML_NOTSERIALIZED);
> + aml_append(method, aml_return(aml_int(0x0f)));
compared to host, yours method does nothing,
read ACPI6.2 "6.3.7 _STA (Status)" one more time
> [2]:
> Device (WERR)
> {
> Name (_HID, EisaId ("PNP0C33")) // _HID: Hardware ID
> Method (_STA, 0, NotSerialized) // _STA: Status
> {
> If (LGreaterEqual (OSYS, 0x07D9))
> {
> Return (0x0F)
> }
> Else
> {
> Return (Zero)
> }
> }
> }
> >
> >> + aml_append(dev, method);
> >> + aml_append(scope, dev);
> >> +}
> >> +
[...]
On 2018/1/3 21:36, Igor Mammedov wrote:
> On Wed, 3 Jan 2018 11:48:30 +0800
> gengdongjiu <gengdongjiu@huawei.com> wrote:
>
>> On 2017/12/28 22:53, Igor Mammedov wrote:
>>> On Thu, 28 Dec 2017 13:54:16 +0800
>>> Dongjiu Geng <gengdongjiu@huawei.com> wrote:
> [...]
>>>> +static void acpi_dsdt_add_error_device(Aml *scope)
>>>> +{
>>>> + Aml *dev = aml_device(ACPI_HARDWARE_ERROR_DEVICE);
>>>> + Aml *method;
>>>> +
>>>> + aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0C33")));
>>>> + aml_append(dev, aml_name_decl("_UID", aml_int(0)));
>>>> +
>>>> + method = aml_method("_STA", 0, AML_NOTSERIALIZED);
>>>> + aml_append(method, aml_return(aml_int(0x0f)));
>>> no need for dummy _STA method, device is assumed to be present if there is no _STA
>> Igor,
>> do you mean remove above two line code as shown in [1]?
>> I dump the DSDT table in my host Ubuntu PC for the error device (PNP0C33), it has the _STA, as shown in [2].
>> do we not want to add the _STA for guest?
>>
>> [1]
>> + method = aml_method("_STA", 0, AML_NOTSERIALIZED);
>> + aml_append(method, aml_return(aml_int(0x0f)));
> compared to host, yours method does nothing,
> read ACPI6.2 "6.3.7 _STA (Status)" one more time
Thanks for the pointing out.
yes, you are right. As the spec statement[1], Device is assumed to be present if there is no _STA.
[1]:
ACPI6.2 "6.3.7 _STA (Status), Return Value Information"
If a device object (including the processor object) does not have an _STA object, then OSPM
assumes that all of the above bits are set (i.e., the device is present, enabled, shown in the UI,
and functioning).
>
>> [2]:
>> Device (WERR)
>> {
>> Name (_HID, EisaId ("PNP0C33")) // _HID: Hardware ID
>> Method (_STA, 0, NotSerialized) // _STA: Status
>> {
>> If (LGreaterEqual (OSYS, 0x07D9))
>> {
>> Return (0x0F)
>> }
>> Else
>> {
>> Return (Zero)
>> }
>> }
>> }
>>>
>>>> + aml_append(dev, method);
>>>> + aml_append(scope, dev);
>>>> +}
>>>> +
> [...]
>
>
> .
>
© 2016 - 2025 Red Hat, Inc.