Re: [PATCH v3 31/40] KVM: x86: Move MMU helper declarations from kvm_host.h => mmu.h

[Sean Christopherson <seanjc@google.com>]

On Fri, Jun 05, 2026, Kai Huang wrote:
> > 
> > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > index 1143140592df..f217403e18fc 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -161,12 +161,6 @@
> >  #define KVM_HPAGE_MASK(x)	(~(KVM_HPAGE_SIZE(x) - 1))
> >  #define KVM_PAGES_PER_HPAGE(x)	(KVM_HPAGE_SIZE(x) / PAGE_SIZE)
> >  
> > -#define KVM_MEMSLOT_PAGES_TO_MMU_PAGES_RATIO 50
> > -#define KVM_MIN_ALLOC_MMU_PAGES 64UL
> > -#define KVM_MMU_HASH_SHIFT 12
> > -#define KVM_NUM_MMU_PAGES (1 << KVM_MMU_HASH_SHIFT)
> > -#define KVM_MIN_FREE_MMU_PAGES 5
> > -#define KVM_REFILL_PAGES 25 
> > 
> 
> [...]
> 
> > diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
> > index d30676935fff..a6b871253bd7 100644
> > --- a/arch/x86/kvm/mmu.h
> > +++ b/arch/x86/kvm/mmu.h
> > @@ -15,6 +15,13 @@ extern bool tdp_mmu_enabled;
> >  extern bool __read_mostly enable_mmio_caching;
> >  extern bool eager_page_split;
> >  
> > +#define KVM_MEMSLOT_PAGES_TO_MMU_PAGES_RATIO 50
> > +#define KVM_MIN_ALLOC_MMU_PAGES 64UL
> > +#define KVM_MMU_HASH_SHIFT 12
> > +#define KVM_NUM_MMU_PAGES (1 << KVM_MMU_HASH_SHIFT)
> > +#define KVM_MIN_FREE_MMU_PAGES 5
> > +#define KVM_REFILL_PAGES 25
> > +
> 
> Btw, I found below defines can be moved to "mmu.h" too:
> 
> #define INVALID_PAGE (~(hpa_t)0)
> #define VALID_PAGE(x) ((x) != INVALID_PAGE)
> 
> #define KVM_MMU_ROOT_INFO_INVALID \
>         ((struct kvm_mmu_root_info) { .pgd = INVALID_PAGE, .hpa = INVALID_PAGE
> })

I saw this one too, but I don't want to split KVM_MMU_ROOT_INFO_INVALID from the
related defs:

  #define KVM_MMU_NUM_PREV_ROOTS 3

  #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)

Hrm, but KVM_MMU_ROOT_INFO_INVALID has exactly one user:

	for (i = 0; i < KVM_MMU_NUM_PREV_ROOTS; i++)
		mmu->prev_roots[i] = KVM_MMU_ROOT_INFO_INVALID;

so leaving it in kvm_host.h is silly.  Oof, but that's odd, the code above it
does:

	mmu->root.hpa = INVALID_PAGE;
	mmu->root.pgd = 0;

as does kvm_mmu_free_roots():

		mmu->root.hpa = INVALID_PAGE;
		mmu->root.pgd = 0;

mmu_alloc_direct_roots() deliberately zeros "pgd" to guarantee matches on the
pgd, as guest CR3 is ignored for direct roots, but the "invalidation" code looks
wrong.

Let's handle KVM_MMU_ROOT_INFO_INVALID separately, as simply moving it probably
isn't what we want to do.

> And even below can be moved to "mmu.h", but perhaps you won't like that:
> 
> #define KVM_HPAGE_GFN_SHIFT(x) (((x) - 1) * 9)
> #define KVM_HPAGE_SHIFT(x)     (PAGE_SHIFT + KVM_HPAGE_GFN_SHIFT(x))
> #define KVM_HPAGE_SIZE(x)      (1UL << KVM_HPAGE_SHIFT(x))
> #define KVM_HPAGE_MASK(x)      (~(KVM_HPAGE_SIZE(x) - 1))
> #define KVM_PAGES_PER_HPAGE(x) (KVM_HPAGE_SIZE(x) / PAGE_SIZE)

Huh.  Oh, /facepalm.  I looked at these, but saw there were references in kvm_host.h
and moved on.  But they're all "self" references.  So yeah, I agree moving these
and INVALID_PAGE+VALID_PAGE makes sense.

> #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)

These I want to keep in kvm_host.h because they're tied to KVM_MMU_NUM_PREV_ROOTS,
which needs to stay in kvm_host.h.

Thanks so much for the thorough review(s)!