[Qemu-devel] [PATCH v2 0/8] Add system_powerdown support on ARM for ACPI and DT

[Qemu-devel] [PATCH v2 0/8] Add system_powerdown support on ARM for ACPI and DT

[Shannon Zhao]

ACPI SPEC 5.0 defines GPIO-signaled ACPI Events for Hardware-reduced
platforms(like ARM). It uses GPIO pin to trigger an event to the guest.
For QEMU, here we add PL061 GPIO controller and use PIN 3 for
system_powerdown, reserving PIN 0, 1, 2 for PCI hotplug, CPU hotplug and
memory hotplug.

This patchset adds system_powerdown support on ARM through both ACPI and
DT ways. It adds a GPIO controller(here is PL061) in machine virt and
uses GPIO-singled event for ACPI while gpio-keys for DT. It can be
fetched from [1] and has been tested for the guests starting by ACPI or
DT while guests use systemd or acpid.

a) ACPI way. Since Graeme send a patchset to make ACPI on ARM64 support
amba device[2], it could use PL061 directly without modification to its
kernel driver code. In addition, we should use ACPI to start VM,
referring to below script. QEMU_EFI.fd can be fetched from [3]. 

    qemu-system-aarch64 \
    -smp 1 -m 1024 -M virt \
    -cpu cortex-a57 -nographic \
    -monitor telnet::4444,server,nowait \
    -bios QEMU_EFI.fd \
    -kernel Image \
    -initrd guestfs.cpio.gz \
    -append "rdinit=/sbin/init console=ttyAMA0 mem=512M root=/dev/ram 
earlycon=pl011,0x9000000 rw acpi=force"

b) DT way. Start vm as usual.

    qemu-system-aarch64 \
    -smp 1 -m 1024 -M virt \
    -cpu cortex-a57 -nographic \
    -monitor telnet::4444,server,nowait \
    -kernel Image \
    -initrd guestfs.cpio.gz \
    -append "rdinit=/sbin/init console=ttyAMA0 mem=512M root=/dev/ram 
earlycon=pl011,0x9000000 rw"

Guest internal setup:
If your guest FS uses systemd, you should check file
/lib/udev/rules.d/70-power-switch.rules and add the following line in it
if it doesn't exist.

SUBSYSTEM=="input", KERNEL=="event*", SUBSYSTEMS=="platform",
ATTRS{keys}=="116", TAG+="power-switch"

If your guest FS uses acpid, you should check it has
/etc/acpi/powerbtn.sh and /etc/acpi/events/powerbtn. Refer to [4] for
the setup of acpid.

Thanks,
Shannon

[1] https://git.linaro.org/people/shannon.zhao/qemu.git/shortlog/refs/heads/PowerButton_v2
[2] https://lkml.org/lkml/2015/9/30/392
[3] http://people.linaro.org/~shannon.zhao/ACPI_ARM/QEMU_EFI.fd
[4] https://wiki.linaro.org/LEG/Engineering/Kernel/ACPI/GPIOPowerButton

Changes since v1:
* rewrite GPIO Connection Descriptor (Michael)

Shannon Zhao (8):
  hw/arm/virt: Add a GPIO controller
  hw/arm/virt-acpi-build: Add GPIO controller in ACPI DSDT table
  hw/arm/virt-acpi-build: Add power button device in ACPI DSDT table
  hw/acpi/aml-build: Add GPIO Connection Descriptor
  hw/acpi/aml-build: Add a wrapper for GPIO Interrupt Connection
  hw/arm/virt-acpi-build: Add _E03 for Power Button
  hw/arm/virt: Add QEMU powerdown notifier and hook it to GPIO Pin 3
  hw/arm/virt: Add gpio-keys node for Poweroff using DT

 hw/acpi/aml-build.c         | 79 +++++++++++++++++++++++++++++++++++++++++++++
 hw/arm/virt-acpi-build.c    | 45 ++++++++++++++++++++++++++
 hw/arm/virt.c               | 60 ++++++++++++++++++++++++++++++++++
 include/hw/acpi/aml-build.h | 26 +++++++++++++++
 include/hw/arm/virt.h       |  1 +
 5 files changed, 211 insertions(+)

-- 
2.1.0

[Qemu-devel] [PATCH v2 1/8] hw/arm/virt: Add a GPIO controller

[Shannon Zhao]

ACPI 5.0 supports GPIO-signaled ACPI Events. This can be used for
powerdown, hotplug evnets. Add a GPIO controller in machine virt,
to support powerdown, maybe can be used for cpu hotplug. And
here we use pl061.

Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 hw/arm/virt.c         | 30 ++++++++++++++++++++++++++++++
 include/hw/arm/virt.h |  1 +
 2 files changed, 31 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 77d9267..5c505a4 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -120,6 +120,7 @@ static const MemMapEntry a15memmap[] = {
     [VIRT_UART] =               { 0x09000000, 0x00001000 },
     [VIRT_RTC] =                { 0x09010000, 0x00001000 },
     [VIRT_FW_CFG] =             { 0x09020000, 0x00000018 },
+    [VIRT_GPIO] =               { 0x09030000, 0x00001000 },
     [VIRT_MMIO] =               { 0x0a000000, 0x00000200 },
     /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
     [VIRT_PLATFORM_BUS] =       { 0x0c000000, 0x02000000 },
@@ -135,6 +136,7 @@ static const int a15irqmap[] = {
     [VIRT_UART] = 1,
     [VIRT_RTC] = 2,
     [VIRT_PCIE] = 3, /* ... to 6 */
+    [VIRT_GPIO] = 7,
     [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
     [VIRT_GIC_V2M] = 48, /* ...to 48 + NUM_GICV2M_SPIS - 1 */
     [VIRT_PLATFORM_BUS] = 112, /* ...to 112 + PLATFORM_BUS_NUM_IRQS -1 */
@@ -538,6 +540,32 @@ static void create_rtc(const VirtBoardInfo *vbi, qemu_irq *pic)
     g_free(nodename);
 }
 
+static void create_gpio(const VirtBoardInfo *vbi, qemu_irq *pic)
+{
+    char *nodename;
+    hwaddr base = vbi->memmap[VIRT_GPIO].base;
+    hwaddr size = vbi->memmap[VIRT_GPIO].size;
+    int irq = vbi->irqmap[VIRT_GPIO];
+    const char compat[] = "arm,pl061\0arm,primecell";
+
+    sysbus_create_simple("pl061", base, pic[irq]);
+
+    nodename = g_strdup_printf("/pl061@%" PRIx64, base);
+    qemu_fdt_add_subnode(vbi->fdt, nodename);
+    qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg",
+                                 2, base, 2, size);
+    qemu_fdt_setprop(vbi->fdt, nodename, "compatible", compat, sizeof(compat));
+    qemu_fdt_setprop_cell(vbi->fdt, nodename, "#gpio-cells", 2);
+    qemu_fdt_setprop(vbi->fdt, nodename, "gpio-controller", NULL, 0);
+    qemu_fdt_setprop_cells(vbi->fdt, nodename, "interrupts",
+                           GIC_FDT_IRQ_TYPE_SPI, irq,
+                           GIC_FDT_IRQ_FLAGS_LEVEL_HI);
+    qemu_fdt_setprop_cell(vbi->fdt, nodename, "clocks", vbi->clock_phandle);
+    qemu_fdt_setprop_string(vbi->fdt, nodename, "clock-names", "apb_pclk");
+
+    g_free(nodename);
+}
+
 static void create_virtio_devices(const VirtBoardInfo *vbi, qemu_irq *pic)
 {
     int i;
@@ -1041,6 +1069,8 @@ static void machvirt_init(MachineState *machine)
 
     create_pcie(vbi, pic, vms->highmem);
 
+    create_gpio(vbi, pic);
+
     /* Create mmio transports, so the user can create virtio backends
      * (which will be automatically plugged in to the transports). If
      * no backend is created the transport will just sit harmlessly idle.
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index f464586..925faa7 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -59,6 +59,7 @@ enum {
     VIRT_PCIE_ECAM,
     VIRT_PLATFORM_BUS,
     VIRT_PCIE_MMIO_HIGH,
+    VIRT_GPIO,
 };
 
 typedef struct MemMapEntry {
-- 
2.1.0

[Qemu-devel] [PATCH v2 2/8] hw/arm/virt-acpi-build: Add GPIO controller in ACPI DSDT table

[Shannon Zhao]

Add GPIO controller in ACPI DSDT table. It can be used for gpio event.

Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 hw/arm/virt-acpi-build.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 1aaff1f..ffb9b91 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -322,6 +322,23 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, int irq,
     aml_append(scope, dev);
 }
 
+static void acpi_dsdt_add_gpio(Aml *scope, const MemMapEntry *gpio_memmap,
+                                           int gpio_irq)
+{
+    Aml *dev = aml_device("GPO0");
+    aml_append(dev, aml_name_decl("_HID", aml_string("ARMH0061")));
+    aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
+    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
+
+    Aml *crs = aml_resource_template();
+    aml_append(crs, aml_memory32_fixed(gpio_memmap->base, gpio_memmap->size,
+                                       AML_READ_WRITE));
+    aml_append(crs, aml_interrupt(AML_CONSUMER, AML_EDGE, AML_ACTIVE_HIGH,
+                                  AML_EXCLUSIVE, gpio_irq));
+    aml_append(dev, aml_name_decl("_CRS", crs));
+    aml_append(scope, dev);
+}
+
 /* RSDP */
 static GArray *
 build_rsdp(GArray *rsdp_table, GArray *linker, unsigned rsdt)
@@ -534,6 +551,8 @@ build_dsdt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
                     (irqmap[VIRT_MMIO] + ARM_SPI_BASE), NUM_VIRTIO_TRANSPORTS);
     acpi_dsdt_add_pci(scope, memmap, (irqmap[VIRT_PCIE] + ARM_SPI_BASE),
                       guest_info->use_highmem);
+    acpi_dsdt_add_gpio(scope, &memmap[VIRT_GPIO],
+                       (irqmap[VIRT_GPIO] + ARM_SPI_BASE));
 
     aml_append(dsdt, scope);
 
-- 
2.1.0

[Qemu-devel] [PATCH v2 3/8] hw/arm/virt-acpi-build: Add power button device in ACPI DSDT table

[Shannon Zhao]

Add power button device in ACPI DSDT table.

Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 hw/arm/virt-acpi-build.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index ffb9b91..80ce51c 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -339,6 +339,18 @@ static void acpi_dsdt_add_gpio(Aml *scope, const MemMapEntry *gpio_memmap,
     aml_append(scope, dev);
 }
 
+static void acpi_dsdt_add_power_button(Aml *scope)
+{
+    Aml *dev = aml_device("PWRB");
+    aml_append(dev, aml_name_decl("_HID", aml_string("PNP0C0C")));
+    aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
+    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
+    Aml *method = aml_method("_STA", 0);
+    aml_append(method, aml_return(aml_int(0x0F)));
+    aml_append(dev, method);
+    aml_append(scope, dev);
+}
+
 /* RSDP */
 static GArray *
 build_rsdp(GArray *rsdp_table, GArray *linker, unsigned rsdt)
@@ -553,6 +565,7 @@ build_dsdt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
                       guest_info->use_highmem);
     acpi_dsdt_add_gpio(scope, &memmap[VIRT_GPIO],
                        (irqmap[VIRT_GPIO] + ARM_SPI_BASE));
+    acpi_dsdt_add_power_button(scope);
 
     aml_append(dsdt, scope);
 
-- 
2.1.0

[Qemu-devel] [PATCH v2 4/8] hw/acpi/aml-build: Add GPIO Connection Descriptor

[Shannon Zhao]

Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 hw/acpi/aml-build.c         | 61 +++++++++++++++++++++++++++++++++++++++++++++
 include/hw/acpi/aml-build.h | 20 +++++++++++++++
 2 files changed, 81 insertions(+)

diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 0d4b324..250941f 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -565,6 +565,67 @@ Aml *aml_call4(const char *method, Aml *arg1, Aml *arg2, Aml *arg3, Aml *arg4)
 }
 
 /*
+ * ACPI 5.0: 6.4.3.8.1 GPIO Connection Descriptor
+ * Type 1, Large Item Name 0xC
+ */
+
+static Aml *aml_gpio_connection(AmlGpioConnectionType type,
+                                AmlConsumerAndProducer con_and_pro,
+                                uint8_t flags, AmlPinConfig pin_cfg,
+                                int16_t output_drive, int16_t debounce_timeout,
+                                int32_t pin_num[], int32_t pin_count,
+                                const char *name, const char *vendor_data)
+{
+    Aml *var = aml_alloc();
+    uint16_t name_len = name ? (strlen(name) + 1) : 0;
+    uint16_t vendor_data_len = vendor_data ? (strlen(vendor_data) + 1) : 0;
+    uint16_t length = 0x16 + name_len + vendor_data_len;
+    uint16_t name_offset = 0x17 + pin_count * 2;
+    uint16_t vendor_data_offset = name_offset + name_len;
+    int i;
+
+    build_append_byte(var->buf, 0x8C);  /* GpioInt Resource Descriptor */
+    build_append_int_noprefix(var->buf, length, 2); /* Length */
+    build_append_byte(var->buf, 1);     /* Revision ID */
+    build_append_byte(var->buf, type);  /* GPIO Connection Type */
+    /* General Flags (2 bytes) */
+    build_append_int_noprefix(var->buf, con_and_pro, 2);
+    /* Interrupt and IO Flags (2 bytes) */
+    build_append_int_noprefix(var->buf, flags, 2);
+    /* Pin Configuration 0 = Default 1 = Pull-up 2 = Pull-down 3 = No Pull */
+    build_append_byte(var->buf, pin_cfg);
+    /* Output Drive Strength (2 bytes) */
+    build_append_int_noprefix(var->buf, output_drive, 2);
+    /* Debounce Timeout (2 bytes) */
+    build_append_int_noprefix(var->buf, debounce_timeout, 2);
+    /* Pin Table Offset (2 bytes) */
+    build_append_int_noprefix(var->buf, 0x17, 2);
+    build_append_byte(var->buf, 0);     /* Resource Source Index */
+    /* Resource Source Name Offset (2 bytes) */
+    build_append_int_noprefix(var->buf, name_offset, 2);
+    /* Vendor Data Offset (2 bytes) */
+    build_append_int_noprefix(var->buf, vendor_data_offset, 2);
+    /* Vendor Data Length (2 bytes) */
+    build_append_int_noprefix(var->buf, vendor_data_len, 2);
+    /* Pin Number (2n bytes)*/
+    for (i = 0; i < pin_count; i++) {
+        build_append_int_noprefix(var->buf, pin_num[i], 2);
+    }
+    /* Resource Source Name */
+    if (name != NULL) {
+        build_append_namestring(var->buf, "%s", name);
+        build_append_byte(var->buf, '\0');
+    }
+    /* Vendor-defined Data */
+    if (vendor_data != NULL) {
+        build_append_namestring(var->buf, "%s", vendor_data);
+        build_append_byte(var->buf, '\0');
+    }
+
+    return var;
+}
+
+/*
  * ACPI 1.0b: 6.4.3.4 32-Bit Fixed Location Memory Range Descriptor
  * (Type 1, Large Item Name 0x6)
  */
diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index 1b632dc..4e88882 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -148,6 +148,26 @@ typedef enum {
     AML_SHARED_AND_WAKE = 3,
 } AmlShared;
 
+/*
+ * ACPI 5.0: Table 6-189 GPIO Connection Descriptor Definition
+ * GPIO Connection Type
+ */
+typedef enum {
+    AML_INTERRUPT_CONNECTION = 0,
+    AML_IO_CONNECTION = 1,
+} AmlGpioConnectionType;
+
+/*
+ * ACPI 5.0: Table 6-189 GPIO Connection Descriptor Definition
+ * _PPI field definition
+ */
+typedef enum {
+    AML_DEFAULT_CONFIG = 0,
+    AML_PULL_UP = 1,
+    AML_PULL_DOWN = 2,
+    AML_NO_PULL = 3,
+} AmlPinConfig;
+
 typedef
 struct AcpiBuildTables {
     GArray *table_data;
-- 
2.1.0

[Qemu-devel] [PATCH v2 5/8] hw/acpi/aml-build: Add a wrapper for GPIO Interrupt Connection

[Shannon Zhao]

Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 hw/acpi/aml-build.c         | 18 ++++++++++++++++++
 include/hw/acpi/aml-build.h |  6 ++++++
 2 files changed, 24 insertions(+)

diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 250941f..895f82d 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -626,6 +626,24 @@ static Aml *aml_gpio_connection(AmlGpioConnectionType type,
 }
 
 /*
+ * ACPI 5.0: 19.5.53
+ * GpioInt(GPIO Interrupt Connection Resource Descriptor Macro)
+ */
+Aml *aml_gpio_int(AmlConsumerAndProducer con_and_pro,
+                  AmlLevelAndEdge level_and_edge,
+                  AmlActiveHighAndLow high_and_low, AmlShared shared,
+                  AmlPinConfig pin_cfg, int16_t debounce_timeout,
+                  int32_t pin_num[], int32_t pin_count, const char *name,
+                  const char *vendor_data)
+{
+    uint8_t flags = level_and_edge | (high_and_low << 1) | (shared << 3);
+
+    return aml_gpio_connection(AML_INTERRUPT_CONNECTION, con_and_pro, flags,
+                               pin_cfg, 0, debounce_timeout, pin_num, pin_count,
+                               name, vendor_data);
+}
+
+/*
  * ACPI 1.0b: 6.4.3.4 32-Bit Fixed Location Memory Range Descriptor
  * (Type 1, Large Item Name 0x6)
  */
diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index 4e88882..bce813b 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -232,6 +232,12 @@ Aml *aml_call1(const char *method, Aml *arg1);
 Aml *aml_call2(const char *method, Aml *arg1, Aml *arg2);
 Aml *aml_call3(const char *method, Aml *arg1, Aml *arg2, Aml *arg3);
 Aml *aml_call4(const char *method, Aml *arg1, Aml *arg2, Aml *arg3, Aml *arg4);
+Aml *aml_gpio_int(AmlConsumerAndProducer con_and_pro,
+                  AmlLevelAndEdge level_and_edge,
+                  AmlActiveHighAndLow high_and_low, AmlShared shared,
+                  AmlPinConfig pin_cfg, int16_t debounce_timeout,
+                  int32_t pin_num[], int32_t pin_count, const char *name,
+                  const char *vendor_data);
 Aml *aml_memory32_fixed(uint32_t addr, uint32_t size,
                         AmlReadAndWrite read_and_write);
 Aml *aml_interrupt(AmlConsumerAndProducer con_and_pro,
-- 
2.1.0

[Qemu-devel] [PATCH v2 6/8] hw/arm/virt-acpi-build: Add _E03 for Power Button

[Shannon Zhao]

Here GPIO pin 3 is used for Power Button, add _E03 in ACPI DSDT table.

Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 hw/arm/virt-acpi-build.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 80ce51c..54f88c9 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -336,6 +336,19 @@ static void acpi_dsdt_add_gpio(Aml *scope, const MemMapEntry *gpio_memmap,
     aml_append(crs, aml_interrupt(AML_CONSUMER, AML_EDGE, AML_ACTIVE_HIGH,
                                   AML_EXCLUSIVE, gpio_irq));
     aml_append(dev, aml_name_decl("_CRS", crs));
+
+    Aml *aei = aml_resource_template();
+    /* Pin 3 for power button */
+    int32_t pin_num[1] = {3};
+    aml_append(aei, aml_gpio_int(AML_CONSUMER_PRODUCER, AML_EDGE,
+                                 AML_ACTIVE_HIGH, AML_EXCLUSIVE, AML_PULL_UP, 0,
+                                 pin_num, 1, "GPO0", NULL));
+    aml_append(dev, aml_name_decl("_AEI", aei));
+
+    /* _E03 is handle for power button */
+    Aml *method = aml_method("_E03", 0);
+    aml_append(method, aml_notify(aml_name("PWRB"), aml_int(0x80)));
+    aml_append(dev, method);
     aml_append(scope, dev);
 }
 
-- 
2.1.0

[Qemu-devel] [PATCH v2 7/8] hw/arm/virt: Add QEMU powerdown notifier and hook it to GPIO Pin 3

[Shannon Zhao]

Currently mach-virt model doesn't support powerdown request. Guest VM
doesn't react to system_powerdown from monitor console (or QMP) because
there is no communication mechanism for such requests. This patch registers
GPIO Pin 3 with powerdown notification. So guest VM can receive notification
when such powerdown request is triggered.

Signed-off-by: Wei Huang <wei@redhat.com>
Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 hw/arm/virt.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 5c505a4..6f1b9d6 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -540,6 +540,18 @@ static void create_rtc(const VirtBoardInfo *vbi, qemu_irq *pic)
     g_free(nodename);
 }
 
+static DeviceState *pl061_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(pl061_dev, 3), 0);
+    qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 1);
+}
+
+static Notifier virt_system_powerdown_notifier = {
+    .notify = virt_powerdown_req
+};
+
 static void create_gpio(const VirtBoardInfo *vbi, qemu_irq *pic)
 {
     char *nodename;
@@ -548,7 +560,7 @@ static void create_gpio(const VirtBoardInfo *vbi, qemu_irq *pic)
     int irq = vbi->irqmap[VIRT_GPIO];
     const char compat[] = "arm,pl061\0arm,primecell";
 
-    sysbus_create_simple("pl061", base, pic[irq]);
+    pl061_dev = sysbus_create_simple("pl061", base, pic[irq]);
 
     nodename = g_strdup_printf("/pl061@%" PRIx64, base);
     qemu_fdt_add_subnode(vbi->fdt, nodename);
@@ -563,6 +575,9 @@ static void create_gpio(const VirtBoardInfo *vbi, qemu_irq *pic)
     qemu_fdt_setprop_cell(vbi->fdt, nodename, "clocks", vbi->clock_phandle);
     qemu_fdt_setprop_string(vbi->fdt, nodename, "clock-names", "apb_pclk");
 
+    /* connect powerdown request */
+    qemu_register_powerdown_notifier(&virt_system_powerdown_notifier);
+
     g_free(nodename);
 }
 
-- 
2.1.0

[Qemu-devel] [PATCH v2 8/8] hw/arm/virt: Add gpio-keys node for Poweroff using DT

[Shannon Zhao]

Add a gpio-keys node. This is used for Poweroff for the systems which
use DT not ACPI.

Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 hw/arm/virt.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 6f1b9d6..75a3a5c 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -562,6 +562,7 @@ static void create_gpio(const VirtBoardInfo *vbi, qemu_irq *pic)
 
     pl061_dev = sysbus_create_simple("pl061", base, pic[irq]);
 
+    uint32_t phandle = qemu_fdt_alloc_phandle(vbi->fdt);
     nodename = g_strdup_printf("/pl061@%" PRIx64, base);
     qemu_fdt_add_subnode(vbi->fdt, nodename);
     qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg",
@@ -574,6 +575,20 @@ static void create_gpio(const VirtBoardInfo *vbi, qemu_irq *pic)
                            GIC_FDT_IRQ_FLAGS_LEVEL_HI);
     qemu_fdt_setprop_cell(vbi->fdt, nodename, "clocks", vbi->clock_phandle);
     qemu_fdt_setprop_string(vbi->fdt, nodename, "clock-names", "apb_pclk");
+    qemu_fdt_setprop_cell(vbi->fdt, nodename, "phandle", phandle);
+
+    qemu_fdt_add_subnode(vbi->fdt, "/gpio-keys");
+    qemu_fdt_setprop_string(vbi->fdt, "/gpio-keys", "compatible", "gpio-keys");
+    qemu_fdt_setprop_cell(vbi->fdt, "/gpio-keys", "#size-cells", 0);
+    qemu_fdt_setprop_cell(vbi->fdt, "/gpio-keys", "#address-cells", 1);
+    qemu_fdt_setprop(vbi->fdt, "/gpio-keys", "autorepeat", NULL, 0);
+
+    qemu_fdt_add_subnode(vbi->fdt, "/gpio-keys/poweroff");
+    qemu_fdt_setprop_string(vbi->fdt, "/gpio-keys/poweroff",
+                            "label", "GPIO Key Poweroff");
+    qemu_fdt_setprop_cell(vbi->fdt, "/gpio-keys/poweroff", "linux,code", 116);
+    qemu_fdt_setprop_cells(vbi->fdt, "/gpio-keys/poweroff",
+                           "gpios", phandle, 3, 0);
 
     /* connect powerdown request */
     qemu_register_powerdown_notifier(&virt_system_powerdown_notifier);
-- 
2.1.0

Re: [Qemu-devel] [PATCH v2 0/8] Add system_powerdown support on ARM for ACPI and DT

[Wei Huang]

On 10/29/2015 09:27 AM, Shannon Zhao wrote:
> ACPI SPEC 5.0 defines GPIO-signaled ACPI Events for Hardware-reduced
> platforms(like ARM). It uses GPIO pin to trigger an event to the guest.
> For QEMU, here we add PL061 GPIO controller and use PIN 3 for
> system_powerdown, reserving PIN 0, 1, 2 for PCI hotplug, CPU hotplug and
> memory hotplug.
> 
> This patchset adds system_powerdown support on ARM through both ACPI and
> DT ways. It adds a GPIO controller(here is PL061) in machine virt and
> uses GPIO-singled event for ACPI while gpio-keys for DT. It can be
> fetched from [1] and has been tested for the guests starting by ACPI or
> DT while guests use systemd or acpid.
> 
> a) ACPI way. Since Graeme send a patchset to make ACPI on ARM64 support
> amba device[2], it could use PL061 directly without modification to its
> kernel driver code. In addition, we should use ACPI to start VM,
> referring to below script. QEMU_EFI.fd can be fetched from [3]. 

Hi Shannon,

Thanks for re-sending it. This is a desired feature because we don't
want to rely on tricks (such as guest-agent) for external VM power
management. I have tested V1 recently by back-porting to my in-house
kernel; it worked well.

I will help review this new version.

Thanks,
-Wei

> 
>     qemu-system-aarch64 \
>     -smp 1 -m 1024 -M virt \
>     -cpu cortex-a57 -nographic \
>     -monitor telnet::4444,server,nowait \
>     -bios QEMU_EFI.fd \
>     -kernel Image \
>     -initrd guestfs.cpio.gz \
>     -append "rdinit=/sbin/init console=ttyAMA0 mem=512M root=/dev/ram 
> earlycon=pl011,0x9000000 rw acpi=force"
> 
> b) DT way. Start vm as usual.
> 
>     qemu-system-aarch64 \
>     -smp 1 -m 1024 -M virt \
>     -cpu cortex-a57 -nographic \
>     -monitor telnet::4444,server,nowait \
>     -kernel Image \
>     -initrd guestfs.cpio.gz \
>     -append "rdinit=/sbin/init console=ttyAMA0 mem=512M root=/dev/ram 
> earlycon=pl011,0x9000000 rw"
> 
> Guest internal setup:
> If your guest FS uses systemd, you should check file
> /lib/udev/rules.d/70-power-switch.rules and add the following line in it
> if it doesn't exist.
> 
> SUBSYSTEM=="input", KERNEL=="event*", SUBSYSTEMS=="platform",
> ATTRS{keys}=="116", TAG+="power-switch"
> 
> If your guest FS uses acpid, you should check it has
> /etc/acpi/powerbtn.sh and /etc/acpi/events/powerbtn. Refer to [4] for
> the setup of acpid.
> 
> Thanks,
> Shannon
> 
> [1] https://git.linaro.org/people/shannon.zhao/qemu.git/shortlog/refs/heads/PowerButton_v2
> [2] https://lkml.org/lkml/2015/9/30/392
> [3] http://people.linaro.org/~shannon.zhao/ACPI_ARM/QEMU_EFI.fd
> [4] https://wiki.linaro.org/LEG/Engineering/Kernel/ACPI/GPIOPowerButton
> 
> Changes since v1:
> * rewrite GPIO Connection Descriptor (Michael)
> 
> Shannon Zhao (8):
>   hw/arm/virt: Add a GPIO controller
>   hw/arm/virt-acpi-build: Add GPIO controller in ACPI DSDT table
>   hw/arm/virt-acpi-build: Add power button device in ACPI DSDT table
>   hw/acpi/aml-build: Add GPIO Connection Descriptor
>   hw/acpi/aml-build: Add a wrapper for GPIO Interrupt Connection
>   hw/arm/virt-acpi-build: Add _E03 for Power Button
>   hw/arm/virt: Add QEMU powerdown notifier and hook it to GPIO Pin 3
>   hw/arm/virt: Add gpio-keys node for Poweroff using DT
> 
>  hw/acpi/aml-build.c         | 79 +++++++++++++++++++++++++++++++++++++++++++++
>  hw/arm/virt-acpi-build.c    | 45 ++++++++++++++++++++++++++
>  hw/arm/virt.c               | 60 ++++++++++++++++++++++++++++++++++
>  include/hw/acpi/aml-build.h | 26 +++++++++++++++
>  include/hw/arm/virt.h       |  1 +
>  5 files changed, 211 insertions(+)
> 

Re: [Qemu-devel] [PATCH v2 0/8] Add system_powerdown support on ARM for ACPI and DT

[Shannon Zhao]

On 2015/10/30 2:17, Wei Huang wrote:
> On 10/29/2015 09:27 AM, Shannon Zhao wrote:
>> ACPI SPEC 5.0 defines GPIO-signaled ACPI Events for Hardware-reduced
>> platforms(like ARM). It uses GPIO pin to trigger an event to the guest.
>> For QEMU, here we add PL061 GPIO controller and use PIN 3 for
>> system_powerdown, reserving PIN 0, 1, 2 for PCI hotplug, CPU hotplug and
>> memory hotplug.
>>
>> This patchset adds system_powerdown support on ARM through both ACPI and
>> DT ways. It adds a GPIO controller(here is PL061) in machine virt and
>> uses GPIO-singled event for ACPI while gpio-keys for DT. It can be
>> fetched from [1] and has been tested for the guests starting by ACPI or
>> DT while guests use systemd or acpid.
>>
>> a) ACPI way. Since Graeme send a patchset to make ACPI on ARM64 support
>> amba device[2], it could use PL061 directly without modification to its
>> kernel driver code. In addition, we should use ACPI to start VM,
>> referring to below script. QEMU_EFI.fd can be fetched from [3]. 
> 
> Hi Shannon,
> 
> Thanks for re-sending it. This is a desired feature because we don't
> want to rely on tricks (such as guest-agent) for external VM power
> management. I have tested V1 recently by back-porting to my in-house
> kernel; it worked well.
> 
> I will help review this new version.
> 
Thanks for your help:)

-- 
Shannon

Re: [Qemu-devel] [PATCH v2 0/8] Add system_powerdown support on ARM for ACPI and DT

[Wei Huang]

On 10/29/2015 09:27 AM, Shannon Zhao wrote:
> ACPI SPEC 5.0 defines GPIO-signaled ACPI Events for Hardware-reduced
> platforms(like ARM). It uses GPIO pin to trigger an event to the guest.
> For QEMU, here we add PL061 GPIO controller and use PIN 3 for
> system_powerdown, reserving PIN 0, 1, 2 for PCI hotplug, CPU hotplug and
> memory hotplug.
> 
> This patchset adds system_powerdown support on ARM through both ACPI and
> DT ways. It adds a GPIO controller(here is PL061) in machine virt and
> uses GPIO-singled event for ACPI while gpio-keys for DT. It can be
> fetched from [1] and has been tested for the guests starting by ACPI or
> DT while guests use systemd or acpid.
> 
> a) ACPI way. Since Graeme send a patchset to make ACPI on ARM64 support
> amba device[2], it could use PL061 directly without modification to its
> kernel driver code. In addition, we should use ACPI to start VM,
> referring to below script. QEMU_EFI.fd can be fetched from [3]. 
> 
>     qemu-system-aarch64 \
>     -smp 1 -m 1024 -M virt \
>     -cpu cortex-a57 -nographic \
>     -monitor telnet::4444,server,nowait \
>     -bios QEMU_EFI.fd \
>     -kernel Image \
>     -initrd guestfs.cpio.gz \
>     -append "rdinit=/sbin/init console=ttyAMA0 mem=512M root=/dev/ram 
> earlycon=pl011,0x9000000 rw acpi=force"
> 
> b) DT way. Start vm as usual.
> 
>     qemu-system-aarch64 \
>     -smp 1 -m 1024 -M virt \
>     -cpu cortex-a57 -nographic \
>     -monitor telnet::4444,server,nowait \
>     -kernel Image \
>     -initrd guestfs.cpio.gz \
>     -append "rdinit=/sbin/init console=ttyAMA0 mem=512M root=/dev/ram 
> earlycon=pl011,0x9000000 rw"
> 
> Guest internal setup:
> If your guest FS uses systemd, you should check file
> /lib/udev/rules.d/70-power-switch.rules and add the following line in it
> if it doesn't exist.
> 
> SUBSYSTEM=="input", KERNEL=="event*", SUBSYSTEMS=="platform",
> ATTRS{keys}=="116", TAG+="power-switch"
> 
> If your guest FS uses acpid, you should check it has
> /etc/acpi/powerbtn.sh and /etc/acpi/events/powerbtn. Refer to [4] for
> the setup of acpid.
> 
> Thanks,
> Shannon
> 
> [1] https://git.linaro.org/people/shannon.zhao/qemu.git/shortlog/refs/heads/PowerButton_v2
> [2] https://lkml.org/lkml/2015/9/30/392
> [3] http://people.linaro.org/~shannon.zhao/ACPI_ARM/QEMU_EFI.fd
> [4] https://wiki.linaro.org/LEG/Engineering/Kernel/ACPI/GPIOPowerButton

Compared with v1, this version doesn't work with system_powerdown
command. In my setup, the testing environment is exactly same for both
v1 and v2. So something changed in v2 caused system_powerdown fail to work.

-Wei

> 
> Changes since v1:
> * rewrite GPIO Connection Descriptor (Michael)
> 
> Shannon Zhao (8):
>   hw/arm/virt: Add a GPIO controller
>   hw/arm/virt-acpi-build: Add GPIO controller in ACPI DSDT table
>   hw/arm/virt-acpi-build: Add power button device in ACPI DSDT table
>   hw/acpi/aml-build: Add GPIO Connection Descriptor
>   hw/acpi/aml-build: Add a wrapper for GPIO Interrupt Connection
>   hw/arm/virt-acpi-build: Add _E03 for Power Button
>   hw/arm/virt: Add QEMU powerdown notifier and hook it to GPIO Pin 3
>   hw/arm/virt: Add gpio-keys node for Poweroff using DT
> 
>  hw/acpi/aml-build.c         | 79 +++++++++++++++++++++++++++++++++++++++++++++
>  hw/arm/virt-acpi-build.c    | 45 ++++++++++++++++++++++++++
>  hw/arm/virt.c               | 60 ++++++++++++++++++++++++++++++++++
>  include/hw/acpi/aml-build.h | 26 +++++++++++++++
>  include/hw/arm/virt.h       |  1 +
>  5 files changed, 211 insertions(+)
> 

Re: [Qemu-devel] [PATCH v2 0/8] Add system_powerdown support on ARM for ACPI and DT

[Wei Huang]

On 11/10/2015 11:38 AM, Wei Huang wrote:
> 
> 
> On 10/29/2015 09:27 AM, Shannon Zhao wrote:
>> ACPI SPEC 5.0 defines GPIO-signaled ACPI Events for Hardware-reduced
>> platforms(like ARM). It uses GPIO pin to trigger an event to the guest.
>> For QEMU, here we add PL061 GPIO controller and use PIN 3 for
>> system_powerdown, reserving PIN 0, 1, 2 for PCI hotplug, CPU hotplug and
>> memory hotplug.
>>
>> This patchset adds system_powerdown support on ARM through both ACPI and
>> DT ways. It adds a GPIO controller(here is PL061) in machine virt and
>> uses GPIO-singled event for ACPI while gpio-keys for DT. It can be
>> fetched from [1] and has been tested for the guests starting by ACPI or
>> DT while guests use systemd or acpid.
>>
>> a) ACPI way. Since Graeme send a patchset to make ACPI on ARM64 support
>> amba device[2], it could use PL061 directly without modification to its
>> kernel driver code. In addition, we should use ACPI to start VM,
>> referring to below script. QEMU_EFI.fd can be fetched from [3]. 
>>
>>     qemu-system-aarch64 \
>>     -smp 1 -m 1024 -M virt \
>>     -cpu cortex-a57 -nographic \
>>     -monitor telnet::4444,server,nowait \
>>     -bios QEMU_EFI.fd \
>>     -kernel Image \
>>     -initrd guestfs.cpio.gz \
>>     -append "rdinit=/sbin/init console=ttyAMA0 mem=512M root=/dev/ram 
>> earlycon=pl011,0x9000000 rw acpi=force"
>>
>> b) DT way. Start vm as usual.
>>
>>     qemu-system-aarch64 \
>>     -smp 1 -m 1024 -M virt \
>>     -cpu cortex-a57 -nographic \
>>     -monitor telnet::4444,server,nowait \
>>     -kernel Image \
>>     -initrd guestfs.cpio.gz \
>>     -append "rdinit=/sbin/init console=ttyAMA0 mem=512M root=/dev/ram 
>> earlycon=pl011,0x9000000 rw"
>>
>> Guest internal setup:
>> If your guest FS uses systemd, you should check file
>> /lib/udev/rules.d/70-power-switch.rules and add the following line in it
>> if it doesn't exist.
>>
>> SUBSYSTEM=="input", KERNEL=="event*", SUBSYSTEMS=="platform",
>> ATTRS{keys}=="116", TAG+="power-switch"
>>
>> If your guest FS uses acpid, you should check it has
>> /etc/acpi/powerbtn.sh and /etc/acpi/events/powerbtn. Refer to [4] for
>> the setup of acpid.
>>
>> Thanks,
>> Shannon
>>
>> [1] https://git.linaro.org/people/shannon.zhao/qemu.git/shortlog/refs/heads/PowerButton_v2
>> [2] https://lkml.org/lkml/2015/9/30/392
>> [3] http://people.linaro.org/~shannon.zhao/ACPI_ARM/QEMU_EFI.fd
>> [4] https://wiki.linaro.org/LEG/Engineering/Kernel/ACPI/GPIOPowerButton
> 
> Compared with v1, this version doesn't work with system_powerdown
> command. In my setup, the testing environment is exactly same for both
> v1 and v2. So something changed in v2 caused system_powerdown fail to work.
> 

I found this was caused by the change of "_HID" name for GPIO device. It
was changed from "LNRO0009" (v1) to "ARMH0061" (v2), which doesn't match
with my stock guest kernel PL061 driver. After changing the guest
kernel, it is working again. So:

Tested-by: Wei Huang <wei@redhat.com>

I will provide reviews to each individual patches.

> -Wei
> 
>>
>> Changes since v1:
>> * rewrite GPIO Connection Descriptor (Michael)
>>
>> Shannon Zhao (8):
>>   hw/arm/virt: Add a GPIO controller
>>   hw/arm/virt-acpi-build: Add GPIO controller in ACPI DSDT table
>>   hw/arm/virt-acpi-build: Add power button device in ACPI DSDT table
>>   hw/acpi/aml-build: Add GPIO Connection Descriptor
>>   hw/acpi/aml-build: Add a wrapper for GPIO Interrupt Connection
>>   hw/arm/virt-acpi-build: Add _E03 for Power Button
>>   hw/arm/virt: Add QEMU powerdown notifier and hook it to GPIO Pin 3
>>   hw/arm/virt: Add gpio-keys node for Poweroff using DT
>>
>>  hw/acpi/aml-build.c         | 79 +++++++++++++++++++++++++++++++++++++++++++++
>>  hw/arm/virt-acpi-build.c    | 45 ++++++++++++++++++++++++++
>>  hw/arm/virt.c               | 60 ++++++++++++++++++++++++++++++++++
>>  include/hw/acpi/aml-build.h | 26 +++++++++++++++
>>  include/hw/arm/virt.h       |  1 +
>>  5 files changed, 211 insertions(+)
>>
> 

Re: [Qemu-devel] [PATCH v2 1/8] hw/arm/virt: Add a GPIO controller

[Wei Huang]

On 10/29/2015 09:27 AM, Shannon Zhao wrote:
> ACPI 5.0 supports GPIO-signaled ACPI Events. This can be used for
> powerdown, hotplug evnets. Add a GPIO controller in machine virt,
> to support powerdown, maybe can be used for cpu hotplug. And
> here we use pl061.
> 
> Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  hw/arm/virt.c         | 30 ++++++++++++++++++++++++++++++
>  include/hw/arm/virt.h |  1 +
>  2 files changed, 31 insertions(+)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 77d9267..5c505a4 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -120,6 +120,7 @@ static const MemMapEntry a15memmap[] = {
>      [VIRT_UART] =               { 0x09000000, 0x00001000 },
>      [VIRT_RTC] =                { 0x09010000, 0x00001000 },
>      [VIRT_FW_CFG] =             { 0x09020000, 0x00000018 },
> +    [VIRT_GPIO] =               { 0x09030000, 0x00001000 },
>      [VIRT_MMIO] =               { 0x0a000000, 0x00000200 },
>      /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
>      [VIRT_PLATFORM_BUS] =       { 0x0c000000, 0x02000000 },
> @@ -135,6 +136,7 @@ static const int a15irqmap[] = {
>      [VIRT_UART] = 1,
>      [VIRT_RTC] = 2,
>      [VIRT_PCIE] = 3, /* ... to 6 */
> +    [VIRT_GPIO] = 7,
>      [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
>      [VIRT_GIC_V2M] = 48, /* ...to 48 + NUM_GICV2M_SPIS - 1 */
>      [VIRT_PLATFORM_BUS] = 112, /* ...to 112 + PLATFORM_BUS_NUM_IRQS -1 */
> @@ -538,6 +540,32 @@ static void create_rtc(const VirtBoardInfo *vbi, qemu_irq *pic)
>      g_free(nodename);
>  }
>  
> +static void create_gpio(const VirtBoardInfo *vbi, qemu_irq *pic)
> +{
> +    char *nodename;
> +    hwaddr base = vbi->memmap[VIRT_GPIO].base;
> +    hwaddr size = vbi->memmap[VIRT_GPIO].size;
> +    int irq = vbi->irqmap[VIRT_GPIO];
> +    const char compat[] = "arm,pl061\0arm,primecell";

The compat list has a small enhancement compared with v1. This is
consistent with PL061 binding document. Other parts look good to me.

Reviewed-by: Wei Huang <wei@redhat.com>

> +
> +    sysbus_create_simple("pl061", base, pic[irq]);
> +
> +    nodename = g_strdup_printf("/pl061@%" PRIx64, base);
> +    qemu_fdt_add_subnode(vbi->fdt, nodename);
> +    qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg",
> +                                 2, base, 2, size);
> +    qemu_fdt_setprop(vbi->fdt, nodename, "compatible", compat, sizeof(compat));
> +    qemu_fdt_setprop_cell(vbi->fdt, nodename, "#gpio-cells", 2);
> +    qemu_fdt_setprop(vbi->fdt, nodename, "gpio-controller", NULL, 0);
> +    qemu_fdt_setprop_cells(vbi->fdt, nodename, "interrupts",
> +                           GIC_FDT_IRQ_TYPE_SPI, irq,
> +                           GIC_FDT_IRQ_FLAGS_LEVEL_HI);
> +    qemu_fdt_setprop_cell(vbi->fdt, nodename, "clocks", vbi->clock_phandle);
> +    qemu_fdt_setprop_string(vbi->fdt, nodename, "clock-names", "apb_pclk");
> +
> +    g_free(nodename);
> +}
> +
>  static void create_virtio_devices(const VirtBoardInfo *vbi, qemu_irq *pic)
>  {
>      int i;
> @@ -1041,6 +1069,8 @@ static void machvirt_init(MachineState *machine)
>  
>      create_pcie(vbi, pic, vms->highmem);
>  
> +    create_gpio(vbi, pic);
> +
>      /* Create mmio transports, so the user can create virtio backends
>       * (which will be automatically plugged in to the transports). If
>       * no backend is created the transport will just sit harmlessly idle.
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index f464586..925faa7 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -59,6 +59,7 @@ enum {
>      VIRT_PCIE_ECAM,
>      VIRT_PLATFORM_BUS,
>      VIRT_PCIE_MMIO_HIGH,
> +    VIRT_GPIO,
>  };
>  
>  typedef struct MemMapEntry {
> 

Re: [Qemu-devel] [PATCH v2 0/8] Add system_powerdown support on ARM for ACPI and DT

[Shannon Zhao]

On 2015/11/11 4:56, Wei Huang wrote:
> 
>>
>> Compared with v1, this version doesn't work with system_powerdown
>> command. In my setup, the testing environment is exactly same for both
>> v1 and v2. So something changed in v2 caused system_powerdown fail to work.
>>
> 
> I found this was caused by the change of "_HID" name for GPIO device. It
> was changed from "LNRO0009" (v1) to "ARMH0061" (v2), which doesn't match
> with my stock guest kernel PL061 driver. After changing the guest
> kernel, it is working again. So:
> 
Hi Wei,

Thanks very much for your help. The reason why I change the _HID is
based on the _HID 0f PL011 which is ARMH0011. About the _HID of ARM
company's devices, I have to say that I didn't see which _HID they
should be at any public place. I heard(maybe it's not correct) there is
a _HID list of ARM devices. If so, I think ARM should publish them in
public, otherwise the only thing we can do is to guess or refer to
existing _HID.

BTW, I think you could use this kernel patches[1] which adds ACPI
support for AMBA device and drop the previous one.

[1] https://lkml.org/lkml/2015/9/30/392

> Tested-by: Wei Huang <wei@redhat.com>
> 
> I will provide reviews to each individual patches.
> 
Thanks again. :)

-- 
Shannon

Re: [Qemu-devel] [PATCH v2 3/8] hw/arm/virt-acpi-build: Add power button device in ACPI DSDT table

[Wei Huang]

On 10/29/2015 09:27 AM, Shannon Zhao wrote:
> Add power button device in ACPI DSDT table.
> 
> Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  hw/arm/virt-acpi-build.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index ffb9b91..80ce51c 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -339,6 +339,18 @@ static void acpi_dsdt_add_gpio(Aml *scope, const MemMapEntry *gpio_memmap,
>      aml_append(scope, dev);
>  }
>  
> +static void acpi_dsdt_add_power_button(Aml *scope)
> +{
> +    Aml *dev = aml_device("PWRB");
> +    aml_append(dev, aml_name_decl("_HID", aml_string("PNP0C0C")));
> +    aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
> +    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
> +    Aml *method = aml_method("_STA", 0);
> +    aml_append(method, aml_return(aml_int(0x0F)));

_STA returns 0x0F means PWRB is a functional device; that is good for
OSPM. Plus I have tested the whole thing using ACPI-enabled guest VM;
that means PWRB was probed correctly. So,

Reviewed-by: Wei Huang <wei@redhat.com>

> +    aml_append(dev, method);
> +    aml_append(scope, dev);
> +}
> +
>  /* RSDP */
>  static GArray *
>  build_rsdp(GArray *rsdp_table, GArray *linker, unsigned rsdt)
> @@ -553,6 +565,7 @@ build_dsdt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
>                        guest_info->use_highmem);
>      acpi_dsdt_add_gpio(scope, &memmap[VIRT_GPIO],
>                         (irqmap[VIRT_GPIO] + ARM_SPI_BASE));
> +    acpi_dsdt_add_power_button(scope);
>  
>      aml_append(dsdt, scope);
>  
> 

Re: [Qemu-devel] [PATCH v2 7/8] hw/arm/virt: Add QEMU powerdown notifier and hook it to GPIO Pin 3

[Wei Huang]

On 10/29/2015 09:27 AM, Shannon Zhao wrote:
> Currently mach-virt model doesn't support powerdown request. Guest VM
> doesn't react to system_powerdown from monitor console (or QMP) because
> there is no communication mechanism for such requests. This patch registers
> GPIO Pin 3 with powerdown notification. So guest VM can receive notification
> when such powerdown request is triggered.
> 
> Signed-off-by: Wei Huang <wei@redhat.com>
> Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  hw/arm/virt.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 5c505a4..6f1b9d6 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -540,6 +540,18 @@ static void create_rtc(const VirtBoardInfo *vbi, qemu_irq *pic)
>      g_free(nodename);
>  }
>  
> +static DeviceState *pl061_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(pl061_dev, 3), 0);
> +    qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 1);

This pull-down and then pull-up is for extra safety, right? It actually
also works with only pull-up call. But I am OK with your design here;
please add additional comments to explain the design.

> +}
> +
> +static Notifier virt_system_powerdown_notifier = {
> +    .notify = virt_powerdown_req
> +};
> +
>  static void create_gpio(const VirtBoardInfo *vbi, qemu_irq *pic)
>  {
>      char *nodename;
> @@ -548,7 +560,7 @@ static void create_gpio(const VirtBoardInfo *vbi, qemu_irq *pic)
>      int irq = vbi->irqmap[VIRT_GPIO];
>      const char compat[] = "arm,pl061\0arm,primecell";
>  
> -    sysbus_create_simple("pl061", base, pic[irq]);
> +    pl061_dev = sysbus_create_simple("pl061", base, pic[irq]);
>  
>      nodename = g_strdup_printf("/pl061@%" PRIx64, base);
>      qemu_fdt_add_subnode(vbi->fdt, nodename);
> @@ -563,6 +575,9 @@ static void create_gpio(const VirtBoardInfo *vbi, qemu_irq *pic)
>      qemu_fdt_setprop_cell(vbi->fdt, nodename, "clocks", vbi->clock_phandle);
>      qemu_fdt_setprop_string(vbi->fdt, nodename, "clock-names", "apb_pclk");
>  
> +    /* connect powerdown request */
> +    qemu_register_powerdown_notifier(&virt_system_powerdown_notifier);
> +
>      g_free(nodename);
>  }
>  
> 

Reviewed-by: Wei Huang <wei@redhat.com>

Re: [Qemu-devel] [PATCH v2 0/8] Add system_powerdown support on ARM for ACPI and DT

[Peter Maydell]

On 11 November 2015 at 01:29, Shannon Zhao <shannon.zhao@linaro.org> wrote:
> On 2015/11/11 4:56, Wei Huang wrote:
>> I found this was caused by the change of "_HID" name for GPIO device. It
>> was changed from "LNRO0009" (v1) to "ARMH0061" (v2), which doesn't match
>> with my stock guest kernel PL061 driver. After changing the guest
>> kernel, it is working again. So:

> Thanks very much for your help. The reason why I change the _HID is
> based on the _HID 0f PL011 which is ARMH0011. About the _HID of ARM
> company's devices, I have to say that I didn't see which _HID they
> should be at any public place. I heard(maybe it's not correct) there is
> a _HID list of ARM devices. If so, I think ARM should publish them in
> public, otherwise the only thing we can do is to guess or refer to
> existing _HID.

Please confirm for definite the right _HID values; don't guess
them. If you say you don't know the right HID values and mark
a patchset as RFC we can go and try to find out the right
answers. If you just put guesses into a patchset we could
easily end up committing the patch with the wrong info...

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 7/8] hw/arm/virt: Add QEMU powerdown notifier and hook it to GPIO Pin 3

[Peter Maydell]

On 11 November 2015 at 06:57, Wei Huang <wei@redhat.com> wrote:
>
>
> On 10/29/2015 09:27 AM, Shannon Zhao wrote:
>> Currently mach-virt model doesn't support powerdown request. Guest VM
>> doesn't react to system_powerdown from monitor console (or QMP) because
>> there is no communication mechanism for such requests. This patch registers
>> GPIO Pin 3 with powerdown notification. So guest VM can receive notification
>> when such powerdown request is triggered.
>>
>> Signed-off-by: Wei Huang <wei@redhat.com>
>> Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>> ---
>>  hw/arm/virt.c | 17 ++++++++++++++++-
>>  1 file changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index 5c505a4..6f1b9d6 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -540,6 +540,18 @@ static void create_rtc(const VirtBoardInfo *vbi, qemu_irq *pic)
>>      g_free(nodename);
>>  }
>>
>> +static DeviceState *pl061_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(pl061_dev, 3), 0);
>> +    qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 1);
>
> This pull-down and then pull-up is for extra safety, right? It actually
> also works with only pull-up call. But I am OK with your design here;
> please add additional comments to explain the design.

It's a bit weird though. Usually you use qemu_pulse_irq,
which does a 0->1 then 1->0 transition, if the thing you're
signalling needs an edge. If the thing you're signalling
only cares about level then you just need to set the
level to 1.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 0/8] Add system_powerdown support on ARM for ACPI and DT

[Shannon Zhao]

On 2015/11/11 16:36, Peter Maydell wrote:
> On 11 November 2015 at 01:29, Shannon Zhao <shannon.zhao@linaro.org> wrote:
>> On 2015/11/11 4:56, Wei Huang wrote:
>>> I found this was caused by the change of "_HID" name for GPIO device. It
>>> was changed from "LNRO0009" (v1) to "ARMH0061" (v2), which doesn't match
>>> with my stock guest kernel PL061 driver. After changing the guest
>>> kernel, it is working again. So:
> 
>> Thanks very much for your help. The reason why I change the _HID is
>> based on the _HID 0f PL011 which is ARMH0011. About the _HID of ARM
>> company's devices, I have to say that I didn't see which _HID they
>> should be at any public place. I heard(maybe it's not correct) there is
>> a _HID list of ARM devices. If so, I think ARM should publish them in
>> public, otherwise the only thing we can do is to guess or refer to
>> existing _HID.
> 
> Please confirm for definite the right _HID values; don't guess
> them. If you say you don't know the right HID values and mark
> a patchset as RFC we can go and try to find out the right
> answers. If you just put guesses into a patchset we could
> easily end up committing the patch with the wrong info...
> 
Of course I will try to find the right _HID and tried before. But what I
want to say is why ARM doesn't publish these _HIDs if there is really a
list( I don't think this is a secret). And if there is not a list, ARM
should create one since these devices belong to ARM.

Thanks,
-- 
Shannon

Re: [Qemu-devel] [PATCH v2 0/8] Add system_powerdown support on ARM for ACPI and DT

[Peter Maydell]

On 29 October 2015 at 14:27, Shannon Zhao <shannon.zhao@linaro.org> wrote:
> ACPI SPEC 5.0 defines GPIO-signaled ACPI Events for Hardware-reduced
> platforms(like ARM). It uses GPIO pin to trigger an event to the guest.
> For QEMU, here we add PL061 GPIO controller and use PIN 3 for
> system_powerdown, reserving PIN 0, 1, 2 for PCI hotplug, CPU hotplug and
> memory hotplug.
>
> This patchset adds system_powerdown support on ARM through both ACPI and
> DT ways. It adds a GPIO controller(here is PL061) in machine virt and
> uses GPIO-singled event for ACPI while gpio-keys for DT. It can be
> fetched from [1] and has been tested for the guests starting by ACPI or
> DT while guests use systemd or acpid.

> Changes since v1:
> * rewrite GPIO Connection Descriptor (Michael)
>
> Shannon Zhao (8):
>   hw/arm/virt: Add a GPIO controller
>   hw/arm/virt-acpi-build: Add GPIO controller in ACPI DSDT table
>   hw/arm/virt-acpi-build: Add power button device in ACPI DSDT table
>   hw/acpi/aml-build: Add GPIO Connection Descriptor
>   hw/acpi/aml-build: Add a wrapper for GPIO Interrupt Connection
>   hw/arm/virt-acpi-build: Add _E03 for Power Button
>   hw/arm/virt: Add QEMU powerdown notifier and hook it to GPIO Pin 3
>   hw/arm/virt: Add gpio-keys node for Poweroff using DT

Thanks; this patchset looks generally good to me (I had a few
minor remarks about patch 8). I would appreciate review from
folks familiar with the ACPI spec for patches 2, 3, 4, 5 and 6, though.

-- PMM

Re: [Qemu-devel] [PATCH v2 0/8] Add system_powerdown support on ARM for ACPI and DT

[Igor Mammedov]

On Fri, 27 Nov 2015 17:18:06 +0000
Peter Maydell <peter.maydell@linaro.org> wrote:

> On 29 October 2015 at 14:27, Shannon Zhao <shannon.zhao@linaro.org> wrote:
> > ACPI SPEC 5.0 defines GPIO-signaled ACPI Events for Hardware-reduced
> > platforms(like ARM). It uses GPIO pin to trigger an event to the guest.
> > For QEMU, here we add PL061 GPIO controller and use PIN 3 for
> > system_powerdown, reserving PIN 0, 1, 2 for PCI hotplug, CPU hotplug and
> > memory hotplug.
> >
> > This patchset adds system_powerdown support on ARM through both ACPI and
> > DT ways. It adds a GPIO controller(here is PL061) in machine virt and
> > uses GPIO-singled event for ACPI while gpio-keys for DT. It can be
> > fetched from [1] and has been tested for the guests starting by ACPI or
> > DT while guests use systemd or acpid.
> 
> > Changes since v1:
> > * rewrite GPIO Connection Descriptor (Michael)
> >
> > Shannon Zhao (8):
> >   hw/arm/virt: Add a GPIO controller
> >   hw/arm/virt-acpi-build: Add GPIO controller in ACPI DSDT table
> >   hw/arm/virt-acpi-build: Add power button device in ACPI DSDT table
> >   hw/acpi/aml-build: Add GPIO Connection Descriptor
> >   hw/acpi/aml-build: Add a wrapper for GPIO Interrupt Connection
> >   hw/arm/virt-acpi-build: Add _E03 for Power Button
> >   hw/arm/virt: Add QEMU powerdown notifier and hook it to GPIO Pin 3
> >   hw/arm/virt: Add gpio-keys node for Poweroff using DT
> 
> Thanks; this patchset looks generally good to me (I had a few
> minor remarks about patch 8). I would appreciate review from
> folks familiar with the ACPI spec for patches 2, 3, 4, 5 and 6, though.
I'll do it.

> 
> -- PMM
>