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

2019-01-28 Thread Dexuan Cui
> From: Dan Williams 
> Sent: Monday, January 28, 2019 1:55 PM
> 
> On Mon, Jan 28, 2019 at 1:40 PM Dexuan Cui  wrote:
> 
> > 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.
> 
> How about something a bit more relevant for the code in question:
> 
 
> There are 4 "legacy" NVDIMM command sets
> (NVDIMM_FAMILY_{INTEL,MSFT,HPE1,HPE2}) that were created before an EFI
> working group was established to constrain this proliferation. The
> nfit driver probes for the supported command set by GUID. Note, If
> you're a platform developer looking to add a new command set to this
> probe consider using an existing set, or otherwise seek approval to
> publish the command set at
> http://www.uefi.org/RFIC_LIST

Looks perfect! Let me use this, rebase the patch to 
https://git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm.git/tag/?h=libnvdimm-fixes-5.0-rc4
and post a v2 later today.

Thanks,
-- Dexuan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2019-01-28 Thread Dan Williams
On Mon, Jan 28, 2019 at 1:40 PM Dexuan Cui  wrote:
>
> > From: Dan Williams 
> > 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  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.
> +*

How about something a bit more relevant for the code in question:

---

There are 4 "legacy" NVDIMM command sets
(NVDIMM_FAMILY_{INTEL,MSFT,HPE1,HPE2}) that were created before an EFI
working group was established to constrain this proliferation. The
nfit driver probes for the supported command set by GUID. Note, If
you're a platform developer looking to add a new command set to this
probe consider using an existing set, or otherwise seek approval to
publish the command set at http://www.uefi.org/RFIC_LIST.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2019-01-28 Thread Dexuan Cui
> From: Dan Williams 
> 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  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 = 0x;
> > +   } 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2019-01-28 Thread Dan Williams
Hi Dexuan,

Looks good. Just one update request and a note below...

On Wed, Jan 23, 2019 at 12:51 PM Dexuan Cui  wrote:
>
>
> Add the Hyper-V _DSM command set to the white list of NVDIMM command
> sets.
>
> This command set is documented at http://www.uefi.org/RFIC_LIST
> (see the link to "Virtual NVDIMM 0x1901" on the page).
>
> Signed-off-by: Dexuan Cui 
> ---
>
> I'm going to change the user-space utility "ndctl" to support Hyper-V Virtual 
> NVDIMM.
> This kernel patch is required first.
>
>  drivers/acpi/nfit/core.c   | 5 -
>  drivers/acpi/nfit/nfit.h   | 6 +-
>  include/uapi/linux/ndctl.h | 1 +
>  3 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
> index 011d3db19c80..fb48cb17a519 100644
> --- 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.

> @@ -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 = 0x;
> +   } 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.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2019-01-26 Thread Michael Kelley
From: Dexuan Cui   Sent: Wednesday, January 23, 2019 12:51 
PM
> 
> Add the Hyper-V _DSM command set to the white list of NVDIMM command
> sets.
> 
> This command set is documented at
> http://www.uefi.org/RFIC_LIST (see the link to "Virtual NVDIMM 0x1901" on the 
> page).
> 
> Signed-off-by: Dexuan Cui 
> ---
> 
> I'm going to change the user-space utility "ndctl" to support Hyper-V Virtual 
> NVDIMM.
> This kernel patch is required first.
> 
>  drivers/acpi/nfit/core.c   | 5 -
>  drivers/acpi/nfit/nfit.h   | 6 +-
>  include/uapi/linux/ndctl.h | 1 +
>  3 files changed, 10 insertions(+), 2 deletions(-)
> 

Reviewed-by:  Michael Kelley 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2019-01-23 Thread Dexuan Cui


Add the Hyper-V _DSM command set to the white list of NVDIMM command
sets.

This command set is documented at http://www.uefi.org/RFIC_LIST
(see the link to "Virtual NVDIMM 0x1901" on the page).

Signed-off-by: Dexuan Cui 
---

I'm going to change the user-space utility "ndctl" to support Hyper-V Virtual 
NVDIMM.
This kernel patch is required first.

 drivers/acpi/nfit/core.c   | 5 -
 drivers/acpi/nfit/nfit.h   | 6 +-
 include/uapi/linux/ndctl.h | 1 +
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 011d3db19c80..fb48cb17a519 100644
--- 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.
 */
@@ -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 = 0x;
+   } else if (nfit_mem->family == NVDIMM_FAMILY_HYPERV) {
+   dsm_mask = 0x1f;
} else {
dev_dbg(dev, "unknown dimm command family\n");
nfit_mem->family = -1;
@@ -3707,6 +3709,7 @@ static __init int nfit_init(void)
guid_parse(UUID_NFIT_DIMM_N_HPE1, &nfit_uuid[NFIT_DEV_DIMM_N_HPE1]);
guid_parse(UUID_NFIT_DIMM_N_HPE2, &nfit_uuid[NFIT_DEV_DIMM_N_HPE2]);
guid_parse(UUID_NFIT_DIMM_N_MSFT, &nfit_uuid[NFIT_DEV_DIMM_N_MSFT]);
+   guid_parse(UUID_NFIT_DIMM_N_HYPERV, &nfit_uuid[NFIT_DEV_DIMM_N_HYPERV]);
 
nfit_wq = create_singlethread_workqueue("nfit");
if (!nfit_wq)
diff --git a/drivers/acpi/nfit/nfit.h b/drivers/acpi/nfit/nfit.h
index 33691aecfcee..9194022f29f3 100644
--- a/drivers/acpi/nfit/nfit.h
+++ b/drivers/acpi/nfit/nfit.h
@@ -34,11 +34,14 @@
 /* https://msdn.microsoft.com/library/windows/hardware/mt604741 */
 #define UUID_NFIT_DIMM_N_MSFT "1ee68b36-d4bd-4a1a-9a16-4f8e53d46e05"
 
+/* http://www.uefi.org/RFIC_LIST */
+#define UUID_NFIT_DIMM_N_HYPERV "5746c5f2-a9a2-4264-ad0e-e4ddc9e09e80"
+
 #define ACPI_NFIT_MEM_FAILED_MASK (ACPI_NFIT_MEM_SAVE_FAILED \
| ACPI_NFIT_MEM_RESTORE_FAILED | ACPI_NFIT_MEM_FLUSH_FAILED \
| ACPI_NFIT_MEM_NOT_ARMED | ACPI_NFIT_MEM_MAP_FAILED)
 
-#define NVDIMM_FAMILY_MAX NVDIMM_FAMILY_MSFT
+#define NVDIMM_FAMILY_MAX NVDIMM_FAMILY_HYPERV
 
 #define NVDIMM_STANDARD_CMDMASK \
 (1 << ND_CMD_SMART | 1 << ND_CMD_SMART_THRESHOLD | 1 << ND_CMD_DIMM_FLAGS \
@@ -94,6 +97,7 @@ enum nfit_uuids {
NFIT_DEV_DIMM_N_HPE1 = NVDIMM_FAMILY_HPE1,
NFIT_DEV_DIMM_N_HPE2 = NVDIMM_FAMILY_HPE2,
NFIT_DEV_DIMM_N_MSFT = NVDIMM_FAMILY_MSFT,
+   NFIT_DEV_DIMM_N_HYPERV = NVDIMM_FAMILY_HYPERV,
NFIT_SPA_VOLATILE,
NFIT_SPA_PM,
NFIT_SPA_DCR,
diff --git a/include/uapi/linux/ndctl.h b/include/uapi/linux/ndctl.h
index f57c9e434d2d..de5d90212409 100644
--- a/include/uapi/linux/ndctl.h
+++ b/include/uapi/linux/ndctl.h
@@ -243,6 +243,7 @@ struct nd_cmd_pkg {
 #define NVDIMM_FAMILY_HPE1 1
 #define NVDIMM_FAMILY_HPE2 2
 #define NVDIMM_FAMILY_MSFT 3
+#define NVDIMM_FAMILY_HYPERV 4
 
 #define ND_IOCTL_CALL  _IOWR(ND_IOCTL, ND_CMD_CALL,\
struct nd_cmd_pkg)
-- 
2.19.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel