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