On Fri, Jan 23, 2015 at 12:37:20PM +0000, Mark Rutland wrote: > On Fri, Jan 23, 2015 at 10:10:45AM +0000, Thierry Reding wrote: > > On Thu, Jan 22, 2015 at 07:20:15PM +0000, Mark Rutland wrote: > > > On Fri, Jan 16, 2015 at 09:12:59AM +0000, Thierry Reding wrote: > > > > On Thu, Jan 15, 2015 at 07:19:37PM +0000, Mark Rutland wrote: > > > > > On Wed, Jan 14, 2015 at 07:57:25AM +0000, Thierry Reding wrote: > > > > > > On Tue, Jan 13, 2015 at 07:44:50PM +0000, Ian Campbell wrote: > > > > > > > Hi Thierry, > > > > > > > > > > > > > > I needed to boot my Jetson in NS mode (in order to boot Xen) and > > > > > > > was > > > > > > > investigating the possibility of PSCI support when I discovered > > > > > > > that you > > > > > > > had already started on it[0]. Hurrah! > > > > > > > > > > > > > > I cherry-picked the relevant commit onto u-boot-tegra#master and > > > > > > > added a > > > > > > > few more patches and now it boots correctly for me, both running > > > > > > > Xen > > > > > > > (some Xen side patches are needed too) and native Linux. > > > > > > > > > > > > > > The main things which was needed was to rebase for some recent > > > > > > > Kconfig > > > > > > > changes relating to virt and nonsec mode and to arrange for the > > > > > > > RAM used > > > > > > > by the secure code to be reserved in the FDT. I also reserved the > > > > > > > RAM > > > > > > > using the hardware MC_SECURITY_CFG registers for good measure. > > > > > > > > > > > > Great, those were all things that I had wanted to do but never got > > > > > > around to. > > > > > > > > > > > > > I also pushed my tree to gitorious: > > > > > > > https://gitorious.org/ijc/u-boot jetson-psci-v1 > > > > > > > > > > > > > > I would Ack your patch, but I don't think you've posted it and it > > > > > > > has no > > > > > > > S-o-b so that would seem a bit premature/rude of me. For the same > > > > > > > reason > > > > > > > I've not actually included it in the series posted (but it is in > > > > > > > the > > > > > > > gitorious branch). > > > > > > > > > > > > Feel free to take ownership of that patch. I currently don't have > > > > > > the > > > > > > time to work on this and it seems you've made good progress on it. > > > > > > > > > > > > It could probably use some cleanup because there's a bit of debug > > > > > > output > > > > > > still in there. Also... > > > > > > > > > > > > > FWIW I think you could drop your stub versions of psci_cpu_off and > > > > > > > psci_cpu_suspend (assuming you don't want to implement them) > > > > > > > since the > > > > > > > common code has stubs. > > > > > > > > > > > > ... I'd think you'd need to implement these so that you can get > > > > > > proper > > > > > > suspend/resume support in the kernel. I've had to disable cpuidle > > > > > > (via > > > > > > #undef CONFIG_PM_SLEEP in arch/arm/mach-tegra/cpuidle-tegra114.c) > > > > > > in the > > > > > > kernel to make that code not powergate CPUs. Ideally I think the > > > > > > kernel > > > > > > would check that it's running with PSCI support and disable the > > > > > > cpuidle > > > > > > driver. Maybe that could be done by introducing a new cpuidle driver > > > > > > that checks for PSCI availability and uses it when present. > > > > > > > > > > We have a generic CPUidle driver on arm64 which can use PSCI as a > > > > > backend; we should try to reuse that. The binding should certainly be > > > > > identical. > > > > > > > > Is there any reason that driver needs to be ARM64-specific? I would've > > > > thought that there could be a generic PSCI driver that works anywhere. > > > > > > Currently the arm and arm64 arch interfaces are a little different, but > > > with some work the bulk of the code could certainly be made common > > > (in drivers/firmware, perhaps). > > > > > > > > It looks like the tegra124 dts in mainline doesn't use enable-method > > > > > in > > > > > the DT, so a better option might be to fail early in > > > > > cpuidle-tegra114.c > > > > > if _any_ enable-method is present. > > > > > > > > Yes, that sounds like a good plan. The absence of an enable-method would > > > > signal that a kernel-native method (if any) should be used. > > > > > > > > And this reminds me that we still need to find a way to synchronize > > > > accesses to the powergate registers between secure firmware and the > > > > kernel. Tegra has a set of hardware semaphores, but it seems like those > > > > can only be used to synchronize between AVP and CPU, whereas for PSCI > > > > we'd need something to synchronize between two CPUs. Do you know of any > > > > existing mechanism to perform that type of synchronization? > > > > > > > > Perhaps an option would be to add some sort of global lock in the kernel > > > > which the cpuidle driver can grab before issuing the SMC instruction. > > > > > > PSCI assumes that the FW is in full control of the registers it's > > > poking. While a lock isn't necessarily bad, I suspect it's going to be > > > very difficult to have that common across all users without the code > > > becoming unmaintainable fast. I'd also hope that for arm64 we wouldn't > > > need it. > > > > > > When/how/why does the kernel to poke these registers? > > > > The PMC is what controls power partitions. Some of these partitions are > > assigned to CPUs, others are assigned to things like SATA, PCIe, display > > and so on. The problem is that if we manage the CPU power partitions via > > the firmware, then they will conflict with calls that we need to make > > from other drivers that need to gate or ungate the partitions for their > > hardware. As I understand it there's no provision in PSCI to manage non- > > CPU devices, so this is a problem. > > Ok. > > > So I think either firmware needs to control everything, in which case we > > are going to need a new interface (or extend PSCI) or it mustn't control > > anything, in which case we need custom code in the kernel for SMP. Well, > > the other alternative would be the lock that we can grab in the > > powergate API and the PSCI calls. > > One reason I'm not so keen on a lock is I could imagine you'd need to > grab this for CPU_SUSPEND calls (i.e. cpuidle), at which point all CPUs > are going to contend for the lock all the time. > > One thing to bear in mind is that PSCI is only one user of the SMC > space. Per SMC calling convention, portions of the SMC ID space are > there to be used for other (vendor-specific) purposes. > > So rather than extending PSCI, a parallel API could be implemented for > power control of other devices, and the backend could arbitrate the two > without the non-secure OS requiring implementation-specific mutual > exclusion. > > I think this has been brought up internally previously; I'll go and poke > around in the area to see if we managed to figure out anything useful.
Okay, I think we'll need to coordinate to provide a common interface for the kernel to talk to firmware. > > Unfortunately this doesn't change on 64-bit Tegra at all. > > I suspected as much. :/ > > How does this bode for the tegra132 dts [1] on LAKML at the moment? Is > it just the "nvidia,tegra132-pmc" device that needs to be poked by both > FW and kernel, or are other devices involved? > > Thanks, > Mark. > > [1] > http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/317013.html Cc'ing Peter and Paul who might be more familiar with SMP. Thierry
pgpNeW7diA8pU.pgp
Description: PGP signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
