| Message ID | 20210112143058.12159-3-maxim.uvarov@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm-virt: add secure pl061 for reset/power down | expand |
On Tue, Jan 12, 2021 at 05:30:58PM +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 | 40 ++++++++++++++++++++++++++++++++++++++++ > include/hw/arm/virt.h | 3 +++ > 3 files changed, 44 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 96985917d3..19605390c2 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -147,6 +147,7 @@ static const MemMapEntry base_memmap[] = { > [VIRT_RTC] = { 0x09010000, 0x00001000 }, > [VIRT_FW_CFG] = { 0x09020000, 0x00000018 }, > [VIRT_GPIO] = { 0x09030000, 0x00001000 }, > + [VIRT_SECURE_GPIO] = { 0x09031000, 0x00001000 }, Does secure world require 4K pages? > [VIRT_SECURE_UART] = { 0x09040000, 0x00001000 }, > [VIRT_SMMU] = { 0x09050000, 0x00020000 }, > [VIRT_PCDIMM_ACPI] = { 0x09070000, MEMORY_HOTPLUG_IO_LEN }, > @@ -864,6 +865,32 @@ static void create_gpio(const VirtMachineState *vms) > g_free(nodename); > } > > +#define ATF_GPIO_POWEROFF 3 > +#define ATF_GPIO_REBOOT 4 > + > +static void create_gpio_secure(const VirtMachineState *vms, MemoryRegion *mem) > +{ > + DeviceState *gpio_pwr_dev; > + SysBusDevice *s; > + hwaddr base = vms->memmap[VIRT_SECURE_GPIO].base; > + DeviceState *pl061_dev; > + > + /* Secure pl061 */ > + 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)); > + > + /* 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)); I don't know anything about secure world, but it seems odd that we don't need to add anything to the DTB. > +} > + > static void create_virtio_devices(const VirtMachineState *vms) > { > int i; > @@ -1993,6 +2020,10 @@ static void machvirt_init(MachineState *machine) > create_gpio(vms); > } > > + if (vms->secure && vms->secure_gpio) { > + create_gpio_secure(vms, secure_sysmem); > + } > + > /* connect powerdown request */ > vms->powerdown_notifier.notify = virt_powerdown_req; > qemu_register_powerdown_notifier(&vms->powerdown_notifier); > @@ -2567,6 +2598,12 @@ static void virt_instance_init(Object *obj) > vms->its = true; > } > > + if (vmc->no_secure_gpio) { > + vms->secure_gpio = false; > + } else { > + vms->secure_gpio = true; > + } nit: vms->secure_gpio = !vmc->no_secure_gpio But do we even need vms->secure_gpio? Why not just do if (vms->secure && !vmc->no_secure_gpio) { create_gpio_secure(vms, secure_sysmem); } in machvirt_init() ? > + > /* Default disallows iommu instantiation */ > vms->iommu = VIRT_IOMMU_NONE; > > @@ -2608,8 +2645,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..a140e75444 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 { > @@ -136,6 +138,7 @@ struct VirtMachineState { > FWCfgState *fw_cfg; > PFlashCFI01 *flash[2]; > bool secure; > + bool secure_gpio; > bool highmem; > bool highmem_ecam; > bool its; > -- > 2.17.1 > > Thanks, drew
On Tue, 12 Jan 2021 at 15:35, Andrew Jones <drjones@redhat.com> wrote: > > On Tue, Jan 12, 2021 at 05:30:58PM +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. > > + /* 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)); > > I don't know anything about secure world, but it seems odd that we don't > need to add anything to the DTB. We should be adding something to the DTB, yes. Look at how create_uart() does this -- you set the 'status' and 'secure-status' properties to indicate that the device is secure-world only. > > + if (vmc->no_secure_gpio) { > > + vms->secure_gpio = false; > > + } else { > > + vms->secure_gpio = true; > > + } > > nit: vms->secure_gpio = !vmc->no_secure_gpio > > But do we even need vms->secure_gpio? Why not just do > > if (vms->secure && !vmc->no_secure_gpio) { > create_gpio_secure(vms, secure_sysmem); > } > > in machvirt_init() ? We're just following the same pattern as vmc->no_its/vms->its, aren't we ? thanks -- PMM
On Tue, Jan 12, 2021 at 04:00:23PM +0000, Peter Maydell wrote: > On Tue, 12 Jan 2021 at 15:35, Andrew Jones <drjones@redhat.com> wrote: > > > > On Tue, Jan 12, 2021 at 05:30:58PM +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. > > > > + /* 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)); > > > > I don't know anything about secure world, but it seems odd that we don't > > need to add anything to the DTB. > > We should be adding something to the DTB, yes. Look at > how create_uart() does this -- you set the 'status' and > 'secure-status' properties to indicate that the device is > secure-world only. > > > > > > + if (vmc->no_secure_gpio) { > > > + vms->secure_gpio = false; > > > + } else { > > > + vms->secure_gpio = true; > > > + } > > > > nit: vms->secure_gpio = !vmc->no_secure_gpio > > > > But do we even need vms->secure_gpio? Why not just do > > > > if (vms->secure && !vmc->no_secure_gpio) { > > create_gpio_secure(vms, secure_sysmem); > > } > > > > in machvirt_init() ? > > We're just following the same pattern as vmc->no_its/vms->its, > aren't we ? > 'its' is a property that can be changed on the command line. Unless we want to be able to manage 'secure-gpio' separately from 'secure', then I think vmc->its plus 'secure' should be sufficient. We don't always need both vmc and vms state, see 'no_ged'. Thanks, drew
On Tue, Jan 12, 2021 at 11:25:30AM -0500, Andrew Jones wrote: > On Tue, Jan 12, 2021 at 04:00:23PM +0000, Peter Maydell wrote: > > On Tue, 12 Jan 2021 at 15:35, Andrew Jones <drjones@redhat.com> wrote: > > > > > > On Tue, Jan 12, 2021 at 05:30:58PM +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. > > > > > > + /* 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)); > > > > > > I don't know anything about secure world, but it seems odd that we don't > > > need to add anything to the DTB. > > > > We should be adding something to the DTB, yes. Look at > > how create_uart() does this -- you set the 'status' and > > 'secure-status' properties to indicate that the device is > > secure-world only. > > > > > > > > > > + if (vmc->no_secure_gpio) { > > > > + vms->secure_gpio = false; > > > > + } else { > > > > + vms->secure_gpio = true; > > > > + } > > > > > > nit: vms->secure_gpio = !vmc->no_secure_gpio > > > > > > But do we even need vms->secure_gpio? Why not just do > > > > > > if (vms->secure && !vmc->no_secure_gpio) { > > > create_gpio_secure(vms, secure_sysmem); > > > } > > > > > > in machvirt_init() ? > > > > We're just following the same pattern as vmc->no_its/vms->its, > > aren't we ? > > > > 'its' is a property that can be changed on the command line. Unless > we want to be able to manage 'secure-gpio' separately from 'secure', > then I think vmc->its plus 'secure' should be sufficient. We don't I meant to write 'vmc->no_secure_gpio and vms->secure' here. Thanks, drew > always need both vmc and vms state, see 'no_ged'. > > Thanks, > drew
- the same size for secure and non secure gpio. Arm doc says that secure memory is also split on 4k pages. So one page here has to be ok. - will add dtb. - I think then less options is better. So I will remove vmc->secure_gpio flag and keep only vmc flag. Regards, Maxim. On Tue, 12 Jan 2021 at 19:28, Andrew Jones <drjones@redhat.com> wrote: > > On Tue, Jan 12, 2021 at 11:25:30AM -0500, Andrew Jones wrote: > > On Tue, Jan 12, 2021 at 04:00:23PM +0000, Peter Maydell wrote: > > > On Tue, 12 Jan 2021 at 15:35, Andrew Jones <drjones@redhat.com> wrote: > > > > > > > > On Tue, Jan 12, 2021 at 05:30:58PM +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. > > > > > > > > + /* 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)); > > > > > > > > I don't know anything about secure world, but it seems odd that we don't > > > > need to add anything to the DTB. > > > > > > We should be adding something to the DTB, yes. Look at > > > how create_uart() does this -- you set the 'status' and > > > 'secure-status' properties to indicate that the device is > > > secure-world only. > > > > > > > > > > > > > > + if (vmc->no_secure_gpio) { > > > > > + vms->secure_gpio = false; > > > > > + } else { > > > > > + vms->secure_gpio = true; > > > > > + } > > > > > > > > nit: vms->secure_gpio = !vmc->no_secure_gpio > > > > > > > > But do we even need vms->secure_gpio? Why not just do > > > > > > > > if (vms->secure && !vmc->no_secure_gpio) { > > > > create_gpio_secure(vms, secure_sysmem); > > > > } > > > > > > > > in machvirt_init() ? > > > > > > We're just following the same pattern as vmc->no_its/vms->its, > > > aren't we ? > > > > > > > 'its' is a property that can be changed on the command line. Unless > > we want to be able to manage 'secure-gpio' separately from 'secure', > > then I think vmc->its plus 'secure' should be sufficient. We don't > > I meant to write 'vmc->no_secure_gpio and vms->secure' here. > > Thanks, > drew > > > always need both vmc and vms state, see 'no_ged'. > > > > Thanks, > > drew >
On Wed, Jan 13, 2021 at 10:30:47AM +0300, Maxim Uvarov wrote: > - the same size for secure and non secure gpio. Arm doc says that > secure memory is also split on 4k pages. So one page here has to be > ok. To be clear, does that means 4k pages must be used? I'm not concerned with the size, but the alignment. If it's possible to use larger page sizes with secure memory, then we need to align to the maximum page size that may be used. Thanks, drew > - will add dtb. > - I think then less options is better. So I will remove > vmc->secure_gpio flag and keep only vmc flag. > > Regards, > Maxim. > > On Tue, 12 Jan 2021 at 19:28, Andrew Jones <drjones@redhat.com> wrote: > > > > On Tue, Jan 12, 2021 at 11:25:30AM -0500, Andrew Jones wrote: > > > On Tue, Jan 12, 2021 at 04:00:23PM +0000, Peter Maydell wrote: > > > > On Tue, 12 Jan 2021 at 15:35, Andrew Jones <drjones@redhat.com> wrote: > > > > > > > > > > On Tue, Jan 12, 2021 at 05:30:58PM +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. > > > > > > > > > > + /* 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)); > > > > > > > > > > I don't know anything about secure world, but it seems odd that we don't > > > > > need to add anything to the DTB. > > > > > > > > We should be adding something to the DTB, yes. Look at > > > > how create_uart() does this -- you set the 'status' and > > > > 'secure-status' properties to indicate that the device is > > > > secure-world only. > > > > > > > > > > > > > > > > > > + if (vmc->no_secure_gpio) { > > > > > > + vms->secure_gpio = false; > > > > > > + } else { > > > > > > + vms->secure_gpio = true; > > > > > > + } > > > > > > > > > > nit: vms->secure_gpio = !vmc->no_secure_gpio > > > > > > > > > > But do we even need vms->secure_gpio? Why not just do > > > > > > > > > > if (vms->secure && !vmc->no_secure_gpio) { > > > > > create_gpio_secure(vms, secure_sysmem); > > > > > } > > > > > > > > > > in machvirt_init() ? > > > > > > > > We're just following the same pattern as vmc->no_its/vms->its, > > > > aren't we ? > > > > > > > > > > 'its' is a property that can be changed on the command line. Unless > > > we want to be able to manage 'secure-gpio' separately from 'secure', > > > then I think vmc->its plus 'secure' should be sufficient. We don't > > > > I meant to write 'vmc->no_secure_gpio and vms->secure' here. > > > > Thanks, > > drew > > > > > always need both vmc and vms state, see 'no_ged'. > > > > > > Thanks, > > > drew > > >
On Thu, 14 Jan 2021 at 00:04, Andrew Jones <drjones@redhat.com> wrote: > > On Wed, Jan 13, 2021 at 10:30:47AM +0300, Maxim Uvarov wrote: > > - the same size for secure and non secure gpio. Arm doc says that > > secure memory is also split on 4k pages. So one page here has to be > > ok. > > To be clear, does that means 4k pages must be used? I'm not concerned > with the size, but the alignment. If it's possible to use larger page > sizes with secure memory, then we need to align to the maximum page > size that may be used. I think we should just align on 64K, to be more future-proof. Even if secure software today uses 4K pages, it doesn't hurt to align the device such that some hypothetical future 64K page using secure software can use it. thanks -- PMM
On Thu, 14 Jan 2021 at 12:50, Peter Maydell <peter.maydell@linaro.org> wrote: > > On Thu, 14 Jan 2021 at 00:04, Andrew Jones <drjones@redhat.com> wrote: > > > > On Wed, Jan 13, 2021 at 10:30:47AM +0300, Maxim Uvarov wrote: > > > - the same size for secure and non secure gpio. Arm doc says that > > > secure memory is also split on 4k pages. So one page here has to be > > > ok. > > > > To be clear, does that means 4k pages must be used? I'm not concerned > > with the size, but the alignment. If it's possible to use larger page > > sizes with secure memory, then we need to align to the maximum page > > size that may be used. > > I think we should just align on 64K, to be more future-proof. > Even if secure software today uses 4K pages, it doesn't hurt > to align the device such that some hypothetical future 64K > page using secure software can use it. > > thanks > -- PMM Does that mean that in that case you need all regions to be 64k aligned? I mean secure and non-secure. Has anybody tested 64k pages under qemu? [VIRT_GIC_V2M] = { 0x08020000, 0x00001000 } [VIRT_UART] = { 0x09000000, 0x00001000 }, [VIRT_RTC] = { 0x09010000, 0x00001000 }, [VIRT_GPIO] = { 0x09030000, 0x00001000 }, [VIRT_SECURE_GPIO] = { 0x09031000, 0x00001000 }, [VIRT_SECURE_UART] = { 0x09040000, 0x00001000 }, [VIRT_MMIO] = { 0x0a000000, 0x00000200 }, Maxim.
On Thu, 14 Jan 2021 at 14:22, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > > On Thu, 14 Jan 2021 at 12:50, Peter Maydell <peter.maydell@linaro.org> wrote: > > > > On Thu, 14 Jan 2021 at 00:04, Andrew Jones <drjones@redhat.com> wrote: > > > > > > On Wed, Jan 13, 2021 at 10:30:47AM +0300, Maxim Uvarov wrote: > > > > - the same size for secure and non secure gpio. Arm doc says that > > > > secure memory is also split on 4k pages. So one page here has to be > > > > ok. > > > > > > To be clear, does that means 4k pages must be used? I'm not concerned > > > with the size, but the alignment. If it's possible to use larger page > > > sizes with secure memory, then we need to align to the maximum page > > > size that may be used. > > > > I think we should just align on 64K, to be more future-proof. > > Even if secure software today uses 4K pages, it doesn't hurt > > to align the device such that some hypothetical future 64K > > page using secure software can use it. > > > > thanks > > -- PMM > > Does that mean that in that case you need all regions to be 64k > aligned? I mean secure and non-secure. > Has anybody tested 64k pages under qemu? > [VIRT_GIC_V2M] = { 0x08020000, 0x00001000 } > [VIRT_UART] = { 0x09000000, 0x00001000 }, > [VIRT_RTC] = { 0x09010000, 0x00001000 }, > [VIRT_GPIO] = { 0x09030000, 0x00001000 }, > [VIRT_SECURE_GPIO] = { 0x09031000, 0x00001000 }, > [VIRT_SECURE_UART] = { 0x09040000, 0x00001000 }, > [VIRT_MMIO] = { 0x0a000000, 0x00000200 }, > > Maxim. I.e. I see comment: * Note that devices should generally be placed at multiples of 0x10000, * to accommodate guests using 64K pages. */ but it's not clear why UART, RTC and GPIO is not aligned to 64k.
On Thu, 14 Jan 2021 at 11:24, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > > On Thu, 14 Jan 2021 at 14:22, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > > Does that mean that in that case you need all regions to be 64k > > aligned? I mean secure and non-secure. > > Has anybody tested 64k pages under qemu? > > [VIRT_GIC_V2M] = { 0x08020000, 0x00001000 } > > [VIRT_UART] = { 0x09000000, 0x00001000 }, > > [VIRT_RTC] = { 0x09010000, 0x00001000 }, > > [VIRT_GPIO] = { 0x09030000, 0x00001000 }, > > [VIRT_SECURE_GPIO] = { 0x09031000, 0x00001000 }, > > [VIRT_SECURE_UART] = { 0x09040000, 0x00001000 }, > > [VIRT_MMIO] = { 0x0a000000, 0x00000200 }, > > > > Maxim. > > I.e. I see comment: > * Note that devices should generally be placed at multiples of 0x10000, > * to accommodate guests using 64K pages. > */ > > but it's not clear why UART, RTC and GPIO is not aligned to 64k. Er, 0x09000000, 0x09010000 and 0x09030000 are all 64K aligned addresses. thanks -- PMM
On Thu, 14 Jan 2021 at 14:48, Peter Maydell <peter.maydell@linaro.org> wrote: > > On Thu, 14 Jan 2021 at 11:24, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > > > > On Thu, 14 Jan 2021 at 14:22, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > > > Does that mean that in that case you need all regions to be 64k > > > aligned? I mean secure and non-secure. > > > Has anybody tested 64k pages under qemu? > > > [VIRT_GIC_V2M] = { 0x08020000, 0x00001000 } > > > [VIRT_UART] = { 0x09000000, 0x00001000 }, > > > [VIRT_RTC] = { 0x09010000, 0x00001000 }, > > > [VIRT_GPIO] = { 0x09030000, 0x00001000 }, > > > [VIRT_SECURE_GPIO] = { 0x09031000, 0x00001000 }, > > > [VIRT_SECURE_UART] = { 0x09040000, 0x00001000 }, > > > [VIRT_MMIO] = { 0x0a000000, 0x00000200 }, > > > > > > Maxim. > > > > I.e. I see comment: > > * Note that devices should generally be placed at multiples of 0x10000, > > * to accommodate guests using 64K pages. > > */ > > > > but it's not clear why UART, RTC and GPIO is not aligned to 64k. > > Er, 0x09000000, 0x09010000 and 0x09030000 are all 64K aligned addresses. > > thanks > -- PMM thanks, will send an updated version.
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 96985917d3..19605390c2 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -147,6 +147,7 @@ static const MemMapEntry base_memmap[] = { [VIRT_RTC] = { 0x09010000, 0x00001000 }, [VIRT_FW_CFG] = { 0x09020000, 0x00000018 }, [VIRT_GPIO] = { 0x09030000, 0x00001000 }, + [VIRT_SECURE_GPIO] = { 0x09031000, 0x00001000 }, [VIRT_SECURE_UART] = { 0x09040000, 0x00001000 }, [VIRT_SMMU] = { 0x09050000, 0x00020000 }, [VIRT_PCDIMM_ACPI] = { 0x09070000, MEMORY_HOTPLUG_IO_LEN }, @@ -864,6 +865,32 @@ static void create_gpio(const VirtMachineState *vms) g_free(nodename); } +#define ATF_GPIO_POWEROFF 3 +#define ATF_GPIO_REBOOT 4 + +static void create_gpio_secure(const VirtMachineState *vms, MemoryRegion *mem) +{ + DeviceState *gpio_pwr_dev; + SysBusDevice *s; + hwaddr base = vms->memmap[VIRT_SECURE_GPIO].base; + DeviceState *pl061_dev; + + /* Secure pl061 */ + 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)); + + /* 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)); +} + static void create_virtio_devices(const VirtMachineState *vms) { int i; @@ -1993,6 +2020,10 @@ static void machvirt_init(MachineState *machine) create_gpio(vms); } + if (vms->secure && vms->secure_gpio) { + create_gpio_secure(vms, secure_sysmem); + } + /* connect powerdown request */ vms->powerdown_notifier.notify = virt_powerdown_req; qemu_register_powerdown_notifier(&vms->powerdown_notifier); @@ -2567,6 +2598,12 @@ static void virt_instance_init(Object *obj) vms->its = true; } + if (vmc->no_secure_gpio) { + vms->secure_gpio = false; + } else { + vms->secure_gpio = true; + } + /* Default disallows iommu instantiation */ vms->iommu = VIRT_IOMMU_NONE; @@ -2608,8 +2645,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..a140e75444 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 { @@ -136,6 +138,7 @@ struct VirtMachineState { FWCfgState *fw_cfg; PFlashCFI01 *flash[2]; bool secure; + bool secure_gpio; bool highmem; bool highmem_ecam; bool its;
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 | 40 ++++++++++++++++++++++++++++++++++++++++ include/hw/arm/virt.h | 3 +++ 3 files changed, 44 insertions(+)