On Thu, Mar 21, 2024 at 12:11:11AM +0000, "Edgecombe, Rick P" <rick.p.edgecombe@xxxxxxxxx> wrote: > On Mon, 2024-02-26 at 00:25 -0800, isaku.yamahata@xxxxxxxxx wrote: > > 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. > > tdp_mmu_alloc_sp() is only called in two places. One for the root, and > one for the mid-level tables. > > In later patches when the kvm_mmu_alloc_private_spt() part is added, > the root case doesn't need anything done. So the code has to take > special care in tdp_mmu_alloc_sp() to avoid doing anything for the > root. > > It only needs to do the special private spt allocation in non-root > case. If we open code that case, I think maybe we could drop this > patch, like the below. > > The benefits are to drop this patch (which looks to already be part of > Paolo's series), and simplify "KVM: x86/mmu: Add a private pointer to > struct kvm_mmu_page". I'm not sure though, what do you think? Only > build tested. Makes sense. Until v18, it had config to disable private mmu part at compile time. Those functions have #ifdef in mmu_internal.h. v19 dropped the config for the feedback. https://lore.kernel.org/kvm/Zcrarct88veirZx7@xxxxxxxxxx/ After looking at mmu_internal.h, I think the following three function could be open coded. kvm_mmu_private_spt(), kvm_mmu_init_private_spt(), kvm_mmu_alloc_private_spt(), and kvm_mmu_free_private_spt(). -- Isaku Yamahata <isaku.yamahata@xxxxxxxxx>