Re: [Qemu-devel] [PATCH] target/i386: kvm: Fix when nested state is needed for migration

[Liran Alon]

> On 2 Jul 2019, at 19:39, Paolo Bonzini <address@hidden> wrote:
> 
> On 25/06/19 01:05, Liran Alon wrote:
>> When vCPU is in VMX operation and enters SMM mode,
>> it temporarily exits VMX operation but KVM maintained nested-state
>> still stores the VMXON region physical address, i.e. even when the
>> vCPU is in SMM mode then (nested_state->hdr.vmx.vmxon_pa != -1ull).
>> 
>> Therefore, there is no need to explicitly check for
>> KVM_STATE_NESTED_SMM_VMXON to determine if it is necessary
>> to save nested-state as part of migration stream.
>> 
>> In addition, destination must enable eVMCS if it is enabled on
>> source as specified by the KVM_STATE_NESTED_EVMCS flag, even if
>> the VMXON region is not set. Thus, change the code to require saving
>> nested-state as part of migration stream in case it is set.
>> 
>> Reviewed-by: Karl Heubaum <address@hidden>
>> Signed-off-by: Liran Alon <address@hidden>
>> ---
>> target/i386/machine.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/target/i386/machine.c b/target/i386/machine.c
>> index 851b249d1a39..e7d72faf9e24 100644
>> --- a/target/i386/machine.c
>> +++ b/target/i386/machine.c
>> @@ -999,7 +999,7 @@ static bool vmx_nested_state_needed(void *opaque)
>> 
>>     return ((nested_state->format == KVM_STATE_NESTED_FORMAT_VMX) &&
>>             ((nested_state->hdr.vmx.vmxon_pa != -1ull) ||
>> -             (nested_state->hdr.vmx.smm.flags & 
>> KVM_STATE_NESTED_SMM_VMXON)));
>> +             (nested_state->flags & KVM_STATE_NESTED_EVMCS)));
>> }
>> 
>> static const VMStateDescription vmstate_vmx_nested_state = {
>> 
> 
> Queued, thanks.
> 
> Paolo

Actually Paolo after I have created KVM patch
("KVM: nVMX: Change KVM_STATE_NESTED_EVMCS to signal vmcs12 is copied from 
eVMCS”)
I think I realised that KVM_STATE_NESTED_EVMCS is actually not a requirement 
for nested-state to be sent.
I suggest to replace this commit with another one that just change 
vmx_nested_state_needed() to return true
In case format is FORMAT_VMX and vmxon_pa != -1ull and that’s it.

As anyway, QEMU provisioned on destination side is going to enable the relevant 
eVMCS capability.
I’m going to send another series that refines QEMU nested-migration a bit more 
so I will do it along the way.
But I think this patch should be un-queued. Sorry for realizing this later but 
at least it’s before it was merged to master :)

-Liran