diff mbox

KVM: MMU: Make cmpxchg_gpte aware of nesting too

Message ID 20110420133316.GA2192@amd.com (mailing list archive)
State New, archived
Headers show

Commit Message

Joerg Roedel April 20, 2011, 1:33 p.m. UTC 
On Wed, Apr 20, 2011 at 07:18:12AM -0400, Avi Kivity wrote:
> On 04/20/2011 02:06 PM, Roedel, Joerg wrote:

> > The cmpxchg_gpte function treats all table_gfns as l1-gfns. I'll send a
> > fix soon.
> 
> Thanks.

Here is a fix for review. I am out-of-office starting in nearly one hour
until next Tuesday. So the corrections will most likely not happen
before :)
The patch ist tested with npt and shadow paging as well as with
npt-on-npt (64 bit wit kvm).

Regards,

	Joerg

From 6b1dcd9f17bbd482061180001d1f45c3adcef430 Mon Sep 17 00:00:00 2001
From: Joerg Roedel <joerg.roedel@amd.com>
Date: Wed, 20 Apr 2011 15:22:21 +0200
Subject: [PATCH] KVM: MMU: Make cmpxchg_gpte aware of nesting too

This patch makes the cmpxchg_gpte() function aware of the

Comments

Takuya Yoshikawa April 21, 2011, 1:02 a.m. UTC | #1 
On Wed, 20 Apr 2011 15:33:16 +0200
"Roedel, Joerg" <Joerg.Roedel@amd.com> wrote:

> @@ -245,13 +257,17 @@ walk:
>  		goto error;
>  
>  	if (write_fault && !is_dirty_gpte(pte)) {
> -		bool ret;
> +		int ret;
>  
>  		trace_kvm_mmu_set_dirty_bit(table_gfn, index, sizeof(pte));
> -		ret = FNAME(cmpxchg_gpte)(vcpu->kvm, table_gfn, index, pte,
> +		ret = FNAME(cmpxchg_gpte)(vcpu, mmu, table_gfn, index, pte,
>  			    pte|PT_DIRTY_MASK);
> -		if (ret)
> +		if (ret < 0) {
> +			present = false;
> +			goto error;
> +		} if (ret)
>  			goto walk;

Preferably else if or another line ? :)

Takuya


> +
>  		mark_page_dirty(vcpu->kvm, table_gfn);
>  		pte |= PT_DIRTY_MASK;
>  		walker->ptes[walker->level - 1] = pte;
> -- 
> 1.7.1
> 

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Avi Kivity April 21, 2011, 8:11 a.m. UTC | #2 
On 04/21/2011 04:02 AM, Takuya Yoshikawa wrote:
> On Wed, 20 Apr 2011 15:33:16 +0200
> "Roedel, Joerg"<Joerg.Roedel@amd.com>  wrote:
>
> >  @@ -245,13 +257,17 @@ walk:
> >   		goto error;
> >
> >   	if (write_fault&&  !is_dirty_gpte(pte)) {
> >  -		bool ret;
> >  +		int ret;
> >
> >   		trace_kvm_mmu_set_dirty_bit(table_gfn, index, sizeof(pte));
> >  -		ret = FNAME(cmpxchg_gpte)(vcpu->kvm, table_gfn, index, pte,
> >  +		ret = FNAME(cmpxchg_gpte)(vcpu, mmu, table_gfn, index, pte,
> >   			pte|PT_DIRTY_MASK);
> >  -		if (ret)
> >  +		if (ret<  0) {
> >  +			present = false;
> >  +			goto error;
> >  +		} if (ret)
> >   			goto walk;
>
> Preferably else if or another line ? :)

Yes, I added an 'else' and applied.  Thanks.
diff mbox

Patch

difference between l1-gfns and l2-gfns when nested
virtualization is in use. This fixes a potential
data-corruption problem in the l1-guest and makes the code
work correct (at least as correct as the hardware which is
emulated in this code) again.

Cc: stable@kernel.org
Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kvm/paging_tmpl.h |   30 +++++++++++++++++++++++-------
 1 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
index 74f8567..e442bf4 100644
--- a/arch/x86/kvm/paging_tmpl.h
+++ b/arch/x86/kvm/paging_tmpl.h
@@ -78,15 +78,21 @@  static gfn_t gpte_to_gfn_lvl(pt_element_t gpte, int lvl)
 	return (gpte & PT_LVL_ADDR_MASK(lvl)) >> PAGE_SHIFT;
 }
 
-static bool FNAME(cmpxchg_gpte)(struct kvm *kvm,
+static int FNAME(cmpxchg_gpte)(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu,
 			 gfn_t table_gfn, unsigned index,
 			 pt_element_t orig_pte, pt_element_t new_pte)
 {
 	pt_element_t ret;
 	pt_element_t *table;
 	struct page *page;
+	gpa_t gpa;
 
-	page = gfn_to_page(kvm, table_gfn);
+	gpa = mmu->translate_gpa(vcpu, table_gfn << PAGE_SHIFT,
+				 PFERR_USER_MASK|PFERR_WRITE_MASK);
+	if (gpa == UNMAPPED_GVA)
+		return -EFAULT;
+
+	page = gfn_to_page(vcpu->kvm, gpa_to_gfn(gpa));
 
 	table = kmap_atomic(page, KM_USER0);
 	ret = CMPXCHG(&table[index], orig_pte, new_pte);
@@ -192,11 +198,17 @@  walk:
 #endif
 
 		if (!eperm && !rsvd_fault && !(pte & PT_ACCESSED_MASK)) {
+			int ret;
 			trace_kvm_mmu_set_accessed_bit(table_gfn, index,
 						       sizeof(pte));
-			if (FNAME(cmpxchg_gpte)(vcpu->kvm, table_gfn,
-			    index, pte, pte|PT_ACCESSED_MASK))
+			ret = FNAME(cmpxchg_gpte)(vcpu, mmu, table_gfn,
+					index, pte, pte|PT_ACCESSED_MASK);
+			if (ret < 0) {
+				present = false;
+				break;
+			} else if (ret)
 				goto walk;
+
 			mark_page_dirty(vcpu->kvm, table_gfn);
 			pte |= PT_ACCESSED_MASK;
 		}
@@ -245,13 +257,17 @@  walk:
 		goto error;
 
 	if (write_fault && !is_dirty_gpte(pte)) {
-		bool ret;
+		int ret;
 
 		trace_kvm_mmu_set_dirty_bit(table_gfn, index, sizeof(pte));
-		ret = FNAME(cmpxchg_gpte)(vcpu->kvm, table_gfn, index, pte,
+		ret = FNAME(cmpxchg_gpte)(vcpu, mmu, table_gfn, index, pte,
 			    pte|PT_DIRTY_MASK);
-		if (ret)
+		if (ret < 0) {
+			present = false;
+			goto error;
+		} if (ret)
 			goto walk;
+
 		mark_page_dirty(vcpu->kvm, table_gfn);
 		pte |= PT_DIRTY_MASK;
 		walker->ptes[walker->level - 1] = pte;