* [PATCH v8 00/10] nvdimm: Add an IOCTL pass thru for DSM calls
@ 2016-03-21 19:37 Jerry Hoemann
2016-03-21 19:37 ` [PATCH v8 01/10] ACPI / util: Fix acpi_evaluate_dsm() argument type Jerry Hoemann
` (11 more replies)
0 siblings, 12 replies; 29+ messages in thread
From: Jerry Hoemann @ 2016-03-21 19:37 UTC (permalink / raw)
To: dan.j.williams; +Cc: linux-nvdimm
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 == "10" 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 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 the Intel Example DSM supports 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 (10):
ACPI / util: Fix acpi_evaluate_dsm() argument type
nvdimm: Add wrapper for IOCTL pass thru
nvdimm: Increase max envelope size for IOCTL
nvdimm: Add UUIDs
nvdimm: Add IOCTL pass thru functions
libnvdimm: nvdimm_bus_descriptor field name change
tools/testing/nvdimm: 'call_dsm' support
nvdimm: command ioctl support
nvdimm: sysfs shows which dsm support full command ioctl.
nvdimm: Add ioctl to return command mask.
drivers/acpi/nfit.c | 154 +++++++++++++++++++++++++++++++++------
drivers/acpi/nfit.h | 6 ++
drivers/acpi/utils.c | 4 +-
drivers/nvdimm/bus.c | 55 +++++++++++++-
drivers/nvdimm/core.c | 3 +-
drivers/nvdimm/dimm_devs.c | 12 ++-
drivers/nvdimm/nd-core.h | 1 +
include/acpi/acpi_bus.h | 6 +-
include/linux/libnvdimm.h | 6 +-
include/uapi/linux/ndctl.h | 62 ++++++++++++++++
tools/testing/nvdimm/test/nfit.c | 15 +++-
11 files changed, 283 insertions(+), 41 deletions(-)
--
1.7.11.3
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v8 01/10] ACPI / util: Fix acpi_evaluate_dsm() argument type
2016-03-21 19:37 [PATCH v8 00/10] nvdimm: Add an IOCTL pass thru for DSM calls Jerry Hoemann
@ 2016-03-21 19:37 ` Jerry Hoemann
2016-03-21 19:37 ` [PATCH v8 02/10] nvdimm: Add wrapper for IOCTL pass thru Jerry Hoemann
` (10 subsequent siblings)
11 siblings, 0 replies; 29+ messages in thread
From: Jerry Hoemann @ 2016-03-21 19:37 UTC (permalink / raw)
To: dan.j.williams; +Cc: linux-nvdimm
The ACPI spec speicifies that arguments "Revision ID" and
"Function Index" to a _DSM are type "Integer." Type Integers
are 64 bit quantities.
The function evaluate_dsm specifies these types as simple "int"
which are 32 bits. Correct type passed to acpi_evaluate_dsm
and its callers and derived callers to pass correct type.
acpi_check_dsm and acpi_evaluate_dsm_typed had similar issue
and were corrected as well.
Signed-off-by: Jerry Hoemann <jerry.hoemann@hpe.com>
---
drivers/acpi/utils.c | 4 ++--
include/acpi/acpi_bus.h | 6 +++---
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
index f2f9873..87bd54d 100644
--- a/drivers/acpi/utils.c
+++ b/drivers/acpi/utils.c
@@ -629,7 +629,7 @@ acpi_status acpi_evaluate_lck(acpi_handle handle, int lock)
* some old BIOSes do expect a buffer or an integer etc.
*/
union acpi_object *
-acpi_evaluate_dsm(acpi_handle handle, const u8 *uuid, int rev, int func,
+acpi_evaluate_dsm(acpi_handle handle, const u8 *uuid, u64 rev, u64 func,
union acpi_object *argv4)
{
acpi_status ret;
@@ -678,7 +678,7 @@ EXPORT_SYMBOL(acpi_evaluate_dsm);
* functions. Currently only support 64 functions at maximum, should be
* enough for now.
*/
-bool acpi_check_dsm(acpi_handle handle, const u8 *uuid, int rev, u64 funcs)
+bool acpi_check_dsm(acpi_handle handle, const u8 *uuid, u64 rev, u64 funcs)
{
int i;
u64 mask = 0;
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 14362a8..f092cc6 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -61,12 +61,12 @@ bool acpi_ata_match(acpi_handle handle);
bool acpi_bay_match(acpi_handle handle);
bool acpi_dock_match(acpi_handle handle);
-bool acpi_check_dsm(acpi_handle handle, const u8 *uuid, int rev, u64 funcs);
+bool acpi_check_dsm(acpi_handle handle, const u8 *uuid, u64 rev, u64 funcs);
union acpi_object *acpi_evaluate_dsm(acpi_handle handle, const u8 *uuid,
- int rev, int func, union acpi_object *argv4);
+ u64 rev, u64 func, union acpi_object *argv4);
static inline union acpi_object *
-acpi_evaluate_dsm_typed(acpi_handle handle, const u8 *uuid, int rev, int func,
+acpi_evaluate_dsm_typed(acpi_handle handle, const u8 *uuid, u64 rev, u64 func,
union acpi_object *argv4, acpi_object_type type)
{
union acpi_object *obj;
--
1.7.11.3
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v8 02/10] nvdimm: Add wrapper for IOCTL pass thru
2016-03-21 19:37 [PATCH v8 00/10] nvdimm: Add an IOCTL pass thru for DSM calls Jerry Hoemann
2016-03-21 19:37 ` [PATCH v8 01/10] ACPI / util: Fix acpi_evaluate_dsm() argument type Jerry Hoemann
@ 2016-03-21 19:37 ` Jerry Hoemann
2016-04-11 18:21 ` Dan Williams
2016-03-21 19:37 ` [PATCH v8 03/10] nvdimm: Increase max envelope size for IOCTL Jerry Hoemann
` (9 subsequent siblings)
11 siblings, 1 reply; 29+ messages in thread
From: Jerry Hoemann @ 2016-03-21 19:37 UTC (permalink / raw)
To: dan.j.williams; +Cc: linux-nvdimm
Add struct nd_cmd_pkg which serves as a warapper for
the data being passed via a pass thru to a NVDIMM DSM.
This wrapper specifies the extra information in a uniform
manner allowing the kenrel to call a DSM without knowing
specifics of the DSM.
Add dsm_call command to nvdimm_bus_cmd_name and nvdimm_cmd_name.
Signed-off-by: Jerry Hoemann <jerry.hoemann@hpe.com>
---
include/uapi/linux/ndctl.h | 53 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/include/uapi/linux/ndctl.h b/include/uapi/linux/ndctl.h
index 7cc28ab..c043cd1 100644
--- a/include/uapi/linux/ndctl.h
+++ b/include/uapi/linux/ndctl.h
@@ -114,6 +114,7 @@ enum {
ND_CMD_ARS_START = 2,
ND_CMD_ARS_STATUS = 3,
ND_CMD_CLEAR_ERROR = 4,
+ /* 10 reserved for ND_CMD_CALL below */
/* per-dimm commands */
ND_CMD_SMART = 1,
@@ -125,6 +126,7 @@ enum {
ND_CMD_VENDOR_EFFECT_LOG_SIZE = 7,
ND_CMD_VENDOR_EFFECT_LOG = 8,
ND_CMD_VENDOR = 9,
+ ND_CMD_CALL = 10,
};
enum {
@@ -139,6 +141,7 @@ static inline const char *nvdimm_bus_cmd_name(unsigned cmd)
[ND_CMD_ARS_START] = "ars_start",
[ND_CMD_ARS_STATUS] = "ars_status",
[ND_CMD_CLEAR_ERROR] = "clear_error",
+ [ND_CMD_CALL] = "cmd_call",
};
if (cmd < ARRAY_SIZE(names) && names[cmd])
@@ -158,6 +161,7 @@ static inline const char *nvdimm_cmd_name(unsigned cmd)
[ND_CMD_VENDOR_EFFECT_LOG_SIZE] = "effect_size",
[ND_CMD_VENDOR_EFFECT_LOG] = "effect_log",
[ND_CMD_VENDOR] = "vendor",
+ [ND_CMD_CALL] = "cmd_call",
};
if (cmd < ARRAY_SIZE(names) && names[cmd])
@@ -224,4 +228,53 @@ enum ars_masks {
ARS_STATUS_MASK = 0x0000FFFF,
ARS_EXT_STATUS_SHIFT = 16,
};
+
+
+/*
+ * struct nd_cmd_pkg
+ *
+ * is a wrapper to a quasi pass thru interface for invoking firmware
+ * associated with nvdimms.
+ *
+ * INPUT PARAMETERS
+ *
+ * nd_family corresponds to the firmware (e.g. DSM) interface.
+ *
+ * nd_command are the function index advertised by the firmware.
+ *
+ * nd_size_in is the size of the input parameters being passed to firmware
+ *
+ * OUTPUT PARAMETERS
+ *
+ * nd_fw_size is the size of the data firmware wants to return for
+ * the call. If nd_fw_size is greater than size of nd_size_out, only
+ * the first nd_size_out bytes are returned.
+ */
+
+struct nd_cmd_pkg {
+ __u64 nd_family; /* family of commands, eg ND_TYPE_BUS */
+ __u64 nd_command;
+ __u32 nd_size_in; /* INPUT: size of input args */
+ __u32 nd_size_out; /* INPUT: size of payload */
+ __u32 nd_reserved2[9]; /* reserved must be zero */
+ __u32 nd_fw_size; /* OUTPUT: size fw wants to return */
+ unsigned char nd_payload[]; /* Contents of call */
+};
+
+/*
+ * the list nd_family of commands. The mapping to firmware handles
+ * is definied in the nfit_cmd_family_tbl
+ *
+ */
+#define ND_TYPE_BUS 1
+#define ND_TYPE_DIMM_INTEL1 2
+#define ND_TYPE_DIMM_N_HPE1 3
+#define ND_TYPE_DIMM_N_HPE2 4
+
+
+#define ND_IOCTL_CALL _IOWR(ND_IOCTL, ND_CMD_CALL,\
+ struct nd_cmd_pkg)
+
+
+
#endif /* __NDCTL_H__ */
--
1.7.11.3
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v8 03/10] nvdimm: Increase max envelope size for IOCTL
2016-03-21 19:37 [PATCH v8 00/10] nvdimm: Add an IOCTL pass thru for DSM calls Jerry Hoemann
2016-03-21 19:37 ` [PATCH v8 01/10] ACPI / util: Fix acpi_evaluate_dsm() argument type Jerry Hoemann
2016-03-21 19:37 ` [PATCH v8 02/10] nvdimm: Add wrapper for IOCTL pass thru Jerry Hoemann
@ 2016-03-21 19:37 ` Jerry Hoemann
2016-03-21 19:37 ` [PATCH v8 04/10] nvdimm: Add UUIDs Jerry Hoemann
` (8 subsequent siblings)
11 siblings, 0 replies; 29+ messages in thread
From: Jerry Hoemann @ 2016-03-21 19:37 UTC (permalink / raw)
To: dan.j.williams; +Cc: linux-nvdimm
In __nd_ioctl must first read in the fixed sized portion of an ioctl
so that it can then determine the size of the variable part.
Prepare for ND_CMD_CALL_DSM calls which have larger fixed portion
wrapper.
Signed-off-by: Jerry Hoemann <jerry.hoemann@hpe.com>
---
include/linux/libnvdimm.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
index 833867b..af31d1c 100644
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -27,7 +27,7 @@ enum {
/* need to set a limit somewhere, but yes, this is likely overkill */
ND_IOCTL_MAX_BUFLEN = SZ_4M,
ND_CMD_MAX_ELEM = 5,
- ND_CMD_MAX_ENVELOPE = 16,
+ ND_CMD_MAX_ENVELOPE = 256,
ND_MAX_MAPPINGS = 32,
/* region flag indicating to direct-map persistent memory by default */
--
1.7.11.3
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v8 04/10] nvdimm: Add UUIDs
2016-03-21 19:37 [PATCH v8 00/10] nvdimm: Add an IOCTL pass thru for DSM calls Jerry Hoemann
` (2 preceding siblings ...)
2016-03-21 19:37 ` [PATCH v8 03/10] nvdimm: Increase max envelope size for IOCTL Jerry Hoemann
@ 2016-03-21 19:37 ` Jerry Hoemann
2016-04-11 18:25 ` Dan Williams
2016-03-21 19:37 ` [PATCH v8 05/10] nvdimm: Add IOCTL pass thru functions Jerry Hoemann
` (7 subsequent siblings)
11 siblings, 1 reply; 29+ messages in thread
From: Jerry Hoemann @ 2016-03-21 19:37 UTC (permalink / raw)
To: dan.j.williams; +Cc: linux-nvdimm
Add UUID associated with HPE Type N NVDIMM.
Signed-off-by: Jerry Hoemann <jerry.hoemann@hpe.com>
---
drivers/acpi/nfit.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/acpi/nfit.h b/drivers/acpi/nfit.h
index c75576b..10dd0d3 100644
--- a/drivers/acpi/nfit.h
+++ b/drivers/acpi/nfit.h
@@ -23,6 +23,8 @@
#define UUID_NFIT_BUS "2f10e7a4-9e91-11e4-89d3-123b93f75cba"
#define UUID_NFIT_DIMM "4309ac30-0d11-11e4-9191-0800200c9a66"
+#define UUID_NFIT_DIMM_N_HPE1 "9002c334-acf3-4c0e-9642-a235f0d53bc6"
+#define UUID_NFIT_DIMM_N_HPE2 "5008664B-B758-41A0-A03C-27C2F2D04F7E"
#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)
@@ -38,6 +40,8 @@ enum nfit_uuids {
NFIT_SPA_PCD,
NFIT_DEV_BUS,
NFIT_DEV_DIMM,
+ NFIT_DEV_DIMM_N_HPE1,
+ NFIT_DEV_DIMM_N_HPE2,
NFIT_UUID_MAX,
};
@@ -110,6 +114,7 @@ struct nfit_mem {
struct list_head list;
struct acpi_device *adev;
unsigned long dsm_mask;
+ const u8 *dsm_uuid;
};
struct acpi_nfit_desc {
--
1.7.11.3
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v8 05/10] nvdimm: Add IOCTL pass thru functions
2016-03-21 19:37 [PATCH v8 00/10] nvdimm: Add an IOCTL pass thru for DSM calls Jerry Hoemann
` (3 preceding siblings ...)
2016-03-21 19:37 ` [PATCH v8 04/10] nvdimm: Add UUIDs Jerry Hoemann
@ 2016-03-21 19:37 ` Jerry Hoemann
2016-04-11 19:15 ` Dan Williams
2016-03-21 19:37 ` [PATCH v8 06/10] libnvdimm: nvdimm_bus_descriptor field name change Jerry Hoemann
` (6 subsequent siblings)
11 siblings, 1 reply; 29+ messages in thread
From: Jerry Hoemann @ 2016-03-21 19:37 UTC (permalink / raw)
To: dan.j.williams; +Cc: linux-nvdimm
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 | 141 ++++++++++++++++++++++++++++++++++++++++++++-------
drivers/nvdimm/bus.c | 43 +++++++++++++++-
2 files changed, 166 insertions(+), 18 deletions(-)
diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c
index d0f35e6..9511ab4 100644
--- a/drivers/acpi/nfit.c
+++ b/drivers/acpi/nfit.c
@@ -56,6 +56,21 @@ 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 */
+ u64 mask; /* 0 bit excludes underlying func.*/
+};
+
+struct cmd_family_tbl nfit_cmd_family_tbl[] = {
+ { NFIT_DEV_BUS, ND_TYPE_BUS, 1, ~0UL},
+ { NFIT_DEV_DIMM, ND_TYPE_DIMM_INTEL1, 1, ~0UL},
+ { NFIT_DEV_DIMM_N_HPE1, ND_TYPE_DIMM_N_HPE1, 1, ~0UL},
+ { NFIT_DEV_DIMM_N_HPE2, ND_TYPE_DIMM_N_HPE2, 1, ~0UL},
+ { -1, -1, -1, 0},
+};
+
static u8 nfit_uuid[NFIT_UUID_MAX][16];
const u8 *to_nfit_uuid(enum nfit_uuids id)
@@ -64,6 +79,19 @@ const u8 *to_nfit_uuid(enum nfit_uuids id)
}
EXPORT_SYMBOL(to_nfit_uuid);
+static const u8 *
+family_to_uuid(int type)
+{
+ 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 +199,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 +209,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 +242,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;
@@ -218,25 +253,49 @@ static int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
in_buf.buffer.length = 0;
/* libnvdimm has already validated the input envelope */
- for (i = 0; i < desc->in_num; i++)
+ for (i = 0; i < desc->in_num; i++) {
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);
+ dev_dbg(dev, "%s:%s unexpected output object type cmd: %s %llu, type: %d\n",
+ __func__, dimm_name, cmd_name, func, out_obj->type);
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);
+ 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,13 +977,60 @@ static struct nvdimm *acpi_nfit_dimm_by_handle(struct acpi_nfit_desc *acpi_desc,
return NULL;
}
+
+/*
+ * determine if the _DSM specified by UUID is supported and return
+ * mask of supported functions in nd_cmd_mask.
+ */
+
+static int acpi_nfit_sup_func(acpi_handle handle, const u8 *uuid,
+ int rev, unsigned long *nd_cmd_mask)
+{
+ int i;
+ u64 mask = 0;
+ union acpi_object *obj;
+
+ obj = acpi_evaluate_dsm(handle, uuid, rev, 0, NULL);
+ if (!obj)
+ return 0;
+ /* For compatibility, old BIOSes may return an integer */
+ if (obj->type == ACPI_TYPE_INTEGER)
+ mask = obj->integer.value;
+ else if (obj->type == ACPI_TYPE_BUFFER)
+ for (i = 0; i < obj->buffer.length && i < 8; i++)
+ mask |= (((u8)obj->buffer.pointer[i]) << (i * 8));
+ ACPI_FREE(obj);
+
+ *nd_cmd_mask = mask;
+
+ return !!mask;
+}
+
+
+static inline void
+to_nfit_uuid_msk(acpi_handle handle, struct cmd_family_tbl *tbl,
+ u8 const **cmd_uuid, unsigned long *cmd_mask)
+{
+ unsigned long mask = 0;
+ 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_nfit_sup_func(handle, uuid, rev, &mask)) {
+ *cmd_mask = mask & tbl[i].mask;
+ *cmd_uuid = uuid;
+ break;
+ }
+ }
+}
+
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);
- int i;
nfit_mem->dsm_mask = acpi_desc->dimm_dsm_force_en;
adev = to_acpi_dev(acpi_desc);
@@ -939,9 +1045,8 @@ 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 (acpi_check_dsm(adev_dimm->handle, uuid, 1, 1ULL << i))
- set_bit(i, &nfit_mem->dsm_mask);
+ to_nfit_uuid_msk(adev_dimm->handle, nfit_cmd_family_tbl,
+ &nfit_mem->dsm_uuid, &nfit_mem->dsm_mask);
return 0;
}
@@ -1012,7 +1117,7 @@ static void acpi_nfit_init_dsms(struct acpi_nfit_desc *acpi_desc)
if (!adev)
return;
- for (i = ND_CMD_ARS_CAP; i <= ND_CMD_CLEAR_ERROR; i++)
+ for (i = 0; i <= ND_CMD_CLEAR_ERROR; i++)
if (acpi_check_dsm(adev->handle, uuid, 1, 1ULL << i))
set_bit(i, &nd_desc->dsm_mask);
}
@@ -2463,6 +2568,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)
diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c
index 3355748..f205d10 100644
--- a/drivers/nvdimm/bus.c
+++ b/drivers/nvdimm/bus.c
@@ -439,6 +439,12 @@ static const struct nd_cmd_desc __nd_cmd_dimm_descs[] = {
.out_num = 3,
.out_sizes = { 4, 4, UINT_MAX, },
},
+ [ND_CMD_CALL] = {
+ .in_num = 2,
+ .in_sizes = {sizeof(struct nd_cmd_pkg), UINT_MAX, },
+ .out_num = 1,
+ .out_sizes = { UINT_MAX, },
+ },
};
const struct nd_cmd_desc *nd_cmd_dimm_desc(int cmd)
@@ -473,6 +479,12 @@ static const struct nd_cmd_desc __nd_cmd_bus_descs[] = {
.out_num = 3,
.out_sizes = { 4, 4, 8, },
},
+ [ND_CMD_CALL] = {
+ .in_num = 2,
+ .in_sizes = {sizeof(struct nd_cmd_pkg), UINT_MAX, },
+ .out_num = 1,
+ .out_sizes = { UINT_MAX, },
+ },
};
const struct nd_cmd_desc *nd_cmd_bus_desc(int cmd)
@@ -500,6 +512,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) {
+ struct nd_cmd_pkg *pkg = buf;
+
+ return pkg->nd_size_in;
}
return UINT_MAX;
@@ -522,6 +538,12 @@ u32 nd_cmd_out_size(struct nvdimm *nvdimm, int cmd,
return out_field[1];
else if (!nvdimm && cmd == ND_CMD_ARS_STATUS && idx == 2)
return out_field[1] - 8;
+ else if (cmd == ND_CMD_CALL) {
+ struct nd_cmd_pkg *pkg =
+ (struct nd_cmd_pkg *) in_field;
+ return pkg->nd_size_out;
+ }
+
return UINT_MAX;
}
@@ -588,7 +610,9 @@ static int __nd_ioctl(struct nvdimm_bus *nvdimm_bus, struct nvdimm *nvdimm,
unsigned int cmd = _IOC_NR(ioctl_cmd);
void __user *p = (void __user *) arg;
struct device *dev = &nvdimm_bus->dev;
+ struct nd_cmd_pkg call_dsm;
const char *cmd_name, *dimm_name;
+ unsigned int func = cmd;
unsigned long dsm_mask;
void *buf;
int rc, i;
@@ -605,8 +629,14 @@ static int __nd_ioctl(struct nvdimm_bus *nvdimm_bus, struct nvdimm *nvdimm,
dimm_name = "bus";
}
+ if (cmd == ND_CMD_CALL) {
+ if (copy_from_user(&call_dsm, p, sizeof(call_dsm)))
+ return -EFAULT;
+ func = call_dsm.nd_command;
+ }
+
if (!desc || (desc->out_num + desc->in_num == 0) ||
- !test_bit(cmd, &dsm_mask))
+ !test_bit(func, &dsm_mask))
return -ENOTTY;
/* fail write commands (when read-only) */
@@ -616,6 +646,7 @@ static int __nd_ioctl(struct nvdimm_bus *nvdimm_bus, struct nvdimm *nvdimm,
case ND_CMD_SET_CONFIG_DATA:
case ND_CMD_ARS_START:
case ND_CMD_CLEAR_ERROR:
+ case ND_CMD_CALL:
dev_dbg(&nvdimm_bus->dev, "'%s' command while read-only.\n",
nvdimm ? nvdimm_cmd_name(cmd)
: nvdimm_bus_cmd_name(cmd));
@@ -643,6 +674,16 @@ static int __nd_ioctl(struct nvdimm_bus *nvdimm_bus, struct nvdimm *nvdimm,
in_len += in_size;
}
+ if (cmd == ND_CMD_CALL) {
+ dev_dbg(dev, "%s:%s, idx: %llu, in: %zu, out: %zu, len %zu\n",
+ __func__, dimm_name, call_dsm.nd_command,
+ in_len, out_len, buf_len);
+
+ for (i = 0; i < ARRAY_SIZE(call_dsm.nd_reserved2); i++)
+ if (call_dsm.nd_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
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v8 06/10] libnvdimm: nvdimm_bus_descriptor field name change
2016-03-21 19:37 [PATCH v8 00/10] nvdimm: Add an IOCTL pass thru for DSM calls Jerry Hoemann
` (4 preceding siblings ...)
2016-03-21 19:37 ` [PATCH v8 05/10] nvdimm: Add IOCTL pass thru functions Jerry Hoemann
@ 2016-03-21 19:37 ` Jerry Hoemann
2016-03-21 19:37 ` [PATCH v8 07/10] tools/testing/nvdimm: 'call_dsm' support Jerry Hoemann
` (5 subsequent siblings)
11 siblings, 0 replies; 29+ messages in thread
From: Jerry Hoemann @ 2016-03-21 19:37 UTC (permalink / raw)
To: dan.j.williams; +Cc: linux-nvdimm
Change the the name of:
nvdimm_bus_descriptor.dsm_mask
to
nvdimm_bus_descriptor.cmd_mask
Signed-off-by: Jerry Hoemann <jerry.hoemann@hpe.com>
---
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(-)
diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c
index 9511ab4..8d933b3 100644
--- a/drivers/acpi/nfit.c
+++ b/drivers/acpi/nfit.c
@@ -232,7 +232,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;
@@ -1112,14 +1112,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 = 0; 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,
diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c
index f205d10..dcfcd8e 100644
--- a/drivers/nvdimm/bus.c
+++ b/drivers/nvdimm/bus.c
@@ -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";
}
diff --git a/drivers/nvdimm/core.c b/drivers/nvdimm/core.c
index 79646d0..ce80767 100644
--- a/drivers/nvdimm/core.c
+++ b/drivers/nvdimm/core.c
@@ -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;
diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
index af31d1c..67a1ba0 100644
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -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);
diff --git a/tools/testing/nvdimm/test/nfit.c b/tools/testing/nvdimm/test/nfit.c
index 3187322..1d3b8ce 100644
--- a/tools/testing/nvdimm/test/nfit.c
+++ b/tools/testing/nvdimm/test/nfit.c
@@ -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) {
--
1.7.11.3
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v8 07/10] tools/testing/nvdimm: 'call_dsm' support
2016-03-21 19:37 [PATCH v8 00/10] nvdimm: Add an IOCTL pass thru for DSM calls Jerry Hoemann
` (5 preceding siblings ...)
2016-03-21 19:37 ` [PATCH v8 06/10] libnvdimm: nvdimm_bus_descriptor field name change Jerry Hoemann
@ 2016-03-21 19:37 ` Jerry Hoemann
2016-03-21 19:37 ` [PATCH v8 08/10] nvdimm: command ioctl support Jerry Hoemann
` (4 subsequent siblings)
11 siblings, 0 replies; 29+ messages in thread
From: Jerry Hoemann @ 2016-03-21 19:37 UTC (permalink / raw)
To: dan.j.williams; +Cc: linux-nvdimm
Enable nfit_test to use the generic 'call_dsm' envelope.
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
tools/testing/nvdimm/test/nfit.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/tools/testing/nvdimm/test/nfit.c b/tools/testing/nvdimm/test/nfit.c
index 1d3b8ce..5afc3d3 100644
--- a/tools/testing/nvdimm/test/nfit.c
+++ b/tools/testing/nvdimm/test/nfit.c
@@ -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 *call_dsm = buf;
+
+ buf_len = call_dsm->nd_size_in + call_dsm->nd_size_out;
+ buf = (void *) call_dsm->nd_payload;
+ func = call_dsm->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;
--
1.7.11.3
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v8 08/10] nvdimm: command ioctl support
2016-03-21 19:37 [PATCH v8 00/10] nvdimm: Add an IOCTL pass thru for DSM calls Jerry Hoemann
` (6 preceding siblings ...)
2016-03-21 19:37 ` [PATCH v8 07/10] tools/testing/nvdimm: 'call_dsm' support Jerry Hoemann
@ 2016-03-21 19:37 ` Jerry Hoemann
2016-03-21 19:37 ` [PATCH v8 09/10] nvdimm: sysfs shows which dsm support full command ioctl Jerry Hoemann
` (3 subsequent siblings)
11 siblings, 0 replies; 29+ messages in thread
From: Jerry Hoemann @ 2016-03-21 19:37 UTC (permalink / raw)
To: dan.j.williams; +Cc: linux-nvdimm
Flags to indicate if object supports the full set of ND_IOCTL
commands (lack indicates just the pass thru.)
Signed-off-by: Jerry Hoemann <jerry.hoemann@hpe.com>
---
drivers/acpi/nfit.h | 1 +
drivers/nvdimm/nd-core.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/acpi/nfit.h b/drivers/acpi/nfit.h
index 10dd0d3..bf353fa 100644
--- a/drivers/acpi/nfit.h
+++ b/drivers/acpi/nfit.h
@@ -115,6 +115,7 @@ struct nfit_mem {
struct acpi_device *adev;
unsigned long dsm_mask;
const u8 *dsm_uuid;
+ int cmd_ioctl;
};
struct acpi_nfit_desc {
diff --git a/drivers/nvdimm/nd-core.h b/drivers/nvdimm/nd-core.h
index 1d1500f..0933f66 100644
--- a/drivers/nvdimm/nd-core.h
+++ b/drivers/nvdimm/nd-core.h
@@ -38,6 +38,7 @@ struct nvdimm {
unsigned long flags;
void *provider_data;
unsigned long *dsm_mask;
+ int cmd_ioctl;
struct device dev;
atomic_t busy;
int id;
--
1.7.11.3
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v8 09/10] nvdimm: sysfs shows which dsm support full command ioctl.
2016-03-21 19:37 [PATCH v8 00/10] nvdimm: Add an IOCTL pass thru for DSM calls Jerry Hoemann
` (7 preceding siblings ...)
2016-03-21 19:37 ` [PATCH v8 08/10] nvdimm: command ioctl support Jerry Hoemann
@ 2016-03-21 19:37 ` Jerry Hoemann
2016-04-11 21:43 ` Dan Williams
2016-03-21 20:55 ` [PATCH v8 10/10] nvdimm: Add ioctl to return command mask Jerry Hoemann
` (2 subsequent siblings)
11 siblings, 1 reply; 29+ messages in thread
From: Jerry Hoemann @ 2016-03-21 19:37 UTC (permalink / raw)
To: dan.j.williams; +Cc: linux-nvdimm
NVDIMM DSMs whose functions map to the Intel Example DSM, can
be called with the ND_IOCTL_.* ioctl interface. Those that
don't map, can only be called with the pass thru command.
Save this indication in struct nvdimm to be passed to
the sysfs interface.
Signed-off-by: Jerry Hoemann <jerry.hoemann@hpe.com>
---
drivers/acpi/nfit.c | 25 +++++++++++++++----------
drivers/nvdimm/core.c | 1 +
drivers/nvdimm/dimm_devs.c | 12 ++++++++----
include/linux/libnvdimm.h | 2 +-
4 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c
index 8d933b3..a35b939 100644
--- a/drivers/acpi/nfit.c
+++ b/drivers/acpi/nfit.c
@@ -61,14 +61,15 @@ struct cmd_family_tbl {
int key_type; /* Exported handle */
int rev; /* _DSM rev */
u64 mask; /* 0 bit excludes underlying func.*/
+ int cmd_ioctl; /* supports full cmd ioctl interface */
};
struct cmd_family_tbl nfit_cmd_family_tbl[] = {
- { NFIT_DEV_BUS, ND_TYPE_BUS, 1, ~0UL},
- { NFIT_DEV_DIMM, ND_TYPE_DIMM_INTEL1, 1, ~0UL},
- { NFIT_DEV_DIMM_N_HPE1, ND_TYPE_DIMM_N_HPE1, 1, ~0UL},
- { NFIT_DEV_DIMM_N_HPE2, ND_TYPE_DIMM_N_HPE2, 1, ~0UL},
- { -1, -1, -1, 0},
+ { NFIT_DEV_BUS, ND_TYPE_BUS, 1, ~0UL, 1},
+ { NFIT_DEV_DIMM, ND_TYPE_DIMM_INTEL1, 1, ~0UL, 1},
+ { NFIT_DEV_DIMM_N_HPE1, ND_TYPE_DIMM_N_HPE1, 1, ~0UL, 0},
+ { NFIT_DEV_DIMM_N_HPE2, ND_TYPE_DIMM_N_HPE2, 1, ~0UL, 0},
+ { -1, -1, -1, -1, 0},
};
static u8 nfit_uuid[NFIT_UUID_MAX][16];
@@ -264,8 +265,8 @@ static int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
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);
+ dev_dbg(dev, "%s unsupported cmd family: %lld\n",
+ dimm_name, call_dsm->nd_family);
return -EINVAL;
}
}
@@ -1009,7 +1010,8 @@ static int acpi_nfit_sup_func(acpi_handle handle, const u8 *uuid,
static inline void
to_nfit_uuid_msk(acpi_handle handle, struct cmd_family_tbl *tbl,
- u8 const **cmd_uuid, unsigned long *cmd_mask)
+ u8 const **cmd_uuid, unsigned long *cmd_mask,
+ int *cmd_ioctl)
{
unsigned long mask = 0;
int i;
@@ -1021,6 +1023,7 @@ to_nfit_uuid_msk(acpi_handle handle, struct cmd_family_tbl *tbl,
if (acpi_nfit_sup_func(handle, uuid, rev, &mask)) {
*cmd_mask = mask & tbl[i].mask;
*cmd_uuid = uuid;
+ *cmd_ioctl = tbl[i].cmd_ioctl;
break;
}
}
@@ -1046,7 +1049,8 @@ static int acpi_nfit_add_dimm(struct acpi_nfit_desc *acpi_desc,
}
to_nfit_uuid_msk(adev_dimm->handle, nfit_cmd_family_tbl,
- &nfit_mem->dsm_uuid, &nfit_mem->dsm_mask);
+ &nfit_mem->dsm_uuid, &nfit_mem->dsm_mask,
+ &nfit_mem->cmd_ioctl);
return 0;
}
@@ -1083,7 +1087,8 @@ 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->dsm_mask,
+ nfit_mem->cmd_ioctl);
if (!nvdimm)
return -ENOMEM;
diff --git a/drivers/nvdimm/core.c b/drivers/nvdimm/core.c
index ce80767..24da865 100644
--- a/drivers/nvdimm/core.c
+++ b/drivers/nvdimm/core.c
@@ -253,6 +253,7 @@ static ssize_t commands_show(struct device *dev,
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, "%s ", nvdimm_cmd_name(ND_CMD_CALL));
len += sprintf(buf + len, "\n");
return len;
}
diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
index c56f882..65614b2 100644
--- a/drivers/nvdimm/dimm_devs.c
+++ b/drivers/nvdimm/dimm_devs.c
@@ -274,14 +274,17 @@ EXPORT_SYMBOL_GPL(nvdimm_provider_data);
static ssize_t commands_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
+ struct nvdimm_bus *nvdimm_bus = walk_to_nvdimm_bus(dev);
struct nvdimm *nvdimm = to_nvdimm(dev);
int cmd, len = 0;
- if (!nvdimm->dsm_mask)
+ if (!nvdimm->dsm_mask || !nvdimm_bus)
return sprintf(buf, "\n");
- for_each_set_bit(cmd, nvdimm->dsm_mask, BITS_PER_LONG)
- len += sprintf(buf + len, "%s ", nvdimm_cmd_name(cmd));
+ if (nvdimm->cmd_ioctl)
+ for_each_set_bit(cmd, nvdimm->dsm_mask, BITS_PER_LONG)
+ len += sprintf(buf + len, "%s ", nvdimm_cmd_name(cmd));
+ len += sprintf(buf + len, "%s ", nvdimm_cmd_name(ND_CMD_CALL));
len += sprintf(buf + len, "\n");
return len;
}
@@ -340,7 +343,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 *dsm_mask, int cmd_ioctl)
{
struct nvdimm *nvdimm = kzalloc(sizeof(*nvdimm), GFP_KERNEL);
struct device *dev;
@@ -356,6 +359,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_ioctl = cmd_ioctl;
atomic_set(&nvdimm->busy, 0);
dev = &nvdimm->dev;
dev_set_name(dev, "nmem%d", nvdimm->id);
diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
index 67a1ba0..154986a 100644
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -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 *dsm_mask, int cmd_ioctl);
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,
--
1.7.11.3
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v8 10/10] nvdimm: Add ioctl to return command mask.
2016-03-21 19:37 [PATCH v8 00/10] nvdimm: Add an IOCTL pass thru for DSM calls Jerry Hoemann
` (8 preceding siblings ...)
2016-03-21 19:37 ` [PATCH v8 09/10] nvdimm: sysfs shows which dsm support full command ioctl Jerry Hoemann
@ 2016-03-21 20:55 ` Jerry Hoemann
2016-03-29 20:39 ` [PATCH v8 00/10] nvdimm: Add an IOCTL pass thru for DSM calls Jerry Hoemann
2016-04-11 22:19 ` Dan Williams
11 siblings, 0 replies; 29+ messages in thread
From: Jerry Hoemann @ 2016-03-21 20:55 UTC (permalink / raw)
To: dan.j.williams; +Cc: linux-nvdimm
The pass thru calls return command mask. Previously, bit zero
wasn't part of command mask, but as it is now, this left commands_show
displaying "unknown" for function zero. Add an ioctl interface
to return command mask.
Signed-off-by: Jerry Hoemann <jerry.hoemann@hpe.com>
---
drivers/nvdimm/bus.c | 10 ++++++++--
include/uapi/linux/ndctl.h | 9 +++++++++
2 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c
index dcfcd8e..a61f069 100644
--- a/drivers/nvdimm/bus.c
+++ b/drivers/nvdimm/bus.c
@@ -404,7 +404,10 @@ void nvdimm_bus_destroy_ndctl(struct nvdimm_bus *nvdimm_bus)
}
static const struct nd_cmd_desc __nd_cmd_dimm_descs[] = {
- [ND_CMD_IMPLEMENTED] = { },
+ [ND_CMD_IMPLEMENTED] = {
+ .out_num = 1,
+ .out_sizes = { 8, },
+ },
[ND_CMD_SMART] = {
.out_num = 2,
.out_sizes = { 4, 8, },
@@ -456,7 +459,10 @@ const struct nd_cmd_desc *nd_cmd_dimm_desc(int cmd)
EXPORT_SYMBOL_GPL(nd_cmd_dimm_desc);
static const struct nd_cmd_desc __nd_cmd_bus_descs[] = {
- [ND_CMD_IMPLEMENTED] = { },
+ [ND_CMD_IMPLEMENTED] = {
+ .out_num = 1,
+ .out_sizes = { 1, },
+ },
[ND_CMD_ARS_CAP] = {
.in_num = 2,
.in_sizes = { 8, 8, },
diff --git a/include/uapi/linux/ndctl.h b/include/uapi/linux/ndctl.h
index c043cd1..cae7c34 100644
--- a/include/uapi/linux/ndctl.h
+++ b/include/uapi/linux/ndctl.h
@@ -20,6 +20,10 @@ struct nd_cmd_smart {
__u8 data[128];
} __packed;
+struct nd_cmd_mask {
+ __u8 data[8];
+} __packed;
+
struct nd_cmd_smart_threshold {
__u32 status;
__u8 data[8];
@@ -137,6 +141,7 @@ enum {
static inline const char *nvdimm_bus_cmd_name(unsigned cmd)
{
static const char * const names[] = {
+ [ND_CMD_IMPLEMENTED] = "cmd_mask",
[ND_CMD_ARS_CAP] = "ars_cap",
[ND_CMD_ARS_START] = "ars_start",
[ND_CMD_ARS_STATUS] = "ars_status",
@@ -152,6 +157,7 @@ static inline const char *nvdimm_bus_cmd_name(unsigned cmd)
static inline const char *nvdimm_cmd_name(unsigned cmd)
{
static const char * const names[] = {
+ [ND_CMD_IMPLEMENTED] = "cmd_mask",
[ND_CMD_SMART] = "smart",
[ND_CMD_SMART_THRESHOLD] = "smart_thresh",
[ND_CMD_DIMM_FLAGS] = "flags",
@@ -171,6 +177,9 @@ static inline const char *nvdimm_cmd_name(unsigned cmd)
#define ND_IOCTL 'N'
+#define ND_IOCTL_CMD_MASK _IOWR(ND_IOCTL, ND_CMD_IMPLEMENTED,\
+ struct nd_cmd_mask)
+
#define ND_IOCTL_SMART _IOWR(ND_IOCTL, ND_CMD_SMART,\
struct nd_cmd_smart)
--
1.7.11.3
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH v8 00/10] nvdimm: Add an IOCTL pass thru for DSM calls
2016-03-21 19:37 [PATCH v8 00/10] nvdimm: Add an IOCTL pass thru for DSM calls Jerry Hoemann
` (9 preceding siblings ...)
2016-03-21 20:55 ` [PATCH v8 10/10] nvdimm: Add ioctl to return command mask Jerry Hoemann
@ 2016-03-29 20:39 ` Jerry Hoemann
2016-04-11 22:19 ` Dan Williams
11 siblings, 0 replies; 29+ messages in thread
From: Jerry Hoemann @ 2016-03-29 20:39 UTC (permalink / raw)
To: dan.j.williams; +Cc: linux-nvdimm
On Mon, Mar 21, 2016 at 01:37:00PM -0600, Jerry Hoemann wrote:
>
> The NVDIMM code in the kernel supports an IOCTL interface to user
> space based upon the Intel Example DSM:
>
[...]
> 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 the Intel Example DSM supports 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.
>
>
>
Dan,
Have you had a chance to look at this version?
Thanks,
Jerry
--
-----------------------------------------------------------------------------
Jerry Hoemann Software Engineer Hewlett Packard Enterprise
-----------------------------------------------------------------------------
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v8 02/10] nvdimm: Add wrapper for IOCTL pass thru
2016-03-21 19:37 ` [PATCH v8 02/10] nvdimm: Add wrapper for IOCTL pass thru Jerry Hoemann
@ 2016-04-11 18:21 ` Dan Williams
2016-04-11 23:16 ` Jerry Hoemann
0 siblings, 1 reply; 29+ messages in thread
From: Dan Williams @ 2016-04-11 18:21 UTC (permalink / raw)
To: Jerry Hoemann; +Cc: linux-nvdimm@lists.01.org
On Mon, Mar 21, 2016 at 12:37 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote:
> Add struct nd_cmd_pkg which serves as a warapper for
> the data being passed via a pass thru to a NVDIMM DSM.
> This wrapper specifies the extra information in a uniform
> manner allowing the kenrel to call a DSM without knowing
> specifics of the DSM.
>
> Add dsm_call command to nvdimm_bus_cmd_name and nvdimm_cmd_name.
>
> Signed-off-by: Jerry Hoemann <jerry.hoemann@hpe.com>
> ---
> include/uapi/linux/ndctl.h | 53 ++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 53 insertions(+)
>
> diff --git a/include/uapi/linux/ndctl.h b/include/uapi/linux/ndctl.h
> index 7cc28ab..c043cd1 100644
> --- a/include/uapi/linux/ndctl.h
> +++ b/include/uapi/linux/ndctl.h
> @@ -114,6 +114,7 @@ enum {
> ND_CMD_ARS_START = 2,
> ND_CMD_ARS_STATUS = 3,
> ND_CMD_CLEAR_ERROR = 4,
> + /* 10 reserved for ND_CMD_CALL below */
>
> /* per-dimm commands */
> ND_CMD_SMART = 1,
> @@ -125,6 +126,7 @@ enum {
> ND_CMD_VENDOR_EFFECT_LOG_SIZE = 7,
> ND_CMD_VENDOR_EFFECT_LOG = 8,
> ND_CMD_VENDOR = 9,
> + ND_CMD_CALL = 10,
> };
>
> enum {
> @@ -139,6 +141,7 @@ static inline const char *nvdimm_bus_cmd_name(unsigned cmd)
> [ND_CMD_ARS_START] = "ars_start",
> [ND_CMD_ARS_STATUS] = "ars_status",
> [ND_CMD_CLEAR_ERROR] = "clear_error",
> + [ND_CMD_CALL] = "cmd_call",
> };
>
> if (cmd < ARRAY_SIZE(names) && names[cmd])
> @@ -158,6 +161,7 @@ static inline const char *nvdimm_cmd_name(unsigned cmd)
> [ND_CMD_VENDOR_EFFECT_LOG_SIZE] = "effect_size",
> [ND_CMD_VENDOR_EFFECT_LOG] = "effect_log",
> [ND_CMD_VENDOR] = "vendor",
> + [ND_CMD_CALL] = "cmd_call",
> };
>
> if (cmd < ARRAY_SIZE(names) && names[cmd])
> @@ -224,4 +228,53 @@ enum ars_masks {
> ARS_STATUS_MASK = 0x0000FFFF,
> ARS_EXT_STATUS_SHIFT = 16,
> };
> +
> +
> +/*
> + * struct nd_cmd_pkg
> + *
> + * is a wrapper to a quasi pass thru interface for invoking firmware
> + * associated with nvdimms.
> + *
> + * INPUT PARAMETERS
> + *
> + * nd_family corresponds to the firmware (e.g. DSM) interface.
> + *
> + * nd_command are the function index advertised by the firmware.
> + *
> + * nd_size_in is the size of the input parameters being passed to firmware
> + *
> + * OUTPUT PARAMETERS
> + *
> + * nd_fw_size is the size of the data firmware wants to return for
> + * the call. If nd_fw_size is greater than size of nd_size_out, only
> + * the first nd_size_out bytes are returned.
> + */
> +
> +struct nd_cmd_pkg {
> + __u64 nd_family; /* family of commands, eg ND_TYPE_BUS */
> + __u64 nd_command;
> + __u32 nd_size_in; /* INPUT: size of input args */
> + __u32 nd_size_out; /* INPUT: size of payload */
> + __u32 nd_reserved2[9]; /* reserved must be zero */
> + __u32 nd_fw_size; /* OUTPUT: size fw wants to return */
> + unsigned char nd_payload[]; /* Contents of call */
> +};
> +
> +/*
> + * the list nd_family of commands. The mapping to firmware handles
> + * is definied in the nfit_cmd_family_tbl
> + *
> + */
> +#define ND_TYPE_BUS 1
> +#define ND_TYPE_DIMM_INTEL1 2
> +#define ND_TYPE_DIMM_N_HPE1 3
> +#define ND_TYPE_DIMM_N_HPE2 4
> +
> +
> +#define ND_IOCTL_CALL _IOWR(ND_IOCTL, ND_CMD_CALL,\
> + struct nd_cmd_pkg)
> +
> +
> +
> #endif /* __NDCTL_H__ */
Looks, good, but for now lets assume there will never be such a thing
as ND_TYPE_BUS commands or an ND_CMD_CALL for the bus. I.e. all the
commands that currently exist for the bus are commands that the kernel
wants to handle. In this way, struct nd_cmd_pkg is a solution to the
problem of commands that the kernel knows exist, but has no use case
to generate or parse itself. Commands that fit that description
currently only exist in leaf node implementations.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v8 04/10] nvdimm: Add UUIDs
2016-03-21 19:37 ` [PATCH v8 04/10] nvdimm: Add UUIDs Jerry Hoemann
@ 2016-04-11 18:25 ` Dan Williams
0 siblings, 0 replies; 29+ messages in thread
From: Dan Williams @ 2016-04-11 18:25 UTC (permalink / raw)
To: Jerry Hoemann; +Cc: linux-nvdimm@lists.01.org
On Mon, Mar 21, 2016 at 12:37 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote:
> Add UUID associated with HPE Type N NVDIMM.
>
> Signed-off-by: Jerry Hoemann <jerry.hoemann@hpe.com>
> ---
> drivers/acpi/nfit.h | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/acpi/nfit.h b/drivers/acpi/nfit.h
> index c75576b..10dd0d3 100644
> --- a/drivers/acpi/nfit.h
> +++ b/drivers/acpi/nfit.h
> @@ -23,6 +23,8 @@
>
> #define UUID_NFIT_BUS "2f10e7a4-9e91-11e4-89d3-123b93f75cba"
> #define UUID_NFIT_DIMM "4309ac30-0d11-11e4-9191-0800200c9a66"
> +#define UUID_NFIT_DIMM_N_HPE1 "9002c334-acf3-4c0e-9642-a235f0d53bc6"
> +#define UUID_NFIT_DIMM_N_HPE2 "5008664B-B758-41A0-A03C-27C2F2D04F7E"
I'll lowercase this definition when applying the patch.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v8 05/10] nvdimm: Add IOCTL pass thru functions
2016-03-21 19:37 ` [PATCH v8 05/10] nvdimm: Add IOCTL pass thru functions Jerry Hoemann
@ 2016-04-11 19:15 ` Dan Williams
2016-04-11 23:43 ` Jerry Hoemann
0 siblings, 1 reply; 29+ messages in thread
From: Dan Williams @ 2016-04-11 19:15 UTC (permalink / raw)
To: Jerry Hoemann; +Cc: linux-nvdimm@lists.01.org
On Mon, Mar 21, 2016 at 12:37 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote:
> 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 | 141 ++++++++++++++++++++++++++++++++++++++++++++-------
> drivers/nvdimm/bus.c | 43 +++++++++++++++-
> 2 files changed, 166 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c
> index d0f35e6..9511ab4 100644
> --- a/drivers/acpi/nfit.c
> +++ b/drivers/acpi/nfit.c
> @@ -56,6 +56,21 @@ 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 */
> + u64 mask; /* 0 bit excludes underlying func.*/
> +};
> +
> +struct cmd_family_tbl nfit_cmd_family_tbl[] = {
> + { NFIT_DEV_BUS, ND_TYPE_BUS, 1, ~0UL},
Per the comment on patch2 lets kill this for now.
> + { NFIT_DEV_DIMM, ND_TYPE_DIMM_INTEL1, 1, ~0UL},
> + { NFIT_DEV_DIMM_N_HPE1, ND_TYPE_DIMM_N_HPE1, 1, ~0UL},
> + { NFIT_DEV_DIMM_N_HPE2, ND_TYPE_DIMM_N_HPE2, 1, ~0UL},
Why does the mask default to all supported? I assume this should be
the known-valid mask for each type, i.e. 0x3fe for
ND_TYPE_DIMM_INTEL1.
> + { -1, -1, -1, 0},
> +};
> +
> static u8 nfit_uuid[NFIT_UUID_MAX][16];
>
> const u8 *to_nfit_uuid(enum nfit_uuids id)
> @@ -64,6 +79,19 @@ const u8 *to_nfit_uuid(enum nfit_uuids id)
> }
> EXPORT_SYMBOL(to_nfit_uuid);
>
> +static const u8 *
> +family_to_uuid(int type)
> +{
> + 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 +199,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 +209,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 +242,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;
> @@ -218,25 +253,49 @@ static int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
> in_buf.buffer.length = 0;
>
> /* libnvdimm has already validated the input envelope */
> - for (i = 0; i < desc->in_num; i++)
> + for (i = 0; i < desc->in_num; i++) {
> 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);
> + dev_dbg(dev, "%s:%s unexpected output object type cmd: %s %llu, type: %d\n",
> + __func__, dimm_name, cmd_name, func, out_obj->type);
> 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);
> + 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,13 +977,60 @@ static struct nvdimm *acpi_nfit_dimm_by_handle(struct acpi_nfit_desc *acpi_desc,
> return NULL;
> }
>
> +
> +/*
> + * determine if the _DSM specified by UUID is supported and return
> + * mask of supported functions in nd_cmd_mask.
> + */
> +
> +static int acpi_nfit_sup_func(acpi_handle handle, const u8 *uuid,
> + int rev, unsigned long *nd_cmd_mask)
> +{
> + int i;
> + u64 mask = 0;
> + union acpi_object *obj;
> +
> + obj = acpi_evaluate_dsm(handle, uuid, rev, 0, NULL);
> + if (!obj)
> + return 0;
> + /* For compatibility, old BIOSes may return an integer */
> + if (obj->type == ACPI_TYPE_INTEGER)
> + mask = obj->integer.value;
> + else if (obj->type == ACPI_TYPE_BUFFER)
> + for (i = 0; i < obj->buffer.length && i < 8; i++)
> + mask |= (((u8)obj->buffer.pointer[i]) << (i * 8));
> + ACPI_FREE(obj);
> +
> + *nd_cmd_mask = mask;
> +
> + return !!mask;
> +}
> +
> +
> +static inline void
> +to_nfit_uuid_msk(acpi_handle handle, struct cmd_family_tbl *tbl,
> + u8 const **cmd_uuid, unsigned long *cmd_mask)
> +{
> + unsigned long mask = 0;
> + 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_nfit_sup_func(handle, uuid, rev, &mask)) {
> + *cmd_mask = mask & tbl[i].mask;
> + *cmd_uuid = uuid;
> + break;
> + }
> + }
> +}
> +
> 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);
> - int i;
>
> nfit_mem->dsm_mask = acpi_desc->dimm_dsm_force_en;
> adev = to_acpi_dev(acpi_desc);
> @@ -939,9 +1045,8 @@ 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 (acpi_check_dsm(adev_dimm->handle, uuid, 1, 1ULL << i))
> - set_bit(i, &nfit_mem->dsm_mask);
> + to_nfit_uuid_msk(adev_dimm->handle, nfit_cmd_family_tbl,
> + &nfit_mem->dsm_uuid, &nfit_mem->dsm_mask);
Why is this replacing acpi_check_dsm() with an open-coded implementation?
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v8 09/10] nvdimm: sysfs shows which dsm support full command ioctl.
2016-03-21 19:37 ` [PATCH v8 09/10] nvdimm: sysfs shows which dsm support full command ioctl Jerry Hoemann
@ 2016-04-11 21:43 ` Dan Williams
2016-04-11 23:58 ` Jerry Hoemann
0 siblings, 1 reply; 29+ messages in thread
From: Dan Williams @ 2016-04-11 21:43 UTC (permalink / raw)
To: Jerry Hoemann; +Cc: linux-nvdimm@lists.01.org
On Mon, Mar 21, 2016 at 12:37 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote:
> NVDIMM DSMs whose functions map to the Intel Example DSM, can
> be called with the ND_IOCTL_.* ioctl interface. Those that
> don't map, can only be called with the pass thru command.
>
> Save this indication in struct nvdimm to be passed to
> the sysfs interface.
>
I'm having trouble understanding the intent of cmd_ioctl. I don't
want a flag for "supports Intel Example DSM", I want a mask of ND
commands the bus provider knows how to handle in its ->ndctl() entry
point for the given dimm. The bus provider is free to provide a
translation of ND command to the bus specific command. It's just an
arbitrary coincidence that the current list of ND commands matches the
Intel ACPI _DSM command format.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v8 00/10] nvdimm: Add an IOCTL pass thru for DSM calls
2016-03-21 19:37 [PATCH v8 00/10] nvdimm: Add an IOCTL pass thru for DSM calls Jerry Hoemann
` (10 preceding siblings ...)
2016-03-29 20:39 ` [PATCH v8 00/10] nvdimm: Add an IOCTL pass thru for DSM calls Jerry Hoemann
@ 2016-04-11 22:19 ` Dan Williams
11 siblings, 0 replies; 29+ messages in thread
From: Dan Williams @ 2016-04-11 22:19 UTC (permalink / raw)
To: Jerry Hoemann; +Cc: linux-nvdimm@lists.01.org
On Mon, Mar 21, 2016 at 12:37 PM, Jerry Hoemann <jerry.hoemann@hpe.com> 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.
>
> 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
These links should appear in the patch changelog that uses details
from the given specification. Otherwise useful information like this
is simply thrown away since the cover letter of a patch series is not
stored in the git history.
> 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 == "10" 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.
>
I have applied the first 4 patches with some fixups to the change log
and squashing the patch4 into patch2. Please base the next revision
of this series on top of the for-4.7/dsm branch:
https://git.kernel.org/cgit/linux/kernel/git/djbw/nvdimm.git/log/?h=for-4.7/dsm
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v8 02/10] nvdimm: Add wrapper for IOCTL pass thru
2016-04-11 18:21 ` Dan Williams
@ 2016-04-11 23:16 ` Jerry Hoemann
2016-04-11 23:23 ` Dan Williams
0 siblings, 1 reply; 29+ messages in thread
From: Jerry Hoemann @ 2016-04-11 23:16 UTC (permalink / raw)
To: Dan Williams; +Cc: linux-nvdimm@lists.01.org
On Mon, Apr 11, 2016 at 11:21:45AM -0700, Dan Williams wrote:
> On Mon, Mar 21, 2016 at 12:37 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote:
> > +
> > +/*
> > + * the list nd_family of commands. The mapping to firmware handles
> > + * is definied in the nfit_cmd_family_tbl
> > + *
> > + */
> > +#define ND_TYPE_BUS 1
> > +#define ND_TYPE_DIMM_INTEL1 2
> > +#define ND_TYPE_DIMM_N_HPE1 3
> > +#define ND_TYPE_DIMM_N_HPE2 4
> > +
> > +
> > +#define ND_IOCTL_CALL _IOWR(ND_IOCTL, ND_CMD_CALL,\
> > + struct nd_cmd_pkg)
> > +
> > +
> > +
> > #endif /* __NDCTL_H__ */
>
> Looks, good, but for now lets assume there will never be such a thing
> as ND_TYPE_BUS commands or an ND_CMD_CALL for the bus. I.e. all the
> commands that currently exist for the bus are commands that the kernel
> wants to handle. In this way, struct nd_cmd_pkg is a solution to the
> problem of commands that the kernel knows exist, but has no use case
> to generate or parse itself. Commands that fit that description
> currently only exist in leaf node implementations.
Would user space application ever want to call commands for the bus?
I assume the answer is yes, as there are ioctls for ND_CMD_ARS_CAP, etc.,
My thoughts here was to make the pass thru uniform, capable of calling
all the dsm functions that a user space agent might want to call. I.e
don't make user call some _DSM functions one way, and other DSM function
another way....
--
-----------------------------------------------------------------------------
Jerry Hoemann Software Engineer Hewlett Packard Enterprise
-----------------------------------------------------------------------------
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v8 02/10] nvdimm: Add wrapper for IOCTL pass thru
2016-04-11 23:16 ` Jerry Hoemann
@ 2016-04-11 23:23 ` Dan Williams
2016-04-12 0:19 ` Jerry Hoemann
0 siblings, 1 reply; 29+ messages in thread
From: Dan Williams @ 2016-04-11 23:23 UTC (permalink / raw)
To: Jerry Hoemann; +Cc: linux-nvdimm@lists.01.org
On Mon, Apr 11, 2016 at 4:16 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote:
> On Mon, Apr 11, 2016 at 11:21:45AM -0700, Dan Williams wrote:
>> On Mon, Mar 21, 2016 at 12:37 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote:
>> > +
>> > +/*
>> > + * the list nd_family of commands. The mapping to firmware handles
>> > + * is definied in the nfit_cmd_family_tbl
>> > + *
>> > + */
>> > +#define ND_TYPE_BUS 1
>> > +#define ND_TYPE_DIMM_INTEL1 2
>> > +#define ND_TYPE_DIMM_N_HPE1 3
>> > +#define ND_TYPE_DIMM_N_HPE2 4
>> > +
>> > +
>> > +#define ND_IOCTL_CALL _IOWR(ND_IOCTL, ND_CMD_CALL,\
>> > + struct nd_cmd_pkg)
>> > +
>> > +
>> > +
>> > #endif /* __NDCTL_H__ */
>>
>> Looks, good, but for now lets assume there will never be such a thing
>> as ND_TYPE_BUS commands or an ND_CMD_CALL for the bus. I.e. all the
>> commands that currently exist for the bus are commands that the kernel
>> wants to handle. In this way, struct nd_cmd_pkg is a solution to the
>> problem of commands that the kernel knows exist, but has no use case
>> to generate or parse itself. Commands that fit that description
>> currently only exist in leaf node implementations.
>
>
> Would user space application ever want to call commands for the bus?
>
> I assume the answer is yes, as there are ioctls for ND_CMD_ARS_CAP, etc.,
>
> My thoughts here was to make the pass thru uniform, capable of calling
> all the dsm functions that a user space agent might want to call. I.e
> don't make user call some _DSM functions one way, and other DSM function
> another way....
>
Indeed that would have been nice had that been the implementation from
the beginning, but binaries using the current more verbose interface
are already shipping in distributions. So, the train has already left
the station on having a "one ioctl interface to rule them all"
interface.
Unless the ACPI working group adds a slew of kernel irrelevant
commands to the bus, I don't see nd_cmd_pkg being needed at that root
device level.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v8 05/10] nvdimm: Add IOCTL pass thru functions
2016-04-11 19:15 ` Dan Williams
@ 2016-04-11 23:43 ` Jerry Hoemann
2016-04-12 0:18 ` Dan Williams
0 siblings, 1 reply; 29+ messages in thread
From: Jerry Hoemann @ 2016-04-11 23:43 UTC (permalink / raw)
To: Dan Williams; +Cc: linux-nvdimm@lists.01.org
On Mon, Apr 11, 2016 at 12:15:14PM -0700, Dan Williams wrote:
> On Mon, Mar 21, 2016 at 12:37 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote:
> > 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 | 141 ++++++++++++++++++++++++++++++++++++++++++++-------
> > drivers/nvdimm/bus.c | 43 +++++++++++++++-
> > 2 files changed, 166 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c
> > index d0f35e6..9511ab4 100644
> > --- a/drivers/acpi/nfit.c
> > +++ b/drivers/acpi/nfit.c
> > @@ -56,6 +56,21 @@ 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 */
> > + u64 mask; /* 0 bit excludes underlying func.*/
> > +};
> > +
> > +struct cmd_family_tbl nfit_cmd_family_tbl[] = {
> > + { NFIT_DEV_BUS, ND_TYPE_BUS, 1, ~0UL},
>
> Per the comment on patch2 lets kill this for now.
>
> > + { NFIT_DEV_DIMM, ND_TYPE_DIMM_INTEL1, 1, ~0UL},
> > + { NFIT_DEV_DIMM_N_HPE1, ND_TYPE_DIMM_N_HPE1, 1, ~0UL},
> > + { NFIT_DEV_DIMM_N_HPE2, ND_TYPE_DIMM_N_HPE2, 1, ~0UL},
>
> Why does the mask default to all supported? I assume this should be
> the known-valid mask for each type, i.e. 0x3fe for
> ND_TYPE_DIMM_INTEL1.
innocent until proven guilty.
This mask is bit and'd with mask returned by firmware, so mask
returned by firmware limits callers to those function firmware
supports.
My having this extra mask, it allows kernel a way to prevent user space
application from calling a certain function totally. e.g. if we
found that a function is dangerous or something that couldn't be
coordinated with the kernel's use of the function.
By not restricting it to currently defined functions, it allow testing
firmware that might be implemented a new function w/o having to give
firmware team a special kernel.
> >
> > +
> > +/*
> > + * determine if the _DSM specified by UUID is supported and return
> > + * mask of supported functions in nd_cmd_mask.
> > + */
> > +
> > +static int acpi_nfit_sup_func(acpi_handle handle, const u8 *uuid,
> > + int rev, unsigned long *nd_cmd_mask)
> > +{
> > + int i;
> > + u64 mask = 0;
> > + union acpi_object *obj;
> > +
> > + obj = acpi_evaluate_dsm(handle, uuid, rev, 0, NULL);
> > + if (!obj)
> > + return 0;
> > + /* For compatibility, old BIOSes may return an integer */
> > + if (obj->type == ACPI_TYPE_INTEGER)
> > + mask = obj->integer.value;
> > + else if (obj->type == ACPI_TYPE_BUFFER)
> > + for (i = 0; i < obj->buffer.length && i < 8; i++)
> > + mask |= (((u8)obj->buffer.pointer[i]) << (i * 8));
> > + ACPI_FREE(obj);
> > +
> > + *nd_cmd_mask = mask;
> > +
> > + return !!mask;
> > +}
> > +
> > +
> > +static inline void
> > +to_nfit_uuid_msk(acpi_handle handle, struct cmd_family_tbl *tbl,
> > + u8 const **cmd_uuid, unsigned long *cmd_mask)
> > +{
> > + unsigned long mask = 0;
> > + 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_nfit_sup_func(handle, uuid, rev, &mask)) {
> > + *cmd_mask = mask & tbl[i].mask;
> > + *cmd_uuid = uuid;
> > + break;
> > + }
> > + }
> > +}
> > +
> > 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);
> > - int i;
> >
> > nfit_mem->dsm_mask = acpi_desc->dimm_dsm_force_en;
> > adev = to_acpi_dev(acpi_desc);
> > @@ -939,9 +1045,8 @@ 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 (acpi_check_dsm(adev_dimm->handle, uuid, 1, 1ULL << i))
> > - set_bit(i, &nfit_mem->dsm_mask);
> > + to_nfit_uuid_msk(adev_dimm->handle, nfit_cmd_family_tbl,
> > + &nfit_mem->dsm_uuid, &nfit_mem->dsm_mask);
>
> Why is this replacing acpi_check_dsm() with an open-coded implementation?
Not sure of the question.
Is it why acpi_nfit_sup_func? or why to_nfit_uuid_msk?
if why to_nfit_uuid_msk? We need to determine both mask and uuid
(previously uuid was assumed.)
if why acpi_nfit_sup_func? A few reasons.
Want to allow user space to get mask. This is a "safe" way to know
that calls are being made to/returned from firmware.
Didn't want to hard code upper limit on function (at least not less
than having function 0 .. 63 which the acpi layer is doing.)
Finally didn't want to call acpi_check_dsm 64 times per nvdimm. :)
But, we can go back to iterating over 0..63, both work.
Question, if non-root nodes are changed from DSM to LSM, won't
we need to add a function for that?
--
-----------------------------------------------------------------------------
Jerry Hoemann Software Engineer Hewlett Packard Enterprise
-----------------------------------------------------------------------------
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v8 09/10] nvdimm: sysfs shows which dsm support full command ioctl.
2016-04-11 21:43 ` Dan Williams
@ 2016-04-11 23:58 ` Jerry Hoemann
2016-04-12 0:30 ` Dan Williams
0 siblings, 1 reply; 29+ messages in thread
From: Jerry Hoemann @ 2016-04-11 23:58 UTC (permalink / raw)
To: Dan Williams; +Cc: linux-nvdimm@lists.01.org
On Mon, Apr 11, 2016 at 02:43:39PM -0700, Dan Williams wrote:
> On Mon, Mar 21, 2016 at 12:37 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote:
> > NVDIMM DSMs whose functions map to the Intel Example DSM, can
> > be called with the ND_IOCTL_.* ioctl interface. Those that
> > don't map, can only be called with the pass thru command.
> >
> > Save this indication in struct nvdimm to be passed to
> > the sysfs interface.
> >
>
> I'm having trouble understanding the intent of cmd_ioctl. I don't
> want a flag for "supports Intel Example DSM", I want a mask of ND
> commands the bus provider knows how to handle in its ->ndctl() entry
> point for the given dimm. The bus provider is free to provide a
> translation of ND command to the bus specific command. It's just an
> arbitrary coincidence that the current list of ND commands matches the
> Intel ACPI _DSM command format.
This is my extension of your earlier RFC patch, so I may
have misunderstood your original intent.
The commands_show functions prints the listing of commands for that device.
There is one version for root, and one version for non-root.
For root commands, both dsm spec match on names and semantics, so no
code works for both nvdimms types.
But for non-root commands the names/semantics don't match. So, this
is an attempt to show that on an NVDIMM-N, only the pass thru command
is supported.
--
-----------------------------------------------------------------------------
Jerry Hoemann Software Engineer Hewlett Packard Enterprise
-----------------------------------------------------------------------------
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v8 05/10] nvdimm: Add IOCTL pass thru functions
2016-04-11 23:43 ` Jerry Hoemann
@ 2016-04-12 0:18 ` Dan Williams
0 siblings, 0 replies; 29+ messages in thread
From: Dan Williams @ 2016-04-12 0:18 UTC (permalink / raw)
To: Jerry Hoemann; +Cc: linux-nvdimm@lists.01.org
On Mon, Apr 11, 2016 at 4:43 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote:
> On Mon, Apr 11, 2016 at 12:15:14PM -0700, Dan Williams wrote:
>> On Mon, Mar 21, 2016 at 12:37 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote:
>> > 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 | 141 ++++++++++++++++++++++++++++++++++++++++++++-------
>> > drivers/nvdimm/bus.c | 43 +++++++++++++++-
>> > 2 files changed, 166 insertions(+), 18 deletions(-)
>> >
>> > diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c
>> > index d0f35e6..9511ab4 100644
>> > --- a/drivers/acpi/nfit.c
>> > +++ b/drivers/acpi/nfit.c
>> > @@ -56,6 +56,21 @@ 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 */
>> > + u64 mask; /* 0 bit excludes underlying func.*/
>> > +};
>> > +
>> > +struct cmd_family_tbl nfit_cmd_family_tbl[] = {
>> > + { NFIT_DEV_BUS, ND_TYPE_BUS, 1, ~0UL},
>>
>> Per the comment on patch2 lets kill this for now.
>>
>> > + { NFIT_DEV_DIMM, ND_TYPE_DIMM_INTEL1, 1, ~0UL},
>> > + { NFIT_DEV_DIMM_N_HPE1, ND_TYPE_DIMM_N_HPE1, 1, ~0UL},
>> > + { NFIT_DEV_DIMM_N_HPE2, ND_TYPE_DIMM_N_HPE2, 1, ~0UL},
>>
>> Why does the mask default to all supported? I assume this should be
>> the known-valid mask for each type, i.e. 0x3fe for
>> ND_TYPE_DIMM_INTEL1.
>
>
> innocent until proven guilty.
>
> This mask is bit and'd with mask returned by firmware, so mask
> returned by firmware limits callers to those function firmware
> supports.
>
> My having this extra mask, it allows kernel a way to prevent user space
> application from calling a certain function totally. e.g. if we
> found that a function is dangerous or something that couldn't be
> coordinated with the kernel's use of the function.
We should wait to cross that bridge when we come to it, not pre-create
infrastructure without a user. However, we can already re-purpose
this for limiting the supported functions to the ones listed in the
specification. I.e. the kernel rejects undocumented commands by
default.
> By not restricting it to currently defined functions, it allow testing
> firmware that might be implemented a new function w/o having to give
> firmware team a special kernel.
No, the kernel is not going to carry support for undefined function
codes on the chance that maybe firmware wants to test with it at some
point down the road. Just provide the firmware team a local kernel
build.
>> > +
>> > +/*
>> > + * determine if the _DSM specified by UUID is supported and return
>> > + * mask of supported functions in nd_cmd_mask.
>> > + */
>> > +
>> > +static int acpi_nfit_sup_func(acpi_handle handle, const u8 *uuid,
>> > + int rev, unsigned long *nd_cmd_mask)
>> > +{
>> > + int i;
>> > + u64 mask = 0;
>> > + union acpi_object *obj;
>> > +
>> > + obj = acpi_evaluate_dsm(handle, uuid, rev, 0, NULL);
>> > + if (!obj)
>> > + return 0;
>> > + /* For compatibility, old BIOSes may return an integer */
>> > + if (obj->type == ACPI_TYPE_INTEGER)
>> > + mask = obj->integer.value;
>> > + else if (obj->type == ACPI_TYPE_BUFFER)
>> > + for (i = 0; i < obj->buffer.length && i < 8; i++)
>> > + mask |= (((u8)obj->buffer.pointer[i]) << (i * 8));
>> > + ACPI_FREE(obj);
>> > +
>> > + *nd_cmd_mask = mask;
>> > +
>> > + return !!mask;
>> > +}
>> > +
>> > +
>> > +static inline void
>> > +to_nfit_uuid_msk(acpi_handle handle, struct cmd_family_tbl *tbl,
>> > + u8 const **cmd_uuid, unsigned long *cmd_mask)
>> > +{
>> > + unsigned long mask = 0;
>> > + 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_nfit_sup_func(handle, uuid, rev, &mask)) {
>> > + *cmd_mask = mask & tbl[i].mask;
>> > + *cmd_uuid = uuid;
>> > + break;
>> > + }
>> > + }
>> > +}
>> > +
>> > 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);
>> > - int i;
>> >
>> > nfit_mem->dsm_mask = acpi_desc->dimm_dsm_force_en;
>> > adev = to_acpi_dev(acpi_desc);
>> > @@ -939,9 +1045,8 @@ 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 (acpi_check_dsm(adev_dimm->handle, uuid, 1, 1ULL << i))
>> > - set_bit(i, &nfit_mem->dsm_mask);
>> > + to_nfit_uuid_msk(adev_dimm->handle, nfit_cmd_family_tbl,
>> > + &nfit_mem->dsm_uuid, &nfit_mem->dsm_mask);
>>
>> Why is this replacing acpi_check_dsm() with an open-coded implementation?
>
>
> Not sure of the question.
>
> Is it why acpi_nfit_sup_func? or why to_nfit_uuid_msk?
>
> if why to_nfit_uuid_msk? We need to determine both mask and uuid
> (previously uuid was assumed.)
For readability I'd prefer two operations in acpi_nfit_add_dimm():
find_family() followed by probing the supported functions in that
family rather than hiding that detail in a combined to_nfit_uuid_msk()
routine. My preference is that a to_*() routine just do one
conversion and return its value directly, not pass-by-reference
multiple conversions.
> if why acpi_nfit_sup_func? A few reasons.
>
> Want to allow user space to get mask. This is a "safe" way to know
> that calls are being made to/returned from firmware.
>
> Didn't want to hard code upper limit on function (at least not less
> than having function 0 .. 63 which the acpi layer is doing.)
>
> Finally didn't want to call acpi_check_dsm 64 times per nvdimm. :)
> But, we can go back to iterating over 0..63, both work.
If we limit the search to the known / specified function codes as
mentioned above then we won't be searching the entire 64-bit space
per-dimm.
> Question, if non-root nodes are changed from DSM to LSM, won't
> we need to add a function for that?
To me, LSM just seems like yet another 'family'.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v8 02/10] nvdimm: Add wrapper for IOCTL pass thru
2016-04-11 23:23 ` Dan Williams
@ 2016-04-12 0:19 ` Jerry Hoemann
2016-04-12 0:27 ` Dan Williams
0 siblings, 1 reply; 29+ messages in thread
From: Jerry Hoemann @ 2016-04-12 0:19 UTC (permalink / raw)
To: Dan Williams; +Cc: linux-nvdimm@lists.01.org
On Mon, Apr 11, 2016 at 04:23:55PM -0700, Dan Williams wrote:
> On Mon, Apr 11, 2016 at 4:16 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote:
> > On Mon, Apr 11, 2016 at 11:21:45AM -0700, Dan Williams wrote:
> >> On Mon, Mar 21, 2016 at 12:37 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote:
> >> > +
> >> > +/*
> >> > + * the list nd_family of commands. The mapping to firmware handles
> >> > + * is definied in the nfit_cmd_family_tbl
> >> > + *
> >> > + */
> >> > +#define ND_TYPE_BUS 1
> >> > +#define ND_TYPE_DIMM_INTEL1 2
> >> > +#define ND_TYPE_DIMM_N_HPE1 3
> >> > +#define ND_TYPE_DIMM_N_HPE2 4
> >> > +
> >> > +
> >> > +#define ND_IOCTL_CALL _IOWR(ND_IOCTL, ND_CMD_CALL,\
> >> > + struct nd_cmd_pkg)
> >> > +
> >> > +
> >> > +
> >> > #endif /* __NDCTL_H__ */
> >>
> >> Looks, good, but for now lets assume there will never be such a thing
> >> as ND_TYPE_BUS commands or an ND_CMD_CALL for the bus. I.e. all the
> >> commands that currently exist for the bus are commands that the kernel
> >> wants to handle. In this way, struct nd_cmd_pkg is a solution to the
> >> problem of commands that the kernel knows exist, but has no use case
> >> to generate or parse itself. Commands that fit that description
> >> currently only exist in leaf node implementations.
> >
> >
> > Would user space application ever want to call commands for the bus?
> >
> > I assume the answer is yes, as there are ioctls for ND_CMD_ARS_CAP, etc.,
> >
> > My thoughts here was to make the pass thru uniform, capable of calling
> > all the dsm functions that a user space agent might want to call. I.e
> > don't make user call some _DSM functions one way, and other DSM function
> > another way....
> >
>
> Indeed that would have been nice had that been the implementation from
> the beginning, but binaries using the current more verbose interface
> are already shipping in distributions. So, the train has already left
This doesn't break existing binaries. They will still work.
I'm looking at the world going forward. :)
> the station on having a "one ioctl interface to rule them all"
> interface.
>
> Unless the ACPI working group adds a slew of kernel irrelevant
> commands to the bus, I don't see nd_cmd_pkg being needed at that root
> device level.
--
-----------------------------------------------------------------------------
Jerry Hoemann Software Engineer Hewlett Packard Enterprise
-----------------------------------------------------------------------------
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v8 02/10] nvdimm: Add wrapper for IOCTL pass thru
2016-04-12 0:19 ` Jerry Hoemann
@ 2016-04-12 0:27 ` Dan Williams
0 siblings, 0 replies; 29+ messages in thread
From: Dan Williams @ 2016-04-12 0:27 UTC (permalink / raw)
To: Jerry Hoemann; +Cc: linux-nvdimm@lists.01.org
On Mon, Apr 11, 2016 at 5:19 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote:
> On Mon, Apr 11, 2016 at 04:23:55PM -0700, Dan Williams wrote:
>> On Mon, Apr 11, 2016 at 4:16 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote:
>> > On Mon, Apr 11, 2016 at 11:21:45AM -0700, Dan Williams wrote:
>> >> On Mon, Mar 21, 2016 at 12:37 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote:
>> >> > +
>> >> > +/*
>> >> > + * the list nd_family of commands. The mapping to firmware handles
>> >> > + * is definied in the nfit_cmd_family_tbl
>> >> > + *
>> >> > + */
>> >> > +#define ND_TYPE_BUS 1
>> >> > +#define ND_TYPE_DIMM_INTEL1 2
>> >> > +#define ND_TYPE_DIMM_N_HPE1 3
>> >> > +#define ND_TYPE_DIMM_N_HPE2 4
>> >> > +
>> >> > +
>> >> > +#define ND_IOCTL_CALL _IOWR(ND_IOCTL, ND_CMD_CALL,\
>> >> > + struct nd_cmd_pkg)
>> >> > +
>> >> > +
>> >> > +
>> >> > #endif /* __NDCTL_H__ */
>> >>
>> >> Looks, good, but for now lets assume there will never be such a thing
>> >> as ND_TYPE_BUS commands or an ND_CMD_CALL for the bus. I.e. all the
>> >> commands that currently exist for the bus are commands that the kernel
>> >> wants to handle. In this way, struct nd_cmd_pkg is a solution to the
>> >> problem of commands that the kernel knows exist, but has no use case
>> >> to generate or parse itself. Commands that fit that description
>> >> currently only exist in leaf node implementations.
>> >
>> >
>> > Would user space application ever want to call commands for the bus?
>> >
>> > I assume the answer is yes, as there are ioctls for ND_CMD_ARS_CAP, etc.,
>> >
>> > My thoughts here was to make the pass thru uniform, capable of calling
>> > all the dsm functions that a user space agent might want to call. I.e
>> > don't make user call some _DSM functions one way, and other DSM function
>> > another way....
>> >
>>
>> Indeed that would have been nice had that been the implementation from
>> the beginning, but binaries using the current more verbose interface
>> are already shipping in distributions. So, the train has already left
>
> This doesn't break existing binaries. They will still work.
>
> I'm looking at the world going forward. :)
Sure, but I can't delete the old stuff, so adding a parallel
submission path adds maintenance overhead that overshadows the
benefit.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v8 09/10] nvdimm: sysfs shows which dsm support full command ioctl.
2016-04-11 23:58 ` Jerry Hoemann
@ 2016-04-12 0:30 ` Dan Williams
2016-04-12 21:41 ` Jerry Hoemann
0 siblings, 1 reply; 29+ messages in thread
From: Dan Williams @ 2016-04-12 0:30 UTC (permalink / raw)
To: Jerry Hoemann; +Cc: linux-nvdimm@lists.01.org
On Mon, Apr 11, 2016 at 4:58 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote:
> On Mon, Apr 11, 2016 at 02:43:39PM -0700, Dan Williams wrote:
>> On Mon, Mar 21, 2016 at 12:37 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote:
>> > NVDIMM DSMs whose functions map to the Intel Example DSM, can
>> > be called with the ND_IOCTL_.* ioctl interface. Those that
>> > don't map, can only be called with the pass thru command.
>> >
>> > Save this indication in struct nvdimm to be passed to
>> > the sysfs interface.
>> >
>>
>> I'm having trouble understanding the intent of cmd_ioctl. I don't
>> want a flag for "supports Intel Example DSM", I want a mask of ND
>> commands the bus provider knows how to handle in its ->ndctl() entry
>> point for the given dimm. The bus provider is free to provide a
>> translation of ND command to the bus specific command. It's just an
>> arbitrary coincidence that the current list of ND commands matches the
>> Intel ACPI _DSM command format.
>
> This is my extension of your earlier RFC patch, so I may
> have misunderstood your original intent.
>
> The commands_show functions prints the listing of commands for that device.
> There is one version for root, and one version for non-root.
>
> For root commands, both dsm spec match on names and semantics, so no
> code works for both nvdimms types.
>
> But for non-root commands the names/semantics don't match. So, this
> is an attempt to show that on an NVDIMM-N, only the pass thru command
> is supported.
Ah, ok. Rather than telling the core about a dsm_mask and a cmd_ioctl
at nvdimm_create() time, it should just be passing the mask of generic
ND commands that the dimm supports. For NVDIMM-N that mask will just
be (1 << 10) it's up to the nfit driver to manage any ND command to
ACPI _DSM command number translation.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v8 09/10] nvdimm: sysfs shows which dsm support full command ioctl.
2016-04-12 0:30 ` Dan Williams
@ 2016-04-12 21:41 ` Jerry Hoemann
2016-04-12 22:05 ` Dan Williams
0 siblings, 1 reply; 29+ messages in thread
From: Jerry Hoemann @ 2016-04-12 21:41 UTC (permalink / raw)
To: Dan Williams; +Cc: linux-nvdimm@lists.01.org
On Mon, Apr 11, 2016 at 05:30:25PM -0700, Dan Williams wrote:
> On Mon, Apr 11, 2016 at 4:58 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote:
> > On Mon, Apr 11, 2016 at 02:43:39PM -0700, Dan Williams wrote:
> >> On Mon, Mar 21, 2016 at 12:37 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote:
> >> > NVDIMM DSMs whose functions map to the Intel Example DSM, can
> >> > be called with the ND_IOCTL_.* ioctl interface. Those that
> >> > don't map, can only be called with the pass thru command.
> >> >
> >> > Save this indication in struct nvdimm to be passed to
> >> > the sysfs interface.
> >> >
> >>
> >> I'm having trouble understanding the intent of cmd_ioctl. I don't
> >> want a flag for "supports Intel Example DSM", I want a mask of ND
> >> commands the bus provider knows how to handle in its ->ndctl() entry
> >> point for the given dimm. The bus provider is free to provide a
> >> translation of ND command to the bus specific command. It's just an
> >> arbitrary coincidence that the current list of ND commands matches the
> >> Intel ACPI _DSM command format.
> >
> > This is my extension of your earlier RFC patch, so I may
> > have misunderstood your original intent.
> >
> > The commands_show functions prints the listing of commands for that device.
> > There is one version for root, and one version for non-root.
> >
> > For root commands, both dsm spec match on names and semantics, so no
> > code works for both nvdimms types.
> >
> > But for non-root commands the names/semantics don't match. So, this
> > is an attempt to show that on an NVDIMM-N, only the pass thru command
> > is supported.
>
> Ah, ok. Rather than telling the core about a dsm_mask and a cmd_ioctl
> at nvdimm_create() time, it should just be passing the mask of generic
> ND commands that the dimm supports. For NVDIMM-N that mask will just
> be (1 << 10) it's up to the nfit driver to manage any ND command to
> ACPI _DSM command number translation.
dsm_mask is used in both acpi_nfit_ctl and __nd_ioctl to filter calls
the functions might make. I think dsm_mask and cmd_ioctl need to be
kept separate, or drop use of filtering by dsm_mask in these two
functions.
Or another way of controlling what commands_show produces?
--
-----------------------------------------------------------------------------
Jerry Hoemann Software Engineer Hewlett Packard Enterprise
-----------------------------------------------------------------------------
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v8 09/10] nvdimm: sysfs shows which dsm support full command ioctl.
2016-04-12 21:41 ` Jerry Hoemann
@ 2016-04-12 22:05 ` Dan Williams
2016-04-14 22:58 ` Jerry Hoemann
0 siblings, 1 reply; 29+ messages in thread
From: Dan Williams @ 2016-04-12 22:05 UTC (permalink / raw)
To: Jerry Hoemann; +Cc: linux-nvdimm@lists.01.org
On Tue, Apr 12, 2016 at 2:41 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote:
> On Mon, Apr 11, 2016 at 05:30:25PM -0700, Dan Williams wrote:
>> On Mon, Apr 11, 2016 at 4:58 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote:
>> > On Mon, Apr 11, 2016 at 02:43:39PM -0700, Dan Williams wrote:
>> >> On Mon, Mar 21, 2016 at 12:37 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote:
>> >> > NVDIMM DSMs whose functions map to the Intel Example DSM, can
>> >> > be called with the ND_IOCTL_.* ioctl interface. Those that
>> >> > don't map, can only be called with the pass thru command.
>> >> >
>> >> > Save this indication in struct nvdimm to be passed to
>> >> > the sysfs interface.
>> >> >
>> >>
>> >> I'm having trouble understanding the intent of cmd_ioctl. I don't
>> >> want a flag for "supports Intel Example DSM", I want a mask of ND
>> >> commands the bus provider knows how to handle in its ->ndctl() entry
>> >> point for the given dimm. The bus provider is free to provide a
>> >> translation of ND command to the bus specific command. It's just an
>> >> arbitrary coincidence that the current list of ND commands matches the
>> >> Intel ACPI _DSM command format.
>> >
>> > This is my extension of your earlier RFC patch, so I may
>> > have misunderstood your original intent.
>> >
>> > The commands_show functions prints the listing of commands for that device.
>> > There is one version for root, and one version for non-root.
>> >
>> > For root commands, both dsm spec match on names and semantics, so no
>> > code works for both nvdimms types.
>> >
>> > But for non-root commands the names/semantics don't match. So, this
>> > is an attempt to show that on an NVDIMM-N, only the pass thru command
>> > is supported.
>>
>> Ah, ok. Rather than telling the core about a dsm_mask and a cmd_ioctl
>> at nvdimm_create() time, it should just be passing the mask of generic
>> ND commands that the dimm supports. For NVDIMM-N that mask will just
>> be (1 << 10) it's up to the nfit driver to manage any ND command to
>> ACPI _DSM command number translation.
>
> dsm_mask is used in both acpi_nfit_ctl and __nd_ioctl to filter calls
> the functions might make. I think dsm_mask and cmd_ioctl need to be
> kept separate, or drop use of filtering by dsm_mask in these two
> functions.
>
> Or another way of controlling what commands_show produces?
I was thinking that the dsm_mask cease being a pointer in struct
nvdimm and instead be a cmd_mask (of ND function numbers). struct
nfit_mem would maintain the dsm_mask. acpi_nfit_ctl is charged with
translating from the nd command to the acpi dsm. This "just works"
for the existing sysfs interface, and we can export the dsm_mask in
the nfit-specific attributes of the nme device.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v8 09/10] nvdimm: sysfs shows which dsm support full command ioctl.
2016-04-12 22:05 ` Dan Williams
@ 2016-04-14 22:58 ` Jerry Hoemann
2016-04-15 2:49 ` Dan Williams
0 siblings, 1 reply; 29+ messages in thread
From: Jerry Hoemann @ 2016-04-14 22:58 UTC (permalink / raw)
To: Dan Williams; +Cc: linux-nvdimm@lists.01.org
On Tue, Apr 12, 2016 at 03:05:47PM -0700, Dan Williams wrote:
> On Tue, Apr 12, 2016 at 2:41 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote:
> > On Mon, Apr 11, 2016 at 05:30:25PM -0700, Dan Williams wrote:
> >> On Mon, Apr 11, 2016 at 4:58 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote:
> >> > On Mon, Apr 11, 2016 at 02:43:39PM -0700, Dan Williams wrote:
> >> >> On Mon, Mar 21, 2016 at 12:37 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote:
> >> >> > NVDIMM DSMs whose functions map to the Intel Example DSM, can
> >> >> > be called with the ND_IOCTL_.* ioctl interface. Those that
> >> >> > don't map, can only be called with the pass thru command.
> >> >> >
> >> >> > Save this indication in struct nvdimm to be passed to
> >> >> > the sysfs interface.
> >> >> >
> >> >>
> >> >> I'm having trouble understanding the intent of cmd_ioctl. I don't
> >> >> want a flag for "supports Intel Example DSM", I want a mask of ND
> >> >> commands the bus provider knows how to handle in its ->ndctl() entry
> >> >> point for the given dimm. The bus provider is free to provide a
> >> >> translation of ND command to the bus specific command. It's just an
> >> >> arbitrary coincidence that the current list of ND commands matches the
> >> >> Intel ACPI _DSM command format.
> >> >
> >> > This is my extension of your earlier RFC patch, so I may
> >> > have misunderstood your original intent.
> >> >
> >> > The commands_show functions prints the listing of commands for that device.
> >> > There is one version for root, and one version for non-root.
> >> >
> >> > For root commands, both dsm spec match on names and semantics, so no
> >> > code works for both nvdimms types.
> >> >
> >> > But for non-root commands the names/semantics don't match. So, this
> >> > is an attempt to show that on an NVDIMM-N, only the pass thru command
> >> > is supported.
> >>
> >> Ah, ok. Rather than telling the core about a dsm_mask and a cmd_ioctl
> >> at nvdimm_create() time, it should just be passing the mask of generic
> >> ND commands that the dimm supports. For NVDIMM-N that mask will just
> >> be (1 << 10) it's up to the nfit driver to manage any ND command to
> >> ACPI _DSM command number translation.
> >
> > dsm_mask is used in both acpi_nfit_ctl and __nd_ioctl to filter calls
> > the functions might make. I think dsm_mask and cmd_ioctl need to be
> > kept separate, or drop use of filtering by dsm_mask in these two
> > functions.
> >
> > Or another way of controlling what commands_show produces?
>
> I was thinking that the dsm_mask cease being a pointer in struct
> nvdimm and instead be a cmd_mask (of ND function numbers). struct
> nfit_mem would maintain the dsm_mask. acpi_nfit_ctl is charged with
> translating from the nd command to the acpi dsm. This "just works"
> for the existing sysfs interface, and we can export the dsm_mask in
> the nfit-specific attributes of the nme device.
Not sure I understand.
Are you suggesting that struct nfit_mem have both a dsm_mask and a cmd_mask
and when nvdimm_create is called we pass just cmd_mask and assign it to
new field u64 cmd_mask (deleting the old pointer to dsm_mask?)
For NVDIMM-N, the cmd_mask would just be (1<<ND_CMD_CALL) and for the
pre-existent nvdimm it would be (1<<ND_CMD_CALL)|dsm_mask
This would have acpi_nfit_ctl testing against cmd_mask and __nd_ioctl
testing against dsm_mask.
I don't know how nvdimm_create would get the nd_cmd_mask if its
not determined in acpi_nfit_add_dimm when we determine uuid......
--
-----------------------------------------------------------------------------
Jerry Hoemann Software Engineer Hewlett Packard Enterprise
-----------------------------------------------------------------------------
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v8 09/10] nvdimm: sysfs shows which dsm support full command ioctl.
2016-04-14 22:58 ` Jerry Hoemann
@ 2016-04-15 2:49 ` Dan Williams
0 siblings, 0 replies; 29+ messages in thread
From: Dan Williams @ 2016-04-15 2:49 UTC (permalink / raw)
To: Jerry Hoemann; +Cc: linux-nvdimm@lists.01.org
On Thu, Apr 14, 2016 at 3:58 PM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote:
> On Tue, Apr 12, 2016 at 03:05:47PM -0700, Dan Williams wrote:
[..]
>> I was thinking that the dsm_mask cease being a pointer in struct
>> nvdimm and instead be a cmd_mask (of ND function numbers). struct
>> nfit_mem would maintain the dsm_mask. acpi_nfit_ctl is charged with
>> translating from the nd command to the acpi dsm. This "just works"
>> for the existing sysfs interface, and we can export the dsm_mask in
>> the nfit-specific attributes of the nme device.
>
> Not sure I understand.
>
> Are you suggesting that struct nfit_mem have both a dsm_mask and a cmd_mask
> and when nvdimm_create is called we pass just cmd_mask and assign it to
> new field u64 cmd_mask (deleting the old pointer to dsm_mask?)
Yes.
> For NVDIMM-N, the cmd_mask would just be (1<<ND_CMD_CALL) and for the
> pre-existent nvdimm it would be (1<<ND_CMD_CALL)|dsm_mask
>
Yes.
> This would have acpi_nfit_ctl testing against cmd_mask and __nd_ioctl
> testing against dsm_mask.
__nd_ioctl would only test against cmd_mask.
acpi_nfit_ctl would check if it can translate the cmd to a DSM. In
the case of ND_CMD_SMART..ND_CMD_VENDOR it translate to the
corresponding DSM (currently a nop for Intel DIMMs and not applicable
to HP DIMMs). In the case of ND_CMD_CALL it will check the nfit_mem's
dsm_mask to see if the DSM is supported and passes it through.
> I don't know how nvdimm_create would get the nd_cmd_mask if its
> not determined in acpi_nfit_add_dimm when we determine uuid......
nfit_mem will have a cmd_mask and dsm_mask both initialized at
acpi_nfit_add_dimm() time. At nvdimm_create we just pass
nfit_mem->cmd_mask.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2016-04-15 2:49 UTC | newest]
Thread overview: 29+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-21 19:37 [PATCH v8 00/10] nvdimm: Add an IOCTL pass thru for DSM calls Jerry Hoemann
2016-03-21 19:37 ` [PATCH v8 01/10] ACPI / util: Fix acpi_evaluate_dsm() argument type Jerry Hoemann
2016-03-21 19:37 ` [PATCH v8 02/10] nvdimm: Add wrapper for IOCTL pass thru Jerry Hoemann
2016-04-11 18:21 ` Dan Williams
2016-04-11 23:16 ` Jerry Hoemann
2016-04-11 23:23 ` Dan Williams
2016-04-12 0:19 ` Jerry Hoemann
2016-04-12 0:27 ` Dan Williams
2016-03-21 19:37 ` [PATCH v8 03/10] nvdimm: Increase max envelope size for IOCTL Jerry Hoemann
2016-03-21 19:37 ` [PATCH v8 04/10] nvdimm: Add UUIDs Jerry Hoemann
2016-04-11 18:25 ` Dan Williams
2016-03-21 19:37 ` [PATCH v8 05/10] nvdimm: Add IOCTL pass thru functions Jerry Hoemann
2016-04-11 19:15 ` Dan Williams
2016-04-11 23:43 ` Jerry Hoemann
2016-04-12 0:18 ` Dan Williams
2016-03-21 19:37 ` [PATCH v8 06/10] libnvdimm: nvdimm_bus_descriptor field name change Jerry Hoemann
2016-03-21 19:37 ` [PATCH v8 07/10] tools/testing/nvdimm: 'call_dsm' support Jerry Hoemann
2016-03-21 19:37 ` [PATCH v8 08/10] nvdimm: command ioctl support Jerry Hoemann
2016-03-21 19:37 ` [PATCH v8 09/10] nvdimm: sysfs shows which dsm support full command ioctl Jerry Hoemann
2016-04-11 21:43 ` Dan Williams
2016-04-11 23:58 ` Jerry Hoemann
2016-04-12 0:30 ` Dan Williams
2016-04-12 21:41 ` Jerry Hoemann
2016-04-12 22:05 ` Dan Williams
2016-04-14 22:58 ` Jerry Hoemann
2016-04-15 2:49 ` Dan Williams
2016-03-21 20:55 ` [PATCH v8 10/10] nvdimm: Add ioctl to return command mask Jerry Hoemann
2016-03-29 20:39 ` [PATCH v8 00/10] nvdimm: Add an IOCTL pass thru for DSM calls Jerry Hoemann
2016-04-11 22:19 ` Dan Williams
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox