All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/6] Add sanity check for interleave setup
@ 2022-08-17 21:21 Dave Jiang
  2022-08-17 21:21 ` [PATCH v4 1/6] cxl: Add check for result of interleave ways plus granularity combo Dave Jiang
                   ` (5 more replies)
  0 siblings, 6 replies; 16+ messages in thread
From: Dave Jiang @ 2022-08-17 21:21 UTC (permalink / raw)
  To: linux-cxl
  Cc: dan.j.williams, vishal.l.verma, ira.weiny, alison.schofield,
	Jonathan.Cameron

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.

v4:
- Add documentation for sysfs entries (Dan)
- Remove unneeded checks for drvdata validity (Dan)
- Add renaming of cxl_port_attribute_groups to cxl_port_dynamic_attr_groups
  (Dan)

v3:
- Move cxl_interleave_capable() to core/region.c. (Dan)
- Open code verify of interleave ways against cap mask. (Dan)

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 (6):
      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
      cxl: change cxl_port_attribute_groups naming to avoid confusion
      cxl: export interleave address mask as port sysfs attribute
      cxl: export intereleave capability as port sysfs attribute


 Documentation/ABI/testing/sysfs-bus-cxl | 24 ++++++++++++
 drivers/cxl/core/hdm.c                  |  6 +++
 drivers/cxl/core/region.c               | 50 ++++++++++++++++++++++++-
 drivers/cxl/cxl.h                       |  2 +
 drivers/cxl/cxlmem.h                    |  5 +++
 drivers/cxl/port.c                      | 33 +++++++++++++++-
 tools/testing/cxl/test/cxl.c            |  3 ++
 7 files changed, 120 insertions(+), 3 deletions(-)

base-commit: 1cd8a2537eb07751d405ab7e2223f20338a90506
--


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v4 1/6] cxl: Add check for result of interleave ways plus granularity combo
  2022-08-17 21:21 [PATCH v4 0/6] Add sanity check for interleave setup Dave Jiang
@ 2022-08-17 21:21 ` Dave Jiang
  2022-08-17 21:21 ` [PATCH v4 2/6] cxl: Add CXL spec v3.0 interleave support Dave Jiang
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Dave Jiang @ 2022-08-17 21:21 UTC (permalink / raw)
  To: linux-cxl
  Cc: dan.j.williams, vishal.l.verma, ira.weiny, alison.schofield,
	Jonathan.Cameron

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.

Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/cxl/core/region.c |   47 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 46 insertions(+), 1 deletion(-)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 401148016978..28272b0196e6 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -940,6 +940,42 @@ static int check_last_peer(struct cxl_endpoint_decoder *cxled,
 	return 0;
 }
 
+static 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_dbg(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;
+}
+
 static int cxl_port_setup_targets(struct cxl_port *port,
 				  struct cxl_region *cxlr,
 				  struct cxl_endpoint_decoder *cxled)
@@ -1047,6 +1083,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;
 	cxld->hpa_range = (struct range) {
@@ -1196,6 +1236,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;
@@ -1214,7 +1260,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) {



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v4 2/6] cxl: Add CXL spec v3.0 interleave support
  2022-08-17 21:21 [PATCH v4 0/6] Add sanity check for interleave setup Dave Jiang
  2022-08-17 21:21 ` [PATCH v4 1/6] cxl: Add check for result of interleave ways plus granularity combo Dave Jiang
@ 2022-08-17 21:21 ` Dave Jiang
  2022-08-24 14:46   ` Jonathan Cameron
  2022-08-24 14:56   ` Jonathan Cameron
  2022-08-17 21:21 ` [PATCH v4 3/6] tools/testing/cxl: Add interleave check support to mock cxl port device Dave Jiang
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 16+ messages in thread
From: Dave Jiang @ 2022-08-17 21:21 UTC (permalink / raw)
  To: linux-cxl
  Cc: dan.j.williams, vishal.l.verma, ira.weiny, alison.schofield,
	Jonathan.Cameron

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.

Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/cxl/core/hdm.c    |    6 ++++++
 drivers/cxl/core/region.c |    3 +++
 drivers/cxl/cxl.h         |    2 ++
 drivers/cxl/cxlmem.h      |    5 +++++
 4 files changed, 16 insertions(+)

diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
index d1d2caea5c62..2f91ff9b0227 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/core/region.c b/drivers/cxl/core/region.c
index 28272b0196e6..9851ab2782f2 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -960,6 +960,9 @@ static int cxl_interleave_capable(struct cxl_port *port, struct device *dev,
 	if (eiw == 0)
 		return 0;
 
+	if (!test_bit(ways, &cxlhdm->interleave_cap))
+		return -EINVAL;
+
 	if (is_power_of_2(eiw))
 		addr_mask = GENMASK(eig + 8 + eiw - 1, eig + 8);
 	else
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index f680450f0b16..11f2a14f42eb 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 88e3a8e54b6a..4e65c9cc1d30 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -393,11 +393,16 @@ 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;
 };
 



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v4 3/6] tools/testing/cxl: Add interleave check support to mock cxl port device
  2022-08-17 21:21 [PATCH v4 0/6] Add sanity check for interleave setup Dave Jiang
  2022-08-17 21:21 ` [PATCH v4 1/6] cxl: Add check for result of interleave ways plus granularity combo Dave Jiang
  2022-08-17 21:21 ` [PATCH v4 2/6] cxl: Add CXL spec v3.0 interleave support Dave Jiang
@ 2022-08-17 21:21 ` Dave Jiang
  2022-08-24 14:59   ` Jonathan Cameron
  2022-08-17 21:21 ` [PATCH v4 4/6] cxl: change cxl_port_attribute_groups naming to avoid confusion Dave Jiang
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 16+ messages in thread
From: Dave Jiang @ 2022-08-17 21:21 UTC (permalink / raw)
  To: linux-cxl
  Cc: dan.j.williams, vishal.l.verma, ira.weiny, alison.schofield,
	Jonathan.Cameron

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.

Reviewed-by: Dan Williams <dan.j.williams@intel.com>
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;
 }
 



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v4 4/6] cxl: change cxl_port_attribute_groups naming to avoid confusion
  2022-08-17 21:21 [PATCH v4 0/6] Add sanity check for interleave setup Dave Jiang
                   ` (2 preceding siblings ...)
  2022-08-17 21:21 ` [PATCH v4 3/6] tools/testing/cxl: Add interleave check support to mock cxl port device Dave Jiang
@ 2022-08-17 21:21 ` Dave Jiang
  2022-08-24 14:48   ` Jonathan Cameron
  2022-08-17 21:22 ` [PATCH v4 5/6] cxl: export interleave address mask as port sysfs attribute Dave Jiang
  2022-08-17 21:22 ` [PATCH v4 6/6] cxl: export intereleave capability " Dave Jiang
  5 siblings, 1 reply; 16+ messages in thread
From: Dave Jiang @ 2022-08-17 21:21 UTC (permalink / raw)
  To: linux-cxl
  Cc: dan.j.williams, vishal.l.verma, ira.weiny, alison.schofield,
	Jonathan.Cameron

Both cxl/port.c and cxl/core/port.c have cxl_port_attribute_groups. Change
cxl_port_attribute_groups in cxl/port.c to cxl_port_dynamic_attr_groups in
order to avoid confusion.

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

diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
index 5453771bf330..c4aa073b7e31 100644
--- a/drivers/cxl/port.c
+++ b/drivers/cxl/port.c
@@ -123,7 +123,7 @@ static struct attribute_group cxl_cdat_attribute_group = {
 	.is_bin_visible = cxl_port_bin_attr_is_visible,
 };
 
-static const struct attribute_group *cxl_port_attribute_groups[] = {
+static const struct attribute_group *cxl_port_dynamic_attr_groups[] = {
 	&cxl_cdat_attribute_group,
 	NULL,
 };
@@ -133,7 +133,7 @@ static struct cxl_driver cxl_port_driver = {
 	.probe = cxl_port_probe,
 	.id = CXL_DEVICE_PORT,
 	.drv = {
-		.dev_groups = cxl_port_attribute_groups,
+		.dev_groups = cxl_port_dynamic_attr_groups,
 	},
 };
 



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v4 5/6] cxl: export interleave address mask as port sysfs attribute
  2022-08-17 21:21 [PATCH v4 0/6] Add sanity check for interleave setup Dave Jiang
                   ` (3 preceding siblings ...)
  2022-08-17 21:21 ` [PATCH v4 4/6] cxl: change cxl_port_attribute_groups naming to avoid confusion Dave Jiang
@ 2022-08-17 21:22 ` Dave Jiang
  2022-08-24 14:34   ` Jonathan Cameron
  2022-08-17 21:22 ` [PATCH v4 6/6] cxl: export intereleave capability " Dave Jiang
  5 siblings, 1 reply; 16+ messages in thread
From: Dave Jiang @ 2022-08-17 21:22 UTC (permalink / raw)
  To: linux-cxl
  Cc: dan.j.williams, vishal.l.verma, ira.weiny, alison.schofield,
	Jonathan.Cameron

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>
---
 Documentation/ABI/testing/sysfs-bus-cxl |   11 +++++++++++
 drivers/cxl/port.c                      |   19 +++++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
index 8494ef27e8d2..c6f533f47e50 100644
--- a/Documentation/ABI/testing/sysfs-bus-cxl
+++ b/Documentation/ABI/testing/sysfs-bus-cxl
@@ -191,6 +191,17 @@ Description:
 		the data is 0 reading the CDAT data failed.  Otherwise the CDAT
 		data is reported.
 
+What:		/sys/bus/cxl/devices/endpointX/interleave_mask
+		/sys/bus/cxl/devices/portX/interleave_mask
+Date:		Aug, 2020
+KernelVersion:	v6.1
+Contact:	linux-cxl@vger.kernel.org
+Description:
+		(RO) Interleve address mask from the HDM decoder attached to the
+		port. The address bits are set depending on the CXL HDM Decoder
+		Capability Register (CXL spec v3 8.2.4.19.1) where the "A11to8
+		Interleave Capable" bit and the "AA14to12 Interleave Capable" bits
+		are set.
 
 What:		/sys/bus/cxl/devices/decoderX.Y/mode
 Date:		May, 2022
diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
index c4aa073b7e31..567f62fd4ded 100644
--- a/drivers/cxl/port.c
+++ b/drivers/cxl/port.c
@@ -123,8 +123,27 @@ 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);
+
+	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_dynamic_attr_groups[] = {
 	&cxl_cdat_attribute_group,
+	&cxl_port_info_attribute_group,
 	NULL,
 };
 



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v4 6/6] cxl: export intereleave capability as port sysfs attribute
  2022-08-17 21:21 [PATCH v4 0/6] Add sanity check for interleave setup Dave Jiang
                   ` (4 preceding siblings ...)
  2022-08-17 21:22 ` [PATCH v4 5/6] cxl: export interleave address mask as port sysfs attribute Dave Jiang
@ 2022-08-17 21:22 ` Dave Jiang
  2022-08-24 14:28   ` Jonathan Cameron
  5 siblings, 1 reply; 16+ messages in thread
From: Dave Jiang @ 2022-08-17 21:22 UTC (permalink / raw)
  To: linux-cxl
  Cc: dan.j.williams, vishal.l.verma, ira.weiny, alison.schofield,
	Jonathan.Cameron

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>
---
 Documentation/ABI/testing/sysfs-bus-cxl |   13 +++++++++++++
 drivers/cxl/port.c                      |   10 ++++++++++
 2 files changed, 23 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
index c6f533f47e50..5a13806a77ab 100644
--- a/Documentation/ABI/testing/sysfs-bus-cxl
+++ b/Documentation/ABI/testing/sysfs-bus-cxl
@@ -203,6 +203,19 @@ Description:
 		Interleave Capable" bit and the "AA14to12 Interleave Capable" bits
 		are set.
 
+What:		/sys/bus/cxl/devices/endpointX/interleave_cap
+		/sys/bus/cxl/devices/portX/interleave_cap
+Date:		Aug, 2020
+KernelVersion:	v6.1
+Contact:	linux-cxl@vger.kernel.org
+Description:
+		(RO) Interleave capability mask from the HDM decoder attached to the
+		port. Each bit in the mask represents the number of interleave ways
+		the decoder supports. For CXL devices designed from CXL spec v2.0 or
+		earlier, 1, 2, 4, and 8 interleave ways are supported. With CXL spec
+		v3.0 or later, the capability register (CXL spec v3 8.2.4.19.1)
+		indicates 3, 6, and 12 ways supported or 16 ways supported.
+
 What:		/sys/bus/cxl/devices/decoderX.Y/mode
 Date:		May, 2022
 KernelVersion:	v5.20
diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
index 567f62fd4ded..f856a31bec65 100644
--- a/drivers/cxl/port.c
+++ b/drivers/cxl/port.c
@@ -132,8 +132,18 @@ static ssize_t interleave_mask_show(struct device *dev,
 }
 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);
+
+	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,
 };
 



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 6/6] cxl: export intereleave capability as port sysfs attribute
  2022-08-17 21:22 ` [PATCH v4 6/6] cxl: export intereleave capability " Dave Jiang
@ 2022-08-24 14:28   ` Jonathan Cameron
  0 siblings, 0 replies; 16+ messages in thread
From: Jonathan Cameron @ 2022-08-24 14:28 UTC (permalink / raw)
  To: Dave Jiang
  Cc: linux-cxl, dan.j.williams, vishal.l.verma, ira.weiny, alison.schofield

On Wed, 17 Aug 2022 14:22:09 -0700
Dave Jiang <dave.jiang@intel.com> 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

State which spec in all references (this one is rev3.0)

Otherwise, whilst it's not a particularly intuitive interface, I guess it
works reasonably well.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> 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>
> ---
>  Documentation/ABI/testing/sysfs-bus-cxl |   13 +++++++++++++
>  drivers/cxl/port.c                      |   10 ++++++++++
>  2 files changed, 23 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
> index c6f533f47e50..5a13806a77ab 100644
> --- a/Documentation/ABI/testing/sysfs-bus-cxl
> +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> @@ -203,6 +203,19 @@ Description:
>  		Interleave Capable" bit and the "AA14to12 Interleave Capable" bits
>  		are set.
>  
> +What:		/sys/bus/cxl/devices/endpointX/interleave_cap
> +		/sys/bus/cxl/devices/portX/interleave_cap
> +Date:		Aug, 2020
> +KernelVersion:	v6.1
> +Contact:	linux-cxl@vger.kernel.org
> +Description:
> +		(RO) Interleave capability mask from the HDM decoder attached to the
> +		port. Each bit in the mask represents the number of interleave ways
> +		the decoder supports. For CXL devices designed from CXL spec v2.0 or
> +		earlier, 1, 2, 4, and 8 interleave ways are supported. With CXL spec
> +		v3.0 or later, the capability register (CXL spec v3 8.2.4.19.1)
> +		indicates 3, 6, and 12 ways supported or 16 ways supported.
> +
>  What:		/sys/bus/cxl/devices/decoderX.Y/mode
>  Date:		May, 2022
>  KernelVersion:	v5.20
> diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
> index 567f62fd4ded..f856a31bec65 100644
> --- a/drivers/cxl/port.c
> +++ b/drivers/cxl/port.c
> @@ -132,8 +132,18 @@ static ssize_t interleave_mask_show(struct device *dev,
>  }
>  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);
> +
> +	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,
>  };
>  
> 
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 5/6] cxl: export interleave address mask as port sysfs attribute
  2022-08-17 21:22 ` [PATCH v4 5/6] cxl: export interleave address mask as port sysfs attribute Dave Jiang
@ 2022-08-24 14:34   ` Jonathan Cameron
  0 siblings, 0 replies; 16+ messages in thread
From: Jonathan Cameron @ 2022-08-24 14:34 UTC (permalink / raw)
  To: Dave Jiang
  Cc: linux-cxl, dan.j.williams, vishal.l.verma, ira.weiny, alison.schofield

On Wed, 17 Aug 2022 14:22:04 -0700
Dave Jiang <dave.jiang@intel.com> 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

the

> Interleave Capable" bit and the "A14to12 Interleave Capable" bit. It
> indicates the decoder supports interleaveing based on those address bits.
interleaving (spelling from the spec.)

> 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>
Looks good other than needing a spell check (not that I can take the high
ground on spelling, or remembering to spell check patches ;)

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
>  Documentation/ABI/testing/sysfs-bus-cxl |   11 +++++++++++
>  drivers/cxl/port.c                      |   19 +++++++++++++++++++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
> index 8494ef27e8d2..c6f533f47e50 100644
> --- a/Documentation/ABI/testing/sysfs-bus-cxl
> +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> @@ -191,6 +191,17 @@ Description:
>  		the data is 0 reading the CDAT data failed.  Otherwise the CDAT
>  		data is reported.
>  
> +What:		/sys/bus/cxl/devices/endpointX/interleave_mask
> +		/sys/bus/cxl/devices/portX/interleave_mask
> +Date:		Aug, 2020
> +KernelVersion:	v6.1
> +Contact:	linux-cxl@vger.kernel.org
> +Description:
> +		(RO) Interleve address mask from the HDM decoder attached to the
Interleave
> +		port. The address bits are set depending on the CXL HDM Decoder
> +		Capability Register (CXL spec v3 8.2.4.19.1) where the "A11to8
rev3.0

Technically it's version 1 of revision 3

> +		Interleave Capable" bit and the "AA14to12 Interleave Capable" bits
> +		are set.
>  
>  What:		/sys/bus/cxl/devices/decoderX.Y/mode
>  Date:		May, 2022
> diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
> index c4aa073b7e31..567f62fd4ded 100644
> --- a/drivers/cxl/port.c
> +++ b/drivers/cxl/port.c
> @@ -123,8 +123,27 @@ 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);
> +
> +	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,

I'd not put a comma after a NULL terminator, but just did a grep
and this inline with rest of drivers/cxl so fair enough.

> +};
> +
> +static struct attribute_group cxl_port_info_attribute_group = {
> +	.attrs = cxl_port_info_attributes,
> +};
> +
>  static const struct attribute_group *cxl_port_dynamic_attr_groups[] = {
>  	&cxl_cdat_attribute_group,
> +	&cxl_port_info_attribute_group,
>  	NULL,
>  };
>  
> 
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 2/6] cxl: Add CXL spec v3.0 interleave support
  2022-08-17 21:21 ` [PATCH v4 2/6] cxl: Add CXL spec v3.0 interleave support Dave Jiang
@ 2022-08-24 14:46   ` Jonathan Cameron
  2022-08-24 21:03     ` Dave Jiang
  2022-08-24 14:56   ` Jonathan Cameron
  1 sibling, 1 reply; 16+ messages in thread
From: Jonathan Cameron @ 2022-08-24 14:46 UTC (permalink / raw)
  To: Dave Jiang
  Cc: linux-cxl, dan.j.williams, vishal.l.verma, ira.weiny, alison.schofield

On Wed, 17 Aug 2022 14:21:48 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> CXL spec v3.0 added 2 CAP bits to the CXL HDM Decoder Capability Register.
rev3.0 - though I don't really care that much...  Dropping the v works too.
CXL 3.0 is fine by me.

> 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.

> 
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>

One comment on naming inline, but other than that bikeshedding.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
>  drivers/cxl/core/hdm.c    |    6 ++++++
>  drivers/cxl/core/region.c |    3 +++
>  drivers/cxl/cxl.h         |    2 ++
>  drivers/cxl/cxlmem.h      |    5 +++++
>  4 files changed, 16 insertions(+)
> 
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index d1d2caea5c62..2f91ff9b0227 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;

DEFAULT is somewhat odd naming for a capability value.
BASELINE maybe?

> +	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/core/region.c b/drivers/cxl/core/region.c
> index 28272b0196e6..9851ab2782f2 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -960,6 +960,9 @@ static int cxl_interleave_capable(struct cxl_port *port, struct device *dev,
>  	if (eiw == 0)
>  		return 0;
>  
> +	if (!test_bit(ways, &cxlhdm->interleave_cap))
> +		return -EINVAL;
> +
>  	if (is_power_of_2(eiw))
>  		addr_mask = GENMASK(eig + 8 + eiw - 1, eig + 8);
>  	else
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index f680450f0b16..11f2a14f42eb 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 88e3a8e54b6a..4e65c9cc1d30 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -393,11 +393,16 @@ 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;
>  };
>  
> 
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 4/6] cxl: change cxl_port_attribute_groups naming to avoid confusion
  2022-08-17 21:21 ` [PATCH v4 4/6] cxl: change cxl_port_attribute_groups naming to avoid confusion Dave Jiang
@ 2022-08-24 14:48   ` Jonathan Cameron
  0 siblings, 0 replies; 16+ messages in thread
From: Jonathan Cameron @ 2022-08-24 14:48 UTC (permalink / raw)
  To: Dave Jiang
  Cc: linux-cxl, dan.j.williams, vishal.l.verma, ira.weiny, alison.schofield

On Wed, 17 Aug 2022 14:21:58 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> Both cxl/port.c and cxl/core/port.c have cxl_port_attribute_groups. Change
> cxl_port_attribute_groups in cxl/port.c to cxl_port_dynamic_attr_groups in
> order to avoid confusion.
> 
> Suggested-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>

Fair enough I guess.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>  drivers/cxl/port.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
> index 5453771bf330..c4aa073b7e31 100644
> --- a/drivers/cxl/port.c
> +++ b/drivers/cxl/port.c
> @@ -123,7 +123,7 @@ static struct attribute_group cxl_cdat_attribute_group = {
>  	.is_bin_visible = cxl_port_bin_attr_is_visible,
>  };
>  
> -static const struct attribute_group *cxl_port_attribute_groups[] = {
> +static const struct attribute_group *cxl_port_dynamic_attr_groups[] = {
>  	&cxl_cdat_attribute_group,
>  	NULL,
>  };
> @@ -133,7 +133,7 @@ static struct cxl_driver cxl_port_driver = {
>  	.probe = cxl_port_probe,
>  	.id = CXL_DEVICE_PORT,
>  	.drv = {
> -		.dev_groups = cxl_port_attribute_groups,
> +		.dev_groups = cxl_port_dynamic_attr_groups,
>  	},
>  };
>  
> 
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 2/6] cxl: Add CXL spec v3.0 interleave support
  2022-08-17 21:21 ` [PATCH v4 2/6] cxl: Add CXL spec v3.0 interleave support Dave Jiang
  2022-08-24 14:46   ` Jonathan Cameron
@ 2022-08-24 14:56   ` Jonathan Cameron
  2022-08-24 21:04     ` Dave Jiang
  1 sibling, 1 reply; 16+ messages in thread
From: Jonathan Cameron @ 2022-08-24 14:56 UTC (permalink / raw)
  To: Dave Jiang
  Cc: linux-cxl, dan.j.williams, vishal.l.verma, ira.weiny, alison.schofield

On Wed, 17 Aug 2022 14:21:48 -0700
Dave Jiang <dave.jiang@intel.com> 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
Whilst looking at mocking patch, I noticed there isn't a cxl_interleave_verify()
renamed?  Looks like that was in v2 from your change log.

> in interleave value.
> 
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/cxl/core/hdm.c    |    6 ++++++
>  drivers/cxl/core/region.c |    3 +++
>  drivers/cxl/cxl.h         |    2 ++
>  drivers/cxl/cxlmem.h      |    5 +++++
>  4 files changed, 16 insertions(+)
> 
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index d1d2caea5c62..2f91ff9b0227 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/core/region.c b/drivers/cxl/core/region.c
> index 28272b0196e6..9851ab2782f2 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -960,6 +960,9 @@ static int cxl_interleave_capable(struct cxl_port *port, struct device *dev,
>  	if (eiw == 0)
>  		return 0;
>  
> +	if (!test_bit(ways, &cxlhdm->interleave_cap))
> +		return -EINVAL;
> +
>  	if (is_power_of_2(eiw))
>  		addr_mask = GENMASK(eig + 8 + eiw - 1, eig + 8);
>  	else
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index f680450f0b16..11f2a14f42eb 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 88e3a8e54b6a..4e65c9cc1d30 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -393,11 +393,16 @@ 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;
>  };
>  
> 
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 3/6] tools/testing/cxl: Add interleave check support to mock cxl port device
  2022-08-17 21:21 ` [PATCH v4 3/6] tools/testing/cxl: Add interleave check support to mock cxl port device Dave Jiang
@ 2022-08-24 14:59   ` Jonathan Cameron
  2022-08-24 21:13     ` Dave Jiang
  0 siblings, 1 reply; 16+ messages in thread
From: Jonathan Cameron @ 2022-08-24 14:59 UTC (permalink / raw)
  To: Dave Jiang
  Cc: linux-cxl, dan.j.williams, vishal.l.verma, ira.weiny, alison.schofield

On Wed, 17 Aug 2022 14:21:53 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> Attach the cxl mock hdm to the port device to allow cxl_interleave_verify()
I couldn't follow what was going on here, until I realized you've renamed
this function.  Now cxl_interleave_capable().

Are the tests broken (null dereference) between patch 1 and 3?

Seem like dev_set_drvdata() should move to patch 1.

> to check the interleave configuration. Set the interleave_mask as well
> to support the new verification code.
> 
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> 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;
>  }
>  
> 
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 2/6] cxl: Add CXL spec v3.0 interleave support
  2022-08-24 14:46   ` Jonathan Cameron
@ 2022-08-24 21:03     ` Dave Jiang
  0 siblings, 0 replies; 16+ messages in thread
From: Dave Jiang @ 2022-08-24 21:03 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-cxl, dan.j.williams, vishal.l.verma, ira.weiny, alison.schofield


On 8/24/2022 7:46 AM, Jonathan Cameron wrote:
> On Wed, 17 Aug 2022 14:21:48 -0700
> Dave Jiang <dave.jiang@intel.com> wrote:
>
>> CXL spec v3.0 added 2 CAP bits to the CXL HDM Decoder Capability Register.
> rev3.0 - though I don't really care that much...  Dropping the v works too.
> CXL 3.0 is fine by me.
I'll make it rev3.0 throughout.
>> 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.
>> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> One comment on naming inline, but other than that bikeshedding.
>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
>> ---
>>   drivers/cxl/core/hdm.c    |    6 ++++++
>>   drivers/cxl/core/region.c |    3 +++
>>   drivers/cxl/cxl.h         |    2 ++
>>   drivers/cxl/cxlmem.h      |    5 +++++
>>   4 files changed, 16 insertions(+)
>>
>> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
>> index d1d2caea5c62..2f91ff9b0227 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;
> DEFAULT is somewhat odd naming for a capability value.
> BASELINE maybe?
Ok will change to BASELINE.
>
>> +	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/core/region.c b/drivers/cxl/core/region.c
>> index 28272b0196e6..9851ab2782f2 100644
>> --- a/drivers/cxl/core/region.c
>> +++ b/drivers/cxl/core/region.c
>> @@ -960,6 +960,9 @@ static int cxl_interleave_capable(struct cxl_port *port, struct device *dev,
>>   	if (eiw == 0)
>>   		return 0;
>>   
>> +	if (!test_bit(ways, &cxlhdm->interleave_cap))
>> +		return -EINVAL;
>> +
>>   	if (is_power_of_2(eiw))
>>   		addr_mask = GENMASK(eig + 8 + eiw - 1, eig + 8);
>>   	else
>> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
>> index f680450f0b16..11f2a14f42eb 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 88e3a8e54b6a..4e65c9cc1d30 100644
>> --- a/drivers/cxl/cxlmem.h
>> +++ b/drivers/cxl/cxlmem.h
>> @@ -393,11 +393,16 @@ 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;
>>   };
>>   
>>
>>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 2/6] cxl: Add CXL spec v3.0 interleave support
  2022-08-24 14:56   ` Jonathan Cameron
@ 2022-08-24 21:04     ` Dave Jiang
  0 siblings, 0 replies; 16+ messages in thread
From: Dave Jiang @ 2022-08-24 21:04 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-cxl, dan.j.williams, vishal.l.verma, ira.weiny, alison.schofield


On 8/24/2022 7:56 AM, Jonathan Cameron wrote:
> On Wed, 17 Aug 2022 14:21:48 -0700
> Dave Jiang <dave.jiang@intel.com> 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
> Whilst looking at mocking patch, I noticed there isn't a cxl_interleave_verify()
> renamed?  Looks like that was in v2 from your change log.
I need to fixup the commit headers all to cxl_interleave_capable(). 
Forgot about that.
>
>> in interleave value.
>>
>> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>> ---
>>   drivers/cxl/core/hdm.c    |    6 ++++++
>>   drivers/cxl/core/region.c |    3 +++
>>   drivers/cxl/cxl.h         |    2 ++
>>   drivers/cxl/cxlmem.h      |    5 +++++
>>   4 files changed, 16 insertions(+)
>>
>> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
>> index d1d2caea5c62..2f91ff9b0227 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/core/region.c b/drivers/cxl/core/region.c
>> index 28272b0196e6..9851ab2782f2 100644
>> --- a/drivers/cxl/core/region.c
>> +++ b/drivers/cxl/core/region.c
>> @@ -960,6 +960,9 @@ static int cxl_interleave_capable(struct cxl_port *port, struct device *dev,
>>   	if (eiw == 0)
>>   		return 0;
>>   
>> +	if (!test_bit(ways, &cxlhdm->interleave_cap))
>> +		return -EINVAL;
>> +
>>   	if (is_power_of_2(eiw))
>>   		addr_mask = GENMASK(eig + 8 + eiw - 1, eig + 8);
>>   	else
>> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
>> index f680450f0b16..11f2a14f42eb 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 88e3a8e54b6a..4e65c9cc1d30 100644
>> --- a/drivers/cxl/cxlmem.h
>> +++ b/drivers/cxl/cxlmem.h
>> @@ -393,11 +393,16 @@ 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;
>>   };
>>   
>>
>>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 3/6] tools/testing/cxl: Add interleave check support to mock cxl port device
  2022-08-24 14:59   ` Jonathan Cameron
@ 2022-08-24 21:13     ` Dave Jiang
  0 siblings, 0 replies; 16+ messages in thread
From: Dave Jiang @ 2022-08-24 21:13 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-cxl, dan.j.williams, vishal.l.verma, ira.weiny, alison.schofield


On 8/24/2022 7:59 AM, Jonathan Cameron wrote:
> On Wed, 17 Aug 2022 14:21:53 -0700
> Dave Jiang <dave.jiang@intel.com> wrote:
>
>> Attach the cxl mock hdm to the port device to allow cxl_interleave_verify()
> I couldn't follow what was going on here, until I realized you've renamed
> this function.  Now cxl_interleave_capable().
>
> Are the tests broken (null dereference) between patch 1 and 3?
>
> Seem like dev_set_drvdata() should move to patch 1.
You are right. Will move.
>
>> to check the interleave configuration. Set the interleave_mask as well
>> to support the new verification code.
>>
>> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
>> 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;
>>   }
>>   
>>
>>

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2022-08-24 21:13 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-17 21:21 [PATCH v4 0/6] Add sanity check for interleave setup Dave Jiang
2022-08-17 21:21 ` [PATCH v4 1/6] cxl: Add check for result of interleave ways plus granularity combo Dave Jiang
2022-08-17 21:21 ` [PATCH v4 2/6] cxl: Add CXL spec v3.0 interleave support Dave Jiang
2022-08-24 14:46   ` Jonathan Cameron
2022-08-24 21:03     ` Dave Jiang
2022-08-24 14:56   ` Jonathan Cameron
2022-08-24 21:04     ` Dave Jiang
2022-08-17 21:21 ` [PATCH v4 3/6] tools/testing/cxl: Add interleave check support to mock cxl port device Dave Jiang
2022-08-24 14:59   ` Jonathan Cameron
2022-08-24 21:13     ` Dave Jiang
2022-08-17 21:21 ` [PATCH v4 4/6] cxl: change cxl_port_attribute_groups naming to avoid confusion Dave Jiang
2022-08-24 14:48   ` Jonathan Cameron
2022-08-17 21:22 ` [PATCH v4 5/6] cxl: export interleave address mask as port sysfs attribute Dave Jiang
2022-08-24 14:34   ` Jonathan Cameron
2022-08-17 21:22 ` [PATCH v4 6/6] cxl: export intereleave capability " Dave Jiang
2022-08-24 14:28   ` Jonathan Cameron

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.