[ndctl PATCH v3 0/5] show broken dimm info with translate SPA feature.

[ndctl PATCH v3 0/5] show broken dimm info with translate SPA feature.

[Yasunori Goto]

Hi,

I wrote v3 patch set to show broken NVDIMM for ndctl list command.
Please check it.

---
Change log since v2 [1]:
 - Make libndctl-nfit.h and libndctl-nfit.c and define new interfaces
   which use translate spa on them.
 - Add sanity checks for new interfaces.
 - Fix some names
      o trans_spa -> translate_spa
      o sub_cmd -> passthru_cmd
   

Change log since v1 [2]:
 - Use ND_CMD_CALL to call translate SPA feature.
 - Separate patch set of ndctl from kernel patch set.
 - Add a interface to check what feature can call via ND_CMD_CALL by reading
   /device/nfit/dsm_mask 
 - Get only one nvdimm handle and DPA via ioctl() for the time being.
 - Bug fix which i found
     fix calculation of SPA from bad block at dev_badblocks_to_json().



[1] https://www.mail-archive.com/linux-nvdimm@lists.01.org/msg05577.html
[2] https://www.mail-archive.com/linux-nvdimm@lists.01.org/msg05287.html


----
This patch set is to show information of broken NVDIMM on ndctl.

When a region has a broken block, user needs to replace
the NVDIMM which includes the block.
However there is no information to find which DIMM module have the block.
Not only ndctl does not have such information, nvdimm driver can not
find it.


Fortunately, ACPI 6.2 has new specification of _DSM.
It is "translate spa" which can get NVDIMM handle
and DPA(Dimm Physical Address) from SPA(system Physical Addreess).
It helps for ndctl command to find broken NVDIMM.

This patch set includes followings.
 - some preparations to call Translate SPA via ND_CMD_CALL.
 - libndctl-nfit supports Translate SPA interface,
 - ndctl list command show bad DIMM with Translate SPA.


Thanks,
---
Yasunori Goto




_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[ndctl PATCH 1/5] Introduce libndctl-nfit.h

[Yasunori Goto]

This patch introduces libndctl-nfit.h.

Since these command can be executed via ND_CMD_CALL,
libndctl.h which is shared between ndctl command and kernel does not
have to include these defintions.

So, libndctl-nfit.h, which is defined for only ndctl, is created instead,
and move definitions from ndctl.h to it.


Signed-off-by: Yasunori Goto <y-goto@jp.fujitsu.com>

---
 ndctl/lib/libndctl-nfit.h | 61 +++++++++++++++++++++++++++++++++++++++++++++++
 ndctl/ndctl.h             | 37 ----------------------------
 2 files changed, 61 insertions(+), 37 deletions(-)

diff --git a/ndctl/lib/libndctl-nfit.h b/ndctl/lib/libndctl-nfit.h
new file mode 100644
index 0000000..1398662
--- /dev/null
+++ b/ndctl/lib/libndctl-nfit.h
@@ -0,0 +1,61 @@
+/*
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU Lesser General Public License,
+ * version 2.1, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT ANY
+ * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public License for
+ * more details.
+ */
+#ifndef __LIBNDCTL_NFIT_H__
+#define __LIBNDCTL_NFIT_H__
+
+#define ND_TRANSLATE_SPA_STATUS_INVALID_SPA  2
+
+/* bus passthru commands */
+enum {
+	NFIT_CMD_TRANSLATE_SPA = 5,
+	NFIT_CMD_ARS_INJECT_SET = 7,
+	NFIT_CMD_ARS_INJECT_CLEAR = 8,
+	NFIT_CMD_ARS_INJECT_GET = 9,
+};
+
+struct nd_cmd_translate_spa {
+	__u64 spa;
+	__u32 status;
+	__u8  flags;
+	__u8  _reserved[3];
+	__u64 translate_length;
+	__u32 num_nvdimms;
+	struct nd_nvdimm_device {
+		__u32 nfit_device_handle;
+		__u32 _reserved;
+		__u64 dpa;
+	} __attribute__((packed)) devices[0];
+
+} __attribute__((packed));
+
+struct nd_cmd_ars_err_inj {
+	__u64 err_inj_spa_range_base;
+	__u64 err_inj_spa_range_length;
+	__u8  err_inj_options;
+	__u32 status;
+} __attribute__((packed));
+
+struct nd_cmd_ars_err_inj_clr {
+	__u64 err_inj_clr_spa_range_base;
+	__u64 err_inj_clr_spa_range_length;
+	__u32 status;
+} __attribute__((packed));
+
+struct nd_cmd_ars_err_inj_stat {
+	__u32 status;
+	__u32 inj_err_rec_count;
+	struct nd_error_stat_query_record {
+		__u64 err_inj_stat_spa_range_base;
+		__u64 err_inj_stat_spa_range_length;
+	} __attribute__((packed)) record[0];
+} __attribute__((packed));
+
+#endif /* __LIBNDCTL_NFIT_H__ */
diff --git a/ndctl/ndctl.h b/ndctl/ndctl.h
index d70b97d..2dd461b 100644
--- a/ndctl/ndctl.h
+++ b/ndctl/ndctl.h
@@ -145,43 +145,6 @@ struct nd_cmd_clear_error {
 	__u64 cleared;
 } __attribute__((packed));
 
-struct nd_cmd_trans_spa {
-	__u64 spa;
-	__u32 status;
-	__u8  flags;
-	__u8  _reserved[3];
-	__u64 trans_length;
-	__u32 num_nvdimms;
-	struct nd_nvdimm_device {
-		__u32 nfit_device_handle;
-		__u32 _reserved;
-		__u64 dpa;
-	} __attribute__((packed)) devices[0];
-
-} __attribute__((packed));
-
-struct nd_cmd_ars_err_inj {
-	__u64 err_inj_spa_range_base;
-	__u64 err_inj_spa_range_length;
-	__u8  err_inj_options;
-	__u32 status;
-} __attribute__((packed));
-
-struct nd_cmd_ars_err_inj_clr {
-	__u64 err_inj_clr_spa_range_base;
-	__u64 err_inj_clr_spa_range_length;
-	__u32 status;
-} __attribute__((packed));
-
-struct nd_cmd_ars_err_inj_stat {
-	__u32 status;
-	__u32 inj_err_rec_count;
-	struct nd_error_stat_query_record {
-		__u64 err_inj_stat_spa_range_base;
-		__u64 err_inj_stat_spa_range_length;
-	} __attribute__((packed)) record[0];
-} __attribute__((packed));
-
 enum {
 	ND_CMD_IMPLEMENTED = 0,
 



_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[ndctl PATCH 2/5] make interface to check device/nfit/dsm_mask flags

[Yasunori Goto]

To check what feature can be called via ND_CMD_CALL, ndctl need to read
device/nfit/dsm_mask. This patch make an interface to check it in libndctl.c


Signed-off-by: Yasunori Goto <y-goto@jp.fujitsu.com>

---
 ndctl/lib/libndctl.c   | 13 +++++++++++++
 ndctl/lib/libndctl.sym |  1 +
 ndctl/libndctl.h.in    |  1 +
 3 files changed, 15 insertions(+)

diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index c2e0efb..803056d 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -102,6 +102,7 @@ struct ndctl_bus {
 	size_t buf_len;
 	char *wait_probe_path;
 	unsigned long dsm_mask;
+	unsigned long passthru_dsm_mask;
 };
 
 /**
@@ -846,6 +847,12 @@ static void *add_bus(void *parent, int id, const char *ctl_base)
 		bus->revision = strtoul(buf, NULL, 0);
 	}
 
+	sprintf(path, "%s/device/nfit/dsm_mask", ctl_base);
+	if (sysfs_read_attr(ctx, path, buf) < 0)
+		bus->passthru_dsm_mask = 0;
+	else
+		bus->passthru_dsm_mask = strtoul(buf, NULL, 16);
+
 	sprintf(path, "%s/device/provider", ctl_base);
 	if (sysfs_read_attr(ctx, path, buf) < 0)
 		goto err_read;
@@ -1101,6 +1108,12 @@ NDCTL_EXPORT int ndctl_bus_is_cmd_supported(struct ndctl_bus *bus,
 	return !!(bus->dsm_mask & (1ULL << cmd));
 }
 
+NDCTL_EXPORT int ndctl_bus_is_passthru_cmd_supported(struct ndctl_bus *bus,
+		int cmd)
+{
+	return !!(bus->passthru_dsm_mask & (1ULL << cmd));
+}
+
 NDCTL_EXPORT unsigned int ndctl_bus_get_revision(struct ndctl_bus *bus)
 {
 	return bus->revision;
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index b8ac65f..48b0a2f 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -30,6 +30,7 @@ global:
 	ndctl_bus_get_by_provider;
 	ndctl_bus_get_cmd_name;
 	ndctl_bus_is_cmd_supported;
+	ndctl_bus_is_passthru_cmd_supported;
 	ndctl_bus_has_nfit;
 	ndctl_bus_get_revision;
 	ndctl_bus_get_id;
diff --git a/ndctl/libndctl.h.in b/ndctl/libndctl.h.in
index 855d883..49b73a4 100644
--- a/ndctl/libndctl.h.in
+++ b/ndctl/libndctl.h.in
@@ -109,6 +109,7 @@ struct ndctl_bus *ndctl_bus_get_by_provider(struct ndctl_ctx *ctx,
 		const char *provider);
 const char *ndctl_bus_get_cmd_name(struct ndctl_bus *bus, int cmd);
 int ndctl_bus_is_cmd_supported(struct ndctl_bus *bus, int cmd);
+int ndctl_bus_is_passthru_cmd_supported(struct ndctl_bus *bus, int cmd);
 unsigned int ndctl_bus_get_revision(struct ndctl_bus *bus);
 unsigned int ndctl_bus_get_id(struct ndctl_bus *bus);
 const char *ndctl_bus_get_provider(struct ndctl_bus *bus);



_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[ndctl PATCH 3/5] allow ND_CMD_CALL for bus

[Yasunori Goto]

Currently ndctl supports ND_CMD_CALL only for DIMM,
but Translate SPA is the feature of bus.
So ND_CMD_CALL must be allowed bus's ioctl().


Signed-off-by: Yasunori Goto <y-goto@jp.fujitsu.com>
---
 ndctl/lib/libndctl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index 803056d..e3dcb1c 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -2330,6 +2330,7 @@ static int to_ioctl_cmd(int cmd, int dimm)
 #ifdef HAVE_NDCTL_CLEAR_ERROR
 		case ND_CMD_CLEAR_ERROR:     return ND_IOCTL_CLEAR_ERROR;
 #endif
+		case ND_CMD_CALL:            return ND_IOCTL_CALL;
 		default:
 						       return 0;
 		};



_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[ndctl PATCH 4/5] Make interfaces to use Translate SPA.

[Yasunori Goto]

This patch makes 2 new interfaces :
  - Call translate SPA featture of ACPI 6.2.
  - Find DIMM which SPA(System Physical Address) belongs to.


Signed-off-by: Yasunori Goto <y-goto@jp.fujitsu.com>
---
 ndctl/Makefile.am         |   1 +
 ndctl/lib/Makefile.am     |   4 +-
 ndctl/lib/libndctl-nfit.c | 147 ++++++++++++++++++++++++++++++++++++++++++++++
 ndctl/lib/libndctl-nfit.h |   5 ++
 4 files changed, 156 insertions(+), 1 deletion(-)

diff --git a/ndctl/Makefile.am b/ndctl/Makefile.am
index d346c04..20d5f59 100644
--- a/ndctl/Makefile.am
+++ b/ndctl/Makefile.am
@@ -25,6 +25,7 @@ endif
 
 ndctl_LDADD =\
 	lib/libndctl.la \
+	lib/libndctl-nfit.la \
 	../daxctl/lib/libdaxctl.la \
 	../libutil.a \
 	$(UUID_LIBS) \
diff --git a/ndctl/lib/Makefile.am b/ndctl/lib/Makefile.am
index 7a446be..cfa54ae 100644
--- a/ndctl/lib/Makefile.am
+++ b/ndctl/lib/Makefile.am
@@ -8,7 +8,7 @@ BUILT_SOURCES = ../libndctl.h
 	$(SED_PROCESS)
 
 pkginclude_HEADERS = ../libndctl.h
-lib_LTLIBRARIES = libndctl.la
+lib_LTLIBRARIES = libndctl.la libndctl-nfit.la
 
 libndctl_la_SOURCES =\
 	libndctl.h \
@@ -35,6 +35,8 @@ libndctl_la_SOURCES += libndctl-hpe1.c
 libndctl_la_SOURCES += libndctl-msft.c
 endif
 
+libndctl_nfit_la_SOURCES = libndctl-nfit.c
+
 EXTRA_DIST += libndctl.sym
 
 libndctl_la_LDFLAGS = $(AM_LDFLAGS) \
diff --git a/ndctl/lib/libndctl-nfit.c b/ndctl/lib/libndctl-nfit.c
new file mode 100644
index 0000000..455815d
--- /dev/null
+++ b/ndctl/lib/libndctl-nfit.c
@@ -0,0 +1,147 @@
+/*
+ * Copyright (c) 2017, FUJITSU LIMITED. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU Lesser General Public License,
+ * version 2.1, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT ANY
+ * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public License for
+ * more details.
+ */
+#include <stdlib.h>
+#include <ndctl/libndctl.h>
+#include "libndctl-private.h"
+#include "libndctl-nfit.h"
+
+static int bus_has_translate_spa(struct ndctl_bus *bus)
+{
+	if (!ndctl_bus_has_nfit(bus))
+		return 0;
+
+	return ndctl_bus_is_passthru_cmd_supported(bus, NFIT_CMD_TRANSLATE_SPA);
+}
+
+static struct ndctl_cmd *ndctl_bus_cmd_new_translate_spa(struct ndctl_bus *bus)
+{
+	struct ndctl_cmd *cmd;
+	struct nd_cmd_pkg *pkg;
+	struct nd_cmd_translate_spa *translate_spa;
+	size_t size, spa_length;
+
+	spa_length = sizeof(struct nd_cmd_translate_spa)
+		+ sizeof(struct nd_nvdimm_device);
+	size = sizeof(*cmd) + sizeof(*pkg) + spa_length;
+	cmd = calloc(1, size);
+	if (!cmd)
+		return NULL;
+
+	cmd->bus = bus;
+	ndctl_cmd_ref(cmd);
+	cmd->type = ND_CMD_CALL;
+	cmd->size = size;
+	cmd->status = 1;
+	pkg = (struct nd_cmd_pkg *)&cmd->cmd_buf[0];
+	pkg->nd_command = NFIT_CMD_TRANSLATE_SPA;
+	pkg->nd_size_in = sizeof(unsigned long long);
+	pkg->nd_size_out = spa_length;
+	pkg->nd_fw_size = spa_length;
+	translate_spa = (struct nd_cmd_translate_spa *)&pkg->nd_payload[0];
+	cmd->firmware_status = &translate_spa->status;
+	translate_spa->translate_length = spa_length;
+
+	return cmd;
+}
+
+static int ndctl_bus_cmd_get_translate_spa(struct ndctl_cmd *cmd,
+					unsigned int *handle, unsigned long long *dpa)
+{
+	struct nd_cmd_pkg *pkg;
+	struct nd_cmd_translate_spa *translate_spa;
+
+	pkg = (struct nd_cmd_pkg *)&cmd->cmd_buf[0];
+	translate_spa = (struct nd_cmd_translate_spa *)&pkg->nd_payload[0];
+
+	if (translate_spa->status == ND_TRANSLATE_SPA_STATUS_INVALID_SPA)
+		return -EINVAL;
+
+	/*
+	 * XXX: Currently NVDIMM mirroring is not supported.
+	 * Even if ACPI returned plural dimms due to mirroring,
+	 * this function returns just the first dimm.
+	 */
+
+	*handle = translate_spa->devices[0].nfit_device_handle;
+	*dpa = translate_spa->devices[0].dpa;
+
+	return 0;
+}
+
+static int is_valid_spa(struct ndctl_bus *bus, unsigned long long spa)
+{
+	struct ndctl_region *region;
+	unsigned long long region_start, region_end;
+
+	ndctl_region_foreach(bus, region) {
+		region_start = ndctl_region_get_resource(region);
+		region_end = region_start + ndctl_region_get_size(region);
+		if (region_start <= spa && spa < region_end)
+			return 1;
+	}
+
+	return 0;
+}
+
+NDCTL_EXPORT int ndctl_bus_cmd_translate_spa(struct ndctl_bus *bus,
+	unsigned long long addr, unsigned int *handle, unsigned long long *dpa)
+{
+
+	struct ndctl_cmd *cmd;
+	struct nd_cmd_pkg *pkg;
+	struct nd_cmd_translate_spa *translate_spa;
+	int rc;
+
+	if (!bus || !handle || !dpa)
+		return -EINVAL;
+
+	if (!bus_has_translate_spa(bus))
+		return -ENOTTY;
+
+	if (!is_valid_spa(bus, addr))
+		return -EINVAL;
+
+	cmd = ndctl_bus_cmd_new_translate_spa(bus);
+	if (!cmd)
+		return -ENOMEM;
+
+	pkg = (struct nd_cmd_pkg *)&cmd->cmd_buf[0];
+	translate_spa = (struct nd_cmd_translate_spa *)&pkg->nd_payload[0];
+	translate_spa->spa = addr;
+
+	rc = ndctl_cmd_submit(cmd);
+	if (rc) {
+		ndctl_cmd_unref(cmd);
+		return rc;
+	}
+
+	rc = ndctl_bus_cmd_get_translate_spa(cmd, handle, dpa);
+	ndctl_cmd_unref(cmd);
+
+	return rc;
+}
+
+NDCTL_EXPORT struct ndctl_dimm *ndctl_dimm_get_by_spa(struct ndctl_bus *bus,
+		unsigned long long spa)
+{
+	int rc;
+	unsigned int handle;
+	unsigned long long dpa;
+
+	/* ndctl_bus_cmd_translate_spa() has sanity check */
+	rc = ndctl_bus_cmd_translate_spa(bus, spa, &handle, &dpa);
+	if (rc)
+		return NULL;
+
+	return ndctl_dimm_get_by_handle(bus, handle);
+}
diff --git a/ndctl/lib/libndctl-nfit.h b/ndctl/lib/libndctl-nfit.h
index 1398662..4a1dac6 100644
--- a/ndctl/lib/libndctl-nfit.h
+++ b/ndctl/lib/libndctl-nfit.h
@@ -58,4 +58,9 @@ struct nd_cmd_ars_err_inj_stat {
 	} __attribute__((packed)) record[0];
 } __attribute__((packed));
 
+int ndctl_bus_cmd_translate_spa(struct ndctl_bus *bus,
+	unsigned long long addr, unsigned int *handle, unsigned long long *dpa);
+struct ndctl_dimm *ndctl_dimm_get_by_spa(struct ndctl_bus *bus,
+	unsigned long long spa);
+
 #endif /* __LIBNDCTL_NFIT_H__ */



_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[ndctl PATCH 5/5] show bad dimm's name by ndctl list command.

[Yasunori Goto]

Show dimm's name which has badblock by ndctl list command.
This patch uses translate SPA interface to get bad dimm info.


Signed-off-by: Yasunori Goto <y-goto@jp.fujitsu.com>
---
 util/json.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/util/json.c b/util/json.c
index 98165b7..dbeb7ef 100644
--- a/util/json.c
+++ b/util/json.c
@@ -27,6 +27,8 @@
 #include <ndctl.h>
 #endif
 
+#include <ndctl/lib/libndctl-nfit.h>
+
 /* adapted from mdadm::human_size_brief() */
 static int display_size(struct json_object *jobj, struct printbuf *pbuf,
 		int level, int flags)
@@ -366,6 +368,15 @@ struct json_object *util_daxctl_region_to_json(struct daxctl_region *region,
 	return NULL;
 }
 
+static struct ndctl_dimm *badblock_to_dimm(struct ndctl_region *region,
+		unsigned long long spa)
+{
+	struct ndctl_bus *bus;
+
+	bus = ndctl_region_get_bus(region);
+	return ndctl_dimm_get_by_spa(bus, spa);
+}
+
 struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
 		unsigned int *bb_count, unsigned long flags)
 {
@@ -381,6 +392,18 @@ struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
 
 	ndctl_region_badblock_foreach(region, bb) {
 		if (flags & UTIL_JSON_MEDIA_ERRORS) {
+			struct ndctl_dimm *dimm = NULL;
+			unsigned long long spa;
+
+			/* get start address of region */
+			spa = ndctl_region_get_resource(region);
+			if (spa == ULONG_MAX)
+				goto err_array;
+
+			/* get address of bad block */
+			spa += bb->offset << 9;
+			dimm = badblock_to_dimm(region, spa);
+
 			jbb = json_object_new_object();
 			if (!jbb)
 				goto err_array;
@@ -395,6 +418,12 @@ struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
 				goto err;
 			json_object_object_add(jbb, "length", jobj);
 
+			if (dimm) {
+				jobj = json_object_new_string(ndctl_dimm_get_devname(dimm));
+				if (!jobj)
+					goto err;
+				json_object_object_add(jbb, "dimm", jobj);
+			}
 			json_object_array_add(jbbs, jbb);
 		}
 
@@ -436,6 +465,7 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
 
 	ndctl_region_badblock_foreach(region, bb) {
 		unsigned long long bb_begin, bb_end, begin, end;
+		struct ndctl_dimm *dimm = NULL;
 
 		bb_begin = region_begin + (bb->offset << 9);
 		bb_end = bb_begin + (bb->len << 9) - 1;
@@ -456,6 +486,8 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
 		offset = (begin - dev_begin) >> 9;
 		len = (end - begin + 1) >> 9;
 
+		dimm = badblock_to_dimm(region, begin);
+
 		if (flags & UTIL_JSON_MEDIA_ERRORS) {
 			/* add to json */
 			jbb = json_object_new_object();
@@ -472,6 +504,13 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
 				goto err;
 			json_object_object_add(jbb, "length", jobj);
 
+			if (dimm) {
+				jobj = json_object_new_string(ndctl_dimm_get_devname(dimm));
+				if (!jobj)
+					goto err;
+				json_object_object_add(jbb, "dimm", jobj);
+			}
+
 			json_object_array_add(jbbs, jbb);
 		}
 		bbs += len;



_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [ndctl PATCH 1/5] Introduce libndctl-nfit.h

[Dan Williams]

Make sure you also put "ndctl:" in your subject lines so that it
appears in the git commit message.

Also include Jerry on any future version of these patches.

On Wed, Aug 30, 2017 at 6:23 PM, Yasunori Goto <y-goto@jp.fujitsu.com> wrote:
>
> This patch introduces libndctl-nfit.h.
>
> Since these command can be executed via ND_CMD_CALL,
> libndctl.h which is shared between ndctl command and kernel does not
> have to include these defintions.
>
> So, libndctl-nfit.h, which is defined for only ndctl, is created instead,
> and move definitions from ndctl.h to it.
>
>
> Signed-off-by: Yasunori Goto <y-goto@jp.fujitsu.com>
>
> ---
>  ndctl/lib/libndctl-nfit.h | 61 +++++++++++++++++++++++++++++++++++++++++++++++

Lets put this one level up in the directory hierarchy next to
libndctl.h.in, that also governs where it gets installed when
generating the devel package.

>  ndctl/ndctl.h             | 37 ----------------------------
>  2 files changed, 61 insertions(+), 37 deletions(-)
>
> diff --git a/ndctl/lib/libndctl-nfit.h b/ndctl/lib/libndctl-nfit.h
> new file mode 100644
> index 0000000..1398662
> --- /dev/null
> +++ b/ndctl/lib/libndctl-nfit.h
> @@ -0,0 +1,61 @@
> +/*
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU Lesser General Public License,
> + * version 2.1, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT ANY
> + * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
> + * FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public License for
> + * more details.
> + */
> +#ifndef __LIBNDCTL_NFIT_H__
> +#define __LIBNDCTL_NFIT_H__
> +
> +#define ND_TRANSLATE_SPA_STATUS_INVALID_SPA  2
> +
> +/* bus passthru commands */
> +enum {
> +       NFIT_CMD_TRANSLATE_SPA = 5,
> +       NFIT_CMD_ARS_INJECT_SET = 7,
> +       NFIT_CMD_ARS_INJECT_CLEAR = 8,
> +       NFIT_CMD_ARS_INJECT_GET = 9,
> +};
> +
> +struct nd_cmd_translate_spa {
> +       __u64 spa;
> +       __u32 status;
> +       __u8  flags;
> +       __u8  _reserved[3];
> +       __u64 translate_length;
> +       __u32 num_nvdimms;
> +       struct nd_nvdimm_device {
> +               __u32 nfit_device_handle;
> +               __u32 _reserved;
> +               __u64 dpa;
> +       } __attribute__((packed)) devices[0];
> +
> +} __attribute__((packed));
> +
> +struct nd_cmd_ars_err_inj {
> +       __u64 err_inj_spa_range_base;
> +       __u64 err_inj_spa_range_length;
> +       __u8  err_inj_options;
> +       __u32 status;
> +} __attribute__((packed));
> +
> +struct nd_cmd_ars_err_inj_clr {
> +       __u64 err_inj_clr_spa_range_base;
> +       __u64 err_inj_clr_spa_range_length;
> +       __u32 status;
> +} __attribute__((packed));
> +
> +struct nd_cmd_ars_err_inj_stat {
> +       __u32 status;
> +       __u32 inj_err_rec_count;
> +       struct nd_error_stat_query_record {
> +               __u64 err_inj_stat_spa_range_base;
> +               __u64 err_inj_stat_spa_range_length;
> +       } __attribute__((packed)) record[0];
> +} __attribute__((packed));

I noticed when trying to push ndctl into the kernel tree that it warns
about structures like this one that are needlessly packed. At some
point I am going to turn on "-Wpacked" for the build, but in the
meantime remove this "__attribute__((packed));" statement and just
have a comment that says "/* naturally packed */" so no one tries to
add it back in the future. I'll work on cleaning up the other
needlessly packed structures. The other ones in this file *do* need
the annotation.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [ndctl PATCH 2/5] make interface to check device/nfit/dsm_mask flags

[Dan Williams]

On Wed, Aug 30, 2017 at 6:25 PM, Yasunori Goto <y-goto@jp.fujitsu.com> wrote:
>
> To check what feature can be called via ND_CMD_CALL, ndctl need to read
> device/nfit/dsm_mask. This patch make an interface to check it in libndctl.c
>
>
> Signed-off-by: Yasunori Goto <y-goto@jp.fujitsu.com>
>
> ---
>  ndctl/lib/libndctl.c   | 13 +++++++++++++
>  ndctl/lib/libndctl.sym |  1 +
>  ndctl/libndctl.h.in    |  1 +
>  3 files changed, 15 insertions(+)
>
> diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
> index c2e0efb..803056d 100644
> --- a/ndctl/lib/libndctl.c
> +++ b/ndctl/lib/libndctl.c
> @@ -102,6 +102,7 @@ struct ndctl_bus {
>         size_t buf_len;
>         char *wait_probe_path;
>         unsigned long dsm_mask;
> +       unsigned long passthru_dsm_mask;
>  };
>
>  /**
> @@ -846,6 +847,12 @@ static void *add_bus(void *parent, int id, const char *ctl_base)
>                 bus->revision = strtoul(buf, NULL, 0);
>         }
>
> +       sprintf(path, "%s/device/nfit/dsm_mask", ctl_base);
> +       if (sysfs_read_attr(ctx, path, buf) < 0)
> +               bus->passthru_dsm_mask = 0;
> +       else
> +               bus->passthru_dsm_mask = strtoul(buf, NULL, 16);

Let's keep the base 0 and 16 like the other calls to strtoul in this file.

> +
>         sprintf(path, "%s/device/provider", ctl_base);
>         if (sysfs_read_attr(ctx, path, buf) < 0)
>                 goto err_read;
> @@ -1101,6 +1108,12 @@ NDCTL_EXPORT int ndctl_bus_is_cmd_supported(struct ndctl_bus *bus,
>         return !!(bus->dsm_mask & (1ULL << cmd));
>  }
>
> +NDCTL_EXPORT int ndctl_bus_is_passthru_cmd_supported(struct ndctl_bus *bus,
> +               int cmd)

I realize that I said to call this "passthru" before, but thinking
about it further these commands are truly nfit specific. The command
numbers that ndctl_bus_is_cmd_supported() reports could theoretically
be emulated by the kernel for other bus types, but these are truly
nfit specific. For example, another bus type could theoretically reuse
the same command numbers for other operations and
ndctl_bus_is_passthru_cmd_supported() would not be able to tell the
difference.

Let's add a new source file called ndctl/lib/nfit.c for these nfit
specifc commands and add a:

    ndctl_bus_is_nfit_cmd_supported()

...that can operate on nfit command numbers directly. We can later
decide to wrap that in a generic front-end if some future nvdimm bus
arrives with similar functionality.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [ndctl PATCH 4/5] Make interfaces to use Translate SPA.

[Dan Williams]

Looks good in general...

Add "ndctl:" to the subject.

On Wed, Aug 30, 2017 at 6:29 PM, Yasunori Goto <y-goto@jp.fujitsu.com> wrote:
>
> This patch makes 2 new interfaces :
>   - Call translate SPA featture of ACPI 6.2.
>   - Find DIMM which SPA(System Physical Address) belongs to.
>
>
> Signed-off-by: Yasunori Goto <y-goto@jp.fujitsu.com>
> ---
>  ndctl/Makefile.am         |   1 +
>  ndctl/lib/Makefile.am     |   4 +-
>  ndctl/lib/libndctl-nfit.c | 147 ++++++++++++++++++++++++++++++++++++++++++++++

Rename this to ndctl/lib/nfit.c, I pushed out a large rename of other
files to drop the "libdctl-" prefix to the 'pending' branch on github.

>  ndctl/lib/libndctl-nfit.h |   5 ++
>  4 files changed, 156 insertions(+), 1 deletion(-)
>
> diff --git a/ndctl/Makefile.am b/ndctl/Makefile.am
> index d346c04..20d5f59 100644
> --- a/ndctl/Makefile.am
> +++ b/ndctl/Makefile.am
> @@ -25,6 +25,7 @@ endif
>
>  ndctl_LDADD =\
>         lib/libndctl.la \
> +       lib/libndctl-nfit.la \

Let's just add these symbols to the existing libndctl.so rather than
add a new one.

>         ../daxctl/lib/libdaxctl.la \
>         ../libutil.a \
>         $(UUID_LIBS) \
> diff --git a/ndctl/lib/Makefile.am b/ndctl/lib/Makefile.am
> index 7a446be..cfa54ae 100644
> --- a/ndctl/lib/Makefile.am
> +++ b/ndctl/lib/Makefile.am
> @@ -8,7 +8,7 @@ BUILT_SOURCES = ../libndctl.h
>         $(SED_PROCESS)
>
>  pkginclude_HEADERS = ../libndctl.h
> -lib_LTLIBRARIES = libndctl.la
> +lib_LTLIBRARIES = libndctl.la libndctl-nfit.la
>
>  libndctl_la_SOURCES =\
>         libndctl.h \
> @@ -35,6 +35,8 @@ libndctl_la_SOURCES += libndctl-hpe1.c
>  libndctl_la_SOURCES += libndctl-msft.c
>  endif
>
> +libndctl_nfit_la_SOURCES = libndctl-nfit.c
> +
>  EXTRA_DIST += libndctl.sym
>
>  libndctl_la_LDFLAGS = $(AM_LDFLAGS) \
> diff --git a/ndctl/lib/libndctl-nfit.c b/ndctl/lib/libndctl-nfit.c
> new file mode 100644
> index 0000000..455815d
> --- /dev/null
> +++ b/ndctl/lib/libndctl-nfit.c
> @@ -0,0 +1,147 @@
> +/*
> + * Copyright (c) 2017, FUJITSU LIMITED. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU Lesser General Public License,
> + * version 2.1, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT ANY
> + * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
> + * FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public License for
> + * more details.
> + */
> +#include <stdlib.h>
> +#include <ndctl/libndctl.h>
> +#include "libndctl-private.h"
> +#include "libndctl-nfit.h"
> +
> +static int bus_has_translate_spa(struct ndctl_bus *bus)
> +{
> +       if (!ndctl_bus_has_nfit(bus))
> +               return 0;
> +
> +       return ndctl_bus_is_passthru_cmd_supported(bus, NFIT_CMD_TRANSLATE_SPA);
> +}
> +
> +static struct ndctl_cmd *ndctl_bus_cmd_new_translate_spa(struct ndctl_bus *bus)
> +{
> +       struct ndctl_cmd *cmd;
> +       struct nd_cmd_pkg *pkg;
> +       struct nd_cmd_translate_spa *translate_spa;
> +       size_t size, spa_length;
> +
> +       spa_length = sizeof(struct nd_cmd_translate_spa)
> +               + sizeof(struct nd_nvdimm_device);
> +       size = sizeof(*cmd) + sizeof(*pkg) + spa_length;
> +       cmd = calloc(1, size);
> +       if (!cmd)
> +               return NULL;
> +
> +       cmd->bus = bus;
> +       ndctl_cmd_ref(cmd);
> +       cmd->type = ND_CMD_CALL;
> +       cmd->size = size;
> +       cmd->status = 1;
> +       pkg = (struct nd_cmd_pkg *)&cmd->cmd_buf[0];
> +       pkg->nd_command = NFIT_CMD_TRANSLATE_SPA;
> +       pkg->nd_size_in = sizeof(unsigned long long);
> +       pkg->nd_size_out = spa_length;
> +       pkg->nd_fw_size = spa_length;
> +       translate_spa = (struct nd_cmd_translate_spa *)&pkg->nd_payload[0];
> +       cmd->firmware_status = &translate_spa->status;
> +       translate_spa->translate_length = spa_length;
> +
> +       return cmd;
> +}
> +
> +static int ndctl_bus_cmd_get_translate_spa(struct ndctl_cmd *cmd,
> +                                       unsigned int *handle, unsigned long long *dpa)
> +{
> +       struct nd_cmd_pkg *pkg;
> +       struct nd_cmd_translate_spa *translate_spa;
> +
> +       pkg = (struct nd_cmd_pkg *)&cmd->cmd_buf[0];
> +       translate_spa = (struct nd_cmd_translate_spa *)&pkg->nd_payload[0];
> +
> +       if (translate_spa->status == ND_TRANSLATE_SPA_STATUS_INVALID_SPA)
> +               return -EINVAL;
> +
> +       /*
> +        * XXX: Currently NVDIMM mirroring is not supported.
> +        * Even if ACPI returned plural dimms due to mirroring,
> +        * this function returns just the first dimm.
> +        */
> +
> +       *handle = translate_spa->devices[0].nfit_device_handle;
> +       *dpa = translate_spa->devices[0].dpa;
> +
> +       return 0;
> +}
> +
> +static int is_valid_spa(struct ndctl_bus *bus, unsigned long long spa)
> +{
> +       struct ndctl_region *region;
> +       unsigned long long region_start, region_end;
> +
> +       ndctl_region_foreach(bus, region) {
> +               region_start = ndctl_region_get_resource(region);
> +               region_end = region_start + ndctl_region_get_size(region);
> +               if (region_start <= spa && spa < region_end)
> +                       return 1;
> +       }
> +
> +       return 0;
> +}
> +
> +NDCTL_EXPORT int ndctl_bus_cmd_translate_spa(struct ndctl_bus *bus,
> +       unsigned long long addr, unsigned int *handle, unsigned long long *dpa)
> +{
> +
> +       struct ndctl_cmd *cmd;
> +       struct nd_cmd_pkg *pkg;
> +       struct nd_cmd_translate_spa *translate_spa;
> +       int rc;
> +
> +       if (!bus || !handle || !dpa)
> +               return -EINVAL;
> +
> +       if (!bus_has_translate_spa(bus))
> +               return -ENOTTY;
> +
> +       if (!is_valid_spa(bus, addr))
> +               return -EINVAL;
> +
> +       cmd = ndctl_bus_cmd_new_translate_spa(bus);
> +       if (!cmd)
> +               return -ENOMEM;
> +
> +       pkg = (struct nd_cmd_pkg *)&cmd->cmd_buf[0];
> +       translate_spa = (struct nd_cmd_translate_spa *)&pkg->nd_payload[0];
> +       translate_spa->spa = addr;
> +
> +       rc = ndctl_cmd_submit(cmd);
> +       if (rc) {
> +               ndctl_cmd_unref(cmd);
> +               return rc;
> +       }
> +
> +       rc = ndctl_bus_cmd_get_translate_spa(cmd, handle, dpa);
> +       ndctl_cmd_unref(cmd);
> +
> +       return rc;
> +}
> +
> +NDCTL_EXPORT struct ndctl_dimm *ndctl_dimm_get_by_spa(struct ndctl_bus *bus,
> +               unsigned long long spa)
> +{
> +       int rc;
> +       unsigned int handle;
> +       unsigned long long dpa;
> +
> +       /* ndctl_bus_cmd_translate_spa() has sanity check */
> +       rc = ndctl_bus_cmd_translate_spa(bus, spa, &handle, &dpa);
> +       if (rc)
> +               return NULL;
> +
> +       return ndctl_dimm_get_by_handle(bus, handle);
> +}

Let's move this out of nfit.c and make it generic in libndctl.c

    ndctl_dimm_get_by_physical_address()

...where the common case of a single DIMM can be handled without
calling any DSMs. But if it is an interleave-set we can check for
ndctl_bus_has_nfit() before trying to call the nfit specific
ndctl_bus_nfit_cmd_translate_spa().
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm