Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1714195 > unrolled thread

[PATCH v2 0/3] KVM: MMU: pending MMU and nEPT patches

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/3] KVM: MMU: pending MMU and nEPT patches Paolo Bonzini <pbonzini@redhat.com> - 2017-08-17 18:40 +0200
    [PATCH 3/3] KVM: x86: fix use of L1 MMIO areas in nested guests Paolo Bonzini <pbonzini@redhat.com> - 2017-08-17 18:40 +0200
      Re: [PATCH 3/3] KVM: x86: fix use of L1 MMIO areas in nested guests David Hildenbrand <david@redhat.com> - 2017-08-18 10:10 +0200
        Re: [PATCH 3/3] KVM: x86: fix use of L1 MMIO areas in nested guests Radim Krčmář <rkrcmar@redhat.com> - 2017-08-18 14:40 +0200
          Re: [PATCH 3/3] KVM: x86: fix use of L1 MMIO areas in nested guests Paolo Bonzini <pbonzini@redhat.com> - 2017-08-18 14:40 +0200

#1714195 — [PATCH v2 0/3] KVM: MMU: pending MMU and nEPT patches

v2 has all the suggestions from David.

Paolo

Brijesh Singh (1):
  KVM: x86: Avoid guest page table walk when gpa_available is set

Paolo Bonzini (3):
  KVM: x86: simplify ept_misconfig
  KVM: x86: fix use of L1 MMIO areas in nested guests
  KVM: x86: fix PML in nested guests

 arch/x86/include/asm/kvm_host.h |  3 ++-
 arch/x86/kvm/mmu.c              | 35 +++++++++++++++++++++++++++++++++--
 arch/x86/kvm/mmu.h              | 17 -----------------
 arch/x86/kvm/paging_tmpl.h      |  3 +--
 arch/x86/kvm/svm.c              |  2 --
 arch/x86/kvm/vmx.c              | 28 ++++++++++++----------------
 arch/x86/kvm/x86.c              | 20 +++++++-------------
 arch/x86/kvm/x86.h              |  6 +++++-
 8 files changed, 60 insertions(+), 54 deletions(-)

-- 
1.8.3.1

[toc] | [next] | [standalone]

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

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@redhat.com>
---
	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.
+	 */
+	if (mmu_is_nested(vcpu))
+		return false;
+
 	if (direct)
 		return vcpu_match_mmio_gpa(vcpu, addr);
 
@@ -4841,7 +4849,7 @@ int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t cr2, u64 error_code,
 {
 	int r, emulation_type = EMULTYPE_RETRY;
 	enum emulation_result er;
-	bool direct = vcpu->arch.mmu.direct_map || mmu_is_nested(vcpu);
+	bool direct = vcpu->arch.mmu.direct_map;
 
 	/* With shadow page tables, fault_address contains a GVA or nGPA.  */
 	if (vcpu->arch.mmu.direct_map) {
diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
index 3bb90ceeb52d..86b68dc5a649 100644
--- a/arch/x86/kvm/paging_tmpl.h
+++ b/arch/x86/kvm/paging_tmpl.h
@@ -790,8 +790,7 @@ static int FNAME(page_fault)(struct kvm_vcpu *vcpu, gva_t addr, u32 error_code,
 			 &map_writable))
 		return 0;
 
-	if (handle_abnormal_pfn(vcpu, mmu_is_nested(vcpu) ? 0 : addr,
-				walker.gfn, pfn, walker.pte_access, &r))
+	if (handle_abnormal_pfn(vcpu, addr, walker.gfn, pfn, walker.pte_access, &r))
 		return r;
 
 	/*
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index e2c8b33c35d1..61389ad784e4 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -6402,8 +6402,13 @@ static int handle_ept_misconfig(struct kvm_vcpu *vcpu)
 	int ret;
 	gpa_t gpa;
 
+	/*
+	 * A nested guest cannot optimize MMIO vmexits, because we have an
+	 * nGPA here instead of the required GPA.
+	 */
 	gpa = vmcs_read64(GUEST_PHYSICAL_ADDRESS);
-	if (!kvm_io_bus_write(vcpu, KVM_FAST_MMIO_BUS, gpa, 0, NULL)) {
+	if (!is_guest_mode(vcpu) &&
+	    !kvm_io_bus_write(vcpu, KVM_FAST_MMIO_BUS, gpa, 0, NULL)) {
 		trace_kvm_fast_mmio(gpa);
 		return kvm_skip_emulated_instruction(vcpu);
 	}
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index 612067074905..113460370a7f 100644
--- 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
+	 * actually a nGPA.
+	 */
+	vcpu->arch.mmio_gva = mmu_is_nested(vcpu) ? 0 : gva & PAGE_MASK;
 	vcpu->arch.access = access;
 	vcpu->arch.mmio_gfn = gfn;
 	vcpu->arch.mmio_gen = kvm_memslots(vcpu->kvm)->generation;
-- 
1.8.3.1

[toc] | [prev] | [next] | [standalone]

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

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@redhat.com>
> ---
> 	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" ?

> +	 */
> +	if (mmu_is_nested(vcpu))
> +		return false;
> +
>  	if (direct)
>  		return vcpu_match_mmio_gpa(vcpu, addr);
>  
> @@ -4841,7 +4849,7 @@ int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t cr2, u64 error_code,
>  {
>  	int r, emulation_type = EMULTYPE_RETRY;
>  	enum emulation_result er;
> -	bool direct = vcpu->arch.mmu.direct_map || mmu_is_nested(vcpu);
> +	bool direct = vcpu->arch.mmu.direct_map;
>  
>  	/* With shadow page tables, fault_address contains a GVA or nGPA.  */
>  	if (vcpu->arch.mmu.direct_map) {
> diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
> index 3bb90ceeb52d..86b68dc5a649 100644
> --- a/arch/x86/kvm/paging_tmpl.h
> +++ b/arch/x86/kvm/paging_tmpl.h
> @@ -790,8 +790,7 @@ static int FNAME(page_fault)(struct kvm_vcpu *vcpu, gva_t addr, u32 error_code,
>  			 &map_writable))
>  		return 0;
>  
> -	if (handle_abnormal_pfn(vcpu, mmu_is_nested(vcpu) ? 0 : addr,
> -				walker.gfn, pfn, walker.pte_access, &r))
> +	if (handle_abnormal_pfn(vcpu, addr, walker.gfn, pfn, walker.pte_access, &r))
>  		return r;
>  
>  	/*
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index e2c8b33c35d1..61389ad784e4 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -6402,8 +6402,13 @@ static int handle_ept_misconfig(struct kvm_vcpu *vcpu)
>  	int ret;
>  	gpa_t gpa;
>  
> +	/*
> +	 * A nested guest cannot optimize MMIO vmexits, because we have an
> +	 * nGPA here instead of the required GPA.
> +	 */
>  	gpa = vmcs_read64(GUEST_PHYSICAL_ADDRESS);
> -	if (!kvm_io_bus_write(vcpu, KVM_FAST_MMIO_BUS, gpa, 0, NULL)) {
> +	if (!is_guest_mode(vcpu) &&
> +	    !kvm_io_bus_write(vcpu, KVM_FAST_MMIO_BUS, gpa, 0, NULL)) {
>  		trace_kvm_fast_mmio(gpa);
>  		return kvm_skip_emulated_instruction(vcpu);
>  	}
> diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
> index 612067074905..113460370a7f 100644
> --- 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/ ?

> +	 * actually a nGPA.
> +	 */
> +	vcpu->arch.mmio_gva = mmu_is_nested(vcpu) ? 0 : gva & PAGE_MASK;
>  	vcpu->arch.access = access;
>  	vcpu->arch.mmio_gfn = gfn;
>  	vcpu->arch.mmio_gen = kvm_memslots(vcpu->kvm)->generation;
> 

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 

Thanks,

David

[toc] | [prev] | [next] | [standalone]

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

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@redhat.com>
> > ---
> > 	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. :)

[toc] | [prev] | [next] | [standalone]

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

On 18/08/2017 14:35, Radim Krčmář wrote:
> 
>>> 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. :)

Exactly. :)

Paolo

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web