Re: [Xen-devel] [PATCH v2 0/6]

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2013-12-04 at 13:16 +0100, Andre Przywara wrote:
> On 12/02/2013 03:52 PM, Ian Campbell wrote:
> > On Mon, 2013-12-02 at 12:08 +0100, Andre Przywara wrote:
> >> This is a major rework of last week's ARM PSCI host support. The code
> >> has been moved into a separate file and the code flow has been
> >> changed substantially (see below for more details).
> >>    ---------
> >>
> >> Xen did not make use of the host provided ARM PSCI (Power State
> >> Coordination Interface [1]) functionality so far, but relied on
> >> platform specific SMP bringup functions.
> >> This series adds support for PSCI on the host by reading the required
> >> information from the DTB and invoking the appropriate handler when
> >> bringing up each single CPU.
> >> Since PSCI is defined for both ARM32 and ARM64, I added code for both
> >> architectures, though only ARM32 is tested on Midway and VExpress
> >> (without any PSCI support on the latter, just a regression test).
> >> ARM64 code was compile tested only.
> >>
> >> The ARM32 SMP boot flow is now as following:
> >> The DTB is scanned for a node announcing PSCI support. If that is
> >> available, call the PSCI handler via SMC to bringup the secondary
> >> cores. If no PSCI has been found, call the platform specific cpu_up()
> >> function.
> >
> > Is this the most useful way round? If both PSCI and a hook are present
> > might it be expected that the hook might want to make the choice to
> > either go manual or call back to PSCI?
> >
> > Perhaps to handle some quirk (aka bug) in the platform which needs
> > something else before/after the PSCI stuff.
> 
> I thought about that too and had it actually that way in the first place.
> My reason to change it was: What happens if platforms get PSCI support?
> Thinking about sunxi or Arndale. If the device tree advertises PSCI, 
> then this is supposed to work. If it doesn't work, the DTB shouldn't 
> carry the PSCI node or u-boot & friends have to remove it.
> 
> In general I agree on the "fix" idea, but I'd ask for that any bug in 
> PSCI kicking should be fixed in the PSCI firmware and not in Xen or 
> other kernels. If we are really in need for SMP fixes, we could still 
> put them into smp_init().
> 
> I can easily change it to prefer platform code, of course.

It's ok, I think I'm convinced by your argument.

> >>   It is now the responsibility of those functions to do the
> >> GIC SGI call to kick the secondary CPUs. For that a function is now
> >> provided which does this, so three platforms now reference this
> >> generic function instead of coding up an empty one and relying on the
> >> GIC kick in Xen code later.
> >
> > Note that the existing kick serves two purposes. The first is to wake up
> > the processor from the firmware on platforms which need that. The second
> > is to wake up secondaries from the loop in head.S under:
> >          /* Non-boot CPUs wait here until __cpu_up is ready for them */
> > where they wait for smp_up_cpu to become them.
> >
> > I should go read the patches to see what you've actually done here.
> 
> But currently there is only one kick per CPU, right? I didn't change 
> anything in this respect, just moved the code into a function and called 
> that a bit earlier.

I think we're covering this in the other subthread, so I won't repeat
here.

Ian


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel