On Wed, 2025-08-27 at 12:08 -0700, Sean Christopherson wrote: > > e.g., Before KVM_TDX_FINALIZE_VM, if userspace performs a zap after the > > TDH.MEM.PAGE.ADD, the page will be removed from the S-EPT. The count of > > nr_premapped will not change after the successful TDH.MEM.RANGE.BLOCK and > > TDH.MEM.PAGE.REMOVE. > > Eww. It would be nice to close that hole, but I suppose it's futile, e.g. the > underlying problem is unexpectedly removing pages from the initial, whether > the > VMM is doing stupid things before vs. after FINALIZE doesn't really matter. > > > As a result, the TD will still run with uninitialized memory. > > No? Because BLOCK+REMOVE means there are no valid S-EPT mappings. There's a > "hole" that the guest might not expect, but that hole will trigger an EPT > violation and only get "filled" if the guest explicitly accepts an AUG'd page. Ah, I just responded on another patch. I wonder if we can get rid of the premap cnt. > > Side topic, why does KVM tolerate tdh_mem_page_add() failure? IIUC, playing > nice with tdh_mem_page_add() failure necessitates both the > tdx_is_sept_zap_err_due_to_premap() craziness and the check in > tdx_td_finalize() > that all pending pages have been consumed. Reasons that tdh_mem_page_add() could get BUSY: 1. If two vCPU's tried to tdh_mem_page_add() the same gpa at the same time they could contend the SEPT entry lock 2. If one vCPU tries to tdh_mem_page_add() while the other zaps (i.e. tdh_mem_range_block()). I guess since we don't hold MMU lock while we tdh_mem_page_add(), 2 is a possibility. > > What reasonable use case is there for gracefully handling tdh_mem_page_add() > failure? > > If there is a need to handle failure, I gotta imagine it's only for the -EBUSY > case. And if it's only for -EBUSY, why can't that be handled by retrying in > tdx_vcpu_init_mem_region()? If tdx_vcpu_init_mem_region() guarantees that all > pages mapped into the S-EPT are ADDed, then it can assert that there are no > pending pages when it completes (even if it "fails"), and similarly > tdx_td_finalize() can KVM_BUG_ON/WARN_ON the number of pending pages being > non-zero. Maybe we could take mmu write lock for the retry of tdh_mem_page_add(). Or maybe even for a single call of it, until someone wants to parallelize the operation.
