Re: [TF-A] [PATCHv8 3/3] arm-virt: add secure pl061 for reset/power down

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