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

[Alexey Kardashevskiy]

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?).

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.

Either way I do not see problems with the patches as they are, they work, I
know how, and I am fine. I just do not like the approach very much (we
could have 2 patches but we have 4 even knowing that the last 2 will never
be used) but my (bad) taste does not matter here. Thanks.


-- 
Alexey