diff mbox series

[ndctl,v5,10/16] libcxl: add representation for an nvdimm bridge object

Message ID 20211111204436.1560365-11-vishal.l.verma@intel.com
State New, archived
Headers show
Series Initial CXL support | expand

Commit Message

Verma, Vishal L Nov. 11, 2021, 8:44 p.m. UTC
Add an nvdimm bridge object representation internal to libcxl. A bridge
object is ited to its parent memdev object, and this patch adds its
first interface, which checks whether a bridge is 'active' - i.e.
implying the label space on the memdev is owned by the kernel.

Cc: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 cxl/lib/private.h  |  9 ++++++
 cxl/lib/libcxl.c   | 73 ++++++++++++++++++++++++++++++++++++++++++++++
 cxl/libcxl.h       |  1 +
 cxl/lib/libcxl.sym |  1 +
 4 files changed, 84 insertions(+)

Comments

Dan Williams Nov. 11, 2021, 11:49 p.m. UTC | #1
On Thu, Nov 11, 2021 at 12:45 PM Vishal Verma <vishal.l.verma@intel.com> wrote:
>
> Add an nvdimm bridge object representation internal to libcxl. A bridge
> object is ited to its parent memdev object, and this patch adds its

s/ited/tied/

> first interface, which checks whether a bridge is 'active' - i.e.
> implying the label space on the memdev is owned by the kernel.

Just some minor fixups below and you can add:

Reviewed-by: Dan Williams <dan.j.williams@intel.com>

>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> ---
>  cxl/lib/private.h  |  9 ++++++
>  cxl/lib/libcxl.c   | 73 ++++++++++++++++++++++++++++++++++++++++++++++
>  cxl/libcxl.h       |  1 +
>  cxl/lib/libcxl.sym |  1 +
>  4 files changed, 84 insertions(+)
>
> diff --git a/cxl/lib/private.h b/cxl/lib/private.h
> index c4ed741..2f0b6ea 100644
> --- a/cxl/lib/private.h
> +++ b/cxl/lib/private.h
> @@ -10,6 +10,14 @@
>
>  #define CXL_EXPORT __attribute__ ((visibility("default")))
>
> +
> +struct cxl_nvdimm_br {

Might as well spell out "bridge".

> +       int id;
> +       void *dev_buf;
> +       size_t buf_len;
> +       char *dev_path;
> +};
> +
>  struct cxl_memdev {
>         int id, major, minor;
>         void *dev_buf;
> @@ -23,6 +31,7 @@ struct cxl_memdev {
>         int payload_max;
>         size_t lsa_size;
>         struct kmod_module *module;
> +       struct cxl_nvdimm_br *bridge;
>  };
>
>  enum cxl_cmd_query_status {
> diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
> index def3a97..7bc0696 100644
> --- a/cxl/lib/libcxl.c
> +++ b/cxl/lib/libcxl.c
> @@ -45,11 +45,19 @@ struct cxl_ctx {
>         void *private_data;
>  };
>
> +static void free_bridge(struct cxl_nvdimm_br *bridge)
> +{
> +       free(bridge->dev_buf);
> +       free(bridge->dev_path);
> +       free(bridge);
> +}
> +
>  static void free_memdev(struct cxl_memdev *memdev, struct list_head *head)
>  {
>         if (head)
>                 list_del_from(head, &memdev->list);
>         kmod_module_unref(memdev->module);
> +       free_bridge(memdev->bridge);
>         free(memdev->firmware_version);
>         free(memdev->dev_buf);
>         free(memdev->dev_path);
> @@ -205,6 +213,40 @@ CXL_EXPORT void cxl_set_log_priority(struct cxl_ctx *ctx, int priority)
>         ctx->ctx.log_priority = priority;
>  }
>
> +static void *add_cxl_bridge(void *parent, int id, const char *br_base)
> +{
> +       const char *devname = devpath_to_devname(br_base);
> +       struct cxl_memdev *memdev = parent;
> +       struct cxl_ctx *ctx = memdev->ctx;
> +       struct cxl_nvdimm_br *bridge;
> +
> +       dbg(ctx, "%s: bridge_base: \'%s\'\n", devname, br_base);
> +
> +       bridge = calloc(1, sizeof(*bridge));
> +       if (!bridge)
> +               goto err_dev;
> +       bridge->id = id;
> +
> +       bridge->dev_path = strdup(br_base);
> +       if (!bridge->dev_path)
> +               goto err_read;
> +
> +       bridge->dev_buf = calloc(1, strlen(br_base) + 50);
> +       if (!bridge->dev_buf)
> +               goto err_read;
> +       bridge->buf_len = strlen(br_base) + 50;
> +
> +       memdev->bridge = bridge;
> +       return bridge;
> +
> + err_read:
> +       free(bridge->dev_buf);
> +       free(bridge->dev_path);
> +       free(bridge);
> + err_dev:
> +       return NULL;
> +}
> +
>  static void *add_cxl_memdev(void *parent, int id, const char *cxlmem_base)
>  {
>         const char *devname = devpath_to_devname(cxlmem_base);
> @@ -271,6 +313,8 @@ static void *add_cxl_memdev(void *parent, int id, const char *cxlmem_base)
>                 goto err_read;
>         memdev->buf_len = strlen(cxlmem_base) + 50;
>
> +       sysfs_device_parse(ctx, cxlmem_base, "pmem", memdev, add_cxl_bridge);
> +
>         cxl_memdev_foreach(ctx, memdev_dup)
>                 if (memdev_dup->id == memdev->id) {
>                         free_memdev(memdev, NULL);
> @@ -362,6 +406,35 @@ CXL_EXPORT size_t cxl_memdev_get_label_size(struct cxl_memdev *memdev)
>         return memdev->lsa_size;
>  }
>
> +static int is_enabled(const char *drvpath)
> +{
> +       struct stat st;
> +
> +       if (lstat(drvpath, &st) < 0 || !S_ISLNK(st.st_mode))
> +               return 0;
> +       else
> +               return 1;
> +}
> +
> +CXL_EXPORT int cxl_memdev_nvdimm_bridge_active(struct cxl_memdev *memdev)
> +{
> +       struct cxl_ctx *ctx = cxl_memdev_get_ctx(memdev);
> +       struct cxl_nvdimm_br *bridge = memdev->bridge;
> +       char *path = memdev->dev_buf;

Should this be: bridge->dev_buf?

> +       int len = memdev->buf_len;

...and this should bridge->buf_len?

Not strictly a bug, but might as well use the 'bridge' version of
these attributes, right?

> +
> +       if (!bridge)
> +               return 0;
> +
> +       if (snprintf(path, len, "%s/driver", bridge->dev_path) >= len) {
> +               err(ctx, "%s: nvdimm bridge buffer too small!\n",
> +                               cxl_memdev_get_devname(memdev));
> +               return 0;
> +       }
> +
> +       return is_enabled(path);
> +}
> +
>  CXL_EXPORT void cxl_cmd_unref(struct cxl_cmd *cmd)
>  {
>         if (!cmd)
> diff --git a/cxl/libcxl.h b/cxl/libcxl.h
> index d3b97a1..535e349 100644
> --- a/cxl/libcxl.h
> +++ b/cxl/libcxl.h
> @@ -43,6 +43,7 @@ unsigned long long cxl_memdev_get_pmem_size(struct cxl_memdev *memdev);
>  unsigned long long cxl_memdev_get_ram_size(struct cxl_memdev *memdev);
>  const char *cxl_memdev_get_firmware_verison(struct cxl_memdev *memdev);
>  size_t cxl_memdev_get_label_size(struct cxl_memdev *memdev);
> +int cxl_memdev_nvdimm_bridge_active(struct cxl_memdev *memdev);
>
>  #define cxl_memdev_foreach(ctx, memdev) \
>          for (memdev = cxl_memdev_get_first(ctx); \
> diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
> index 858e953..f3b0c63 100644
> --- a/cxl/lib/libcxl.sym
> +++ b/cxl/lib/libcxl.sym
> @@ -65,6 +65,7 @@ global:
>         cxl_cmd_new_read_label;
>         cxl_cmd_read_label_get_payload;
>         cxl_memdev_get_label_size;
> +       cxl_memdev_nvdimm_bridge_active;
>  local:
>          *;
>  };
> --
> 2.31.1
>
Verma, Vishal L Nov. 12, 2021, 9:53 p.m. UTC | #2
On Thu, 2021-11-11 at 15:49 -0800, Dan Williams wrote:
> On Thu, Nov 11, 2021 at 12:45 PM Vishal Verma <vishal.l.verma@intel.com> wrote:
> > 
> > Add an nvdimm bridge object representation internal to libcxl. A bridge
> > object is ited to its parent memdev object, and this patch adds its
> 
> s/ited/tied/
> 
> > first interface, which checks whether a bridge is 'active' - i.e.
> > implying the label space on the memdev is owned by the kernel.
> 
> Just some minor fixups below and you can add:

Thanks Dan - fixed all these up in v6

> 
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> 
> > 
> > Cc: Dan Williams <dan.j.williams@intel.com>
> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> > ---
> >  cxl/lib/private.h  |  9 ++++++
> >  cxl/lib/libcxl.c   | 73 ++++++++++++++++++++++++++++++++++++++++++++++
> >  cxl/libcxl.h       |  1 +
> >  cxl/lib/libcxl.sym |  1 +
> >  4 files changed, 84 insertions(+)
> > 
> > diff --git a/cxl/lib/private.h b/cxl/lib/private.h
> > index c4ed741..2f0b6ea 100644
> > --- a/cxl/lib/private.h
> > +++ b/cxl/lib/private.h
> > @@ -10,6 +10,14 @@
> > 
> >  #define CXL_EXPORT __attribute__ ((visibility("default")))
> > 
> > +
> > +struct cxl_nvdimm_br {
> 
> Might as well spell out "bridge".
> 
> > +       int id;
> > +       void *dev_buf;
> > +       size_t buf_len;
> > +       char *dev_path;
> > +};
> > +
> >  struct cxl_memdev {
> >         int id, major, minor;
> >         void *dev_buf;
> > @@ -23,6 +31,7 @@ struct cxl_memdev {
> >         int payload_max;
> >         size_t lsa_size;
> >         struct kmod_module *module;
> > +       struct cxl_nvdimm_br *bridge;
> >  };
> > 
> >  enum cxl_cmd_query_status {
> > diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
> > index def3a97..7bc0696 100644
> > --- a/cxl/lib/libcxl.c
> > +++ b/cxl/lib/libcxl.c
> > @@ -45,11 +45,19 @@ struct cxl_ctx {
> >         void *private_data;
> >  };
> > 
> > +static void free_bridge(struct cxl_nvdimm_br *bridge)
> > +{
> > +       free(bridge->dev_buf);
> > +       free(bridge->dev_path);
> > +       free(bridge);
> > +}
> > +
> >  static void free_memdev(struct cxl_memdev *memdev, struct list_head *head)
> >  {
> >         if (head)
> >                 list_del_from(head, &memdev->list);
> >         kmod_module_unref(memdev->module);
> > +       free_bridge(memdev->bridge);
> >         free(memdev->firmware_version);
> >         free(memdev->dev_buf);
> >         free(memdev->dev_path);
> > @@ -205,6 +213,40 @@ CXL_EXPORT void cxl_set_log_priority(struct cxl_ctx *ctx, int priority)
> >         ctx->ctx.log_priority = priority;
> >  }
> > 
> > +static void *add_cxl_bridge(void *parent, int id, const char *br_base)
> > +{
> > +       const char *devname = devpath_to_devname(br_base);
> > +       struct cxl_memdev *memdev = parent;
> > +       struct cxl_ctx *ctx = memdev->ctx;
> > +       struct cxl_nvdimm_br *bridge;
> > +
> > +       dbg(ctx, "%s: bridge_base: \'%s\'\n", devname, br_base);
> > +
> > +       bridge = calloc(1, sizeof(*bridge));
> > +       if (!bridge)
> > +               goto err_dev;
> > +       bridge->id = id;
> > +
> > +       bridge->dev_path = strdup(br_base);
> > +       if (!bridge->dev_path)
> > +               goto err_read;
> > +
> > +       bridge->dev_buf = calloc(1, strlen(br_base) + 50);
> > +       if (!bridge->dev_buf)
> > +               goto err_read;
> > +       bridge->buf_len = strlen(br_base) + 50;
> > +
> > +       memdev->bridge = bridge;
> > +       return bridge;
> > +
> > + err_read:
> > +       free(bridge->dev_buf);
> > +       free(bridge->dev_path);
> > +       free(bridge);
> > + err_dev:
> > +       return NULL;
> > +}
> > +
> >  static void *add_cxl_memdev(void *parent, int id, const char *cxlmem_base)
> >  {
> >         const char *devname = devpath_to_devname(cxlmem_base);
> > @@ -271,6 +313,8 @@ static void *add_cxl_memdev(void *parent, int id, const char *cxlmem_base)
> >                 goto err_read;
> >         memdev->buf_len = strlen(cxlmem_base) + 50;
> > 
> > +       sysfs_device_parse(ctx, cxlmem_base, "pmem", memdev, add_cxl_bridge);
> > +
> >         cxl_memdev_foreach(ctx, memdev_dup)
> >                 if (memdev_dup->id == memdev->id) {
> >                         free_memdev(memdev, NULL);
> > @@ -362,6 +406,35 @@ CXL_EXPORT size_t cxl_memdev_get_label_size(struct cxl_memdev *memdev)
> >         return memdev->lsa_size;
> >  }
> > 
> > +static int is_enabled(const char *drvpath)
> > +{
> > +       struct stat st;
> > +
> > +       if (lstat(drvpath, &st) < 0 || !S_ISLNK(st.st_mode))
> > +               return 0;
> > +       else
> > +               return 1;
> > +}
> > +
> > +CXL_EXPORT int cxl_memdev_nvdimm_bridge_active(struct cxl_memdev *memdev)
> > +{
> > +       struct cxl_ctx *ctx = cxl_memdev_get_ctx(memdev);
> > +       struct cxl_nvdimm_br *bridge = memdev->bridge;
> > +       char *path = memdev->dev_buf;
> 
> Should this be: bridge->dev_buf?
> 
> > +       int len = memdev->buf_len;
> 
> ...and this should bridge->buf_len?
> 
> Not strictly a bug, but might as well use the 'bridge' version of
> these attributes, right?
> 
> > +
> > +       if (!bridge)
> > +               return 0;
> > +
> > +       if (snprintf(path, len, "%s/driver", bridge->dev_path) >= len) {
> > +               err(ctx, "%s: nvdimm bridge buffer too small!\n",
> > +                               cxl_memdev_get_devname(memdev));
> > +               return 0;
> > +       }
> > +
> > +       return is_enabled(path);
> > +}
> > +
> >  CXL_EXPORT void cxl_cmd_unref(struct cxl_cmd *cmd)
> >  {
> >         if (!cmd)
> > diff --git a/cxl/libcxl.h b/cxl/libcxl.h
> > index d3b97a1..535e349 100644
> > --- a/cxl/libcxl.h
> > +++ b/cxl/libcxl.h
> > @@ -43,6 +43,7 @@ unsigned long long cxl_memdev_get_pmem_size(struct cxl_memdev *memdev);
> >  unsigned long long cxl_memdev_get_ram_size(struct cxl_memdev *memdev);
> >  const char *cxl_memdev_get_firmware_verison(struct cxl_memdev *memdev);
> >  size_t cxl_memdev_get_label_size(struct cxl_memdev *memdev);
> > +int cxl_memdev_nvdimm_bridge_active(struct cxl_memdev *memdev);
> > 
> >  #define cxl_memdev_foreach(ctx, memdev) \
> >          for (memdev = cxl_memdev_get_first(ctx); \
> > diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
> > index 858e953..f3b0c63 100644
> > --- a/cxl/lib/libcxl.sym
> > +++ b/cxl/lib/libcxl.sym
> > @@ -65,6 +65,7 @@ global:
> >         cxl_cmd_new_read_label;
> >         cxl_cmd_read_label_get_payload;
> >         cxl_memdev_get_label_size;
> > +       cxl_memdev_nvdimm_bridge_active;
> >  local:
> >          *;
> >  };
> > --
> > 2.31.1
> >
diff mbox series

Patch

diff --git a/cxl/lib/private.h b/cxl/lib/private.h
index c4ed741..2f0b6ea 100644
--- a/cxl/lib/private.h
+++ b/cxl/lib/private.h
@@ -10,6 +10,14 @@ 
 
 #define CXL_EXPORT __attribute__ ((visibility("default")))
 
+
+struct cxl_nvdimm_br {
+	int id;
+	void *dev_buf;
+	size_t buf_len;
+	char *dev_path;
+};
+
 struct cxl_memdev {
 	int id, major, minor;
 	void *dev_buf;
@@ -23,6 +31,7 @@  struct cxl_memdev {
 	int payload_max;
 	size_t lsa_size;
 	struct kmod_module *module;
+	struct cxl_nvdimm_br *bridge;
 };
 
 enum cxl_cmd_query_status {
diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
index def3a97..7bc0696 100644
--- a/cxl/lib/libcxl.c
+++ b/cxl/lib/libcxl.c
@@ -45,11 +45,19 @@  struct cxl_ctx {
 	void *private_data;
 };
 
+static void free_bridge(struct cxl_nvdimm_br *bridge)
+{
+	free(bridge->dev_buf);
+	free(bridge->dev_path);
+	free(bridge);
+}
+
 static void free_memdev(struct cxl_memdev *memdev, struct list_head *head)
 {
 	if (head)
 		list_del_from(head, &memdev->list);
 	kmod_module_unref(memdev->module);
+	free_bridge(memdev->bridge);
 	free(memdev->firmware_version);
 	free(memdev->dev_buf);
 	free(memdev->dev_path);
@@ -205,6 +213,40 @@  CXL_EXPORT void cxl_set_log_priority(struct cxl_ctx *ctx, int priority)
 	ctx->ctx.log_priority = priority;
 }
 
+static void *add_cxl_bridge(void *parent, int id, const char *br_base)
+{
+	const char *devname = devpath_to_devname(br_base);
+	struct cxl_memdev *memdev = parent;
+	struct cxl_ctx *ctx = memdev->ctx;
+	struct cxl_nvdimm_br *bridge;
+
+	dbg(ctx, "%s: bridge_base: \'%s\'\n", devname, br_base);
+
+	bridge = calloc(1, sizeof(*bridge));
+	if (!bridge)
+		goto err_dev;
+	bridge->id = id;
+
+	bridge->dev_path = strdup(br_base);
+	if (!bridge->dev_path)
+		goto err_read;
+
+	bridge->dev_buf = calloc(1, strlen(br_base) + 50);
+	if (!bridge->dev_buf)
+		goto err_read;
+	bridge->buf_len = strlen(br_base) + 50;
+
+	memdev->bridge = bridge;
+	return bridge;
+
+ err_read:
+	free(bridge->dev_buf);
+	free(bridge->dev_path);
+	free(bridge);
+ err_dev:
+	return NULL;
+}
+
 static void *add_cxl_memdev(void *parent, int id, const char *cxlmem_base)
 {
 	const char *devname = devpath_to_devname(cxlmem_base);
@@ -271,6 +313,8 @@  static void *add_cxl_memdev(void *parent, int id, const char *cxlmem_base)
 		goto err_read;
 	memdev->buf_len = strlen(cxlmem_base) + 50;
 
+	sysfs_device_parse(ctx, cxlmem_base, "pmem", memdev, add_cxl_bridge);
+
 	cxl_memdev_foreach(ctx, memdev_dup)
 		if (memdev_dup->id == memdev->id) {
 			free_memdev(memdev, NULL);
@@ -362,6 +406,35 @@  CXL_EXPORT size_t cxl_memdev_get_label_size(struct cxl_memdev *memdev)
 	return memdev->lsa_size;
 }
 
+static int is_enabled(const char *drvpath)
+{
+	struct stat st;
+
+	if (lstat(drvpath, &st) < 0 || !S_ISLNK(st.st_mode))
+		return 0;
+	else
+		return 1;
+}
+
+CXL_EXPORT int cxl_memdev_nvdimm_bridge_active(struct cxl_memdev *memdev)
+{
+	struct cxl_ctx *ctx = cxl_memdev_get_ctx(memdev);
+	struct cxl_nvdimm_br *bridge = memdev->bridge;
+	char *path = memdev->dev_buf;
+	int len = memdev->buf_len;
+
+	if (!bridge)
+		return 0;
+
+	if (snprintf(path, len, "%s/driver", bridge->dev_path) >= len) {
+		err(ctx, "%s: nvdimm bridge buffer too small!\n",
+				cxl_memdev_get_devname(memdev));
+		return 0;
+	}
+
+	return is_enabled(path);
+}
+
 CXL_EXPORT void cxl_cmd_unref(struct cxl_cmd *cmd)
 {
 	if (!cmd)
diff --git a/cxl/libcxl.h b/cxl/libcxl.h
index d3b97a1..535e349 100644
--- a/cxl/libcxl.h
+++ b/cxl/libcxl.h
@@ -43,6 +43,7 @@  unsigned long long cxl_memdev_get_pmem_size(struct cxl_memdev *memdev);
 unsigned long long cxl_memdev_get_ram_size(struct cxl_memdev *memdev);
 const char *cxl_memdev_get_firmware_verison(struct cxl_memdev *memdev);
 size_t cxl_memdev_get_label_size(struct cxl_memdev *memdev);
+int cxl_memdev_nvdimm_bridge_active(struct cxl_memdev *memdev);
 
 #define cxl_memdev_foreach(ctx, memdev) \
         for (memdev = cxl_memdev_get_first(ctx); \
diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
index 858e953..f3b0c63 100644
--- a/cxl/lib/libcxl.sym
+++ b/cxl/lib/libcxl.sym
@@ -65,6 +65,7 @@  global:
 	cxl_cmd_new_read_label;
 	cxl_cmd_read_label_get_payload;
 	cxl_memdev_get_label_size;
+	cxl_memdev_nvdimm_bridge_active;
 local:
         *;
 };