On Fri, 2024-05-17 at 02:03 -0700, Isaku Yamahata wrote: > > On top of your patch, I created the following patch to remove > kvm_gfn_for_root(). > Although I haven't tested it yet, I think the following shows my idea. > > - Add gfn_shared_mask to struct tdp_iter. > - Use iter.gfn_shared_mask to determine the starting sptep in the root. > - Remove kvm_gfn_for_root() I investigated it. After this, gfn_t's never have shared bit. It's a simple rule. The MMU mostly thinks it's operating on a shared root that is mapped at the normal GFN. Only the iterator knows that the shared PTEs are actually in a different location. There are some negative side effects: 1. The struct kvm_mmu_page's gfn doesn't match it's actual mapping anymore. 2. As a result of above, the code that flushes TLBs for a specific GFN will be confused. It won't functionally matter for TDX, just look buggy to see flushing code called with the wrong gfn. 3. A lot of tracepoints no longer have the "real" gfn 4. mmio spte doesn't have the shared bit, as previous (no effect) 5. Some zapping code (__tdp_mmu_zap_root(), tdp_mmu_zap_leafs()) intends to actually operating on the raw_gfn. It wants to iterate the whole EPT, so it goes from 0 to tdp_mmu_max_gfn_exclusive(). So now for mirrored it does, but for shared it only covers the shared range. Basically kvm_mmu_max_gfn() is wrong if we pretend shared GFNs are just strangely mapped normal GFNs. Maybe we could just fix this up to report based on GPAW for TDX? Feels wrong. On the positive effects side: 1. There is code that passes sp->gfn into things that it shouldn't (if it has shared bits) like memslot lookups. 2. Also code that passes iter.gfn into things it shouldn't like kvm_mmu_max_mapping_level(). These places are not called by TDX, but if you know that gfn's might include shared bits, then that code looks buggy. I think the solution in the diff is more elegant then before, because it hides what is really going on with the shared root. That is both good and bad. Can we accept the downsides?
