diff mbox

RFC: powerpc: add PVR compatibility check

Message ID 1383536166-8673-1-git-send-email-aik@ozlabs.ru
State New
Headers show

Commit Message

Alexey Kardashevskiy Nov. 4, 2013, 3:36 a.m. UTC
If QEMU is started with KVM enabled and -cpu specified, and the CPU is not
from the family which the host is running on, an error should be displayed
so this the patch does.

Cc: Andreas Färber <afaerber@suse.de>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>

---

Is that correct to assume that the closest abstract class is a CPU family?
It is most likely true but I want to double check :)

Is there any nicer way of doing what the patch does?

Thanks.


---
 target-ppc/translate_init.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

Comments

Alexander Graf Nov. 4, 2013, 7:47 a.m. UTC | #1
On 04.11.2013, at 04:36, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:

> If QEMU is started with KVM enabled and -cpu specified, and the CPU is not
> from the family which the host is running on, an error should be displayed
> so this the patch does.
> 
> Cc: Andreas Färber <afaerber@suse.de>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> 
> ---
> 
> Is that correct to assume that the closest abstract class is a CPU family?
> It is most likely true but I want to double check :)

I don't think this is correct. KVM on POWER7 is compatible to run a 750 vcpu for example.

> Is there any nicer way of doing what the patch does?

The only instance that knows whether it's compatible or not is the kvm kernel module. Currently the only way we can check compatibility is through the "pvr" value that user space pushes into the kernel.

I see two ways we can enhance this. We could add checks to kvm's HV mode to make sure the guest vcpu type is compatible. Since the list of supported PVRs is quite small this might even be feasible.

The other thing that would be nice would be to transfer a full blob of capabilities into kvm that we can match for, similar to how cpuid on x86 works. That way we can then match host features with guest features and can check compatibility on a much more fine grained level.

The big benefit of the second approach is that when someone crazy enough comes in to implement e500mc on book3s kvm for example, he could do that simply by setting a few different capability bits. It would also make paired single selection more obvious for example. And we could limit Altivec access to only CPUs that have it rather than exposing it for all as we do today.


Alex
Alexey Kardashevskiy Nov. 4, 2013, 8:58 a.m. UTC | #2
On 11/04/2013 06:47 PM, Alexander Graf wrote:
> 
> On 04.11.2013, at 04:36, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> 
>> If QEMU is started with KVM enabled and -cpu specified, and the CPU is not
>> from the family which the host is running on, an error should be displayed
>> so this the patch does.
>>
>> Cc: Andreas Färber <afaerber@suse.de>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>
>> ---
>>
>> Is that correct to assume that the closest abstract class is a CPU family?
>> It is most likely true but I want to double check :)
> 
> I don't think this is correct. KVM on POWER7 is compatible to run a 750
> vcpu for example.


Are you talking about PR KVM or HV KVM now? How does it work? What are the
PCR's v2.05/v.206 bits in this case? They must be set to something, no?

I understood this as with KVM we have to create CPU of the family which the
host CPU belongs to and if we want to support some lower version, then we
use compatibility mode. Hm.


>> Is there any nicer way of doing what the patch does?
> 

> The only instance that knows whether it's compatible or not is the kvm
> kernel module. Currently the only way we can check compatibility is
> through the "pvr" value that user space pushes into the kernel.

HV KVM does not virtualize PVR and the userspace can only try PCR which
defines very few compatibility modes and KVM can fail on setting wrong modes.

> I see two ways we can enhance this. We could add checks to kvm's HV
> mode to make sure the guest vcpu type is compatible. Since the list
> of supported PVRs is quite small this might even be feasible.

Since the list is small and we know all possible combinations - why to
bother about this in the kernel?

> The other thing that would be nice would be to transfer a full blob of
> capabilities into kvm that we can match for, similar to how cpuid on x86
> works. That way we can then match host features with guest features and
> can check compatibility on a much more fine grained level.

We have such a blob, it is called "client-architecture-support" :) But it
is PAPR, i.e. "proprietary" :( And again, there is nothing (yet?) which we
cannot process in QEMU and can in KVM.


> The big benefit of the second approach is that when someone crazy enough
> comes in to implement e500mc on book3s kvm for example, he could do that
> simply by setting a few different capability bits. It would also make
> paired single selection more obvious for example. And we could limit
> Altivec access to only CPUs that have it rather than exposing it for all
> as we do today.

I am confused. How do existing guest kernels know if Altivec is supported
or not? I thought this is nailed to PVR and you cannot expose standalone
features.

Adding Paul to cc:.
Alexander Graf Nov. 4, 2013, 9:05 a.m. UTC | #3
On 04.11.2013, at 09:58, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:

> On 11/04/2013 06:47 PM, Alexander Graf wrote:
>> 
>> On 04.11.2013, at 04:36, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>> 
>>> If QEMU is started with KVM enabled and -cpu specified, and the CPU is not
>>> from the family which the host is running on, an error should be displayed
>>> so this the patch does.
>>> 
>>> Cc: Andreas Färber <afaerber@suse.de>
>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>> 
>>> ---
>>> 
>>> Is that correct to assume that the closest abstract class is a CPU family?
>>> It is most likely true but I want to double check :)
>> 
>> I don't think this is correct. KVM on POWER7 is compatible to run a 750
>> vcpu for example.
> 
> 
> Are you talking about PR KVM or HV KVM now?

We are talking about QEMU here which means we always have to take the whole picture into account. The 750-on-POWER7 case only works with PR KVM.

> How does it work? What are the
> PCR's v2.05/v.206 bits in this case? They must be set to something, no?
> 
> I understood this as with KVM we have to create CPU of the family which the
> host CPU belongs to and if we want to support some lower version, then we
> use compatibility mode. Hm.

That's HV KVM specific. There is no reason a user couldn't use QEMU on PR KVM.

> 
> 
>>> Is there any nicer way of doing what the patch does?
>> 
> 
>> The only instance that knows whether it's compatible or not is the kvm
>> kernel module. Currently the only way we can check compatibility is
>> through the "pvr" value that user space pushes into the kernel.
> 
> HV KVM does not virtualize PVR and the userspace can only try PCR which
> defines very few compatibility modes and KVM can fail on setting wrong modes.

HV KVM should simply fail when vcpu pvr != host pvr.

> 
>> I see two ways we can enhance this. We could add checks to kvm's HV
>> mode to make sure the guest vcpu type is compatible. Since the list
>> of supported PVRs is quite small this might even be feasible.
> 
> Since the list is small and we know all possible combinations - why to
> bother about this in the kernel?

Yeah, we really need to check that guest vpcu == host vcpu for HV KVM.

> 
>> The other thing that would be nice would be to transfer a full blob of
>> capabilities into kvm that we can match for, similar to how cpuid on x86
>> works. That way we can then match host features with guest features and
>> can check compatibility on a much more fine grained level.
> 
> We have such a blob, it is called "client-architecture-support" :) But it
> is PAPR, i.e. "proprietary" :( And again, there is nothing (yet?) which we
> cannot process in QEMU and can in KVM.

Please start to think outside of the HV KVM box.

> 
> 
>> The big benefit of the second approach is that when someone crazy enough
>> comes in to implement e500mc on book3s kvm for example, he could do that
>> simply by setting a few different capability bits. It would also make
>> paired single selection more obvious for example. And we could limit
>> Altivec access to only CPUs that have it rather than exposing it for all
>> as we do today.
> 
> I am confused. How do existing guest kernels know if Altivec is supported
> or not? I thought this is nailed to PVR and you cannot expose standalone
> features.

Yes, today the only way we tell the kernel whether a guest vcpu supports Altivec is through PVR. That was a bad design decision. I think it would make more sense to give kvm a full list of features it can then base on rather than only the PVR. We could then check those features against host features, in the emulator and in external feature (altivec, spe, fpu, etc) enablement.


Alex
Alexey Kardashevskiy Nov. 4, 2013, 9:24 a.m. UTC | #4
On 11/04/2013 08:05 PM, Alexander Graf wrote:
> 
> On 04.11.2013, at 09:58, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> 
>> On 11/04/2013 06:47 PM, Alexander Graf wrote:
>>>
>>> On 04.11.2013, at 04:36, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>
>>>> If QEMU is started with KVM enabled and -cpu specified, and the CPU is not
>>>> from the family which the host is running on, an error should be displayed
>>>> so this the patch does.
>>>>
>>>> Cc: Andreas Färber <afaerber@suse.de>
>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>>>
>>>> ---
>>>>
>>>> Is that correct to assume that the closest abstract class is a CPU family?
>>>> It is most likely true but I want to double check :)
>>>
>>> I don't think this is correct. KVM on POWER7 is compatible to run a 750
>>> vcpu for example.
>>
>>
>> Are you talking about PR KVM or HV KVM now?
> 
> We are talking about QEMU here which means we always have to take the
> whole picture into account. The 750-on-POWER7 case only works with PR
> KVM.
>
>> How does it work? What are the
>> PCR's v2.05/v.206 bits in this case? They must be set to something, no?
>>
>> I understood this as with KVM we have to create CPU of the family which the
>> host CPU belongs to and if we want to support some lower version, then we
>> use compatibility mode. Hm.
> 
> That's HV KVM specific. There is no reason a user couldn't use QEMU on PR KVM.
> 
>>
>>
>>>> Is there any nicer way of doing what the patch does?
>>>
>>
>>> The only instance that knows whether it's compatible or not is the kvm
>>> kernel module. Currently the only way we can check compatibility is
>>> through the "pvr" value that user space pushes into the kernel.
>>
>> HV KVM does not virtualize PVR and the userspace can only try PCR which
>> defines very few compatibility modes and KVM can fail on setting wrong modes.
> 
> HV KVM should simply fail when vcpu pvr != host pvr.


More precisely, pvr&mask != host_pvr&mask. That is what I really wanted
here but I do not know how to distinguish PR and HV KVM in
target-ppc/translate_init.c.


>>> I see two ways we can enhance this. We could add checks to kvm's HV
>>> mode to make sure the guest vcpu type is compatible. Since the list
>>> of supported PVRs is quite small this might even be feasible.
>>
>> Since the list is small and we know all possible combinations - why to
>> bother about this in the kernel?
> 
> Yeah, we really need to check that guest vpcu == host vcpu for HV KVM.
> 
>>
>>> The other thing that would be nice would be to transfer a full blob of
>>> capabilities into kvm that we can match for, similar to how cpuid on x86
>>> works. That way we can then match host features with guest features and
>>> can check compatibility on a much more fine grained level.
>>
>> We have such a blob, it is called "client-architecture-support" :) But it
>> is PAPR, i.e. "proprietary" :( And again, there is nothing (yet?) which we
>> cannot process in QEMU and can in KVM.
> 
> Please start to think outside of the HV KVM box.


I am trying. I looked through PowerISA and did not find any mechanism to
tell the guest whether the host supports Altivec or not. So I assumed that
PR KVM can only do it by setting a PVR of a CPU which does or does not
support Altivec. Yes, my patch does not take PR KVM into account, this is
why it is an "RFC" patch and I am asking all these stupid questions as I do
not really understand where to insert such a check in QEMU.

After all, now it seems right to do this check in KVM to avoid having PR
vs. HV cases in QEMU.



>>> The big benefit of the second approach is that when someone crazy enough
>>> comes in to implement e500mc on book3s kvm for example, he could do that
>>> simply by setting a few different capability bits. It would also make
>>> paired single selection more obvious for example. And we could limit
>>> Altivec access to only CPUs that have it rather than exposing it for all
>>> as we do today.
>>
>> I am confused. How do existing guest kernels know if Altivec is supported
>> or not? I thought this is nailed to PVR and you cannot expose standalone
>> features.
> 

> Yes, today the only way we tell the kernel whether a guest vcpu supports
> Altivec is through PVR. That was a bad design decision. I think it would
> make more sense to give kvm a full list of features it can then base on
> rather than only the PVR. We could then check those features against
> host features, in the emulator and in external feature (altivec, spe,
> fpu, etc) enablement.

What will KVM do with those bits? How exactly will it tell the _guest_ that
it does or does not support Altivec? I mean, in addition to setting a
correct PVR? May be there is some good specification (besides PowerISA)
which I am missing?
Alexander Graf Nov. 4, 2013, 9:29 a.m. UTC | #5
On 04.11.2013, at 10:24, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:

> On 11/04/2013 08:05 PM, Alexander Graf wrote:
>> 
>> On 04.11.2013, at 09:58, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>> 
>>> On 11/04/2013 06:47 PM, Alexander Graf wrote:
>>>> 
>>>> On 04.11.2013, at 04:36, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>> 
>>>>> If QEMU is started with KVM enabled and -cpu specified, and the CPU is not
>>>>> from the family which the host is running on, an error should be displayed
>>>>> so this the patch does.
>>>>> 
>>>>> Cc: Andreas Färber <afaerber@suse.de>
>>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>>>> 
>>>>> ---
>>>>> 
>>>>> Is that correct to assume that the closest abstract class is a CPU family?
>>>>> It is most likely true but I want to double check :)
>>>> 
>>>> I don't think this is correct. KVM on POWER7 is compatible to run a 750
>>>> vcpu for example.
>>> 
>>> 
>>> Are you talking about PR KVM or HV KVM now?
>> 
>> We are talking about QEMU here which means we always have to take the
>> whole picture into account. The 750-on-POWER7 case only works with PR
>> KVM.
>> 
>>> How does it work? What are the
>>> PCR's v2.05/v.206 bits in this case? They must be set to something, no?
>>> 
>>> I understood this as with KVM we have to create CPU of the family which the
>>> host CPU belongs to and if we want to support some lower version, then we
>>> use compatibility mode. Hm.
>> 
>> That's HV KVM specific. There is no reason a user couldn't use QEMU on PR KVM.
>> 
>>> 
>>> 
>>>>> Is there any nicer way of doing what the patch does?
>>>> 
>>> 
>>>> The only instance that knows whether it's compatible or not is the kvm
>>>> kernel module. Currently the only way we can check compatibility is
>>>> through the "pvr" value that user space pushes into the kernel.
>>> 
>>> HV KVM does not virtualize PVR and the userspace can only try PCR which
>>> defines very few compatibility modes and KVM can fail on setting wrong modes.
>> 
>> HV KVM should simply fail when vcpu pvr != host pvr.
> 
> 
> More precisely, pvr&mask != host_pvr&mask.

Are you sure? After all, we can not fake a different PVR to the guest in the first place, no? So if we declare -cpu POWER7_v10 on a POWER7_v20 system as supported the result the guest sees would be wrong, as it would see a v20 CPU, not a v10 CPU.

> That is what I really wanted
> here but I do not know how to distinguish PR and HV KVM in
> target-ppc/translate_init.c.

You shouldn't. Only the kernel should really care about the difference.

> 
> 
>>>> I see two ways we can enhance this. We could add checks to kvm's HV
>>>> mode to make sure the guest vcpu type is compatible. Since the list
>>>> of supported PVRs is quite small this might even be feasible.
>>> 
>>> Since the list is small and we know all possible combinations - why to
>>> bother about this in the kernel?
>> 
>> Yeah, we really need to check that guest vpcu == host vcpu for HV KVM.
>> 
>>> 
>>>> The other thing that would be nice would be to transfer a full blob of
>>>> capabilities into kvm that we can match for, similar to how cpuid on x86
>>>> works. That way we can then match host features with guest features and
>>>> can check compatibility on a much more fine grained level.
>>> 
>>> We have such a blob, it is called "client-architecture-support" :) But it
>>> is PAPR, i.e. "proprietary" :( And again, there is nothing (yet?) which we
>>> cannot process in QEMU and can in KVM.
>> 
>> Please start to think outside of the HV KVM box.
> 
> 
> I am trying. I looked through PowerISA and did not find any mechanism to
> tell the guest whether the host supports Altivec or not. So I assumed that
> PR KVM can only do it by setting a PVR of a CPU which does or does not
> support Altivec. Yes, my patch does not take PR KVM into account, this is
> why it is an "RFC" patch and I am asking all these stupid questions as I do
> not really understand where to insert such a check in QEMU.
> 
> After all, now it seems right to do this check in KVM to avoid having PR
> vs. HV cases in QEMU.

Yup :).

> 
> 
> 
>>>> The big benefit of the second approach is that when someone crazy enough
>>>> comes in to implement e500mc on book3s kvm for example, he could do that
>>>> simply by setting a few different capability bits. It would also make
>>>> paired single selection more obvious for example. And we could limit
>>>> Altivec access to only CPUs that have it rather than exposing it for all
>>>> as we do today.
>>> 
>>> I am confused. How do existing guest kernels know if Altivec is supported
>>> or not? I thought this is nailed to PVR and you cannot expose standalone
>>> features.
>> 
> 
>> Yes, today the only way we tell the kernel whether a guest vcpu supports
>> Altivec is through PVR. That was a bad design decision. I think it would
>> make more sense to give kvm a full list of features it can then base on
>> rather than only the PVR. We could then check those features against
>> host features, in the emulator and in external feature (altivec, spe,
>> fpu, etc) enablement.
> 
> What will KVM do with those bits? How exactly will it tell the _guest_ that
> it does or does not support Altivec? I mean, in addition to setting a
> correct PVR? May be there is some good specification (besides PowerISA)
> which I am missing?

Ok, let's take a closer look at Altivec as a nice example.

Today, when the guest tries to execute an Altivec instruction, it will fault with an "Altivec unavailable" interrupt in the host. We deflect that back into the guest.
A real 750 CPU however would have triggered an "Illegal instruction" interrupt. So we diverge from what real hardware does and issue an interrupt in the guest which it wouldn't expect.


Alex
Paul Mackerras Nov. 5, 2013, 4 a.m. UTC | #6
On Mon, Nov 04, 2013 at 10:05:58AM +0100, Alexander Graf wrote:
> 
> Yeah, we really need to check that guest vpcu == host vcpu for HV KVM.

In general I agree, but the one difficulty I see is that a check for
exact equality will interact badly with qemu's habit of picking a
specific processor version when the user specifies something general
like "POWER7".  So if the user does -cpu POWER7 on a machine with
(for example) a POWER7 v2.1 processor, but qemu arbitrarily picks the
PVR for POWER7 v2.3, then it will fail, which will be completely
puzzling to the user -- "I asked for POWER7, and it is a POWER7,
what's the problem??".

Maybe if the user asks for a non-specific processor type, and the
host's PVR matches the request, then qemu should take the host's PVR
rather than just picking some arbitrary processor version.

Paul.
Alexander Graf Nov. 5, 2013, 6:05 a.m. UTC | #7
Am 05.11.2013 um 05:00 schrieb Paul Mackerras <paulus@samba.org>:

> On Mon, Nov 04, 2013 at 10:05:58AM +0100, Alexander Graf wrote:
>> 
>> Yeah, we really need to check that guest vpcu == host vcpu for HV KVM.
> 
> In general I agree, but the one difficulty I see is that a check for
> exact equality will interact badly with qemu's habit of picking a
> specific processor version when the user specifies something general
> like "POWER7".  So if the user does -cpu POWER7 on a machine with
> (for example) a POWER7 v2.1 processor, but qemu arbitrarily picks the
> PVR for POWER7 v2.3, then it will fail, which will be completely
> puzzling to the user -- "I asked for POWER7, and it is a POWER7,
> what's the problem??".
> 
> Maybe if the user asks for a non-specific processor type, and the
> host's PVR matches the request, then qemu should take the host's PVR
> rather than just picking some arbitrary processor version.

Yup.

Alex
Andreas Färber Nov. 5, 2013, 4:16 p.m. UTC | #8
Am 05.11.2013 07:05, schrieb Alexander Graf:
> 
> 
> Am 05.11.2013 um 05:00 schrieb Paul Mackerras <paulus@samba.org>:
> 
>> On Mon, Nov 04, 2013 at 10:05:58AM +0100, Alexander Graf wrote:
>>>
>>> Yeah, we really need to check that guest vpcu == host vcpu for HV KVM.
>>
>> In general I agree, but the one difficulty I see is that a check for
>> exact equality will interact badly with qemu's habit of picking a
>> specific processor version when the user specifies something general
>> like "POWER7".  So if the user does -cpu POWER7 on a machine with
>> (for example) a POWER7 v2.1 processor, but qemu arbitrarily picks the
>> PVR for POWER7 v2.3, then it will fail, which will be completely
>> puzzling to the user -- "I asked for POWER7, and it is a POWER7,
>> what's the problem??".
>>
>> Maybe if the user asks for a non-specific processor type, and the
>> host's PVR matches the request, then qemu should take the host's PVR
>> rather than just picking some arbitrary processor version.
> 
> Yup.

But then it's no longer generally reproducible: "POWER7" won't be
"POWER7" on another machine.

One thing I original did iirc was to hide the aliases from QMP. You can
always do stupid things on the command line and then we can blame you,
but if libvirt and upper layers don't offer "POWER7" to the end user
then we don't need to worry about the average user misinterpreting its
semantics.

Andreas
Alexander Graf Nov. 5, 2013, 4:18 p.m. UTC | #9
On 05.11.2013, at 17:16, Andreas Färber <afaerber@suse.de> wrote:

> Am 05.11.2013 07:05, schrieb Alexander Graf:
>> 
>> 
>> Am 05.11.2013 um 05:00 schrieb Paul Mackerras <paulus@samba.org>:
>> 
>>> On Mon, Nov 04, 2013 at 10:05:58AM +0100, Alexander Graf wrote:
>>>> 
>>>> Yeah, we really need to check that guest vpcu == host vcpu for HV KVM.
>>> 
>>> In general I agree, but the one difficulty I see is that a check for
>>> exact equality will interact badly with qemu's habit of picking a
>>> specific processor version when the user specifies something general
>>> like "POWER7".  So if the user does -cpu POWER7 on a machine with
>>> (for example) a POWER7 v2.1 processor, but qemu arbitrarily picks the
>>> PVR for POWER7 v2.3, then it will fail, which will be completely
>>> puzzling to the user -- "I asked for POWER7, and it is a POWER7,
>>> what's the problem??".
>>> 
>>> Maybe if the user asks for a non-specific processor type, and the
>>> host's PVR matches the request, then qemu should take the host's PVR
>>> rather than just picking some arbitrary processor version.
>> 
>> Yup.
> 
> But then it's no longer generally reproducible: "POWER7" won't be
> "POWER7" on another machine.
> 
> One thing I original did iirc was to hide the aliases from QMP. You can
> always do stupid things on the command line and then we can blame you,
> but if libvirt and upper layers don't offer "POWER7" to the end user
> then we don't need to worry about the average user misinterpreting its
> semantics.

We could also just be nice and error out with a sane error message:

  Your selected CPU POWER7_v10 is not compatible with the host's CPU POWER7_20
  with your currently selected KVM mode. Please use the latter or -cpu host.


Alex
Paul Mackerras Nov. 6, 2013, 3:02 a.m. UTC | #10
On Tue, Nov 05, 2013 at 05:16:33PM +0100, Andreas Färber wrote:
> Am 05.11.2013 07:05, schrieb Alexander Graf:
> > 
> > 
> > Am 05.11.2013 um 05:00 schrieb Paul Mackerras <paulus@samba.org>:
> > 
> >> On Mon, Nov 04, 2013 at 10:05:58AM +0100, Alexander Graf wrote:
> >>>
> >>> Yeah, we really need to check that guest vpcu == host vcpu for HV KVM.
> >>
> >> In general I agree, but the one difficulty I see is that a check for
> >> exact equality will interact badly with qemu's habit of picking a
> >> specific processor version when the user specifies something general
> >> like "POWER7".  So if the user does -cpu POWER7 on a machine with
> >> (for example) a POWER7 v2.1 processor, but qemu arbitrarily picks the
> >> PVR for POWER7 v2.3, then it will fail, which will be completely
> >> puzzling to the user -- "I asked for POWER7, and it is a POWER7,
> >> what's the problem??".
> >>
> >> Maybe if the user asks for a non-specific processor type, and the
> >> host's PVR matches the request, then qemu should take the host's PVR
> >> rather than just picking some arbitrary processor version.
> > 
> > Yup.
> 
> But then it's no longer generally reproducible: "POWER7" won't be
> "POWER7" on another machine.

There aren't any observable differences between POWER7 versions that
have been sold to customers, as far as I have been able to ascertain
(other than the PVR value, of course).  So this whole business of
carefully distinguishing between POWER7 v2.2 and POWER7 v2.3 is
largely a waste of time as far as I can see.

I admit that in the past we (IBM) did a silly thing in releasing the
POWER5+ v3.0 chip with some architecturally new features (64k pages
and some other MMU changes).  That was a mistake and I don't think
we'll do it again.

I think the default assumption should be that versions of a given IBM
POWER chip (identified by the upper 16 bits of the PVR) are
architecturally identical, and behaviourally identical at the level
at which QEMU models the chip.  Differences between chips would
normally be limited to bug fixes and performance improvements.  Then
we just need a way to cope with POWER5+ v3.0.

> One thing I original did iirc was to hide the aliases from QMP. You can
> always do stupid things on the command line and then we can blame you,
> but if libvirt and upper layers don't offer "POWER7" to the end user
> then we don't need to worry about the average user misinterpreting its
> semantics.

Given that the only difference between POWER7 v2.2 and POWER7 v2.3
(say) will be which set of host systems you get an error on, there
doesn't seem to me to be a lot of point.

Paul.
Alexander Graf Nov. 6, 2013, 12:19 p.m. UTC | #11
On 06.11.2013, at 04:02, Paul Mackerras <paulus@samba.org> wrote:

> On Tue, Nov 05, 2013 at 05:16:33PM +0100, Andreas Färber wrote:
>> Am 05.11.2013 07:05, schrieb Alexander Graf:
>>> 
>>> 
>>> Am 05.11.2013 um 05:00 schrieb Paul Mackerras <paulus@samba.org>:
>>> 
>>>> On Mon, Nov 04, 2013 at 10:05:58AM +0100, Alexander Graf wrote:
>>>>> 
>>>>> Yeah, we really need to check that guest vpcu == host vcpu for HV KVM.
>>>> 
>>>> In general I agree, but the one difficulty I see is that a check for
>>>> exact equality will interact badly with qemu's habit of picking a
>>>> specific processor version when the user specifies something general
>>>> like "POWER7".  So if the user does -cpu POWER7 on a machine with
>>>> (for example) a POWER7 v2.1 processor, but qemu arbitrarily picks the
>>>> PVR for POWER7 v2.3, then it will fail, which will be completely
>>>> puzzling to the user -- "I asked for POWER7, and it is a POWER7,
>>>> what's the problem??".
>>>> 
>>>> Maybe if the user asks for a non-specific processor type, and the
>>>> host's PVR matches the request, then qemu should take the host's PVR
>>>> rather than just picking some arbitrary processor version.
>>> 
>>> Yup.
>> 
>> But then it's no longer generally reproducible: "POWER7" won't be
>> "POWER7" on another machine.
> 
> There aren't any observable differences between POWER7 versions that
> have been sold to customers, as far as I have been able to ascertain
> (other than the PVR value, of course).  So this whole business of
> carefully distinguishing between POWER7 v2.2 and POWER7 v2.3 is
> largely a waste of time as far as I can see.
> 
> I admit that in the past we (IBM) did a silly thing in releasing the
> POWER5+ v3.0 chip with some architecturally new features (64k pages
> and some other MMU changes).  That was a mistake and I don't think
> we'll do it again.
> 
> I think the default assumption should be that versions of a given IBM
> POWER chip (identified by the upper 16 bits of the PVR) are
> architecturally identical, and behaviourally identical at the level
> at which QEMU models the chip.  Differences between chips would
> normally be limited to bug fixes and performance improvements.  Then
> we just need a way to cope with POWER5+ v3.0.
> 
>> One thing I original did iirc was to hide the aliases from QMP. You can
>> always do stupid things on the command line and then we can blame you,
>> but if libvirt and upper layers don't offer "POWER7" to the end user
>> then we don't need to worry about the average user misinterpreting its
>> semantics.
> 
> Given that the only difference between POWER7 v2.2 and POWER7 v2.3
> (say) will be which set of host systems you get an error on, there
> doesn't seem to me to be a lot of point.

Given that for HV KVM the only possible cpu type that is ever guaranteed to work is -cpu host, why bother with a database that maintains compatibility flags between different versions? Just tell the user to use -cpu host and call it a day, no?


Alex
diff mbox

Patch

diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 3d3952c..6888214 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -8314,6 +8314,14 @@  static ObjectClass *ppc_cpu_class_by_name(const char *name)
     return ret;
 }
 
+ObjectClass *object_class_get_abstract_parent(ObjectClass *class)
+{
+    while (class && !object_class_is_abstract(class)) {
+        class = object_class_get_parent(class);
+    }
+    return class;
+}
+
 PowerPCCPU *cpu_ppc_init(const char *cpu_model)
 {
     PowerPCCPU *cpu;
@@ -8325,6 +8333,26 @@  PowerPCCPU *cpu_ppc_init(const char *cpu_model)
         return NULL;
     }
 
+    /*
+     * Make sure that if we run with a specific CPU in the command line when
+     * KVM is enabled, it still from the same CPU family as the host CPU
+     * because if they are not, the compatibility mode should be used.
+     */
+    if (strcmp(object_class_get_name(oc), TYPE_HOST_POWERPC_CPU)) {
+        ObjectClass *oc_host = object_class_by_name(TYPE_HOST_POWERPC_CPU);
+        ObjectClass *ocp = object_class_get_abstract_parent(oc);
+
+        if (oc_host) {
+            ObjectClass *ocp_host = object_class_get_abstract_parent(oc_host);
+            if (ocp != ocp_host) {
+                error_report("The chosen CPU \"%s\" is incompatible with the host CPU \"%s\"",
+                             object_class_get_name(oc),
+                             object_class_get_name(ocp_host));
+                return NULL;
+            }
+        }
+    }
+
     cpu = POWERPC_CPU(object_new(object_class_get_name(oc)));
 
     object_property_set_bool(OBJECT(cpu), true, "realized", &err);