[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] target/i386: kvm: Fix when nested state is need
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH] target/i386: kvm: Fix when nested state is needed for migration |
Date: |
Thu, 4 Jul 2019 17:29:01 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.2 |
On 04/07/19 16:31, Liran Alon wrote:
>
>
>> 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 :)
Replaced with
diff --git a/target/i386/machine.c b/target/i386/machine.c
index 851b249d1a..704ba6de46 100644
--- a/target/i386/machine.c
+++ b/target/i386/machine.c
@@ -997,9 +997,8 @@ static bool vmx_nested_state_needed(void *opaque)
{
struct kvm_nested_state *nested_state = 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)));
+ return (nested_state->format == KVM_STATE_NESTED_FORMAT_VMX &&
+ nested_state->hdr.vmx.vmxon_pa != -1ull);
}
static const VMStateDescription vmstate_vmx_nested_state = {
and dropped the last paragraph of the commit message.
Paolo