Re: [PATCH v11 050/113] KVM: x86/tdp_mmu: Ignore unsupported mmu operation on private GFNs

From: Huang, Kai
Date: Mon Jan 16 2023 - 21:49:00 EST


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.

[snip]