Date:   Mon, 27 Feb 2023 13:50:48 -0800
From:   Isaku Yamahata <isaku.yamahata@gmail.com>
To:     Zhi Wang <zhi.wang.linux@gmail.com>
Cc:     Sean Christopherson <seanjc@google.com>, isaku.yamahata@intel.com,
        kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
        isaku.yamahata@gmail.com, Paolo Bonzini <pbonzini@redhat.com>,
        erdemaktas@google.com, Sagi Shahar <sagis@google.com>,
        David Matlack <dmatlack@google.com>,
        Sean Christopherson <sean.j.christopherson@intel.com>,
        kai.huang@intel.com
Subject: Re: [PATCH v11 030/113] KVM: x86/mmu: Replace hardcoded value 0 for
 the initial value for SPTE
Message-ID: <20230227215048.GK4175971@ls.amr.corp.intel.com>
References: <cover.1673539699.git.isaku.yamahata@intel.com>
 <dee30f0562d8be0102547d8eb9fc77736eae679d.1673539699.git.isaku.yamahata@intel.com>
 <20230125112434.0000512a@gmail.com>
 <Y9Fj/vgPEzfU1eof@google.com>
 <20230127233513.0000367c@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20230127233513.0000367c@gmail.com>
Precedence: bulk

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

> On Wed, 25 Jan 2023 17:22:08 +0000
> Sean Christopherson <seanjc@google.com> wrote:
> 
> > On Wed, Jan 25, 2023, Zhi Wang wrote:
> > > On Thu, 12 Jan 2023 08:31:38 -0800
> > > isaku.yamahata@intel.com 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@google.com>
> > 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@gmail.com>