Hi Andre,
On 06/12/13 11:43, Andre Przywara wrote:
On 11/21/2013 09:59 AM, Marc Zyngier wrote:
PSCI is an ARM standard that provides a generic interface that ...
Thanks again for posting this, I like the idea of adding PSCI handlers to u-boot for several platforms very much.
This patch series contains 3 parts:
- the first four patches are just bug fixes
Those are fine, I already acked those patches.
- the next three contain the generic PSCI code and build infrastructure
As I heard you will rework these anyway, I will refrain from commenting in detail, just some generic comments on the approach:
- Is the creation of a top-level psci directory for holding the PSCI
binaries really necessary? Should this mimic the spl approach? Can you consider to move this at least into the arch/arm directory, as PSCI is ARM specific? Or add it to the SPL directory, as this serves a similar purpose? But maybe your new approach renders this all moot.
The code base I have at the moment completely gets rid of the psci directory, and make that code a separate section that can be relocated to secure memory or simply marked as reserved.
- Can you keep the SMP bringup code in place and re-use it from the PSCI
handlers instead of "#ifndef PSCI"ing it? So maybe rename arch/arm/cpu/armv7/sunxi/psci.S to .../sunxi/smp.S? My idea here is to make PSCI an option in addition to the current SMP HYP mode code. So that for instance on VExpress (or better: Arndale) you could either use the existing code using the kernel's platform SMP code or enable PSCI in u-boot and let the kernel use that, too.
The main problem is the SMP wake-up code in u-boot. With PSCI, you really don't want to wake up the secondaries at all, and you wait until the kernel does an SMC. The current code wakes up the CPUs unconditionnally, and put them in a separate pen, and I'd really like to avoid dealing with a pen in the PSCI case (stupid platforms like VExpress might still require it, but that's an orthogonal discussion).
I maybe ask too much for the first incarnation of the code, but that is how I would like to eventually see it. AFAIK Linux prefers PSCI over platform-defined SMP code, so this should work out of the box.
Not really. So far, it will use the the platform SMP code if it is defined, and fallback to PSCI otherwise. There were patches from Stephen Boyd to use the "enable-method" property in the cpu node to select PSCI though. I need to ping him on that.
- Is the use of TPIDRPRW & Co. really safe? It looks like as we seem to
be the only secure user (and they are banked), but I am just curious whether there is any "prior art" in using those registers temporarily.
As you said, we own secure mode entirely. So they really are scratch registers, free to be used.
... The kernel now boots in HYP mode, finds its secondary CPU without any additional SMP code, and runs KVM out of the box. Hopefully, the Xen/ARM guys can do the same fairly easily.
BTW: Yesterday my PSCI host patches for Xen have been committed, so Xen should be able to use that feature just like the kernel does.
Excellent! I really need to sort these patches out and repost the whole series...
Thanks,
M.