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 | 50 +++++++++++++++++++++++++++++++++++++++++++
include/hw/arm/virt.h | 2 ++
3 files changed, 53 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 26bb66e8e1..436ae894c9 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,46 @@ static void create_gpio_keys(const VirtMachineState *vms,
"gpios", phandle, 3, 0);
}
+#define ATF_GPIO_POWEROFF 3
+#define ATF_GPIO_REBOOT 4
+
+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, ATF_GPIO_POWEROFF,
+ qdev_get_gpio_in_named(gpio_pwr_dev, "reset", 0));
+ qdev_connect_gpio_out(pl061_dev, ATF_GPIO_REBOOT,
+ qdev_get_gpio_in_named(gpio_pwr_dev, "shutdown", 0));
+
+ qemu_fdt_add_subnode(vms->fdt, "/gpio-pwr");
+ qemu_fdt_setprop_string(vms->fdt, "/gpio-pwr", "compatible", "gpio-pwr");
+ qemu_fdt_setprop_cell(vms->fdt, "/gpio-pwr", "#size-cells", 0);
+ qemu_fdt_setprop_cell(vms->fdt, "/gpio-pwr", "#address-cells", 1);
+
+ qemu_fdt_add_subnode(vms->fdt, "/gpio-pwr/poweroff");
+ qemu_fdt_setprop_string(vms->fdt, "/gpio-pwr/poweroff",
+ "label", "GPIO PWR Poweroff");
+ qemu_fdt_setprop_cell(vms->fdt, "/gpio-pwr/poweroff", "code",
+ ATF_GPIO_POWEROFF);
+ qemu_fdt_setprop_cells(vms->fdt, "/gpio-pwr/poweroff",
+ "gpios", phandle, 3, 0);
+
+ qemu_fdt_add_subnode(vms->fdt, "/gpio-pwr/reboot");
+ qemu_fdt_setprop_string(vms->fdt, "/gpio-pwr/reboot",
+ "label", "GPIO PWR Reboot");
+ qemu_fdt_setprop_cell(vms->fdt, "/gpio-pwr/reboot", "code",
+ ATF_GPIO_REBOOT);
+ qemu_fdt_setprop_cells(vms->fdt, "/gpio-pwr/reboot",
+ "gpios", phandle, 3, 0);
+}
+
static void create_gpio_devices(const VirtMachineState *vms, int gpio,
MemoryRegion *mem)
{
@@ -888,6 +929,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);
}
}
@@ -2020,6 +2063,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);
@@ -2635,8 +2682,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
On Fri, 15 Jan 2021 at 10:11, 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>
> ---
> hw/arm/Kconfig | 1 +
> hw/arm/virt.c | 50 +++++++++++++++++++++++++++++++++++++++++++
> include/hw/arm/virt.h | 2 ++
> 3 files changed, 53 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 26bb66e8e1..436ae894c9 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,46 @@ static void create_gpio_keys(const VirtMachineState *vms,
> "gpios", phandle, 3, 0);
> }
>
> +#define ATF_GPIO_POWEROFF 3
> +#define ATF_GPIO_REBOOT 4
These aren't ATF specific, so you could name them SECURE_GPIO_POWEROFF
and SECURE_GPIO_REBOOT.
Remind me why we start with GPIO line number 3 and not 0 ?
> +
> +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, ATF_GPIO_POWEROFF,
> + qdev_get_gpio_in_named(gpio_pwr_dev, "reset", 0));
> + qdev_connect_gpio_out(pl061_dev, ATF_GPIO_REBOOT,
> + qdev_get_gpio_in_named(gpio_pwr_dev, "shutdown", 0));
You've connected the POWEROFF gpio line to 'reset' and the
REBOOT line to 'shutdown'. This looks like it's backwards.
> + qemu_fdt_add_subnode(vms->fdt, "/gpio-pwr");
> + qemu_fdt_setprop_string(vms->fdt, "/gpio-pwr", "compatible", "gpio-pwr");
> + qemu_fdt_setprop_cell(vms->fdt, "/gpio-pwr", "#size-cells", 0);
> + qemu_fdt_setprop_cell(vms->fdt, "/gpio-pwr", "#address-cells", 1);
> +
> + qemu_fdt_add_subnode(vms->fdt, "/gpio-pwr/poweroff");
> + qemu_fdt_setprop_string(vms->fdt, "/gpio-pwr/poweroff",
> + "label", "GPIO PWR Poweroff");
> + qemu_fdt_setprop_cell(vms->fdt, "/gpio-pwr/poweroff", "code",
> + ATF_GPIO_POWEROFF);
> + qemu_fdt_setprop_cells(vms->fdt, "/gpio-pwr/poweroff",
> + "gpios", phandle, 3, 0);
> +
> + qemu_fdt_add_subnode(vms->fdt, "/gpio-pwr/reboot");
> + qemu_fdt_setprop_string(vms->fdt, "/gpio-pwr/reboot",
> + "label", "GPIO PWR Reboot");
> + qemu_fdt_setprop_cell(vms->fdt, "/gpio-pwr/reboot", "code",
> + ATF_GPIO_REBOOT);
> + qemu_fdt_setprop_cells(vms->fdt, "/gpio-pwr/reboot",
> + "gpios", phandle, 3, 0);
There doesn't seem to be any documented 'gpio-pwr' devicetree
binding. Where does this come from ?
I think the bindings you want to be using are
https://www.kernel.org/doc/Documentation/devicetree/bindings/power/reset/gpio-restart.txt
https://www.kernel.org/doc/Documentation/devicetree/bindings/power/reset/gpio-poweroff.txt
thanks
-- PMM
On Tue, 19 Jan 2021 at 16:07, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Fri, 15 Jan 2021 at 10:11, 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>
> > ---
> > hw/arm/Kconfig | 1 +
> > hw/arm/virt.c | 50 +++++++++++++++++++++++++++++++++++++++++++
> > include/hw/arm/virt.h | 2 ++
> > 3 files changed, 53 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 26bb66e8e1..436ae894c9 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,46 @@ static void create_gpio_keys(const VirtMachineState *vms,
> > "gpios", phandle, 3, 0);
> > }
> >
> > +#define ATF_GPIO_POWEROFF 3
> > +#define ATF_GPIO_REBOOT 4
>
> These aren't ATF specific, so you could name them SECURE_GPIO_POWEROFF
> and SECURE_GPIO_REBOOT.
>
OK.
> Remind me why we start with GPIO line number 3 and not 0 ?
>
Original gpio power key use 3 and 4 (non-secure). I just selected the
same to be consistent.
> > +
> > +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, ATF_GPIO_POWEROFF,
> > + qdev_get_gpio_in_named(gpio_pwr_dev, "reset", 0));
> > + qdev_connect_gpio_out(pl061_dev, ATF_GPIO_REBOOT,
> > + qdev_get_gpio_in_named(gpio_pwr_dev, "shutdown", 0));
>
> You've connected the POWEROFF gpio line to 'reset' and the
> REBOOT line to 'shutdown'. This looks like it's backwards.
>
Oh, yes. Thanks for finding that.
> > + qemu_fdt_add_subnode(vms->fdt, "/gpio-pwr");
> > + qemu_fdt_setprop_string(vms->fdt, "/gpio-pwr", "compatible", "gpio-pwr");
> > + qemu_fdt_setprop_cell(vms->fdt, "/gpio-pwr", "#size-cells", 0);
> > + qemu_fdt_setprop_cell(vms->fdt, "/gpio-pwr", "#address-cells", 1);
> > +
> > + qemu_fdt_add_subnode(vms->fdt, "/gpio-pwr/poweroff");
> > + qemu_fdt_setprop_string(vms->fdt, "/gpio-pwr/poweroff",
> > + "label", "GPIO PWR Poweroff");
> > + qemu_fdt_setprop_cell(vms->fdt, "/gpio-pwr/poweroff", "code",
> > + ATF_GPIO_POWEROFF);
> > + qemu_fdt_setprop_cells(vms->fdt, "/gpio-pwr/poweroff",
> > + "gpios", phandle, 3, 0);
> > +
> > + qemu_fdt_add_subnode(vms->fdt, "/gpio-pwr/reboot");
> > + qemu_fdt_setprop_string(vms->fdt, "/gpio-pwr/reboot",
> > + "label", "GPIO PWR Reboot");
> > + qemu_fdt_setprop_cell(vms->fdt, "/gpio-pwr/reboot", "code",
> > + ATF_GPIO_REBOOT);
> > + qemu_fdt_setprop_cells(vms->fdt, "/gpio-pwr/reboot",
> > + "gpios", phandle, 3, 0);
>
> There doesn't seem to be any documented 'gpio-pwr' devicetree
> binding. Where does this come from ?
>
gpio-pwr created from the first patch. There are no bindings yet.
> I think the bindings you want to be using are
> https://www.kernel.org/doc/Documentation/devicetree/bindings/power/reset/gpio-restart.txt
> https://www.kernel.org/doc/Documentation/devicetree/bindings/power/reset/gpio-poweroff.txt
>
These handles are from 'secure memory' where linux does not have
access. But I think we can use that
binding with other compatible. Like compatible = "gpio-poweroff,secure".
Maxim.
> thanks
> -- PMM
On Tue, 19 Jan 2021 at 13:47, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > > On Tue, 19 Jan 2021 at 16:07, Peter Maydell <peter.maydell@linaro.org> wrote: > > > > On Fri, 15 Jan 2021 at 10:11, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > > Remind me why we start with GPIO line number 3 and not 0 ? > > > > Original gpio power key use 3 and 4 (non-secure). I just selected the > same to be consistent. Those are different GPIO lines on a different PL061 doing a different job. I don't think they need to be the same number. The power keys are on 3 and 4 because pins 0, 1 and 2 were reserved for PCI hotplug, CPU hotplug and memory hotplug. Unless you have some similar reason why you need to reserve pins on the secure PL061, I would just start from 0. > > > + qemu_fdt_add_subnode(vms->fdt, "/gpio-pwr"); > > > + qemu_fdt_setprop_string(vms->fdt, "/gpio-pwr", "compatible", "gpio-pwr"); > > > + qemu_fdt_setprop_cell(vms->fdt, "/gpio-pwr", "#size-cells", 0); > > > + qemu_fdt_setprop_cell(vms->fdt, "/gpio-pwr", "#address-cells", 1); > > > + > > > + qemu_fdt_add_subnode(vms->fdt, "/gpio-pwr/poweroff"); > > > + qemu_fdt_setprop_string(vms->fdt, "/gpio-pwr/poweroff", > > > + "label", "GPIO PWR Poweroff"); > > > + qemu_fdt_setprop_cell(vms->fdt, "/gpio-pwr/poweroff", "code", > > > + ATF_GPIO_POWEROFF); > > > + qemu_fdt_setprop_cells(vms->fdt, "/gpio-pwr/poweroff", > > > + "gpios", phandle, 3, 0); > > > + > > > + qemu_fdt_add_subnode(vms->fdt, "/gpio-pwr/reboot"); > > > + qemu_fdt_setprop_string(vms->fdt, "/gpio-pwr/reboot", > > > + "label", "GPIO PWR Reboot"); > > > + qemu_fdt_setprop_cell(vms->fdt, "/gpio-pwr/reboot", "code", > > > + ATF_GPIO_REBOOT); > > > + qemu_fdt_setprop_cells(vms->fdt, "/gpio-pwr/reboot", > > > + "gpios", phandle, 3, 0); > > > > There doesn't seem to be any documented 'gpio-pwr' devicetree > > binding. Where does this come from ? > > > gpio-pwr created from the first patch. There are no bindings yet. You can't use bindings you've just made up -- you have to get them accepted into the kernel's official devicetree documentation if the ones already there aren't sufficient, before you can add code to QEMU that generates them. > > I think the bindings you want to be using are > > https://www.kernel.org/doc/Documentation/devicetree/bindings/power/reset/gpio-restart.txt > > https://www.kernel.org/doc/Documentation/devicetree/bindings/power/reset/gpio-poweroff.txt > > > These handles are from 'secure memory' where linux does not have > access. But I think we can use that > binding with other compatible. Like compatible = "gpio-poweroff,secure". That's not how you specify that a node is only relevant to the secure world: you set the 'status' property to 'disabled' and the 'secure-status' property to 'okay': https://www.kernel.org/doc/Documentation/devicetree/bindings/arm/secure.txt thanks -- PMM
© 2016 - 2026 Red Hat, Inc.