[PATCH 0/6] KVM: Trivial cleanups

[PATCH 0/6] KVM: Trivial cleanups

[Lai Jiangshan]

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

A small collection of trivial cleanups.

Lai Jiangshan (6):
  KVM: X86/MMU: Remove unused macros from paging_tmpl.h
  KVM: X86/MMU: Remove unused PT32_DIR_BASE_ADDR_MASK from mmu.c
  KVM: X86/MMU: Update comments in paging_tmpl.h for the kinds of guest
    PTEs
  KVM: Rename ack_flush() to ack_kick()
  KVM: X86/MMU: Remove useless mmu_topup_memory_caches() in
    kvm_mmu_pte_write()
  KVM: X86/SVM: Use root_level in svm_load_mmu_pgd()

 arch/x86/kvm/mmu/mmu.c         |  9 ---------
 arch/x86/kvm/mmu/paging_tmpl.h | 16 ++--------------
 arch/x86/kvm/svm/svm.c         |  2 +-
 virt/kvm/kvm_main.c            |  4 ++--
 4 files changed, 5 insertions(+), 26 deletions(-)

-- 
2.19.1.6.gb485710b

[PATCH 1/6] KVM: X86/MMU: Remove unused macros from paging_tmpl.h

[Lai Jiangshan]

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

Remove unused PT_BASE_ADDR_MASK, PT_LEVEL_BITS, and CMPXCHG.

Signed-off-by: Lai Jiangshan <jiangshan.ljs@antgroup.com>
---
 arch/x86/kvm/mmu/paging_tmpl.h | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/arch/x86/kvm/mmu/paging_tmpl.h b/arch/x86/kvm/mmu/paging_tmpl.h
index db80f7ccaa4e..2375bd5fd9f4 100644
--- a/arch/x86/kvm/mmu/paging_tmpl.h
+++ b/arch/x86/kvm/mmu/paging_tmpl.h
@@ -24,17 +24,14 @@
 	#define pt_element_t u64
 	#define guest_walker guest_walker64
 	#define FNAME(name) paging##64_##name
-	#define PT_BASE_ADDR_MASK GUEST_PT64_BASE_ADDR_MASK
 	#define PT_LVL_ADDR_MASK(lvl) PT64_LVL_ADDR_MASK(lvl)
 	#define PT_LVL_OFFSET_MASK(lvl) PT64_LVL_OFFSET_MASK(lvl)
 	#define PT_INDEX(addr, level) PT64_INDEX(addr, level)
-	#define PT_LEVEL_BITS PT64_LEVEL_BITS
 	#define PT_GUEST_DIRTY_SHIFT PT_DIRTY_SHIFT
 	#define PT_GUEST_ACCESSED_SHIFT PT_ACCESSED_SHIFT
 	#define PT_HAVE_ACCESSED_DIRTY(mmu) true
 	#ifdef CONFIG_X86_64
 	#define PT_MAX_FULL_LEVELS PT64_ROOT_MAX_LEVEL
-	#define CMPXCHG "cmpxchgq"
 	#else
 	#define PT_MAX_FULL_LEVELS 2
 	#endif
@@ -42,30 +39,24 @@
 	#define pt_element_t u32
 	#define guest_walker guest_walker32
 	#define FNAME(name) paging##32_##name
-	#define PT_BASE_ADDR_MASK PT32_BASE_ADDR_MASK
 	#define PT_LVL_ADDR_MASK(lvl) PT32_LVL_ADDR_MASK(lvl)
 	#define PT_LVL_OFFSET_MASK(lvl) PT32_LVL_OFFSET_MASK(lvl)
 	#define PT_INDEX(addr, level) PT32_INDEX(addr, level)
-	#define PT_LEVEL_BITS PT32_LEVEL_BITS
 	#define PT_MAX_FULL_LEVELS 2
 	#define PT_GUEST_DIRTY_SHIFT PT_DIRTY_SHIFT
 	#define PT_GUEST_ACCESSED_SHIFT PT_ACCESSED_SHIFT
 	#define PT_HAVE_ACCESSED_DIRTY(mmu) true
-	#define CMPXCHG "cmpxchgl"
 #elif PTTYPE == PTTYPE_EPT
 	#define pt_element_t u64
 	#define guest_walker guest_walkerEPT
 	#define FNAME(name) ept_##name
-	#define PT_BASE_ADDR_MASK GUEST_PT64_BASE_ADDR_MASK
 	#define PT_LVL_ADDR_MASK(lvl) PT64_LVL_ADDR_MASK(lvl)
 	#define PT_LVL_OFFSET_MASK(lvl) PT64_LVL_OFFSET_MASK(lvl)
 	#define PT_INDEX(addr, level) PT64_INDEX(addr, level)
-	#define PT_LEVEL_BITS PT64_LEVEL_BITS
 	#define PT_GUEST_DIRTY_SHIFT 9
 	#define PT_GUEST_ACCESSED_SHIFT 8
 	#define PT_HAVE_ACCESSED_DIRTY(mmu) (!(mmu)->cpu_role.base.ad_disabled)
 	#ifdef CONFIG_X86_64
-	#define CMPXCHG "cmpxchgq"
 	#endif
 	#define PT_MAX_FULL_LEVELS PT64_ROOT_MAX_LEVEL
 #else
@@ -1076,15 +1067,12 @@ static int FNAME(sync_page)(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp)
 #undef pt_element_t
 #undef guest_walker
 #undef FNAME
-#undef PT_BASE_ADDR_MASK
 #undef PT_INDEX
 #undef PT_LVL_ADDR_MASK
 #undef PT_LVL_OFFSET_MASK
-#undef PT_LEVEL_BITS
 #undef PT_MAX_FULL_LEVELS
 #undef gpte_to_gfn
 #undef gpte_to_gfn_lvl
-#undef CMPXCHG
 #undef PT_GUEST_ACCESSED_MASK
 #undef PT_GUEST_DIRTY_MASK
 #undef PT_GUEST_DIRTY_SHIFT
-- 
2.19.1.6.gb485710b

[PATCH 2/6] KVM: X86/MMU: Remove unused PT32_DIR_BASE_ADDR_MASK from mmu.c

[Lai Jiangshan]

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

It is unused.

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

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index efe5a3dca1e0..c935fdfc2544 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -125,8 +125,6 @@ module_param(dbg, bool, 0644);
 
 
 #define PT32_BASE_ADDR_MASK PAGE_MASK
-#define PT32_DIR_BASE_ADDR_MASK \
-	(PAGE_MASK & ~((1ULL << (PAGE_SHIFT + PT32_LEVEL_BITS)) - 1))
 #define PT32_LVL_ADDR_MASK(level) \
 	(PAGE_MASK & ~((1ULL << (PAGE_SHIFT + (((level) - 1) \
 					    * PT32_LEVEL_BITS))) - 1))
-- 
2.19.1.6.gb485710b

[PATCH 3/6] KVM: X86/MMU: Update comments in paging_tmpl.h for the kinds of guest PTEs

[Lai Jiangshan]

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

Add EPT PTEs into the comments.

Signed-off-by: Lai Jiangshan <jiangshan.ljs@antgroup.com>
---
 arch/x86/kvm/mmu/paging_tmpl.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/mmu/paging_tmpl.h b/arch/x86/kvm/mmu/paging_tmpl.h
index 2375bd5fd9f4..f4e02ba04744 100644
--- a/arch/x86/kvm/mmu/paging_tmpl.h
+++ b/arch/x86/kvm/mmu/paging_tmpl.h
@@ -16,8 +16,8 @@
  */
 
 /*
- * We need the mmu code to access both 32-bit and 64-bit guest ptes,
- * so the code in this file is compiled twice, once per pte size.
+ * We need the MMU code to access 32-bit, 64-bit PTEs and 64-bit EPT PTEs,
+ * so the code in this file is compiled three times, once per a kind of PTE.
  */
 
 #if PTTYPE == 64
-- 
2.19.1.6.gb485710b

[PATCH 4/6] KVM: Rename ack_flush() to ack_kick()

[Lai Jiangshan]

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

Make it use the same verb as in kvm_kick_many_cpus().

Signed-off-by: Lai Jiangshan <jiangshan.ljs@antgroup.com>
---
 virt/kvm/kvm_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index e089db822c12..00c899393418 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -235,7 +235,7 @@ static bool kvm_request_needs_ipi(struct kvm_vcpu *vcpu, unsigned req)
 	return mode == IN_GUEST_MODE;
 }
 
-static void ack_flush(void *_completed)
+static void ack_kick(void *_completed)
 {
 }
 
@@ -244,7 +244,7 @@ static inline bool kvm_kick_many_cpus(struct cpumask *cpus, bool wait)
 	if (cpumask_empty(cpus))
 		return false;
 
-	smp_call_function_many(cpus, ack_flush, NULL, wait);
+	smp_call_function_many(cpus, ack_kick, NULL, wait);
 	return true;
 }
 
-- 
2.19.1.6.gb485710b

[PATCH 5/6] KVM: X86/MMU: Remove useless mmu_topup_memory_caches() in kvm_mmu_pte_write()

[Lai Jiangshan]

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

Since the commit c5e2184d1544("KVM: x86/mmu: Remove the defunct
update_pte() paging hook"), kvm_mmu_pte_write() no longer uses the rmap
cache.

So remove mmu_topup_memory_caches() in it.

Cc: Sean Christopherson <seanjc@google.com>
Signed-off-by: Lai Jiangshan <jiangshan.ljs@antgroup.com>
---
 arch/x86/kvm/mmu/mmu.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index c935fdfc2544..086f32dffdbe 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -5326,13 +5326,6 @@ static void kvm_mmu_pte_write(struct kvm_vcpu *vcpu, gpa_t gpa,
 
 	pgprintk("%s: gpa %llx bytes %d\n", __func__, gpa, bytes);
 
-	/*
-	 * No need to care whether allocation memory is successful
-	 * or not since pte prefetch is skipped if it does not have
-	 * enough objects in the cache.
-	 */
-	mmu_topup_memory_caches(vcpu, true);
-
 	write_lock(&vcpu->kvm->mmu_lock);
 
 	gentry = mmu_pte_write_fetch_gpte(vcpu, &gpa, &bytes);
-- 
2.19.1.6.gb485710b

[PATCH 6/6] KVM: X86/SVM: Use root_level in svm_load_mmu_pgd()

[Lai Jiangshan]

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

The caller always call it with root_level = vcpu->arch.mmu->root_role.level.

Signed-off-by: Lai Jiangshan <jiangshan.ljs@antgroup.com>
---
 arch/x86/kvm/svm/svm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 3b49337998ec..f45d11739314 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -3951,7 +3951,7 @@ static void svm_load_mmu_pgd(struct kvm_vcpu *vcpu, hpa_t root_hpa,
 		hv_track_root_tdp(vcpu, root_hpa);
 
 		cr3 = vcpu->arch.cr3;
-	} else if (vcpu->arch.mmu->root_role.level >= PT64_ROOT_4LEVEL) {
+	} else if (root_level >= PT64_ROOT_4LEVEL) {
 		cr3 = __sme_set(root_hpa) | kvm_get_active_pcid(vcpu);
 	} else {
 		/* PCID in the guest should be impossible with a 32-bit MMU. */
-- 
2.19.1.6.gb485710b

Re: [PATCH 6/6] KVM: X86/SVM: Use root_level in svm_load_mmu_pgd()

[David Matlack]

On Sun, Jun 05, 2022 at 02:34:17PM +0800, Lai Jiangshan wrote:
> From: Lai Jiangshan <jiangshan.ljs@antgroup.com>
> 
> The caller always call it with root_level = vcpu->arch.mmu->root_role.level.

It'd be helpful to be more specific about the caller. e.g.

  Use root_level in svm_load_mmu_pg() rather that looking up the root
  level in vcpu->arch.mmu->root_role.level. svm_load_mmu_pgd() has only
  one caller, kvm_mmu_load_pgd(), which always passes
  vcpu->arch.mmu->root_role.level as root_level.

  No functional change intended.

> 
> Signed-off-by: Lai Jiangshan <jiangshan.ljs@antgroup.com>
> ---
>  arch/x86/kvm/svm/svm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 3b49337998ec..f45d11739314 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -3951,7 +3951,7 @@ static void svm_load_mmu_pgd(struct kvm_vcpu *vcpu, hpa_t root_hpa,
>  		hv_track_root_tdp(vcpu, root_hpa);
>  
>  		cr3 = vcpu->arch.cr3;
> -	} else if (vcpu->arch.mmu->root_role.level >= PT64_ROOT_4LEVEL) {
> +	} else if (root_level >= PT64_ROOT_4LEVEL) {
>  		cr3 = __sme_set(root_hpa) | kvm_get_active_pcid(vcpu);
>  	} else {
>  		/* PCID in the guest should be impossible with a 32-bit MMU. */
> -- 
> 2.19.1.6.gb485710b
> 

Re: [PATCH 0/6] KVM: Trivial cleanups

[David Matlack]

On Sun, Jun 05, 2022 at 02:34:11PM +0800, Lai Jiangshan wrote:
> From: Lai Jiangshan <jiangshan.ljs@antgroup.com>
> 
> A small collection of trivial cleanups.

Nice cleanups. My only feedback is on the commit messages, which are a
bit terse. Here's what I would recommend:

 - Explain what the commit does in the first sentence/paragraph of the
   commit message and then explain why/background info.

 - Include "No functional change intended." for commits that are
   expected to be no-ops. It's pretty obvious for most of these changes
   but it's still nice that have to convey your intent.

Commit messages aside:

Reviewed-by: David Matlack <dmatlack@google.com>

> 
> Lai Jiangshan (6):
>   KVM: X86/MMU: Remove unused macros from paging_tmpl.h
>   KVM: X86/MMU: Remove unused PT32_DIR_BASE_ADDR_MASK from mmu.c
>   KVM: X86/MMU: Update comments in paging_tmpl.h for the kinds of guest
>     PTEs
>   KVM: Rename ack_flush() to ack_kick()
>   KVM: X86/MMU: Remove useless mmu_topup_memory_caches() in
>     kvm_mmu_pte_write()
>   KVM: X86/SVM: Use root_level in svm_load_mmu_pgd()
> 
>  arch/x86/kvm/mmu/mmu.c         |  9 ---------
>  arch/x86/kvm/mmu/paging_tmpl.h | 16 ++--------------
>  arch/x86/kvm/svm/svm.c         |  2 +-
>  virt/kvm/kvm_main.c            |  4 ++--
>  4 files changed, 5 insertions(+), 26 deletions(-)
> 
> -- 
> 2.19.1.6.gb485710b
> 

Re: [PATCH 0/6] KVM: Trivial cleanups

[Paolo Bonzini]

Queued patches 2 and 4-6, thanks.

I also applied David's rewarded commit mesage for patch 6.

Paolo