On Tue, Jan 17, 2023 at 02:40:46AM +0000, "Huang, Kai" <kai.huang@xxxxxxxxx> wrote: > On Thu, 2023-01-12 at 08:31 -0800, isaku.yamahata@xxxxxxxxx wrote: > > From: Isaku Yamahata <isaku.yamahata@xxxxxxxxx> > > > > Some KVM MMU operations (dirty page logging, page migration, aging page) > > aren't supported for private GFNs (yet) with the first generation of TDX. > > Silently return on unsupported TDX KVM MMU operations. > > > > Signed-off-by: Isaku Yamahata <isaku.yamahata@xxxxxxxxx> > > > You already have previous patches to do similar things: > > [PATCH v11 034/113] KVM: x86/mmu: Disallow fast page fault on private GPA > [PATCH v11 043/113] KVM: x86/tdp_mmu: Don't zap private pages for unsupported > cases > [PATCH v11 048/113] KVM: x86/mmu: Disallow dirty logging for x86 TDX > [PATCH v11 049/113] KVM: x86/mmu: TDX: Do not enable page track for TD guest > > Now you have this patch: > > [PATCH v11 050/113] KVM: x86/tdp_mmu: Ignore unsupported mmu operation on > private GFNs > > They are very confusing to me. Those previous patches are all "unsupported > operations", correct? > > For instance, this patch says "dirty page logging isn't supported for private > GFNs" (and why there's a 'yet' after it?), so based on the patch title my > understanding is you are going to _ignore_ "dirty page logging". But you > already have a previous patch to "Disallow dirty logging for x86 TDX". > > Shouldn't the two be in the same patch? Or you were trying to highlight the > different between "x86/mmu" and "x86/tdp_mmu"? > > Please try to make the whole thing more clear. My first glance is, if it was > me, I would probably have _ONE_ dedicated patch for _EACH_ unsupported > operation, and make it very clear in the patch title. But you may have your own > way to make things more clearer. Agreed, merged this patch into [PATCH v11 048/113] KVM: x86/mmu: Disallow dirty logging for x86 TDX. -- Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
