On Thu, 2023-01-26 at 21:59 +0000, Sean Christopherson wrote: > On Thu, Jan 26, 2023, Huang, Kai wrote: > > On Thu, 2023-01-26 at 17:28 +0000, Sean Christopherson wrote: > > > In other words, once the PTE is zapped/blocked (branch is pruned), it's completely > > > removed from the paging tree and no other tasks can access the branch (page table > > > and its children). I.e. the only remaining reference to the branch is the pointer > > > handed to the RCU callback. That means the RCU callback has exclusive access to the > > > branch, i.e. can operate as if it were holding mmu_lock for write. Furthermore, the > > > RCU callback also doesn't need to flush TLBs because that was again done when > > > pruning the branch. > > > > > > It's the same idea that KVM already uses for root SPs, the only difference is how > > > KVM determines that there is exactly one entity that holds a reference to the SP. > > > > Right. This works fine for normal non-TDX case. However for TDX unfortunately > > the access to the removed branch (or the removed sub-page-table) isn't that > > "exclusive" as the SEAMCALL to truly zap that branch still needs to hold the > > write lock of the entire Secure EPT tree, so it can still conflict with other > > threads handling new faults. > > I thought TDX was smart enough to read-lock only the part of the tree that it's > actually consuming, and write-lock only the part of the tree that it's actually > modifying? The spec says there's only exclusive/shared access to the "whole Secure EPT tree": 8.6. Secure EPT Concurrency Secure EPT concurrency rules are designed to support the expected usage and yet be as simple as possible. Host-Side (SEAMCALL) Functions: • Functions that manage Secure EPT acquire exclusive access to the whole Secure EPT tree of the target TD. • In specific cases where a Secure EPT entry update may collide with a concurrent update done by the guest TD, such host-side functions update the Secure EPT entry as a transaction, using atomic compare and exchange operations. • TDH.MEM.SEPT.RD acquire shared access to the whole Secure EPT tree of the target TD to help prevent changes to the tree while they execute. • Other functions that only read Secure EPT for GPA-to-HPA translation (e.g., TDH.MR.EXTEND) acquire shared access to the whole Secure EPT tree of the target TD to help prevent changes to the tree while they execute. > > Hrm, but even if TDX takes a read-lock, there's still the problem of it needing > to walk the upper levels, i.e. KVM needs to keep mid-level page tables reachable > until they're fully removed. Blech. That should be a non-issue at this time > though, as I don't think KVM will ever REMOVE a page table of a live guest. I > need to look at the PROMOTE/DEMOTE flows... In this series, if I read correctly, when slot is removed/moved, private mappings are zapped too. It's kinda buried in: [PATCH v11 043/113] KVM: x86/tdp_mmu: Don't zap private pages for unsupported cases (it's not easy to find -- I had to use 'git blame' in the actual repo to find the commit, sigh.)
