Re: [PATCH] KVM: MMU: Add shadow_accessed_shift

["Yang, Sheng" <sheng.yang@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 2992 bytes --]

On Sunday 31 August 2008 23:13:54 Avi Kivity wrote:
> yasker@gmail.com wrote:
> > From: Sheng Yang <sheng.yang@intel.com>
> >
> > We use a "fake" A/D bit for EPT, to keep epte behaviour consistent with
> > shadow spte. But it's not that good for MMU notifier. Now we can only
> > expect return young=0 for clean_flush_young() in most condition.
>
> Perhaps we are better off setting shadow_accessed_mask to 0 for ept, and
> adding a test for clear_flush_young()?  This is the only place that
> needs adjusting as far as I can tell.
>
> I don't see what having a fake accessed bit buys us, and I'd like the
> patch to be as small as possible, since it needs to go into
> 2.6.26-stable and 2.6.27-rc.

Though I still think fake accessed bit here makes logic consistent, here is 
the patch follow your comment. But I think it may not necessary for the 
2.6.26-stable?

----
From: Sheng Yang <sheng.yang@intel.com>
Date: Mon, 1 Sep 2008 13:22:09 +0800
Subject: [PATCH] KVM: VMX: Always return 0 for clear_flush_young() when using 
EPT

As well as discard fake accessed bit and dirty bit of EPT.

Signed-off-by: Sheng Yang <sheng.yang@intel.com>
---
 arch/x86/kvm/mmu.c |   15 +++++++++++----
 arch/x86/kvm/vmx.c |    3 +--
 arch/x86/kvm/vmx.h |    2 --
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index f33c594..e437985 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -716,10 +716,17 @@ static int kvm_age_rmapp(struct kvm *kvm, unsigned long 
*rmapp)
 		int _young;
 		u64 _spte = *spte;
 		BUG_ON(!(_spte & PT_PRESENT_MASK));
-		_young = _spte & PT_ACCESSED_MASK;
-		if (_young) {
-			young = 1;
-			clear_bit(PT_ACCESSED_SHIFT, (unsigned long *)spte);
+
+		/* always return old for EPT */
+		if (!shadow_accessed_mask)
+			_young = 0;
+		else {
+			_young = _spte & PT_ACCESSED_MASK;
+			if (_young) {
+				young = 1;
+				clear_bit(PT_ACCESSED_SHIFT,
+					  (unsigned long *)spte);
+			}
 		}
 		spte = rmap_next(kvm, rmapp, spte);
 	}
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 81c121c..d637897 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -3557,8 +3557,7 @@ static int __init vmx_init(void)
 		kvm_mmu_set_base_ptes(VMX_EPT_READABLE_MASK |
 			VMX_EPT_WRITABLE_MASK |
 			VMX_EPT_DEFAULT_MT << VMX_EPT_MT_EPTE_SHIFT);
-		kvm_mmu_set_mask_ptes(0ull, VMX_EPT_FAKE_ACCESSED_MASK,
-				VMX_EPT_FAKE_DIRTY_MASK, 0ull,
+		kvm_mmu_set_mask_ptes(0ull, 0ull, 0ull, 0ull,
 				VMX_EPT_EXECUTABLE_MASK);
 		kvm_enable_tdp();
 	} else
diff --git a/arch/x86/kvm/vmx.h b/arch/x86/kvm/vmx.h
index 0c22e5f..41e8c10 100644
--- a/arch/x86/kvm/vmx.h
+++ b/arch/x86/kvm/vmx.h
@@ -370,8 +370,6 @@ enum vmcs_field {
 #define VMX_EPT_READABLE_MASK			0x1ull
 #define VMX_EPT_WRITABLE_MASK			0x2ull
 #define VMX_EPT_EXECUTABLE_MASK			0x4ull
-#define VMX_EPT_FAKE_ACCESSED_MASK		(1ull << 62)
-#define VMX_EPT_FAKE_DIRTY_MASK			(1ull << 63)

 #define VMX_EPT_IDENTITY_PAGETABLE_ADDR		0xfffbc000ul

--
1.5.4.5




[-- Attachment #2: 0001-KVM-VMX-Always-return-0-for-clear_flush_young-wh.patch --]
[-- Type: text/x-diff, Size: 2261 bytes --]

From 23229946e717294091bf54cee704fb3b1cd4167d Mon Sep 17 00:00:00 2001
From: Sheng Yang <sheng.yang@intel.com>
Date: Mon, 1 Sep 2008 13:22:09 +0800
Subject: [PATCH] KVM: VMX: Always return 0 for clear_flush_young() when using EPT

As well as discard fake accessed bit and dirty bit of EPT.

Signed-off-by: Sheng Yang <sheng.yang@intel.com>
---
 arch/x86/kvm/mmu.c |   15 +++++++++++----
 arch/x86/kvm/vmx.c |    3 +--
 arch/x86/kvm/vmx.h |    2 --
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index f33c594..e437985 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -716,10 +716,17 @@ static int kvm_age_rmapp(struct kvm *kvm, unsigned long *rmapp)
 		int _young;
 		u64 _spte = *spte;
 		BUG_ON(!(_spte & PT_PRESENT_MASK));
-		_young = _spte & PT_ACCESSED_MASK;
-		if (_young) {
-			young = 1;
-			clear_bit(PT_ACCESSED_SHIFT, (unsigned long *)spte);
+
+		/* always return old for EPT */
+		if (!shadow_accessed_mask)
+			_young = 0;
+		else {
+			_young = _spte & PT_ACCESSED_MASK;
+			if (_young) {
+				young = 1;
+				clear_bit(PT_ACCESSED_SHIFT,
+					  (unsigned long *)spte);
+			}
 		}
 		spte = rmap_next(kvm, rmapp, spte);
 	}
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 81c121c..d637897 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -3557,8 +3557,7 @@ static int __init vmx_init(void)
 		kvm_mmu_set_base_ptes(VMX_EPT_READABLE_MASK |
 			VMX_EPT_WRITABLE_MASK |
 			VMX_EPT_DEFAULT_MT << VMX_EPT_MT_EPTE_SHIFT);
-		kvm_mmu_set_mask_ptes(0ull, VMX_EPT_FAKE_ACCESSED_MASK,
-				VMX_EPT_FAKE_DIRTY_MASK, 0ull,
+		kvm_mmu_set_mask_ptes(0ull, 0ull, 0ull, 0ull,
 				VMX_EPT_EXECUTABLE_MASK);
 		kvm_enable_tdp();
 	} else
diff --git a/arch/x86/kvm/vmx.h b/arch/x86/kvm/vmx.h
index 0c22e5f..41e8c10 100644
--- a/arch/x86/kvm/vmx.h
+++ b/arch/x86/kvm/vmx.h
@@ -370,8 +370,6 @@ enum vmcs_field {
 #define VMX_EPT_READABLE_MASK			0x1ull
 #define VMX_EPT_WRITABLE_MASK			0x2ull
 #define VMX_EPT_EXECUTABLE_MASK			0x4ull
-#define VMX_EPT_FAKE_ACCESSED_MASK		(1ull << 62)
-#define VMX_EPT_FAKE_DIRTY_MASK			(1ull << 63)
 
 #define VMX_EPT_IDENTITY_PAGETABLE_ADDR		0xfffbc000ul
 
-- 
1.5.4.5