Re: [Qemu-devel] [PATCH 3/3] ppc/pnv: add a PowerNVCPUCore object

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2315 bytes --]

On Tue, Aug 30, 2016 at 09:23:40AM +0200, Cédric Le Goater wrote:
> On 08/30/2016 08:15 AM, Benjamin Herrenschmidt wrote:
> > On Mon, 2016-08-29 at 10:30 -0400, David Gibson wrote:
> >>
> >> Possibly.  In fact, I'm planning to eliminate cpu->cpu_dt_id at some
> >> point, in favour of having the machine type construct the id when it
> >> actually builds the dt.  It's not really a cpu level construct.
> 
> >From my understanding, cs->cpu_index is becoming the main CPU identifier.
> sPAPRCPUCore assigns it :
> 
> 	cs->cpu_index = cc->core_id + i

Uh.. yes and no.  It's the main internal identifier, and it's become
stable at least on platforms which support cpu hotplug.  This means
that it should be possible to derive any other platform specific
identifiers from cpu_index in a consistent way.

However, cpu_index values must still lie in the range 0..max_cpus-1,
which means it's not suitable for directly representing non-contiguous
platform identifiers.

> which I reused in PnvCPUCore to hold the real HW identifiers. 
> ppc_get_vcpu_by_dt_id() can also safely use cs->cpu_index I think. 

Well, there's a different function for getting a cpu by cpu_index.

> So pnv mostly work without ->cpu_dt_id but there is :
> 
> > On PowerNV it is as it's equal to the PIR, the HW interrupt server,
> > etc...
> 
> xics in the way ... Now that pnv uses real hw core ids, it is 
> interesting to see how lost it gets without a cpu with index=0 ...
> 
> The most obvious issue is the way we look for the ICPState of a cpu :
> 
>     ICPState *ss = &xics->ss[cs->cpu_index];
> 
> how about introducing a helper like (this one I hacked) :
> 
> +ICPState *xics_find_icp(XICSState *xics, int cpu_index)
> +{
> +    int i;
> +
> +    for (i = 0 ; i < xics->nr_servers; i++) {
> +        ICPState *ss = &xics->ss[i];
> +        if (ss->cs && ss->cs->cpu_index == cpu_index)
> +            return ss;
> +    }
> +
> +    return NULL;
> +}

That's probably reasonable.

> +
> 
> That might have been already discussed on the mailing list ? 
> 
> Thanks,
> 
> C.
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]