Vaibhav Jain <vaibhav(a)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(a)linux.ibm.com>
---
Changelog:
v1..v2:
* None
Looks good to me. For the series,
Reviewed-by: Santosh S <santosh(a)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