linux-kernel - RE: [PATCH] nfit: add Hyper-V NVDIMM DSM command set to white list

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <PU1P153MB0169CE6B02AE7D67E25DDD89BF960@PU1P153MB0169.APCP153.PROD.OUTLOOK.COM>
Date:   Mon, 28 Jan 2019 21:39:50 +0000
From:   Dexuan Cui <decui@...rosoft.com>
To:     Dan Williams <dan.j.williams@...el.com>
CC:     Ross Zwisler <zwisler@...nel.org>,
        Vishal Verma <vishal.l.verma@...el.com>,
        Dave Jiang <dave.jiang@...el.com>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Len Brown <lenb@...nel.org>,
        "linux-nvdimm@...ts.01.org" <linux-nvdimm@...ts.01.org>,
        "linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Michael Kelley <mikelley@...rosoft.com>,
        Sasha Levin <Alexander.Levin@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        KY Srinivasan <kys@...rosoft.com>,
        Josh Poulson <jopoulso@...rosoft.com>,
        "driverdev-devel@...uxdriverproject.org" 
        <driverdev-devel@...uxdriverproject.org>
Subject: RE: [PATCH] nfit: add Hyper-V NVDIMM DSM command set to white list

> From: Dan Williams <dan.j.williams@...el.com>
> Sent: Monday, January 28, 2019 1:01 PM
> 
> Hi Dexuan,
> Looks good. Just one update request and a note below...
> 
> On Wed, Jan 23, 2019 at 12:51 PM Dexuan Cui <decui@...rosoft.com> wrote:
> > ...
> > --- a/drivers/acpi/nfit/core.c
> > +++ b/drivers/acpi/nfit/core.c
> > @@ -1840,7 +1840,7 @@ static int acpi_nfit_add_dimm(struct
> acpi_nfit_desc *acpi_desc,
> >         dev_set_drvdata(&adev_dimm->dev, nfit_mem);
> >
> >         /*
> > -        * Until standardization materializes we need to consider 4
> > +        * Until standardization materializes we need to consider 5
> >          * different command sets.  Note, that checking for function0
> (bit0)
> >          * tells us if any commands are reachable through this GUID.
> >          */
> 
> This comment is stale. This "HYPERV" family is the first example of
> the "right" way to define a new NVDIMM command set. Lets update it to
> mention the process and considerations for submitting new command sets
> to UEFI (http://www.uefi.org/RFIC_LIST). The fact that there's now a
> defined process is a step forward from when this comment was initially
> written. Also, the fact that the process encourages "adopt" vs
> "supersede" addresses the main concern about vendor-specific
> command-set proliferation.

I made the below simple change. Is this enough? Please suggest the proper
wording/sentence, as I'm relatively new to NVDIMM, and I don't really know the
history of the standardization process.

--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -1732,8 +1732,10 @@ static int acpi_nfit_add_dimm(struct acpi_nfit_desc *acpi_desc,
        dev_set_drvdata(&adev_dimm->dev, nfit_mem);

        /*
-        * Until standardization materializes we need to consider 4
-        * different command sets.  Note, that checking for function0 (bit0)
+        * New command sets should be submitted to UEFI
+        * http://www.uefi.org/RFIC_LIST.
+        *
+        * Note, that checking for function0 (bit0)
         * tells us if any commands are reachable through this GUID.
         */
        for (i = 0; i <= NVDIMM_FAMILY_MAX; i++)

 
> > @@ -1865,6 +1865,8 @@ static int acpi_nfit_add_dimm(struct
> acpi_nfit_desc *acpi_desc,
> >                         dsm_mask &= ~(1 << 8);
> >         } else if (nfit_mem->family == NVDIMM_FAMILY_MSFT) {
> >                 dsm_mask = 0xffffffff;
> > +       } else if (nfit_mem->family == NVDIMM_FAMILY_HYPERV) {
> > +               dsm_mask = 0x1f;
> 
> Just a note that starting with commit 5e9e38d0db1d "acpi/nfit: Block
> function zero DSMs", bit0 in this mask will be cleared to ensure that
> only the kernel has the ability to probe for supported DSM functions.

Thanks for the note!

Thanks,
-- Dexuan