nvdimm: Add an IOCTL pass thru for DSM calls

PATCH New Jerry Hoemann 1 participant 8 messages Apr 27, 2016

Subsystems (2)

ACPI libnvdimm: non-volatile memory device

lore

Share & tools

lore

Jerry Hoemann

author

2016-04-27 22:46 UTC #0 lore raw

v10 cover nvdimm: Add an IOCTL pass thru for DSM calls

The NVDIMM code in the kernel supports an IOCTL interface to user space based upon the Intel Example DSM:

http://pmem.io/documents/NVDIMM_DSM_Interface_Example.pdf

This interface cannot be used by other NVDIMM DSMs that support incompatible functions.

An alternative DSM specification for Type N DSM being developed by Hewlett Packard Enterprise can be found at:

https://github.com/HewlettPackard/hpe-nvm/tree/master/Documentation

To accommodate multiple and conflicting DSM specifications, this patch set adds a generic "pass-thru" IOCTL interface which is not tied to a particular DSM.

A new _IOC_NR ND_CMD_CALL == "30" is added for the pass thru call.

The new data structure nd_cmd_pkg serves as a wrapper for the pass-thru calls. This wrapper supplies the data that the kernel needs to make the _DSM call.

Unlike the definitions of the _DSM functions themselves, the nd_cmd_pkg provides the calling information (input/output sizes) in an uniform manner making the kernel marshaling of the arguments straight forward.

This shifts the marshaling burden from the kernel to the user space application while still permitting the kernel to internally call _DSM functions.

The kernel functions __nd_ioctl and acpi_nfit_ctl were modified to accommodate ND_CMD_CALL.

Changes in version 10:
---------------------
Code review comments:
0. "static const" on nfit_cmd_family_tbl
1. change name ND_MAX_CMD to ND_MAX_DSM_FUN_IDX
2. use nfit_cmd_family_tbl locally
3. add comment on passthru return semantics for size mis-match.
4. Misc white space changes.
5. Change name call_dsm back to pkg
6. set bit ND_CMD_CALL to acpi_desc->bus_dsm_force_en in nfit_test0_setup

0005-nvdimm-Extend-nfit_cmd_family_tbl-to-allow-masking-d:
---------------------------------------------------------
A new patch was added specifically dealing with the nfit_cmd_family_tbl. A mask to be bit-and with the firmware dsm_mask was added.
A mask to be bit-or with the cmd_mask were added.
The table is searched once and pointer to the entry was returned. This allows for the minor modification required for each nvdimm type. I left this as a separate patch so change was more obvious for reviewers, but it can be squashed later.

0006-tools-testing-nvdimm-dimm_dsm_force_en:
--------------------------------------------
A new patch was added to change name of dimm_dsm_force_en and to set its value in nfit_test0_setup().

0007-nvmdimm-ND_CMD_CALL:
-------------------------
An RFC patch was added to change value of ND_CMD_CALL to 30 to avoid/delay collision with commands which are directly mapped to the Intel DSM. The maintainers tree has ND_CMD_CALL value set to 10. I'm okay with either value as long as it doesn't change once accepted.

Changes in version 9:
---------------------
0. Based on https://git.kernel.org/cgit/linux/kernel/git/djbw/nvdimm.git/log/?h=for-4.7/dsm

1. Fixed a broken printk statement in error path.

Addressed the following code review requests:

1. Separated determination of uuid and dsm_mask into separate functions.

2. Reverted to the repeatedly calling acpi_check_dsm for each bit in dsm_mask instead of just calling firmware once. Function is called 22 times per nvdimm.

4. Removed bit mask from nfit_cmd_family_tbl.

5. Added separate cmd_mask to struct nfit_mem.

6. Changed *dsm_mask to cmd_mask in struct nvdimm.

7. changed __nd_ioctl to filter based upon cmd_mask not dsm_mask.

Changes in version 8:
---------------------
1. augmented family_to_uuid() to return uuid. This to address bug in prior version where acpi_nfit_ctl wasn't updating uuid with value associated with command family.

2. patch 0006 changes name of nvdimm_bus_descriptor.dsm_mask to .cmd_mask

3. patch 0008 adds field cmd_ioctl if kernel supports full ioctl as with Intel example dsm.

4. patch 0009 make determination if kernel supports the full cmd_ioctl for that dsm. Updates the commands_show function to invert the sense of display of commands. All dsm support pass-thru, only Intel example support the full ioctl interface.

5. patch 0010 adds explicit ioctl interface to return command mask. This was done in part to avoid "unknown" command in sysfs.

Changes in version 7:
--------------------
0. change name ND_CMD_CALL_DSM to ND_CMD_CALL
- part of abstracting out DSM missed in version 6.

1. change name in struct nd_call_dsm

    a) "ncp_" -> "nd_"
    b) ncp_pot_size -> nd_fw_size
    c) ncp_type -> nd_family
    o) cascade name changes to other patches

2. Expanded comment around data structure nd_cmd_pkg

3. At Dan's request, hard coding "root" UUID.
a) retract extension of dsm_uuid to nvdimm_bus_descriptor.
b) reverted nfit.c/acpi_nfit_init_dsms() with the exception of allowing function 0 in mask.

4. At Dan's request, removed "rev" from nd_cmd_pkg. Hard-coding use of rev "1" in acpi_nfit_ctl.

Changes in version 6:
---------------------
Built against
git://git.kernel.org/pub/scm/linux/kernel/git/djbw/nvdimm.git
libnvdimm-pending

0. Patches "Clean-up access mode check" and "Fix security issue with DSM IOCTL" already in above libnvdimm-pending. So omitted here.

1. Incorporated changes from Dan's RFC patch set
https://lists.01.org/pipermail/linux-nvdimm/2016-January/004049.html

2. Dan asked me to abstract out the DSM aspects from the ndm_cmd_dsmcall_pkg. This became nd_cmd_pkg. UUIDs are no longer passed in from user applications.

3. To accommodate multiple UUIDS, added table cmd_type_tbl which is used to determine UUID for the acpi object by calling function 0 for each UUID in table until success.

This table also provides a MASK field that the kernel can use to exclude functions being called.

This table can be thought of a list of "acceptable" DSMs.

4. The cmd_type_tbl is also used by acpi_nfit_ctl to map the external handle of calls to internal handle, UUID.

Note, code only validates that the requested type of call is one in cmd_type_tbl, but it might not necessarily be the same found during acpi_nfit_add_dimm. The ACPI SPEC appears to allow and firmware does implement multiple UUID per object.

In the case where type is in table, but the UUID isn't supported by the underlying firmware, firmware shall return an error when called.

This allows for use of a secondary DSM on an object. This could be considered a feature or a defect. This can be tightened up if needed.

Changes in version 5:
---------------------
0. Fixed submit comment for drivers/acpi/utils.c.

Changes in version 4:
---------------------
0. Added patch to correct parameter type passed to acpi_evaluate_dsm ACPI defines arguments rev and fun as 64 bit quantities and the ioctl exports to user face rev and func. We want those to match the ACPI spec.

Also modified acpi_evaluate_dsm_typed and acpi_check dsm which had similar issue.

1. nd_cmd_dsmcall_pkg rearrange a reserve and rounded up total size to 16 byte boundary.

2. Created stand alone patch for the pre-existing security issue related to "read only" IOCTL calls.

3. Added patch for increasing envelope size of IOCTL. Needed to be able to read in the wrapper to know remaining size to copy in.

Note: in_env, out_env are statics sized based upon this change.

4. Moved copyin code to table driven nd_cmd_desc

Note, the last 40 lines or so of acpi_nfit_ctl will not return _DSM data unless the size allocated in user space buffer equals out_obj->buffer.length.

The semantic we want in the pass thru case is to return as much of the _DSM data as the user space buffer would accommodate.

Hence, in acpi_nfit_ctl I have retained the line:

		memcpy(pkg->dsm_buf + pkg->h.dsm_in,
			out_obj->buffer.pointer,
			min(pkg->h.dsm_size, pkg->h.dsm_out));

and the early return from the function.

Changes in version 3:
---------------------
1. Changed name ND_CMD_PASSTHRU to ND_CMD_CALL_DSM.

2. Value of ND_CMD_CALL_DSM is 10, not 100.

3. Changed name of nd_passthru_pkg to nd_cmd_dsmcall_pkg.

4. Removed separate functions for handling ND_CMD_CALL_DSM. Moved functionality to __nd_ioctl and acpi_nfit_ctl proper. The resultant code looks very different from prior versions.

5. BUGFIX: __nd_ioctl: Change the if read_only switch to use _IOC_NR cmd (not ioctl_cmd) for better protection.

Do we want to make a stand alone patch for this issue?

Changes in version 2:
---------------------
1. Cleanup access mode check in nd_ioctl and nvdimm_ioctl.
2. Change name of ndn_pkg to nd_passthru_pkg
3. Adjust sizes in nd_passthru_pkg. DSM integers are 64 bit.
4. No new ioctl type, instead tunnel into the existing number space.
5. Push down one function level where determine ioctl cmd type.
6. re-work diagnostic print/dump message in pass-thru functions. Jerry Hoemann (7):
nvdimm: Add IOCTL pass thru functions
libnvdimm: nvdimm_bus_descriptor field name change tools/testing/nvdimm: 'call_dsm' support
nvdimm: Add concept of cmd mask
nvdimm: Extend nfit_cmd_family_tbl to allow masking dsm_mask tools/testing/nvdimm: dimm_dsm_force_en
nvmdimm: ND_CMD_CALL

 drivers/acpi/nfit.c              | 127 ++++++++++++++++++++++++++++++++++-----
 drivers/acpi/nfit.h              |   3 +-
 drivers/nvdimm/bus.c             |  47 +++++++++++++--
 drivers/nvdimm/core.c            |   2 +-
 drivers/nvdimm/dimm_devs.c       |  12 ++--
 drivers/nvdimm/nd-core.h         |   2 +-
 include/linux/libnvdimm.h        |   4 +-
 include/uapi/linux/ndctl.h       |   2 +-
 tools/testing/nvdimm/test/nfit.c |  22 +++++--
 9 files changed, 184 insertions(+), 37 deletions(-)

Signature · 6 lines

1.7.11.3

_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm

Jerry Hoemann

author

2016-04-27 22:46 UTC #1 lore raw

PATCH v10 1/7 nvdimm: Add IOCTL pass thru functions

Add ioctl command ND_CMD_CALL_DSM to acpi_nfit_ctl and __nd_ioctl which allow kernel to call a nvdimm's _DSM as a passthru without using the marshaling code of the nd_cmd_desc.

This supports DSM as defined in:

http://pmem.io/documents/NVDIMM_DSM_Interface_Example.pdf
https://github.com/HewlettPackard/hpe-nvm/tree/master/Documentation

Signed-off-by Jerry Hoemann <[email protected]>

---
 drivers/acpi/nfit.c  | 111 ++++++++++++++++++++++++++++++++++++++++++++++-----
 drivers/nvdimm/bus.c |  43 +++++++++++++++++++-
 2 files changed, 143 insertions(+), 11 deletions(-)

drivers/acpi/nfit.c +101 ~~-10~~

index d0f35e6..2379f79 100644
@@ -56,6 +56,24 @@ struct nfit_table_prev {
 	struct list_head flushes;
 };
 
+struct cmd_family_tbl {
+	enum nfit_uuids	key_uuid;	/* Internal handle		*/
+	int		key_type;	/* Exported handle		*/
+	int		rev;		/* _DSM rev			*/
+};
+
+/*
+ * If adding new cmd family, determine maximum function index
+ */
+#define ND_MAX_DSM_FUN_IDX	20
+
+static const struct cmd_family_tbl nfit_cmd_family_tbl[] = {
+	{ NFIT_DEV_DIMM,	ND_TYPE_DIMM_INTEL1,	1},
+	{ NFIT_DEV_DIMM_N_HPE1, ND_TYPE_DIMM_N_HPE1,	1},
+	{ NFIT_DEV_DIMM_N_HPE2, ND_TYPE_DIMM_N_HPE2,	1},
+	{ -1, -1, -1},
+};
+
 static u8 nfit_uuid[NFIT_UUID_MAX][16];
 
 const u8 *to_nfit_uuid(enum nfit_uuids id)

@@ -64,6 +82,19 @@ const u8 *to_nfit_uuid(enum nfit_uuids id)
 }
 EXPORT_SYMBOL(to_nfit_uuid);
 
+static const u8 *
+family_to_uuid(int type)
+{
+	const struct cmd_family_tbl *tbl = nfit_cmd_family_tbl;
+	int i;
+
+	for (i = 0;  tbl[i].key_type >= 0 ; i++) {
+		if (tbl[i].key_type == type)
+			return to_nfit_uuid(tbl[i].key_uuid);
+	}
+	return 0;
+}
+
 static struct acpi_nfit_desc *to_acpi_nfit_desc(
 		struct nvdimm_bus_descriptor *nd_desc)
 {

@@ -171,8 +202,9 @@ static int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
 		unsigned int buf_len, int *cmd_rc)
 {
 	struct acpi_nfit_desc *acpi_desc = to_acpi_nfit_desc(nd_desc);
-	const struct nd_cmd_desc *desc = NULL;
 	union acpi_object in_obj, in_buf, *out_obj;
+	struct nd_cmd_pkg *call_dsm = NULL;
+	const struct nd_cmd_desc *desc = NULL;
 	struct device *dev = acpi_desc->dev;
 	const char *cmd_name, *dimm_name;
 	unsigned long dsm_mask;

@@ -180,6 +212,12 @@ static int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
 	const u8 *uuid;
 	u32 offset;
 	int rc, i;
+	__u64 rev = 1, func = cmd;
+
+	if (cmd == ND_CMD_CALL) {
+		call_dsm = buf;
+		func = call_dsm->nd_command;
+	}
 
 	if (nvdimm) {
 		struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);

@@ -207,7 +245,7 @@ static int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
 	if (!desc || (cmd && (desc->out_num + desc->in_num == 0)))
 		return -ENOTTY;
 
-	if (!test_bit(cmd, &dsm_mask))
+	if (!test_bit(func, &dsm_mask))
 		return -ENOTTY;
 
 	in_obj.type = ACPI_TYPE_PACKAGE;

@@ -222,21 +260,50 @@ static int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
 		in_buf.buffer.length += nd_cmd_in_size(nvdimm, cmd, desc,
 				i, buf);
 
+	if (call_dsm) {
+		/* must skip over package wrapper */
+		in_buf.buffer.pointer = (void *) &call_dsm->nd_payload;
+		in_buf.buffer.length = call_dsm->nd_size_in;
+		uuid = family_to_uuid(call_dsm->nd_family);
+		if (!uuid) {
+			dev_dbg(dev, "%s:%s unsupported uuid\n", dimm_name,
+					cmd_name);
+			return -EINVAL;
+		}
+	}
+
 	if (IS_ENABLED(CONFIG_ACPI_NFIT_DEBUG)) {
-		dev_dbg(dev, "%s:%s cmd: %s input length: %d\n", __func__,
-				dimm_name, cmd_name, in_buf.buffer.length);
-		print_hex_dump_debug(cmd_name, DUMP_PREFIX_OFFSET, 4,
-				4, in_buf.buffer.pointer, min_t(u32, 128,
-					in_buf.buffer.length), true);
+		dev_dbg(dev, "%s:%s cmd: %d: %llu input length: %d\n", __func__,
+				dimm_name, cmd, func, in_buf.buffer.length);
+		print_hex_dump_debug("nvdimm in  ", DUMP_PREFIX_OFFSET, 4, 4,
+			in_buf.buffer.pointer,
+			min_t(u32, 256, in_buf.buffer.length), true);
+
 	}
 
-	out_obj = acpi_evaluate_dsm(handle, uuid, 1, cmd, &in_obj);
+	out_obj = acpi_evaluate_dsm(handle, uuid, rev, func, &in_obj);
 	if (!out_obj) {
 		dev_dbg(dev, "%s:%s _DSM failed cmd: %s\n", __func__, dimm_name,
 				cmd_name);
 		return -EINVAL;
 	}
 
+	if (call_dsm) {
+		call_dsm->nd_fw_size = out_obj->buffer.length;
+		memcpy(call_dsm->nd_payload + call_dsm->nd_size_in,
+			out_obj->buffer.pointer,
+			min(call_dsm->nd_fw_size, call_dsm->nd_size_out));
+
+		ACPI_FREE(out_obj);
+		/*
+		 * Need to support FW function w/o known size in advance.
+		 * Caller can determine required size based upon nd_fw_size.
+		 * If call returned -ENXIO (like elsewhere) then caller wouldn't
+		 * be able to rely upon data returned to make calculation.
+		 */
+		return 0;
+	}
+
 	if (out_obj->package.type != ACPI_TYPE_BUFFER) {
 		dev_dbg(dev, "%s:%s unexpected output object type cmd: %s type: %d\n",
 				__func__, dimm_name, cmd_name, out_obj->type);

@@ -918,12 +985,30 @@ static struct nvdimm *acpi_nfit_dimm_by_handle(struct acpi_nfit_desc *acpi_desc,
 	return NULL;
 }
 
+static int determine_uuid(acpi_handle handle, u8 const **cmd_uuid)
+{
+	const struct cmd_family_tbl *tbl = nfit_cmd_family_tbl;
+	int i;
+
+	for (i = 0;  tbl[i].key_uuid >= 0 ; i++) {
+		const u8 *uuid = to_nfit_uuid(tbl[i].key_uuid);
+		int rev = tbl[i].rev;
+
+		if (acpi_check_dsm(handle, uuid, rev, 1ULL)) {
+			*cmd_uuid = uuid;
+			return 1;
+		}
+	}
+	return 0;
+}
+
+
 static int acpi_nfit_add_dimm(struct acpi_nfit_desc *acpi_desc,
 		struct nfit_mem *nfit_mem, u32 device_handle)
 {
 	struct acpi_device *adev, *adev_dimm;
 	struct device *dev = acpi_desc->dev;
-	const u8 *uuid = to_nfit_uuid(NFIT_DEV_DIMM);
+	const u8 *uuid;
 	int i;
 
 	nfit_mem->dsm_mask = acpi_desc->dimm_dsm_force_en;

@@ -939,7 +1024,11 @@ static int acpi_nfit_add_dimm(struct acpi_nfit_desc *acpi_desc,
 		return force_enable_dimms ? 0 : -ENODEV;
 	}
 
-	for (i = ND_CMD_SMART; i <= ND_CMD_VENDOR; i++)
+	if (!determine_uuid(adev_dimm->handle, &nfit_mem->dsm_uuid))
+		return force_enable_dimms ? 0 : -ENODEV;
+
+	uuid = nfit_mem->dsm_uuid;
+	for (i = 0; i <= ND_MAX_DSM_FUN_IDX; i++)
 		if (acpi_check_dsm(adev_dimm->handle, uuid, 1, 1ULL << i))
 			set_bit(i, &nfit_mem->dsm_mask);

@@ -2463,6 +2552,8 @@ static __init int nfit_init(void)
 	acpi_str_to_uuid(UUID_PERSISTENT_VIRTUAL_CD, nfit_uuid[NFIT_SPA_PCD]);
 	acpi_str_to_uuid(UUID_NFIT_BUS, nfit_uuid[NFIT_DEV_BUS]);
 	acpi_str_to_uuid(UUID_NFIT_DIMM, nfit_uuid[NFIT_DEV_DIMM]);
+	acpi_str_to_uuid(UUID_NFIT_DIMM_N_HPE1, nfit_uuid[NFIT_DEV_DIMM_N_HPE1]);
+	acpi_str_to_uuid(UUID_NFIT_DIMM_N_HPE2, nfit_uuid[NFIT_DEV_DIMM_N_HPE2]);
 
 	nfit_wq = create_singlethread_workqueue("nfit");
 	if (!nfit_wq)

drivers/nvdimm/bus.c +42 -1

93 lines not on this page · raw message

Signature · 6 lines

1.7.11.3

_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm

Jerry Hoemann

author

2016-04-27 22:46 UTC #2 lore raw

PATCH v10 2/7 libnvdimm: nvdimm_bus_descriptor field name change

In reply to Jerry Hoemann

Change the the name of:

nvdimm_bus_descriptor.dsm_mask
to
nvdimm_bus_descriptor.cmd_mask

Signed-off-by Jerry Hoemann <[email protected]>

---
 drivers/acpi/nfit.c              | 6 +++---
 drivers/nvdimm/bus.c             | 2 +-
 drivers/nvdimm/core.c            | 2 +-
 include/linux/libnvdimm.h        | 2 +-
 tools/testing/nvdimm/test/nfit.c | 2 +-
 5 files changed, 7 insertions(+), 7 deletions(-)

drivers/acpi/nfit.c +3 -3

index 2379f79..cc5999e 100644
@@ -235,7 +235,7 @@ static int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
 		struct acpi_device *adev = to_acpi_dev(acpi_desc);
 
 		cmd_name = nvdimm_bus_cmd_name(cmd);
-		dsm_mask = nd_desc->dsm_mask;
+		dsm_mask = nd_desc->cmd_mask;
 		desc = nd_cmd_bus_desc(cmd);
 		uuid = to_nfit_uuid(NFIT_DEV_BUS);
 		handle = adev->handle;

@@ -1096,14 +1096,14 @@ static void acpi_nfit_init_dsms(struct acpi_nfit_desc *acpi_desc)
 	struct acpi_device *adev;
 	int i;
 
-	nd_desc->dsm_mask = acpi_desc->bus_dsm_force_en;
+	nd_desc->cmd_mask = acpi_desc->bus_dsm_force_en;
 	adev = to_acpi_dev(acpi_desc);
 	if (!adev)
 		return;
 
 	for (i = ND_CMD_ARS_CAP; i <= ND_CMD_CLEAR_ERROR; i++)
 		if (acpi_check_dsm(adev->handle, uuid, 1, 1ULL << i))
-			set_bit(i, &nd_desc->dsm_mask);
+			set_bit(i, &nd_desc->cmd_mask);
 }
 
 static ssize_t range_index_show(struct device *dev,

drivers/nvdimm/bus.c +1 -1

index 685e3ba..ed8bfae 100644
@@ -625,7 +625,7 @@ static int __nd_ioctl(struct nvdimm_bus *nvdimm_bus, struct nvdimm *nvdimm,
 	} else {
 		desc = nd_cmd_bus_desc(cmd);
 		cmd_name = nvdimm_bus_cmd_name(cmd);
-		dsm_mask = nd_desc->dsm_mask;
+		dsm_mask = nd_desc->cmd_mask;
 		dimm_name = "bus";
 	}

drivers/nvdimm/core.c +1 -1

index 182a93f..e8688a1 100644
@@ -251,7 +251,7 @@ static ssize_t commands_show(struct device *dev,
 	struct nvdimm_bus *nvdimm_bus = to_nvdimm_bus(dev);
 	struct nvdimm_bus_descriptor *nd_desc = nvdimm_bus->nd_desc;
 
-	for_each_set_bit(cmd, &nd_desc->dsm_mask, BITS_PER_LONG)
+	for_each_set_bit(cmd, &nd_desc->cmd_mask, BITS_PER_LONG)
 		len += sprintf(buf + len, "%s ", nvdimm_bus_cmd_name(cmd));
 	len += sprintf(buf + len, "\n");
 	return len;

include/linux/libnvdimm.h +1 -1

index af31d1c..67a1ba0 100644
@@ -68,7 +68,7 @@ struct nd_mapping {
 
 struct nvdimm_bus_descriptor {
 	const struct attribute_group **attr_groups;
-	unsigned long dsm_mask;
+	unsigned long cmd_mask;
 	char *provider_name;
 	ndctl_fn ndctl;
 	int (*flush_probe)(struct nvdimm_bus_descriptor *nd_desc);

tools/testing/nvdimm/test/nfit.c +1 -1

index 3187322..1d3b8ce 100644
@@ -374,7 +374,7 @@ static int nfit_test_ctl(struct nvdimm_bus_descriptor *nd_desc,
 	} else {
 		struct ars_state *ars_state = &t->ars_state;
 
-		if (!nd_desc || !test_bit(cmd, &nd_desc->dsm_mask))
+		if (!nd_desc || !test_bit(cmd, &nd_desc->cmd_mask))
 			return -ENOTTY;
 
 		switch (cmd) {

Signature · 6 lines

1.7.11.3

_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm

Jerry Hoemann

author

2016-04-27 22:46 UTC #3 lore raw

PATCH v10 3/7 tools/testing/nvdimm: 'call_dsm' support

In reply to Jerry Hoemann

Enable nfit_test to use the generic 'call_dsm' envelope.

Signed-off-by Dan Williams <[email protected]>

---
 tools/testing/nvdimm/test/nfit.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

tools/testing/nvdimm/test/nfit.c +11 -2

index 1d3b8ce..6c89811 100644
@@ -336,12 +336,21 @@ static int nfit_test_ctl(struct nvdimm_bus_descriptor *nd_desc,
 {
 	struct acpi_nfit_desc *acpi_desc = to_acpi_desc(nd_desc);
 	struct nfit_test *t = container_of(acpi_desc, typeof(*t), acpi_desc);
+	unsigned int func = cmd;
 	int i, rc = 0, __cmd_rc;
 
 	if (!cmd_rc)
 		cmd_rc = &__cmd_rc;
 	*cmd_rc = 0;
 
+	if (cmd == ND_CMD_CALL) {
+		struct nd_cmd_pkg *pkg = buf;
+
+		buf_len = pkg->nd_size_in + pkg->nd_size_out;
+		buf = (void *) pkg->nd_payload;
+		func = pkg->nd_command;
+	}
+
 	if (nvdimm) {
 		struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);

@@ -356,7 +365,7 @@ static int nfit_test_ctl(struct nvdimm_bus_descriptor *nd_desc,
 		if (i >= ARRAY_SIZE(handle))
 			return -ENXIO;
 
-		switch (cmd) {
+		switch (func) {
 		case ND_CMD_GET_CONFIG_SIZE:
 			rc = nfit_test_cmd_get_config_size(buf, buf_len);
 			break;

@@ -377,7 +386,7 @@ static int nfit_test_ctl(struct nvdimm_bus_descriptor *nd_desc,
 		if (!nd_desc || !test_bit(cmd, &nd_desc->cmd_mask))
 			return -ENOTTY;
 
-		switch (cmd) {
+		switch (func) {
 		case ND_CMD_ARS_CAP:
 			rc = nfit_test_cmd_ars_cap(buf, buf_len);
 			break;

Signature · 6 lines

1.7.11.3

_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm

Jerry Hoemann

author

2016-04-27 22:46 UTC #4 lore raw

PATCH v10 4/7 nvdimm: Add concept of cmd mask

In reply to Jerry Hoemann

DSMs have a mask of commands implemented on the DSM.

The current IOCTL interface exported for the NVDIMM DSM maps directly onto the underlying DSM.

With the addition of the pass thru mechanism, the IOCTL doesn't map directly onto the underlying DSM functions as the Pass thru isn't itself a DSM function and some DSM can use only the pass thru.

These changes separate out the concept of a command mask that applies to function that are supported by the IOCTL and a dsm mask that apply to the underlying firmware.

There is one noticiable exception, the cmd_mask == the dsm_mask for root functions.

Signed-off-by Jerry Hoemann <[email protected]>

---
 drivers/acpi/nfit.c        | 27 +++++++++++++++++++++++----
 drivers/acpi/nfit.h        |  1 +
 drivers/nvdimm/bus.c       | 10 ++++------
 drivers/nvdimm/dimm_devs.c | 12 ++++++------
 drivers/nvdimm/nd-core.h   |  2 +-
 include/linux/libnvdimm.h  |  2 +-
 6 files changed, 36 insertions(+), 18 deletions(-)

drivers/acpi/nfit.c +23 -4

index cc5999e..a15fd92 100644
@@ -60,6 +60,7 @@ struct cmd_family_tbl {
 	enum nfit_uuids	key_uuid;	/* Internal handle		*/
 	int		key_type;	/* Exported handle		*/
 	int		rev;		/* _DSM rev			*/
+	int		does_ioctls;	/* Family supports all commands? */
 };
 
 /*

@@ -68,9 +69,9 @@ struct cmd_family_tbl {
 #define ND_MAX_DSM_FUN_IDX	20
 
 static const struct cmd_family_tbl nfit_cmd_family_tbl[] = {
-	{ NFIT_DEV_DIMM,	ND_TYPE_DIMM_INTEL1,	1},
-	{ NFIT_DEV_DIMM_N_HPE1, ND_TYPE_DIMM_N_HPE1,	1},
-	{ NFIT_DEV_DIMM_N_HPE2, ND_TYPE_DIMM_N_HPE2,	1},
+	{ NFIT_DEV_DIMM,	ND_TYPE_DIMM_INTEL1,	1, 1},
+	{ NFIT_DEV_DIMM_N_HPE1, ND_TYPE_DIMM_N_HPE1,	1, 0},
+	{ NFIT_DEV_DIMM_N_HPE2, ND_TYPE_DIMM_N_HPE2,	1, 0},
 	{ -1, -1, -1},
 };

@@ -1002,6 +1003,20 @@ static int determine_uuid(acpi_handle handle, u8 const **cmd_uuid)
 	return 0;
 }
 
+static int determine_cmd_mask(u8 const *cmd_uuid)
+{
+	const struct cmd_family_tbl *tbl = nfit_cmd_family_tbl;
+	int i;
+
+	for (i = 0;  tbl[i].key_uuid >= 0 ; i++) {
+		const u8 *uuid = to_nfit_uuid(tbl[i].key_uuid);
+
+		if (memcmp(uuid, cmd_uuid, 16) == 0)
+			return tbl[i].does_ioctls;
+	}
+	return 0;
+}
+
 
 static int acpi_nfit_add_dimm(struct acpi_nfit_desc *acpi_desc,
 		struct nfit_mem *nfit_mem, u32 device_handle)

@@ -1032,6 +1047,10 @@ static int acpi_nfit_add_dimm(struct acpi_nfit_desc *acpi_desc,
 		if (acpi_check_dsm(adev_dimm->handle, uuid, 1, 1ULL << i))
 			set_bit(i, &nfit_mem->dsm_mask);
 
+	nfit_mem->cmd_mask = (1 << ND_CMD_CALL);
+	if (determine_cmd_mask(uuid))
+		nfit_mem->cmd_mask |= nfit_mem->dsm_mask;
+
 	return 0;
 }

@@ -1067,7 +1086,7 @@ static int acpi_nfit_register_dimms(struct acpi_nfit_desc *acpi_desc)
 
 		nvdimm = nvdimm_create(acpi_desc->nvdimm_bus, nfit_mem,
 				acpi_nfit_dimm_attribute_groups,
-				flags, &nfit_mem->dsm_mask);
+				flags, nfit_mem->cmd_mask);
 		if (!nvdimm)
 			return -ENOMEM;

drivers/acpi/nfit.h +1 -0

index 7179ea6..e2fcab3 100644
@@ -114,6 +114,7 @@ struct nfit_mem {
 	struct list_head list;
 	struct acpi_device *adev;
 	unsigned long dsm_mask;
+	unsigned long cmd_mask;
 	const u8 *dsm_uuid;
 };

drivers/nvdimm/bus.c +4 -6

index ed8bfae..395a9fb 100644
@@ -612,31 +612,29 @@ static int __nd_ioctl(struct nvdimm_bus *nvdimm_bus, struct nvdimm *nvdimm,
 	struct device *dev = &nvdimm_bus->dev;
 	struct nd_cmd_pkg pkg;
 	const char *cmd_name, *dimm_name;
-	unsigned int func = cmd;
-	unsigned long dsm_mask;
+	unsigned long cmd_mask;
 	void *buf;
 	int rc, i;
 
 	if (nvdimm) {
 		desc = nd_cmd_dimm_desc(cmd);
 		cmd_name = nvdimm_cmd_name(cmd);
-		dsm_mask = nvdimm->dsm_mask ? *(nvdimm->dsm_mask) : 0;
+		cmd_mask = nvdimm->cmd_mask;
 		dimm_name = dev_name(&nvdimm->dev);
 	} else {
 		desc = nd_cmd_bus_desc(cmd);
 		cmd_name = nvdimm_bus_cmd_name(cmd);
-		dsm_mask = nd_desc->cmd_mask;
+		cmd_mask = nd_desc->cmd_mask;
 		dimm_name = "bus";
 	}
 
 	if (cmd == ND_CMD_CALL) {
 		if (copy_from_user(&pkg, p, sizeof(pkg)))
 			return -EFAULT;
-		func = pkg.nd_command;
 	}
 
 	if (!desc || (desc->out_num + desc->in_num == 0) ||
-			!test_bit(func, &dsm_mask))
+			!test_bit(cmd, &cmd_mask))
 		return -ENOTTY;
 
 	/* fail write commands (when read-only) */

drivers/nvdimm/dimm_devs.c +6 -6

index c56f882..b86b45a 100644
@@ -37,9 +37,9 @@ static int __validate_dimm(struct nvdimm_drvdata *ndd)
 
 	nvdimm = to_nvdimm(ndd->dev);
 
-	if (!nvdimm->dsm_mask)
+	if (!nvdimm->cmd_mask)
 		return -ENXIO;
-	if (!test_bit(ND_CMD_GET_CONFIG_DATA, nvdimm->dsm_mask))
+	if (!test_bit(ND_CMD_GET_CONFIG_DATA, &nvdimm->cmd_mask))
 		return -ENXIO;
 
 	return 0;

@@ -277,10 +277,10 @@ static ssize_t commands_show(struct device *dev,
 	struct nvdimm *nvdimm = to_nvdimm(dev);
 	int cmd, len = 0;
 
-	if (!nvdimm->dsm_mask)
+	if (!nvdimm->cmd_mask)
 		return sprintf(buf, "\n");
 
-	for_each_set_bit(cmd, nvdimm->dsm_mask, BITS_PER_LONG)
+	for_each_set_bit(cmd, &nvdimm->cmd_mask, BITS_PER_LONG)
 		len += sprintf(buf + len, "%s ", nvdimm_cmd_name(cmd));
 	len += sprintf(buf + len, "\n");
 	return len;

@@ -340,7 +340,7 @@ EXPORT_SYMBOL_GPL(nvdimm_attribute_group);
 
 struct nvdimm *nvdimm_create(struct nvdimm_bus *nvdimm_bus, void *provider_data,
 		const struct attribute_group **groups, unsigned long flags,
-		unsigned long *dsm_mask)
+		unsigned long cmd_mask)
 {
 	struct nvdimm *nvdimm = kzalloc(sizeof(*nvdimm), GFP_KERNEL);
 	struct device *dev;

@@ -355,7 +355,7 @@ struct nvdimm *nvdimm_create(struct nvdimm_bus *nvdimm_bus, void *provider_data,
 	}
 	nvdimm->provider_data = provider_data;
 	nvdimm->flags = flags;
-	nvdimm->dsm_mask = dsm_mask;
+	nvdimm->cmd_mask = cmd_mask;
 	atomic_set(&nvdimm->busy, 0);
 	dev = &nvdimm->dev;
 	dev_set_name(dev, "nmem%d", nvdimm->id);

drivers/nvdimm/nd-core.h +1 -1

index 1d1500f..da0d322 100644
@@ -37,7 +37,7 @@ struct nvdimm_bus {
 struct nvdimm {
 	unsigned long flags;
 	void *provider_data;
-	unsigned long *dsm_mask;
+	unsigned long cmd_mask;
 	struct device dev;
 	atomic_t busy;
 	int id;

include/linux/libnvdimm.h +1 -1

index 67a1ba0..41c975a 100644
@@ -133,7 +133,7 @@ const char *nvdimm_name(struct nvdimm *nvdimm);
 void *nvdimm_provider_data(struct nvdimm *nvdimm);
 struct nvdimm *nvdimm_create(struct nvdimm_bus *nvdimm_bus, void *provider_data,
 		const struct attribute_group **groups, unsigned long flags,
-		unsigned long *dsm_mask);
+		unsigned long cmd_mask);
 const struct nd_cmd_desc *nd_cmd_dimm_desc(int cmd);
 const struct nd_cmd_desc *nd_cmd_bus_desc(int cmd);
 u32 nd_cmd_in_size(struct nvdimm *nvdimm, int cmd,

Signature · 6 lines

1.7.11.3

_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm

Jerry Hoemann

author

2016-04-27 22:46 UTC #5 lore raw

PATCH v10 5/7 nvdimm: Extend nfit_cmd_family_tbl to allow masking dsm_mask

In reply to Jerry Hoemann

Extend nfit_cmd_family_tbl to have both cmd_mask and dsm_masks modifiers. The masking of dsm_mask allows for Linux to block block calls to only those functions defined in their respective specifications.

The cmd_mask are different in that with the pass-thru only ND_CMD_CALL is allowed. But in case of cmd_mask its not as simple as taking result of (1ULL << ND_CMD_CALL) | dsm_mask.

acpi_nfit_add_dimm() calls determine_cmd_family() to return table entry that has separate configuration features of the nvdimm.

Signed-off-by Jerry Hoemann <[email protected]>

---
 drivers/acpi/nfit.c | 45 ++++++++++++++++-----------------------------
 1 file changed, 16 insertions(+), 29 deletions(-)

drivers/acpi/nfit.c +16 ~~-29~~

index a15fd92..90b63f9 100644
@@ -60,7 +60,8 @@ struct cmd_family_tbl {
 	enum nfit_uuids	key_uuid;	/* Internal handle		*/
 	int		key_type;	/* Exported handle		*/
 	int		rev;		/* _DSM rev			*/
-	int		does_ioctls;	/* Family supports all commands? */
+	int		cmd_mask;	/* Family supports these commands */
+	unsigned long	family_mask;	/* Make sure _DSM mask is per spec */
 };
 
 /*

@@ -69,9 +70,9 @@ struct cmd_family_tbl {
 #define ND_MAX_DSM_FUN_IDX	20
 
 static const struct cmd_family_tbl nfit_cmd_family_tbl[] = {
-	{ NFIT_DEV_DIMM,	ND_TYPE_DIMM_INTEL1,	1, 1},
-	{ NFIT_DEV_DIMM_N_HPE1, ND_TYPE_DIMM_N_HPE1,	1, 0},
-	{ NFIT_DEV_DIMM_N_HPE2, ND_TYPE_DIMM_N_HPE2,	1, 0},
+	{ NFIT_DEV_DIMM,	ND_TYPE_DIMM_INTEL1,	1, 0x3fe, 0x0000003ff},
+	{ NFIT_DEV_DIMM_N_HPE1, ND_TYPE_DIMM_N_HPE1,	1, 0,	  0x3ffffffff},
+	{ NFIT_DEV_DIMM_N_HPE2, ND_TYPE_DIMM_N_HPE2,	1, 0,	  0x00007ffff},
 	{ -1, -1, -1},
 };

@@ -986,7 +987,8 @@ static struct nvdimm *acpi_nfit_dimm_by_handle(struct acpi_nfit_desc *acpi_desc,
 	return NULL;
 }
 
-static int determine_uuid(acpi_handle handle, u8 const **cmd_uuid)
+
+static const struct cmd_family_tbl *determine_cmd_family(acpi_handle handle)
 {
 	const struct cmd_family_tbl *tbl = nfit_cmd_family_tbl;
 	int i;

@@ -996,31 +998,16 @@ static int determine_uuid(acpi_handle handle, u8 const **cmd_uuid)
 		int rev = tbl[i].rev;
 
 		if (acpi_check_dsm(handle, uuid, rev, 1ULL)) {
-			*cmd_uuid = uuid;
-			return 1;
+			return &tbl[i];
 		}
 	}
-	return 0;
-}
-
-static int determine_cmd_mask(u8 const *cmd_uuid)
-{
-	const struct cmd_family_tbl *tbl = nfit_cmd_family_tbl;
-	int i;
-
-	for (i = 0;  tbl[i].key_uuid >= 0 ; i++) {
-		const u8 *uuid = to_nfit_uuid(tbl[i].key_uuid);
-
-		if (memcmp(uuid, cmd_uuid, 16) == 0)
-			return tbl[i].does_ioctls;
-	}
-	return 0;
+	return NULL;
 }
 
-
 static int acpi_nfit_add_dimm(struct acpi_nfit_desc *acpi_desc,
 		struct nfit_mem *nfit_mem, u32 device_handle)
 {
+	const struct cmd_family_tbl *tbl;
 	struct acpi_device *adev, *adev_dimm;
 	struct device *dev = acpi_desc->dev;
 	const u8 *uuid;

@@ -1039,17 +1026,17 @@ static int acpi_nfit_add_dimm(struct acpi_nfit_desc *acpi_desc,
 		return force_enable_dimms ? 0 : -ENODEV;
 	}
 
-	if (!determine_uuid(adev_dimm->handle, &nfit_mem->dsm_uuid))
+	tbl = determine_cmd_family(adev_dimm->handle);
+	if (!tbl)
 		return force_enable_dimms ? 0 : -ENODEV;
 
-	uuid = nfit_mem->dsm_uuid;
-	for (i = 0; i <= ND_MAX_DSM_FUN_IDX; i++)
+	uuid = nfit_mem->dsm_uuid = to_nfit_uuid(tbl->key_uuid);
+	for_each_set_bit(i, &(tbl->family_mask), BITS_PER_LONG)
 		if (acpi_check_dsm(adev_dimm->handle, uuid, 1, 1ULL << i))
 			set_bit(i, &nfit_mem->dsm_mask);
 
-	nfit_mem->cmd_mask = (1 << ND_CMD_CALL);
-	if (determine_cmd_mask(uuid))
-		nfit_mem->cmd_mask |= nfit_mem->dsm_mask;
+	nfit_mem->cmd_mask = (1ULL << ND_CMD_CALL);
+	nfit_mem->cmd_mask |= tbl->cmd_mask;
 
 	return 0;
 }

Signature · 6 lines

1.7.11.3

_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm

Jerry Hoemann

author

2016-04-27 22:46 UTC #6 lore raw

PATCH v10 6/7 tools/testing/nvdimm: dimm_dsm_force_en

In reply to Jerry Hoemann

Set ND_CMD_CALL in dimm_dsm_force_en. Change name dimm_dsm_force_en to dimm_cmd_force_en.

Signed-off-by Jerry Hoemann <[email protected]>

---
 drivers/acpi/nfit.c              | 2 +-
 drivers/acpi/nfit.h              | 2 +-
 tools/testing/nvdimm/test/nfit.c | 7 ++++---
 3 files changed, 6 insertions(+), 5 deletions(-)

drivers/acpi/nfit.c +1 -1

index 90b63f9..e412b98 100644
@@ -1013,7 +1013,7 @@ static int acpi_nfit_add_dimm(struct acpi_nfit_desc *acpi_desc,
 	const u8 *uuid;
 	int i;
 
-	nfit_mem->dsm_mask = acpi_desc->dimm_dsm_force_en;
+	nfit_mem->cmd_mask = acpi_desc->dimm_cmd_force_en;
 	adev = to_acpi_dev(acpi_desc);
 	if (!adev)
 		return 0;

drivers/acpi/nfit.h +1 -1

index e2fcab3..3b5759a 100644
@@ -138,7 +138,7 @@ struct acpi_nfit_desc {
 	size_t ars_status_size;
 	struct work_struct work;
 	unsigned int cancel:1;
-	unsigned long dimm_dsm_force_en;
+	unsigned long dimm_cmd_force_en;
 	unsigned long bus_dsm_force_en;
 	int (*blk_do_io)(struct nd_blk_region *ndbr, resource_size_t dpa,
 			void *iobuf, u64 len, int rw);

tools/testing/nvdimm/test/nfit.c +4 -3

index 6c89811..c04a2c5 100644
@@ -1260,9 +1260,10 @@ static void nfit_test0_setup(struct nfit_test *t)
 	post_ars_status(&t->ars_state, t->spa_set_dma[0], SPA0_SIZE);
 
 	acpi_desc = &t->acpi_desc;
-	set_bit(ND_CMD_GET_CONFIG_SIZE, &acpi_desc->dimm_dsm_force_en);
-	set_bit(ND_CMD_GET_CONFIG_DATA, &acpi_desc->dimm_dsm_force_en);
-	set_bit(ND_CMD_SET_CONFIG_DATA, &acpi_desc->dimm_dsm_force_en);
+	set_bit(ND_CMD_GET_CONFIG_SIZE, &acpi_desc->dimm_cmd_force_en);
+	set_bit(ND_CMD_GET_CONFIG_DATA, &acpi_desc->dimm_cmd_force_en);
+	set_bit(ND_CMD_SET_CONFIG_DATA, &acpi_desc->dimm_cmd_force_en);
+	set_bit(ND_CMD_CALL,		&acpi_desc->dimm_cmd_force_en);
 	set_bit(ND_CMD_ARS_CAP, &acpi_desc->bus_dsm_force_en);
 	set_bit(ND_CMD_ARS_START, &acpi_desc->bus_dsm_force_en);
 	set_bit(ND_CMD_ARS_STATUS, &acpi_desc->bus_dsm_force_en);

Signature · 6 lines

1.7.11.3

_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm

Jerry Hoemann

author

2016-04-27 22:46 UTC #7 lore raw

RFC v10 7/7 nvmdimm: ND_CMD_CALL

In reply to Jerry Hoemann

Increase value of ND_CMD_CALL to delay need to remap ioctl numbers to underpinning DSM spec.

Signed-off-by Jerry Hoemann <[email protected]>

---
 include/uapi/linux/ndctl.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

include/uapi/linux/ndctl.h +1 -1

index 9214af7..2ba1e33 100644
@@ -125,7 +125,7 @@ enum {
 	ND_CMD_VENDOR_EFFECT_LOG_SIZE = 7,
 	ND_CMD_VENDOR_EFFECT_LOG = 8,
 	ND_CMD_VENDOR = 9,
-	ND_CMD_CALL = 10,
+	ND_CMD_CALL = 30,
 };
 
 enum {

Signature · 6 lines

1.7.11.3

_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm