* [ndctl PATCH v2 1/6] libndctl: Refactor out add_dimm() to handle NFIT specific init
2020-04-20 7:55 [ndctl PATCH v2 0/6] Add support for reporting papr-scm nvdimm health Vaibhav Jain
@ 2020-04-20 7:55 ` Vaibhav Jain
2020-04-24 3:18 ` Santosh Sivaraj
2020-04-29 7:52 ` Aneesh Kumar K.V
2020-04-20 7:55 ` [ndctl PATCH v2 2/6] libncdtl: Add initial support for NVDIMM_FAMILY_PAPR_SCM dimm family Vaibhav Jain
` (4 subsequent siblings)
5 siblings, 2 replies; 12+ messages in thread
From: Vaibhav Jain @ 2020-04-20 7:55 UTC (permalink / raw)
To: linux-nvdimm; +Cc: Vaibhav Jain, Aneesh Kumar K . V
Presently add_dimm() only probes dimms that support NFIT/ACPI. Hence
this patch refactors this functionality into two functions namely
add_dimm() and add_nfit_dimm(). Function add_dimm() performs
allocation and common 'struct ndctl_dimm' initialization and depending
on whether the dimm-bus supports NIFT, calls add_nfit_dimm(). Once
the probe is completed based on the value of 'ndctl_dimm.cmd_family'
appropriate dimm-ops are assigned to the dimm.
In case dimm-bus is of unknown type or doesn't support NFIT the
initialization still continues, with no dimm-ops assigned to the
'struct ndctl_dimm' there-by limiting the functionality available.
This patch shouldn't introduce any behavioral change.
Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
Changelog:
v1..v2:
* None
---
ndctl/lib/libndctl.c | 193 +++++++++++++++++++++++++------------------
1 file changed, 112 insertions(+), 81 deletions(-)
diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index ee737cbbfe3e..d76dbf7e17de 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -1441,82 +1441,15 @@ static int ndctl_bind(struct ndctl_ctx *ctx, struct kmod_module *module,
static int ndctl_unbind(struct ndctl_ctx *ctx, const char *devpath);
static struct kmod_module *to_module(struct ndctl_ctx *ctx, const char *alias);
-static void *add_dimm(void *parent, int id, const char *dimm_base)
+static int add_nfit_dimm(struct ndctl_dimm *dimm, const char *dimm_base)
{
- int formats, i;
- struct ndctl_dimm *dimm;
+ int i, rc = -1;
char buf[SYSFS_ATTR_SIZE];
- struct ndctl_bus *bus = parent;
- struct ndctl_ctx *ctx = bus->ctx;
+ struct ndctl_ctx *ctx = dimm->bus->ctx;
char *path = calloc(1, strlen(dimm_base) + 100);
if (!path)
- return NULL;
-
- sprintf(path, "%s/nfit/formats", dimm_base);
- if (sysfs_read_attr(ctx, path, buf) < 0)
- formats = 1;
- else
- formats = clamp(strtoul(buf, NULL, 0), 1UL, 2UL);
-
- dimm = calloc(1, sizeof(*dimm) + sizeof(int) * formats);
- if (!dimm)
- goto err_dimm;
- dimm->bus = bus;
- dimm->id = id;
-
- sprintf(path, "%s/dev", dimm_base);
- if (sysfs_read_attr(ctx, path, buf) < 0)
- goto err_read;
- if (sscanf(buf, "%d:%d", &dimm->major, &dimm->minor) != 2)
- goto err_read;
-
- sprintf(path, "%s/commands", dimm_base);
- if (sysfs_read_attr(ctx, path, buf) < 0)
- goto err_read;
- dimm->cmd_mask = parse_commands(buf, 1);
-
- dimm->dimm_buf = calloc(1, strlen(dimm_base) + 50);
- if (!dimm->dimm_buf)
- goto err_read;
- dimm->buf_len = strlen(dimm_base) + 50;
-
- dimm->dimm_path = strdup(dimm_base);
- if (!dimm->dimm_path)
- goto err_read;
-
- sprintf(path, "%s/modalias", dimm_base);
- if (sysfs_read_attr(ctx, path, buf) < 0)
- goto err_read;
- dimm->module = to_module(ctx, buf);
-
- dimm->handle = -1;
- dimm->phys_id = -1;
- dimm->serial = -1;
- dimm->vendor_id = -1;
- dimm->device_id = -1;
- dimm->revision_id = -1;
- dimm->health_eventfd = -1;
- dimm->dirty_shutdown = -ENOENT;
- dimm->subsystem_vendor_id = -1;
- dimm->subsystem_device_id = -1;
- dimm->subsystem_revision_id = -1;
- dimm->manufacturing_date = -1;
- dimm->manufacturing_location = -1;
- dimm->cmd_family = -1;
- dimm->nfit_dsm_mask = ULONG_MAX;
- for (i = 0; i < formats; i++)
- dimm->format[i] = -1;
-
- sprintf(path, "%s/flags", dimm_base);
- if (sysfs_read_attr(ctx, path, buf) < 0) {
- dimm->locked = -1;
- dimm->aliased = -1;
- } else
- parse_dimm_flags(dimm, buf);
-
- if (!ndctl_bus_has_nfit(bus))
- goto out;
+ return -1;
/*
* 'unique_id' may not be available on older kernels, so don't
@@ -1582,24 +1515,15 @@ static void *add_dimm(void *parent, int id, const char *dimm_base)
sprintf(path, "%s/nfit/family", dimm_base);
if (sysfs_read_attr(ctx, path, buf) == 0)
dimm->cmd_family = strtoul(buf, NULL, 0);
- if (dimm->cmd_family == NVDIMM_FAMILY_INTEL)
- dimm->ops = intel_dimm_ops;
- if (dimm->cmd_family == NVDIMM_FAMILY_HPE1)
- dimm->ops = hpe1_dimm_ops;
- if (dimm->cmd_family == NVDIMM_FAMILY_MSFT)
- dimm->ops = msft_dimm_ops;
- if (dimm->cmd_family == NVDIMM_FAMILY_HYPERV)
- dimm->ops = hyperv_dimm_ops;
sprintf(path, "%s/nfit/dsm_mask", dimm_base);
if (sysfs_read_attr(ctx, path, buf) == 0)
dimm->nfit_dsm_mask = strtoul(buf, NULL, 0);
- dimm->formats = formats;
sprintf(path, "%s/nfit/format", dimm_base);
if (sysfs_read_attr(ctx, path, buf) == 0)
dimm->format[0] = strtoul(buf, NULL, 0);
- for (i = 1; i < formats; i++) {
+ for (i = 1; i < dimm->formats; i++) {
sprintf(path, "%s/nfit/format%d", dimm_base, i);
if (sysfs_read_attr(ctx, path, buf) == 0)
dimm->format[i] = strtoul(buf, NULL, 0);
@@ -1610,7 +1534,114 @@ static void *add_dimm(void *parent, int id, const char *dimm_base)
parse_nfit_mem_flags(dimm, buf);
dimm->health_eventfd = open(path, O_RDONLY|O_CLOEXEC);
+ rc = 0;
+ err_read:
+
+ free(path);
+ return rc;
+}
+
+static void *add_dimm(void *parent, int id, const char *dimm_base)
+{
+ int formats, i, rc = -ENODEV;
+ struct ndctl_dimm *dimm = NULL;
+ char buf[SYSFS_ATTR_SIZE];
+ struct ndctl_bus *bus = parent;
+ struct ndctl_ctx *ctx = bus->ctx;
+ char *path = calloc(1, strlen(dimm_base) + 100);
+
+ if (!path)
+ return NULL;
+
+ sprintf(path, "%s/nfit/formats", dimm_base);
+ if (sysfs_read_attr(ctx, path, buf) < 0)
+ formats = 1;
+ else
+ formats = clamp(strtoul(buf, NULL, 0), 1UL, 2UL);
+
+ dimm = calloc(1, sizeof(*dimm) + sizeof(int) * formats);
+ if (!dimm)
+ goto err_dimm;
+ dimm->bus = bus;
+ dimm->id = id;
+
+ sprintf(path, "%s/dev", dimm_base);
+ if (sysfs_read_attr(ctx, path, buf) < 0)
+ goto err_read;
+ if (sscanf(buf, "%d:%d", &dimm->major, &dimm->minor) != 2)
+ goto err_read;
+
+ sprintf(path, "%s/commands", dimm_base);
+ if (sysfs_read_attr(ctx, path, buf) < 0)
+ goto err_read;
+ dimm->cmd_mask = parse_commands(buf, 1);
+
+ dimm->dimm_buf = calloc(1, strlen(dimm_base) + 50);
+ if (!dimm->dimm_buf)
+ goto err_read;
+ dimm->buf_len = strlen(dimm_base) + 50;
+
+ dimm->dimm_path = strdup(dimm_base);
+ if (!dimm->dimm_path)
+ goto err_read;
+
+ sprintf(path, "%s/modalias", dimm_base);
+ if (sysfs_read_attr(ctx, path, buf) < 0)
+ goto err_read;
+ dimm->module = to_module(ctx, buf);
+
+ dimm->handle = -1;
+ dimm->phys_id = -1;
+ dimm->serial = -1;
+ dimm->vendor_id = -1;
+ dimm->device_id = -1;
+ dimm->revision_id = -1;
+ dimm->health_eventfd = -1;
+ dimm->dirty_shutdown = -ENOENT;
+ dimm->subsystem_vendor_id = -1;
+ dimm->subsystem_device_id = -1;
+ dimm->subsystem_revision_id = -1;
+ dimm->manufacturing_date = -1;
+ dimm->manufacturing_location = -1;
+ dimm->cmd_family = -1;
+ dimm->nfit_dsm_mask = ULONG_MAX;
+ for (i = 0; i < formats; i++)
+ dimm->format[i] = -1;
+
+ sprintf(path, "%s/flags", dimm_base);
+ if (sysfs_read_attr(ctx, path, buf) < 0) {
+ dimm->locked = -1;
+ dimm->aliased = -1;
+ } else
+ parse_dimm_flags(dimm, buf);
+
+ /* Check if the given dimm supports nfit */
+ if (ndctl_bus_has_nfit(bus)) {
+ dimm->formats = formats;
+ rc = add_nfit_dimm(dimm, dimm_base);
+ }
+
+ if (rc == -ENODEV) {
+ /* Unprobed dimm with no family */
+ rc = 0;
+ goto out;
+ }
+
+ /* Assign dimm-ops based on command family */
+ if (dimm->cmd_family == NVDIMM_FAMILY_INTEL)
+ dimm->ops = intel_dimm_ops;
+ if (dimm->cmd_family == NVDIMM_FAMILY_HPE1)
+ dimm->ops = hpe1_dimm_ops;
+ if (dimm->cmd_family == NVDIMM_FAMILY_MSFT)
+ dimm->ops = msft_dimm_ops;
+ if (dimm->cmd_family == NVDIMM_FAMILY_HYPERV)
+ dimm->ops = hyperv_dimm_ops;
out:
+ if (rc) {
+ err(ctx, "Unable to probe dimm:%d. Err:%d\n", id, rc);
+ goto err_read;
+ }
+
list_add(&bus->dimms, &dimm->list);
free(path);
--
2.25.3
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [ndctl PATCH v2 1/6] libndctl: Refactor out add_dimm() to handle NFIT specific init
2020-04-20 7:55 ` [ndctl PATCH v2 1/6] libndctl: Refactor out add_dimm() to handle NFIT specific init Vaibhav Jain
@ 2020-04-24 3:18 ` Santosh Sivaraj
2020-04-29 7:52 ` Aneesh Kumar K.V
1 sibling, 0 replies; 12+ messages in thread
From: Santosh Sivaraj @ 2020-04-24 3:18 UTC (permalink / raw)
To: Vaibhav Jain, linux-nvdimm; +Cc: Vaibhav Jain, Aneesh Kumar K . V
Vaibhav Jain <vaibhav@linux.ibm.com> writes:
> Presently add_dimm() only probes dimms that support NFIT/ACPI. Hence
> this patch refactors this functionality into two functions namely
> add_dimm() and add_nfit_dimm(). Function add_dimm() performs
> allocation and common 'struct ndctl_dimm' initialization and depending
> on whether the dimm-bus supports NIFT, calls add_nfit_dimm(). Once
> the probe is completed based on the value of 'ndctl_dimm.cmd_family'
> appropriate dimm-ops are assigned to the dimm.
>
> In case dimm-bus is of unknown type or doesn't support NFIT the
> initialization still continues, with no dimm-ops assigned to the
> 'struct ndctl_dimm' there-by limiting the functionality available.
>
> This patch shouldn't introduce any behavioral change.
>
> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
> ---
> Changelog:
>
> v1..v2:
> * None
Looks good to me. For the series,
Reviewed-by: Santosh S <santosh@fossix.org>
Thanks,
Santosh
> ---
> ndctl/lib/libndctl.c | 193 +++++++++++++++++++++++++------------------
> 1 file changed, 112 insertions(+), 81 deletions(-)
>
> diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
> index ee737cbbfe3e..d76dbf7e17de 100644
> --- a/ndctl/lib/libndctl.c
> +++ b/ndctl/lib/libndctl.c
> @@ -1441,82 +1441,15 @@ static int ndctl_bind(struct ndctl_ctx *ctx, struct kmod_module *module,
> static int ndctl_unbind(struct ndctl_ctx *ctx, const char *devpath);
> static struct kmod_module *to_module(struct ndctl_ctx *ctx, const char *alias);
>
> -static void *add_dimm(void *parent, int id, const char *dimm_base)
> +static int add_nfit_dimm(struct ndctl_dimm *dimm, const char *dimm_base)
> {
> - int formats, i;
> - struct ndctl_dimm *dimm;
> + int i, rc = -1;
> char buf[SYSFS_ATTR_SIZE];
> - struct ndctl_bus *bus = parent;
> - struct ndctl_ctx *ctx = bus->ctx;
> + struct ndctl_ctx *ctx = dimm->bus->ctx;
> char *path = calloc(1, strlen(dimm_base) + 100);
>
> if (!path)
> - return NULL;
> -
> - sprintf(path, "%s/nfit/formats", dimm_base);
> - if (sysfs_read_attr(ctx, path, buf) < 0)
> - formats = 1;
> - else
> - formats = clamp(strtoul(buf, NULL, 0), 1UL, 2UL);
> -
> - dimm = calloc(1, sizeof(*dimm) + sizeof(int) * formats);
> - if (!dimm)
> - goto err_dimm;
> - dimm->bus = bus;
> - dimm->id = id;
> -
> - sprintf(path, "%s/dev", dimm_base);
> - if (sysfs_read_attr(ctx, path, buf) < 0)
> - goto err_read;
> - if (sscanf(buf, "%d:%d", &dimm->major, &dimm->minor) != 2)
> - goto err_read;
> -
> - sprintf(path, "%s/commands", dimm_base);
> - if (sysfs_read_attr(ctx, path, buf) < 0)
> - goto err_read;
> - dimm->cmd_mask = parse_commands(buf, 1);
> -
> - dimm->dimm_buf = calloc(1, strlen(dimm_base) + 50);
> - if (!dimm->dimm_buf)
> - goto err_read;
> - dimm->buf_len = strlen(dimm_base) + 50;
> -
> - dimm->dimm_path = strdup(dimm_base);
> - if (!dimm->dimm_path)
> - goto err_read;
> -
> - sprintf(path, "%s/modalias", dimm_base);
> - if (sysfs_read_attr(ctx, path, buf) < 0)
> - goto err_read;
> - dimm->module = to_module(ctx, buf);
> -
> - dimm->handle = -1;
> - dimm->phys_id = -1;
> - dimm->serial = -1;
> - dimm->vendor_id = -1;
> - dimm->device_id = -1;
> - dimm->revision_id = -1;
> - dimm->health_eventfd = -1;
> - dimm->dirty_shutdown = -ENOENT;
> - dimm->subsystem_vendor_id = -1;
> - dimm->subsystem_device_id = -1;
> - dimm->subsystem_revision_id = -1;
> - dimm->manufacturing_date = -1;
> - dimm->manufacturing_location = -1;
> - dimm->cmd_family = -1;
> - dimm->nfit_dsm_mask = ULONG_MAX;
> - for (i = 0; i < formats; i++)
> - dimm->format[i] = -1;
> -
> - sprintf(path, "%s/flags", dimm_base);
> - if (sysfs_read_attr(ctx, path, buf) < 0) {
> - dimm->locked = -1;
> - dimm->aliased = -1;
> - } else
> - parse_dimm_flags(dimm, buf);
> -
> - if (!ndctl_bus_has_nfit(bus))
> - goto out;
> + return -1;
>
> /*
> * 'unique_id' may not be available on older kernels, so don't
> @@ -1582,24 +1515,15 @@ static void *add_dimm(void *parent, int id, const char *dimm_base)
> sprintf(path, "%s/nfit/family", dimm_base);
> if (sysfs_read_attr(ctx, path, buf) == 0)
> dimm->cmd_family = strtoul(buf, NULL, 0);
> - if (dimm->cmd_family == NVDIMM_FAMILY_INTEL)
> - dimm->ops = intel_dimm_ops;
> - if (dimm->cmd_family == NVDIMM_FAMILY_HPE1)
> - dimm->ops = hpe1_dimm_ops;
> - if (dimm->cmd_family == NVDIMM_FAMILY_MSFT)
> - dimm->ops = msft_dimm_ops;
> - if (dimm->cmd_family == NVDIMM_FAMILY_HYPERV)
> - dimm->ops = hyperv_dimm_ops;
>
> sprintf(path, "%s/nfit/dsm_mask", dimm_base);
> if (sysfs_read_attr(ctx, path, buf) == 0)
> dimm->nfit_dsm_mask = strtoul(buf, NULL, 0);
>
> - dimm->formats = formats;
> sprintf(path, "%s/nfit/format", dimm_base);
> if (sysfs_read_attr(ctx, path, buf) == 0)
> dimm->format[0] = strtoul(buf, NULL, 0);
> - for (i = 1; i < formats; i++) {
> + for (i = 1; i < dimm->formats; i++) {
> sprintf(path, "%s/nfit/format%d", dimm_base, i);
> if (sysfs_read_attr(ctx, path, buf) == 0)
> dimm->format[i] = strtoul(buf, NULL, 0);
> @@ -1610,7 +1534,114 @@ static void *add_dimm(void *parent, int id, const char *dimm_base)
> parse_nfit_mem_flags(dimm, buf);
>
> dimm->health_eventfd = open(path, O_RDONLY|O_CLOEXEC);
> + rc = 0;
> + err_read:
> +
> + free(path);
> + return rc;
> +}
> +
> +static void *add_dimm(void *parent, int id, const char *dimm_base)
> +{
> + int formats, i, rc = -ENODEV;
> + struct ndctl_dimm *dimm = NULL;
> + char buf[SYSFS_ATTR_SIZE];
> + struct ndctl_bus *bus = parent;
> + struct ndctl_ctx *ctx = bus->ctx;
> + char *path = calloc(1, strlen(dimm_base) + 100);
> +
> + if (!path)
> + return NULL;
> +
> + sprintf(path, "%s/nfit/formats", dimm_base);
> + if (sysfs_read_attr(ctx, path, buf) < 0)
> + formats = 1;
> + else
> + formats = clamp(strtoul(buf, NULL, 0), 1UL, 2UL);
> +
> + dimm = calloc(1, sizeof(*dimm) + sizeof(int) * formats);
> + if (!dimm)
> + goto err_dimm;
> + dimm->bus = bus;
> + dimm->id = id;
> +
> + sprintf(path, "%s/dev", dimm_base);
> + if (sysfs_read_attr(ctx, path, buf) < 0)
> + goto err_read;
> + if (sscanf(buf, "%d:%d", &dimm->major, &dimm->minor) != 2)
> + goto err_read;
> +
> + sprintf(path, "%s/commands", dimm_base);
> + if (sysfs_read_attr(ctx, path, buf) < 0)
> + goto err_read;
> + dimm->cmd_mask = parse_commands(buf, 1);
> +
> + dimm->dimm_buf = calloc(1, strlen(dimm_base) + 50);
> + if (!dimm->dimm_buf)
> + goto err_read;
> + dimm->buf_len = strlen(dimm_base) + 50;
> +
> + dimm->dimm_path = strdup(dimm_base);
> + if (!dimm->dimm_path)
> + goto err_read;
> +
> + sprintf(path, "%s/modalias", dimm_base);
> + if (sysfs_read_attr(ctx, path, buf) < 0)
> + goto err_read;
> + dimm->module = to_module(ctx, buf);
> +
> + dimm->handle = -1;
> + dimm->phys_id = -1;
> + dimm->serial = -1;
> + dimm->vendor_id = -1;
> + dimm->device_id = -1;
> + dimm->revision_id = -1;
> + dimm->health_eventfd = -1;
> + dimm->dirty_shutdown = -ENOENT;
> + dimm->subsystem_vendor_id = -1;
> + dimm->subsystem_device_id = -1;
> + dimm->subsystem_revision_id = -1;
> + dimm->manufacturing_date = -1;
> + dimm->manufacturing_location = -1;
> + dimm->cmd_family = -1;
> + dimm->nfit_dsm_mask = ULONG_MAX;
> + for (i = 0; i < formats; i++)
> + dimm->format[i] = -1;
> +
> + sprintf(path, "%s/flags", dimm_base);
> + if (sysfs_read_attr(ctx, path, buf) < 0) {
> + dimm->locked = -1;
> + dimm->aliased = -1;
> + } else
> + parse_dimm_flags(dimm, buf);
> +
> + /* Check if the given dimm supports nfit */
> + if (ndctl_bus_has_nfit(bus)) {
> + dimm->formats = formats;
> + rc = add_nfit_dimm(dimm, dimm_base);
> + }
> +
> + if (rc == -ENODEV) {
> + /* Unprobed dimm with no family */
> + rc = 0;
> + goto out;
> + }
> +
> + /* Assign dimm-ops based on command family */
> + if (dimm->cmd_family == NVDIMM_FAMILY_INTEL)
> + dimm->ops = intel_dimm_ops;
> + if (dimm->cmd_family == NVDIMM_FAMILY_HPE1)
> + dimm->ops = hpe1_dimm_ops;
> + if (dimm->cmd_family == NVDIMM_FAMILY_MSFT)
> + dimm->ops = msft_dimm_ops;
> + if (dimm->cmd_family == NVDIMM_FAMILY_HYPERV)
> + dimm->ops = hyperv_dimm_ops;
> out:
> + if (rc) {
> + err(ctx, "Unable to probe dimm:%d. Err:%d\n", id, rc);
> + goto err_read;
> + }
> +
> list_add(&bus->dimms, &dimm->list);
> free(path);
>
> --
> 2.25.3
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [ndctl PATCH v2 1/6] libndctl: Refactor out add_dimm() to handle NFIT specific init
2020-04-20 7:55 ` [ndctl PATCH v2 1/6] libndctl: Refactor out add_dimm() to handle NFIT specific init Vaibhav Jain
2020-04-24 3:18 ` Santosh Sivaraj
@ 2020-04-29 7:52 ` Aneesh Kumar K.V
2020-05-04 8:05 ` Vaibhav Jain
1 sibling, 1 reply; 12+ messages in thread
From: Aneesh Kumar K.V @ 2020-04-29 7:52 UTC (permalink / raw)
To: Vaibhav Jain, linux-nvdimm; +Cc: Vaibhav Jain
Vaibhav Jain <vaibhav@linux.ibm.com> writes:
> Presently add_dimm() only probes dimms that support NFIT/ACPI. Hence
> this patch refactors this functionality into two functions namely
> add_dimm() and add_nfit_dimm(). Function add_dimm() performs
> allocation and common 'struct ndctl_dimm' initialization and depending
> on whether the dimm-bus supports NIFT, calls add_nfit_dimm(). Once
> the probe is completed based on the value of 'ndctl_dimm.cmd_family'
> appropriate dimm-ops are assigned to the dimm.
>
> In case dimm-bus is of unknown type or doesn't support NFIT the
> initialization still continues, with no dimm-ops assigned to the
> 'struct ndctl_dimm' there-by limiting the functionality available.
>
> This patch shouldn't introduce any behavioral change.
>
> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
> ---
> Changelog:
>
> v1..v2:
> * None
> ---
> ndctl/lib/libndctl.c | 193 +++++++++++++++++++++++++------------------
> 1 file changed, 112 insertions(+), 81 deletions(-)
>
> diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
> index ee737cbbfe3e..d76dbf7e17de 100644
> --- a/ndctl/lib/libndctl.c
> +++ b/ndctl/lib/libndctl.c
> @@ -1441,82 +1441,15 @@ static int ndctl_bind(struct ndctl_ctx *ctx, struct kmod_module *module,
> static int ndctl_unbind(struct ndctl_ctx *ctx, const char *devpath);
> static struct kmod_module *to_module(struct ndctl_ctx *ctx, const char *alias);
>
> -static void *add_dimm(void *parent, int id, const char *dimm_base)
> +static int add_nfit_dimm(struct ndctl_dimm *dimm, const char *dimm_base)
> {
> - int formats, i;
> - struct ndctl_dimm *dimm;
> + int i, rc = -1;
> char buf[SYSFS_ATTR_SIZE];
> - struct ndctl_bus *bus = parent;
> - struct ndctl_ctx *ctx = bus->ctx;
> + struct ndctl_ctx *ctx = dimm->bus->ctx;
> char *path = calloc(1, strlen(dimm_base) + 100);
>
> if (!path)
> - return NULL;
> -
> - sprintf(path, "%s/nfit/formats", dimm_base);
> - if (sysfs_read_attr(ctx, path, buf) < 0)
> - formats = 1;
> - else
....
> + rc = 0;
> + err_read:
> +
> + free(path);
> + return rc;
> +}
> +
> +static void *add_dimm(void *parent, int id, const char *dimm_base)
> +{
> + int formats, i, rc = -ENODEV;
> + struct ndctl_dimm *dimm = NULL;
> + char buf[SYSFS_ATTR_SIZE];
> + struct ndctl_bus *bus = parent;
> + struct ndctl_ctx *ctx = bus->ctx;
> + char *path = calloc(1, strlen(dimm_base) + 100);
> +
> + if (!path)
> + return NULL;
> +
> + sprintf(path, "%s/nfit/formats", dimm_base);
Witht that abstraction this should be part of add_nfit_dimm?
> + if (sysfs_read_attr(ctx, path, buf) < 0)
> + formats = 1;
> + else
> + formats = clamp(strtoul(buf, NULL, 0), 1UL, 2UL);
> +
> + dimm = calloc(1, sizeof(*dimm) + sizeof(int) * formats);
> + if (!dimm)
> + goto err_dimm;
> + dimm->bus = bus;
> + dimm->id = id;
> +
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [ndctl PATCH v2 1/6] libndctl: Refactor out add_dimm() to handle NFIT specific init
2020-04-29 7:52 ` Aneesh Kumar K.V
@ 2020-05-04 8:05 ` Vaibhav Jain
0 siblings, 0 replies; 12+ messages in thread
From: Vaibhav Jain @ 2020-05-04 8:05 UTC (permalink / raw)
To: Aneesh Kumar K.V, linux-nvdimm
Hi Aneesh,
Thanks for looking into this patch.
"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
> Vaibhav Jain <vaibhav@linux.ibm.com> writes:
>
>> Presently add_dimm() only probes dimms that support NFIT/ACPI. Hence
>> this patch refactors this functionality into two functions namely
>> add_dimm() and add_nfit_dimm(). Function add_dimm() performs
>> allocation and common 'struct ndctl_dimm' initialization and depending
>> on whether the dimm-bus supports NIFT, calls add_nfit_dimm(). Once
>> the probe is completed based on the value of 'ndctl_dimm.cmd_family'
>> appropriate dimm-ops are assigned to the dimm.
>>
>> In case dimm-bus is of unknown type or doesn't support NFIT the
>> initialization still continues, with no dimm-ops assigned to the
>> 'struct ndctl_dimm' there-by limiting the functionality available.
>>
>> This patch shouldn't introduce any behavioral change.
>>
>> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
>> ---
>> Changelog:
>>
>> v1..v2:
>> * None
>> ---
>> ndctl/lib/libndctl.c | 193 +++++++++++++++++++++++++------------------
>> 1 file changed, 112 insertions(+), 81 deletions(-)
>>
>> diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
>> index ee737cbbfe3e..d76dbf7e17de 100644
>> --- a/ndctl/lib/libndctl.c
>> +++ b/ndctl/lib/libndctl.c
>> @@ -1441,82 +1441,15 @@ static int ndctl_bind(struct ndctl_ctx *ctx, struct kmod_module *module,
>> static int ndctl_unbind(struct ndctl_ctx *ctx, const char *devpath);
>> static struct kmod_module *to_module(struct ndctl_ctx *ctx, const char *alias);
>>
>> -static void *add_dimm(void *parent, int id, const char *dimm_base)
>> +static int add_nfit_dimm(struct ndctl_dimm *dimm, const char *dimm_base)
>> {
>> - int formats, i;
>> - struct ndctl_dimm *dimm;
>> + int i, rc = -1;
>> char buf[SYSFS_ATTR_SIZE];
>> - struct ndctl_bus *bus = parent;
>> - struct ndctl_ctx *ctx = bus->ctx;
>> + struct ndctl_ctx *ctx = dimm->bus->ctx;
>> char *path = calloc(1, strlen(dimm_base) + 100);
>>
>> if (!path)
>> - return NULL;
>> -
>> - sprintf(path, "%s/nfit/formats", dimm_base);
>> - if (sysfs_read_attr(ctx, path, buf) < 0)
>> - formats = 1;
>> - else
>
> ....
>> + rc = 0;
>> + err_read:
>> +
>> + free(path);
>> + return rc;
>> +}
>> +
>> +static void *add_dimm(void *parent, int id, const char *dimm_base)
>> +{
>> + int formats, i, rc = -ENODEV;
>> + struct ndctl_dimm *dimm = NULL;
>> + char buf[SYSFS_ATTR_SIZE];
>> + struct ndctl_bus *bus = parent;
>> + struct ndctl_ctx *ctx = bus->ctx;
>> + char *path = calloc(1, strlen(dimm_base) + 100);
>> +
>> + if (!path)
>> + return NULL;
>> +
>> + sprintf(path, "%s/nfit/formats", dimm_base);
>
> Witht that abstraction this should be part of add_nfit_dimm?
This part is needed to calculate the size of 'struct ndctl_dimm' to be
allocated which is based on number of nfit formats reported in
sysfs.
>
>> + if (sysfs_read_attr(ctx, path, buf) < 0)
>> + formats = 1;
>> + else
>> + formats = clamp(strtoul(buf, NULL, 0), 1UL, 2UL);
>> +
>> + dimm = calloc(1, sizeof(*dimm) + sizeof(int) * formats);
>> + if (!dimm)
>> + goto err_dimm;
>> + dimm->bus = bus;
>> + dimm->id = id;
>> +
> _______________________________________________
> Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
> To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
~ Vaibhav
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
^ permalink raw reply [flat|nested] 12+ messages in thread
* [ndctl PATCH v2 2/6] libncdtl: Add initial support for NVDIMM_FAMILY_PAPR_SCM dimm family
2020-04-20 7:55 [ndctl PATCH v2 0/6] Add support for reporting papr-scm nvdimm health Vaibhav Jain
2020-04-20 7:55 ` [ndctl PATCH v2 1/6] libndctl: Refactor out add_dimm() to handle NFIT specific init Vaibhav Jain
@ 2020-04-20 7:55 ` Vaibhav Jain
2020-04-29 7:55 ` Aneesh Kumar K.V
2020-04-20 7:55 ` [ndctl PATCH v2 3/6] libndctl: Introduce new dimm-ops dimm_init() & dimm_uninit() Vaibhav Jain
` (3 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Vaibhav Jain @ 2020-04-20 7:55 UTC (permalink / raw)
To: linux-nvdimm; +Cc: Vaibhav Jain, Aneesh Kumar K . V
Add necessary scaffolding in libndctl for dimms that support papr_scm
specification[1]. Since there can be platforms that support
Open-Firmware[2] but not the papr_scm specification, hence the changes
proposed first add support for probing if the dimm bus supports
Open-Firmware. This is done via querying for sysfs attribute 'of_node'
in dimm device sysfs directory. If available newly introduced member
'struct ndctl_bus.has_of_node' is set. During the probe of the dimm
and execution of add_dimm(), the newly introduced add_of_pmem_dimm()
is called if dimm bus reports supports Open-Firmware.
Function add_of_pmem_dimm() queries the 'compatible' device tree
attribute and based on its value assign NVDIMM_FAMILY_PAPR_SCM to the
dimm command family. In future, based on the contents of 'compatible'
attribute more of_pmem dimm families can be queried.
We also add support for parsing the dimm flags for
NVDIMM_FAMILY_PAPR_SCM supporting nvdimms as described at [3]. A newly
introduced function parse_papr_scm_flags() reads the contents of this
flag file and sets appropriate flag bits in 'struct
ndctl_dimm.flags'.
Also we advertise support for monitor mode by allocating a file
descriptor to the dimm 'flags' file and assigning it to 'struct
ndctl_dimm.health_event_fd'.
The dimm-ops implementation for NVDIMM_FAMILY_PAPR_SCM is
available in global variable 'papr_scm_dimm_ops' which points to
skeleton implementation in newly introduced file 'lib/papr_scm.c'.
References:
[1] Documentation/powerpc/papr_hcalls.rst
https://lore.kernel.org/linux-nvdimm/20200420070711.223545-2-vaibhav@linux.ibm.com/
[2] https://en.wikipedia.org/wiki/Open_Firmware
[3] Documentation/ABI/testing/sysfs-bus-papr-scm
https://lore.kernel.org/linux-nvdimm/20200420070711.223545-3-vaibhav@linux.ibm.com/
Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
Changelog:
v1..v2:
* Squashed the patch to parse dimm flags
* Updated the dimm flags parsing to add case for 'restore_fail' and
'flush_fail'.
* Renamed parse_of_pmem_flags() to parse_papr_scm_flags().
* Updated the path to dimm flags file to 'papr/flags'.
* Introduced 'papr_scm.c' file in this patch rather than later in the
patch series.
* Update add_of_pmem_dimm() to parse dimm flags and enable monitoring
only if 'ibm,pmemory' compatible nvdimm is found.
---
ndctl/lib/Makefile.am | 1 +
ndctl/lib/libndctl.c | 78 +++++++++++++++++++++++++++++++++++++++++++
ndctl/lib/papr_scm.c | 32 ++++++++++++++++++
ndctl/lib/private.h | 2 ++
ndctl/libndctl.h | 1 +
ndctl/ndctl.h | 1 +
6 files changed, 115 insertions(+)
create mode 100644 ndctl/lib/papr_scm.c
diff --git a/ndctl/lib/Makefile.am b/ndctl/lib/Makefile.am
index d6be5c3acd26..90ba680bee56 100644
--- a/ndctl/lib/Makefile.am
+++ b/ndctl/lib/Makefile.am
@@ -23,6 +23,7 @@ libndctl_la_SOURCES =\
hpe1.c \
msft.c \
hyperv.c \
+ papr_scm.c \
ars.c \
firmware.c \
libndctl.c \
diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index d76dbf7e17de..60dad8e56f5a 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -799,6 +799,28 @@ static void parse_nfit_mem_flags(struct ndctl_dimm *dimm, char *flags)
ndctl_dimm_get_devname(dimm), flags);
}
+static void parse_papr_scm_flags(struct ndctl_dimm *dimm, char *flags)
+{
+ char *start, *end;
+
+ start = flags;
+ while ((end = strchr(start, ' '))) {
+ *end = '\0';
+ if (strcmp(start, "not_armed") == 0)
+ dimm->flags.f_arm = 1;
+ else if (strcmp(start, "flush_fail") == 0)
+ dimm->flags.f_flush = 1;
+ else if (strcmp(start, "restore_fail") == 0)
+ dimm->flags.f_restore = 1;
+ else if (strcmp(start, "smart_notify") == 0)
+ dimm->flags.f_smart = 1;
+ start = end + 1;
+ }
+ if (end != start)
+ dbg(ndctl_dimm_get_ctx(dimm), "%s: %s\n",
+ ndctl_dimm_get_devname(dimm), flags);
+}
+
static void parse_dimm_flags(struct ndctl_dimm *dimm, char *flags)
{
char *start, *end;
@@ -856,6 +878,12 @@ static void *add_bus(void *parent, int id, const char *ctl_base)
bus->revision = strtoul(buf, NULL, 0);
}
+ sprintf(path, "%s/device/of_node/compatible", ctl_base);
+ if (sysfs_read_attr(ctx, path, buf) < 0)
+ bus->has_of_node = 0;
+ else
+ bus->has_of_node = 1;
+
sprintf(path, "%s/device/nfit/dsm_mask", ctl_base);
if (sysfs_read_attr(ctx, path, buf) < 0)
bus->nfit_dsm_mask = 0;
@@ -964,6 +992,10 @@ NDCTL_EXPORT int ndctl_bus_has_nfit(struct ndctl_bus *bus)
return bus->has_nfit;
}
+NDCTL_EXPORT int ndctl_bus_has_of_node(struct ndctl_bus *bus)
+{
+ return bus->has_of_node;
+}
/**
* ndctl_bus_get_major - nd bus character device major number
* @bus: ndctl_bus instance returned from ndctl_bus_get_{first|next}
@@ -1441,6 +1473,47 @@ static int ndctl_bind(struct ndctl_ctx *ctx, struct kmod_module *module,
static int ndctl_unbind(struct ndctl_ctx *ctx, const char *devpath);
static struct kmod_module *to_module(struct ndctl_ctx *ctx, const char *alias);
+static int add_of_pmem_dimm(struct ndctl_dimm *dimm, const char *dimm_base)
+{
+ int rc = -ENODEV;
+ char buf[SYSFS_ATTR_SIZE];
+ struct ndctl_ctx *ctx = dimm->bus->ctx;
+ char *path = calloc(1, strlen(dimm_base) + 100);
+
+ dbg(ctx, "Probing of_pmem dimm %d at %s\n", dimm->id, dimm_base);
+
+ if (!path)
+ return -ENOMEM;
+
+ sprintf(path, "%s/../of_node/compatible", dimm_base);
+ if (sysfs_read_attr(ctx, path, buf) < 0)
+ goto out;
+
+
+ dbg(ctx, "Compatible of_pmem dimm %d at %s\n", dimm->id, buf);
+ /* construct path to the papr compatible dimm flags file */
+ sprintf(path, "%s/papr/flags", dimm_base);
+
+ if (strcmp(buf, "ibm,pmemory") == 0 &&
+ sysfs_read_attr(ctx, path, buf) == 0) {
+
+ dbg(ctx, "Found papr_scm dimm %d at %s\n", dimm->id, buf);
+ dimm->cmd_family = NVDIMM_FAMILY_PAPR_SCM;
+
+ /* Parse dimm flags */
+ parse_papr_scm_flags(dimm, buf);
+
+ /* Allocate monitor mode fd */
+ dimm->health_eventfd = open(path, O_RDONLY|O_CLOEXEC);
+ rc = 0;
+ }
+ /* Ignore dimm if unknown type */
+
+ out:
+ free(path);
+ return rc;
+}
+
static int add_nfit_dimm(struct ndctl_dimm *dimm, const char *dimm_base)
{
int i, rc = -1;
@@ -1619,6 +1692,8 @@ static void *add_dimm(void *parent, int id, const char *dimm_base)
if (ndctl_bus_has_nfit(bus)) {
dimm->formats = formats;
rc = add_nfit_dimm(dimm, dimm_base);
+ } else if (ndctl_bus_has_of_node(bus)) {
+ rc = add_of_pmem_dimm(dimm, dimm_base);
}
if (rc == -ENODEV) {
@@ -1636,6 +1711,9 @@ static void *add_dimm(void *parent, int id, const char *dimm_base)
dimm->ops = msft_dimm_ops;
if (dimm->cmd_family == NVDIMM_FAMILY_HYPERV)
dimm->ops = hyperv_dimm_ops;
+ if (dimm->cmd_family == NVDIMM_FAMILY_PAPR_SCM)
+ dimm->ops = papr_scm_dimm_ops;
+
out:
if (rc) {
err(ctx, "Unable to probe dimm:%d. Err:%d\n", id, rc);
diff --git a/ndctl/lib/papr_scm.c b/ndctl/lib/papr_scm.c
new file mode 100644
index 000000000000..14ff8879f627
--- /dev/null
+++ b/ndctl/lib/papr_scm.c
@@ -0,0 +1,32 @@
+/*
+ * Copyright (C) 2020 IBM Corporation
+ *
+ * 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 <stdint.h>
+#include <stdlib.h>
+#include <limits.h>
+#include <util/log.h>
+#include <ndctl.h>
+#include <ndctl/libndctl.h>
+#include <lib/private.h>
+
+static bool papr_cmd_is_supported(struct ndctl_dimm *dimm, int cmd)
+{
+ /* Handle this separately to support monitor mode */
+ if (cmd == ND_CMD_SMART)
+ return true;
+
+ return !!(dimm->cmd_mask & (1ULL << cmd));
+}
+
+struct ndctl_dimm_ops * const papr_scm_dimm_ops = &(struct ndctl_dimm_ops) {
+ .cmd_is_supported = papr_cmd_is_supported,
+};
diff --git a/ndctl/lib/private.h b/ndctl/lib/private.h
index 2e537f0a8649..05c703ed71b4 100644
--- a/ndctl/lib/private.h
+++ b/ndctl/lib/private.h
@@ -167,6 +167,7 @@ struct ndctl_bus {
int dimms_init;
int regions_init;
int has_nfit;
+ int has_of_node;
char *bus_path;
char *bus_buf;
size_t buf_len;
@@ -352,6 +353,7 @@ extern struct ndctl_dimm_ops * const intel_dimm_ops;
extern struct ndctl_dimm_ops * const hpe1_dimm_ops;
extern struct ndctl_dimm_ops * const msft_dimm_ops;
extern struct ndctl_dimm_ops * const hyperv_dimm_ops;
+extern struct ndctl_dimm_ops * const papr_scm_dimm_ops;
static inline struct ndctl_bus *cmd_to_bus(struct ndctl_cmd *cmd)
{
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index 2580f433ade8..daf11b8ce4ea 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -119,6 +119,7 @@ struct ndctl_bus *ndctl_bus_get_next(struct ndctl_bus *bus);
bus = ndctl_bus_get_next(bus))
struct ndctl_ctx *ndctl_bus_get_ctx(struct ndctl_bus *bus);
int ndctl_bus_has_nfit(struct ndctl_bus *bus);
+int ndctl_bus_has_of_node(struct ndctl_bus *bus);
unsigned int ndctl_bus_get_major(struct ndctl_bus *bus);
unsigned int ndctl_bus_get_minor(struct ndctl_bus *bus);
const char *ndctl_bus_get_devname(struct ndctl_bus *bus);
diff --git a/ndctl/ndctl.h b/ndctl/ndctl.h
index 008f81cdeb9f..426708e1fd9b 100644
--- a/ndctl/ndctl.h
+++ b/ndctl/ndctl.h
@@ -263,6 +263,7 @@ struct nd_cmd_pkg {
#define NVDIMM_FAMILY_HPE2 2
#define NVDIMM_FAMILY_MSFT 3
#define NVDIMM_FAMILY_HYPERV 4
+#define NVDIMM_FAMILY_PAPR_SCM 5
#define ND_IOCTL_CALL _IOWR(ND_IOCTL, ND_CMD_CALL,\
struct nd_cmd_pkg)
--
2.25.3
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [ndctl PATCH v2 2/6] libncdtl: Add initial support for NVDIMM_FAMILY_PAPR_SCM dimm family
2020-04-20 7:55 ` [ndctl PATCH v2 2/6] libncdtl: Add initial support for NVDIMM_FAMILY_PAPR_SCM dimm family Vaibhav Jain
@ 2020-04-29 7:55 ` Aneesh Kumar K.V
2020-05-04 8:31 ` Vaibhav Jain
0 siblings, 1 reply; 12+ messages in thread
From: Aneesh Kumar K.V @ 2020-04-29 7:55 UTC (permalink / raw)
To: Vaibhav Jain, linux-nvdimm; +Cc: Vaibhav Jain
Vaibhav Jain <vaibhav@linux.ibm.com> writes:
> Add necessary scaffolding in libndctl for dimms that support papr_scm
> specification[1]. Since there can be platforms that support
> Open-Firmware[2] but not the papr_scm specification, hence the changes
> proposed first add support for probing if the dimm bus supports
> Open-Firmware. This is done via querying for sysfs attribute 'of_node'
> in dimm device sysfs directory. If available newly introduced member
> 'struct ndctl_bus.has_of_node' is set. During the probe of the dimm
> and execution of add_dimm(), the newly introduced add_of_pmem_dimm()
> is called if dimm bus reports supports Open-Firmware.
>
> Function add_of_pmem_dimm() queries the 'compatible' device tree
> attribute and based on its value assign NVDIMM_FAMILY_PAPR_SCM to the
> dimm command family. In future, based on the contents of 'compatible'
> attribute more of_pmem dimm families can be queried.
>
> We also add support for parsing the dimm flags for
> NVDIMM_FAMILY_PAPR_SCM supporting nvdimms as described at [3]. A newly
> introduced function parse_papr_scm_flags() reads the contents of this
> flag file and sets appropriate flag bits in 'struct
> ndctl_dimm.flags'.
The mixing of of_pmem and papr_scm is confuring here considering we have
two different driver in the kernel. If both can be handled by the same
code them possibly function that indicate both? ie, replace
add_of_pmem_dimm() with something more generic?
>
> Also we advertise support for monitor mode by allocating a file
> descriptor to the dimm 'flags' file and assigning it to 'struct
> ndctl_dimm.health_event_fd'.
>
> The dimm-ops implementation for NVDIMM_FAMILY_PAPR_SCM is
> available in global variable 'papr_scm_dimm_ops' which points to
> skeleton implementation in newly introduced file 'lib/papr_scm.c'.
>
-aneesh
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [ndctl PATCH v2 2/6] libncdtl: Add initial support for NVDIMM_FAMILY_PAPR_SCM dimm family
2020-04-29 7:55 ` Aneesh Kumar K.V
@ 2020-05-04 8:31 ` Vaibhav Jain
0 siblings, 0 replies; 12+ messages in thread
From: Vaibhav Jain @ 2020-05-04 8:31 UTC (permalink / raw)
To: Aneesh Kumar K.V, linux-nvdimm
"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
> Vaibhav Jain <vaibhav@linux.ibm.com> writes:
>
>> Add necessary scaffolding in libndctl for dimms that support papr_scm
>> specification[1]. Since there can be platforms that support
>> Open-Firmware[2] but not the papr_scm specification, hence the changes
>> proposed first add support for probing if the dimm bus supports
>> Open-Firmware. This is done via querying for sysfs attribute 'of_node'
>> in dimm device sysfs directory. If available newly introduced member
>> 'struct ndctl_bus.has_of_node' is set. During the probe of the dimm
>> and execution of add_dimm(), the newly introduced add_of_pmem_dimm()
>> is called if dimm bus reports supports Open-Firmware.
>>
>> Function add_of_pmem_dimm() queries the 'compatible' device tree
>> attribute and based on its value assign NVDIMM_FAMILY_PAPR_SCM to the
>> dimm command family. In future, based on the contents of 'compatible'
>> attribute more of_pmem dimm families can be queried.
>>
>> We also add support for parsing the dimm flags for
>> NVDIMM_FAMILY_PAPR_SCM supporting nvdimms as described at [3]. A newly
>> introduced function parse_papr_scm_flags() reads the contents of this
>> flag file and sets appropriate flag bits in 'struct
>> ndctl_dimm.flags'.
>
> The mixing of of_pmem and papr_scm is confuring here considering we have
> two different driver in the kernel. If both can be handled by the same
> code them possibly function that indicate both? ie, replace
> add_of_pmem_dimm() with something more generic?
Currently this function handles only papr compliant memory hence will
rename the function to add_papr_pmem_dimm() as discussed offline.
>
>>
>> Also we advertise support for monitor mode by allocating a file
>> descriptor to the dimm 'flags' file and assigning it to 'struct
>> ndctl_dimm.health_event_fd'.
>>
>> The dimm-ops implementation for NVDIMM_FAMILY_PAPR_SCM is
>> available in global variable 'papr_scm_dimm_ops' which points to
>> skeleton implementation in newly introduced file 'lib/papr_scm.c'.
>>
>
>
> -aneesh
> _______________________________________________
> Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
> To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
~ Vaibhav
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
^ permalink raw reply [flat|nested] 12+ messages in thread
* [ndctl PATCH v2 3/6] libndctl: Introduce new dimm-ops dimm_init() & dimm_uninit()
2020-04-20 7:55 [ndctl PATCH v2 0/6] Add support for reporting papr-scm nvdimm health Vaibhav Jain
2020-04-20 7:55 ` [ndctl PATCH v2 1/6] libndctl: Refactor out add_dimm() to handle NFIT specific init Vaibhav Jain
2020-04-20 7:55 ` [ndctl PATCH v2 2/6] libncdtl: Add initial support for NVDIMM_FAMILY_PAPR_SCM dimm family Vaibhav Jain
@ 2020-04-20 7:55 ` Vaibhav Jain
2020-04-20 7:55 ` [ndctl PATCH v2 4/6] libndctl,papr_scm: Add definitions for PAPR nvdimm specific methods Vaibhav Jain
` (2 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Vaibhav Jain @ 2020-04-20 7:55 UTC (permalink / raw)
To: linux-nvdimm; +Cc: Vaibhav Jain, Aneesh Kumar K . V
There are scenarios when a dimm-provider need to allocate some
per-dimm data that can be quickly retrieved. This data can be used to
cache data that spans multiple 'struct ndctl_cmd' submissions.
Unfortunately currently in libnvdimm there is no easy way to implement
this. Even if this data is some how stored in some field of 'struct
ndctl_dimm', managing its lifetime is a challenge.
To solve this problem, the patch proposes a new member 'struct
ndctl_dimm.dimm_user_data' to store per-dimm data interpretation of
which is specific to a dimm-provider. Also two new dimm-ops namely
dimm_init() & dimm_uninit() are introduced that can be used to manage
the lifetime of this per-dimm data.
Semantics
=========
int (*dimm_init)(struct ndctl_dimm *):
This callback will be called just after dimm-probe inside add_dimm()
is completed. Dimm-providers should use this callback to allocate
per-dimm data and assign it to 'struct ndctl_dimm.dimm_user_data'
member. In case this function returns an error, dimm initialization is
halted and errors out.
void (*dimm_uninit)(struct ndctl_dimm *):
This callback will be called during free_dimm() and is only called if
previous call to 'dimm_ops->dimm_init()' had reported no
error. Dimm-providers should use this callback to unallocate and
cleanup 'dimm_user_data'.
Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
Changelog:
v1..v2:
* Changed the patch order
---
ndctl/lib/libndctl.c | 11 +++++++++++
ndctl/lib/private.h | 5 +++++
2 files changed, 16 insertions(+)
diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index 60dad8e56f5a..cda17ff32410 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -598,6 +598,11 @@ static void free_dimm(struct ndctl_dimm *dimm)
{
if (!dimm)
return;
+
+ /* If needed call the dimm uninitialization function */
+ if (dimm->ops && dimm->ops->dimm_uninit)
+ dimm->ops->dimm_uninit(dimm);
+
free(dimm->unique_id);
free(dimm->dimm_buf);
free(dimm->dimm_path);
@@ -1714,8 +1719,14 @@ static void *add_dimm(void *parent, int id, const char *dimm_base)
if (dimm->cmd_family == NVDIMM_FAMILY_PAPR_SCM)
dimm->ops = papr_scm_dimm_ops;
+ /* Call the dimm initialization function if needed */
+ if (!rc && dimm->ops && dimm->ops->dimm_init)
+ rc = dimm->ops->dimm_init(dimm);
+
out:
if (rc) {
+ /* Ensure dimm_uninit() is not called during free_dimm() */
+ dimm->ops = NULL;
err(ctx, "Unable to probe dimm:%d. Err:%d\n", id, rc);
goto err_read;
}
diff --git a/ndctl/lib/private.h b/ndctl/lib/private.h
index 05c703ed71b4..679e359a1070 100644
--- a/ndctl/lib/private.h
+++ b/ndctl/lib/private.h
@@ -99,6 +99,7 @@ struct ndctl_dimm {
} flags;
int locked;
int aliased;
+ void *dimm_user_data;
struct list_node list;
int formats;
int format[0];
@@ -347,6 +348,10 @@ struct ndctl_dimm_ops {
int (*fw_update_supported)(struct ndctl_dimm *);
int (*xlat_firmware_status)(struct ndctl_cmd *);
u32 (*get_firmware_status)(struct ndctl_cmd *);
+ /* Called just after dimm is initialized and probed */
+ int (*dimm_init)(struct ndctl_dimm *);
+ /* Called just before struct ndctl_dimm is de-allocated */
+ void (*dimm_uninit)(struct ndctl_dimm *);
};
extern struct ndctl_dimm_ops * const intel_dimm_ops;
--
2.25.3
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
^ permalink raw reply related [flat|nested] 12+ messages in thread* [ndctl PATCH v2 4/6] libndctl,papr_scm: Add definitions for PAPR nvdimm specific methods
2020-04-20 7:55 [ndctl PATCH v2 0/6] Add support for reporting papr-scm nvdimm health Vaibhav Jain
` (2 preceding siblings ...)
2020-04-20 7:55 ` [ndctl PATCH v2 3/6] libndctl: Introduce new dimm-ops dimm_init() & dimm_uninit() Vaibhav Jain
@ 2020-04-20 7:55 ` Vaibhav Jain
2020-04-20 7:55 ` [ndctl PATCH v2 5/6] libndctl,papr_scm: Add scaffolding to issue and handle PDSM requests Vaibhav Jain
2020-04-20 7:55 ` [ndctl PATCH v2 6/6] libndctl,papr_scm: Implement support for PAPR_SCM_PDSM_HEALTH Vaibhav Jain
5 siblings, 0 replies; 12+ messages in thread
From: Vaibhav Jain @ 2020-04-20 7:55 UTC (permalink / raw)
To: linux-nvdimm; +Cc: Vaibhav Jain, Aneesh Kumar K . V
Pull the kernel definition of PAPR_SCM DSM command which is located in
the proposed kernel tree patches at Ref[1] & [2].
References:
[1] "ndctl/papr_scm,uapi: Add support for PAPR nvdimm specific methods"
https://lore.kernel.org/linux-nvdimm/20200420070711.223545-4-vaibhav@linux.ibm.com/
[2] "powerpc/papr_scm: Implement support for PAPR_SCM_PDSM_HEALTH"
https://lore.kernel.org/linux-nvdimm/20200420070711.223545-5-vaibhav@linux.ibm.com/
Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
Changelog:
v1..v2:
* Switched from obsolete papr_scm_dsm.h to recent papr_scm_pdsm.h that
describes the ND_CMD_CALL interface between libndctl and papr_scm
module.
---
ndctl/lib/papr_scm.c | 1 +
ndctl/lib/papr_scm_pdsm.h | 192 ++++++++++++++++++++++++++++++++++++++
2 files changed, 193 insertions(+)
create mode 100644 ndctl/lib/papr_scm_pdsm.h
diff --git a/ndctl/lib/papr_scm.c b/ndctl/lib/papr_scm.c
index 14ff8879f627..57ae943c0260 100644
--- a/ndctl/lib/papr_scm.c
+++ b/ndctl/lib/papr_scm.c
@@ -17,6 +17,7 @@
#include <ndctl.h>
#include <ndctl/libndctl.h>
#include <lib/private.h>
+#include <papr_scm_pdsm.h>
static bool papr_cmd_is_supported(struct ndctl_dimm *dimm, int cmd)
{
diff --git a/ndctl/lib/papr_scm_pdsm.h b/ndctl/lib/papr_scm_pdsm.h
new file mode 100644
index 000000000000..f387ed3958bb
--- /dev/null
+++ b/ndctl/lib/papr_scm_pdsm.h
@@ -0,0 +1,192 @@
+/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
+/*
+ * PAPR-SCM Dimm specific methods (PDSM) and structs for libndctl
+ *
+ * (C) Copyright IBM 2020
+ *
+ * Author: Vaibhav Jain <vaibhav at linux.ibm.com>
+ */
+
+#ifndef _UAPI_ASM_POWERPC_PAPR_SCM_PDSM_H_
+#define _UAPI_ASM_POWERPC_PAPR_SCM_PDSM_H_
+
+#include <linux/types.h>
+
+#ifdef __KERNEL__
+#include <linux/ndctl.h>
+#else
+#include <ndctl.h>
+#endif
+
+/*
+ * PDSM Envelope:
+ *
+ * The ioctl ND_CMD_CALL transfers data between user-space and kernel via
+ * 'envelopes' which consists of a header and user-defined payload sections.
+ * The header is described by 'struct nd_pdsm_cmd_pkg' which expects a
+ * payload following it and offset of which relative to the struct is provided
+ * by 'nd_pdsm_cmd_pkg.payload_offset'. *
+ *
+ * +-------------+---------------------+---------------------------+
+ * | 64-Bytes | 8-Bytes | Max 184-Bytes |
+ * +-------------+---------------------+---------------------------+
+ * | nd_pdsm_cmd_pkg | |
+ * |-------------+ | |
+ * | nd_cmd_pkg | | |
+ * +-------------+---------------------+---------------------------+
+ * | nd_family | | |
+ * | nd_size_out | cmd_status | |
+ * | nd_size_in | payload_version | PAYLOAD |
+ * | nd_command | payload_offset -----> |
+ * | nd_fw_size | | |
+ * +-------------+---------------------+---------------------------+
+ *
+ * PDSM Header:
+ *
+ * The header is defined as 'struct nd_pdsm_cmd_pkg' which embeds a
+ * 'struct nd_cmd_pkg' instance. The PDSM command is assigned to member
+ * 'nd_cmd_pkg.nd_command'. Apart from size information of the envelope which is
+ * contained in 'struct nd_cmd_pkg', the header also has members following
+ * members:
+ *
+ * 'cmd_status' : (Out) Errors if any encountered while servicing PDSM.
+ * 'payload_version' : (In/Out) Version number associated with the payload.
+ * 'payload_offset' : (In)Relative offset of payload from start of envelope.
+ *
+ * PDSM Payload:
+ *
+ * The layout of the PDSM Payload is defined by various structs shared between
+ * papr_scm and libndctl so that contents of payload can be interpreted. During
+ * servicing of a PDSM the papr_scm module will read input args from the payload
+ * field by casting its contents to an appropriate struct pointer based on the
+ * PDSM command. Similarly the output of servicing the PDSM command will be
+ * copied to the payload field using the same struct.
+ *
+ * 'libnvdimm' enforces a hard limit of 256 bytes on the envelope size, which
+ * leaves around 184 bytes for the envelope payload (ignoring any padding that
+ * the compiler may silently introduce).
+ *
+ * Payload Version:
+ *
+ * A 'payload_version' field is present in PDSM header that indicates a specific
+ * version of the structure present in PDSM Payload for a given PDSM command.
+ * This provides backward compatibility in case the PDSM Payload structure
+ * evolves and different structures are supported by 'papr_scm' and 'libndctl'.
+ *
+ * When sending a PDSM Payload to 'papr_scm', 'libndctl' should send the version
+ * of the payload struct it supports via 'payload_version' field. The 'papr_scm'
+ * module when servicing the PDSM envelope checks the 'payload_version' and then
+ * uses 'payload struct version' == MIN('payload_version field',
+ * 'max payload-struct-version supported by papr_scm') to service the PDSM.
+ * After servicing the PDSM, 'papr_scm' put the negotiated version of payload
+ * struct in returned 'payload_version' field.
+ *
+ * Libndctl on receiving the envelope back from papr_scm again checks the
+ * 'payload_version' field and based on it use the appropriate version dsm
+ * struct to parse the results.
+ *
+ * Backward Compatibility:
+ *
+ * Above scheme of exchanging different versioned PDSM struct between libndctl
+ * and papr_scm should provide backward compatibility until following two
+ * assumptions/conditions when defining new PDSM structs hold:
+ *
+ * Let T(X) = { set of attributes in PDSM struct 'T' versioned X }
+ *
+ * 1. T(X) is a proper subset of T(Y) if X > Y.
+ * i.e Each new version of PDSM struct should retain existing struct
+ * attributes from previous version
+ *
+ * 2. If an entity (libndctl or papr_scm) supports a PDSM struct T(X) then
+ * it should also support T(1), T(2)...T(X - 1).
+ * i.e When adding support for new version of a PDSM struct, libndctl
+ * and papr_scm should retain support of the existing PDSM struct
+ * version they support.
+ */
+
+#ifndef __packed
+#define __packed __attribute__((packed))
+#endif
+
+/* Papr-scm-header + payload expected with ND_CMD_CALL ioctl from libnvdimm */
+struct nd_pdsm_cmd_pkg {
+ struct nd_cmd_pkg hdr; /* Package header containing sub-cmd */
+ __s32 cmd_status; /* Out: Sub-cmd status returned back */
+ __u16 payload_offset; /* In: offset from start of struct */
+ __u16 payload_version; /* In/Out: version of the payload */
+ __u8 payload[]; /* In/Out: Sub-cmd data buffer */
+} __packed;
+
+/*
+ * Methods to be embedded in ND_CMD_CALL request. These are sent to the kernel
+ * via 'nd_pdsm_cmd_pkg.hdr.nd_command' member of the ioctl struct
+ */
+enum papr_scm_pdsm {
+ PAPR_SCM_PDSM_MIN = 0x0,
+ PAPR_SCM_PDSM_HEALTH,
+ PAPR_SCM_PDSM_MAX,
+};
+
+/* Helpers to evaluate the size of PDSM envelope */
+/* Calculate the papr_scm-header size */
+#define ND_PDSM_ENVELOPE_CONTENT_HDR_SIZE \
+ (sizeof(struct nd_pdsm_cmd_pkg) - sizeof(struct nd_cmd_pkg))
+
+/* Given a type calculate envelope-content size (papr_scm-header + payload) */
+#define ND_PDSM_ENVELOPE_CONTENT_SIZE(_type_) \
+ (sizeof(_type_) + ND_PDSM_ENVELOPE_CONTENT_HDR_SIZE)
+
+/* Convert a libnvdimm nd_cmd_pkg to pdsm specific pkg */
+static struct nd_pdsm_cmd_pkg *nd_to_pdsm_cmd_pkg(struct nd_cmd_pkg *cmd)
+{
+ return (struct nd_pdsm_cmd_pkg *) cmd;
+}
+
+/* Return the payload pointer for a given pcmd */
+static void *pdsm_cmd_to_payload(struct nd_pdsm_cmd_pkg *pcmd)
+{
+ if (pcmd->hdr.nd_size_in == 0 && pcmd->hdr.nd_size_out == 0)
+ return NULL;
+ else
+ return (void *)((__u8 *) pcmd + pcmd->payload_offset);
+}
+
+/* Various scm-dimm health indicators */
+#define PAPR_PDSM_DIMM_HEALTHY 0
+#define PAPR_PDSM_DIMM_UNHEALTHY 1
+#define PAPR_PDSM_DIMM_CRITICAL 2
+#define PAPR_PDSM_DIMM_FATAL 3
+
+/*
+ * Struct exchanged between kernel & ndctl in for PAPR_SCM_PDSM_HEALTH
+ * Various flags indicate the health status of the dimm.
+ *
+ * dimm_unarmed : Dimm not armed. So contents wont persist.
+ * dimm_bad_shutdown : Previous shutdown did not persist contents.
+ * dimm_bad_restore : Contents from previous shutdown werent restored.
+ * dimm_scrubbed : Contents of the dimm have been scrubbed.
+ * dimm_locked : Contents of the dimm cant be modified until CEC reboot
+ * dimm_encrypted : Contents of dimm are encrypted.
+ * dimm_health : Dimm health indicator. One of PAPR_PDSM_DIMM_XXXX
+ */
+struct nd_papr_pdsm_health_v1 {
+ __u8 dimm_unarmed;
+ __u8 dimm_bad_shutdown;
+ __u8 dimm_bad_restore;
+ __u8 dimm_scrubbed;
+ __u8 dimm_locked;
+ __u8 dimm_encrypted;
+ __u16 dimm_health;
+} __packed;
+
+/*
+ * Typedef the current struct for dimm_health so that any application
+ * or kernel recompiled after introducing a new version automatically
+ * supports the new version.
+ */
+#define nd_papr_pdsm_health nd_papr_pdsm_health_v1
+
+/* Current version number for the dimm health struct */
+#define ND_PAPR_PDSM_HEALTH_VERSION 1
+
+#endif /* _UAPI_ASM_POWERPC_PAPR_SCM_PDSM_H_ */
--
2.25.3
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
^ permalink raw reply related [flat|nested] 12+ messages in thread* [ndctl PATCH v2 5/6] libndctl,papr_scm: Add scaffolding to issue and handle PDSM requests
2020-04-20 7:55 [ndctl PATCH v2 0/6] Add support for reporting papr-scm nvdimm health Vaibhav Jain
` (3 preceding siblings ...)
2020-04-20 7:55 ` [ndctl PATCH v2 4/6] libndctl,papr_scm: Add definitions for PAPR nvdimm specific methods Vaibhav Jain
@ 2020-04-20 7:55 ` Vaibhav Jain
2020-04-20 7:55 ` [ndctl PATCH v2 6/6] libndctl,papr_scm: Implement support for PAPR_SCM_PDSM_HEALTH Vaibhav Jain
5 siblings, 0 replies; 12+ messages in thread
From: Vaibhav Jain @ 2020-04-20 7:55 UTC (permalink / raw)
To: linux-nvdimm; +Cc: Vaibhav Jain, Aneesh Kumar K . V
This patch implement necessary infrastructure inside 'papr_scm.c' to
issue and handle PDSM requests. Changes implemented are:
* Implement dimm initialization/un-initialization functions
papr_dimm_init()/unint() to allocate a per-dimm 'struct dimm_priv'
instance.
* New helper function allocate_cmd() to allocate command packages for
a specific PDSM command and payload size.
* New function update_dimm_state() to parse a given command payload
and update per dimm 'struct dimm_priv'.
* Provide an implementation of 'dimm_ops->smart_get_flags' to send the
submitted instance of 'struct ndctl_cmd' to update_dimm_state().
* Logging helpers for papr_scm that use the underlying libndctl
provided logging.
Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
Changelog:
v1..v2:
* Added new dimm callback 'papr_get_firmware_status'
* Switched to new papr_scm interface as described by papr_scm_dsm.h
* Changed the case of logging functions [ Santosh Sivaraj ]
* Removed redundant logging functions.
* Minor updates to patch description to s/DSM/PDSM/
---
ndctl/lib/papr_scm.c | 176 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 176 insertions(+)
diff --git a/ndctl/lib/papr_scm.c b/ndctl/lib/papr_scm.c
index 57ae943c0260..9c11dd31a133 100644
--- a/ndctl/lib/papr_scm.c
+++ b/ndctl/lib/papr_scm.c
@@ -19,6 +19,23 @@
#include <lib/private.h>
#include <papr_scm_pdsm.h>
+/* Utility logging maros for simplify logging */
+#define papr_dbg(_dimm_, _format_str_, ...) dbg(_dimm_->bus->ctx, \
+ "papr_scm:"#_format_str_, \
+ ##__VA_ARGS__)
+#define papr_err(_dimm_, _format_str_, ...) err(_dimm_->bus->ctx, \
+ "papr_scm:"#_format_str_, \
+ ##__VA_ARGS__)
+
+/* Command flags to indicate if a given command is parsed of not */
+#define CMD_PKG_SUBMITTED 1
+#define CMD_PKG_PARSED 2
+
+/* Per dimm data. Holds per-dimm data parsed from the cmd_pkgs */
+struct dimm_priv {
+ /* Empty for now */
+};
+
static bool papr_cmd_is_supported(struct ndctl_dimm *dimm, int cmd)
{
/* Handle this separately to support monitor mode */
@@ -28,6 +45,165 @@ static bool papr_cmd_is_supported(struct ndctl_dimm *dimm, int cmd)
return !!(dimm->cmd_mask & (1ULL << cmd));
}
+static __u64 pcmd_to_pdsm(const struct nd_pdsm_cmd_pkg *pcmd)
+{
+ return pcmd->hdr.nd_command;
+}
+
+static u32 papr_get_firmware_status(struct ndctl_cmd *cmd)
+{
+ const struct nd_pdsm_cmd_pkg *pcmd = nd_to_pdsm_cmd_pkg(cmd->pkg);
+
+ return (u32) pcmd->cmd_status;
+}
+
+/* Verify if the given command is supported and valid */
+static bool cmd_is_valid(struct ndctl_dimm *dimm, struct ndctl_cmd *cmd)
+{
+ const struct nd_pdsm_cmd_pkg *pcmd = nd_to_pdsm_cmd_pkg(cmd->pkg);
+
+ if (dimm == NULL)
+ return false;
+
+ if (cmd == NULL) {
+ papr_err(dimm, "Invalid command\n");
+ return false;
+ }
+
+ /* Verify the command family */
+ if (pcmd->hdr.nd_family != NVDIMM_FAMILY_PAPR_SCM) {
+ papr_err(dimm, "Invalid command family:0x%016llx\n",
+ pcmd->hdr.nd_family);
+ return false;
+ }
+
+ /* Verify the PDSM */
+ if (pcmd_to_pdsm(pcmd) <= PAPR_SCM_PDSM_MIN ||
+ pcmd_to_pdsm(pcmd) >= PAPR_SCM_PDSM_MAX) {
+ papr_err(dimm, "Invalid command :0x%016llx\n",
+ pcmd->hdr.nd_command);
+ return false;
+ }
+
+ return true;
+}
+
+/* Parse a command payload and update dimm flags/private data */
+static int update_dimm_stats(struct ndctl_dimm *dimm, struct ndctl_cmd *cmd)
+{
+ const struct nd_pdsm_cmd_pkg *pcmd;
+
+ if (!cmd_is_valid(dimm, cmd))
+ return -EINVAL;
+
+ /*
+ * Silently prevent parsing of an already parsed ndctl_cmd else
+ * mark the command as parsed.
+ */
+ if (cmd->status >= CMD_PKG_PARSED) {
+ return 0;
+ } else if (cmd->status < 0) {
+ papr_err(dimm, "Command error %d\n", cmd->status);
+ return -ENXIO;
+ }
+
+ /* Mark the command as parsed */
+ cmd->status = CMD_PKG_PARSED;
+
+ /* Get the pdsm request and handle it */
+ pcmd = nd_to_pdsm_cmd_pkg(cmd->pkg);
+ switch (pcmd_to_pdsm(pcmd)) {
+ default:
+ papr_err(dimm, "Unhandled pdsm-request 0x%016llx\n",
+ pcmd_to_pdsm(pcmd));
+ return -ENOENT;
+ }
+}
+
+/* Allocate a struct ndctl_cmd for given pdsm request with payload size */
+static struct ndctl_cmd *allocate_cmd(struct ndctl_dimm *dimm,
+ __u64 pdsm_cmd, size_t payload_size,
+ uint16_t payload_version)
+{
+ struct ndctl_cmd *cmd;
+ struct nd_pdsm_cmd_pkg *pcmd;
+ size_t size;
+
+ size = sizeof(struct ndctl_cmd) +
+ sizeof(struct nd_pdsm_cmd_pkg) + payload_size;
+ cmd = calloc(1, size);
+ if (!cmd)
+ return NULL;
+ pcmd = nd_to_pdsm_cmd_pkg(cmd->pkg);
+
+ ndctl_cmd_ref(cmd);
+ cmd->dimm = dimm;
+ cmd->type = ND_CMD_CALL;
+ cmd->size = size;
+ cmd->status = CMD_PKG_SUBMITTED;
+ cmd->get_firmware_status = &papr_get_firmware_status;
+
+ /* Populate the nd_cmd_pkg contained in nd_pdsm_cmd_pkg */
+ pcmd->hdr.nd_family = NVDIMM_FAMILY_PAPR_SCM;
+ pcmd->hdr.nd_command = pdsm_cmd;
+
+ pcmd->payload_version = payload_version;
+ pcmd->payload_offset = sizeof(struct nd_pdsm_cmd_pkg);
+
+ /* Keep payload size empty. To be populated by called */
+ pcmd->hdr.nd_fw_size = 0;
+ pcmd->hdr.nd_size_out = 0;
+ pcmd->hdr.nd_size_in = 0;
+
+ return cmd;
+}
+
+static unsigned int papr_smart_get_flags(struct ndctl_cmd *cmd)
+{
+ /* In case of error return empty flags * */
+ if (update_dimm_stats(cmd->dimm, cmd))
+ return 0;
+
+ /* Return empty flags for now as no DSM support */
+ return 0;
+}
+
+static int papr_dimm_init(struct ndctl_dimm *dimm)
+{
+ struct dimm_priv *p;
+
+ if (dimm->dimm_user_data) {
+ papr_dbg(dimm, "Dimm already initialized !!\n");
+ return 0;
+ }
+
+ p = calloc(1, sizeof(struct dimm_priv));
+ if (!p) {
+ papr_err(dimm, "Unable to allocate memory for dimm-private\n");
+ return -1;
+ }
+
+ dimm->dimm_user_data = p;
+ return 0;
+}
+
+static void papr_dimm_uninit(struct ndctl_dimm *dimm)
+{
+ struct dimm_priv *p = dimm->dimm_user_data;
+
+ if (!p) {
+ papr_dbg(dimm, "Dimm already un-initialized !!\n");
+ return;
+ }
+
+ dimm->dimm_user_data = NULL;
+ free(p);
+}
+
struct ndctl_dimm_ops * const papr_scm_dimm_ops = &(struct ndctl_dimm_ops) {
.cmd_is_supported = papr_cmd_is_supported,
+ .dimm_init = papr_dimm_init,
+ .dimm_uninit = papr_dimm_uninit,
+ .smart_get_flags = papr_smart_get_flags,
+ .get_firmware_status = papr_get_firmware_status,
};
--
2.25.3
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
^ permalink raw reply related [flat|nested] 12+ messages in thread* [ndctl PATCH v2 6/6] libndctl,papr_scm: Implement support for PAPR_SCM_PDSM_HEALTH
2020-04-20 7:55 [ndctl PATCH v2 0/6] Add support for reporting papr-scm nvdimm health Vaibhav Jain
` (4 preceding siblings ...)
2020-04-20 7:55 ` [ndctl PATCH v2 5/6] libndctl,papr_scm: Add scaffolding to issue and handle PDSM requests Vaibhav Jain
@ 2020-04-20 7:55 ` Vaibhav Jain
5 siblings, 0 replies; 12+ messages in thread
From: Vaibhav Jain @ 2020-04-20 7:55 UTC (permalink / raw)
To: linux-nvdimm; +Cc: Vaibhav Jain, Aneesh Kumar K . V
Add support for reporting DIMM health and shutdown state by issuing
PAPR_SCM_PDSM_HEALTH request to papr_scm module. It returns an
instance of 'struct nd_papr_pdsm_health' as defined in
'papr_scm_pdsm.h'. The patch provides support for dimm-ops
'new_smart', 'smart_get_health' & 'smart_get_shutdown_state' as newly
introduced functions papr_new_smart_health(), papr_smart_get_health()
& papr_smart_get_shutdown_state() respectively. These callbacks should
enable ndctl to report DIMM health.
Also a new member 'struct dimm_priv.health' is introduced which holds
the current health status of the dimm. This member is set inside newly
added function 'update_dimm_health_v1()' which parses the v1 payload
returned by the kernel after servicing PAPR_SCM_PDSM_HEALTH. The
function will also update dimm-flags viz 'struct ndctl_dimm.flags.f_*'
based on the flags set in the returned payload.
Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
Changelog:
v1..v2:
* Squashed patch to report nvdimm bad shutdown state with this patch.
* Switched to new structs/enums as defined in papr_scm_pdsm.h
---
ndctl/lib/papr_scm.c | 90 ++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 87 insertions(+), 3 deletions(-)
diff --git a/ndctl/lib/papr_scm.c b/ndctl/lib/papr_scm.c
index 9c11dd31a133..a54e632fa72c 100644
--- a/ndctl/lib/papr_scm.c
+++ b/ndctl/lib/papr_scm.c
@@ -33,7 +33,9 @@
/* Per dimm data. Holds per-dimm data parsed from the cmd_pkgs */
struct dimm_priv {
- /* Empty for now */
+
+ /* Cache the dimm health status */
+ struct nd_papr_pdsm_health health;
};
static bool papr_cmd_is_supported(struct ndctl_dimm *dimm, int cmd)
@@ -88,6 +90,43 @@ static bool cmd_is_valid(struct ndctl_dimm *dimm, struct ndctl_cmd *cmd)
return true;
}
+/*
+ * Parse the nd_papr_pdsm_health_v1 payload embedded in ndctl_cmd and
+ * update dimm health/flags
+ */
+static int update_dimm_health_v1(struct ndctl_dimm *dimm, struct ndctl_cmd *cmd)
+{
+ struct nd_pdsm_cmd_pkg *pcmd = nd_to_pdsm_cmd_pkg(cmd->pkg);
+ struct dimm_priv *p = dimm->dimm_user_data;
+ const struct nd_papr_pdsm_health_v1 *health =
+ pdsm_cmd_to_payload(pcmd);
+
+ /* Update the dimm flags */
+ dimm->flags.f_arm = health->dimm_unarmed;
+ dimm->flags.f_flush = health->dimm_bad_shutdown;
+ dimm->flags.f_restore = health->dimm_bad_restore;
+ dimm->flags.f_smart = (health->dimm_health != 0);
+
+ /* Cache the dimm health information */
+ memcpy(&p->health, health, sizeof(*health));
+ return 0;
+}
+
+/* Check payload version returned and pass the packet to appropriate handler */
+static int update_dimm_health(struct ndctl_dimm *dimm, struct ndctl_cmd *cmd)
+{
+ const struct nd_pdsm_cmd_pkg *pcmd = nd_to_pdsm_cmd_pkg(cmd->pkg);
+
+ if (pcmd->payload_version == 1)
+ return update_dimm_health_v1(dimm, cmd);
+
+ /* unknown version */
+ papr_err(dimm, "Unknown payload version for dimm_health.\n");
+ papr_dbg(dimm, "dimm_health payload Ver=%d, Supported=%d\n",
+ pcmd->payload_version, ND_PAPR_PDSM_HEALTH_VERSION);
+ return -EINVAL;
+}
+
/* Parse a command payload and update dimm flags/private data */
static int update_dimm_stats(struct ndctl_dimm *dimm, struct ndctl_cmd *cmd)
{
@@ -113,6 +152,8 @@ static int update_dimm_stats(struct ndctl_dimm *dimm, struct ndctl_cmd *cmd)
/* Get the pdsm request and handle it */
pcmd = nd_to_pdsm_cmd_pkg(cmd->pkg);
switch (pcmd_to_pdsm(pcmd)) {
+ case PAPR_SCM_PDSM_HEALTH:
+ return update_dimm_health(dimm, cmd);
default:
papr_err(dimm, "Unhandled pdsm-request 0x%016llx\n",
pcmd_to_pdsm(pcmd));
@@ -158,14 +199,54 @@ static struct ndctl_cmd *allocate_cmd(struct ndctl_dimm *dimm,
return cmd;
}
+static struct ndctl_cmd *papr_new_smart_health(struct ndctl_dimm *dimm)
+{
+ struct ndctl_cmd *cmd_ret;
+
+ cmd_ret = allocate_cmd(dimm, PAPR_SCM_PDSM_HEALTH,
+ sizeof(struct nd_papr_pdsm_health),
+ ND_PAPR_PDSM_HEALTH_VERSION);
+ if (!cmd_ret) {
+ papr_err(dimm, "Unable to allocate smart_health command\n");
+ return NULL;
+ }
+
+ cmd_ret->pkg[0].nd_size_out = ND_PDSM_ENVELOPE_CONTENT_SIZE(
+ struct nd_papr_pdsm_health);
+
+ return cmd_ret;
+}
+
+static unsigned int papr_smart_get_health(struct ndctl_cmd *cmd)
+{
+ struct dimm_priv *p = cmd->dimm->dimm_user_data;
+
+ /*
+ * Update the dimm stats and use some math to return one of
+ * defined ND_SMART_*_HEALTH values
+ */
+ if (update_dimm_stats(cmd->dimm, cmd) || !p->health.dimm_health)
+ return 0;
+ else
+ return 1 << (p->health.dimm_health - 1);
+}
+
+static unsigned int papr_smart_get_shutdown_state(struct ndctl_cmd *cmd)
+{
+ struct dimm_priv *p = cmd->dimm->dimm_user_data;
+
+ /* Update dimm state and return f_flush */
+ return update_dimm_stats(cmd->dimm, cmd) ?
+ 0 : p->health.dimm_bad_shutdown;
+}
+
static unsigned int papr_smart_get_flags(struct ndctl_cmd *cmd)
{
/* In case of error return empty flags * */
if (update_dimm_stats(cmd->dimm, cmd))
return 0;
- /* Return empty flags for now as no DSM support */
- return 0;
+ return ND_SMART_HEALTH_VALID | ND_SMART_SHUTDOWN_VALID;
}
static int papr_dimm_init(struct ndctl_dimm *dimm)
@@ -206,4 +287,7 @@ struct ndctl_dimm_ops * const papr_scm_dimm_ops = &(struct ndctl_dimm_ops) {
.dimm_uninit = papr_dimm_uninit,
.smart_get_flags = papr_smart_get_flags,
.get_firmware_status = papr_get_firmware_status,
+ .new_smart = papr_new_smart_health,
+ .smart_get_health = papr_smart_get_health,
+ .smart_get_shutdown_state = papr_smart_get_shutdown_state,
};
--
2.25.3
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
^ permalink raw reply related [flat|nested] 12+ messages in thread