Skip to contentSkip to conversation

KVM: MMU: optimize for set_spte

PATCH Applied Xiao Guangrong 3 participants 4 messages Dec 3, 2012 – Dec 6, 2012

Subsystems (3)
x86 KVM KVM x86
lore
Share & tools
lore

Xiao Guangrong

author
#0 lore raw

PATCH 1/1 KVM: MMU: optimize for set_spte

There are two cases we need to adjust page size in set_spte:
1): the one is other vcpu creates new sp in the window between mapping_level()
and acquiring mmu-lock.
2): the another case is the new sp is created by itself (page-fault path) when
guest uses the target gfn as its page table.

In current code, set_spte drop the spte and emulate the access for these case, it works not good:
- for the case 1, it may destroy the mapping established by other vcpu, and do expensive instruction emulation.
- for the case 2, it may emulate the access even if the guest is accessing the page which not used as page table. There is a example, 0~2M is used as huge page in guest, in this huge page, only page 3 used as page table, then guest read/writes on other pages can cause instruction emulation.

Both of these cases can be fixed by allowing guest to retry the access, it will refault, then we can establish the mapping by using small page

Signed-off-by Xiao Guangrong <[email protected]>
---
 arch/x86/kvm/mmu.c |   16 ++++++++++++----
 1 files changed, 12 insertions(+), 4 deletions(-)
arch/x86/kvm/mmu.c +12 -4
index b875a9e..01d7c2a 100644
@@ -2382,12 +2382,20 @@ static int set_spte(struct kvm_vcpu *vcpu, u64 *sptep,
 	    || (!vcpu->arch.mmu.direct_map && write_fault
 		&& !is_write_protection(vcpu) && !user_fault)) {
 
+		/*
+		 * There are two cases:
+		 * - the one is other vcpu creates new sp in the window
+		 *   between mapping_level() and acquiring mmu-lock.
+		 * - the another case is the new sp is created by itself
+		 *   (page-fault path) when guest uses the target gfn as
+		 *   its page table.
+		 * Both of these cases can be fixed by allowing guest to
+		 * retry the access, it will refault, then we can establish
+		 * the mapping by using small page.
+		 */
 		if (level > PT_PAGE_TABLE_LEVEL &&
-		    has_wrprotected_page(vcpu->kvm, gfn, level)) {
-			ret = 1;
-			drop_spte(vcpu->kvm, sptep);
+		    has_wrprotected_page(vcpu->kvm, gfn, level))
 			goto done;
-		}
 
 		spte |= PT_WRITABLE_MASK | SPTE_MMU_WRITEABLE;
Signature · 1 line
1.7.7.6
#1 lore raw
Quoted 54 lines · Xiao Guangrong

There are two cases we need to adjust page size in set_spte:
1): the one is other vcpu creates new sp in the window between mapping_level()
and acquiring mmu-lock.
2): the another case is the new sp is created by itself (page-fault path) when
guest uses the target gfn as its page table.

In current code, set_spte drop the spte and emulate the access for these case, it works not good:
- for the case 1, it may destroy the mapping established by other vcpu, and do expensive instruction emulation.
- for the case 2, it may emulate the access even if the guest is accessing the page which not used as page table. There is a example, 0~2M is used as huge page in guest, in this huge page, only page 3 used as page table, then guest read/writes on other pages can cause instruction emulation.

Both of these cases can be fixed by allowing guest to retry the access, it will refault, then we can establish the mapping by using small page

Signed-off-by Xiao Guangrong <[email protected]>
---
 arch/x86/kvm/mmu.c |   16 ++++++++++++----
 1 files changed, 12 insertions(+), 4 deletions(-)
arch/x86/kvm/mmu.c +12 -4
index b875a9e..01d7c2a 100644
@@ -2382,12 +2382,20 @@ static int set_spte(struct kvm_vcpu *vcpu, u64 *sptep,
 	    || (!vcpu->arch.mmu.direct_map && write_fault
 		&& !is_write_protection(vcpu) && !user_fault)) {
 
+		/*
+		 * There are two cases:
+		 * - the one is other vcpu creates new sp in the window
+		 *   between mapping_level() and acquiring mmu-lock.
+		 * - the another case is the new sp is created by itself
+		 *   (page-fault path) when guest uses the target gfn as
+		 *   its page table.
+		 * Both of these cases can be fixed by allowing guest to
+		 * retry the access, it will refault, then we can establish
+		 * the mapping by using small page.
+		 */
 		if (level > PT_PAGE_TABLE_LEVEL &&
-		    has_wrprotected_page(vcpu->kvm, gfn, level)) {
-			ret = 1;
-			drop_spte(vcpu->kvm, sptep);
+		    has_wrprotected_page(vcpu->kvm, gfn, level))
 			goto done;
-		}
 
 		spte |= PT_WRITABLE_MASK | SPTE_MMU_WRITEABLE;
Signature · 1 line
1.7.7.6

ACK.

Does it fix your testcase?

author
#2 lore raw
Quoted 51 lines · Marcelo Tosatti
Quoted 50 lines · Xiao Guangrong

There are two cases we need to adjust page size in set_spte:
1): the one is other vcpu creates new sp in the window between mapping_level()
and acquiring mmu-lock.
2): the another case is the new sp is created by itself (page-fault path) when
guest uses the target gfn as its page table.

In current code, set_spte drop the spte and emulate the access for these case, it works not good:
- for the case 1, it may destroy the mapping established by other vcpu, and do expensive instruction emulation.
- for the case 2, it may emulate the access even if the guest is accessing the page which not used as page table. There is a example, 0~2M is used as huge page in guest, in this huge page, only page 3 used as page table, then guest read/writes on other pages can cause instruction emulation.

Both of these cases can be fixed by allowing guest to retry the access, it will refault, then we can establish the mapping by using small page

Signed-off-by Xiao Guangrong <[email protected]>
---
 arch/x86/kvm/mmu.c |   16 ++++++++++++----
 1 files changed, 12 insertions(+), 4 deletions(-)
arch/x86/kvm/mmu.c +12 -4
index b875a9e..01d7c2a 100644
@@ -2382,12 +2382,20 @@ static int set_spte(struct kvm_vcpu *vcpu, u64 *sptep,
 	    || (!vcpu->arch.mmu.direct_map && write_fault
 		&& !is_write_protection(vcpu) && !user_fault)) {
 
+		/*
+		 * There are two cases:
+		 * - the one is other vcpu creates new sp in the window
+		 *   between mapping_level() and acquiring mmu-lock.
+		 * - the another case is the new sp is created by itself
+		 *   (page-fault path) when guest uses the target gfn as
+		 *   its page table.
+		 * Both of these cases can be fixed by allowing guest to
+		 * retry the access, it will refault, then we can establish
+		 * the mapping by using small page.
+		 */
 		if (level > PT_PAGE_TABLE_LEVEL &&
-		    has_wrprotected_page(vcpu->kvm, gfn, level)) {
-			ret = 1;
-			drop_spte(vcpu->kvm, sptep);
+		    has_wrprotected_page(vcpu->kvm, gfn, level))
 			goto done;
-		}
Quoted 8 lines
Quoted 4 lines
arch/x86/kvm/mmu.c +0 -0
 		spte |= PT_WRITABLE_MASK | SPTE_MMU_WRITEABLE;
Signature · 1 line
1.7.7.6

ACK.

Does it fix your testcase?

No.
I will post the new version patch of improving reexecute_instruction.

Applied
#3 lore raw
In reply to Xiao Guangrong
Quoted 15 lines · Xiao Guangrong

There are two cases we need to adjust page size in set_spte:
1): the one is other vcpu creates new sp in the window between mapping_level()
and acquiring mmu-lock.
2): the another case is the new sp is created by itself (page-fault path) when
guest uses the target gfn as its page table.

In current code, set_spte drop the spte and emulate the access for these case, it works not good:
- for the case 1, it may destroy the mapping established by other vcpu, and do expensive instruction emulation.
- for the case 2, it may emulate the access even if the guest is accessing the page which not used as page table. There is a example, 0~2M is used as huge page in guest, in this huge page, only page 3 used as page table, then guest read/writes on other pages can cause instruction emulation.

Quoted 4 lines

Both of these cases can be fixed by allowing guest to retry the access, it will refault, then we can establish the mapping by using small page

Signed-off-by Xiao Guangrong <[email protected]>

Applied to queue. Thanks.

Quoted 35 lines
---
 arch/x86/kvm/mmu.c |   16 ++++++++++++----
 1 files changed, 12 insertions(+), 4 deletions(-)
arch/x86/kvm/mmu.c +12 -4
index b875a9e..01d7c2a 100644
@@ -2382,12 +2382,20 @@ static int set_spte(struct kvm_vcpu *vcpu, u64 *sptep,
 	    || (!vcpu->arch.mmu.direct_map && write_fault
 		&& !is_write_protection(vcpu) && !user_fault)) {
 
+		/*
+		 * There are two cases:
+		 * - the one is other vcpu creates new sp in the window
+		 *   between mapping_level() and acquiring mmu-lock.
+		 * - the another case is the new sp is created by itself
+		 *   (page-fault path) when guest uses the target gfn as
+		 *   its page table.
+		 * Both of these cases can be fixed by allowing guest to
+		 * retry the access, it will refault, then we can establish
+		 * the mapping by using small page.
+		 */
 		if (level > PT_PAGE_TABLE_LEVEL &&
-		    has_wrprotected_page(vcpu->kvm, gfn, level)) {
-			ret = 1;
-			drop_spte(vcpu->kvm, sptep);
+		    has_wrprotected_page(vcpu->kvm, gfn, level))
 			goto done;
-		}
 
 		spte |= PT_WRITABLE_MASK | SPTE_MMU_WRITEABLE;
Signature · 1 line
1.7.7.6

--
Gleb.