On Thu, Aug 13, 2020 at 6:59 PM Sean Christopherson
<sean.j.christopherson@xxxxxxxxx> wrote:
>
> On Thu, Aug 13, 2020 at 03:44:08PM -0500, Oliver Upton wrote:
> > On Thu, Aug 13, 2020 at 12:03 PM Sean Christopherson
> > > > + *
> > > > + * Rather than synthesizing a VM-exit into L1 for every possible
> > > > + * instruction just flush the TLB, resume L2, and let hardware generate
> > > > + * the appropriate VM-exit.
> > > > + */
> > > > + vmx_flush_tlb_gva(vcpu, kvm_rip_read(vcpu));
> > >
> > > This is wrong, it should flush kvm_get_linear_rip(vcpu).
> > >
> >
> > I do not believe that the aim of this patch will work anymore, since:
> >
> > 1dbf5d68af6f ("KVM: VMX: Add guest physical address check in EPT
> > violation and misconfig")
> >
> > Since it is possible to get into the emulator on any instruction that
> > induces an EPT violation, we'd wind up looping when we believe the
> > instruction needs to exit to L1 (TLB flush, resume guest, hit the same
> > EPT violation. Rinse, wash, repeat).
>
> kvm_get_linear_rip() doesn't walk any page tables, it simply accounts for a
> non-zero CS.base when !64-bit mode. If we really wanted to, this could use
> the emulation context's cached _eip, but I don't see any value in that since
> both GUEST_CS_* and GUEST_RIP will already be cached by KVM.
>
> unsigned long kvm_get_linear_rip(struct kvm_vcpu *vcpu)
> {
> if (is_64_bit_mode(vcpu))
> return kvm_rip_read(vcpu);
> return (u32)(get_segment_base(vcpu, VCPU_SREG_CS) +
> kvm_rip_read(vcpu));
> }
Sorry, I was a tad imprecise. I haven't any issues with your
suggestion. Rather, I believe that my overall patch is ineffective.
Suppose we had an EPT violation for a GPA that exceeded the guest's
MAXPHYADDR. Let's also say that the EPT violation occurred on the
memory operand of an LMSW instruction. Per the aforementioned patch,
we will dive into the emulator. Since we check intercepts before
reading the operand out of memory, we will fall through to the default
case, set intercepted = true, flush TLB and resume.
>
>
>
> >
> > > > + return X86EMUL_RETRY_INSTR;
> > > > }
> > > >
> > > > #ifdef CONFIG_X86_64
> > > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > > > index 00c88c2f34e4..2ab47485100f 100644
> > > > --- a/arch/x86/kvm/x86.c
> > > > +++ b/arch/x86/kvm/x86.c
> > > > @@ -6967,7 +6967,7 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
> > > >
> > > > r = x86_emulate_insn(ctxt);
> > > >
> > > > - if (r == EMULATION_INTERCEPTED)
> > > > + if (r == EMULATION_INTERCEPTED || r == EMULATION_RETRY_INSTR)
> > > > return 1;
> > > >
> > > > if (r == EMULATION_FAILED) {
> > > > --
> > > > 2.27.0.290.gba653c62da-goog
> > > >
