Re: [Qemu-devel] [PATCH v2 2/4] spapr: Use DeviceClass::fw_name for device tree CPU node

[Alexander Graf]

On 30.08.2013, at 16:00, Andreas Färber wrote:

> Am 30.08.2013 15:54, schrieb Alexander Graf:
>> 
>> On 30.08.2013, at 08:05, Alexey Kardashevskiy wrote:
>> 
>>> On 08/29/2013 03:30 PM, Andreas Färber wrote:
>>>> Am 29.08.2013 06:29, schrieb Alexey Kardashevskiy:
>>>>> On 08/16/2013 08:35 AM, Andreas Färber wrote:
>>>>>> Instead of relying on cpu_model, obtain the device tree node label
>>>>>> per CPU. Use DeviceClass::fw_name when available. This implicitly
>>>>>> resolves address@hidden node labels for those CPUs through inheritance.
>>>>>> 
>>>>>> Whenever DeviceClass::fw_name is not available, derive it from the CPU's
>>>>>> type name and fill it in for that class with a "PowerPC," prefix for
>>>>>> PAPR compliance.
>>>>> 
>>>>> 
>>>>> I'd rather use the family's @desc instead of CPU class name, would be
>>>>> simpler and we would not have nodes like "PowerPC,address@hidden" (this is
>>>>> what I get when comment out dc->fw_name for power7 with my PVR patch, just
>>>>> to test).
>>>> 
>>>> Negative, desc is a free-text field and may contain spaces, parenthesis,
>>>> etc. Each model may set desc differently btw, so given my change request
>>>> for the comparison, we might end up with "POWER7 v2.1" on that
>>>> particular PVR.
>>> 
>>> These patches are for spapr and spapr-supported CPUs have short nice names
>>> in @desc. But ok, may be that's a wrong idea.
>>> 
>>> 
>>>>> Either way, in what case do you expect that code to work at all? power7,
>>>>> 7+, 8 have fw_name field initialized, what else is really supported for
>>>>> spapr and requires this workaround?
>>>> 
>>>> 970 comes to mind? Anyway, this was just a more direct way to address
>>>> the issues raised by Prerna. If you guys don't see the need to enforce
>>>> these naming rules beyond a supported list of POWER CPUs then we can
>>>> strip it down further, possibly falling back to a fixed
>>>> "PowerPC,UNKNOWN" rather than trying to construct a name.
>>> 
>>> 
>>> The direct way would be to finish what the series started and assign
>>> reasonable fw_name values to every existing family class (970, cell,
>>> power6, rs64?).
>> 
>> I agree :). Then there's no need for magic fw_name generation anymore and we 
>> have a very obvious code path, making the code simple.
>> 
>>> There is very limited set of spapr CPU families, they are all there (except
>>> power7+ but I'll have a patch for that too) and new CPU family will require
>>> a new class anyway (so we will put fw_name there when we'll be adding the
>>> class) OR we implement "compatibility mode" which will use one of existing
>>> classes so we get a correct fw_name either way.
>> 
>> Well, I think it makes sense to always provide a fw_name field, regardless 
>> of whether that particular CPU works with sPAPR or not. We could use the 
>> same field for ePAPR too for example.
> 
> So does ePAPR have the same or similar naming rules? Could you supply us
> with verified e500 etc. names so that it's not pure speculation on my part?

I don't see any rules on the cpu node naming in ePAPR, but if I look at the 
variety of namings in Linux's dts's I don't think we have to worry overly much, 
as long as we keep the names simple. I think in almost all cases 
sub-powerpc-class_name == fw_name is a pretty reasonable assumption.


Alex

kvm $ grep -R PowerPC arch/powerpc/boot/dts | sed -n 's/.*PowerPC,\(.*\) 
{/\1/p' | sort | address@hidden
address@hidden
address@hidden
603e  /* Really 8241 */
7447
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden
address@hidden