On Tue, 2021-01-12 at 15:00 -0500, Bandan Das wrote: > Sean Christopherson <sea...@google.com> writes: > ... > > > - if ((emulation_type & EMULTYPE_VMWARE_GP) && > > > - !is_vmware_backdoor_opcode(ctxt)) { > > > - kvm_queue_exception_e(vcpu, GP_VECTOR, 0); > > > - return 1; > > > + if (emulation_type & EMULTYPE_PARAVIRT_GP) { > > > + vminstr = is_vm_instr_opcode(ctxt); > > > + if (!vminstr && !is_vmware_backdoor_opcode(ctxt)) { > > > + kvm_queue_exception_e(vcpu, GP_VECTOR, 0); > > > + return 1; > > > + } > > > + if (vminstr) > > > + return vminstr; > > > > I'm pretty sure this doesn't correctly handle a VM-instr in L2 that hits a > > bad > > L0 GPA and that L1 wants to intercept. The intercept bitmap isn't checked > > until > > x86_emulate_insn(), and the vm*_interception() helpers expect nested > > VM-Exits to > > be handled further up the stack.
Actually IMHO this exactly what we want. We want L0 to always intercept these #GPs, and hide them from the guest. What we do need to do (and I prepared and attached a patch for that, is that if we run a guest, we want to inject corresponding vmexit (like SVM_EXIT_VMRUN) instead of emulating the instruction. The attached patch does this, and it made my kvm unit test pass, even if the test was run in a VM (with an unpatched kernel). This together with setting that X86_FEATURE_SVME_ADDR_CHK bit for the guest will allow us to hide that errata completely from the guest which is a very good thing. (for example for guests that we can't modify) Best regards, Maxim Levitsky > > > So, the condition is that L2 executes a vmload and #GPs on a reserved > address, jumps to L0 - L0 doesn't > check if L1 has asked for the instruction to be intercepted and goes on with > emulating > vmload and returning back to L2 ? > > > > } > > > > > > /* > > > -- > > > 2.27.0 > > >
commit 28ab89aaa11380306bafbf49265222f2a2da71da Author: Maxim Levitsky <mlevi...@redhat.com> Date: Thu Jan 14 10:53:25 2021 +0200 kvm: x86: fix that errata for nested guests diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c index c31e005252d69..9cfa5946fac69 100644 --- a/arch/x86/kvm/svm/svm.c +++ b/arch/x86/kvm/svm/svm.c @@ -2027,6 +2027,26 @@ static int svm_emulate_vm_instr(struct kvm_vcpu *vcpu, u8 modrm) { struct vcpu_svm *svm = to_svm(vcpu); + if (is_guest_mode(vcpu)) { + switch (modrm) { + case 0xd8: /* VMRUN */ + svm->vmcb->control.exit_code = SVM_EXIT_VMRUN; + break; + case 0xda: /* VMLOAD */ + svm->vmcb->control.exit_code = SVM_EXIT_VMLOAD; + break; + case 0xdb: /* VMSAVE */ + svm->vmcb->control.exit_code = SVM_EXIT_VMLOAD; + break; + default: + goto inject_exception; + } + + svm->vmcb->control.exit_info_1 = 0; + svm->vmcb->control.exit_info_2 = 0; + return nested_svm_vmexit(svm); + } + switch (modrm) { case 0xd8: /* VMRUN */ return vmrun_interception(svm); @@ -2035,6 +2055,7 @@ static int svm_emulate_vm_instr(struct kvm_vcpu *vcpu, u8 modrm) case 0xdb: /* VMSAVE */ return vmsave_interception(svm); default: +inject_exception: /* inject a #GP for all other cases */ kvm_queue_exception_e(vcpu, GP_VECTOR, 0); return 1;