Re: [Qemu-devel] [PATCH 3/7] KVM: i386: Add support for KVM_CAP_EXCEPTION_PAYLOAD

[Liran Alon]

> On 16 Jun 2019, at 15:38, Liran Alon <address@hidden> wrote:
> 
> 
> 
>> On 15 Jun 2019, at 3:57, Liran Alon <address@hidden> wrote:
>> 
>>> On 15 Jun 2019, at 3:42, Paolo Bonzini <address@hidden> wrote:
>>> 
>>> From: Liran Alon <address@hidden>
>>> 
>>> +static bool is_vmx_enabled(CPUX86State *env)
>>> +{
>>> +    return (IS_INTEL_CPU(env) && (env->cr[4] & CR4_VMXE_MASK));
>>> +}
>>> +
>>> +static bool is_svm_enabled(CPUX86State *env)
>>> +{
>>> +    return (IS_AMD_CPU(env) && (env->efer & MSR_EFER_SVME));
>>> +}
>>> +
>>> +static bool is_nested_virt_enabled(CPUX86State *env)
>>> +{
>>> +    return (is_vmx_enabled(env) || is_svm_enabled(env));
>>> +}
>> 
>> I have later realised that this nested_virt_enabled() function is not enough 
>> to determine if nested_state is required to be sent.
>> This is because it may be that vCPU is running L2 but have momentarily 
>> entered SMM due to SMI.
>> In this case, CR4 & MSR_EFER are saved in SMRAM and are set to 0 on entering 
>> to SMM.
>> This means that in case (env->hflags & HF_SMM_MASK), we theoretically should 
>> have read saved CR4 & MSR_EFER from SMRAM.
>> However, because we cannot reference guest memory at this point (Not valid 
>> in case we migrate guest in post-copy), I should change
>> code to assume that in case (env->hflags & HF_SMM_MASK), we need to assume 
>> that nested-state is needed.
>> This should break backwards-compatability migration only in very rare cases 
>> and therefore I think it should be sufficient.
>> Any objections to this idea?
>> 
> 
> Actually, this is even worse than I originally thought.
> Even in case guest is not currently in SMM mode, if it’s in VMX non-root 
> mode, the CR4 read here is L2 CR4. Not L1 CR4.
> Therefore, CR4.VMXE doesn’t necessarily indicate if guest have 
> nested-virtualization enabled. Same is true for MSR_EFER in case of SVM.
> 
> Putting this all together, in case kernel doesn’t support extracting 
> nested-state, there is no decent way to know if guest is running 
> nested-virtualization.
> Which means that in theory we always need to fail migration in case kernel 
> doesn’t support KVM_CAP_NESTED_STATE or KVM_CAP_EXCEPTION_PAYLOAD
> and vCPU is exposed with VMX/SVM capability.
> 
> I can condition this behaviour with a flag that can be manipulated using QMP 
> to allow user to indicate it wishes to migrate guest anyway in this case.
> This however bring me back to the entire discussion I had with Dr. David Alan 
> Gilbert on migration backwards compatibility in general and the fact that I 
> believe
> we should have a generic QMP command which allows to provide list of VMState 
> subsections that can be ignored in migration…
> See: https://www.mail-archive.com/address@hidden/msg622274.html
> 
> Paolo, What are your thoughts on how I would proceed with this?
> 
> -Liran
> 

Paolo, can you also elaborate what was your original intent in this QEMU commit 
you made:
f8dc4c645ec2 ("target/i386: rename HF_SVMI_MASK to HF_GUEST_MASK")

How did you expect this flag to be used in nVMX migration?
Currently the HF_GUEST_MASK flag in KVM that is set in vcpu->arch.hflags is 
never propagated to userspace.
Did you expect to set HF_GUEST_MASK in QEMU’s hflag based on nested_state 
returned by KVM_GET_NESTED_STATE ioctl?
What is the value of this hflag in QEMU given that we have all the information 
we need in env->nested_state?
(E.g. We can deduce vCPU is in VMX non-root mode by testing for 
(env->nested_state.flags & KVM_STATE_NESTED_GUEST_MODE)).

-Liran