[PATCH 0/4] nvdimm: Add an IOCTL pass thru for DSM calls

[PATCH 0/4] nvdimm: Add an IOCTL pass thru for DSM calls

[Jerry Hoemann]

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.

This patch set adds a generic "passthru" IOCTL interface which
is not tied to a particular DSM.

A new IOCTL type "P" is added for the pass thru call.

The new data structure ndn_pkg serves as a wrapper for the passthru
calls.  This wrapper supplies the data that the kernel needs to
make the _DSM call.

Unlike the definitions of the _DSM functions themselves, the ndn_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
calling _DSM functions.

To make the resultant kernel code easier to understand the existing
functions acpi_nfit_ctl and __nd_ioctl were renamed to .*_intel to
denote calling mechanism as in 4.2 tailored to the Intel Example DSM.
New functions acpi_nfit_ctl_passthru and __nd_ioctl_passthru were
created to supply the pass thru interface.


These changes are based upon the 4.3 kernel.


Jerry Hoemann (4):
  nvdimm: Add wrapper for IOCTL pass thru.
  nvdimm: Add IOCTL pass thru
  nvdimm: Add IOCTL pass thru
  nvdimm: rename functions that aren't IOCTL passthru

 drivers/acpi/nfit.c        |  91 ++++++++++++++++++++++++++++++++--
 drivers/nvdimm/bus.c       | 118 +++++++++++++++++++++++++++++++++++++++++----
 drivers/nvdimm/dimm_devs.c |   6 +--
 include/linux/libnvdimm.h  |   3 +-
 include/uapi/linux/ndctl.h |  20 +++++++-
 5 files changed, 220 insertions(+), 18 deletions(-)

-- 
1.7.11.3

[PATCH 1/4] nvdimm: Add wrapper for IOCTL pass thru.

[Jerry Hoemann]

Add IOCTL type 'P' to denote NVDIMM_TYPE_PASSTHRU.

Add struct ndn_pkg which the pass thru IOCTL interfaces uses.
ndn_pkg serves as a wrapper for the data being passed to the
underlying DSM and specifies siz data in a uniform manner allowing
the kernel to call the DSM without knowing specifics of the DSM.

Signed-off-by: Jerry Hoemann <jerry.hoemann@hpe.com>
---
 include/uapi/linux/ndctl.h | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/ndctl.h b/include/uapi/linux/ndctl.h
index 5b4a4be..1c81a99 100644
--- a/include/uapi/linux/ndctl.h
+++ b/include/uapi/linux/ndctl.h
@@ -15,6 +15,9 @@
 
 #include <linux/types.h>
 
+#define NVDIMM_TYPE_INTEL		'N'
+#define NVDIMM_TYPE_PASSTHRU		'P'
+
 struct nd_cmd_smart {
 	__u32 status;
 	__u8 data[128];
@@ -148,7 +151,8 @@ static inline const char *nvdimm_cmd_name(unsigned cmd)
 	return "unknown";
 }
 
-#define ND_IOCTL 'N'
+#define ND_IOCTL			NVDIMM_TYPE_INTEL
+
 
 #define ND_IOCTL_SMART			_IOWR(ND_IOCTL, ND_CMD_SMART,\
 					struct nd_cmd_smart)
@@ -204,4 +208,18 @@ enum ars_masks {
 	ARS_STATUS_MASK = 0x0000FFFF,
 	ARS_EXT_STATUS_SHIFT = 16,
 };
+
+
+struct ndn_pkg {
+	struct {
+		__u8	dsm_uuid[16];
+		__u32	dsm_in;			/* size of _DSM input    */
+		__u32	dsm_out;		/* size of user buffer   */
+		__u32	dsm_rev;		/* revision of dsm call  */
+		__u32	res[8];			/* reserved must be zero */
+		__u32	dsm_size;		/* size _DSM would write */
+	} h;
+	unsigned char buf[];
+} __packed;
+
 #endif /* __NDCTL_H__ */
-- 
1.7.11.3

[PATCH 2/4] nvdimm: Add IOCTL pass thru

[Jerry Hoemann]

Add internal data structure for ndctl_passthru call.

Signed-off-by: Jerry Hoemann <jerry.hoemann@hpe.com>
---
 include/linux/libnvdimm.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
index 3f021dc..01117e1 100644
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -72,6 +72,7 @@ struct nvdimm_bus_descriptor {
 	unsigned long dsm_mask;
 	char *provider_name;
 	ndctl_fn ndctl;
+	ndctl_fn ndctl_passthru;
 };
 
 struct nd_cmd_desc {
-- 
1.7.11.3

[PATCH 3/4] nvdimm: Add IOCTL pass thru

[Jerry Hoemann]

Add functions acpi_nfit_ctl_passthru and __nd_ioctl_passthru which allow
kernel to call a nvdimm's _DSM as a passthru without the marshaling code
of the nd_cmd_desc.

Signed-off-by: Jerry Hoemann <jerry.hoemann@hpe.com>
---
 drivers/acpi/nfit.c  |  85 +++++++++++++++++++++++++++++++++++++++++
 drivers/nvdimm/bus.c | 105 +++++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 186 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c
index c1b8d03..a6b458a 100644
--- a/drivers/acpi/nfit.c
+++ b/drivers/acpi/nfit.c
@@ -190,6 +190,90 @@ static int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
 	return rc;
 }
 
+
+static int acpi_nfit_ctl_passthru(struct nvdimm_bus_descriptor *nd_desc,
+		struct nvdimm *nvdimm, unsigned int cmd, void *buf,
+		unsigned int buf_len)
+{
+	struct acpi_nfit_desc *acpi_desc = to_acpi_nfit_desc(nd_desc);
+	union acpi_object in_obj, in_buf, *out_obj;
+	struct device *dev = acpi_desc->dev;
+	const char *dimm_name;
+	acpi_handle handle;
+	const u8 *uuid;
+	int rc = 0;
+	int rev = 0;
+
+	struct ndn_pkg *pkg = buf;
+
+	if (nvdimm) {
+		struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
+		struct acpi_device *adev = nfit_mem->adev;
+
+		if (!adev)
+			return -ENOTTY;
+		dimm_name = nvdimm_name(nvdimm);
+		handle = adev->handle;
+	} else {
+		struct acpi_device *adev = to_acpi_dev(acpi_desc);
+
+		handle = adev->handle;
+		dimm_name = "bus";
+	}
+	uuid = pkg->h.dsm_uuid;
+	rev  = pkg->h.dsm_rev ? pkg->h.dsm_rev : 1;
+
+	in_obj.type = ACPI_TYPE_PACKAGE;
+	in_obj.package.count = 1;
+	in_obj.package.elements = &in_buf;
+	in_buf.type = ACPI_TYPE_BUFFER;
+	in_buf.buffer.pointer = (void *) &pkg->buf;
+
+	in_buf.buffer.length = pkg->h.dsm_in;
+
+	if (IS_ENABLED(CONFIG_ACPI_NFIT_DEBUG)) {
+		dev_dbg(dev, "%s:%s cmd: %d input length: %d\n", __func__,
+				dimm_name, cmd, in_buf.buffer.length);
+		print_hex_dump_debug("cmd: ", DUMP_PREFIX_OFFSET, 4,
+				4, in_buf.buffer.pointer, min_t(u32, 128,
+					in_buf.buffer.length), true);
+	}
+
+	out_obj = acpi_evaluate_dsm(handle, uuid, rev, cmd, &in_obj);
+	if (!out_obj) {
+		dev_dbg(dev, "%s:%s _DSM failed cmd: %d\n", __func__, dimm_name,
+				cmd);
+		return -EINVAL;
+	}
+
+	if (out_obj->package.type != ACPI_TYPE_BUFFER) {
+		dev_dbg(dev, "%s:%s unexpected output object type cmd: %d type: %d\n",
+				__func__, dimm_name, cmd, out_obj->type);
+		rc = -EINVAL;
+		goto out;
+	}
+
+	if (IS_ENABLED(CONFIG_ACPI_NFIT_DEBUG)) {
+		dev_dbg(dev, "%s:%s cmd: %d output length: %d\n", __func__,
+				dimm_name, cmd, out_obj->buffer.length);
+		print_hex_dump_debug("cmd: ", DUMP_PREFIX_OFFSET, 4,
+				4, out_obj->buffer.pointer, min_t(u32, 128,
+					out_obj->buffer.length), true);
+	}
+
+	pkg->h.dsm_size = out_obj->buffer.length;
+	memcpy(pkg->buf + pkg->h.dsm_in,
+			out_obj->buffer.pointer,
+			min(pkg->h.dsm_size, pkg->h.dsm_out));
+
+
+ out:
+	ACPI_FREE(out_obj);
+
+	return rc;
+}
+
+
 static const char *spa_type_name(u16 type)
 {
 	static const char *to_name[] = {
@@ -1614,6 +1698,7 @@ static int acpi_nfit_add(struct acpi_device *adev)
 	nd_desc = &acpi_desc->nd_desc;
 	nd_desc->provider_name = "ACPI.NFIT";
 	nd_desc->ndctl = acpi_nfit_ctl;
+	nd_desc->ndctl_passthru = acpi_nfit_ctl_passthru;
 	nd_desc->attr_groups = acpi_nfit_attribute_groups;
 
 	acpi_desc->nvdimm_bus = nvdimm_bus_register(dev, nd_desc);
diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c
index 7e2c43f..cfb10eb 100644
--- a/drivers/nvdimm/bus.c
+++ b/drivers/nvdimm/bus.c
@@ -599,18 +599,103 @@ static int __nd_ioctl(struct nvdimm_bus *nvdimm_bus, struct nvdimm *nvdimm,
 	return rc;
 }
 
+
+static int __nd_ioctl_passthru(struct nvdimm_bus *nvdimm_bus,
+		struct nvdimm *nvdimm, int read_only, unsigned
+		int ioctl_cmd, unsigned long arg)
+{
+	struct nvdimm_bus_descriptor *nd_desc = nvdimm_bus->nd_desc;
+	size_t buf_len = 0, in_len = 0, out_len = 0;
+	unsigned int cmd = _IOC_NR(ioctl_cmd);
+	unsigned int size = _IOC_SIZE(ioctl_cmd);
+	void __user *p = (void __user *) arg;
+	struct device *dev = &nvdimm_bus->dev;
+	const char *dimm_name = "";
+	void *buf = NULL;
+	int i, rc;
+	struct ndn_pkg pkg;
+
+	if (nvdimm)
+		dimm_name = dev_name(&nvdimm->dev);
+	else
+		dimm_name = "bus";
+
+	if (copy_from_user(&pkg, p, sizeof(pkg))) {
+		rc = -EFAULT;
+		goto out;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(pkg.h.res); i++)
+		if (pkg.h.res[i])
+			return -EINVAL;
+
+	/* Caller must tell us size of input to _DSM. */
+	/* This may be bigger that the fixed portion of the pakcage */
+	in_len  = pkg.h.dsm_in;
+	out_len = pkg.h.dsm_out;
+	buf_len = sizeof(pkg.h) + in_len + out_len;
+
+
+	dev_dbg(dev, "%s:%s cmd: %d, size: %d, in: %zu, out: %zu len: %zu\n",
+				__func__,
+				dimm_name, cmd, size,
+				in_len, out_len, buf_len);
+
+	if (buf_len > ND_IOCTL_MAX_BUFLEN) {
+		dev_dbg(dev, "%s:%s cmd: %d buf_len: %zu > %d\n", __func__,
+				dimm_name, cmd, buf_len,
+				ND_IOCTL_MAX_BUFLEN);
+		return -EINVAL;
+	}
+
+	buf = vmalloc(buf_len);
+	if (!buf)
+		return -ENOMEM;
+
+	if (copy_from_user(buf, p, buf_len)) {
+		rc = -EFAULT;
+		goto out;
+	}
+
+	nvdimm_bus_lock(&nvdimm_bus->dev);
+	rc = nd_cmd_clear_to_send(nvdimm, cmd);
+	if (rc)
+		goto out_unlock;
+
+	rc = nd_desc->ndctl_passthru(nd_desc, nvdimm, cmd, buf, buf_len);
+	if (rc < 0)
+		goto out_unlock;
+	if (copy_to_user(p, buf, buf_len))
+		rc = -EFAULT;
+ out_unlock:
+	nvdimm_bus_unlock(&nvdimm_bus->dev);
+ out:
+	vfree(buf);
+	return rc;
+}
+
 static long nd_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
 	long id = (long) file->private_data;
 	int rc = -ENXIO, read_only;
 	struct nvdimm_bus *nvdimm_bus;
+	unsigned int type = _IOC_TYPE(cmd);
 
 	read_only = (O_RDWR != (file->f_flags & O_ACCMODE));
 	mutex_lock(&nvdimm_bus_list_mutex);
 	list_for_each_entry(nvdimm_bus, &nvdimm_bus_list, list) {
-		if (nvdimm_bus->id == id) {
+		if (nvdimm_bus->id != id)
+			continue;
+
+		switch (type) {
+		case NVDIMM_TYPE_INTEL:
 			rc = __nd_ioctl(nvdimm_bus, NULL, read_only, cmd, arg);
 			break;
+		case NVDIMM_TYPE_PASSTHRU:
+			rc = __nd_ioctl_passthru(nvdimm_bus, NULL, 0, cmd, arg);
+			break;
+		default:
+			rc = -ENOTTY;
 		}
 	}
 	mutex_unlock(&nvdimm_bus_list_mutex);
@@ -633,10 +718,11 @@ static int match_dimm(struct device *dev, void *data)
 
 static long nvdimm_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
-	int rc = -ENXIO, read_only;
+	int rc = -ENXIO, ro;
 	struct nvdimm_bus *nvdimm_bus;
+	unsigned int type = _IOC_TYPE(cmd);
 
-	read_only = (O_RDWR != (file->f_flags & O_ACCMODE));
+	ro = (O_RDWR != (file->f_flags & O_ACCMODE));
 	mutex_lock(&nvdimm_bus_list_mutex);
 	list_for_each_entry(nvdimm_bus, &nvdimm_bus_list, list) {
 		struct device *dev = device_find_child(&nvdimm_bus->dev,
@@ -647,7 +733,18 @@ static long nvdimm_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 			continue;
 
 		nvdimm = to_nvdimm(dev);
-		rc = __nd_ioctl(nvdimm_bus, nvdimm, read_only, cmd, arg);
+
+		switch (type) {
+		case NVDIMM_TYPE_INTEL:
+			rc = __nd_ioctl(nvdimm_bus, nvdimm, ro, cmd, arg);
+			break;
+		case NVDIMM_TYPE_PASSTHRU:
+			rc = __nd_ioctl_passthru(nvdimm_bus, nvdimm, ro, cmd, arg);
+			break;
+		default:
+			rc = -ENOTTY;
+		}
+
 		put_device(dev);
 		break;
 	}
-- 
1.7.11.3

[PATCH 4/4] nvdimm: rename functions that aren't IOCTL passthru

[Jerry Hoemann]

rename functions acpi_nfit_ctl, __nd_ioctl, ndctl, to *_intel to denote
that the functions implement the Intel example _DSM.

Signed-off-by: Jerry Hoemann <jerry.hoemann@hpe.com>
---
 drivers/acpi/nfit.c        |  6 +++---
 drivers/nvdimm/bus.c       | 15 ++++++++-------
 drivers/nvdimm/dimm_devs.c |  6 +++---
 include/linux/libnvdimm.h  |  2 +-
 4 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c
index a6b458a..f85200d 100644
--- a/drivers/acpi/nfit.c
+++ b/drivers/acpi/nfit.c
@@ -62,7 +62,7 @@ static struct acpi_device *to_acpi_dev(struct acpi_nfit_desc *acpi_desc)
 	return to_acpi_device(acpi_desc->dev);
 }
 
-static int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
+static int acpi_nfit_ctl_intel(struct nvdimm_bus_descriptor *nd_desc,
 		struct nvdimm *nvdimm, unsigned int cmd, void *buf,
 		unsigned int buf_len)
 {
@@ -1352,7 +1352,7 @@ static int acpi_nfit_blk_get_flags(struct nvdimm_bus_descriptor *nd_desc,
 	int rc;
 
 	memset(&flags, 0, sizeof(flags));
-	rc = nd_desc->ndctl(nd_desc, nvdimm, ND_CMD_DIMM_FLAGS, &flags,
+	rc = nd_desc->ndctl_intel(nd_desc, nvdimm, ND_CMD_DIMM_FLAGS, &flags,
 			sizeof(flags));
 
 	if (rc >= 0 && flags.status == 0)
@@ -1697,7 +1697,7 @@ static int acpi_nfit_add(struct acpi_device *adev)
 	acpi_desc->blk_do_io = acpi_nfit_blk_region_do_io;
 	nd_desc = &acpi_desc->nd_desc;
 	nd_desc->provider_name = "ACPI.NFIT";
-	nd_desc->ndctl = acpi_nfit_ctl;
+	nd_desc->ndctl_intel = acpi_nfit_ctl_intel;
 	nd_desc->ndctl_passthru = acpi_nfit_ctl_passthru;
 	nd_desc->attr_groups = acpi_nfit_attribute_groups;
 
diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c
index cfb10eb..4587d30 100644
--- a/drivers/nvdimm/bus.c
+++ b/drivers/nvdimm/bus.c
@@ -479,8 +479,9 @@ static int nd_cmd_clear_to_send(struct nvdimm *nvdimm, unsigned int cmd)
 	return 0;
 }
 
-static int __nd_ioctl(struct nvdimm_bus *nvdimm_bus, struct nvdimm *nvdimm,
-		int read_only, unsigned int ioctl_cmd, unsigned long arg)
+static int __nd_ioctl_intel(struct nvdimm_bus *nvdimm_bus,
+		struct nvdimm *nvdimm, int read_only, unsigned int ioctl_cmd,
+		unsigned long arg)
 {
 	struct nvdimm_bus_descriptor *nd_desc = nvdimm_bus->nd_desc;
 	size_t buf_len = 0, in_len = 0, out_len = 0;
@@ -587,7 +588,7 @@ static int __nd_ioctl(struct nvdimm_bus *nvdimm_bus, struct nvdimm *nvdimm,
 	if (rc)
 		goto out_unlock;
 
-	rc = nd_desc->ndctl(nd_desc, nvdimm, cmd, buf, buf_len);
+	rc = nd_desc->ndctl_intel(nd_desc, nvdimm, cmd, buf, buf_len);
 	if (rc < 0)
 		goto out_unlock;
 	if (copy_to_user(p, buf, buf_len))
@@ -677,11 +678,11 @@ static int __nd_ioctl_passthru(struct nvdimm_bus *nvdimm_bus,
 static long nd_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
 	long id = (long) file->private_data;
-	int rc = -ENXIO, read_only;
+	int rc = -ENXIO, ro;
 	struct nvdimm_bus *nvdimm_bus;
 	unsigned int type = _IOC_TYPE(cmd);
 
-	read_only = (O_RDWR != (file->f_flags & O_ACCMODE));
+	ro = (O_RDWR != (file->f_flags & O_ACCMODE));
 	mutex_lock(&nvdimm_bus_list_mutex);
 	list_for_each_entry(nvdimm_bus, &nvdimm_bus_list, list) {
 		if (nvdimm_bus->id != id)
@@ -689,7 +690,7 @@ static long nd_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 
 		switch (type) {
 		case NVDIMM_TYPE_INTEL:
-			rc = __nd_ioctl(nvdimm_bus, NULL, read_only, cmd, arg);
+			rc = __nd_ioctl_intel(nvdimm_bus, NULL, ro, cmd, arg);
 			break;
 		case NVDIMM_TYPE_PASSTHRU:
 			rc = __nd_ioctl_passthru(nvdimm_bus, NULL, 0, cmd, arg);
@@ -736,7 +737,7 @@ static long nvdimm_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 
 		switch (type) {
 		case NVDIMM_TYPE_INTEL:
-			rc = __nd_ioctl(nvdimm_bus, nvdimm, ro, cmd, arg);
+			rc = __nd_ioctl_intel(nvdimm_bus, nvdimm, ro, cmd, arg);
 			break;
 		case NVDIMM_TYPE_PASSTHRU:
 			rc = __nd_ioctl_passthru(nvdimm_bus, nvdimm, ro, cmd, arg);
diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
index 651b8d1..9c0cc23 100644
--- a/drivers/nvdimm/dimm_devs.c
+++ b/drivers/nvdimm/dimm_devs.c
@@ -74,7 +74,7 @@ int nvdimm_init_nsarea(struct nvdimm_drvdata *ndd)
 
 	memset(cmd, 0, sizeof(*cmd));
 	nd_desc = nvdimm_bus->nd_desc;
-	return nd_desc->ndctl(nd_desc, to_nvdimm(ndd->dev),
+	return nd_desc->ndctl_intel(nd_desc, to_nvdimm(ndd->dev),
 			ND_CMD_GET_CONFIG_SIZE, cmd, sizeof(*cmd));
 }
 
@@ -118,7 +118,7 @@ int nvdimm_init_config_data(struct nvdimm_drvdata *ndd)
 			offset += cmd->in_length) {
 		cmd->in_length = min(config_size, max_cmd_size);
 		cmd->in_offset = offset;
-		rc = nd_desc->ndctl(nd_desc, to_nvdimm(ndd->dev),
+		rc = nd_desc->ndctl_intel(nd_desc, to_nvdimm(ndd->dev),
 				ND_CMD_GET_CONFIG_DATA, cmd,
 				cmd->in_length + sizeof(*cmd));
 		if (rc || cmd->status) {
@@ -170,7 +170,7 @@ int nvdimm_set_config_data(struct nvdimm_drvdata *ndd, size_t offset,
 		cmd_size = sizeof(*cmd) + cmd->in_length + sizeof(u32);
 		status = ((void *) cmd) + cmd_size - sizeof(u32);
 
-		rc = nd_desc->ndctl(nd_desc, to_nvdimm(ndd->dev),
+		rc = nd_desc->ndctl_intel(nd_desc, to_nvdimm(ndd->dev),
 				ND_CMD_SET_CONFIG_DATA, cmd, cmd_size);
 		if (rc || *status) {
 			rc = rc ? rc : -ENXIO;
diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
index 01117e1..594a772 100644
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -71,7 +71,7 @@ struct nvdimm_bus_descriptor {
 	const struct attribute_group **attr_groups;
 	unsigned long dsm_mask;
 	char *provider_name;
-	ndctl_fn ndctl;
+	ndctl_fn ndctl_intel;
 	ndctl_fn ndctl_passthru;
 };
 
-- 
1.7.11.3

RE: [PATCH 1/4] nvdimm: Add wrapper for IOCTL pass thru.

[Elliott, Robert (Persistent Memory)]

> -----Original Message-----
> From: Linux-nvdimm [mailto:linux-nvdimm-bounces@lists.01.org] On Behalf Of
> Hoemann, Jerry
> Sent: Friday, November 6, 2015 4:27 PM
> Subject: [PATCH 1/4] nvdimm: Add wrapper for IOCTL pass thru.
...
> diff --git a/include/uapi/linux/ndctl.h b/include/uapi/linux/ndctl.h
...
> +struct ndn_pkg {
> +	struct {
> +		__u8	dsm_uuid[16];
> +		__u32	dsm_in;			/* size of _DSM input    */
> +		__u32	dsm_out;		/* size of user buffer   */
> +		__u32	dsm_rev;		/* revision of dsm call  */
> +		__u32	res[8];			/* reserved must be zero */
> +		__u32	dsm_size;		/* size _DSM would write */
> +	} h;
> +	unsigned char buf[];
> +} __packed;

Given that the _DSM arguments are defined as:
* Arg0 UUID: Buffer of 16 bytes
* Arg1 Revision ID: Integer (8 bytes)
* Arg2 Function Index: Integer (8 bytes)
* Arg3 Package: function-specific

1. The __u32 for dsm_rev is not big enough to express all
possible 8 byte Revision IDs.

2. The unsigned int cmd (carried outside this structure)
is not big enough on all platforms (e.g., 32-bit) to 
express all possible Function Indexes.

3. The Revision ID and Function Index values passed to 
the _DSM are defined as little-endian.  Are they
intended to use native endianness or be little-endian
in this structure?

---
Robert Elliott, HPE Persistent Memory

Re: [PATCH 1/4] nvdimm: Add wrapper for IOCTL pass thru.

[Jerry Hoemann]

On Tue, Nov 10, 2015 at 12:04:22PM -0700, Elliott, Robert (Persistent Memory) wrote:
> > -----Original Message-----
> > From: Linux-nvdimm [mailto:linux-nvdimm-bounces@lists.01.org] On Behalf Of
> > Hoemann, Jerry
> > Sent: Friday, November 6, 2015 4:27 PM
> > Subject: [PATCH 1/4] nvdimm: Add wrapper for IOCTL pass thru.
> ...
> > diff --git a/include/uapi/linux/ndctl.h b/include/uapi/linux/ndctl.h
> ...
> > +struct ndn_pkg {
> > +	struct {
> > +		__u8	dsm_uuid[16];
> > +		__u32	dsm_in;			/* size of _DSM input    */
> > +		__u32	dsm_out;		/* size of user buffer   */
> > +		__u32	dsm_rev;		/* revision of dsm call  */
> > +		__u32	res[8];			/* reserved must be zero */
> > +		__u32	dsm_size;		/* size _DSM would write */
> > +	} h;
> > +	unsigned char buf[];
> > +} __packed;
> 
> Given that the _DSM arguments are defined as:
> * Arg0 UUID: Buffer of 16 bytes
> * Arg1 Revision ID: Integer (8 bytes)
> * Arg2 Function Index: Integer (8 bytes)
> * Arg3 Package: function-specific
> 
> 1. The __u32 for dsm_rev is not big enough to express all
> possible 8 byte Revision IDs.
> 
> 2. The unsigned int cmd (carried outside this structure)
> is not big enough on all platforms (e.g., 32-bit) to 
> express all possible Function Indexes.
> 
> 3. The Revision ID and Function Index values passed to 
> the _DSM are defined as little-endian.  Are they
> intended to use native endianness or be little-endian
> in this structure?
> 

  Thanks, Robert.  I will look at these for version 2.

  Jerry
-- 

-----------------------------------------------------------------------------
Jerry Hoemann            Software Engineer      Hewlett-Packard Enterprise

3404 E Harmony Rd. MS 36                        phone:  (970) 898-1022
Ft. Collins, CO 80528                           FAX:    (970) 898-0707
                                                email:  jerry.hoemann@hpe.com
-----------------------------------------------------------------------------