On Mon, 2024-05-20 at 12:02 -0700, Rick Edgecombe wrote: > > reflect is a nice name. I'm trying this path right now. I'll share a branch. Here is the branch: https://github.com/rpedgeco/linux/commit/674cd68b6ba626e48fe2446797d067e38dca80e3 TODO: - kvm_mmu_max_gfn() updates from iterator changes - kvm_flush_remote_tlbs_gfn() updates from iterator changes The historically controversial mmu.h helpers: static inline gfn_t kvm_gfn_direct_mask(const struct kvm *kvm) { /* Only TDX sets this and it's the shared mask */ return kvm->arch.gfn_shared_mask; } /* The VM keeps a mirrored copy of the private memory */ static inline bool kvm_has_mirrored_tdp(const struct kvm *kvm) { return kvm->arch.vm_type == KVM_X86_TDX_VM; } static inline bool kvm_on_mirror(const struct kvm *kvm, enum kvm_process process) { if (!kvm_has_mirrored_tdp(kvm)) return false; return process & KVM_PROCESS_PRIVATE; } static inline bool kvm_on_direct(const struct kvm *kvm, enum kvm_process process) { if (!kvm_has_mirrored_tdp(kvm)) return true; return process & KVM_PROCESS_SHARED; } static inline bool kvm_zap_leafs_only(const struct kvm *kvm) { return kvm->arch.vm_type == KVM_X86_TDX_VM; } In this solution, the tdp_mmu.c doesn't have a concept of private vs shared EPT or GPA aliases. It just knows KVM_PROCESS_PRIVATE/SHARED, and fault->is_private. Based on the PROCESS enums or fault->is_private, helpers in mmu.h encapsulate whether to operate on the normal "direct" roots or the mirrored roots. When !TDX, it always operates on direct. The code that does PTE setting/zapping etc, calls out the mirrored "reflect" helper and does the extra atomicity stuff when it sees the mirrored role bit. In Isaku's code to make gfn's never have shared bits, there was still the concept of "shared" in the TDP MMU. But now since the TDP MMU focuses on mirrored vs direct instead, an abstraction is introduced to just ask for the mask for the root. For TDX the direct root is for shared memory, so instead the kvm_gfn_direct_mask() gets applied when operating on the direct root. I think there are still some things to be polished in the branch, but overall it does a good job of cleaning up the confusion about the connection between private and mirrored. And also between this and the previous changes, improves littering the generic MMU code with private/shared alias concepts. At the same time, I think the abstractions have a small cost in clarity if you are looking at the code from TDX's perspective. It probably wont raise any eyebrows for people used to tracing nested EPT violations through paging_tmpl.h. But compared to naming everything mirrored_private, there is more obfuscation of the bits twiddled.
