- 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 <drjo...@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 <drjo...@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 >