Re: [PATCH 11/21] KVM: x86/tdp_mmu: Init role member of struct kvm_mmu_page at allocation

[Edgecombe, Rick P]

On Tue, 2024-02-27 at 18:20 -0500, Paolo Bonzini wrote:
> From: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
> 
> Refactor tdp_mmu_alloc_sp() and tdp_mmu_init_sp and eliminate
> tdp_mmu_init_child_sp().  Currently tdp_mmu_init_sp() (or
> tdp_mmu_init_child_sp()) sets kvm_mmu_page.role after tdp_mmu_alloc_sp()
> allocating struct kvm_mmu_page and its page table page.  This patch makes
> tdp_mmu_alloc_sp() initialize kvm_mmu_page.role instead of
> tdp_mmu_init_sp().
> 
> To handle private page tables, argument of is_private needs to be passed
> down.  Given that already page level is passed down, it would be cumbersome
> to add one more parameter about sp. Instead replace the level argument with
> union kvm_mmu_page_role.  Thus the number of argument won't be increased
> and more info about sp can be passed down.
> 
> For private sp, secure page table will be also allocated in addition to
> struct kvm_mmu_page and page table (spt member).  The allocation functions
> (tdp_mmu_alloc_sp() and __tdp_mmu_alloc_sp_for_split()) need to know if the
> allocation is for the conventional page table or private page table.  Pass
> union kvm_mmu_role to those functions and initialize role member of struct
> kvm_mmu_page.
> 
> Signed-off-by: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
> Message-Id: <d69acdd7f0b0b104f330a6d42ac28f9a9b1b5850.1705965635.git.isaku.yamahata@xxxxxxxxx>
> Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>

We were discussing on v19 of the TDX series whether we could drop this patch and end up with simpler
code in later patches:
https://lore.kernel.org/lkml/20240321212412.GR1994522@xxxxxxxxxxxxxxxxxxxxx/

TDX can manage in either case, but it might not be needed for TDX. Does it have any benefit for SNP?