[PATCH 0/3] KVM: x86/mmu: kvm_mmu_page cleanups

[PATCH 0/3] KVM: x86/mmu: kvm_mmu_page cleanups

[Sean Christopherson]

Patch 1 is from Jia He to remove a defunct boolean from kvm_mmu_page
(link[*] below if you want to take it directly).

Patch 2 builds on that patch to micro-optimize the TDP MMU flag.

Patch 3 is another micro-optimization that probably doesn't buy much
performance (I didn't check), feel free to ignore it.

[*] https://lkml.kernel.org/r/20210830145336.27183-1-justin.he@arm.com

Jia He (1):
  KVM: x86/mmu: Remove unused field mmio_cached in struct kvm_mmu_page

Sean Christopherson (2):
  KVM: x86/mmu: Relocate kvm_mmu_page.tdp_mmu_page for better cache
    locality
  KVM: x86/mmu: Move lpage_disallowed_link further "down" in
    kvm_mmu_page

 arch/x86/kvm/mmu/mmu_internal.h | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

-- 
2.33.0.153.gba50c8fa24-goog

[PATCH 1/3] KVM: x86/mmu: Remove unused field mmio_cached in struct kvm_mmu_page

[Sean Christopherson]

From: Jia He <justin.he@arm.com>

After reverting and restoring the fast tlb invalidation patch series,
the mmio_cached is not removed. Hence a unused field is left in
kvm_mmu_page.

Reviewed-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Jia He <justin.he@arm.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/mmu/mmu_internal.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/x86/kvm/mmu/mmu_internal.h b/arch/x86/kvm/mmu/mmu_internal.h
index 658d8d228d43..9bfa46b35201 100644
--- a/arch/x86/kvm/mmu/mmu_internal.h
+++ b/arch/x86/kvm/mmu/mmu_internal.h
@@ -37,7 +37,6 @@ struct kvm_mmu_page {
 
 	bool unsync;
 	u8 mmu_valid_gen;
-	bool mmio_cached;
 	bool lpage_disallowed; /* Can't be replaced by an equiv large page */
 
 	/*
-- 
2.33.0.153.gba50c8fa24-goog

[PATCH 2/3] KVM: x86/mmu: Relocate kvm_mmu_page.tdp_mmu_page for better cache locality

[Sean Christopherson]

Move "tdp_mmu_page" into the 1-byte void left by the recently removed
"mmio_cached" so that it resides in the first 64 bytes of kvm_mmu_page,
i.e. in the same cache line as the most commonly accessed fields.

Don't bother wrapping tdp_mmu_page in CONFIG_X86_64, including the field in
32-bit builds doesn't affect the size of kvm_mmu_page, and a future patch
can always wrap the field in the unlikely event KVM gains a 1-byte flag
that is 32-bit specific.

Note, the size of kvm_mmu_page is also unchanged on CONFIG_X86_64=y due
to it previously sharing an 8-byte chunk with write_flooding_count.

No functional change intended.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/mmu/mmu_internal.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/x86/kvm/mmu/mmu_internal.h b/arch/x86/kvm/mmu/mmu_internal.h
index 9bfa46b35201..4b06e6040e90 100644
--- a/arch/x86/kvm/mmu/mmu_internal.h
+++ b/arch/x86/kvm/mmu/mmu_internal.h
@@ -35,6 +35,7 @@ struct kvm_mmu_page {
 	struct hlist_node hash_link;
 	struct list_head lpage_disallowed_link;
 
+	bool tdp_mmu_page;
 	bool unsync;
 	u8 mmu_valid_gen;
 	bool lpage_disallowed; /* Can't be replaced by an equiv large page */
@@ -70,8 +71,6 @@ struct kvm_mmu_page {
 	atomic_t write_flooding_count;
 
 #ifdef CONFIG_X86_64
-	bool tdp_mmu_page;
-
 	/* Used for freeing the page asynchronously if it is a TDP MMU page. */
 	struct rcu_head rcu_head;
 #endif
-- 
2.33.0.153.gba50c8fa24-goog

[PATCH 3/3] KVM: x86/mmu: Move lpage_disallowed_link further "down" in kvm_mmu_page

[Sean Christopherson]

Move "lpage_disallowed_link" out of the first 64 bytes, i.e. out of the
first cache line, of kvm_mmu_page so that "spt" and to a lesser extent
"gfns" land in the first cache line.  "lpage_disallowed_link" is accessed
relatively infrequently compared to "spt", which is accessed any time KVM
is walking and/or manipulating the shadow page tables.

No functional change intended.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/mmu/mmu_internal.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/mmu/mmu_internal.h b/arch/x86/kvm/mmu/mmu_internal.h
index 4b06e6040e90..cae932afc8d3 100644
--- a/arch/x86/kvm/mmu/mmu_internal.h
+++ b/arch/x86/kvm/mmu/mmu_internal.h
@@ -31,9 +31,12 @@ extern bool dbg;
 #define IS_VALID_PAE_ROOT(x)	(!!(x))
 
 struct kvm_mmu_page {
+	/*
+	 * Note, "link" through "spt" fit in a single 64 byte cache line on
+	 * 64-bit kernels, keep it that way unless there's a reason not to.
+	 */
 	struct list_head link;
 	struct hlist_node hash_link;
-	struct list_head lpage_disallowed_link;
 
 	bool tdp_mmu_page;
 	bool unsync;
@@ -59,6 +62,7 @@ struct kvm_mmu_page {
 	struct kvm_rmap_head parent_ptes; /* rmap pointers to parent sptes */
 	DECLARE_BITMAP(unsync_child_bitmap, 512);
 
+	struct list_head lpage_disallowed_link;
 #ifdef CONFIG_X86_32
 	/*
 	 * Used out of the mmu-lock to avoid reading spte values while an
-- 
2.33.0.153.gba50c8fa24-goog

Re: [PATCH 0/3] KVM: x86/mmu: kvm_mmu_page cleanups

[Paolo Bonzini]

On 02/09/21 00:10, Sean Christopherson wrote:
> Patch 1 is from Jia He to remove a defunct boolean from kvm_mmu_page
> (link[*] below if you want to take it directly).
> 
> Patch 2 builds on that patch to micro-optimize the TDP MMU flag.
> 
> Patch 3 is another micro-optimization that probably doesn't buy much
> performance (I didn't check), feel free to ignore it.
> 
> [*] https://lkml.kernel.org/r/20210830145336.27183-1-justin.he@arm.com
> 
> Jia He (1):
>    KVM: x86/mmu: Remove unused field mmio_cached in struct kvm_mmu_page
> 
> Sean Christopherson (2):
>    KVM: x86/mmu: Relocate kvm_mmu_page.tdp_mmu_page for better cache
>      locality
>    KVM: x86/mmu: Move lpage_disallowed_link further "down" in
>      kvm_mmu_page
> 
>   arch/x86/kvm/mmu/mmu_internal.h | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
> 

Queued, thanks.

Paolo