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

[Andreas Färber]

Am 15.08.2013 13:03, schrieb Alexander Graf:
> 
> On 15.08.2013, at 12:52, Andreas Färber wrote:
> 
>> Am 15.08.2013 10:45, schrieb Alexander Graf:
>>>
>>> Yes, I think it makes sense to keep the full PVR around when we want to be 
>>> specific. What I'm referring to is class specific logic that can assemble 
>>> major/minor numbers from the command line. So
>>>
>>>  -cpu POWER7,major=2,minor=0
>>>
>>> would result in a PVR value that is identical to POWER7_v2.0. The assembly 
>>> of this PVR value is class specific, because different classes of CPUs have 
>>> different semantics for their major and minor numbers.
>>>
>>> That way in the future we won't have to add any new version specific CPU 
>>> types but instead the user can assemble those himself, making everyone's 
>>> life a lot easier.
>>>
>>> My point was that if we have that logic, we could at the same place just 
>>> say "if my major/minor is 0, default to something reasonable".
>>>
>>> But let's ask Andreas whether he has a better idea here :).
>>
>> If you read the previous discussion on the initial POWER7+ patch, I
>> believe I had proposed major-version / minor-version or so properties at
>> family level, to be able to use different implementations or none at all
>> where we don't see a scheme.
> 
> Sounds like a good idea.
> 
>> However if we want to use that from -cpu as
>> in your example above, we would have to implement custom parsing code
>> for cpu_model, which I would rather avoid, given we want to replace it
>> with -device in the future.
> 
> Can't we make this generic QOM property parsing code?
> 
>   -cpu POWER7,major-version=2,minor-version=0
> 
> would do
> 
>   cpu = new POWER7(major-version = 2, minor_version = 0);
> 
> and then the POWER7 class can decide what to do with this additional 
> information?

That is "custom parsing code for cpu_model" in target-ppc then. x86 has
its own implementation and so does sparc, both not fully QOM'ified yet,
so there is no one-size-fits-all.

>> But maybe I didn't fully catch the exact question. :)
>>
>> The custom parenting strikes me as a wrong consequence of us not having
>> fully QOM'ified / cleaned up the family classes yet. We had discussed
>> two ways: Either have, e.g., POWER7+ inherit from POWER7 (which looks
>> like the only reason this is being done here) and/or have, e.g., POWER5+
>> copy and modify 970fx values via #defines.
> 
> IIUC the family parenting is orthogonal to this. Here we're looking at having 
> families as classes at all. Currently we don't - we only have explicit 
> versioned implementations as classes.

That's simply not true!!! All is hidden by macros as requested by you -
sounds as if that was a bad idea after all. :/

We do have the following:

"object"
+- "device"
   +- "cpu"
      +- "powerpc64-cpu"
         +- "POWER7-family-powerpc64-cpu" -> POWERPC_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.

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.

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.

> Whether we have
> 
> PowerPC
>   `- POWER7
>     `- POWER7+
>       `- POWER7+ v1.0
> 
> or
> 
> PowerPC
>   `- POWER7+
>     `- POWER7+ v1.0
> 
> is a different question I think.

My question is: Why are you guys trying to create yet another type for
"POWER7" when we already have one. The only plausible-to-me explanation
was that avoidance of separate POWER7P family was the core cause, but
apparently the core problem is that no one except me is actually
grasping the macro'fied code or at least you lost the overview during
your vacation... :(

Andreas

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