Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <3cfffe9a29e53ae58dc59d0af3d52128babde79f.camel@intel.com>
Subject: Re: [RFC PATCH v5 037/104] KVM: x86/mmu: Allow non-zero init value
 for shadow PTE
From:   Kai Huang <kai.huang@intel.com>
To:     isaku.yamahata@intel.com, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org
Cc:     isaku.yamahata@gmail.com, Paolo Bonzini <pbonzini@redhat.com>,
        Jim Mattson <jmattson@google.com>, erdemaktas@google.com,
        Connor Kuehl <ckuehl@redhat.com>,
        Sean Christopherson <seanjc@google.com>
Date:   Fri, 01 Apr 2022 20:13:13 +1300
In-Reply-To: <968de4765e63d8255ae1b3ac7062ffdca64706e4.camel@intel.com>
References: <cover.1646422845.git.isaku.yamahata@intel.com>
         <b74b3660f9d16deafe83f2670539a8287bef988f.1646422845.git.isaku.yamahata@intel.com>
         <968de4765e63d8255ae1b3ac7062ffdca64706e4.camel@intel.com>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.42.4 (3.42.4-1.fc35) 
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

On Fri, 2022-04-01 at 18:13 +1300, Kai Huang wrote:
> > --- a/arch/x86/kvm/mmu/mmu.c
> > +++ b/arch/x86/kvm/mmu/mmu.c
> > @@ -617,9 +617,9 @@ static int mmu_spte_clear_track_bits(struct kvm *kvm,
> > u64 *sptep)
> >   	int level = sptep_to_sp(sptep)->role.level;
> >   
> >   	if (!spte_has_volatile_bits(old_spte))
> > -		__update_clear_spte_fast(sptep, 0ull);
> > +		__update_clear_spte_fast(sptep, shadow_init_value);
> >   	else
> > -		old_spte = __update_clear_spte_slow(sptep, 0ull);
> > +		old_spte = __update_clear_spte_slow(sptep,
> > shadow_init_value);
> 
> I guess it's better to have some comment here.  Allow non-zero init value for
> shadow PTE doesn't necessarily mean the initial value should be used when one
> PTE is zapped.  I think mmu_spte_clear_track_bits() is only called for mapping
> of normal (shared) memory but not MMIO? Then perhaps it's better to have a
> comment to explain we want "suppress #VE" set to get a real EPT violation for
> normal memory access from guest?

Btw, I think the relevant part of TDP MMU change should be included in this
patch too otherwise TDP MMU is broken with this patch.

Actually in this series legacy MMU is not supported to work with TDX, so above
change to legacy MMU doesn't matter actually.  Instead, TDP MMU change should be
here.

-- 
Thanks,
-Kai