Re: [RFC PATCH 09/12] KVM: TDX: Fold tdx_mem_page_record_premap_cnt() into its sole caller

["Edgecombe, Rick P" <rick.p.edgecombe@intel.com>]

On Thu, 2025-08-28 at 14:57 -0700, Sean Christopherson wrote:
> Agreed, and it would eliminate the need for a "flags" argument.  But keeping it
> in the mmu_lock critical section means KVM can WARN on failures.  If it's moved
> out, then zapping S-EPT entries could induce failure, and I don't think it's
> worth going through the effort to ensure it's impossible to trigger S-EPT removal.
> 
> Note, temoving S-EPT entries during initialization of the image isn't something
> I want to official support, rather it's an endless stream of whack-a-mole due to
> obsurce edge cases
> 
> Hmm, actually, maybe I take that back.  slots_lock prevents memslot updates,
> filemap_invalidate_lock() prevents guest_memfd updates, and mmu_notifier events
> shouldn't ever hit S-EPT.  I was worried about kvm_zap_gfn_range(), but the call
> from sev.c is obviously mutually exclusive, TDX disallows KVM_X86_QUIRK_IGNORE_GUEST_PAT
> so same goes for kvm_noncoherent_dma_assignment_start_or_stop, and
> 

Yea, in the other thread Yan was suggesting the same thing from the KVM side:
https://lore.kernel.org/all/aK%2Fsdr2OQqYv9DBZ@yzhao56-desk.sh.intel.com/

But was concerned about "Unexpected zaps" (kvm_zap_gfn_range()). I think maybe
we could think about KVM_BUG_ON() in the case of mirror EPT to cover it from
another angle. IIRC we discussed this at some point.

I was wondering about TDH.MR.EXTEND error conditions. Coming back now, I'm not
sure what I was thinking.

> while I'm 99% certain there's a way to trip __kvm_set_or_clear_apicv_inhibit(),
> the APIC page has its own non-guest_memfd memslot and so can't be used for the
> initial image, which means that too is mutually exclusive.

Hmm, well maybe KVM_BUG_ON() for kvm_zap_gfn_range() only if this gets
addressed.

> 
> So yeah, let's give it a shot.  Worst case scenario we're wrong and TDH_MR_EXTEND
> errors can be triggered by userspace.
> 
> > But maybe a better reason is that we could better handle errors
> > outside the fault. (i.e. no 5 line comment about why not to return an error in
> > tdx_mem_page_add() due to code in another file).
> > 
> > I wonder if Yan can give an analysis of any zapping races if we do that.
> 
> As above, I think we're good?

Works for me.