On Thu, May 16, 2024 at 12:13:44AM +0000, "Edgecombe, Rick P" <rick.p.edgecombe@xxxxxxxxx> wrote: > On Thu, 2024-05-16 at 11:38 +1200, Huang, Kai wrote: > > On 16/05/2024 11:14 am, Edgecombe, Rick P wrote: > > > On Thu, 2024-05-16 at 10:17 +1200, Huang, Kai wrote: > > > > > TDX has several aspects related to the TDP MMU. > > > > > 1) Based on the faulting GPA, determine which KVM page table to walk. > > > > > (private-vs-shared) > > > > > 2) Need to call TDX SEAMCALL to operate on Secure-EPT instead of direct > > > > > memory > > > > > load/store. TDP MMU needs hooks for it. > > > > > 3) The tables must be zapped from the leaf. not the root or the middle. > > > > > > > > > > For 1) and 2), what about something like this? TDX backend code will > > > > > set > > > > > kvm->arch.has_mirrored_pt = true; I think we will use > > > > > kvm_gfn_shared_mask() > > > > > only > > > > > for address conversion (shared<->private). > > > > > > 1 and 2 are not the same as "mirrored" though. You could have a design that > > > mirrors half of the EPT and doesn't track it with separate roots. In fact, 1 > > > might be just a KVM design choice, even for TDX. > > > > I am not sure whether I understand this correctly. If they are not > > tracked with separate roots, it means they use the same page table (root). > > There are three roots, right? Shared, private and mirrored. Shared and mirrored > don't have to be different roots, but it makes some operations arguably easier > to have it that way. Do you have something like KVM_X86_SW_PROTECTED_VM with mirrored PT in mind? or TDX thing? > > So IIUC what you said is to support "mirror PT" at any sub-tree of the > > page table? > > > > That will only complicate things. I don't think we should consider > > this. In reality, we only have TDX and SEV-SNP. We should have a > > simple solution to cover both of them. > > Look at "bool is_private" in kvm_tdp_mmu_map(). Do you see how it switches > between different roots in the iterator? That is one use. > > The second use is to decide whether to call out to the x86_ops. It happens via > the role bit in the sp, which is copied from the parent sp role. The root's bit > is set originally via a kvm_gfn_shared_mask() check. > > BTW, the role bit is the thing I'm wondering if we really need, because we have > shared_mask. While the shared_mask is used for lots of things today, we need > still need it for masking GPAs. Where as the role bit is only needed to know if > a SP is for private (which we can tell from the GPA). I started the discussion at [1] for it. [1] https://lore.kernel.org/kvm/20240516001530.GG168153@xxxxxxxxxxxxxxxxxxxxx/ -- Isaku Yamahata <isaku.yamahata@xxxxxxxxx>