On 16/08/21 14:54, Maxim Levitsky wrote:
Then on nested VM exit,
we would only restore the kvm's guest visible values (vcpu->arch.cr*/efer)
from that 'somewhere else', and could do this without any checks/etc, since these already passed all checks.
This needs to save these values in the migration stream as well of course.
Note that there could be differences between the guest-visible values
and the processor values of CRx. In particular, say you have a
hypervisor that uses PSE for its page tables. The CR4 would have
CR4.PSE=1 on a machine that uses NPT and CR4.PAE=1 on a machine that
doesn't.
Finally I propose that SVM nested state would be:
* L1 save area.
* L1 guest visible CR*/EFER/etc values (vcpu->arch.cr* values)
* Full VMCB12 (save and control area)
So your proposal would basically be to:
* do the equivalent of sync_vmcs02_to_vmcs12+sync_vmcs02_to_vmcs12_rare
on KVM_GET_NESTED_STATE
* discard the current state on KVM_SET_NESTED_STATE.
That does make sense. It wasn't done this way just because the "else"
branch of
if (is_guest_mode(vcpu)) {
sync_vmcs02_to_vmcs12(vcpu, vmcs12);
sync_vmcs02_to_vmcs12_rare(vcpu, vmcs12);
} else {
copy_vmcs02_to_vmcs12_rare(vcpu, get_vmcs12(vcpu));
if (!vmx->nested.need_vmcs12_to_shadow_sync) {
if (vmx->nested.hv_evmcs)
copy_enlightened_to_vmcs12(vmx);
else if (enable_shadow_vmcs)
copy_shadow_to_vmcs12(vmx);
}
}
isn't needed on SVM and thus it seemed "obvious" to remove the "then"
branch as well. I just focused on enter_svm_guest_mode when refactoring
the common bits of vmentry and KVM_SET_NESTED_STATE, so there is not
even a function like sync_vmcb02_to_vmcb12 (instead it's just done in
nested_svm_vmexit).
It does have some extra complications, for example with SREGS2 and the
always more complicated ordering of KVM_SET_* ioctls.
On the other hand, issues like not migrating PDPTRs were not specific to
nested SVM, and merely exposed by it. The ordering issues with
KVM_SET_VCPU_EVENTS and KVM_SET_MP_STATE's handling of INIT and SIPI are
also not specific to nested SVM. I am not sure that including the full
VMCB12 in the SVM nested state would solve any of these. Anything that
would be solved, probably would be just because migrating a large blob
is easier than juggling a dozen ioctls.
Thanks,
Paolo
