[PATCH v2 0/3] Add sanity check for interleave setup

[PATCH v2 0/3] Add sanity check for interleave setup

[Dave Jiang]

The small series adds sanity check for the combination of interleave ways
and interleave granularity during region and port configuration. The
calculation references CXL spec 3.0 8.2.4.19.13 implementation note #3. The
checks also added HDM CAP retrieval for the support of new interleave ways
where 3, 6, and 12 ways support as well as 16 ways support.

v2:
- Change cxl_interleave_verify() to cxl_interleave_capable(). (Dan)
- Move error output inside verify function. (Dan)
- Remove unneeded enums. (Dan)
- Use is_power_of_2() to detect encoded interleave ways. (Dan)
- Change iw to eiw and ig to eig for encoded values. (Alison)
- Change interleave capabilities to mask for easier comparison. (Dan)
- Change valid_interleave() to valid_interleave_ways()
- Add setting fo interleave_cap to cxl_test. (Dan)

---

Dave Jiang (3):
      cxl: Add check for result of interleave ways plus granularity combo
      cxl: Add CXL spec v3.0 interleave support
      tools/testing/cxl: Add interleave check support to mock cxl port device


 drivers/cxl/core/hdm.c       |  6 +++++
 drivers/cxl/core/region.c    | 11 +++++++-
 drivers/cxl/cxl.h            |  2 ++
 drivers/cxl/cxlmem.h         | 50 ++++++++++++++++++++++++++++++++++++
 tools/testing/cxl/test/cxl.c |  3 +++
 5 files changed, 71 insertions(+), 1 deletion(-)

--

[PATCH v2 1/3] cxl: Add check for result of interleave ways plus granularity combo

[Dave Jiang]

Add a helper function to check the combination of interleave ways and
interleave granularity together is sane against the interleave mask from
the HDM decoder. Add the check to cxl_region_attach() to make sure the
region config is sane. Add the check to cxl_port_setup_targets() to make
sure the port setup config is also sane.

Calculation refers to CXL spec v3 8.2.4.19.13 implementation note #3.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/cxl/core/region.c |   11 ++++++++++-
 drivers/cxl/cxlmem.h      |   37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index cf5d5811fe4c..6f77ad22d47d 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -1081,6 +1081,10 @@ static int cxl_port_setup_targets(struct cxl_port *port,
 		return rc;
 	}
 
+	rc = cxl_interleave_capable(port, &cxlr->dev, iw, ig);
+	if (rc)
+		return rc;
+
 	cxld->interleave_ways = iw;
 	cxld->interleave_granularity = ig;
 	dev_dbg(&cxlr->dev, "%s:%s iw: %d ig: %d\n", dev_name(port->uport),
@@ -1218,6 +1222,12 @@ static int cxl_region_attach(struct cxl_region *cxlr,
 		return -EBUSY;
 	}
 
+	ep_port = cxled_to_port(cxled);
+	rc = cxl_interleave_capable(ep_port, &cxlr->dev, p->interleave_ways,
+				    p->interleave_granularity);
+	if (rc)
+		return rc;
+
 	for (i = 0; i < p->interleave_ways; i++) {
 		struct cxl_endpoint_decoder *cxled_target;
 		struct cxl_memdev *cxlmd_target;
@@ -1236,7 +1246,6 @@ static int cxl_region_attach(struct cxl_region *cxlr,
 		}
 	}
 
-	ep_port = cxled_to_port(cxled);
 	root_port = cxlrd_to_port(cxlrd);
 	dport = cxl_find_dport_by_dev(root_port, ep_port->host_bridge);
 	if (!dport) {
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index 88e3a8e54b6a..625fce0b6c2c 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -401,6 +401,43 @@ struct cxl_hdm {
 	struct cxl_port *port;
 };
 
+static inline int cxl_interleave_capable(struct cxl_port *port,
+					 struct device *dev,
+					 int ways, int granularity)
+{
+	struct cxl_hdm *cxlhdm = dev_get_drvdata(&port->dev);
+	unsigned int addr_mask;
+	u16 eig;
+	u8 eiw;
+	int rc;
+
+	rc = granularity_to_cxl(granularity, &eig);
+	if (rc)
+		return rc;
+
+	rc = ways_to_cxl(ways, &eiw);
+	if (rc)
+		return rc;
+
+	if (eiw == 0)
+		return 0;
+
+	if (is_power_of_2(eiw))
+		addr_mask = GENMASK(eig + 8 + eiw - 1, eig + 8);
+	else
+		addr_mask = GENMASK((eig + eiw) / 3 - 1, eig + 8);
+
+	if (~cxlhdm->interleave_mask & addr_mask) {
+		dev_warn(dev,
+			 "%s:%s interleave (eig: %d eiw: %d mask: %#x) exceed cap (mask: %#x)\n",
+			 dev_name(port->uport), dev_name(&port->dev), eig, eiw,
+			 cxlhdm->interleave_mask, addr_mask);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 struct seq_file;
 struct dentry *cxl_debugfs_create_dir(const char *dir);
 void cxl_dpa_debug(struct seq_file *file, struct cxl_dev_state *cxlds);

[PATCH v2 2/3] cxl: Add CXL spec v3.0 interleave support

[Dave Jiang]

CXL spec v3.0 added 2 CAP bits to the CXL HDM Decoder Capability Register.
CXL spec v3.0 8.2.4.19.1. Bit 11 indicates that 3, 6, and 12 way interleave
is capable. Bit 12 indicates that 16 way interleave is capable.

Add code to parse_hdm_decoder_caps() to cache those new bits. Add check in
cxl_interleave_verify() call to make sure those CAP bits matches the passed
in interleave value.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/cxl/core/hdm.c |    6 ++++++
 drivers/cxl/cxl.h      |    2 ++
 drivers/cxl/cxlmem.h   |   13 +++++++++++++
 3 files changed, 21 insertions(+)

diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
index 8143e2615957..0baf3c4820a5 100644
--- a/drivers/cxl/core/hdm.c
+++ b/drivers/cxl/core/hdm.c
@@ -80,6 +80,12 @@ static void parse_hdm_decoder_caps(struct cxl_hdm *cxlhdm)
 		cxlhdm->interleave_mask |= GENMASK(11, 8);
 	if (FIELD_GET(CXL_HDM_DECODER_INTERLEAVE_14_12, hdm_cap))
 		cxlhdm->interleave_mask |= GENMASK(14, 12);
+
+	cxlhdm->interleave_cap = CXL_HDM_INTERLEAVE_CAP_DEFAULT;
+	if (FIELD_GET(CXL_HDM_DECODER_INTERLEAVE_3_6_12_WAY, hdm_cap))
+		cxlhdm->interleave_cap |= CXL_HDM_INTERLEAVE_CAP_3_6_12;
+	if (FIELD_GET(CXL_HDM_DECODER_INTERLEAVE_16_WAY, hdm_cap))
+		cxlhdm->interleave_cap |= CXL_HDM_INTERLEAVE_CAP_16;
 }
 
 static void __iomem *map_hdm_decoder_regs(struct cxl_port *port,
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index bc604b7e44fb..105d814941e7 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -42,6 +42,8 @@
 #define   CXL_HDM_DECODER_TARGET_COUNT_MASK GENMASK(7, 4)
 #define   CXL_HDM_DECODER_INTERLEAVE_11_8 BIT(8)
 #define   CXL_HDM_DECODER_INTERLEAVE_14_12 BIT(9)
+#define   CXL_HDM_DECODER_INTERLEAVE_3_6_12_WAY BIT(11)
+#define   CXL_HDM_DECODER_INTERLEAVE_16_WAY BIT(12)
 #define CXL_HDM_DECODER_CTRL_OFFSET 0x4
 #define   CXL_HDM_DECODER_ENABLE BIT(1)
 #define CXL_HDM_DECODER0_BASE_LOW_OFFSET(i) (0x20 * (i) + 0x10)
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index 625fce0b6c2c..ebd645d8bbb4 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -393,14 +393,24 @@ static inline void cxl_mem_active_dec(void)
 }
 #endif
 
+#define CXL_HDM_INTERLEAVE_CAP_DEFAULT BIT(1) | BIT(2) | BIT(4) | BIT(8)
+#define CXL_HDM_INTERLEAVE_CAP_3_6_12 BIT(3) | BIT(6) | BIT(12)
+#define CXL_HDM_INTERLEAVE_CAP_16 BIT(16)
+
 struct cxl_hdm {
 	struct cxl_component_regs regs;
 	unsigned int decoder_count;
 	unsigned int target_count;
 	unsigned int interleave_mask;
+	unsigned long interleave_cap;
 	struct cxl_port *port;
 };
 
+static inline bool valid_interleave_ways(struct cxl_hdm *cxlhdm, u8 iw)
+{
+	return test_bit(iw, &cxlhdm->interleave_cap);
+}
+
 static inline int cxl_interleave_capable(struct cxl_port *port,
 					 struct device *dev,
 					 int ways, int granularity)
@@ -422,6 +432,9 @@ static inline int cxl_interleave_capable(struct cxl_port *port,
 	if (eiw == 0)
 		return 0;
 
+	if (!valid_interleave_ways(cxlhdm, ways))
+		return -EINVAL;
+
 	if (is_power_of_2(eiw))
 		addr_mask = GENMASK(eig + 8 + eiw - 1, eig + 8);
 	else

[PATCH v2 3/3] tools/testing/cxl: Add interleave check support to mock cxl port device

[Dave Jiang]

Attach the cxl mock hdm to the port device to allow cxl_interleave_verify()
to check the interleave configuration. Set the interleave_mask as well
to support the new verification code.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 tools/testing/cxl/test/cxl.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
index a072b2d3e726..3ce353a20b80 100644
--- a/tools/testing/cxl/test/cxl.c
+++ b/tools/testing/cxl/test/cxl.c
@@ -398,6 +398,9 @@ static struct cxl_hdm *mock_cxl_setup_hdm(struct cxl_port *port)
 		return ERR_PTR(-ENOMEM);
 
 	cxlhdm->port = port;
+	cxlhdm->interleave_mask = GENMASK(14, 8);
+	cxlhdm->interleave_cap = CXL_HDM_INTERLEAVE_CAP_DEFAULT;
+	dev_set_drvdata(&port->dev, cxlhdm);
 	return cxlhdm;
 }
 

RE: [PATCH v2 1/3] cxl: Add check for result of interleave ways plus granularity combo

[Dan Williams]

Dave Jiang wrote:
> Add a helper function to check the combination of interleave ways and
> interleave granularity together is sane against the interleave mask from
> the HDM decoder. Add the check to cxl_region_attach() to make sure the
> region config is sane. Add the check to cxl_port_setup_targets() to make
> sure the port setup config is also sane.
> 
> Calculation refers to CXL spec v3 8.2.4.19.13 implementation note #3.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/cxl/core/region.c |   11 ++++++++++-
>  drivers/cxl/cxlmem.h      |   37 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 47 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index cf5d5811fe4c..6f77ad22d47d 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -1081,6 +1081,10 @@ static int cxl_port_setup_targets(struct cxl_port *port,
>  		return rc;
>  	}
>  
> +	rc = cxl_interleave_capable(port, &cxlr->dev, iw, ig);
> +	if (rc)
> +		return rc;
> +
>  	cxld->interleave_ways = iw;
>  	cxld->interleave_granularity = ig;
>  	dev_dbg(&cxlr->dev, "%s:%s iw: %d ig: %d\n", dev_name(port->uport),
> @@ -1218,6 +1222,12 @@ static int cxl_region_attach(struct cxl_region *cxlr,
>  		return -EBUSY;
>  	}
>  
> +	ep_port = cxled_to_port(cxled);
> +	rc = cxl_interleave_capable(ep_port, &cxlr->dev, p->interleave_ways,
> +				    p->interleave_granularity);
> +	if (rc)
> +		return rc;
> +
>  	for (i = 0; i < p->interleave_ways; i++) {
>  		struct cxl_endpoint_decoder *cxled_target;
>  		struct cxl_memdev *cxlmd_target;
> @@ -1236,7 +1246,6 @@ static int cxl_region_attach(struct cxl_region *cxlr,
>  		}
>  	}
>  
> -	ep_port = cxled_to_port(cxled);
>  	root_port = cxlrd_to_port(cxlrd);
>  	dport = cxl_find_dport_by_dev(root_port, ep_port->host_bridge);
>  	if (!dport) {
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index 88e3a8e54b6a..625fce0b6c2c 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -401,6 +401,43 @@ struct cxl_hdm {
>  	struct cxl_port *port;
>  };
>  
> +static inline int cxl_interleave_capable(struct cxl_port *port,
> +					 struct device *dev,
> +					 int ways, int granularity)
> +{
> +	struct cxl_hdm *cxlhdm = dev_get_drvdata(&port->dev);
> +	unsigned int addr_mask;
> +	u16 eig;
> +	u8 eiw;
> +	int rc;
> +
> +	rc = granularity_to_cxl(granularity, &eig);
> +	if (rc)
> +		return rc;
> +
> +	rc = ways_to_cxl(ways, &eiw);
> +	if (rc)
> +		return rc;
> +
> +	if (eiw == 0)
> +		return 0;
> +
> +	if (is_power_of_2(eiw))
> +		addr_mask = GENMASK(eig + 8 + eiw - 1, eig + 8);
> +	else
> +		addr_mask = GENMASK((eig + eiw) / 3 - 1, eig + 8);
> +
> +	if (~cxlhdm->interleave_mask & addr_mask) {
> +		dev_warn(dev,
> +			 "%s:%s interleave (eig: %d eiw: %d mask: %#x) exceed cap (mask: %#x)\n",
> +			 dev_name(port->uport), dev_name(&port->dev), eig, eiw,
> +			 cxlhdm->interleave_mask, addr_mask);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}

Looks good, just this function is too big to be a static inline. It's
also not used outside of core/region.c, so it can just be private to
core/region.c.

Other than that you can add:

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

RE: [PATCH v2 2/3] cxl: Add CXL spec v3.0 interleave support

[Dan Williams]

Dave Jiang wrote:
> CXL spec v3.0 added 2 CAP bits to the CXL HDM Decoder Capability Register.
> CXL spec v3.0 8.2.4.19.1. Bit 11 indicates that 3, 6, and 12 way interleave
> is capable. Bit 12 indicates that 16 way interleave is capable.
> 
> Add code to parse_hdm_decoder_caps() to cache those new bits. Add check in
> cxl_interleave_verify() call to make sure those CAP bits matches the passed
> in interleave value.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/cxl/core/hdm.c |    6 ++++++
>  drivers/cxl/cxl.h      |    2 ++
>  drivers/cxl/cxlmem.h   |   13 +++++++++++++
>  3 files changed, 21 insertions(+)
> 
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 8143e2615957..0baf3c4820a5 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -80,6 +80,12 @@ static void parse_hdm_decoder_caps(struct cxl_hdm *cxlhdm)
>  		cxlhdm->interleave_mask |= GENMASK(11, 8);
>  	if (FIELD_GET(CXL_HDM_DECODER_INTERLEAVE_14_12, hdm_cap))
>  		cxlhdm->interleave_mask |= GENMASK(14, 12);
> +
> +	cxlhdm->interleave_cap = CXL_HDM_INTERLEAVE_CAP_DEFAULT;
> +	if (FIELD_GET(CXL_HDM_DECODER_INTERLEAVE_3_6_12_WAY, hdm_cap))
> +		cxlhdm->interleave_cap |= CXL_HDM_INTERLEAVE_CAP_3_6_12;
> +	if (FIELD_GET(CXL_HDM_DECODER_INTERLEAVE_16_WAY, hdm_cap))
> +		cxlhdm->interleave_cap |= CXL_HDM_INTERLEAVE_CAP_16;
>  }
>  
>  static void __iomem *map_hdm_decoder_regs(struct cxl_port *port,
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index bc604b7e44fb..105d814941e7 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -42,6 +42,8 @@
>  #define   CXL_HDM_DECODER_TARGET_COUNT_MASK GENMASK(7, 4)
>  #define   CXL_HDM_DECODER_INTERLEAVE_11_8 BIT(8)
>  #define   CXL_HDM_DECODER_INTERLEAVE_14_12 BIT(9)
> +#define   CXL_HDM_DECODER_INTERLEAVE_3_6_12_WAY BIT(11)
> +#define   CXL_HDM_DECODER_INTERLEAVE_16_WAY BIT(12)
>  #define CXL_HDM_DECODER_CTRL_OFFSET 0x4
>  #define   CXL_HDM_DECODER_ENABLE BIT(1)
>  #define CXL_HDM_DECODER0_BASE_LOW_OFFSET(i) (0x20 * (i) + 0x10)
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index 625fce0b6c2c..ebd645d8bbb4 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -393,14 +393,24 @@ static inline void cxl_mem_active_dec(void)
>  }
>  #endif
>  
> +#define CXL_HDM_INTERLEAVE_CAP_DEFAULT BIT(1) | BIT(2) | BIT(4) | BIT(8)
> +#define CXL_HDM_INTERLEAVE_CAP_3_6_12 BIT(3) | BIT(6) | BIT(12)
> +#define CXL_HDM_INTERLEAVE_CAP_16 BIT(16)
> +
>  struct cxl_hdm {
>  	struct cxl_component_regs regs;
>  	unsigned int decoder_count;
>  	unsigned int target_count;
>  	unsigned int interleave_mask;
> +	unsigned long interleave_cap;
>  	struct cxl_port *port;
>  };
>  
> +static inline bool valid_interleave_ways(struct cxl_hdm *cxlhdm, u8 iw)
> +{
> +	return test_bit(iw, &cxlhdm->interleave_cap);
> +}

I think "test_bit(iw, &cxlhdm->interleave_cap)" is self explanatory, no
need for the valid_interleave_ways() wrapper.

Other than that:

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

RE: [PATCH v2 3/3] tools/testing/cxl: Add interleave check support to mock cxl port device

[Dan Williams]

Dave Jiang wrote:
> Attach the cxl mock hdm to the port device to allow cxl_interleave_verify()
> to check the interleave configuration. Set the interleave_mask as well
> to support the new verification code.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  tools/testing/cxl/test/cxl.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
> index a072b2d3e726..3ce353a20b80 100644
> --- a/tools/testing/cxl/test/cxl.c
> +++ b/tools/testing/cxl/test/cxl.c
> @@ -398,6 +398,9 @@ static struct cxl_hdm *mock_cxl_setup_hdm(struct cxl_port *port)
>  		return ERR_PTR(-ENOMEM);
>  
>  	cxlhdm->port = port;
> +	cxlhdm->interleave_mask = GENMASK(14, 8);
> +	cxlhdm->interleave_cap = CXL_HDM_INTERLEAVE_CAP_DEFAULT;
> +	dev_set_drvdata(&port->dev, cxlhdm);
>  	return cxlhdm;
>  }

Looks good,

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

RE: [PATCH v2 0/3] Add sanity check for interleave setup

[Dan Williams]

Dave Jiang wrote:
> The small series adds sanity check for the combination of interleave ways
> and interleave granularity during region and port configuration. The
> calculation references CXL spec 3.0 8.2.4.19.13 implementation note #3. The
> checks also added HDM CAP retrieval for the support of new interleave ways
> where 3, 6, and 12 ways support as well as 16 ways support.
> 
> v2:
> - Change cxl_interleave_verify() to cxl_interleave_capable(). (Dan)
> - Move error output inside verify function. (Dan)
> - Remove unneeded enums. (Dan)
> - Use is_power_of_2() to detect encoded interleave ways. (Dan)
> - Change iw to eiw and ig to eig for encoded values. (Alison)
> - Change interleave capabilities to mask for easier comparison. (Dan)
> - Change valid_interleave() to valid_interleave_ways()
> - Add setting fo interleave_cap to cxl_test. (Dan)
> 
> ---
> 
> Dave Jiang (3):
>       cxl: Add check for result of interleave ways plus granularity combo
>       cxl: Add CXL spec v3.0 interleave support
>       tools/testing/cxl: Add interleave check support to mock cxl port device

After reviewing Vishal's create-region series, I think this patch-kit
wants one more patch in the next posting, or as a follow-on. Add
'interleave_mask' and 'interleave_cap' as new port sysfs attributes.
Likely they need to be added to cxl_port_attribute_groups since they are
only valid while the cxl_port driver is attached.

[PATCH 1/2] cxl: export interleave address mask as port sysfs attribute

[Dave Jiang]

Export the interleave address mask as a sysfs attribute for a port. The
interleave address mask is created based off the CXL HDM Decoder Capability
Register (CXL spec v3 8.2.4.19.1) and sets the bits indicated by th "A11to8
Interleave Capable" bit and the "A14to12 Interleave Capable" bit. It
indicates the decoder supports interleaveing based on those address bits.
The exported sysfs attribute will help user region creation to do more valid
configuration checking.

Suggested-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/cxl/port.c |   22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
index 5453771bf330..e3e93e1b663e 100644
--- a/drivers/cxl/port.c
+++ b/drivers/cxl/port.c
@@ -123,8 +123,30 @@ static struct attribute_group cxl_cdat_attribute_group = {
 	.is_bin_visible = cxl_port_bin_attr_is_visible,
 };
 
+static ssize_t interleave_mask_show(struct device *dev, struct device_attribute *attr,
+				     char *buf)
+{
+	struct cxl_hdm *cxlhdm = dev_get_drvdata(dev);
+
+	if (!cxlhdm)
+		return 0;
+
+	return sysfs_emit(buf, "%#x\n", cxlhdm->interleave_mask);
+}
+static DEVICE_ATTR_RO(interleave_mask);
+
+static struct attribute *cxl_port_info_attributes[] = {
+	&dev_attr_interleave_mask.attr,
+	NULL,
+};
+
+static struct attribute_group cxl_port_info_attribute_group = {
+	.attrs = cxl_port_info_attributes,
+};
+
 static const struct attribute_group *cxl_port_attribute_groups[] = {
 	&cxl_cdat_attribute_group,
+	&cxl_port_info_attribute_group,
 	NULL,
 };
 

[PATCH 2/2] cxl: export intereleave capability as port sysfs attribute

[Dave Jiang]

Export the interleave capability as a sysfs attribute for a port. The
exported mask is interpreted from the CXL HDM Decoder Capability Register
(CXL spec v 8.2.4.19.1). Each bit in the mask represents the number of
interleave ways the decoder supports. For example, CXL devices designed
from CXL spec v2.0 supports 1, 2, 4, and 8 interleave ways. The exported
mask would show 0x116. The exported sysfs attribute will help user region
creation to do more valid configuration checking.

Suggested-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/cxl/port.c |   13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
index e3e93e1b663e..231cfe888918 100644
--- a/drivers/cxl/port.c
+++ b/drivers/cxl/port.c
@@ -135,8 +135,21 @@ static ssize_t interleave_mask_show(struct device *dev, struct device_attribute
 }
 static DEVICE_ATTR_RO(interleave_mask);
 
+static ssize_t interleave_cap_show(struct device *dev, struct device_attribute *attr,
+				   char *buf)
+{
+	struct cxl_hdm *cxlhdm = dev_get_drvdata(dev);
+
+	if (!cxlhdm)
+		return 0;
+
+	return sysfs_emit(buf, "%#lx\n", cxlhdm->interleave_cap);
+}
+static DEVICE_ATTR_RO(interleave_cap);
+
 static struct attribute *cxl_port_info_attributes[] = {
 	&dev_attr_interleave_mask.attr,
+	&dev_attr_interleave_cap.attr,
 	NULL,
 };
 

RE: [PATCH 1/2] cxl: export interleave address mask as port sysfs attribute

[Dan Williams]

Dave Jiang wrote:
> Export the interleave address mask as a sysfs attribute for a port. The
> interleave address mask is created based off the CXL HDM Decoder Capability
> Register (CXL spec v3 8.2.4.19.1) and sets the bits indicated by th "A11to8
> Interleave Capable" bit and the "A14to12 Interleave Capable" bit. It
> indicates the decoder supports interleaveing based on those address bits.
> The exported sysfs attribute will help user region creation to do more valid
> configuration checking.
> 
> Suggested-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/cxl/port.c |   22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)

This also needs updates to:

Documentation/driver-api/cxl/memory-devices.rst

> 
> diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
> index 5453771bf330..e3e93e1b663e 100644
> --- a/drivers/cxl/port.c
> +++ b/drivers/cxl/port.c
> @@ -123,8 +123,30 @@ static struct attribute_group cxl_cdat_attribute_group = {
>  	.is_bin_visible = cxl_port_bin_attr_is_visible,
>  };
>  
> +static ssize_t interleave_mask_show(struct device *dev, struct device_attribute *attr,
> +				     char *buf)
> +{
> +	struct cxl_hdm *cxlhdm = dev_get_drvdata(dev);
> +
> +	if (!cxlhdm)
> +		return 0;

Given that this is an attribute in 'struct device_driver'.dev_groups it
is guaranteed to only be visible while the device is successfully
attached to its driver. So this check is not necessary.

> +
> +	return sysfs_emit(buf, "%#x\n", cxlhdm->interleave_mask);
> +}
> +static DEVICE_ATTR_RO(interleave_mask);
> +
> +static struct attribute *cxl_port_info_attributes[] = {
> +	&dev_attr_interleave_mask.attr,
> +	NULL,
> +};
> +
> +static struct attribute_group cxl_port_info_attribute_group = {
> +	.attrs = cxl_port_info_attributes,
> +};
> +
>  static const struct attribute_group *cxl_port_attribute_groups[] = {

Lets do a lead in patch to rename this to avoid the confusion with the
static port attributes in the core:

s/cxl_port_attribute_groups/cxl_port_dynamic_attr_groups/

RE: [PATCH 2/2] cxl: export intereleave capability as port sysfs attribute

[Dan Williams]

Dave Jiang wrote:
> Export the interleave capability as a sysfs attribute for a port. The
> exported mask is interpreted from the CXL HDM Decoder Capability Register
> (CXL spec v 8.2.4.19.1). Each bit in the mask represents the number of
> interleave ways the decoder supports. For example, CXL devices designed
> from CXL spec v2.0 supports 1, 2, 4, and 8 interleave ways. The exported
> mask would show 0x116. The exported sysfs attribute will help user region
> creation to do more valid configuration checking.
> 
> Suggested-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/cxl/port.c |   13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
> index e3e93e1b663e..231cfe888918 100644
> --- a/drivers/cxl/port.c
> +++ b/drivers/cxl/port.c
> @@ -135,8 +135,21 @@ static ssize_t interleave_mask_show(struct device *dev, struct device_attribute
>  }
>  static DEVICE_ATTR_RO(interleave_mask);
>  
> +static ssize_t interleave_cap_show(struct device *dev, struct device_attribute *attr,
> +				   char *buf)
> +{
> +	struct cxl_hdm *cxlhdm = dev_get_drvdata(dev);
> +
> +	if (!cxlhdm)
> +		return 0;

Similar Documentation + "delete this check" comments as patch1.