Re: [Qemu-devel] [PATCH] Revert "target-ppc: Create versionless CPU class per family if KVM"

[Andreas Färber]

Am 03.03.2015 um 23:14 schrieb Alexey Kardashevskiy:
> On 03/04/2015 07:43 AM, Alexander Graf wrote:
>> On 03.03.15 01:42, Alexey Kardashevskiy wrote:
>>> On 03/03/2015 12:51 AM, Alexander Graf wrote:
>>>> On 02.03.15 14:42, Andreas Färber wrote:
>>>>> Am 02.03.2015 um 14:37 schrieb Alexander Graf:
>>>>>> On 01.03.15 01:31, Andreas Färber wrote:
>>>>>>> This reverts commit 5b79b1cadd3e565b6d1a5ba59764bd47af58b271 to
>>>>>>> avoid
>>>>>>> double-registration of types:
>>>>>>>
>>>>>>>     Registering `POWER5+-powerpc64-cpu' which already exists
>>>>>>>
>>>>>>> Taking the textual description of a CPU type as part of a new type
>>>>>>> name
>>>>>>> is plain wrong, and so is unconditionally registering a new type
>>>>>>> here.
>>>>>>>
>>>>>>> Cc: Alexey Kardashevskiy <address@hidden>
>>>>>>> Cc: address@hidden
>>>>>>> Signed-off-by: Andreas Färber <address@hidden>
>>>>>>
>>>>>> Doesn't this break p8 support?
>>>>>
>>>>> Maybe, but p5 support was in longer and this is definitely a
>>>>> regression
>>>>> and really really wrong. If you know a way to fix it without
>>>>> handing it
>>>>> back to the IBM guys for more thought, feel free to give it a shot.
>>>>
>>>> I honestly don't fully remember what this was about. Wasn't this our
>>>> special KVM class that we use to create a compatible cpu type on the
>>>> fly?
>>>>
>>>> Alexey, please take a look at it.
>>>
>>>
>>> I sent a note yesterday :-/ Here it is again:
>>>
>>> With this revert, running qemu with HV KVM and -cpu POWER7 fails on real
>>> POWER7 machine as my machine has pvr 003f 0201 and POWER7 is an alias of
>>> POWER7_v2.3 (pvr 003f 0203); and this is what I tried to fix at the
>>> first place. QEMU looks at classes first, and if not found - at aliases,
>>> so this worked.
>>>
>>> I would rename "POWER5+" to "POWER5+_0.0" and make "POWER5+" an alias
>>> for POWER5+_v2.1 (or POWER5+_0.0).
>>
>> Care to send a patch?
> 
> I wonder if Andreas has a better solution to my initial problem - he
> obviously won't like the proposed patch :)

Quite predictable, am I? ;)

Could you please explain in detail what problem you are seeing on POWER8
without this patch?

>From your new patch it rather sounds as if this was totally unrelated to
-cpu host and a new KVM-only feature, reinforcing my feeling that my
function is the wrong place for your code.

Also, as I pointed out, the description cannot safely be used as part of
the type name, as it may contain prohibited characters, so this still
needs fixing.

And for sure, if registering new types is indeed needed here, then a
check is needed for whether that type already exists and appropriate
error handling. I just don't understand why that is needed at all with
-cpu host taking the PVR as you say is needed here.

If you can precisely tell me what it is that you need then I'd be happy
to cook up a patch.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)