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

[Alexander Graf]

On 15.08.2013, at 17:43, Andreas Färber wrote:

> Am 15.08.2013 17:29, schrieb Alexander Graf:
>> 
>> On 15.08.2013, at 16:47, Andreas Färber wrote:
>> 
>>> There is nothing wrong with finding a mask or wildcard solution to that
>>> problem, I already indicated so on the original POWER+ patch. The point
>>> of the whole discussion is how to get there in the least invasive way.
>>> Not whether, just how.
>>> 
>>> I think - unlike Alex apparently - that the least invasive way is to
>>> leave models as they are and to add masking support to families and KVM
>>> code only.
>> 
>> Not sure I understand. What is KVM specific about this?
> 
> -cpu host is, it's in kvm.c.
> 
> These patches are changing sort comparison code in translate_ppc.c
> though, which is used in multiple places.
> 
>> 
>>> I'm already trying to get away from extending the
>>> POWERPC_DEF* macros for Prerna's fw_name, which are starting to get a
>>> big conflict point these days and a future pain if everyone extends them
>>> for the feature of the day. Note that I started with reading v3, not
>>> everything from the start, and am therefore not pointing fingers at
>>> anyone. It may be that you were given some unfortunate suggestions and
>>> too quick in implementing them.
>>> 
>>> When we instantiate a -cpu POWER9 then having one POWER9_vX.Y around to
>>> back it doesn't really hurt. Unlike ARM's MIDR there doesn't seem to be
>>> an encoding of IBM vendor or POWER family in the PVR. The macros and
>>> their new implementation are not the way they are because I consider
>>> them the nicest thing in the world but because the name+pvr+svr+family
>>> combination made them work for the whole zoo of models we carry around
>>> and started to give us some inheritance through QOM. Making the POWER7
>>> family non-abstract would require the same kind of macro "overloading"
>>> for POWERPC_FAMILY that I'm trying to contain for POWERPC_DEF ATM. So
>>> what I am still thinking about is how to handle there being multiple
>>> matches for a PVR - I am considering putting them into a list and
>>> comparing values for closest match. So that if you have a v2.4 and QEMU
>>> knows v2.1 and v2.3 we take v2.3 and fill in the v2.4 PVR.
>> 
>> I think this goes into the wrong direction. We should have one single 
>> unified scheme to model core versions and -cpu host should be able to 
>> override them for a family, no? I don't see how instantiating a POWER7_v20 
>> object on a POWER7_v23 system is any improvement over instantiating a POWER7 
>> object.
> 
> There is no one unified scheme, as we have discussed in your absence.
> 
> My point is, a) -cpu POWER7 should result in valid values

Yes :)

> and b) you
> asked to have a unified macro scheme that works for all CPUs, you got
> it, now instead you are asking for something that is nice to POWERx, and
> we cannot make POWERx family different from the rest wrt macros unless
> we break the scheme, which you specifically wanted to have, to avoid
> boilerplate QOM code you said. Now you want the full customization
> goodness that you were against just before! :)

Ah, nonono, I don't want POWER to be any different. I want things unified and 
consistent. Any time I mention "POWER7" I also mean "e500" or "440" or any 
other family class we have out there.

What I was proposing was to make _all_ families non-abstract and have _all_ 
families support major/minor parameters.

> Andreas
> 
> P.S. Please configure your mail client to break lines. Replying is
> really hard this way...

Phew - no idea how :).


Alex