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).
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.
What we are really trying to do here is not put "is tdx" logic in the generic
code. We could rely on the fact that TDX is the only one with mirrored TDP, but
that is kind of what we are already doing with kvm_gfn_shared_mask().
How about we do helpers for each of your bullets, and they all just check:
vm_type == KVM_X86_TDX_VM
So like:
diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
index a578ea09dfb3..c0beed5b090a 100644
--- a/arch/x86/kvm/mmu.h
+++ b/arch/x86/kvm/mmu.h
@@ -355,4 +355,19 @@ static inline bool kvm_is_private_gpa(const struct kvm
*kvm, gpa_t gpa)
return mask && !(gpa_to_gfn(gpa) & mask);
}
+static inline bool kvm_has_mirrored_tdp(struct kvm *kvm)
+{
+ return kvm->arch.vm_type == KVM_X86_TDX_VM;
+}
+
+static inline bool kvm_has_private_root(struct kvm *kvm)
+{
+ return kvm->arch.vm_type == KVM_X86_TDX_VM;
+}
I don't think we need to distinguish the two.
Even we do this, if I understand your saying correctly,
kvm_has_private_root() isn't just enough -- theoretically we can have a
mirror pt at a sub-tree at any level.
