Re: [Qemu-ppc] [RFC PATCH] ppc/xics: introduce helpers to find an ICP from some (CPU) index

[Cédric Le Goater]

On 09/20/2016 03:25 PM, David Gibson wrote:
> On Mon, Sep 19, 2016 at 10:54:10AM +0200, Cédric Le Goater wrote:
>> Today, the CPU index is used to index the icp array under xics. This
>> works correctly when the indexes are sync but that is an assumption
>> that could break future implementations. For instance, the PowerNV
>> platform CPUs use real HW ids and they are not contiguous.
>>
>> Let's introduce some helpers to hide the underlying structure of the
>> ICP array. The criteria to look for an ICPstate is still the CPU index
>> but it is decorrelated from the array index.
>>
>> This is an RFC to see what people think. It is used on the powernv
>> branch but it won't apply as it is on upstream. It should certainly be
>> optimised when a large number of CPUs are configured.
>>
>> Signed-off-by: Cédric Le Goater <address@hidden>
> 
> Hm, so.
> 
> First, I think the helpers to get the right icp state are a good idea,
> regardless of anything else.  I'm happy to go ahead with a patch which
> introduces just that, because it will make future revisions easier.

yes. that would be a first step, to hide the underlying implementation. 

> But, the rest of the changes seem to be predicated on allowing
> non-contiguous cpu_index values.  Maybe we want to do that eventually,
> but we're a pretty long way off at present, doing so involves work in
> libvirt as well as qemu itself, and it's not clear we actually want
> it.

OK. The pir being used in most places in the code, this is not a big
problem, and even this part in the core object :

+    env->spr[SPR_PIR] = cs->cpu_index;

could be easily worked out. I was just lazy. 

So the only area where we are using cpu_index is in xics for the 
ICPState indexing.

> I think for the time being you'd be better off keeping the simple
> array of icp states indexed by contiguous (barring cpu hotplug)
> cpu_index values, and dissociating the physical IDs (PIR == interrupt
> server number == DT id, IIUC) from the cpu_index.  Obviously you'll
> need helpers to convert between cpu_index and physical ID at the
> machine level.

If we keep the helper idea but transform them into XICSStateClass 
methods, we can define customs ops in xics_native and handle the 
conversion real-id <-> icp id there. I think a simple array under 
xics_native would do.

I will try that in v4.

Thanks,

C.