Re: [PATCH v12 1/8] cxl/mbox: Flag support for Dynamic Capacity Devices (DCD)

On Fri, Jul 31, 2026 at 01:48:06AM -0700, Anisa Su wrote:
> From: Ira Weiny <[email protected]>
> 
> Per the CXL 4.0 specification software must check the Command Effects
> Log (CEL) for dynamic capacity command support.
> 
> Detect support for the DCD commands while reading the CEL, including:
> 
>         Get DC Config
>         Get DC Extent List
>         Add DC Response
>         Release DC
> 
> Based on an original patch by Navneet Singh.
> 
> Signed-off-by: Ira Weiny <[email protected]>
> Signed-off-by: Anisa Su <[email protected]>
> Tested-by: Wonjae Lee <[email protected]>
> Tested-by: Junhee Park <[email protected]>
> Tested-by: Heesoo Kim <[email protected]>
> 
> ---
> Changes:
> 1. mbox.c: leave mds->dcd_supported false so the hardware enablement
>    patches can be upstreamed ahead of the extent and DAX work; the
>    event handling patch re-enables it.
> ---
>  drivers/cxl/core/mbox.c | 44 +++++++++++++++++++++++++++++++++++++++++
>  drivers/cxl/cxlmem.h    | 20 +++++++++++++++++++
>  2 files changed, 64 insertions(+)
> 
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index 7c6c5b7450a5..4790524c32a7 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -165,6 +165,38 @@ static void cxl_set_security_cmd_enabled(struct cxl_security_state *security,
>  	}
>  }
>  
> +static bool cxl_is_dcd_command(u16 opcode)
> +{
> +#define CXL_MBOX_OP_DCD_CMDS 0x48
> +
> +	return (opcode >> 8) == CXL_MBOX_OP_DCD_CMDS;
> +}
> +
> +static void cxl_set_dcd_cmd_enabled(u16 opcode, unsigned long *cmd_mask)
> +{
> +	switch (opcode) {
> +	case CXL_MBOX_OP_GET_DC_CONFIG:
> +		set_bit(CXL_DCD_ENABLED_GET_CONFIG, cmd_mask);
> +		break;
> +	case CXL_MBOX_OP_GET_DC_EXTENT_LIST:
> +		set_bit(CXL_DCD_ENABLED_GET_EXTENT_LIST, cmd_mask);
> +		break;
> +	case CXL_MBOX_OP_ADD_DC_RESPONSE:
> +		set_bit(CXL_DCD_ENABLED_ADD_RESPONSE, cmd_mask);
> +		break;
> +	case CXL_MBOX_OP_RELEASE_DC:
> +		set_bit(CXL_DCD_ENABLED_RELEASE, cmd_mask);
> +		break;
> +	default:
> +		break;
> +	}
> +}
> +
> +static bool cxl_verify_dcd_cmds(unsigned long *cmds_seen)
> +{
> +	return bitmap_full(cmds_seen, CXL_DCD_ENABLED_MAX);
> +}
> +
>  static bool cxl_is_poison_command(u16 opcode)
>  {
>  #define CXL_MBOX_OP_POISON_CMDS 0x43
> @@ -757,6 +789,7 @@ static void cxl_walk_cel(struct cxl_memdev_state *mds, size_t size, u8 *cel)
>  	struct cxl_mailbox *cxl_mbox = &mds->cxlds.cxl_mbox;
>  	struct cxl_cel_entry *cel_entry;
>  	const int cel_entries = size / sizeof(*cel_entry);
> +	DECLARE_BITMAP(dcd_cmds, CXL_DCD_ENABLED_MAX) = {};
>  	struct device *dev = mds->cxlds.dev;
>  	int i, ro_cmds = 0, wr_cmds = 0;
>  
> @@ -785,11 +818,22 @@ static void cxl_walk_cel(struct cxl_memdev_state *mds, size_t size, u8 *cel)
>  			enabled++;
>  		}
>  
> +		if (cxl_is_dcd_command(opcode)) {
> +			cxl_set_dcd_cmd_enabled(opcode, dcd_cmds);
> +			enabled++;
> +		}
> +
>  		dev_dbg(dev, "Opcode 0x%04x %s\n", opcode,
>  			enabled ? "enabled" : "unsupported by driver");
>  	}
>  
>  	set_features_cap(cxl_mbox, ro_cmds, wr_cmds);
> +	/*
> +	 * Disabled until event handling implemented.
> +	 */
> +	if (cxl_verify_dcd_cmds(dcd_cmds))
> +		dev_dbg(dev, "Device supports DCD; capability disabled\n");
> +	mds->dcd_supported = false;
>  }

When I view how this lands in the entirety of cxl_walk_cel() it seems
to needlessly be different from the pattern set by poison and security
handling in the same function and structures. Could this follow that
more closely?  Doing so would remove this 'tail' work here of checking
and setting the boolean.

This appended diff touches patch 2 where it has the first caller, but
pasting all here for simplicity - you'll apply per patch if you take it.

The bitmap lives in mds structure and it is already where the bool is now.
The only piece of the poison and security pattern not worth copying
yet is the wrapper struct cxl_poison_state and struct cxl_security_state
exist because each grew a mutex and other per-class state, and DCD has
none of that in this series (yet).

diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
index b18ea02ed2e6..27cfe0ee4752 100644
--- a/drivers/cxl/core/mbox.c
+++ b/drivers/cxl/core/mbox.c
@@ -172,7 +172,7 @@ static bool cxl_is_dcd_command(u16 opcode)
        return (opcode >> 8) == CXL_MBOX_OP_DCD_CMDS;
 }

-static void cxl_set_dcd_cmd_enabled(u16 opcode, unsigned long *cmd_mask)
+static void cxl_set_dcd_cmd_enabled(unsigned long *cmd_mask, u16 opcode)
 {
        switch (opcode) {
        case CXL_MBOX_OP_GET_DC_CONFIG:
@@ -192,11 +192,6 @@ static void cxl_set_dcd_cmd_enabled(u16 opcode, unsigned long *cmd_mask)
        }
 }

-static bool cxl_verify_dcd_cmds(unsigned long *cmds_seen)
-{
-       return bitmap_full(cmds_seen, CXL_DCD_ENABLED_MAX);
-}
-
 static bool cxl_is_poison_command(u16 opcode)
 {
 #define CXL_MBOX_OP_POISON_CMDS 0x43
@@ -789,7 +784,6 @@ static void cxl_walk_cel(struct cxl_memdev_state *mds, size_t size, u8 *cel)
        struct cxl_mailbox *cxl_mbox = &mds->cxlds.cxl_mbox;
        struct cxl_cel_entry *cel_entry;
        const int cel_entries = size / sizeof(*cel_entry);
-       DECLARE_BITMAP(dcd_cmds, CXL_DCD_ENABLED_MAX) = {};
        struct device *dev = mds->cxlds.dev;
        int i, ro_cmds = 0, wr_cmds = 0;

@@ -819,7 +813,7 @@ static void cxl_walk_cel(struct cxl_memdev_state *mds, size_t size, u8 *cel)
                }

                if (cxl_is_dcd_command(opcode)) {
-                       cxl_set_dcd_cmd_enabled(opcode, dcd_cmds);
+                       cxl_set_dcd_cmd_enabled(mds->dcd_enabled_cmds, opcode);
                        enabled++;
                }

@@ -828,12 +822,6 @@ static void cxl_walk_cel(struct cxl_memdev_state *mds, size_t size, u8 *cel)
        }

        set_features_cap(cxl_mbox, ro_cmds, wr_cmds);
-       /*
-        * Disabled until event handling implemented.
-        */
-       if (cxl_verify_dcd_cmds(dcd_cmds))
-               dev_dbg(dev, "Device supports DCD; capability disabled\n");
-       mds->dcd_supported = false;
 }

 static struct cxl_mbox_get_supported_logs *cxl_get_gsl(struct cxl_memdev_state *mds)
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index 77f6417a1de7..bfa746c5a7c8 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -443,7 +443,7 @@ static inline struct cxl_dev_state *mbox_to_cxlds(struct cxl_mailbox *cxl_mbox)
  * @partition_align_bytes: alignment size for partition-able capacity
  * @active_volatile_bytes: sum of hard + soft volatile
  * @active_persistent_bytes: sum of hard + soft persistent
- * @dcd_supported: all DCD commands are supported
+ * @dcd_enabled_cmds: DCD commands the device enabled in the CEL
  * @event: event log driver state
  * @poison: poison driver state info
  * @security: security driver state info
@@ -463,7 +463,7 @@ struct cxl_memdev_state {
        u64 partition_align_bytes;
        u64 active_volatile_bytes;
        u64 active_persistent_bytes;
-       bool dcd_supported;
+       DECLARE_BITMAP(dcd_enabled_cmds, CXL_DCD_ENABLED_MAX);

        struct cxl_event_state event;
        struct cxl_poison_state poison;
@@ -878,12 +878,16 @@ int cxl_arm_dirty_shutdown(struct cxl_memdev_state *mds);

 static inline bool cxl_dcd_supported(struct cxl_memdev_state *mds)
 {
-       return mds->dcd_supported;
+       /*
+        * Disabled until extent event handling is implemented.  Enable by
+        * returning bitmap_full(mds->dcd_enabled_cmds, CXL_DCD_ENABLED_MAX).
+        */
+       return false;
 }

 static inline void cxl_disable_dcd(struct cxl_memdev_state *mds)
 {
-       mds->dcd_supported = false;
+       bitmap_zero(mds->dcd_enabled_cmds, CXL_DCD_ENABLED_MAX);
 }

 int cxl_set_timestamp(struct cxl_memdev_state *mds);

>
snip