diff mbox series

[1/2] KVM: x86: expand the LRU cache of previous CR3s

Message ID 20241029031400.622854-2-alexyonghe@tencent.com (mailing list archive)
State New
Headers show
Series Introduce configuration for LRU cache of previous CR3s | expand

Commit Message

zhuangel570 Oct. 29, 2024, 3:13 a.m. UTC 
From: Yong He <alexyonghe@tencent.com>

Expand max number of LRU cache of previous CR3s, so that
we could cache more entry when needed, such as KPTI is
enabled inside guest.

Signed-off-by: Yong He <alexyonghe@tencent.com>
---
 arch/x86/include/asm/kvm_host.h | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Sean Christopherson Oct. 29, 2024, 2:40 p.m. UTC | #1 
KVM: x86/mmu:

On Tue, Oct 29, 2024, Yong He wrote:
> From: Yong He <alexyonghe@tencent.com>
> 
> Expand max number of LRU cache of previous CR3s, so that
> we could cache more entry when needed, such as KPTI is

No "we".  Documentation/process/maintainer-kvm-x86.rst

And I would argue this changelog is misleading.  I was expecting that the patch
would actually change the number of roots that KVM caches, whereas this simply
increases the capacity.  The changelog should also mention that the whole reason
for doing so is to allow for a module param.

Something like:

  KVM: x86/mmu: Expand max capacity of per-MMU CR3/PGD caches

  Expand the maximum capacity of the "previous roots" cache in kvm_mmu so
  that a future patch can make the number of roots configurable via module
  param, without needing to dynamically allocate the array.

That said, I hope we can avoid this entirely.  More in the next patch.
zhuangel570 Oct. 30, 2024, 12:08 p.m. UTC | #2 
On Wed, Oct 30, 2024 at 3:44 PM Sean Christopherson <seanjc@google.com> wrote:
>
> KVM: x86/mmu:
>
> On Tue, Oct 29, 2024, Yong He wrote:
> > From: Yong He <alexyonghe@tencent.com>
> >
> > Expand max number of LRU cache of previous CR3s, so that
> > we could cache more entry when needed, such as KPTI is
>
> No "we".  Documentation/process/maintainer-kvm-x86.rst
>
> And I would argue this changelog is misleading.  I was expecting that the patch
> would actually change the number of roots that KVM caches, whereas this simply
> increases the capacity.  The changelog should also mention that the whole reason
> for doing so is to allow for a module param.
>
> Something like:
>
>   KVM: x86/mmu: Expand max capacity of per-MMU CR3/PGD caches
>
>   Expand the maximum capacity of the "previous roots" cache in kvm_mmu so
>   that a future patch can make the number of roots configurable via module
>   param, without needing to dynamically allocate the array.
>
> That said, I hope we can avoid this entirely.  More in the next patch.

Thanks for pointing out the problem, will fix in next version.
diff mbox series

Patch

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 4a68cb3eb..02528d4a2 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -430,11 +430,12 @@  struct kvm_mmu_root_info {
 #define KVM_MMU_ROOT_INFO_INVALID \
 	((struct kvm_mmu_root_info) { .pgd = INVALID_PAGE, .hpa = INVALID_PAGE })
 
-#define KVM_MMU_NUM_PREV_ROOTS 3
+#define KVM_MMU_NUM_PREV_ROOTS		3
+#define KVM_MMU_NUM_PREV_ROOTS_MAX	11
 
 #define KVM_MMU_ROOT_CURRENT		BIT(0)
 #define KVM_MMU_ROOT_PREVIOUS(i)	BIT(1+i)
-#define KVM_MMU_ROOTS_ALL		(BIT(1 + KVM_MMU_NUM_PREV_ROOTS) - 1)
+#define KVM_MMU_ROOTS_ALL		(BIT(1 + KVM_MMU_NUM_PREV_ROOTS_MAX) - 1)
 
 #define KVM_HAVE_MMU_RWLOCK
 
@@ -469,7 +470,7 @@  struct kvm_mmu {
 	*/
 	u32 pkru_mask;
 
-	struct kvm_mmu_root_info prev_roots[KVM_MMU_NUM_PREV_ROOTS];
+	struct kvm_mmu_root_info prev_roots[KVM_MMU_NUM_PREV_ROOTS_MAX];
 
 	/*
 	 * Bitmap; bit set = permission fault