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

[Alexey Kardashevskiy]

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. 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?


-- 
Alexey