Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1290785 > unrolled thread

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

Started byJerry Hoemann <jerry.hoemann@hpe.com>
First post2015-12-14 03:00 +0100
Last post2015-12-14 18:40 +0100
Articles 3 — 1 participant

Back to article view | Back to linux.kernel

Contents

  [PATCH v3 0/6] nvdimm: Add an IOCTL pass thru for DSM calls Jerry Hoemann <jerry.hoemann@hpe.com> - 2015-12-14 03:00 +0100
    [PATCH v3 6/6] nvdimm: Add IOCTL pass thru functions Jerry Hoemann <jerry.hoemann@hpe.com> - 2015-12-14 03:00 +0100
    Re: [PATCH v4 0/6] nvdimm: Add an IOCTL pass thru for DSM calls Jerry Hoemann <jerry.hoemann@hpe.com> - 2015-12-14 18:40 +0100

#1290785 — [PATCH v3 0/6] nvdimm: Add an IOCTL pass thru for DSM calls

FromJerry Hoemann <jerry.hoemann@hpe.com>
Date2015-12-14 03:00 +0100
Subject[PATCH v3 0/6] nvdimm: Add an IOCTL pass thru for DSM calls
Message-ID<qFqAF-7R7-7@gated-at.bofh.it>
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 _IOC_NR ND_CMD_CALL_DSM == "10" is added for the pass thru call.

The new data structure nd_cmd_dsmcall_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 nd_cmd_dsmcall_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 accomodate ND_CMD_CALL_DSM.

These changes are based upon the 4.3 kernel.


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 quanties 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 rearange 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 accomodate.

  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 intergers 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 (6):
  acpica: Correct parameter type to acpi_evaluate_dsm
  nvdimm: Clean-up access mode check.
  nvdimm: Add wrapper for IOCTL pass thru
  nvdimm: Fix security issue with DSM IOCTL.
  nvdimm: Increase max envelope size for IOCTL
  nvdimm: Add IOCTL pass thru functions

 drivers/acpi/nfit.c        | 52 ++++++++++++++++++++++++++---------
 drivers/acpi/utils.c       |  4 +--
 drivers/nvdimm/bus.c       | 67 +++++++++++++++++++++++++++++++++++++---------
 include/acpi/acpi_bus.h    |  6 ++---
 include/linux/libnvdimm.h  |  2 +-
 include/uapi/linux/ndctl.h | 19 +++++++++++++
 6 files changed, 118 insertions(+), 32 deletions(-)

-- 
1.7.11.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1290789 — [PATCH v3 6/6] nvdimm: Add IOCTL pass thru functions

FromJerry Hoemann <jerry.hoemann@hpe.com>
Date2015-12-14 03:00 +0100
Subject[PATCH v3 6/6] nvdimm: Add IOCTL pass thru functions
Message-ID<qFqAG-7R7-23@gated-at.bofh.it>
In reply to#1290785
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.

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

diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c
index c1b8d03..8a8ec3f 100644
--- a/drivers/acpi/nfit.c
+++ b/drivers/acpi/nfit.c
@@ -76,6 +76,10 @@ static int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
 	const u8 *uuid;
 	u32 offset;
 	int rc, i;
+	__u64 rev = 1, func = cmd;
+
+	struct nd_cmd_dsmcall_pkg *pkg = buf;
+	int dsm_call = (cmd == ND_CMD_CALL_DSM);
 
 	if (nvdimm) {
 		struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
@@ -99,6 +103,8 @@ static int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
 		handle = adev->handle;
 		dimm_name = "bus";
 	}
+	if (dsm_call)
+		dsm_mask = ~0UL;
 
 	if (!desc || (cmd && (desc->out_num + desc->in_num == 0)))
 		return -ENOTTY;
@@ -118,15 +124,25 @@ 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 (dsm_call) {
+		/* must skip over package wrapper */
+		in_buf.buffer.pointer = (void *) &pkg->dsm_buf;
+		in_buf.buffer.length = pkg->h.dsm_in;
+		/* for pass thru must use value sent in from user space. */
+		uuid = pkg->h.dsm_uuid;
+		rev  = pkg->h.dsm_rev;
+		func = pkg->h.dsm_fun_idx;
+	}
+
 	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);
@@ -134,18 +150,28 @@ static int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
 	}
 
 	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);
+		dev_dbg(dev, "%s:%s unexpected output object type cmd: %s %llu, type: %d\n",
+			__func__, dimm_name, cmd_name, func, out_obj->type);
 		rc = -EINVAL;
 		goto out;
 	}
 
 	if (IS_ENABLED(CONFIG_ACPI_NFIT_DEBUG)) {
-		dev_dbg(dev, "%s:%s cmd: %s output length: %d\n", __func__,
-				dimm_name, cmd_name, out_obj->buffer.length);
-		print_hex_dump_debug(cmd_name, DUMP_PREFIX_OFFSET, 4,
-				4, out_obj->buffer.pointer, min_t(u32, 128,
-					out_obj->buffer.length), true);
+		dev_dbg(dev, "%s:%s cmd %d: %llu output length %d\n", __func__,
+				dimm_name, cmd, func, out_obj->buffer.length);
+		print_hex_dump_debug("nvdimm out ", DUMP_PREFIX_OFFSET, 4, 4,
+			out_obj->buffer.pointer,
+			min_t(u32, 256, out_obj->buffer.length), true);
+	}
+
+	if (dsm_call) {
+		pkg->h.dsm_size = out_obj->buffer.length;
+		memcpy(pkg->dsm_buf + pkg->h.dsm_in,
+			out_obj->buffer.pointer,
+			min(pkg->h.dsm_size, pkg->h.dsm_out));
+
+		ACPI_FREE(out_obj);
+		return 0;
 	}
 
 	for (i = 0, offset = 0; i < desc->out_num; i++) {
diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c
index 87fe545..8d3a64b 100644
--- a/drivers/nvdimm/bus.c
+++ b/drivers/nvdimm/bus.c
@@ -367,6 +367,12 @@ static const struct nd_cmd_desc __nd_cmd_dimm_descs[] = {
 		.out_num = 3,
 		.out_sizes = { 4, 4, UINT_MAX, },
 	},
+	[ND_CMD_CALL_DSM] = {
+		.in_num = 2,
+		.in_sizes = {sizeof(struct nd_cmd_dsmcall_pkg), UINT_MAX, },
+		.out_num = 1,
+		.out_sizes = { UINT_MAX, },
+	},
 };
 
 const struct nd_cmd_desc *nd_cmd_dimm_desc(int cmd)
@@ -395,6 +401,12 @@ static const struct nd_cmd_desc __nd_cmd_bus_descs[] = {
 		.out_num = 2,
 		.out_sizes = { 4, UINT_MAX, },
 	},
+	[ND_CMD_CALL_DSM] = {
+		.in_num = 2,
+		.in_sizes = {sizeof(struct nd_cmd_dsmcall_pkg), UINT_MAX, },
+		.out_num = 1,
+		.out_sizes = { UINT_MAX, },
+	},
 };
 
 const struct nd_cmd_desc *nd_cmd_bus_desc(int cmd)
@@ -422,6 +434,10 @@ u32 nd_cmd_in_size(struct nvdimm *nvdimm, int cmd,
 		struct nd_cmd_vendor_hdr *hdr = buf;
 
 		return hdr->in_length;
+	} else if (cmd == ND_CMD_CALL_DSM) {
+		struct nd_cmd_dsmcall_pkg *pkg = buf;
+
+		return pkg->h.dsm_in;
 	}
 
 	return UINT_MAX;
@@ -444,6 +460,12 @@ u32 nd_cmd_out_size(struct nvdimm *nvdimm, int cmd,
 		return out_field[1];
 	else if (!nvdimm && cmd == ND_CMD_ARS_STATUS && idx == 1)
 		return ND_CMD_ARS_STATUS_MAX;
+	else if (cmd == ND_CMD_CALL_DSM) {
+		struct nd_cmd_dsmcall_pkg *pkg =
+				(struct nd_cmd_dsmcall_pkg *) in_field;
+		return pkg->h.dsm_out;
+	}
+
 
 	return UINT_MAX;
 }
@@ -491,19 +513,24 @@ static int __nd_ioctl(struct nvdimm_bus *nvdimm_bus, struct nvdimm *nvdimm,
 	void __user *p = (void __user *) arg;
 	struct device *dev = &nvdimm_bus->dev;
 	const char *cmd_name, *dimm_name;
-	unsigned long dsm_mask;
+	unsigned long dsm_mask = ~0UL;
 	void *buf;
 	int rc, i;
 
+	struct nd_cmd_dsmcall_pkg *pkg;
+	int dsm_call = (cmd == ND_CMD_CALL_DSM);
+
 	if (nvdimm) {
 		desc = nd_cmd_dimm_desc(cmd);
 		cmd_name = nvdimm_cmd_name(cmd);
-		dsm_mask = nvdimm->dsm_mask ? *(nvdimm->dsm_mask) : 0;
+		if (!dsm_call)
+			dsm_mask = nvdimm->dsm_mask ? *(nvdimm->dsm_mask) : 0;
 		dimm_name = dev_name(&nvdimm->dev);
 	} else {
 		desc = nd_cmd_bus_desc(cmd);
 		cmd_name = nvdimm_bus_cmd_name(cmd);
-		dsm_mask = nd_desc->dsm_mask;
+		if (!dsm_call)
+			dsm_mask = nd_desc->dsm_mask;
 		dimm_name = "bus";
 	}
 
@@ -517,6 +544,7 @@ static int __nd_ioctl(struct nvdimm_bus *nvdimm_bus, struct nvdimm *nvdimm,
 		case ND_CMD_VENDOR:
 		case ND_CMD_SET_CONFIG_DATA:
 		case ND_CMD_ARS_START:
+		case ND_CMD_CALL_DSM:
 			dev_dbg(&nvdimm_bus->dev, "'%s' command while read-only.\n",
 					nvdimm ? nvdimm_cmd_name(cmd)
 					: nvdimm_bus_cmd_name(cmd));
@@ -544,6 +572,19 @@ static int __nd_ioctl(struct nvdimm_bus *nvdimm_bus, struct nvdimm *nvdimm,
 		in_len += in_size;
 	}
 
+	if (dsm_call) {
+		pkg = (struct nd_cmd_dsmcall_pkg *) in_env;
+
+		dev_dbg(dev, "%s:%s rev: %llu, idx: %llu, in: %zu, out: %zu, len %zu\n",
+			__func__, dimm_name,
+			pkg->h.dsm_rev, pkg->h.dsm_fun_idx,
+			in_len, out_len, buf_len);
+
+		for (i = 0; i < ARRAY_SIZE(pkg->h.reserved2); i++)
+			if (pkg->h.reserved2[i])
+				return -EINVAL;
+	}
+
 	/* process an output envelope */
 	for (i = 0; i < desc->out_num; i++) {
 		u32 out_size = nd_cmd_out_size(nvdimm, cmd, desc, i,
-- 
1.7.11.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1291368 — Re: [PATCH v4 0/6] nvdimm: Add an IOCTL pass thru for DSM calls

FromJerry Hoemann <jerry.hoemann@hpe.com>
Date2015-12-14 18:40 +0100
SubjectRe: [PATCH v4 0/6] nvdimm: Add an IOCTL pass thru for DSM calls
Message-ID<qFFgn-Lk-31@gated-at.bofh.it>
In reply to#1290785
On Sun, Dec 13, 2015 at 06:54:21PM -0700, Jerry Hoemann wrote:
> 
> 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.
> 

  ...


> 
> Changes in version 4:
> ---------------------

...


Sorry, forgot to update version number in the email title. 

This is version 4 of this patch series.



-- 

-----------------------------------------------------------------------------
Jerry Hoemann                  Software Engineer   Hewlett Packard Enterprise
-----------------------------------------------------------------------------
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web