* [NDCTL PATCH v3] ndctl: cxl: Remove dependency for attributes derived from IDENTIFY command
@ 2024-04-22 21:17 Dave Jiang
2024-04-22 21:35 ` Ira Weiny
0 siblings, 1 reply; 2+ messages in thread
From: Dave Jiang @ 2024-04-22 21:17 UTC (permalink / raw)
To: linux-cxl; +Cc: vishal.l.verma, Dan Williams
A memdev may optionally not host a mailbox and therefore not able to execute
the IDENTIFY command. Currently the kernel emits empty strings for some of
the attributes instead of making them invisible in order to keep backward
compatibility for CXL CLI. Remove dependency of CXL CLI on the existance of
these attributes and only expose them if they exist. Without the dependency
the kernel will be able to make the non-existant attributes invisible.
Link: https://lore.kernel.org/all/20230606121534.00003870@Huawei.com/
Suggested-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v3:
- Check against SIZE_MAX (Vishal)
---
cxl/lib/libcxl.c | 48 ++++++++++++++++++++++++++----------------------
cxl/memdev.c | 18 +++++++++++++-----
2 files changed, 39 insertions(+), 27 deletions(-)
diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
index 91725ac7ffd8..91eedd1c4688 100644
--- a/cxl/lib/libcxl.c
+++ b/cxl/lib/libcxl.c
@@ -1280,14 +1280,12 @@ static void *add_cxl_memdev(void *parent, int id, const char *cxlmem_base)
memdev->minor = minor(st.st_rdev);
sprintf(path, "%s/pmem/size", cxlmem_base);
- if (sysfs_read_attr(ctx, path, buf) < 0)
- goto err_read;
- memdev->pmem_size = strtoull(buf, NULL, 0);
+ if (sysfs_read_attr(ctx, path, buf) == 0)
+ memdev->pmem_size = strtoull(buf, NULL, 0);
sprintf(path, "%s/ram/size", cxlmem_base);
- if (sysfs_read_attr(ctx, path, buf) < 0)
- goto err_read;
- memdev->ram_size = strtoull(buf, NULL, 0);
+ if (sysfs_read_attr(ctx, path, buf) == 0)
+ memdev->ram_size = strtoull(buf, NULL, 0);
sprintf(path, "%s/pmem/qos_class", cxlmem_base);
if (sysfs_read_attr(ctx, path, buf) < 0)
@@ -1302,18 +1300,22 @@ static void *add_cxl_memdev(void *parent, int id, const char *cxlmem_base)
memdev->ram_qos_class = atoi(buf);
sprintf(path, "%s/payload_max", cxlmem_base);
- if (sysfs_read_attr(ctx, path, buf) < 0)
- goto err_read;
- memdev->payload_max = strtoull(buf, NULL, 0);
- if (memdev->payload_max < 0)
- goto err_read;
+ if (sysfs_read_attr(ctx, path, buf) == 0) {
+ memdev->payload_max = strtoull(buf, NULL, 0);
+ if (memdev->payload_max < 0)
+ goto err_read;
+ } else {
+ memdev->payload_max = -1;
+ }
sprintf(path, "%s/label_storage_size", cxlmem_base);
- if (sysfs_read_attr(ctx, path, buf) < 0)
- goto err_read;
- memdev->lsa_size = strtoull(buf, NULL, 0);
- if (memdev->lsa_size == ULLONG_MAX)
- goto err_read;
+ if (sysfs_read_attr(ctx, path, buf) == 0) {
+ memdev->lsa_size = strtoull(buf, NULL, 0);
+ if (memdev->lsa_size == ULLONG_MAX)
+ goto err_read;
+ } else {
+ memdev->lsa_size = SIZE_MAX;
+ }
sprintf(path, "%s/serial", cxlmem_base);
if (sysfs_read_attr(ctx, path, buf) < 0)
@@ -1340,12 +1342,11 @@ static void *add_cxl_memdev(void *parent, int id, const char *cxlmem_base)
host[0] = '\0';
sprintf(path, "%s/firmware_version", cxlmem_base);
- if (sysfs_read_attr(ctx, path, buf) < 0)
- goto err_read;
-
- memdev->firmware_version = strdup(buf);
- if (!memdev->firmware_version)
- goto err_read;
+ if (sysfs_read_attr(ctx, path, buf) == 0) {
+ memdev->firmware_version = strdup(buf);
+ if (!memdev->firmware_version)
+ goto err_read;
+ }
memdev->dev_buf = calloc(1, strlen(cxlmem_base) + 50);
if (!memdev->dev_buf)
@@ -4570,6 +4571,9 @@ static int lsa_op(struct cxl_memdev *memdev, int op, void *buf,
if (length == 0)
return 0;
+ if (memdev->payload_max < 0)
+ return -EINVAL;
+
label_iter_max = memdev->payload_max - sizeof(struct cxl_cmd_set_lsa);
while (remaining) {
cur_len = min((size_t)label_iter_max, remaining);
diff --git a/cxl/memdev.c b/cxl/memdev.c
index 35920fc81d66..6e44d1578d03 100644
--- a/cxl/memdev.c
+++ b/cxl/memdev.c
@@ -487,10 +487,13 @@ static int action_zero(struct cxl_memdev *memdev, struct action_context *actx)
size_t size;
int rc;
- if (param.len)
+ if (param.len) {
size = param.len;
- else
+ } else {
size = cxl_memdev_get_label_size(memdev);
+ if (size == SIZE_MAX)
+ return -EINVAL;
+ }
if (cxl_memdev_nvdimm_bridge_active(memdev)) {
log_err(&ml,
@@ -523,6 +526,9 @@ static int action_write(struct cxl_memdev *memdev, struct action_context *actx)
if (!size) {
size_t label_size = cxl_memdev_get_label_size(memdev);
+ if (label_size == SIZE_MAX)
+ return -EINVAL;
+
fseek(actx->f_in, 0L, SEEK_END);
size = ftell(actx->f_in);
fseek(actx->f_in, 0L, SEEK_SET);
@@ -561,11 +567,13 @@ static int action_read(struct cxl_memdev *memdev, struct action_context *actx)
char *buf;
int rc;
- if (param.len)
+ if (param.len) {
size = param.len;
- else
+ } else {
size = cxl_memdev_get_label_size(memdev);
-
+ if (size == SIZE_MAX)
+ return -EINVAL;
+ }
buf = calloc(1, size);
if (!buf)
return -ENOMEM;
--
2.44.0
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [NDCTL PATCH v3] ndctl: cxl: Remove dependency for attributes derived from IDENTIFY command
2024-04-22 21:17 [NDCTL PATCH v3] ndctl: cxl: Remove dependency for attributes derived from IDENTIFY command Dave Jiang
@ 2024-04-22 21:35 ` Ira Weiny
0 siblings, 0 replies; 2+ messages in thread
From: Ira Weiny @ 2024-04-22 21:35 UTC (permalink / raw)
To: Dave Jiang, linux-cxl; +Cc: vishal.l.verma, Dan Williams
Dave Jiang wrote:
> A memdev may optionally not host a mailbox and therefore not able to execute
> the IDENTIFY command. Currently the kernel emits empty strings for some of
> the attributes instead of making them invisible in order to keep backward
> compatibility for CXL CLI. Remove dependency of CXL CLI on the existance of
> these attributes and only expose them if they exist. Without the dependency
> the kernel will be able to make the non-existant attributes invisible.
>
> Link: https://lore.kernel.org/all/20230606121534.00003870@Huawei.com/
> Suggested-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Patch seems ok but what is the deprecation schedule for the kernel to drop
the attributes?
Ira
> ---
> v3:
> - Check against SIZE_MAX (Vishal)
> ---
> cxl/lib/libcxl.c | 48 ++++++++++++++++++++++++++----------------------
> cxl/memdev.c | 18 +++++++++++++-----
> 2 files changed, 39 insertions(+), 27 deletions(-)
>
> diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
> index 91725ac7ffd8..91eedd1c4688 100644
> --- a/cxl/lib/libcxl.c
> +++ b/cxl/lib/libcxl.c
> @@ -1280,14 +1280,12 @@ static void *add_cxl_memdev(void *parent, int id, const char *cxlmem_base)
> memdev->minor = minor(st.st_rdev);
>
> sprintf(path, "%s/pmem/size", cxlmem_base);
> - if (sysfs_read_attr(ctx, path, buf) < 0)
> - goto err_read;
> - memdev->pmem_size = strtoull(buf, NULL, 0);
> + if (sysfs_read_attr(ctx, path, buf) == 0)
> + memdev->pmem_size = strtoull(buf, NULL, 0);
>
> sprintf(path, "%s/ram/size", cxlmem_base);
> - if (sysfs_read_attr(ctx, path, buf) < 0)
> - goto err_read;
> - memdev->ram_size = strtoull(buf, NULL, 0);
> + if (sysfs_read_attr(ctx, path, buf) == 0)
> + memdev->ram_size = strtoull(buf, NULL, 0);
>
> sprintf(path, "%s/pmem/qos_class", cxlmem_base);
> if (sysfs_read_attr(ctx, path, buf) < 0)
> @@ -1302,18 +1300,22 @@ static void *add_cxl_memdev(void *parent, int id, const char *cxlmem_base)
> memdev->ram_qos_class = atoi(buf);
>
> sprintf(path, "%s/payload_max", cxlmem_base);
> - if (sysfs_read_attr(ctx, path, buf) < 0)
> - goto err_read;
> - memdev->payload_max = strtoull(buf, NULL, 0);
> - if (memdev->payload_max < 0)
> - goto err_read;
> + if (sysfs_read_attr(ctx, path, buf) == 0) {
> + memdev->payload_max = strtoull(buf, NULL, 0);
> + if (memdev->payload_max < 0)
> + goto err_read;
> + } else {
> + memdev->payload_max = -1;
> + }
>
> sprintf(path, "%s/label_storage_size", cxlmem_base);
> - if (sysfs_read_attr(ctx, path, buf) < 0)
> - goto err_read;
> - memdev->lsa_size = strtoull(buf, NULL, 0);
> - if (memdev->lsa_size == ULLONG_MAX)
> - goto err_read;
> + if (sysfs_read_attr(ctx, path, buf) == 0) {
> + memdev->lsa_size = strtoull(buf, NULL, 0);
> + if (memdev->lsa_size == ULLONG_MAX)
> + goto err_read;
> + } else {
> + memdev->lsa_size = SIZE_MAX;
> + }
>
> sprintf(path, "%s/serial", cxlmem_base);
> if (sysfs_read_attr(ctx, path, buf) < 0)
> @@ -1340,12 +1342,11 @@ static void *add_cxl_memdev(void *parent, int id, const char *cxlmem_base)
> host[0] = '\0';
>
> sprintf(path, "%s/firmware_version", cxlmem_base);
> - if (sysfs_read_attr(ctx, path, buf) < 0)
> - goto err_read;
> -
> - memdev->firmware_version = strdup(buf);
> - if (!memdev->firmware_version)
> - goto err_read;
> + if (sysfs_read_attr(ctx, path, buf) == 0) {
> + memdev->firmware_version = strdup(buf);
> + if (!memdev->firmware_version)
> + goto err_read;
> + }
>
> memdev->dev_buf = calloc(1, strlen(cxlmem_base) + 50);
> if (!memdev->dev_buf)
> @@ -4570,6 +4571,9 @@ static int lsa_op(struct cxl_memdev *memdev, int op, void *buf,
> if (length == 0)
> return 0;
>
> + if (memdev->payload_max < 0)
> + return -EINVAL;
> +
> label_iter_max = memdev->payload_max - sizeof(struct cxl_cmd_set_lsa);
> while (remaining) {
> cur_len = min((size_t)label_iter_max, remaining);
> diff --git a/cxl/memdev.c b/cxl/memdev.c
> index 35920fc81d66..6e44d1578d03 100644
> --- a/cxl/memdev.c
> +++ b/cxl/memdev.c
> @@ -487,10 +487,13 @@ static int action_zero(struct cxl_memdev *memdev, struct action_context *actx)
> size_t size;
> int rc;
>
> - if (param.len)
> + if (param.len) {
> size = param.len;
> - else
> + } else {
> size = cxl_memdev_get_label_size(memdev);
> + if (size == SIZE_MAX)
> + return -EINVAL;
> + }
>
> if (cxl_memdev_nvdimm_bridge_active(memdev)) {
> log_err(&ml,
> @@ -523,6 +526,9 @@ static int action_write(struct cxl_memdev *memdev, struct action_context *actx)
> if (!size) {
> size_t label_size = cxl_memdev_get_label_size(memdev);
>
> + if (label_size == SIZE_MAX)
> + return -EINVAL;
> +
> fseek(actx->f_in, 0L, SEEK_END);
> size = ftell(actx->f_in);
> fseek(actx->f_in, 0L, SEEK_SET);
> @@ -561,11 +567,13 @@ static int action_read(struct cxl_memdev *memdev, struct action_context *actx)
> char *buf;
> int rc;
>
> - if (param.len)
> + if (param.len) {
> size = param.len;
> - else
> + } else {
> size = cxl_memdev_get_label_size(memdev);
> -
> + if (size == SIZE_MAX)
> + return -EINVAL;
> + }
> buf = calloc(1, size);
> if (!buf)
> return -ENOMEM;
> --
> 2.44.0
>
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-04-22 21:35 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-22 21:17 [NDCTL PATCH v3] ndctl: cxl: Remove dependency for attributes derived from IDENTIFY command Dave Jiang
2024-04-22 21:35 ` Ira Weiny
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox