Re: [Qemu-devel] [PATCH] RFC: powerpc: add PVR compatibility check

[Alexander Graf]

On 04.11.2013, at 10:24, Alexey Kardashevskiy <address@hidden> wrote:

> On 11/04/2013 08:05 PM, Alexander Graf wrote:
>> 
>> On 04.11.2013, at 09:58, Alexey Kardashevskiy <address@hidden> wrote:
>> 
>>> On 11/04/2013 06:47 PM, Alexander Graf wrote:
>>>> 
>>>> On 04.11.2013, at 04:36, Alexey Kardashevskiy <address@hidden> 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 <address@hidden>
>>>>> Signed-off-by: Alexey Kardashevskiy <address@hidden>
>>>>> 
>>>>> ---
>>>>> 
>>>>> 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