Re: [Xen-devel] [PATCH V6 03/10] xen/arm: Use the new functions for vCPUID/vaffinity transformation

[Julien Grall <julien.grall@xxxxxxxxxx>]

Hi Ian,

On 08/06/2015 11:05, Ian Campbell wrote:
> On Fri, 2015-06-05 at 19:18 +0100, Julien Grall wrote:
> > On 05/06/2015 16:56, Ian Campbell wrote:
> > > On Mon, 2015-06-01 at 20:56 +0800, Chen Baozi wrote:
> > > > From: Chen Baozi <baozich@xxxxxxxxx>
> > > >
> > > > There are 3 places to change:
> > > >
> > > > * Initialise vMPIDR value in vcpu_initialise()
> > > > * Find the vCPU from vMPIDR affinity information when accessing GICD
> > > >     registers in vGIC
> > > > * Find the vCPU from vMPIDR affinity information when booting with vPSCI
> > > >     in vGIC
> > > >     - Also make the code for PSCI 0.1 use MPIDR-like value as the cpuid.
> > > Does this "- Also ..." not need to be done at the same time as the
> > > change to how we describe things in the FDT? Since that is where the
> > > guest gets the parameter from, isn't it?
> > Well, we only support 8 CPUs.
> I was assuming this would change later in the patch. I don't think PSCI
> 0.1 is limited to gic v2, is it? (If it is then this is worth mentioning
> in the commit message too)
It's not limited to GICv2. But I wasn't able to find the definition of 
CPUID parameter.
> >   So this changes will return the same value
> > as before. It may be worth to mention it.
> >
> > In another side, both PSCI 0.1 and PSCI 0.2 are modified to respect the
> > MPIDR like within this patch. The working in the commit message may be
> > misleading.
> Yes, possibly.
>
> > Somehow the code path slightly differ when the PSCI 0.2 for guest has
> > been added. The spec says (PSCI 0.1 Section 6.3 (ARM DEN 0022A)):
> >
> > "Ideally platform discovery mechanism such as firmware tables would be
> > used by secure firmware to describe the set of valid CPUIDs to the
> > hypervisor or Rich OS, if the former is not present. The hypervisor in
> > turn can create and supply virtual discovery mechanisms to its guests.""
> >
> > I interpreted this as CPUID is equal to the "reg" register in DT (which
> > is an MPIDR-like value).
> >
> > FWIW, this is the interpretation made by Linux too.
> Documentation/devicetree/bindings/arm/cpus.txt requires it to be the
> MPIDR value on armv7 and v8.
Well this document only describes the device tree binding. I don't find 
anything which say that this value should be directly passed to the 
PSCI. Did I miss something?
Not really related to this patch in particular, but the size of "reg" 
may be 2 cells which we currently don't handle.
Regards,

--
Julien Grall

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