diff mbox series

[ndctl,5/7] libcxl: add interfaces for SET_PARTITION_INFO mailbox command

Message ID fa45e95e5d28981b4ec41db65aab82c103bff0c3.1641233076.git.alison.schofield@intel.com
State Superseded
Headers show
Series Add partitioning support for CXL memdevs | expand

Commit Message

Alison Schofield Jan. 3, 2022, 8:16 p.m. UTC 
From: Alison Schofield <alison.schofield@intel.com>

Add APIs to allocate and send a SET_PARTITION_INFO mailbox command.

Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 cxl/lib/private.h  |  9 +++++++++
 cxl/libcxl.h       |  4 ++++
 cxl/lib/libcxl.c   | 45 +++++++++++++++++++++++++++++++++++++++++++++
 cxl/lib/libcxl.sym |  3 +++
 4 files changed, 61 insertions(+)

Comments

Ira Weiny Jan. 6, 2022, 8:53 p.m. UTC | #1 
On Mon, Jan 03, 2022 at 12:16:16PM -0800, Schofield, Alison wrote:
> From: Alison Schofield <alison.schofield@intel.com>
> 
> Add APIs to allocate and send a SET_PARTITION_INFO mailbox command.
> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
>  cxl/lib/private.h  |  9 +++++++++
>  cxl/libcxl.h       |  4 ++++
>  cxl/lib/libcxl.c   | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  cxl/lib/libcxl.sym |  3 +++
>  4 files changed, 61 insertions(+)
> 
> diff --git a/cxl/lib/private.h b/cxl/lib/private.h
> index dd9234f..841aa80 100644
> --- a/cxl/lib/private.h
> +++ b/cxl/lib/private.h
> @@ -114,6 +114,15 @@ struct cxl_cmd_get_partition_info {
>  
>  #define CXL_CAPACITY_MULTIPLIER		SZ_256M
>  
> +struct cxl_cmd_set_partition_info {
> +	le64 volatile_capacity;
> +	u8 flags;
> +} __attribute__((packed));
> +
> +/* CXL 2.0 8.2.9.5.2 Set Partition Info */
> +#define CXL_CMD_SET_PARTITION_INFO_NO_FLAG				(0)
> +#define CXL_CMD_SET_PARTITION_INFO_IMMEDIATE_FLAG			(1)

BIT(0) and BIT(1)?

I can't remember which bit is the immediate flag.

> +
>  /* CXL 2.0 8.2.9.5.3 Byte 0 Health Status */
>  #define CXL_CMD_HEALTH_INFO_STATUS_MAINTENANCE_NEEDED_MASK		BIT(0)
>  #define CXL_CMD_HEALTH_INFO_STATUS_PERFORMANCE_DEGRADED_MASK		BIT(1)
> diff --git a/cxl/libcxl.h b/cxl/libcxl.h
> index d333b6d..67d6ffc 100644
> --- a/cxl/libcxl.h
> +++ b/cxl/libcxl.h
> @@ -50,6 +50,8 @@ 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);
> +int cxl_memdev_set_partition_info(struct cxl_memdev *memdev,
> +		unsigned long long volatile_capacity, int flags);
>  
>  #define cxl_memdev_foreach(ctx, memdev) \
>          for (memdev = cxl_memdev_get_first(ctx); \
> @@ -117,6 +119,8 @@ unsigned long long cxl_cmd_get_partition_info_get_active_volatile_cap(struct cxl
>  unsigned long long cxl_cmd_get_partition_info_get_active_persistent_cap(struct cxl_cmd *cmd);
>  unsigned long long cxl_cmd_get_partition_info_get_next_volatile_cap(struct cxl_cmd *cmd);
>  unsigned long long cxl_cmd_get_partition_info_get_next_persistent_cap(struct cxl_cmd *cmd);
> +struct cxl_cmd *cxl_cmd_new_set_partition_info(struct cxl_memdev *memdev,
> +		unsigned long long volatile_capacity, int flags);
>  
>  #ifdef __cplusplus
>  } /* extern "C" */
> diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
> index 85a6c0e..877a783 100644
> --- a/cxl/lib/libcxl.c
> +++ b/cxl/lib/libcxl.c
> @@ -1227,6 +1227,21 @@ cxl_cmd_get_partition_info_get_next_persistent_cap(struct cxl_cmd *cmd)
>  	cmd_partition_get_capacity_field(cmd, next_persistent_cap);
>  }
>  
> +CXL_EXPORT struct cxl_cmd *cxl_cmd_new_set_partition_info(struct cxl_memdev *memdev,
> +		unsigned long long volatile_capacity, int flags)
> +{
> +	struct cxl_cmd_set_partition_info *set_partition;
> +	struct cxl_cmd *cmd;
> +
> +	cmd = cxl_cmd_new_generic(memdev,
> +			CXL_MEM_COMMAND_ID_SET_PARTITION_INFO);
> +
> +	set_partition = (struct cxl_cmd_set_partition_info *)cmd->send_cmd->in.payload;
> +	set_partition->volatile_capacity = cpu_to_le64(volatile_capacity);
> +	set_partition->flags = flags;
> +	return cmd;
> +}
> +
>  CXL_EXPORT int cxl_cmd_submit(struct cxl_cmd *cmd)
>  {
>  	struct cxl_memdev *memdev = cmd->memdev;
> @@ -1425,3 +1440,33 @@ CXL_EXPORT int cxl_memdev_read_label(struct cxl_memdev *memdev, void *buf,
>  {
>  	return lsa_op(memdev, LSA_OP_GET, buf, length, offset);
>  }
> +
> +CXL_EXPORT int cxl_memdev_set_partition_info(struct cxl_memdev *memdev,
> +	       unsigned long long volatile_capacity, int flags)
> +{
> +	struct cxl_ctx *ctx = cxl_memdev_get_ctx(memdev);
> +	struct cxl_cmd *cmd;
> +	int rc;
> +
> +	dbg(ctx, "%s: enter cap: %llx, flags %d\n", __func__,
> +		volatile_capacity, flags);
> +
> +	cmd = cxl_cmd_new_set_partition_info(memdev,
> +			volatile_capacity / CXL_CAPACITY_MULTIPLIER, flags);

I'll reiterate that I agree with this capacity being in bytes.  But I'm not
sure what the rest of libcxl does.

Ira

> +	if (!cmd)
> +		return -ENXIO;
> +
> +	rc = cxl_cmd_submit(cmd);
> +	if (rc < 0) {
> +		err(ctx, "cmd submission failed: %s\n", strerror(-rc));
> +		goto err;
> +	}
> +	rc = cxl_cmd_get_mbox_status(cmd);
> +	if (rc != 0) {
> +		err(ctx, "%s: mbox status: %d\n", __func__, rc);
> +		rc = -ENXIO;
> +	}
> +err:
> +	cxl_cmd_unref(cmd);
> +	return rc;
> +}
> diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
> index bed6427..5d02c45 100644
> --- a/cxl/lib/libcxl.sym
> +++ b/cxl/lib/libcxl.sym
> @@ -78,6 +78,9 @@ global:
>  	cxl_cmd_get_partition_info_get_active_persistent_cap;
>  	cxl_cmd_get_partition_info_get_next_volatile_cap;
>  	cxl_cmd_get_partition_info_get_next_persistent_cap;
> +	cxl_cmd_new_set_partition_info;
> +	cxl_memdev_set_partition_info;
> +
>  local:
>          *;
>  };
> -- 
> 2.31.1
>
Alison Schofield Jan. 8, 2022, 1:51 a.m. UTC | #2 
On Thu, Jan 06, 2022 at 12:53:02PM -0800, Ira Weiny wrote:
> On Mon, Jan 03, 2022 at 12:16:16PM -0800, Schofield, Alison wrote:
> > From: Alison Schofield <alison.schofield@intel.com>
> > 
> > Add APIs to allocate and send a SET_PARTITION_INFO mailbox command.
> > 
> > +	le64 volatile_capacity;
> > +	u8 flags;
> > +} __attribute__((packed));
> > +
> > +/* CXL 2.0 8.2.9.5.2 Set Partition Info */
> > +#define CXL_CMD_SET_PARTITION_INFO_NO_FLAG				(0)
> > +#define CXL_CMD_SET_PARTITION_INFO_IMMEDIATE_FLAG			(1)
> 
> BIT(0) and BIT(1)?
> 
> I can't remember which bit is the immediate flag.
> 
Immediate flag is BIT(0).
Seemed awkward/overkill to use bit macro -
+#define CXL_CMD_SET_PARTITION_INFO_NO_FLAG				(0)
+#define CXL_CMD_SET_PARTITION_INFO_IMMEDIATE_FLAG			BIT(1)

I just added api to use this so you'll see it in action in v2
of this patchset and can comment again.
Dan Williams Jan. 8, 2022, 2:27 a.m. UTC | #3 
On Fri, Jan 7, 2022 at 5:46 PM Alison Schofield
<alison.schofield@intel.com> wrote:
>
> On Thu, Jan 06, 2022 at 12:53:02PM -0800, Ira Weiny wrote:
> > On Mon, Jan 03, 2022 at 12:16:16PM -0800, Schofield, Alison wrote:
> > > From: Alison Schofield <alison.schofield@intel.com>
> > >
> > > Add APIs to allocate and send a SET_PARTITION_INFO mailbox command.
> > >
> > > +   le64 volatile_capacity;
> > > +   u8 flags;
> > > +} __attribute__((packed));
> > > +
> > > +/* CXL 2.0 8.2.9.5.2 Set Partition Info */
> > > +#define CXL_CMD_SET_PARTITION_INFO_NO_FLAG                         (0)
> > > +#define CXL_CMD_SET_PARTITION_INFO_IMMEDIATE_FLAG                  (1)
> >
> > BIT(0) and BIT(1)?
> >
> > I can't remember which bit is the immediate flag.
> >
> Immediate flag is BIT(0).
> Seemed awkward/overkill to use bit macro -
> +#define CXL_CMD_SET_PARTITION_INFO_NO_FLAG                             (0)
> +#define CXL_CMD_SET_PARTITION_INFO_IMMEDIATE_FLAG                      BIT(1)
>
> I just added api to use this so you'll see it in action in v2
> of this patchset and can comment again.

Why is a "no flag" definition needed? Isn't that just "!IMMEDIATE"

Also BIT(1) == 0x2, so that should be BIT(0), right?
Alison Schofield Jan. 10, 2022, 2:13 a.m. UTC | #4 
On Fri, Jan 07, 2022 at 06:27:40PM -0800, Dan Williams wrote:
> On Fri, Jan 7, 2022 at 5:46 PM Alison Schofield
> <alison.schofield@intel.com> wrote:
> >
> > On Thu, Jan 06, 2022 at 12:53:02PM -0800, Ira Weiny wrote:
> > > On Mon, Jan 03, 2022 at 12:16:16PM -0800, Schofield, Alison wrote:
> > > > From: Alison Schofield <alison.schofield@intel.com>
> > > >
> > > > Add APIs to allocate and send a SET_PARTITION_INFO mailbox command.
> > > >
> > > > +   le64 volatile_capacity;
> > > > +   u8 flags;
> > > > +} __attribute__((packed));
> > > > +
> > > > +/* CXL 2.0 8.2.9.5.2 Set Partition Info */
> > > > +#define CXL_CMD_SET_PARTITION_INFO_NO_FLAG                         (0)
> > > > +#define CXL_CMD_SET_PARTITION_INFO_IMMEDIATE_FLAG                  (1)
> > >
> > > BIT(0) and BIT(1)?
> > >
> > > I can't remember which bit is the immediate flag.
> > >
> > Immediate flag is BIT(0).
> > Seemed awkward/overkill to use bit macro -
> > +#define CXL_CMD_SET_PARTITION_INFO_NO_FLAG                             (0)
> > +#define CXL_CMD_SET_PARTITION_INFO_IMMEDIATE_FLAG                      BIT(1)
> >
> > I just added api to use this so you'll see it in action in v2
> > of this patchset and can comment again.
> 
> Why is a "no flag" definition needed? Isn't that just "!IMMEDIATE"
>
You are right. The no flag set case is !IMMEDIATE.

>Also BIT(1) == 0x2, so that should be BIT(0), right?
Yes IMMEDIATE FLAG IS BIT(0). 

This chatter is related to using something more descriptive that '0'
for !IMMEDIATE when the cxl command makes the call to the api. (Patch 7)
I added a new accessor in v2 that returns the bit.
diff mbox series

Patch

diff --git a/cxl/lib/private.h b/cxl/lib/private.h
index dd9234f..841aa80 100644
--- a/cxl/lib/private.h
+++ b/cxl/lib/private.h
@@ -114,6 +114,15 @@  struct cxl_cmd_get_partition_info {
 
 #define CXL_CAPACITY_MULTIPLIER		SZ_256M
 
+struct cxl_cmd_set_partition_info {
+	le64 volatile_capacity;
+	u8 flags;
+} __attribute__((packed));
+
+/* CXL 2.0 8.2.9.5.2 Set Partition Info */
+#define CXL_CMD_SET_PARTITION_INFO_NO_FLAG				(0)
+#define CXL_CMD_SET_PARTITION_INFO_IMMEDIATE_FLAG			(1)
+
 /* CXL 2.0 8.2.9.5.3 Byte 0 Health Status */
 #define CXL_CMD_HEALTH_INFO_STATUS_MAINTENANCE_NEEDED_MASK		BIT(0)
 #define CXL_CMD_HEALTH_INFO_STATUS_PERFORMANCE_DEGRADED_MASK		BIT(1)
diff --git a/cxl/libcxl.h b/cxl/libcxl.h
index d333b6d..67d6ffc 100644
--- a/cxl/libcxl.h
+++ b/cxl/libcxl.h
@@ -50,6 +50,8 @@  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);
+int cxl_memdev_set_partition_info(struct cxl_memdev *memdev,
+		unsigned long long volatile_capacity, int flags);
 
 #define cxl_memdev_foreach(ctx, memdev) \
         for (memdev = cxl_memdev_get_first(ctx); \
@@ -117,6 +119,8 @@  unsigned long long cxl_cmd_get_partition_info_get_active_volatile_cap(struct cxl
 unsigned long long cxl_cmd_get_partition_info_get_active_persistent_cap(struct cxl_cmd *cmd);
 unsigned long long cxl_cmd_get_partition_info_get_next_volatile_cap(struct cxl_cmd *cmd);
 unsigned long long cxl_cmd_get_partition_info_get_next_persistent_cap(struct cxl_cmd *cmd);
+struct cxl_cmd *cxl_cmd_new_set_partition_info(struct cxl_memdev *memdev,
+		unsigned long long volatile_capacity, int flags);
 
 #ifdef __cplusplus
 } /* extern "C" */
diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
index 85a6c0e..877a783 100644
--- a/cxl/lib/libcxl.c
+++ b/cxl/lib/libcxl.c
@@ -1227,6 +1227,21 @@  cxl_cmd_get_partition_info_get_next_persistent_cap(struct cxl_cmd *cmd)
 	cmd_partition_get_capacity_field(cmd, next_persistent_cap);
 }
 
+CXL_EXPORT struct cxl_cmd *cxl_cmd_new_set_partition_info(struct cxl_memdev *memdev,
+		unsigned long long volatile_capacity, int flags)
+{
+	struct cxl_cmd_set_partition_info *set_partition;
+	struct cxl_cmd *cmd;
+
+	cmd = cxl_cmd_new_generic(memdev,
+			CXL_MEM_COMMAND_ID_SET_PARTITION_INFO);
+
+	set_partition = (struct cxl_cmd_set_partition_info *)cmd->send_cmd->in.payload;
+	set_partition->volatile_capacity = cpu_to_le64(volatile_capacity);
+	set_partition->flags = flags;
+	return cmd;
+}
+
 CXL_EXPORT int cxl_cmd_submit(struct cxl_cmd *cmd)
 {
 	struct cxl_memdev *memdev = cmd->memdev;
@@ -1425,3 +1440,33 @@  CXL_EXPORT int cxl_memdev_read_label(struct cxl_memdev *memdev, void *buf,
 {
 	return lsa_op(memdev, LSA_OP_GET, buf, length, offset);
 }
+
+CXL_EXPORT int cxl_memdev_set_partition_info(struct cxl_memdev *memdev,
+	       unsigned long long volatile_capacity, int flags)
+{
+	struct cxl_ctx *ctx = cxl_memdev_get_ctx(memdev);
+	struct cxl_cmd *cmd;
+	int rc;
+
+	dbg(ctx, "%s: enter cap: %llx, flags %d\n", __func__,
+		volatile_capacity, flags);
+
+	cmd = cxl_cmd_new_set_partition_info(memdev,
+			volatile_capacity / CXL_CAPACITY_MULTIPLIER, flags);
+	if (!cmd)
+		return -ENXIO;
+
+	rc = cxl_cmd_submit(cmd);
+	if (rc < 0) {
+		err(ctx, "cmd submission failed: %s\n", strerror(-rc));
+		goto err;
+	}
+	rc = cxl_cmd_get_mbox_status(cmd);
+	if (rc != 0) {
+		err(ctx, "%s: mbox status: %d\n", __func__, rc);
+		rc = -ENXIO;
+	}
+err:
+	cxl_cmd_unref(cmd);
+	return rc;
+}
diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
index bed6427..5d02c45 100644
--- a/cxl/lib/libcxl.sym
+++ b/cxl/lib/libcxl.sym
@@ -78,6 +78,9 @@  global:
 	cxl_cmd_get_partition_info_get_active_persistent_cap;
 	cxl_cmd_get_partition_info_get_next_volatile_cap;
 	cxl_cmd_get_partition_info_get_next_persistent_cap;
+	cxl_cmd_new_set_partition_info;
+	cxl_memdev_set_partition_info;
+
 local:
         *;
 };