linux-kernel - Re: [PATCH] x86/kvm/mmu: Remove unnecessary ‘NULL’ values from sptep

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <f542fbdf53280f009f435af99c2c57b7e203db21.camel@intel.com>
Date:   Tue, 5 Sep 2023 23:31:20 +0000
From:   "Huang, Kai" <kai.huang@...el.com>
To:     "Christopherson,, Sean" <seanjc@...gle.com>
CC:     "zeming@...china.com" <zeming@...china.com>,
        "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        "bp@...en8.de" <bp@...en8.de>, "x86@...nel.org" <x86@...nel.org>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "pbonzini@...hat.com" <pbonzini@...hat.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] x86/kvm/mmu: Remove unnecessary ‘NULL’ values from sptep

On Tue, 2023-09-05 at 14:09 -0700, Sean Christopherson wrote:
> On Mon, Sep 04, 2023, Kai Huang wrote:
> > On Fri, 2023-09-01 at 09:48 -0700, Sean Christopherson wrote:
> > > @@ -3447,6 +3447,14 @@ static int fast_page_fault(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault)
> > >                 else
> > >                         sptep = fast_pf_get_last_sptep(vcpu, fault->addr, &spte);
> > >  
> > > +               /*
> > > +                * It's entirely possible for the mapping to have been zapped
> > > +                * by a different task, but the root page is should always be
> > > +                * available as the vCPU holds a reference to its root(s).
> > > +                */
> > > +               if (WARN_ON_ONCE(!sptep))
> > > +                       spte = REMOVED_SPTE;
> > 
> > If I recall correctly, REMOVED_SPTE is only used by TDP MMU code.  Should we use
> > 0 (or initial SPTE value for case like TDX) instead of REMOVED_SPTE?
> 
> I deliberately suggested REMOVED_SPTE in part because of TDX introducing "initial
> SPTE"; finding/remembering '0' initialization of SPTEs is hard.  Though FWIW, '0'
> would be totally fine for TDX because the value is never exposed to hardware.
> 
> I think it's totally fine to use REMOVED_SPTE like this in common code, I would
> be quite surprised if it causes confusion.  Even though REMOVED_SPTE was introduced
> by the TDP MMU, its value/semantics are generic.

Yeah certainly no harm here. :-)

My thinking was REMOVED_SPTE is supposed to be an intermediate state for one
SPTE, which is actually "in the page table", while multiple threads can operate
on the page table entry concurrently.  Slightly mismatch the case here IIUC. 
But I guess it also depends on how we view this case here.