Re: [PATCH 3/3] KVM: x86: fix use of L1 MMIO areas in nested guests

[Radim KrÄmÃÅ]

2017-08-18 09:59+0200, David Hildenbrand:
> On 17.08.2017 18:36, Paolo Bonzini wrote:
> > There is currently some confusion between nested and L1 GPAs.  The
> > assignment to "direct" in kvm_mmu_page_fault tries to fix that, but
> > it is not enough.  What this patch does is fence off the MMIO cache
> > completely when using shadow nested page tables, since we have neither
> > a GVA nor an L1 GPA to put in the cache.  This also allows some
> > simplifications in kvm_mmu_page_fault and FNAME(page_fault).
> > 
> > The EPT misconfig likewise does not have an L1 GPA to pass to
> > kvm_io_bus_write, so that must be skipped for guest mode.
> > 
> > Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> > ---
> > 	v1->v2: standardize on "nGPA" moniker, replace nested ifs with &&
> > 
> >  arch/x86/kvm/mmu.c         | 10 +++++++++-
> >  arch/x86/kvm/paging_tmpl.h |  3 +--
> >  arch/x86/kvm/vmx.c         |  7 ++++++-
> >  arch/x86/kvm/x86.h         |  6 +++++-
> >  4 files changed, 21 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> > index a2c592b14617..02f8c507b160 100644
> > --- a/arch/x86/kvm/mmu.c
> > +++ b/arch/x86/kvm/mmu.c
> > @@ -3596,6 +3596,14 @@ static bool is_shadow_zero_bits_set(struct kvm_mmu *mmu, u64 spte, int level)
> >  
> >  static bool mmio_info_in_cache(struct kvm_vcpu *vcpu, u64 addr, bool direct)
> >  {
> > +	/*
> > +	 * A nested guest cannot use the MMIO cache if it is using nested
> > +	 * page tables, because cr2 is a nGPA while the cache stores L1's
> > +	 * physical addresses.
> 
> ... "while the cache stores GPAs" ?

Makes sense, changed while applying.

> > diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
> > @@ -90,7 +90,11 @@ static inline u32 bit(int bitno)
> >  static inline void vcpu_cache_mmio_info(struct kvm_vcpu *vcpu,
> >  					gva_t gva, gfn_t gfn, unsigned access)
> >  {
> > -	vcpu->arch.mmio_gva = gva & PAGE_MASK;
> > +	/*
> > +	 * If this is a shadow nested page table, the "GVA" is
> 
> s/"GVA"/GVA/ ?

I prefer the former, we're talking about "gva_t gva" that isn't GVA. :)