On 12/06/2013 01:12 PM, Marc Zyngier wrote:
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.

Nice!


* 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,

Yes, and I want to do away with this if PSCI is defined - as this is not needed. I just asked for keeping the SMP wakeup code as generic as possible and neither tie it to PSCI nor HYP mode, so that either of them can reference that.
But, ...

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).

Yes, there is indeed this problem with the pen. Maybe one can use your upcoming relocation code to move the pen to a more secure place (defined per platform). With "avoid dealing with a pen" you mean to wakeup from the system's pen and immediately jump to the OS init code, without staying in a wfi loop in some special memory?


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.

Indeed. So I was mistaken on this. I thought it worked this way once at least with Highbank/Midway (could have been an internal branch, though).

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.

That one that is mandatory on ARM64? Makes sense, then.

Regards,
Andre.


* 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.


_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to