All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] KVM: X86/MMU: Fix problem for shadowing 5-level NPT for 4-level NPT L1 guest
@ 2022-04-20 13:12 Lai Jiangshan
  2022-04-20 13:12 ` [PATCH 1/2] KVM: X86/MMU: Add sp_has_gptes() Lai Jiangshan
  2022-04-20 13:12 ` [PATCH 2/2] KVM: X86/MMU: Introduce role.passthrough for shadowing 5-level NPT for 4-level NPT L1 guest Lai Jiangshan
  0 siblings, 2 replies; 6+ messages in thread
From: Lai Jiangshan @ 2022-04-20 13:12 UTC (permalink / raw)
  To: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson; +Cc: Lai Jiangshan

From: Lai Jiangshan <jiangshan.ljs@antgroup.com>

When shadowing 5-level NPT for 4-level NPT L1 guest, the root_sp is
allocated with role.level = 5 and the guest pagetable's root gfn.

And root_sp->spt[0] is also allocated with the same gfn and the same
role except role.level = 4.  Luckily that they are different shadow
pages, but only root_sp->spt[0] is the real translation of the guest
pagetable.

Here comes a problem:

If the guest switches from gCR4_LA57=0 to gCR4_LA57=1 (or vice verse)
and uses the same gfn as the root page for nested NPT before and after
switching gCR4_LA57.  The host (hCR4_LA57=1) might use the same root_sp
for the guest even the guest switches gCR4_LA57.  The guest will see
unexpected page mapped and L2 may exploit the bug and hurt L1.  It is
lucky that the problem can't hurt L0.

Fix it by introducing role.passthrough.

Lai Jiangshan (2):
  KVM: X86/MMU: Add sp_has_gptes()
  KVM: X86/MMU: Introduce role.passthrough for shadowing 5-level NPT for
    4-level NPT L1 guest

 Documentation/virt/kvm/mmu.rst  |  3 +++
 arch/x86/include/asm/kvm_host.h |  5 +++--
 arch/x86/kvm/mmu/mmu.c          | 38 +++++++++++++++++++++++++++------
 arch/x86/kvm/mmu/paging_tmpl.h  |  1 +
 4 files changed, 38 insertions(+), 9 deletions(-)

-- 
2.19.1.6.gb485710b


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] KVM: X86/MMU: Add sp_has_gptes()
  2022-04-20 13:12 [PATCH 0/2] KVM: X86/MMU: Fix problem for shadowing 5-level NPT for 4-level NPT L1 guest Lai Jiangshan
@ 2022-04-20 13:12 ` Lai Jiangshan
  2022-04-20 20:48   ` Paolo Bonzini
  2022-04-20 13:12 ` [PATCH 2/2] KVM: X86/MMU: Introduce role.passthrough for shadowing 5-level NPT for 4-level NPT L1 guest Lai Jiangshan
  1 sibling, 1 reply; 6+ messages in thread
From: Lai Jiangshan @ 2022-04-20 13:12 UTC (permalink / raw)
  To: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson
  Cc: Lai Jiangshan, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin

From: Lai Jiangshan <jiangshan.ljs@antgroup.com>

Add sp_has_gptes() which equals to !sp->role.direct currently.

Shadow page having gptes needs to be write-protected, accounted and
responded to kvm_mmu_pte_write().

Use it in these places to replace !sp->role.direct and rename
for_each_gfn_indirect_valid_sp.

Signed-off-by: Lai Jiangshan <jiangshan.ljs@antgroup.com>
---
 arch/x86/kvm/mmu/mmu.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 1361eb4599b4..1bdff55218ef 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -1856,15 +1856,23 @@ static bool kvm_mmu_prepare_zap_page(struct kvm *kvm, struct kvm_mmu_page *sp,
 static void kvm_mmu_commit_zap_page(struct kvm *kvm,
 				    struct list_head *invalid_list);
 
+static bool sp_has_gptes(struct kvm_mmu_page *sp)
+{
+	if (sp->role.direct)
+		return false;
+
+	return true;
+}
+
 #define for_each_valid_sp(_kvm, _sp, _list)				\
 	hlist_for_each_entry(_sp, _list, hash_link)			\
 		if (is_obsolete_sp((_kvm), (_sp))) {			\
 		} else
 
-#define for_each_gfn_indirect_valid_sp(_kvm, _sp, _gfn)			\
+#define for_each_gfn_valid_sp_has_gptes(_kvm, _sp, _gfn)		\
 	for_each_valid_sp(_kvm, _sp,					\
 	  &(_kvm)->arch.mmu_page_hash[kvm_page_table_hashfn(_gfn)])	\
-		if ((_sp)->gfn != (_gfn) || (_sp)->role.direct) {} else
+		if ((_sp)->gfn != (_gfn) || !sp_has_gptes(_sp)) {} else
 
 static bool kvm_sync_page(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp,
 			 struct list_head *invalid_list)
@@ -2112,7 +2120,7 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu,
 	sp->gfn = gfn;
 	sp->role = role;
 	hlist_add_head(&sp->hash_link, sp_list);
-	if (!direct) {
+	if (sp_has_gptes(sp)) {
 		account_shadowed(vcpu->kvm, sp);
 		if (level == PG_LEVEL_4K && kvm_vcpu_write_protect_gfn(vcpu, gfn))
 			kvm_flush_remote_tlbs_with_address(vcpu->kvm, gfn, 1);
@@ -2321,7 +2329,7 @@ static bool __kvm_mmu_prepare_zap_page(struct kvm *kvm,
 	/* Zapping children means active_mmu_pages has become unstable. */
 	list_unstable = *nr_zapped;
 
-	if (!sp->role.invalid && !sp->role.direct)
+	if (!sp->role.invalid && sp_has_gptes(sp))
 		unaccount_shadowed(kvm, sp);
 
 	if (sp->unsync)
@@ -2501,7 +2509,7 @@ int kvm_mmu_unprotect_page(struct kvm *kvm, gfn_t gfn)
 	pgprintk("%s: looking for gfn %llx\n", __func__, gfn);
 	r = 0;
 	write_lock(&kvm->mmu_lock);
-	for_each_gfn_indirect_valid_sp(kvm, sp, gfn) {
+	for_each_gfn_valid_sp_has_gptes(kvm, sp, gfn) {
 		pgprintk("%s: gfn %llx role %x\n", __func__, gfn,
 			 sp->role.word);
 		r = 1;
@@ -2563,7 +2571,7 @@ int mmu_try_to_unsync_pages(struct kvm *kvm, const struct kvm_memory_slot *slot,
 	 * that case, KVM must complete emulation of the guest TLB flush before
 	 * allowing shadow pages to become unsync (writable by the guest).
 	 */
-	for_each_gfn_indirect_valid_sp(kvm, sp, gfn) {
+	for_each_gfn_valid_sp_has_gptes(kvm, sp, gfn) {
 		if (!can_unsync)
 			return -EPERM;
 
@@ -5311,7 +5319,7 @@ static void kvm_mmu_pte_write(struct kvm_vcpu *vcpu, gpa_t gpa,
 
 	++vcpu->kvm->stat.mmu_pte_write;
 
-	for_each_gfn_indirect_valid_sp(vcpu->kvm, sp, gfn) {
+	for_each_gfn_valid_sp_has_gptes(vcpu->kvm, sp, gfn) {
 		if (detect_write_misaligned(sp, gpa, bytes) ||
 		      detect_write_flooding(sp)) {
 			kvm_mmu_prepare_zap_page(vcpu->kvm, sp, &invalid_list);
-- 
2.19.1.6.gb485710b


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] KVM: X86/MMU: Introduce role.passthrough for shadowing 5-level NPT for 4-level NPT L1 guest
  2022-04-20 13:12 [PATCH 0/2] KVM: X86/MMU: Fix problem for shadowing 5-level NPT for 4-level NPT L1 guest Lai Jiangshan
  2022-04-20 13:12 ` [PATCH 1/2] KVM: X86/MMU: Add sp_has_gptes() Lai Jiangshan
@ 2022-04-20 13:12 ` Lai Jiangshan
  2022-04-20 17:15   ` Paolo Bonzini
  1 sibling, 1 reply; 6+ messages in thread
From: Lai Jiangshan @ 2022-04-20 13:12 UTC (permalink / raw)
  To: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson
  Cc: Lai Jiangshan, Jonathan Corbet, Vitaly Kuznetsov, Wanpeng Li,
	Jim Mattson, Joerg Roedel, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, linux-doc

From: Lai Jiangshan <jiangshan.ljs@antgroup.com>

When shadowing 5-level NPT for 4-level NPT L1 guest, the root_sp is
allocated with role.level = 5 and the guest pagetable's root gfn.

And root_sp->spt[0] is also allocated with the same gfn and the same
role except role.level = 4.  Luckily that they are different shadow
pages, but only root_sp->spt[0] is the real translation of the guest
pagetable.

Here comes a problem:

If the guest switches from gCR4_LA57=0 to gCR4_LA57=1 (or vice verse)
and uses the same gfn as the root page for nested NPT before and after
switching gCR4_LA57.  The host (hCR4_LA57=1) might use the same root_sp
for the guest even the guest switches gCR4_LA57.  The guest will see
unexpected page mapped and L2 may exploit the bug and hurt L1.  It is
lucky that the problem can't hurt L0.

And three special cases need to be handled:

The root_sp should be like role.direct=1 sometimes: its contents are
not backed by gptes, root_sp->gfns is meaningless.  (For a normal high
level sp in shadow paging, sp->gfns is often unused and kept zero, but
it could be relevant and meaningful if sp->gfns is used because they
are backed by concrete gptes.)

For such root_sp in the case, root_sp is just a portal to contribute
root_sp->spt[0], and root_sp->gfns should not be used and
root_sp->spt[0] should not be dropped if gpte[0] of the guest root
pagetable is changed.

Such root_sp should not be accounted too.

So add role.passthrough to distinguish the shadow pages in the hash
when gCR4_LA57 is toggled and fix above special cases by using it in
kvm_mmu_page_{get|set}_gfn() and sp_has_gptes().

Signed-off-by: Lai Jiangshan <jiangshan.ljs@antgroup.com>
---
 Documentation/virt/kvm/mmu.rst  |  3 +++
 arch/x86/include/asm/kvm_host.h |  5 +++--
 arch/x86/kvm/mmu/mmu.c          | 16 ++++++++++++++++
 arch/x86/kvm/mmu/paging_tmpl.h  |  1 +
 4 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/Documentation/virt/kvm/mmu.rst b/Documentation/virt/kvm/mmu.rst
index 5b1ebad24c77..4018b9d7a0d3 100644
--- a/Documentation/virt/kvm/mmu.rst
+++ b/Documentation/virt/kvm/mmu.rst
@@ -202,6 +202,9 @@ Shadow pages contain the following information:
     Is 1 if the MMU instance cannot use A/D bits.  EPT did not have A/D
     bits before Haswell; shadow EPT page tables also cannot use A/D bits
     if the L1 hypervisor does not enable them.
+  role.passthrough:
+    Is 1 if role.level = 5 when shadowing 5-level shadow NPT for
+    4-level NPT L1.
   gfn:
     Either the guest page table containing the translations shadowed by this
     page, or the base page frame for linear translations.  See role.direct.
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 9694dd5e6ccc..d4f8f4784d87 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -285,7 +285,7 @@ struct kvm_kernel_irq_routing_entry;
  * minimize the size of kvm_memory_slot.arch.gfn_track, i.e. allows allocating
  * 2 bytes per gfn instead of 4 bytes per gfn.
  *
- * Indirect upper-level shadow pages are tracked for write-protection via
+ * Upper-level shadow pages having gptes are tracked for write-protection via
  * gfn_track.  As above, gfn_track is a 16 bit counter, so KVM must not create
  * more than 2^16-1 upper-level shadow pages at a single gfn, otherwise
  * gfn_track will overflow and explosions will ensure.
@@ -331,7 +331,8 @@ union kvm_mmu_page_role {
 		unsigned smap_andnot_wp:1;
 		unsigned ad_disabled:1;
 		unsigned guest_mode:1;
-		unsigned :6;
+		unsigned passthrough:1;
+		unsigned :5;
 
 		/*
 		 * This is left at the top of the word so that
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 1bdff55218ef..d14cb6f99cb1 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -737,6 +737,9 @@ static void mmu_free_pte_list_desc(struct pte_list_desc *pte_list_desc)
 
 static gfn_t kvm_mmu_page_get_gfn(struct kvm_mmu_page *sp, int index)
 {
+	if (sp->role.passthrough)
+		return sp->gfn;
+
 	if (!sp->role.direct)
 		return sp->gfns[index];
 
@@ -745,6 +748,11 @@ static gfn_t kvm_mmu_page_get_gfn(struct kvm_mmu_page *sp, int index)
 
 static void kvm_mmu_page_set_gfn(struct kvm_mmu_page *sp, int index, gfn_t gfn)
 {
+	if (sp->role.passthrough) {
+		WARN_ON_ONCE(gfn != sp->gfn);
+		return;
+	}
+
 	if (!sp->role.direct) {
 		sp->gfns[index] = gfn;
 		return;
@@ -1861,6 +1869,9 @@ static bool sp_has_gptes(struct kvm_mmu_page *sp)
 	if (sp->role.direct)
 		return false;
 
+	if (sp->role.passthrough)
+		return false;
+
 	return true;
 }
 
@@ -2059,6 +2070,8 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu,
 		quadrant &= (1 << ((PT32_PT_BITS - PT64_PT_BITS) * level)) - 1;
 		role.quadrant = quadrant;
 	}
+	if (level <= vcpu->arch.mmu->root_level)
+		role.passthrough = 0;
 
 	sp_list = &vcpu->kvm->arch.mmu_page_hash[kvm_page_table_hashfn(gfn)];
 	for_each_valid_sp(vcpu->kvm, sp, sp_list) {
@@ -4890,6 +4903,9 @@ kvm_calc_shadow_npt_root_page_role(struct kvm_vcpu *vcpu,
 
 	role.base.direct = false;
 	role.base.level = kvm_mmu_get_tdp_level(vcpu);
+	if (role.base.level == PT64_ROOT_5LEVEL &&
+	    role_regs_to_root_level(regs) == PT64_ROOT_4LEVEL)
+		role.base.passthrough = 1;
 
 	return role;
 }
diff --git a/arch/x86/kvm/mmu/paging_tmpl.h b/arch/x86/kvm/mmu/paging_tmpl.h
index 8621188b46df..c1b975fb85a2 100644
--- a/arch/x86/kvm/mmu/paging_tmpl.h
+++ b/arch/x86/kvm/mmu/paging_tmpl.h
@@ -1042,6 +1042,7 @@ static int FNAME(sync_page)(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp)
 		.level = 0xf,
 		.access = 0x7,
 		.quadrant = 0x3,
+		.passthrough = 0x1,
 	};
 
 	/*
-- 
2.19.1.6.gb485710b


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] KVM: X86/MMU: Introduce role.passthrough for shadowing 5-level NPT for 4-level NPT L1 guest
  2022-04-20 13:12 ` [PATCH 2/2] KVM: X86/MMU: Introduce role.passthrough for shadowing 5-level NPT for 4-level NPT L1 guest Lai Jiangshan
@ 2022-04-20 17:15   ` Paolo Bonzini
  2022-04-21  0:23     ` Lai Jiangshan
  0 siblings, 1 reply; 6+ messages in thread
From: Paolo Bonzini @ 2022-04-20 17:15 UTC (permalink / raw)
  To: Lai Jiangshan, linux-kernel, kvm, Sean Christopherson
  Cc: Lai Jiangshan, Jonathan Corbet, Vitaly Kuznetsov, Wanpeng Li,
	Jim Mattson, Joerg Roedel, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, linux-doc

On 4/20/22 15:12, Lai Jiangshan wrote:
> From: Lai Jiangshan <jiangshan.ljs@antgroup.com>
> 
> When shadowing 5-level NPT for 4-level NPT L1 guest, the root_sp is
> allocated with role.level = 5 and the guest pagetable's root gfn.
> 
> And root_sp->spt[0] is also allocated with the same gfn and the same
> role except role.level = 4.  Luckily that they are different shadow
> pages, but only root_sp->spt[0] is the real translation of the guest
> pagetable.
> 
> Here comes a problem:
> 
> If the guest switches from gCR4_LA57=0 to gCR4_LA57=1 (or vice verse)
> and uses the same gfn as the root page for nested NPT before and after
> switching gCR4_LA57.  The host (hCR4_LA57=1) might use the same root_sp
> for the guest even the guest switches gCR4_LA57.  The guest will see
> unexpected page mapped and L2 may exploit the bug and hurt L1.  It is
> lucky that the problem can't hurt L0.
> 
> And three special cases need to be handled:
> 
> The root_sp should be like role.direct=1 sometimes: its contents are
> not backed by gptes, root_sp->gfns is meaningless.  (For a normal high
> level sp in shadow paging, sp->gfns is often unused and kept zero, but
> it could be relevant and meaningful if sp->gfns is used because they
> are backed by concrete gptes.)
> 
> For such root_sp in the case, root_sp is just a portal to contribute
> root_sp->spt[0], and root_sp->gfns should not be used and
> root_sp->spt[0] should not be dropped if gpte[0] of the guest root
> pagetable is changed.
> 
> Such root_sp should not be accounted too.
> 
> So add role.passthrough to distinguish the shadow pages in the hash
> when gCR4_LA57 is toggled and fix above special cases by using it in
> kvm_mmu_page_{get|set}_gfn() and sp_has_gptes().
> 
> Signed-off-by: Lai Jiangshan <jiangshan.ljs@antgroup.com>

Looks good, and is easy to backport; thanks.

However, once we have also your series "KVM: X86/MMU: Use one-off 
special shadow page for special roots", perhaps the passthrough sp can 
be converted to a special root (but now with a shadow_page) as well?

Paolo

> ---
>   Documentation/virt/kvm/mmu.rst  |  3 +++
>   arch/x86/include/asm/kvm_host.h |  5 +++--
>   arch/x86/kvm/mmu/mmu.c          | 16 ++++++++++++++++
>   arch/x86/kvm/mmu/paging_tmpl.h  |  1 +
>   4 files changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/virt/kvm/mmu.rst b/Documentation/virt/kvm/mmu.rst
> index 5b1ebad24c77..4018b9d7a0d3 100644
> --- a/Documentation/virt/kvm/mmu.rst
> +++ b/Documentation/virt/kvm/mmu.rst
> @@ -202,6 +202,9 @@ Shadow pages contain the following information:
>       Is 1 if the MMU instance cannot use A/D bits.  EPT did not have A/D
>       bits before Haswell; shadow EPT page tables also cannot use A/D bits
>       if the L1 hypervisor does not enable them.
> +  role.passthrough:
> +    Is 1 if role.level = 5 when shadowing 5-level shadow NPT for
> +    4-level NPT L1.
>     gfn:
>       Either the guest page table containing the translations shadowed by this
>       page, or the base page frame for linear translations.  See role.direct.
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 9694dd5e6ccc..d4f8f4784d87 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -285,7 +285,7 @@ struct kvm_kernel_irq_routing_entry;
>    * minimize the size of kvm_memory_slot.arch.gfn_track, i.e. allows allocating
>    * 2 bytes per gfn instead of 4 bytes per gfn.
>    *
> - * Indirect upper-level shadow pages are tracked for write-protection via
> + * Upper-level shadow pages having gptes are tracked for write-protection via
>    * gfn_track.  As above, gfn_track is a 16 bit counter, so KVM must not create
>    * more than 2^16-1 upper-level shadow pages at a single gfn, otherwise
>    * gfn_track will overflow and explosions will ensure.
> @@ -331,7 +331,8 @@ union kvm_mmu_page_role {
>   		unsigned smap_andnot_wp:1;
>   		unsigned ad_disabled:1;
>   		unsigned guest_mode:1;
> -		unsigned :6;
> +		unsigned passthrough:1;
> +		unsigned :5;
>   
>   		/*
>   		 * This is left at the top of the word so that
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 1bdff55218ef..d14cb6f99cb1 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -737,6 +737,9 @@ static void mmu_free_pte_list_desc(struct pte_list_desc *pte_list_desc)
>   
>   static gfn_t kvm_mmu_page_get_gfn(struct kvm_mmu_page *sp, int index)
>   {
> +	if (sp->role.passthrough)
> +		return sp->gfn;
> +
>   	if (!sp->role.direct)
>   		return sp->gfns[index];
>   
> @@ -745,6 +748,11 @@ static gfn_t kvm_mmu_page_get_gfn(struct kvm_mmu_page *sp, int index)
>   
>   static void kvm_mmu_page_set_gfn(struct kvm_mmu_page *sp, int index, gfn_t gfn)
>   {
> +	if (sp->role.passthrough) {
> +		WARN_ON_ONCE(gfn != sp->gfn);
> +		return;
> +	}
> +
>   	if (!sp->role.direct) {
>   		sp->gfns[index] = gfn;
>   		return;
> @@ -1861,6 +1869,9 @@ static bool sp_has_gptes(struct kvm_mmu_page *sp)
>   	if (sp->role.direct)
>   		return false;
>   
> +	if (sp->role.passthrough)
> +		return false;
> +
>   	return true;
>   }
>   
> @@ -2059,6 +2070,8 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu,
>   		quadrant &= (1 << ((PT32_PT_BITS - PT64_PT_BITS) * level)) - 1;
>   		role.quadrant = quadrant;
>   	}
> +	if (level <= vcpu->arch.mmu->root_level)
> +		role.passthrough = 0;
>   
>   	sp_list = &vcpu->kvm->arch.mmu_page_hash[kvm_page_table_hashfn(gfn)];
>   	for_each_valid_sp(vcpu->kvm, sp, sp_list) {
> @@ -4890,6 +4903,9 @@ kvm_calc_shadow_npt_root_page_role(struct kvm_vcpu *vcpu,
>   
>   	role.base.direct = false;
>   	role.base.level = kvm_mmu_get_tdp_level(vcpu);
> +	if (role.base.level == PT64_ROOT_5LEVEL &&
> +	    role_regs_to_root_level(regs) == PT64_ROOT_4LEVEL)
> +		role.base.passthrough = 1;
>   
>   	return role;
>   }
> diff --git a/arch/x86/kvm/mmu/paging_tmpl.h b/arch/x86/kvm/mmu/paging_tmpl.h
> index 8621188b46df..c1b975fb85a2 100644
> --- a/arch/x86/kvm/mmu/paging_tmpl.h
> +++ b/arch/x86/kvm/mmu/paging_tmpl.h
> @@ -1042,6 +1042,7 @@ static int FNAME(sync_page)(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp)
>   		.level = 0xf,
>   		.access = 0x7,
>   		.quadrant = 0x3,
> +		.passthrough = 0x1,
>   	};
>   
>   	/*


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] KVM: X86/MMU: Add sp_has_gptes()
  2022-04-20 13:12 ` [PATCH 1/2] KVM: X86/MMU: Add sp_has_gptes() Lai Jiangshan
@ 2022-04-20 20:48   ` Paolo Bonzini
  0 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2022-04-20 20:48 UTC (permalink / raw)
  To: Lai Jiangshan, linux-kernel, kvm, Sean Christopherson
  Cc: Lai Jiangshan, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H. Peter Anvin

On 4/20/22 15:12, Lai Jiangshan wrote:
> From: Lai Jiangshan <jiangshan.ljs@antgroup.com>
> 
> Add sp_has_gptes() which equals to !sp->role.direct currently.
> 
> Shadow page having gptes needs to be write-protected, accounted and
> responded to kvm_mmu_pte_write().
> 
> Use it in these places to replace !sp->role.direct and rename
> for_each_gfn_indirect_valid_sp.
> 
> -#define for_each_gfn_indirect_valid_sp(_kvm, _sp, _gfn)			\
> +#define for_each_gfn_valid_sp_has_gptes(_kvm, _sp, _gfn)		\

Small nit, for_each_gfn_valid_sp_with_gptes might be a little more 
grammatical (not much).

>   	for_each_valid_sp(_kvm, _sp,					\
>   	  &(_kvm)->arch.mmu_page_hash[kvm_page_table_hashfn(_gfn)])	\
> -		if ((_sp)->gfn != (_gfn) || (_sp)->role.direct) {} else
> +		if ((_sp)->gfn != (_gfn) || !sp_has_gptes(_sp)) {} else
>   
>   static bool kvm_sync_page(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp,
>   			 struct list_head *invalid_list)
> @@ -2112,7 +2120,7 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu,
>   	sp->gfn = gfn;
>   	sp->role = role;
>   	hlist_add_head(&sp->hash_link, sp_list);
> -	if (!direct) {
> +	if (sp_has_gptes(sp)) {
>   		account_shadowed(vcpu->kvm, sp);
>   		if (level == PG_LEVEL_4K && kvm_vcpu_write_protect_gfn(vcpu, gfn))
>   			kvm_flush_remote_tlbs_with_address(vcpu->kvm, gfn, 1);
> @@ -2321,7 +2329,7 @@ static bool __kvm_mmu_prepare_zap_page(struct kvm *kvm,
>   	/* Zapping children means active_mmu_pages has become unstable. */
>   	list_unstable = *nr_zapped;
>   
> -	if (!sp->role.invalid && !sp->role.direct)
> +	if (!sp->role.invalid && sp_has_gptes(sp))
>   		unaccount_shadowed(kvm, sp);
>   
>   	if (sp->unsync)
> @@ -2501,7 +2509,7 @@ int kvm_mmu_unprotect_page(struct kvm *kvm, gfn_t gfn)
>   	pgprintk("%s: looking for gfn %llx\n", __func__, gfn);
>   	r = 0;
>   	write_lock(&kvm->mmu_lock);
> -	for_each_gfn_indirect_valid_sp(kvm, sp, gfn) {
> +	for_each_gfn_valid_sp_has_gptes(kvm, sp, gfn) {
>   		pgprintk("%s: gfn %llx role %x\n", __func__, gfn,
>   			 sp->role.word);
>   		r = 1;
> @@ -2563,7 +2571,7 @@ int mmu_try_to_unsync_pages(struct kvm *kvm, const struct kvm_memory_slot *slot,
>   	 * that case, KVM must complete emulation of the guest TLB flush before
>   	 * allowing shadow pages to become unsync (writable by the guest).
>   	 */
> -	for_each_gfn_indirect_valid_sp(kvm, sp, gfn) {
> +	for_each_gfn_valid_sp_has_gptes(kvm, sp, gfn) {
>   		if (!can_unsync)
>   			return -EPERM;
>   
> @@ -5311,7 +5319,7 @@ static void kvm_mmu_pte_write(struct kvm_vcpu *vcpu, gpa_t gpa,
>   
>   	++vcpu->kvm->stat.mmu_pte_write;
>   
> -	for_each_gfn_indirect_valid_sp(vcpu->kvm, sp, gfn) {
> +	for_each_gfn_valid_sp_has_gptes(vcpu->kvm, sp, gfn) {
>   		if (detect_write_misaligned(sp, gpa, bytes) ||
>   		      detect_write_flooding(sp)) {
>   			kvm_mmu_prepare_zap_page(vcpu->kvm, sp, &invalid_list);


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] KVM: X86/MMU: Introduce role.passthrough for shadowing 5-level NPT for 4-level NPT L1 guest
  2022-04-20 17:15   ` Paolo Bonzini
@ 2022-04-21  0:23     ` Lai Jiangshan
  0 siblings, 0 replies; 6+ messages in thread
From: Lai Jiangshan @ 2022-04-21  0:23 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: LKML, kvm, Sean Christopherson, Lai Jiangshan, Jonathan Corbet,
	Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
	X86 ML, H. Peter Anvin, linux-doc

On Thu, Apr 21, 2022 at 1:15 AM Paolo Bonzini <pbonzini@redhat.com> wrote:

>
> Looks good, and is easy to backport; thanks.
>
> However, once we have also your series "KVM: X86/MMU: Use one-off
> special shadow page for special roots", perhaps the passthrough sp can
> be converted to a special root (but now with a shadow_page) as well?
>

It will be much more complicated.

The current code allows fast_pgd_switch() for shadowing 5-level NPT
for 4-level NPT L1 guests because both the host and the guest are
64 bit. Using a special shadow page will disallow fast_pgd_switch().

And it will make using_special_root_page() more complicated.

It will require a different kind of special shadow page to make it
able to do fast switch.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-04-21  0:23 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-20 13:12 [PATCH 0/2] KVM: X86/MMU: Fix problem for shadowing 5-level NPT for 4-level NPT L1 guest Lai Jiangshan
2022-04-20 13:12 ` [PATCH 1/2] KVM: X86/MMU: Add sp_has_gptes() Lai Jiangshan
2022-04-20 20:48   ` Paolo Bonzini
2022-04-20 13:12 ` [PATCH 2/2] KVM: X86/MMU: Introduce role.passthrough for shadowing 5-level NPT for 4-level NPT L1 guest Lai Jiangshan
2022-04-20 17:15   ` Paolo Bonzini
2022-04-21  0:23     ` Lai Jiangshan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.