Re: [PATCH v2 7/9] KVM: SEV: Forcefully invalidate SNP VMSA if its backing gmem page is zapped

[Sean Christopherson <seanjc@google.com>]

On Mon, Jun 29, 2026, Ackerley Tng wrote:
> Sean Christopherson <seanjc@google.com> writes:
> 
> >
> > [...snip...]
> >
> > diff --git a/arch/x86/include/asm/kvm-x86-ops.h b/arch/x86/include/asm/kvm-x86-ops.h
> > index e36eba952705..69ca2a848ad6 100644
> > --- a/arch/x86/include/asm/kvm-x86-ops.h
> > +++ b/arch/x86/include/asm/kvm-x86-ops.h
> > @@ -134,6 +134,7 @@ KVM_X86_OP_OPTIONAL(mem_enc_unregister_region)
> >  KVM_X86_OP_OPTIONAL(vm_copy_enc_context_from)
> >  KVM_X86_OP_OPTIONAL(vm_move_enc_context_from)
> >  KVM_X86_OP_OPTIONAL(guest_memory_reclaimed)
> > +KVM_X86_OP_OPTIONAL(reload_vmsa)
> 
> Regarding naming, this seems to be the most platform-specific KVM_X86_OP
> there is (VMSA is a SEV-ES/SNP only thing), though I can't think of a
> way to make this seem more general, to make it stick out less among the
> x86 ops. Maybe it's not necessary to make it blend in anyway.

Yeah, it sucks.  I wanted to turn KVM_REQ_APIC_PAGE_RELOAD into a generic "reload
vendor specific thing" request, but it's not feasible to do that without a pile
of ugly conditionals (or potential false positives on AMD).

> >  KVM_X86_OP(get_feature_msr)
> >  KVM_X86_OP(check_emulate_instruction)
> >  KVM_X86_OP(apic_init_signal_blocked)
> > @@ -148,6 +149,7 @@ KVM_X86_OP_OPTIONAL(alloc_apic_backing_page)
> >  KVM_X86_OP_OPTIONAL_RET0(gmem_prepare)
> >  KVM_X86_OP_OPTIONAL_RET0(gmem_max_mapping_level)
> >  #ifdef CONFIG_HAVE_KVM_ARCH_GMEM_INVALIDATE
> > +KVM_X86_OP_OPTIONAL(gmem_invalidate_range)
> 
> Clarification: .gmem_invalidate_range() should be thought of as "this is
> the hook called when some guest_memfd memory is invalidated" (as in,
> this is a point in the lifecycle of guest_memfd memory) and not "this
> hook invalidates guest memory in this range", right?

Yes, the former, though I would phrase it more along the lines of "something about
this memory may be changing, drop all references".

> >  KVM_X86_OP_OPTIONAL(gmem_free_folio)
> >  #endif
> >  #endif
> >
> > [...snip...]
> >
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index adc1e1b244c7..9df6acf9a982 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -8167,6 +8167,8 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
> >  				goto out;
> >  			}
> >  		}
> > +		if (kvm_check_request(KVM_REQ_VMSA_PAGE_RELOAD, vcpu))
> > +			kvm_x86_call(reload_vmsa)(vcpu);
> 
> Should this be wrapped with an #ifdef to skip the check entirely for
> non-SNP platforms?

No, we generally avoid using #ifdefs as micro-optimizations, and this would be
more like a pico-optimazation.  It's a single uop that in practice will be easily
predicted by hardware.

> >  	}
> >
> >  	if (kvm_check_request(KVM_REQ_EVENT, vcpu) || req_int_win ||
> > @@ -10592,6 +10594,10 @@ int kvm_arch_gmem_prepare(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn, int max_ord
> >  #endif
> >
> >
> > [...snip...]
> >
> > diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> > index 1618acc3ca64..8ec5041934db 100644
> > --- a/virt/kvm/guest_memfd.c
> > +++ b/virt/kvm/guest_memfd.c
> > @@ -185,6 +185,10 @@ static void __kvm_gmem_invalidate_start(struct gmem_file *f, pgoff_t start,
> >  		}
> >
> >  		flush |= kvm_mmu_unmap_gfn_range(kvm, &gfn_range);
> > +
> > +#ifdef CONFIG_HAVE_KVM_ARCH_GMEM_INVALIDATE
> > +		kvm_arch_gmem_invalidate_range(kvm, &gfn_range);
> > +#endif
> 
> In general, how do you think of using stubs vs @ifdefs?

They both have their uses.  Generally speaking, KVM (and x86 in general) tries to
avoid #ifdefs as they tend to make the code harder to read.  But there are cases
where stubs simply don't make sense, e.g. all of the code guarded by CONFIG_KVM_XEN,
CONFIG_KVM_IOAPIC, and CONFIG_KVM_HYPERV is so tightly coupled to its specific
"thing" that using a stub makes very little sense.  And providing stubs can be
actively dangerous, in that it allows selecting a Kconfig without wiring up the
plumbing, whereas using #ifdefs will result in build failures.

> Would it be better to define a stub for kvm_arch_gmem_invalidate_range()
> that does nothing #ifdef CONFIG_HAVE_KVM_ARCH_GMEM_INVALIDATE?

I don't have a super strong preference, but given that the whole point of the
Kconfig is to opt-in to calling an arch hook, providing a stub falls into the
"actively dangerous" category.

And I don't want to diverge from what we do for CONFIG_HAVE_KVM_ARCH_GMEM_POPULATE,
i.e. *if* we want to provide stubs instead of using #ifdefs, then I want to do
that separately, in a concerted effort.