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

[Alexander Graf]

On 15.08.2013, at 08:28, Benjamin Herrenschmidt wrote:

> On Thu, 2013-08-15 at 08:10 +0200, Alexander Graf wrote:
> 
>> So you're saying it's good to remove a well established feature on 5% 
>> of the supported CPUs, leave the others inconsistent with the change 
>> and then declare the whole thing an improvement?
> 
> WTF are you talking about ?
> 
> To need an exact PVR definition to the last bit means every time a minor
> chip rev comes out of IBM, KVM will stop working until qemu is updated
> to know about that revision.
> 
> This is dumb.

No disagreement here.

> 
> Being able to emulate a P7 2.1 vs a P7 2.3 is completely pointless since
> essentially they expose the same architecture and the bugs that are
> fixed between those revisions are for the most part not guest visible
> nor even emulated by qemu to begin with.
> 
> Now there *might* be some value in being able to specify among "known
> supported" versions for things like P5 (but frankly, who gives a damn ?
> Who is actually going to *use* that ? Nobody really ....)
> 
> In that case it's easy ... have a name match with the table entry.

Have you read my previous reply?

> With the mask & value, you can do like the kernel, ie, have first in the
> list the specific entries you want to match against (ie, P7_2_1,
> P7_2_3, ...) and fallback to a generic "P7 any revision" entry. That way
> qemu will still work if IBM releases a P7 v2.4 you don't know about.
> 
> As for selecting it, similarly, you can do an exact match on the name
> (or a partial match as a fallback, I don't care) and pickup the PVR
> value out of the table for emulation.
> 
> Point is, what we have now is crap. This is the best fix I've seen so
> far. It's useful, cover the 99.9% of the possible use cases I can think
> of, but you seem to care more about hypothetical scenario that have no
> practical interest on the field.

The patch makes things inconsistent. It moves POWER7, POWER7+ and POWER8 to a 
different model from everything else and removes already existing -cpu names. 
This is just plain wrong. I want a consistent solution that's future proof.


Alex