Re: [PATCH v11 030/113] KVM: x86/mmu: Replace hardcoded value 0 for the initial value for SPTE

[Isaku Yamahata]

On Fri, Jan 27, 2023 at 11:36:52PM +0200,
Zhi Wang <zhi.wang.linux@xxxxxxxxx> wrote:

> On Wed, 25 Jan 2023 17:22:08 +0000
> Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
> 
> > On Wed, Jan 25, 2023, Zhi Wang wrote:
> > > On Thu, 12 Jan 2023 08:31:38 -0800
> > > isaku.yamahata@xxxxxxxxx wrote:
> > > 
> > > This refactor patch is quite hacky.
> > > 
> > > Why not change the purpose of vcpu->arch.mmu_shadow_page.gfp_zero and let the
> > > callers respect that the initial value of spte can be configurable? It will be
> > > generic and not TDX-specific, then kvm_init_shadow_page() is not required,
> > > mmu_topup_shadow_page_cache() can be left un-touched as the refactor can cover
> > > other architectures.
> > > 
> > > 1) Let it store the expected nonpresent value and rename it to nonpresent_spte.
> > 
> > 
> > I agree that handling this in the common code would be cleaner, but repurposing
> > gfp_zero gets kludgy because it would require a magic value to say "don't initialize
> > the data", e.g. x86's mmu_shadowed_info_cache isn't pre-filled.
> > 
> > And supporting a custom 64-bit init value for kmem_cache-backed caches would require
> > restricting such caches to be a multiple of 8 bytes in size.
> > 
> > How about this?  Lightly tested.
> > 
> > From: Sean Christopherson <seanjc@xxxxxxxxxx>
> > Date: Wed, 25 Jan 2023 16:55:01 +0000
> > Subject: [PATCH] KVM: Allow page-sized MMU caches to be initialized with
> >  custom 64-bit values
> >
> 
> It looks good enough so far although it only supports 64bit init value. But
> it can be extended in the future. 
> 
> Just want to make sure people are thinking the same:
> 
> 1) Keep the changes of SHADOW_NONPRESENT_VALUE and REMOVED_SPTE in TDX patch.
> init_value stays as a generic feature in the kvm mmu cache layer. It is *not*
> going to replace SHADOW_NONPRESENT_VALUE.
> 
> 2) TDX kvm_x86_vcpu_create sets the SHADOW_NONPRESENT value into init_value.
> 
> 3) mmu cache topping up function initializes the page according to init_value
> with Sean's patch. 

The patch worked for me. Included into v12 patch series.
-- 
Isaku Yamahata <isaku.yamahata@xxxxxxxxx>