Re: [PATCH 2/5] KVM: TDX: Exit with MEMORY_FAULT on unexpected pending S-EPT Violation

[Sean Christopherson <seanjc@google.com>]

On Tue, Jul 29, 2025, Rick P Edgecombe wrote:
> On Tue, 2025-07-29 at 15:54 -0700, Sean Christopherson wrote:
> > > The vm_dead was added because mirror EPT will KVM_BUG_ON() if there is an
> > > attempt to set the mirror EPT entry when it is already present. And the
> > > unaccepted memory access will trigger an EPT violation for a mirror PTE
> > > that is already set. I think this is a better solution irrespective of
> > > the vm_dead changes.
> > 
> > In that case, this change will expose KVM to the KVM_BUG_ON(), because nothing
> > prevents userspace from re-running the vCPU. 
> 
> If userspace runs the vCPU again then an EPT violation gets triggered again,
> which again gets kicked out to userspace. The new check will prevent it from
> getting into the fault handler, right?

Yes?  But I'm confused about why you mentioned vm_dead, and why you're calling
this a "new check".  This effectively does two things: drops kvm_vm_dead() and
switches from EOI => EFAULT.  _If_ setting vm_dead was necessary, then we have a
problem.

I assume by "The vm_dead was added" you really mean "forcing an exit to userspace",
and that kvm_vm_dead()+EIO was a somewhat arbitrary way of forcing an exit?

> >  Which KVM_BUG_ON() exactly gets hit?
> 
> Should be:
> 
> static int __must_check set_external_spte_present(struct kvm *kvm, tdp_ptep_t
> sptep,
> 						 gfn_t gfn, u64 old_spte,
> 						 u64 new_spte, int level)
> {
> 	bool was_present = is_shadow_present_pte(old_spte);
> 	bool is_present = is_shadow_present_pte(new_spte);
> 	bool is_leaf = is_present && is_last_spte(new_spte, level);
> 	kvm_pfn_t new_pfn = spte_to_pfn(new_spte);
> 	int ret = 0;
> 
> 	KVM_BUG_ON(was_present, kvm);

Yeah, I don't see how that can be reach in this scenario.