v8: - use gpio 0 and 1, align dtb with kernel gpio-restart, gpio-poweroff, change define names, trigger on upper front. (Peter Maydell). v7: - same as v6, but resplit patches: patch 2 no function changes and refactor gpio setup for virt platfrom and patch 3 adds secure gpio. v6: - 64k align gpio memory region (Andrew Jones) - adjusted memory region to map this address in the corresponding atf patch v5: - removed vms flag, added fdt (Andrew Jones) - added patch3 to combine secure and non secure pl061. It has to be more easy to review if this changes are in the separate patch. v4: rework patches accodring to Peter Maydells comments: - split patches on gpio-pwr driver and arm-virt integration. - start secure gpio only from virt-6.0. - rework qemu interface for gpio-pwr to use 2 named gpio. - put secure gpio to secure name space. v3: added missed include qemu/log.h for qemu_log(.. v2: replace printf with qemu_log (Philippe Mathieu-Daudé)
This patch works together with ATF patch: https://github.com/muvarov/arm-trusted-firmware/commit/886965bddb0624bdf8510...
Maxim Uvarov (3): hw: gpio: implement gpio-pwr driver for qemu reset/poweroff arm-virt: refactor gpios creation arm-virt: add secure pl061 for reset/power down
hw/arm/Kconfig | 1 + hw/arm/virt.c | 111 ++++++++++++++++++++++++++++++++++-------- hw/gpio/Kconfig | 3 ++ hw/gpio/gpio_pwr.c | 70 ++++++++++++++++++++++++++ hw/gpio/meson.build | 1 + include/hw/arm/virt.h | 2 + 6 files changed, 167 insertions(+), 21 deletions(-) create mode 100644 hw/gpio/gpio_pwr.c
Implement gpio-pwr driver to allow reboot and poweroff machine. This is simple driver with just 2 gpios lines. Current use case is to reboot and poweroff virt machine in secure mode. Secure pl066 gpio chip is needed for that.
Signed-off-by: Maxim Uvarov maxim.uvarov@linaro.org Reviewed-by: Hao Wu wuhaotsh@google.com --- hw/gpio/Kconfig | 3 ++ hw/gpio/gpio_pwr.c | 70 +++++++++++++++++++++++++++++++++++++++++++++ hw/gpio/meson.build | 1 + 3 files changed, 74 insertions(+) create mode 100644 hw/gpio/gpio_pwr.c
diff --git a/hw/gpio/Kconfig b/hw/gpio/Kconfig index b6fdaa2586..f0e7405f6e 100644 --- a/hw/gpio/Kconfig +++ b/hw/gpio/Kconfig @@ -8,5 +8,8 @@ config PL061 config GPIO_KEY bool
+config GPIO_PWR + bool + config SIFIVE_GPIO bool diff --git a/hw/gpio/gpio_pwr.c b/hw/gpio/gpio_pwr.c new file mode 100644 index 0000000000..7714fa0dc4 --- /dev/null +++ b/hw/gpio/gpio_pwr.c @@ -0,0 +1,70 @@ +/* + * GPIO qemu power controller + * + * Copyright (c) 2020 Linaro Limited + * + * Author: Maxim Uvarov maxim.uvarov@linaro.org + * + * Virtual gpio driver which can be used on top of pl061 + * to reboot and shutdown qemu virtual machine. One of use + * case is gpio driver for secure world application (ARM + * Trusted Firmware.). + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +/* + * QEMU interface: + * two named input GPIO lines: + * 'reset' : when asserted, trigger system reset + * 'shutdown' : when asserted, trigger system shutdown + */ + +#include "qemu/osdep.h" +#include "hw/sysbus.h" +#include "sysemu/runstate.h" + +#define TYPE_GPIOPWR "gpio-pwr" +OBJECT_DECLARE_SIMPLE_TYPE(GPIO_PWR_State, GPIOPWR) + +struct GPIO_PWR_State { + SysBusDevice parent_obj; +}; + +static void gpio_pwr_reset(void *opaque, int n, int level) +{ + if (level) { + qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET); + } +} + +static void gpio_pwr_shutdown(void *opaque, int n, int level) +{ + if (level) { + qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN); + } +} + +static void gpio_pwr_init(Object *obj) +{ + DeviceState *dev = DEVICE(obj); + + qdev_init_gpio_in_named(dev, gpio_pwr_reset, "reset", 1); + qdev_init_gpio_in_named(dev, gpio_pwr_shutdown, "shutdown", 1); +} + +static const TypeInfo gpio_pwr_info = { + .name = TYPE_GPIOPWR, + .parent = TYPE_SYS_BUS_DEVICE, + .instance_size = sizeof(GPIO_PWR_State), + .instance_init = gpio_pwr_init, +}; + +static void gpio_pwr_register_types(void) +{ + type_register_static(&gpio_pwr_info); +} + +type_init(gpio_pwr_register_types) diff --git a/hw/gpio/meson.build b/hw/gpio/meson.build index 5c0a7d7b95..79568f00ce 100644 --- a/hw/gpio/meson.build +++ b/hw/gpio/meson.build @@ -1,5 +1,6 @@ softmmu_ss.add(when: 'CONFIG_E500', if_true: files('mpc8xxx.c')) softmmu_ss.add(when: 'CONFIG_GPIO_KEY', if_true: files('gpio_key.c')) +softmmu_ss.add(when: 'CONFIG_GPIO_PWR', if_true: files('gpio_pwr.c')) softmmu_ss.add(when: 'CONFIG_MAX7310', if_true: files('max7310.c')) softmmu_ss.add(when: 'CONFIG_PL061', if_true: files('pl061.c')) softmmu_ss.add(when: 'CONFIG_PUV3', if_true: files('puv3_gpio.c'))
On Wed, 20 Jan 2021 at 09:27, Maxim Uvarov maxim.uvarov@linaro.org wrote:
Implement gpio-pwr driver to allow reboot and poweroff machine. This is simple driver with just 2 gpios lines. Current use case is to reboot and poweroff virt machine in secure mode. Secure pl066 gpio chip is needed for that.
Signed-off-by: Maxim Uvarov maxim.uvarov@linaro.org Reviewed-by: Hao Wu wuhaotsh@google.com
hw/gpio/Kconfig | 3 ++ hw/gpio/gpio_pwr.c | 70 +++++++++++++++++++++++++++++++++++++++++++++ hw/gpio/meson.build | 1 + 3 files changed, 74 insertions(+) create mode 100644 hw/gpio/gpio_pwr.c
Reviewed-by: Peter Maydell peter.maydell@linaro.org
thanks -- PMM
No functional change. Just refactor code to better support secure and normal world gpios.
Signed-off-by: Maxim Uvarov maxim.uvarov@linaro.org --- hw/arm/virt.c | 64 ++++++++++++++++++++++++++++++++++----------------- 1 file changed, 43 insertions(+), 21 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 96985917d3..c427ce5f81 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -820,17 +820,43 @@ static void virt_powerdown_req(Notifier *n, void *opaque) } }
-static void create_gpio(const VirtMachineState *vms) +static void create_gpio_keys(const VirtMachineState *vms, + DeviceState *pl061_dev, + uint32_t phandle) +{ + gpio_key_dev = sysbus_create_simple("gpio-key", -1, + qdev_get_gpio_in(pl061_dev, 3)); + + 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); + qemu_fdt_setprop_cell(vms->fdt, "/gpio-keys", "#address-cells", 1); + + qemu_fdt_add_subnode(vms->fdt, "/gpio-keys/poweroff"); + qemu_fdt_setprop_string(vms->fdt, "/gpio-keys/poweroff", + "label", "GPIO Key Poweroff"); + qemu_fdt_setprop_cell(vms->fdt, "/gpio-keys/poweroff", "linux,code", + KEY_POWER); + qemu_fdt_setprop_cells(vms->fdt, "/gpio-keys/poweroff", + "gpios", phandle, 3, 0); +} + +static void create_gpio_devices(const VirtMachineState *vms, int gpio, + MemoryRegion *mem) { char *nodename; DeviceState *pl061_dev; - hwaddr base = vms->memmap[VIRT_GPIO].base; - hwaddr size = vms->memmap[VIRT_GPIO].size; - int irq = vms->irqmap[VIRT_GPIO]; + hwaddr base = vms->memmap[gpio].base; + hwaddr size = vms->memmap[gpio].size; + int irq = vms->irqmap[gpio]; const char compat[] = "arm,pl061\0arm,primecell"; + SysBusDevice *s;
- pl061_dev = sysbus_create_simple("pl061", base, - qdev_get_gpio_in(vms->gic, irq)); + pl061_dev = qdev_new("pl061"); + s = SYS_BUS_DEVICE(pl061_dev); + sysbus_realize_and_unref(s, &error_fatal); + memory_region_add_subregion(mem, base, sysbus_mmio_get_region(s, 0)); + sysbus_connect_irq(s, 0, qdev_get_gpio_in(vms->gic, irq));
uint32_t phandle = qemu_fdt_alloc_phandle(vms->fdt); nodename = g_strdup_printf("/pl061@%" PRIx64, base); @@ -847,21 +873,17 @@ static void create_gpio(const VirtMachineState *vms) qemu_fdt_setprop_string(vms->fdt, nodename, "clock-names", "apb_pclk"); qemu_fdt_setprop_cell(vms->fdt, nodename, "phandle", phandle);
- gpio_key_dev = sysbus_create_simple("gpio-key", -1, - qdev_get_gpio_in(pl061_dev, 3)); - 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); - qemu_fdt_setprop_cell(vms->fdt, "/gpio-keys", "#address-cells", 1); - - qemu_fdt_add_subnode(vms->fdt, "/gpio-keys/poweroff"); - qemu_fdt_setprop_string(vms->fdt, "/gpio-keys/poweroff", - "label", "GPIO Key Poweroff"); - qemu_fdt_setprop_cell(vms->fdt, "/gpio-keys/poweroff", "linux,code", - KEY_POWER); - qemu_fdt_setprop_cells(vms->fdt, "/gpio-keys/poweroff", - "gpios", phandle, 3, 0); + if (gpio != VIRT_GPIO) { + /* Mark as not usable by the normal world */ + qemu_fdt_setprop_string(vms->fdt, nodename, "status", "disabled"); + qemu_fdt_setprop_string(vms->fdt, nodename, "secure-status", "okay"); + } g_free(nodename); + + /* Child gpio devices */ + if (gpio == VIRT_GPIO) { + create_gpio_keys(vms, pl061_dev, phandle); + } }
static void create_virtio_devices(const VirtMachineState *vms) @@ -1990,7 +2012,7 @@ 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(vms); + create_gpio_devices(vms, VIRT_GPIO, sysmem); }
/* connect powerdown request */
On Wed, Jan 20, 2021 at 12:27:47PM +0300, Maxim Uvarov wrote:
No functional change. Just refactor code to better support secure and normal world gpios.
Signed-off-by: Maxim Uvarov maxim.uvarov@linaro.org
hw/arm/virt.c | 64 ++++++++++++++++++++++++++++++++++----------------- 1 file changed, 43 insertions(+), 21 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 96985917d3..c427ce5f81 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -820,17 +820,43 @@ static void virt_powerdown_req(Notifier *n, void *opaque) } } -static void create_gpio(const VirtMachineState *vms) +static void create_gpio_keys(const VirtMachineState *vms,
DeviceState *pl061_dev,
uint32_t phandle)
+{
- gpio_key_dev = sysbus_create_simple("gpio-key", -1,
qdev_get_gpio_in(pl061_dev, 3));
- 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);
- qemu_fdt_setprop_cell(vms->fdt, "/gpio-keys", "#address-cells", 1);
- qemu_fdt_add_subnode(vms->fdt, "/gpio-keys/poweroff");
- qemu_fdt_setprop_string(vms->fdt, "/gpio-keys/poweroff",
"label", "GPIO Key Poweroff");
- qemu_fdt_setprop_cell(vms->fdt, "/gpio-keys/poweroff", "linux,code",
KEY_POWER);
- qemu_fdt_setprop_cells(vms->fdt, "/gpio-keys/poweroff",
"gpios", phandle, 3, 0);
+}
+static void create_gpio_devices(const VirtMachineState *vms, int gpio,
MemoryRegion *mem)
{ char *nodename; DeviceState *pl061_dev;
- hwaddr base = vms->memmap[VIRT_GPIO].base;
- hwaddr size = vms->memmap[VIRT_GPIO].size;
- int irq = vms->irqmap[VIRT_GPIO];
- hwaddr base = vms->memmap[gpio].base;
- hwaddr size = vms->memmap[gpio].size;
- int irq = vms->irqmap[gpio]; const char compat[] = "arm,pl061\0arm,primecell";
- SysBusDevice *s;
- pl061_dev = sysbus_create_simple("pl061", base,
qdev_get_gpio_in(vms->gic, irq));
- pl061_dev = qdev_new("pl061");
- s = SYS_BUS_DEVICE(pl061_dev);
- sysbus_realize_and_unref(s, &error_fatal);
- memory_region_add_subregion(mem, base, sysbus_mmio_get_region(s, 0));
- sysbus_connect_irq(s, 0, qdev_get_gpio_in(vms->gic, irq));
uint32_t phandle = qemu_fdt_alloc_phandle(vms->fdt); nodename = g_strdup_printf("/pl061@%" PRIx64, base); @@ -847,21 +873,17 @@ static void create_gpio(const VirtMachineState *vms) qemu_fdt_setprop_string(vms->fdt, nodename, "clock-names", "apb_pclk"); qemu_fdt_setprop_cell(vms->fdt, nodename, "phandle", phandle);
- gpio_key_dev = sysbus_create_simple("gpio-key", -1,
qdev_get_gpio_in(pl061_dev, 3));
- 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);
- qemu_fdt_setprop_cell(vms->fdt, "/gpio-keys", "#address-cells", 1);
- qemu_fdt_add_subnode(vms->fdt, "/gpio-keys/poweroff");
- qemu_fdt_setprop_string(vms->fdt, "/gpio-keys/poweroff",
"label", "GPIO Key Poweroff");
- qemu_fdt_setprop_cell(vms->fdt, "/gpio-keys/poweroff", "linux,code",
KEY_POWER);
- qemu_fdt_setprop_cells(vms->fdt, "/gpio-keys/poweroff",
"gpios", phandle, 3, 0);
- if (gpio != VIRT_GPIO) {
/* Mark as not usable by the normal world */
qemu_fdt_setprop_string(vms->fdt, nodename, "status", "disabled");
qemu_fdt_setprop_string(vms->fdt, nodename, "secure-status", "okay");
- }
nit: The above if-block could/should have waited until the next patch to be added.
g_free(nodename);
- /* Child gpio devices */
- if (gpio == VIRT_GPIO) {
Same nit as above, the next patch is where we should start conditionally doing stuff based on the gpio type.
create_gpio_keys(vms, pl061_dev, phandle);
- }
} static void create_virtio_devices(const VirtMachineState *vms) @@ -1990,7 +2012,7 @@ 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(vms);
}create_gpio_devices(vms, VIRT_GPIO, sysmem);
/* connect powerdown request */ -- 2.17.1
Reviewed-by: Andrew Jones drjones@redhat.com
Thanks, drew
Add secure pl061 for reset/power down machine from the secure world (Arm Trusted Firmware). Connect it with gpio-pwr driver.
Signed-off-by: Maxim Uvarov maxim.uvarov@linaro.org --- hw/arm/Kconfig | 1 + hw/arm/virt.c | 47 +++++++++++++++++++++++++++++++++++++++++++ include/hw/arm/virt.h | 2 ++ 3 files changed, 50 insertions(+)
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig index 0a242e4c5d..13cc42dcc8 100644 --- a/hw/arm/Kconfig +++ b/hw/arm/Kconfig @@ -17,6 +17,7 @@ config ARM_VIRT select PL011 # UART select PL031 # RTC select PL061 # GPIO + select GPIO_PWR select PLATFORM_BUS select SMBIOS select VIRTIO_MMIO diff --git a/hw/arm/virt.c b/hw/arm/virt.c index c427ce5f81..060a5f492e 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -153,6 +153,7 @@ static const MemMapEntry base_memmap[] = { [VIRT_ACPI_GED] = { 0x09080000, ACPI_GED_EVT_SEL_LEN }, [VIRT_NVDIMM_ACPI] = { 0x09090000, NVDIMM_ACPI_IO_LEN}, [VIRT_PVTIME] = { 0x090a0000, 0x00010000 }, + [VIRT_SECURE_GPIO] = { 0x090b0000, 0x00001000 }, [VIRT_MMIO] = { 0x0a000000, 0x00000200 }, /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */ [VIRT_PLATFORM_BUS] = { 0x0c000000, 0x02000000 }, @@ -841,6 +842,43 @@ static void create_gpio_keys(const VirtMachineState *vms, "gpios", phandle, 3, 0); }
+#define SECURE_GPIO_POWEROFF 0 +#define SECURE_GPIO_REBOOT 1 + +static void create_gpio_pwr(const VirtMachineState *vms, + DeviceState *pl061_dev, + uint32_t phandle) +{ + DeviceState *gpio_pwr_dev; + + /* gpio-pwr */ + gpio_pwr_dev = sysbus_create_simple("gpio-pwr", -1, NULL); + + /* connect secure pl061 to gpio-pwr */ + qdev_connect_gpio_out(pl061_dev, SECURE_GPIO_REBOOT, + qdev_get_gpio_in_named(gpio_pwr_dev, "reset", 0)); + qdev_connect_gpio_out(pl061_dev, SECURE_GPIO_POWEROFF, + qdev_get_gpio_in_named(gpio_pwr_dev, "shutdown", 0)); + + qemu_fdt_add_subnode(vms->fdt, "/gpio-poweroff"); + qemu_fdt_setprop_string(vms->fdt, "/gpio-poweroff", "compatible", + "gpio-poweroff"); + qemu_fdt_setprop_cells(vms->fdt, "/gpio-poweroff", + "gpios", phandle, SECURE_GPIO_POWEROFF, 0); + qemu_fdt_setprop_string(vms->fdt, "/gpio-poweroff", "status", "disabled"); + qemu_fdt_setprop_string(vms->fdt, "/gpio-poweroff", "secure-status", + "okay"); + + qemu_fdt_add_subnode(vms->fdt, "/gpio-restart"); + qemu_fdt_setprop_string(vms->fdt, "/gpio-restart", "compatible", + "gpio-restart"); + qemu_fdt_setprop_cells(vms->fdt, "/gpio-restart", + "gpios", phandle, SECURE_GPIO_REBOOT, 0); + qemu_fdt_setprop_string(vms->fdt, "/gpio-restart", "status", "disabled"); + qemu_fdt_setprop_string(vms->fdt, "/gpio-restart", "secure-status", + "okay"); +} + static void create_gpio_devices(const VirtMachineState *vms, int gpio, MemoryRegion *mem) { @@ -883,6 +921,8 @@ static void create_gpio_devices(const VirtMachineState *vms, int gpio, /* Child gpio devices */ if (gpio == VIRT_GPIO) { create_gpio_keys(vms, pl061_dev, phandle); + } else { + create_gpio_pwr(vms, pl061_dev, phandle); } }
@@ -2015,6 +2055,10 @@ static void machvirt_init(MachineState *machine) create_gpio_devices(vms, VIRT_GPIO, sysmem); }
+ if (vms->secure && !vmc->no_secure_gpio) { + create_gpio_devices(vms, VIRT_SECURE_GPIO, secure_sysmem); + } + /* connect powerdown request */ vms->powerdown_notifier.notify = virt_powerdown_req; qemu_register_powerdown_notifier(&vms->powerdown_notifier); @@ -2630,8 +2674,11 @@ DEFINE_VIRT_MACHINE_AS_LATEST(6, 0)
static void virt_machine_5_2_options(MachineClass *mc) { + VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc)); + virt_machine_6_0_options(mc); compat_props_add(mc->compat_props, hw_compat_5_2, hw_compat_5_2_len); + vmc->no_secure_gpio = true; } DEFINE_VIRT_MACHINE(5, 2)
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h index abf54fab49..6f6c85ffcf 100644 --- a/include/hw/arm/virt.h +++ b/include/hw/arm/virt.h @@ -81,6 +81,7 @@ enum { VIRT_GPIO, VIRT_SECURE_UART, VIRT_SECURE_MEM, + VIRT_SECURE_GPIO, VIRT_PCDIMM_ACPI, VIRT_ACPI_GED, VIRT_NVDIMM_ACPI, @@ -127,6 +128,7 @@ struct VirtMachineClass { bool kvm_no_adjvtime; bool no_kvm_steal_time; bool acpi_expose_flash; + bool no_secure_gpio; };
struct VirtMachineState {
On Wed, Jan 20, 2021 at 12:27:48PM +0300, Maxim Uvarov wrote:
Add secure pl061 for reset/power down machine from the secure world (Arm Trusted Firmware). Connect it with gpio-pwr driver.
Signed-off-by: Maxim Uvarov maxim.uvarov@linaro.org
hw/arm/Kconfig | 1 + hw/arm/virt.c | 47 +++++++++++++++++++++++++++++++++++++++++++ include/hw/arm/virt.h | 2 ++ 3 files changed, 50 insertions(+)
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig index 0a242e4c5d..13cc42dcc8 100644 --- a/hw/arm/Kconfig +++ b/hw/arm/Kconfig @@ -17,6 +17,7 @@ config ARM_VIRT select PL011 # UART select PL031 # RTC select PL061 # GPIO
- select GPIO_PWR select PLATFORM_BUS select SMBIOS select VIRTIO_MMIO
diff --git a/hw/arm/virt.c b/hw/arm/virt.c index c427ce5f81..060a5f492e 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -153,6 +153,7 @@ static const MemMapEntry base_memmap[] = { [VIRT_ACPI_GED] = { 0x09080000, ACPI_GED_EVT_SEL_LEN }, [VIRT_NVDIMM_ACPI] = { 0x09090000, NVDIMM_ACPI_IO_LEN}, [VIRT_PVTIME] = { 0x090a0000, 0x00010000 },
- [VIRT_SECURE_GPIO] = { 0x090b0000, 0x00001000 }, [VIRT_MMIO] = { 0x0a000000, 0x00000200 }, /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */ [VIRT_PLATFORM_BUS] = { 0x0c000000, 0x02000000 },
@@ -841,6 +842,43 @@ static void create_gpio_keys(const VirtMachineState *vms, "gpios", phandle, 3, 0); } +#define SECURE_GPIO_POWEROFF 0 +#define SECURE_GPIO_REBOOT 1
+static void create_gpio_pwr(const VirtMachineState *vms,
This function is specific to the secure view. I think it should have "secure" in its name.
DeviceState *pl061_dev,
uint32_t phandle)
+{
- DeviceState *gpio_pwr_dev;
- /* gpio-pwr */
- gpio_pwr_dev = sysbus_create_simple("gpio-pwr", -1, NULL);
Should this device be in secure memory?
- /* connect secure pl061 to gpio-pwr */
- qdev_connect_gpio_out(pl061_dev, SECURE_GPIO_REBOOT,
qdev_get_gpio_in_named(gpio_pwr_dev, "reset", 0));
- qdev_connect_gpio_out(pl061_dev, SECURE_GPIO_POWEROFF,
qdev_get_gpio_in_named(gpio_pwr_dev, "shutdown", 0));
- qemu_fdt_add_subnode(vms->fdt, "/gpio-poweroff");
- qemu_fdt_setprop_string(vms->fdt, "/gpio-poweroff", "compatible",
"gpio-poweroff");
- qemu_fdt_setprop_cells(vms->fdt, "/gpio-poweroff",
"gpios", phandle, SECURE_GPIO_POWEROFF, 0);
- qemu_fdt_setprop_string(vms->fdt, "/gpio-poweroff", "status", "disabled");
- qemu_fdt_setprop_string(vms->fdt, "/gpio-poweroff", "secure-status",
"okay");
- qemu_fdt_add_subnode(vms->fdt, "/gpio-restart");
- qemu_fdt_setprop_string(vms->fdt, "/gpio-restart", "compatible",
"gpio-restart");
- qemu_fdt_setprop_cells(vms->fdt, "/gpio-restart",
"gpios", phandle, SECURE_GPIO_REBOOT, 0);
- qemu_fdt_setprop_string(vms->fdt, "/gpio-restart", "status", "disabled");
- qemu_fdt_setprop_string(vms->fdt, "/gpio-restart", "secure-status",
"okay");
+}
static void create_gpio_devices(const VirtMachineState *vms, int gpio, MemoryRegion *mem) { @@ -883,6 +921,8 @@ static void create_gpio_devices(const VirtMachineState *vms, int gpio, /* Child gpio devices */ if (gpio == VIRT_GPIO) { create_gpio_keys(vms, pl061_dev, phandle);
- } else {
}create_gpio_pwr(vms, pl061_dev, phandle);
} @@ -2015,6 +2055,10 @@ static void machvirt_init(MachineState *machine) create_gpio_devices(vms, VIRT_GPIO, sysmem); }
- if (vms->secure && !vmc->no_secure_gpio) {
create_gpio_devices(vms, VIRT_SECURE_GPIO, secure_sysmem);
- }
/* connect powerdown request */ vms->powerdown_notifier.notify = virt_powerdown_req; qemu_register_powerdown_notifier(&vms->powerdown_notifier);
@@ -2630,8 +2674,11 @@ DEFINE_VIRT_MACHINE_AS_LATEST(6, 0) static void virt_machine_5_2_options(MachineClass *mc) {
- VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc));
- virt_machine_6_0_options(mc); compat_props_add(mc->compat_props, hw_compat_5_2, hw_compat_5_2_len);
- vmc->no_secure_gpio = true;
} DEFINE_VIRT_MACHINE(5, 2) diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h index abf54fab49..6f6c85ffcf 100644 --- a/include/hw/arm/virt.h +++ b/include/hw/arm/virt.h @@ -81,6 +81,7 @@ enum { VIRT_GPIO, VIRT_SECURE_UART, VIRT_SECURE_MEM,
- VIRT_SECURE_GPIO, VIRT_PCDIMM_ACPI, VIRT_ACPI_GED, VIRT_NVDIMM_ACPI,
@@ -127,6 +128,7 @@ struct VirtMachineClass { bool kvm_no_adjvtime; bool no_kvm_steal_time; bool acpi_expose_flash;
- bool no_secure_gpio;
}; struct VirtMachineState { -- 2.17.1
Thanks, drew
On Fri, 22 Jan 2021 at 08:29, Andrew Jones drjones@redhat.com wrote:
On Wed, Jan 20, 2021 at 12:27:48PM +0300, Maxim Uvarov wrote:
Add secure pl061 for reset/power down machine from the secure world (Arm Trusted Firmware). Connect it with gpio-pwr driver.
Signed-off-by: Maxim Uvarov maxim.uvarov@linaro.org
hw/arm/Kconfig | 1 + hw/arm/virt.c | 47 +++++++++++++++++++++++++++++++++++++++++++ include/hw/arm/virt.h | 2 ++ 3 files changed, 50 insertions(+)
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig index 0a242e4c5d..13cc42dcc8 100644 --- a/hw/arm/Kconfig +++ b/hw/arm/Kconfig @@ -17,6 +17,7 @@ config ARM_VIRT select PL011 # UART select PL031 # RTC select PL061 # GPIO
- select GPIO_PWR select PLATFORM_BUS select SMBIOS select VIRTIO_MMIO
diff --git a/hw/arm/virt.c b/hw/arm/virt.c index c427ce5f81..060a5f492e 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -153,6 +153,7 @@ static const MemMapEntry base_memmap[] = { [VIRT_ACPI_GED] = { 0x09080000, ACPI_GED_EVT_SEL_LEN }, [VIRT_NVDIMM_ACPI] = { 0x09090000, NVDIMM_ACPI_IO_LEN}, [VIRT_PVTIME] = { 0x090a0000, 0x00010000 },
- [VIRT_SECURE_GPIO] = { 0x090b0000, 0x00001000 }, [VIRT_MMIO] = { 0x0a000000, 0x00000200 }, /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */ [VIRT_PLATFORM_BUS] = { 0x0c000000, 0x02000000 },
@@ -841,6 +842,43 @@ static void create_gpio_keys(const VirtMachineState *vms, "gpios", phandle, 3, 0); }
+#define SECURE_GPIO_POWEROFF 0 +#define SECURE_GPIO_REBOOT 1
+static void create_gpio_pwr(const VirtMachineState *vms,
This function is specific to the secure view. I think it should have "secure" in its name.
DeviceState *pl061_dev,
uint32_t phandle)
+{
- DeviceState *gpio_pwr_dev;
- /* gpio-pwr */
- gpio_pwr_dev = sysbus_create_simple("gpio-pwr", -1, NULL);
Should this device be in secure memory?
It's not in any memory at all -- -1 as the address argument to sysbus_create_simple() means "no MMIO regions to map". The only way it's connected to the rest of the system is via the secure-only PL061, so the NS world can't get at it.
(sysbus_create_simple("device", -1, NULL) is equivalent to: dev = qdev_new("device"); sysbus_realize_and_unref(SYSBUS_DEVICE(dev), &error_fatal); )
thanks -- PMM
On Fri, Jan 22, 2021 at 10:09:35AM +0000, Peter Maydell wrote:
On Fri, 22 Jan 2021 at 08:29, Andrew Jones drjones@redhat.com wrote:
On Wed, Jan 20, 2021 at 12:27:48PM +0300, Maxim Uvarov wrote:
Add secure pl061 for reset/power down machine from the secure world (Arm Trusted Firmware). Connect it with gpio-pwr driver.
Signed-off-by: Maxim Uvarov maxim.uvarov@linaro.org
hw/arm/Kconfig | 1 + hw/arm/virt.c | 47 +++++++++++++++++++++++++++++++++++++++++++ include/hw/arm/virt.h | 2 ++ 3 files changed, 50 insertions(+)
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig index 0a242e4c5d..13cc42dcc8 100644 --- a/hw/arm/Kconfig +++ b/hw/arm/Kconfig @@ -17,6 +17,7 @@ config ARM_VIRT select PL011 # UART select PL031 # RTC select PL061 # GPIO
- select GPIO_PWR select PLATFORM_BUS select SMBIOS select VIRTIO_MMIO
diff --git a/hw/arm/virt.c b/hw/arm/virt.c index c427ce5f81..060a5f492e 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -153,6 +153,7 @@ static const MemMapEntry base_memmap[] = { [VIRT_ACPI_GED] = { 0x09080000, ACPI_GED_EVT_SEL_LEN }, [VIRT_NVDIMM_ACPI] = { 0x09090000, NVDIMM_ACPI_IO_LEN}, [VIRT_PVTIME] = { 0x090a0000, 0x00010000 },
- [VIRT_SECURE_GPIO] = { 0x090b0000, 0x00001000 }, [VIRT_MMIO] = { 0x0a000000, 0x00000200 }, /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */ [VIRT_PLATFORM_BUS] = { 0x0c000000, 0x02000000 },
@@ -841,6 +842,43 @@ static void create_gpio_keys(const VirtMachineState *vms, "gpios", phandle, 3, 0); }
+#define SECURE_GPIO_POWEROFF 0 +#define SECURE_GPIO_REBOOT 1
+static void create_gpio_pwr(const VirtMachineState *vms,
This function is specific to the secure view. I think it should have "secure" in its name.
DeviceState *pl061_dev,
uint32_t phandle)
+{
- DeviceState *gpio_pwr_dev;
- /* gpio-pwr */
- gpio_pwr_dev = sysbus_create_simple("gpio-pwr", -1, NULL);
Should this device be in secure memory?
It's not in any memory at all -- -1 as the address argument to sysbus_create_simple() means "no MMIO regions to map". The only way it's connected to the rest of the system is via the secure-only PL061, so the NS world can't get at it.
(sysbus_create_simple("device", -1, NULL) is equivalent to: dev = qdev_new("device"); sysbus_realize_and_unref(SYSBUS_DEVICE(dev), &error_fatal); )
Thanks, I should have looked more closely at that.
With the function name change to include "secure".
Reviewed-by: Andrew Jones drjones@redhat.com
On Wed, 20 Jan 2021 at 09:27, Maxim Uvarov maxim.uvarov@linaro.org wrote:
Add secure pl061 for reset/power down machine from the secure world (Arm Trusted Firmware). Connect it with gpio-pwr driver.
Signed-off-by: Maxim Uvarov maxim.uvarov@linaro.org
A nit, which I raise only because you'll need a respin anyway:
- /* connect secure pl061 to gpio-pwr */
- qdev_connect_gpio_out(pl061_dev, SECURE_GPIO_REBOOT,
qdev_get_gpio_in_named(gpio_pwr_dev, "reset", 0));
- qemu_fdt_add_subnode(vms->fdt, "/gpio-restart");
We have three different names for the same thing here: 'reboot', 'reset' and 'restart'. If we name the GPIO line SECURE_GPIO_RESET we can at least get that down to two.
thanks -- PMM
tf-a@lists.trustedfirmware.org