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;
Date:   Tue, 1 Mar 2022 10:18:52 -0800
From:   Ira Weiny <ira.weiny@intel.com>
To:     Dan Williams <dan.j.williams@intel.com>
Cc:     Dave Hansen <dave.hansen@linux.intel.com>,
        "H. Peter Anvin" <hpa@zytor.com>,
        Fenghua Yu <fenghua.yu@intel.com>,
        Rick Edgecombe <rick.p.edgecombe@intel.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH V8 43/44] nvdimm/pmem: Enable stray access protection
Message-ID: <Yh5jjKMSNucV9J/D@iweiny-desk3>
References: <20220127175505.851391-1-ira.weiny@intel.com>
 <20220127175505.851391-44-ira.weiny@intel.com>
 <CAPcyv4idgNDaxJfj2MZgJnuGyygFJ9ozvrEghSnsR7zXB=pj0A@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAPcyv4idgNDaxJfj2MZgJnuGyygFJ9ozvrEghSnsR7zXB=pj0A@mail.gmail.com>
Precedence: bulk

On Fri, Feb 04, 2022 at 01:10:53PM -0800, Dan Williams wrote:
> On Thu, Jan 27, 2022 at 9:55 AM <ira.weiny@intel.com> wrote:
> >
> > From: Ira Weiny <ira.weiny@intel.com>
> >
> > Now that all valid kernel access' to PMEM have been annotated with
> > {__}pgmap_mk_{readwrite,noaccess}() PGMAP_PROTECTION is safe to enable
> > in the pmem layer.
> >
> > Implement the pmem_map_protected() and pmem_mk_{readwrite,noaccess}() to
> > communicate this memory has extra protection to the upper layers if
> > PGMAP_PROTECTION is specified.
> >
> > Internally, the pmem driver uses a cached virtual address,
> > pmem->virt_addr (pmem_addr).  Use __pgmap_mk_{readwrite,noaccess}()
> > directly when PGMAP_PROTECTION is active on the device.
> >
> > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> >
> > ---
> > Changes for V8
> >         Rebase to 5.17-rc1
> >         Remove global param
> >         Add internal structure which uses the pmem device and pgmap
> >                 device directly in the *_mk_*() calls.
> >         Add pmem dax ops callbacks
> >         Use pgmap_protection_available()
> >         s/PGMAP_PKEY_PROTECT/PGMAP_PROTECTION
> > ---
> >  drivers/nvdimm/pmem.c | 52 ++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 51 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
> > index 58d95242a836..2afff8157233 100644
> > --- a/drivers/nvdimm/pmem.c
> > +++ b/drivers/nvdimm/pmem.c
> > @@ -138,6 +138,18 @@ static blk_status_t read_pmem(struct page *page, unsigned int off,
> >         return BLK_STS_OK;
> >  }
> >
> > +static void __pmem_mk_readwrite(struct pmem_device *pmem)
> > +{
> > +       if (pmem->pgmap.flags & PGMAP_PROTECTION)
> > +               __pgmap_mk_readwrite(&pmem->pgmap);
> > +}
> > +
> > +static void __pmem_mk_noaccess(struct pmem_device *pmem)
> > +{
> > +       if (pmem->pgmap.flags & PGMAP_PROTECTION)
> > +               __pgmap_mk_noaccess(&pmem->pgmap);
> > +}
> > +
> 
> Per previous feedback let's find a way for the pmem driver to stay out
> of the loop, and just let these toggles by pgmap generic operations.

I want to clarify.  Yes the pmem driver is now out of the dax driver loop.
However, these calls must remain because the pmem driver caches pmem->virt_addr
and uses that address to access the maps directly.

Therefore these specific calls need to remain for the pmem drivers internal
use.  In addition to the commit message I've added comments to the call sites
to clarify this fact.

Ira