From: "Andreas Färber" <afaerber@suse.de>
To: Alexander Graf <agraf@suse.de>, qemu-devel@nongnu.org
Cc: Alexey Kardashevskiy <aik@ozlabs.ru>,
qemu-stable@nongnu.org, Paolo Bonzini <pbonzini@redhat.com>,
qemu-ppc <qemu-ppc@nongnu.org>, kvm <kvm@vger.kernel.org>
Subject: Re: [PATCH] Revert "target-ppc: Create versionless CPU class per family if KVM"
Date: Mon, 02 Mar 2015 15:07:39 +0100 [thread overview]
Message-ID: <54F46EAB.6090205@suse.de> (raw)
In-Reply-To: <54F46AF7.60209@suse.de>
Am 02.03.2015 um 14:51 schrieb Alexander Graf:
> 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 <aik@ozlabs.ru>
>>>> Cc: qemu-stable@nongnu.org
>>>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>>>
>>> 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?
No, the class I create on the fly is a few lines above:
pvr_pcc = ppc_cpu_class_by_pvr(host_pvr);
if (pvr_pcc == NULL) {
pvr_pcc = ppc_cpu_class_by_pvr_mask(host_pvr);
}
if (pvr_pcc == NULL) {
return -1;
}
type_info.parent = object_class_get_name(OBJECT_CLASS(pvr_pcc));
type_register(&type_info);
So, if no matching class is returned, we never reach the offending code.
Here, a second type with the same parent was being created in the
kvm_ppc_register_host_cpu_type() function that is supposed to create
that host CPU type. Why? The host CPU type by definition should already
have the right PVR taken from the host. kvmppc_host_cpu_class_init():
/* Now fix up the class with information we can query from the host */
pcc->pvr = mfpvr();
> Alexey, please take a look at it.
Thanks,
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)
WARNING: multiple messages have this Message-ID (diff)
From: "Andreas Färber" <afaerber@suse.de>
To: Alexander Graf <agraf@suse.de>, qemu-devel@nongnu.org
Cc: Alexey Kardashevskiy <aik@ozlabs.ru>,
Paolo Bonzini <pbonzini@redhat.com>,
qemu-ppc <qemu-ppc@nongnu.org>,
qemu-stable@nongnu.org, kvm <kvm@vger.kernel.org>
Subject: Re: [Qemu-devel] [PATCH] Revert "target-ppc: Create versionless CPU class per family if KVM"
Date: Mon, 02 Mar 2015 15:07:39 +0100 [thread overview]
Message-ID: <54F46EAB.6090205@suse.de> (raw)
In-Reply-To: <54F46AF7.60209@suse.de>
Am 02.03.2015 um 14:51 schrieb Alexander Graf:
> 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 <aik@ozlabs.ru>
>>>> Cc: qemu-stable@nongnu.org
>>>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>>>
>>> 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?
No, the class I create on the fly is a few lines above:
pvr_pcc = ppc_cpu_class_by_pvr(host_pvr);
if (pvr_pcc == NULL) {
pvr_pcc = ppc_cpu_class_by_pvr_mask(host_pvr);
}
if (pvr_pcc == NULL) {
return -1;
}
type_info.parent = object_class_get_name(OBJECT_CLASS(pvr_pcc));
type_register(&type_info);
So, if no matching class is returned, we never reach the offending code.
Here, a second type with the same parent was being created in the
kvm_ppc_register_host_cpu_type() function that is supposed to create
that host CPU type. Why? The host CPU type by definition should already
have the right PVR taken from the host. kvmppc_host_cpu_class_init():
/* Now fix up the class with information we can query from the host */
pcc->pvr = mfpvr();
> Alexey, please take a look at it.
Thanks,
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)
next prev parent reply other threads:[~2015-03-02 14:07 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-03-01 0:31 [PATCH] Revert "target-ppc: Create versionless CPU class per family if KVM" Andreas Färber
2015-03-01 0:31 ` [Qemu-devel] " Andreas Färber
[not found] ` <54F4678D.20909@suse.de>
2015-03-02 13:42 ` Andreas Färber
2015-03-02 13:42 ` [Qemu-devel] " Andreas Färber
2015-03-02 13:51 ` Alexander Graf
2015-03-02 13:51 ` [Qemu-devel] " Alexander Graf
2015-03-02 14:07 ` Andreas Färber [this message]
2015-03-02 14:07 ` Andreas Färber
2015-03-03 0:42 ` Alexey Kardashevskiy
2015-03-03 0:42 ` [Qemu-devel] " Alexey Kardashevskiy
2015-03-03 20:43 ` Alexander Graf
2015-03-03 20:43 ` [Qemu-devel] " Alexander Graf
2015-03-03 22:14 ` Alexey Kardashevskiy
2015-03-03 22:14 ` [Qemu-devel] " Alexey Kardashevskiy
2015-03-04 14:55 ` Andreas Färber
2015-03-04 14:55 ` [Qemu-devel] " Andreas Färber
2015-03-04 23:50 ` Alexey Kardashevskiy
2015-03-04 23:50 ` [Qemu-devel] " Alexey Kardashevskiy
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=54F46EAB.6090205@suse.de \
--to=afaerber@suse.de \
--cc=agraf@suse.de \
--cc=aik@ozlabs.ru \
--cc=kvm@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-ppc@nongnu.org \
--cc=qemu-stable@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.