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;
