>On 14/09/2018 09:16, Paolo Bonzini wrote:
>Heh, I was going to send a similar patch. However, things are a bit
>more complex for two reason.
>
>First, I'd prefer to reuse the hflags and hflags2 fields that we already
>have, and only store the VMCS blob in the subsection. For example,
>HF_SVMI_MASK is really the same as HF_GUEST_MASK in KVM source code and
>KVM_STATE_NESTED_GUEST_MODE in the nested virt state.
>
Do you mean you intend to only save/restore the “vmx” field in struct kvm_nested_state?
(That is, struct kvm_vmx_nested_state)
If yes, that is problematic as kvm_nested_state.flags also hold other flags besides KVM_STATE_NESTED_GUEST_MODE.
How do you expect to save/restore for example the vmx->nested.nested_run_pending flag that is specified in KVM_STATE_NESTED_RUN_PENDING?
In addition, why is it important to avoid save/restore the entire kvm_nested_state struct?
It seems to simplify things to just save/restore the entire struct.
>More important, this:
>
>>
>> +static int nested_state_post_load(void *opaque, int version_id)
>> +{
>> + X86CPU *cpu = opaque;
>> + CPUX86State *env = &cpu->env;
>> +
>> + /*
>> + * Verify that the size specified in given struct is set
>> + * to no more than the size that our kernel support
>> + */
>> + if (env->nested_state->size > env->nested_state_len) {
>> + return -EINVAL;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static bool nested_state_needed(void *opaque)
>
>doesn't work if nested_state_len differs between source and destination,
>and could overflow the nested_state buffer if nested_state_len is larger
>on the source.
This is not accurate.
I have actually given a lot of thought to this aspect in the patch.
The above post_load() method actually prevents an overflow to happen on dest.
Note that env->nested_state_len is not passed as part of migration stream.
It is only set by local kernel KVM_CAP_NESTED_STATE.
Therefore, we allow the following migrations to execute successfully:
1) Migration from a source with smaller KVM_CAP_NESTED_STATE to dest with a bigger one.
The above post_load() check will succeed as size specified in migrated nested_state->size
is smaller than dest KVM_CAP_NESTED_STATE (stored in env->nested_state_len).
2) Migration from source to dest when they both have same KVM_CAP_NESTED_STATE size.
Obvious.
3) Migration from source to dest when source have a bigger KVM_CAP_NESTED_STATE than dest.
This will only succeed in case size specified in nested_state->size is smaller than dest KVM_CAP_NESTED_STATE.
-Liran
>
>I'll send my version today or next Monday.
>
>Thanks,
>
>Paolo
>
