On 9/9/24 23:03, Edgecombe, Rick P wrote:
KVM code tends to be less wrapped and a tdx_vm_destory would be a
oneline function. So I think it fits in other ways.
Yes, no problem there. Sometimes one-line functions are ok (see
ept_sync_global() case elsewhere in the series), sometimes they're
overkill, especially if they wrap a function defined in the same file as
the wrapper.
+ * which causes gmem invalidation to zap all spte.
+ * Population is only allowed after KVM_TDX_INIT_VM.
+ */
What does the second sentence ("Population ...") meaning? Why is it
relevant here?
How about:
/*
* HKID is released after all private pages have been removed,
* and set before any might be populated. Warn if zapping is
* attempted when there can't be anything populated in the private
* EPT.
*/
But actually, I wonder if we need to remove the KVM_BUG_ON(). I think if you did
a KVM_PRE_FAULT_MEMORY and then deleted the memslot you could hit it?
I think all paths to handle_removed_pt() are safe:
__tdp_mmu_zap_root
tdp_mmu_zap_root
kvm_tdp_mmu_zap_all
kvm_arch_flush_shadow_all
kvm_flush_shadow_all
kvm_destroy_vm (*)
kvm_mmu_notifier_release (*)
kvm_tdp_mmu_zap_invalidated_roots
kvm_mmu_zap_all_fast (**)
kvm_tdp_mmu_zap_sp
kvm_recover_nx_huge_pages (***)
(*) only called at destroy time
(**) only invalidates direct roots
(***) shouldn't apply to TDX I hope?
Paolo
