Re: [PATCH 06/17] KVM: x86/tdp_mmu: Morph the !is_frozen_spte() check into a KVM_MMU_WARN_ON()

[Sean Christopherson <seanjc@google.com>]

On Thu, Apr 02, 2026, Yan Zhao wrote:
> On Wed, Apr 01, 2026 at 12:37:51AM +0800, Edgecombe, Rick P wrote:
> > On Mon, 2026-03-30 at 13:00 +0800, Yan Zhao wrote:
> > 
> > Yep on the typos. 
> > 
> > > > --- a/arch/x86/kvm/mmu/tdp_mmu.c
> > > > +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> > > > @@ -656,7 +656,13 @@ static inline int __must_check
> > > > __tdp_mmu_set_spte_atomic(struct kvm *kvm,
> > > >   	 */
> > > >   	WARN_ON_ONCE(iter->yielded || is_frozen_spte(iter-
> > > > >old_spte));
> > > >   
> > > > -	if (is_mirror_sptep(iter->sptep) &&
> > > > !is_frozen_spte(new_spte)) {
> > > > +	/*
> > > > +	 * FROZEN_SPTE is a temporary state and should never be
> > > > set via higher
> > > > +	 * level helpers.
> > > > +	 */
> > > > +	KVM_MMU_WARN_ON(is_frozen_spte(new_spte));
> > > Why is KVM_MMU_WARN_ON() used here for new_spte while WARN_ON_ONCE()
> > > is used
> > > above for old_spte?
> > 
> > For the KVM_MMU_WARN_ON() it was Sean's suggestion.
> > 
> > https://lore.kernel.org/lkml/aYYn0nf2cayYu8e7@google.com/
> > 
> > It allows for compiling it out, so probably a better choice. So I see
> > the options are leave them different or opportunistically convert the
> > other one to KVM_MMU_WARN_ON(). Thoughts?
> I see there are mixed WARN_ON_ONCE() and KVM_MMU_WARN_ON() calls in mmu.c and
> tdp_mmu.c. I'm not sure if there's a rule for which one to use.

KVM_MMU_WARN_ON() is intended to be used only when having a WARN_ON_ONCE() enabled
in production environments isn't worth the cost of the code.  E.g. if the code in
question is a low level helper that used "everywhere" and in super hot paths, or
in extremely paranoid sanity checks where the platform is beyond hosed if the
sanity check fails.

The other thing with KVM_MMU_WARN_ON() is that it doesn't evaluate the expression
when CONFIG_KVM_PROVE_MMU=n, i.e. can't be used in if-statements (the only case
where it's used in an if-statement is wrapped with a CONFIG_KVM_PROVE_MMU #ifdef).

For infrequently used and/or slow paths, and for cases where KVM needs to take
action no matter what, WARN_ON_ONCE() is preferred because if something goes
sideways, we'll get a helpful splat even in production.

> Is it necessary to evaluate them all and have a separate patch to convert
> WARN_ON_ONCE() to KVM_MMU_WARN_ON()?

Nope.  We could probably convert a few select ones if there's good reason to do
so, but we definitely don't want to do a bulk conversion.