Re: [Qemu-devel] [RFC PATCH v3] powerpc: add PVR mask support

[Andreas Färber]

Am 15.08.2013 13:59, schrieb Alexander Graf:
> 
> On 15.08.2013, at 13:48, Andreas Färber wrote:
> 
>> We do have the following:
>>
>> "object"
>> +- "device"
>>   +- "cpu"
>>      +- "powerpc64-cpu"
>>         +- "POWER7-family-powerpc64-cpu" -> POWERPC_FAMILY()
> 
> Ah, there is the family :).
> 
>>            +- "POWER7_v2.0-powerpc64-cpu" -> POWERPC_DEF_SVR()
>>               +- "host-powerpc64-cpu" (depending on host PVR)
>>
>> That's why I was saying: If we need POWER7+-specific family code, we
>> need to have a POWER7P family and not reuse POWER7 as conveniently done
>> today. All is there to implement properties or whatever at that level.
> 
> Ah, so your point is that POWER7+ is-a POWER7 today? That's most likely 
> wrong, yes. Who pulled in that code?

Today POWER7+_v2.1-powerpc64-cpu is-a POWER7-family-powerpc64-cpu, yes.

http://git.qemu.org/?p=qemu.git;a=commit;h=03a15a5436ed7723f406f15cc3798aa9991e75b5
http://git.qemu.org/?p=qemu.git;a=commit;h=a7d6b9f084765a834110bb9a2a0329b1a96de792

>> And that's also why trying to do the parent tweaking in
>> POWERPC_DEF_FAMILY_MEMBER() is bogus. The existing infrastructure just
>> needs to be used the right way, sigh.
> 
> That's not what this patch is about. It's about making the family classes 
> instantiable. Could we just remove the abstract = true piece from the POWER7 
> class and create that one as our generic POWER7 CPU?

That seems unnecessary to me since the host type is already
instantiatable and we can set the host's PVR there, whether we derive
from an abstract or non-abstract type.

>> And to clean up the aliases business, we should simply move them into
>> the POWER7_v2.0-powerpc64-cpu level class as an array, I think. That
>> would greatly simplify -cpu ?, and the alias-to-type lookup would get
>> faster at the same time since we wouldn't be looking at unavailable
>> models anymore.
> 
> Not sure I follow you on this one :).

You had complained about and uglily bandaided aliases for -cpu ?, some
versions of this patch here touched on aliases and so did the sPAPR
device tree patches I reworked by now to avoid exactly that.

Point is, let's keep separate aspects separate, then we can fix one
problem at a time and gradually move forward rather than constantly
ending up with colliding patches. :)

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg