[U-Boot] ARMv8 spin-table patches

[U-Boot] ARMv8 spin-table patches

[York Sun]

Dear Albert, Wolfgang, Tom,

I have seen some patches for PSCI. We don't have PSCI enabled on Freescale ARMv8
SoCs. Will spin-table patches be acceptable?

York

[U-Boot] ARMv8 spin-table patches

[Tom Rini]

On Fri, Jun 27, 2014 at 09:11:39AM -0700, York Sun wrote:

> Dear Albert, Wolfgang, Tom,
> 
> I have seen some patches for PSCI. We don't have PSCI enabled on
> Freescale ARMv8 SoCs. Will spin-table patches be acceptable?

Baring some technical reasons why no, you can't do that, yes, lets see
the patches :)

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140627/ed5d9bf8/attachment.pgp>

[U-Boot] ARMv8 spin-table patches

[Mark Rutland]

Hi,

Apologies for the late reply.

On Fri, Jun 27, 2014 at 05:44:05PM +0100, Tom Rini wrote:
> On Fri, Jun 27, 2014 at 09:11:39AM -0700, York Sun wrote:
> 
> > Dear Albert, Wolfgang, Tom,
> > 
> > I have seen some patches for PSCI. We don't have PSCI enabled on
> > Freescale ARMv8 SoCs. Will spin-table patches be acceptable?
> 
> Baring some technical reasons why no, you can't do that, yes, lets see
> the patches :)

I'd point out that it's decidedly sub-optimal as spin-table provides no
provision for CPU hotplug (which for Linux will affect kexec and other
features relying on CPU hotplug support).

Additionally, spin-table has the unfortunate property of allowing the
firmware to throw an unbound number of CPUs into the kernel at once
(when they share a cpu-release-addr), where they can spend a lot of time
spinning pointlessly (executing kernel code from memory and possibly
fetching it into I-caches) depending on the number of events a CPU
happens to generate at runtime.

Linux will continue to support spin-table, but it's far preferable to
use PSCI.

Another possibility raised was trying to fix spin-table to provide
provisions for CPU hotplug, but I've not had the time to look into this
in great detail.

A while ago there were some PSCI patches for (32-bit) ARM [1]...

Thanks,
Mark.

[1] https://git.kernel.org/cgit/linux/kernel/git/maz/u-boot.git/log/?h=wip/psci-v4

[U-Boot] ARMv8 spin-table patches

[Scott Wood]

On Fri, 2014-07-04 at 10:29 +0100, Mark Rutland wrote:
> Hi,
> 
> Apologies for the late reply.
> 
> On Fri, Jun 27, 2014 at 05:44:05PM +0100, Tom Rini wrote:
> > On Fri, Jun 27, 2014 at 09:11:39AM -0700, York Sun wrote:
> > 
> > > Dear Albert, Wolfgang, Tom,
> > > 
> > > I have seen some patches for PSCI. We don't have PSCI enabled on
> > > Freescale ARMv8 SoCs. Will spin-table patches be acceptable?
> > 
> > Baring some technical reasons why no, you can't do that, yes, lets see
> > the patches :)
> 
> I'd point out that it's decidedly sub-optimal as spin-table provides no
> provision for CPU hotplug (which for Linux will affect kexec and other
> features relying on CPU hotplug support).

I don't think we're ruling PSCI out entirely, but we don't have it yet
and it's not imminent.  Currently we don't have any firmware that stays
resident after Linux takes over (the tiny spin table code is not
comparable to something that needs to be able to provide runtime
services to a core that has already run OS code).

> Additionally, spin-table has the unfortunate property of allowing the
> firmware to throw an unbound number of CPUs into the kernel at once
> (when they share a cpu-release-addr), where they can spend a lot of time
> spinning pointlessly (executing kernel code from memory and possibly
> fetching it into I-caches) depending on the number of events a CPU
> happens to generate at runtime.

Why do some ARM implementations of the spin table use the same address
for all CPUs?  It looks like ARM's use of the spin table was patterned
after what we do on PPC (Documentation/devicetree/bindings/arm/cpus.txt
even claims to follow ePAPR v1.1), but PPC always had a separate release
address for each CPU, plus the ability to set a register to give the CPU
some context after spinning up.

-Scott

[U-Boot] ARMv8 spin-table patches

[Mark Rutland]

On Tue, Jul 08, 2014 at 04:48:00AM +0100, Scott Wood wrote:
> On Fri, 2014-07-04 at 10:29 +0100, Mark Rutland wrote:
> > Hi,
> > 
> > Apologies for the late reply.
> > 
> > On Fri, Jun 27, 2014 at 05:44:05PM +0100, Tom Rini wrote:
> > > On Fri, Jun 27, 2014 at 09:11:39AM -0700, York Sun wrote:
> > > 
> > > > Dear Albert, Wolfgang, Tom,
> > > > 
> > > > I have seen some patches for PSCI. We don't have PSCI enabled on
> > > > Freescale ARMv8 SoCs. Will spin-table patches be acceptable?
> > > 
> > > Baring some technical reasons why no, you can't do that, yes, lets see
> > > the patches :)
> > 
> > I'd point out that it's decidedly sub-optimal as spin-table provides no
> > provision for CPU hotplug (which for Linux will affect kexec and other
> > features relying on CPU hotplug support).
> 
> I don't think we're ruling PSCI out entirely, but we don't have it yet
> and it's not imminent.  Currently we don't have any firmware that stays
> resident after Linux takes over (the tiny spin table code is not
> comparable to something that needs to be able to provide runtime
> services to a core that has already run OS code).

Sure. I understand that's the case. I just wanted to point out the
limitations of spin-table over alternatives.

I would point out that a trivial PSCI implementation (for version 0.1)
which only provides CPU_ON and CPU_OFF can be relatively small
(especially if CPUs just spin at EL3), and that provides the major
functionality for the use-cases described above.

> > Additionally, spin-table has the unfortunate property of allowing the
> > firmware to throw an unbound number of CPUs into the kernel at once
> > (when they share a cpu-release-addr), where they can spend a lot of time
> > spinning pointlessly (executing kernel code from memory and possibly
> > fetching it into I-caches) depending on the number of events a CPU
> > happens to generate at runtime.
> 
> Why do some ARM implementations of the spin table use the same address
> for all CPUs?  It looks like ARM's use of the spin table was patterned
> after what we do on PPC (Documentation/devicetree/bindings/arm/cpus.txt
> even claims to follow ePAPR v1.1), but PPC always had a separate release
> address for each CPU, plus the ability to set a register to give the CPU
> some context after spinning up.

Unfortunately I don't have an answer to that; the arm64 Linux spin-table
documentation and code were all written before I was involved.

The unfortunate truth is that a lot of the ARM DT and boot unification
work was done somewhat blindly, with many subtleties being lost. Someone
implemented spin-table with a shared address because it happened to be
easier, and then it got copied. Now that people are actively using it
it's not possible to remove it, and it's difficult to dissuade others
from following the crowd.

If U-Boot provides each CPU with its own unique address, then that would
be fantastic, and certainly avoids one nasty edge-case.

Thanks,
Mark.

[U-Boot] ARMv8 spin-table patches

[York Sun]

On 07/14/2014 07:03 AM, Mark Rutland wrote:
> Unfortunately I don't have an answer to that; the arm64 Linux spin-table
> documentation and code were all written before I was involved.
> 
> The unfortunate truth is that a lot of the ARM DT and boot unification
> work was done somewhat blindly, with many subtleties being lost. Someone
> implemented spin-table with a shared address because it happened to be
> easier, and then it got copied. Now that people are actively using it
> it's not possible to remove it, and it's difficult to dissuade others
> from following the crowd.
> 
> If U-Boot provides each CPU with its own unique address, then that would
> be fantastic, and certainly avoids one nasty edge-case.

In the patch set I sent for review, each CPU has its own spin table. It has an
option to use a single release address, or individual release address.

York

[U-Boot] ARMv8 spin-table patches

[Mark Rutland]

On Mon, Jul 14, 2014 at 09:21:26PM +0100, York Sun wrote:
> On 07/14/2014 07:03 AM, Mark Rutland wrote:
> > Unfortunately I don't have an answer to that; the arm64 Linux spin-table
> > documentation and code were all written before I was involved.
> > 
> > The unfortunate truth is that a lot of the ARM DT and boot unification
> > work was done somewhat blindly, with many subtleties being lost. Someone
> > implemented spin-table with a shared address because it happened to be
> > easier, and then it got copied. Now that people are actively using it
> > it's not possible to remove it, and it's difficult to dissuade others
> > from following the crowd.
> > 
> > If U-Boot provides each CPU with its own unique address, then that would
> > be fantastic, and certainly avoids one nasty edge-case.
> 
> In the patch set I sent for review, each CPU has its own spin table. It has an
> option to use a single release address, or individual release address.

Ok, each having their own table is good.

I would strongly recommend against sharing the release address for the
reasons I described in my earlier email.

Cheers,
Mark.