Messages in this thread | | | Date | Mon, 4 Apr 2022 19:45:52 +0200 | From | "Maciej S. Szmigiero" <> | Subject | Re: [PATCH 1/8] KVM: nSVM: Sync next_rip field from vmcb12 to vmcb02 |
| |
On 4.04.2022 19:21, Sean Christopherson wrote: > On Mon, Apr 04, 2022, Maciej S. Szmigiero wrote: >>> @@ -1606,7 +1622,7 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu, >>> nested_copy_vmcb_control_to_cache(svm, ctl); >>> svm_switch_vmcb(svm, &svm->nested.vmcb02); >>> - nested_vmcb02_prepare_control(svm); >>> + nested_vmcb02_prepare_control(svm, save->rip); >> >> ^ >> I guess this should be "svm->vmcb->save.rip", since >> KVM_{GET,SET}_NESTED_STATE "save" field contains vmcb01 data, >> not vmcb{0,1}2 (in contrast to the "control" field). > > Argh, yes. Is userspace required to set L2 guest state prior to KVM_SET_NESTED_STATE? > If not, this will result in garbage being loaded into vmcb02.
I'm not sure about particular KVM API guarantees, but looking at the code I guess it is supposed to handle both cases: 1) VMM loads the usual basic KVM state via KVM_SET_{S,}REGS then immediately issues KVM_SET_NESTED_STATE to load the remaining nested data.
Assuming that it was the L2 that was running at the save time, at first the basic L2 state will be loaded into vmcb01, then at KVM_SET_NESTED_STATE time: > if (is_guest_mode(vcpu)) > svm_leave_nested(vcpu); > else > svm->nested.vmcb02.ptr->save = svm->vmcb01.ptr->save;
The !is_guest_mode(vcpu) branch will be taken (since the new VM haven't entered the guest mode yet), which will copy the basic L2 state from vmcb01 to vmcb02 and then the remaining code will restore vmcb01 save and vmcb{0,1}2 control normally and then enter the guest mode.
2) VMM first issues KVM_SET_NESTED_STATE then immediately loads the basic state.
Sane as the above, only some initial VM state will be copied into vmcb02 from vmcb01 by the code mentioned above, then vmcb01 save and vmcb{0,1}2 control will be restored and guest mode will be entered. If the VMM then immediately issues KVM_SET_{S,}REGS then it will restore L2 basic state straight into vmcb02.
However, this all is my guess work from just looking at the relevant code, I haven't run any tests to make sure that I haven't missed something.
Thanks, Maciej
| |