diff mbox series

[ndctl,v4,12/17] libcxl: add interfaces for label operations

Message ID 20211007082139.3088615-13-vishal.l.verma@intel.com (mailing list archive)
State Superseded
Headers show
Series Initial CXL support | expand

Commit Message

Verma, Vishal L Oct. 7, 2021, 8:21 a.m. UTC
Add libcxl interfaces to allow performinfg label (LSA) manipulations.
Add a 'cxl_cmd_new_set_lsa' interface to create a 'Set LSA' mailbox
command payload, and interfaces to read, write, and zero the LSA area on
a memdev.

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

Comments

Dan Williams Oct. 14, 2021, 9:27 p.m. UTC | #1
On Thu, Oct 7, 2021 at 1:22 AM Vishal Verma <vishal.l.verma@intel.com> wrote:
>
> Add libcxl interfaces to allow performinfg label (LSA) manipulations.
> Add a 'cxl_cmd_new_set_lsa' interface to create a 'Set LSA' mailbox
> command payload, and interfaces to read, write, and zero the LSA area on
> a memdev.
>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> ---
>  cxl/lib/private.h  |   6 ++
>  cxl/lib/libcxl.c   | 137 +++++++++++++++++++++++++++++++++++++++++++++
>  cxl/libcxl.h       |   7 +++
>  cxl/lib/libcxl.sym |   4 ++
>  4 files changed, 154 insertions(+)
>
> diff --git a/cxl/lib/private.h b/cxl/lib/private.h
> index 671f12f..89212df 100644
> --- a/cxl/lib/private.h
> +++ b/cxl/lib/private.h
> @@ -79,6 +79,12 @@ struct cxl_cmd_get_lsa_in {
>         le32 length;
>  } __attribute__((packed));
>
> +struct cxl_cmd_set_lsa {
> +       le32 offset;
> +       le32 rsvd;
> +       unsigned char lsa_data[0];
> +} __attribute__ ((packed));
> +
>  struct cxl_cmd_get_health_info {
>         u8 health_status;
>         u8 media_status;
> diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
> index 59d091c..8dd69cf 100644
> --- a/cxl/lib/libcxl.c
> +++ b/cxl/lib/libcxl.c
> @@ -1126,3 +1126,140 @@ CXL_EXPORT int cxl_cmd_get_out_size(struct cxl_cmd *cmd)
>  {
>         return cmd->send_cmd->out.size;
>  }
> +
> +CXL_EXPORT struct cxl_cmd *cxl_cmd_new_write_label(struct cxl_memdev *memdev,
> +               void *lsa_buf, unsigned int offset, unsigned int length)
> +{
> +       struct cxl_ctx *ctx = cxl_memdev_get_ctx(memdev);
> +       struct cxl_cmd_set_lsa *set_lsa;
> +       struct cxl_cmd *cmd;
> +       int rc;
> +
> +       cmd = cxl_cmd_new_generic(memdev, CXL_MEM_COMMAND_ID_SET_LSA);
> +       if (!cmd)
> +               return NULL;
> +
> +       /* this will allocate 'in.payload' */
> +       rc = cxl_cmd_set_input_payload(cmd, NULL, sizeof(*set_lsa) + length);
> +       if (rc) {
> +               err(ctx, "%s: cmd setup failed: %s\n",
> +                       cxl_memdev_get_devname(memdev), strerror(-rc));
> +               goto out_fail;
> +       }
> +       set_lsa = (void *)cmd->send_cmd->in.payload;

...the cast is still nagging at me especially when this knows what the
payload is supposed to be. What about a helper per command type of the
form:

struct cxl_cmd_$name *to_cxl_cmd_$name(struct cxl_cmd *cmd)
{
    if (cmd->send_cmd->id != CXL_MEM_COMMAND_ID_$NAME)
        return NULL;
    return (struct cxl_cmd_$name *) cmd->send_cmd->in.payload;
}

> +       set_lsa->offset = cpu_to_le32(offset);
> +       memcpy(set_lsa->lsa_data, lsa_buf, length);
> +
> +       return cmd;
> +
> +out_fail:
> +       cxl_cmd_unref(cmd);
> +       return NULL;
> +}
> +
> +enum lsa_op {
> +       LSA_OP_GET,
> +       LSA_OP_SET,
> +       LSA_OP_ZERO,
> +};
> +
> +static int lsa_op(struct cxl_memdev *memdev, int op, void **buf,
> +               size_t length, size_t offset)
> +{
> +       const char *devname = cxl_memdev_get_devname(memdev);
> +       struct cxl_ctx *ctx = cxl_memdev_get_ctx(memdev);
> +       struct cxl_cmd *cmd;
> +       void *zero_buf = NULL;
> +       ssize_t ret_len;
> +       int rc = 0;
> +
> +       if (op != LSA_OP_ZERO && (buf == NULL || *buf == NULL)) {
> +               err(ctx, "%s: LSA buffer cannot be NULL\n", devname);
> +               return -EINVAL;
> +       }
> +
> +       /* TODO: handle the case for offset + len > mailbox payload size */
> +       switch (op) {
> +       case LSA_OP_GET:
> +               if (length == 0)
> +                       length = memdev->lsa_size;

What's the use case to support a default size for get? If someone
wants to do a zero length lsa_op shouldn't that just return immediate
success just like 0 length read(2)?

> +               cmd = cxl_cmd_new_read_label(memdev, offset, length);
> +               if (!cmd)
> +                       return -ENOMEM;
> +               rc = cxl_cmd_set_output_payload(cmd, *buf, length);
> +               if (rc) {
> +                       err(ctx, "%s: cmd setup failed: %s\n",
> +                           cxl_memdev_get_devname(memdev), strerror(-rc));
> +                       goto out;
> +               }
> +               break;
> +       case LSA_OP_ZERO:
> +               if (length == 0)

This one makes sense because the caller just wants the whole area zeroed.

> +                       length = memdev->lsa_size;
> +               zero_buf = calloc(1, length);
> +               if (!zero_buf)
> +                       return -ENOMEM;
> +               buf = &zero_buf;


> +               /* fall through */
> +       case LSA_OP_SET:

...and if length == 0 here there's no need to go any further, we're done.

> +               cmd = cxl_cmd_new_write_label(memdev, *buf, offset, length);
> +               if (!cmd) {
> +                       rc = -ENOMEM;
> +                       goto out_free;
> +               }
> +               break;
> +       default:
> +               return -EOPNOTSUPP;
> +       }
> +
> +       rc = cxl_cmd_submit(cmd);
> +       if (rc < 0) {
> +               err(ctx, "%s: cmd submission failed: %s\n",
> +                       devname, strerror(-rc));
> +               goto out;
> +       }
> +
> +       rc = cxl_cmd_get_mbox_status(cmd);
> +       if (rc != 0) {
> +               err(ctx, "%s: firmware status: %d\n",
> +                       devname, rc);
> +               rc = -ENXIO;
> +               goto out;
> +       }
> +
> +       if (op == LSA_OP_GET) {
> +               ret_len = cxl_cmd_read_label_get_payload(cmd, *buf, length);
> +               if (ret_len < 0) {
> +                       rc = ret_len;
> +                       goto out;
> +               }
> +       }
> +
> +       /*
> +        * TODO: If writing, the memdev may need to be disabled/re-enabled to
> +        * refresh any cached LSA data in the kernel.
> +        */

I think we're sufficiently protected by the nvdimm-bridge up/down
dependency. I.e. if the above commands actually worked then nothing
should have had the labels cached in the kernel.

After fixing the the length == 0 case you can add:

Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Verma, Vishal L Oct. 14, 2021, 10:18 p.m. UTC | #2
On Thu, 2021-10-14 at 14:27 -0700, Dan Williams wrote:
> On Thu, Oct 7, 2021 at 1:22 AM Vishal Verma <vishal.l.verma@intel.com> wrote:
> > 
> > Add libcxl interfaces to allow performinfg label (LSA) manipulations.
> > Add a 'cxl_cmd_new_set_lsa' interface to create a 'Set LSA' mailbox
> > command payload, and interfaces to read, write, and zero the LSA area on
> > a memdev.
> > 
> > Cc: Dan Williams <dan.j.williams@intel.com>
> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> > ---
> >  cxl/lib/private.h  |   6 ++
> >  cxl/lib/libcxl.c   | 137 +++++++++++++++++++++++++++++++++++++++++++++
> >  cxl/libcxl.h       |   7 +++
> >  cxl/lib/libcxl.sym |   4 ++
> >  4 files changed, 154 insertions(+)
> > 
> > diff --git a/cxl/lib/private.h b/cxl/lib/private.h
> > index 671f12f..89212df 100644
> > --- a/cxl/lib/private.h
> > +++ b/cxl/lib/private.h
> > @@ -79,6 +79,12 @@ struct cxl_cmd_get_lsa_in {
> >         le32 length;
> >  } __attribute__((packed));
> > 
> > +struct cxl_cmd_set_lsa {
> > +       le32 offset;
> > +       le32 rsvd;
> > +       unsigned char lsa_data[0];
> > +} __attribute__ ((packed));
> > +
> >  struct cxl_cmd_get_health_info {
> >         u8 health_status;
> >         u8 media_status;
> > diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
> > index 59d091c..8dd69cf 100644
> > --- a/cxl/lib/libcxl.c
> > +++ b/cxl/lib/libcxl.c
> > @@ -1126,3 +1126,140 @@ CXL_EXPORT int cxl_cmd_get_out_size(struct cxl_cmd *cmd)
> >  {
> >         return cmd->send_cmd->out.size;
> >  }
> > +
> > +CXL_EXPORT struct cxl_cmd *cxl_cmd_new_write_label(struct cxl_memdev *memdev,
> > +               void *lsa_buf, unsigned int offset, unsigned int length)
> > +{
> > +       struct cxl_ctx *ctx = cxl_memdev_get_ctx(memdev);
> > +       struct cxl_cmd_set_lsa *set_lsa;
> > +       struct cxl_cmd *cmd;
> > +       int rc;
> > +
> > +       cmd = cxl_cmd_new_generic(memdev, CXL_MEM_COMMAND_ID_SET_LSA);
> > +       if (!cmd)
> > +               return NULL;
> > +
> > +       /* this will allocate 'in.payload' */
> > +       rc = cxl_cmd_set_input_payload(cmd, NULL, sizeof(*set_lsa) + length);
> > +       if (rc) {
> > +               err(ctx, "%s: cmd setup failed: %s\n",
> > +                       cxl_memdev_get_devname(memdev), strerror(-rc));
> > +               goto out_fail;
> > +       }
> > +       set_lsa = (void *)cmd->send_cmd->in.payload;
> 
> ...the cast is still nagging at me especially when this knows what the
> payload is supposed to be. What about a helper per command type of the
> form:
> 
> struct cxl_cmd_$name *to_cxl_cmd_$name(struct cxl_cmd *cmd)
> {
>     if (cmd->send_cmd->id != CXL_MEM_COMMAND_ID_$NAME)
>         return NULL;
>     return (struct cxl_cmd_$name *) cmd->send_cmd->in.payload;
> }
> 
> > +       set_lsa->offset = cpu_to_le32(offset);
> > +       memcpy(set_lsa->lsa_data, lsa_buf, length);
> > +
> > +       return cmd;
> > +
> > +out_fail:
> > +       cxl_cmd_unref(cmd);
> > +       return NULL;
> > +}
> > +
> > +enum lsa_op {
> > +       LSA_OP_GET,
> > +       LSA_OP_SET,
> > +       LSA_OP_ZERO,
> > +};
> > +
> > +static int lsa_op(struct cxl_memdev *memdev, int op, void **buf,
> > +               size_t length, size_t offset)
> > +{
> > +       const char *devname = cxl_memdev_get_devname(memdev);
> > +       struct cxl_ctx *ctx = cxl_memdev_get_ctx(memdev);
> > +       struct cxl_cmd *cmd;
> > +       void *zero_buf = NULL;
> > +       ssize_t ret_len;
> > +       int rc = 0;
> > +
> > +       if (op != LSA_OP_ZERO && (buf == NULL || *buf == NULL)) {
> > +               err(ctx, "%s: LSA buffer cannot be NULL\n", devname);
> > +               return -EINVAL;
> > +       }
> > +
> > +       /* TODO: handle the case for offset + len > mailbox payload size */
> > +       switch (op) {
> > +       case LSA_OP_GET:
> > +               if (length == 0)
> > +                       length = memdev->lsa_size;
> 
> What's the use case to support a default size for get? If someone
> wants to do a zero length lsa_op shouldn't that just return immediate
> success just like 0 length read(2)?

I was going for a convenience shortcut where a user can get the whole
thing without needing to worry about what the memdev's lsa_size is, but
in hindsight, this doesn't make much sense. The API can take length to
be literal - and not do any work for length == 0, and the cli can have
the convenience function of implying a full label read if no length
option was supplied.

> 
> > +               cmd = cxl_cmd_new_read_label(memdev, offset, length);
> > +               if (!cmd)
> > +                       return -ENOMEM;
> > +               rc = cxl_cmd_set_output_payload(cmd, *buf, length);
> > +               if (rc) {
> > +                       err(ctx, "%s: cmd setup failed: %s\n",
> > +                           cxl_memdev_get_devname(memdev), strerror(-rc));
> > +                       goto out;
> > +               }
> > +               break;
> > +       case LSA_OP_ZERO:
> > +               if (length == 0)
> 
> This one makes sense because the caller just wants the whole area zeroed.

I think the API between all three of these should be consistent in how
'length' is treated. It would make sense to zero everything if the
length were removed from the API entirely, but if it is part of it,
then we should just do what read/write do.

> 
> > +                       length = memdev->lsa_size;
> > +               zero_buf = calloc(1, length);
> > +               if (!zero_buf)
> > +                       return -ENOMEM;
> > +               buf = &zero_buf;
> 
> 
> > +               /* fall through */
> > +       case LSA_OP_SET:
> 
> ...and if length == 0 here there's no need to go any further, we're done.

Yep will change.

> 
> > +               cmd = cxl_cmd_new_write_label(memdev, *buf, offset, length);
> > +               if (!cmd) {
> > +                       rc = -ENOMEM;
> > +                       goto out_free;
> > +               }
> > +               break;
> > +       default:
> > +               return -EOPNOTSUPP;
> > +       }
> > +
> > +       rc = cxl_cmd_submit(cmd);
> > +       if (rc < 0) {
> > +               err(ctx, "%s: cmd submission failed: %s\n",
> > +                       devname, strerror(-rc));
> > +               goto out;
> > +       }
> > +
> > +       rc = cxl_cmd_get_mbox_status(cmd);
> > +       if (rc != 0) {
> > +               err(ctx, "%s: firmware status: %d\n",
> > +                       devname, rc);
> > +               rc = -ENXIO;
> > +               goto out;
> > +       }
> > +
> > +       if (op == LSA_OP_GET) {
> > +               ret_len = cxl_cmd_read_label_get_payload(cmd, *buf, length);
> > +               if (ret_len < 0) {
> > +                       rc = ret_len;
> > +                       goto out;
> > +               }
> > +       }
> > +
> > +       /*
> > +        * TODO: If writing, the memdev may need to be disabled/re-enabled to
> > +        * refresh any cached LSA data in the kernel.
> > +        */
> 
> I think we're sufficiently protected by the nvdimm-bridge up/down
> dependency. I.e. if the above commands actually worked then nothing
> should have had the labels cached in the kernel.

Yep makes sense, I'll drop this.

> 
> After fixing the the length == 0 case you can add:
> 
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Verma, Vishal L Oct. 14, 2021, 10:24 p.m. UTC | #3
On Thu, 2021-10-14 at 14:27 -0700, Dan Williams wrote:
> On Thu, Oct 7, 2021 at 1:22 AM Vishal Verma <vishal.l.verma@intel.com> wrote:
> 

[..]

> > +
> > +CXL_EXPORT struct cxl_cmd *cxl_cmd_new_write_label(struct cxl_memdev *memdev,
> > +               void *lsa_buf, unsigned int offset, unsigned int length)
> > +{
> > +       struct cxl_ctx *ctx = cxl_memdev_get_ctx(memdev);
> > +       struct cxl_cmd_set_lsa *set_lsa;
> > +       struct cxl_cmd *cmd;
> > +       int rc;
> > +
> > +       cmd = cxl_cmd_new_generic(memdev, CXL_MEM_COMMAND_ID_SET_LSA);
> > +       if (!cmd)
> > +               return NULL;
> > +
> > +       /* this will allocate 'in.payload' */
> > +       rc = cxl_cmd_set_input_payload(cmd, NULL, sizeof(*set_lsa) + length);
> > +       if (rc) {
> > +               err(ctx, "%s: cmd setup failed: %s\n",
> > +                       cxl_memdev_get_devname(memdev), strerror(-rc));
> > +               goto out_fail;
> > +       }
> > +       set_lsa = (void *)cmd->send_cmd->in.payload;
> 
> ...the cast is still nagging at me especially when this knows what the
> payload is supposed to be. What about a helper per command type of the
> form:
> 
> struct cxl_cmd_$name *to_cxl_cmd_$name(struct cxl_cmd *cmd)
> {
>     if (cmd->send_cmd->id != CXL_MEM_COMMAND_ID_$NAME)
>         return NULL;
>     return (struct cxl_cmd_$name *) cmd->send_cmd->in.payload;
> }
> 
Is the nag just from using a void cast, or having to cast at all? I
think the void cast was just laziness - it should be cast to 
(struct cxl_cmd_$name *) instead of (void *).

Having a helper for to_cxl_cmd_$name() does look cleaner, but do we
need the validation step there? In all these cases, the cmd would've
been allocated just a few lines above, with cxl_cmd_new_generic(memdev,
CXL_MEM_COMMAND_ID_$NAME) - so it seems unnecessary?
>
Dan Williams Oct. 14, 2021, 10:45 p.m. UTC | #4
On Thu, Oct 14, 2021 at 3:25 PM Verma, Vishal L
<vishal.l.verma@intel.com> wrote:
>
> On Thu, 2021-10-14 at 14:27 -0700, Dan Williams wrote:
> > On Thu, Oct 7, 2021 at 1:22 AM Vishal Verma <vishal.l.verma@intel.com> wrote:
> >
>
> [..]
>
> > > +
> > > +CXL_EXPORT struct cxl_cmd *cxl_cmd_new_write_label(struct cxl_memdev *memdev,
> > > +               void *lsa_buf, unsigned int offset, unsigned int length)
> > > +{
> > > +       struct cxl_ctx *ctx = cxl_memdev_get_ctx(memdev);
> > > +       struct cxl_cmd_set_lsa *set_lsa;
> > > +       struct cxl_cmd *cmd;
> > > +       int rc;
> > > +
> > > +       cmd = cxl_cmd_new_generic(memdev, CXL_MEM_COMMAND_ID_SET_LSA);
> > > +       if (!cmd)
> > > +               return NULL;
> > > +
> > > +       /* this will allocate 'in.payload' */
> > > +       rc = cxl_cmd_set_input_payload(cmd, NULL, sizeof(*set_lsa) + length);
> > > +       if (rc) {
> > > +               err(ctx, "%s: cmd setup failed: %s\n",
> > > +                       cxl_memdev_get_devname(memdev), strerror(-rc));
> > > +               goto out_fail;
> > > +       }
> > > +       set_lsa = (void *)cmd->send_cmd->in.payload;
> >
> > ...the cast is still nagging at me especially when this knows what the
> > payload is supposed to be. What about a helper per command type of the
> > form:
> >
> > struct cxl_cmd_$name *to_cxl_cmd_$name(struct cxl_cmd *cmd)
> > {
> >     if (cmd->send_cmd->id != CXL_MEM_COMMAND_ID_$NAME)
> >         return NULL;
> >     return (struct cxl_cmd_$name *) cmd->send_cmd->in.payload;
> > }
> >
> Is the nag just from using a void cast, or having to cast at all? I
> think the void cast was just laziness - it should be cast to
> (struct cxl_cmd_$name *) instead of (void *).

I'd feel better about that to have one explicit cast, then an explicit
'void *' cast just to get default implicit cast behavior.

>
> Having a helper for to_cxl_cmd_$name() does look cleaner, but do we
> need the validation step there? In all these cases, the cmd would've
> been allocated just a few lines above, with cxl_cmd_new_generic(memdev,
> CXL_MEM_COMMAND_ID_$NAME) - so it seems unnecessary?

Yeah, if there's never any other users of it outside of the
'cxl_cmd_new_$name' then the validation and even the helper are
unnecessary if you do the strict type conversion.
diff mbox series

Patch

diff --git a/cxl/lib/private.h b/cxl/lib/private.h
index 671f12f..89212df 100644
--- a/cxl/lib/private.h
+++ b/cxl/lib/private.h
@@ -79,6 +79,12 @@  struct cxl_cmd_get_lsa_in {
 	le32 length;
 } __attribute__((packed));
 
+struct cxl_cmd_set_lsa {
+	le32 offset;
+	le32 rsvd;
+	unsigned char lsa_data[0];
+} __attribute__ ((packed));
+
 struct cxl_cmd_get_health_info {
 	u8 health_status;
 	u8 media_status;
diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
index 59d091c..8dd69cf 100644
--- a/cxl/lib/libcxl.c
+++ b/cxl/lib/libcxl.c
@@ -1126,3 +1126,140 @@  CXL_EXPORT int cxl_cmd_get_out_size(struct cxl_cmd *cmd)
 {
 	return cmd->send_cmd->out.size;
 }
+
+CXL_EXPORT struct cxl_cmd *cxl_cmd_new_write_label(struct cxl_memdev *memdev,
+		void *lsa_buf, unsigned int offset, unsigned int length)
+{
+	struct cxl_ctx *ctx = cxl_memdev_get_ctx(memdev);
+	struct cxl_cmd_set_lsa *set_lsa;
+	struct cxl_cmd *cmd;
+	int rc;
+
+	cmd = cxl_cmd_new_generic(memdev, CXL_MEM_COMMAND_ID_SET_LSA);
+	if (!cmd)
+		return NULL;
+
+	/* this will allocate 'in.payload' */
+	rc = cxl_cmd_set_input_payload(cmd, NULL, sizeof(*set_lsa) + length);
+	if (rc) {
+		err(ctx, "%s: cmd setup failed: %s\n",
+			cxl_memdev_get_devname(memdev), strerror(-rc));
+		goto out_fail;
+	}
+	set_lsa = (void *)cmd->send_cmd->in.payload;
+	set_lsa->offset = cpu_to_le32(offset);
+	memcpy(set_lsa->lsa_data, lsa_buf, length);
+
+	return cmd;
+
+out_fail:
+	cxl_cmd_unref(cmd);
+	return NULL;
+}
+
+enum lsa_op {
+	LSA_OP_GET,
+	LSA_OP_SET,
+	LSA_OP_ZERO,
+};
+
+static int lsa_op(struct cxl_memdev *memdev, int op, void **buf,
+		size_t length, size_t offset)
+{
+	const char *devname = cxl_memdev_get_devname(memdev);
+	struct cxl_ctx *ctx = cxl_memdev_get_ctx(memdev);
+	struct cxl_cmd *cmd;
+	void *zero_buf = NULL;
+	ssize_t ret_len;
+	int rc = 0;
+
+	if (op != LSA_OP_ZERO && (buf == NULL || *buf == NULL)) {
+		err(ctx, "%s: LSA buffer cannot be NULL\n", devname);
+		return -EINVAL;
+	}
+
+	/* TODO: handle the case for offset + len > mailbox payload size */
+	switch (op) {
+	case LSA_OP_GET:
+		if (length == 0)
+			length = memdev->lsa_size;
+		cmd = cxl_cmd_new_read_label(memdev, offset, length);
+		if (!cmd)
+			return -ENOMEM;
+		rc = cxl_cmd_set_output_payload(cmd, *buf, length);
+		if (rc) {
+			err(ctx, "%s: cmd setup failed: %s\n",
+			    cxl_memdev_get_devname(memdev), strerror(-rc));
+			goto out;
+		}
+		break;
+	case LSA_OP_ZERO:
+		if (length == 0)
+			length = memdev->lsa_size;
+		zero_buf = calloc(1, length);
+		if (!zero_buf)
+			return -ENOMEM;
+		buf = &zero_buf;
+		/* fall through */
+	case LSA_OP_SET:
+		cmd = cxl_cmd_new_write_label(memdev, *buf, offset, length);
+		if (!cmd) {
+			rc = -ENOMEM;
+			goto out_free;
+		}
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	rc = cxl_cmd_submit(cmd);
+	if (rc < 0) {
+		err(ctx, "%s: cmd submission failed: %s\n",
+			devname, strerror(-rc));
+		goto out;
+	}
+
+	rc = cxl_cmd_get_mbox_status(cmd);
+	if (rc != 0) {
+		err(ctx, "%s: firmware status: %d\n",
+			devname, rc);
+		rc = -ENXIO;
+		goto out;
+	}
+
+	if (op == LSA_OP_GET) {
+		ret_len = cxl_cmd_read_label_get_payload(cmd, *buf, length);
+		if (ret_len < 0) {
+			rc = ret_len;
+			goto out;
+		}
+	}
+
+	/*
+	 * TODO: If writing, the memdev may need to be disabled/re-enabled to
+	 * refresh any cached LSA data in the kernel.
+	 */
+
+out:
+	cxl_cmd_unref(cmd);
+out_free:
+	free(zero_buf);
+	return rc;
+}
+
+CXL_EXPORT int cxl_memdev_zero_label(struct cxl_memdev *memdev)
+{
+	return lsa_op(memdev, LSA_OP_ZERO, NULL, 0, 0);
+}
+
+CXL_EXPORT int cxl_memdev_write_label(struct cxl_memdev *memdev, void *buf,
+		size_t length, size_t offset)
+{
+	return lsa_op(memdev, LSA_OP_SET, &buf, length, offset);
+}
+
+CXL_EXPORT int cxl_memdev_read_label(struct cxl_memdev *memdev, void *buf,
+		size_t length, size_t offset)
+{
+	return lsa_op(memdev, LSA_OP_GET, &buf, length, offset);
+}
diff --git a/cxl/libcxl.h b/cxl/libcxl.h
index 2e24371..a832806 100644
--- a/cxl/libcxl.h
+++ b/cxl/libcxl.h
@@ -44,6 +44,11 @@  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_is_active(struct cxl_memdev *memdev);
+int cxl_memdev_zero_label(struct cxl_memdev *memdev);
+int cxl_memdev_read_label(struct cxl_memdev *memdev, void *buf, size_t length,
+		size_t offset);
+int cxl_memdev_write_label(struct cxl_memdev *memdev, void *buf, size_t length,
+		size_t offset);
 
 #define cxl_memdev_foreach(ctx, memdev) \
         for (memdev = cxl_memdev_get_first(ctx); \
@@ -101,6 +106,8 @@  struct cxl_cmd *cxl_cmd_new_read_label(struct cxl_memdev *memdev,
 		unsigned int offset, unsigned int length);
 ssize_t cxl_cmd_read_label_get_payload(struct cxl_cmd *cmd, void *buf,
 		unsigned int length);
+struct cxl_cmd *cxl_cmd_new_write_label(struct cxl_memdev *memdev,
+		void *buf, unsigned int offset, unsigned int length);
 
 #ifdef __cplusplus
 } /* extern "C" */
diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
index 0e82030..490d6da 100644
--- a/cxl/lib/libcxl.sym
+++ b/cxl/lib/libcxl.sym
@@ -80,4 +80,8 @@  LIBCXL_4 {
 global:
 	cxl_memdev_get_label_size;
 	cxl_memdev_is_active;
+	cxl_cmd_new_write_label;
+	cxl_memdev_zero_label;
+	cxl_memdev_write_label;
+	cxl_memdev_read_label;
 } LIBCXL_3;