* [PATCH 1/4] cxl/acpi: Store interleave granularity absolutely
2022-01-27 21:29 [PATCH 0/4] Unify meaning of interleave attributes Ben Widawsky
@ 2022-01-27 21:29 ` Ben Widawsky
2022-01-27 22:46 ` Dan Williams
2022-01-27 21:29 ` [PATCH 2/4] cxl/core: Add more decoder attributes to sysfs Ben Widawsky
` (3 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Ben Widawsky @ 2022-01-27 21:29 UTC (permalink / raw)
To: linux-cxl
Cc: patches, Ben Widawsky, Alison Schofield, Dan Williams, Ira Weiny,
Jonathan Cameron, Vishal Verma
This matches the usages everywhere else in the driver and matches the
existing kernel documentation defined for CXL decoders.
Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
---
drivers/cxl/acpi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
index f64d98bfcb3b..f99cad8350fd 100644
--- a/drivers/cxl/acpi.c
+++ b/drivers/cxl/acpi.c
@@ -11,7 +11,7 @@
/* Encode defined in CXL 2.0 8.2.5.12.7 HDM Decoder Control Register */
#define CFMWS_INTERLEAVE_WAYS(x) (1 << (x)->interleave_ways)
-#define CFMWS_INTERLEAVE_GRANULARITY(x) ((x)->granularity + 8)
+#define CFMWS_INTERLEAVE_GRANULARITY(x) (1 << ((x)->granularity + 8))
static unsigned long cfmws_to_decoder_flags(int restrictions)
{
--
2.35.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/4] cxl/acpi: Store interleave granularity absolutely
2022-01-27 21:29 ` [PATCH 1/4] cxl/acpi: Store interleave granularity absolutely Ben Widawsky
@ 2022-01-27 22:46 ` Dan Williams
0 siblings, 0 replies; 9+ messages in thread
From: Dan Williams @ 2022-01-27 22:46 UTC (permalink / raw)
To: Ben Widawsky
Cc: linux-cxl, patches, Alison Schofield, Ira Weiny,
Jonathan Cameron, Vishal Verma
On Thu, Jan 27, 2022 at 1:29 PM Ben Widawsky <ben.widawsky@intel.com> wrote:
>
> This matches the usages everywhere else in the driver and matches the
> existing kernel documentation defined for CXL decoders.
>
> Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
> ---
> drivers/cxl/acpi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> index f64d98bfcb3b..f99cad8350fd 100644
> --- a/drivers/cxl/acpi.c
> +++ b/drivers/cxl/acpi.c
> @@ -11,7 +11,7 @@
>
> /* Encode defined in CXL 2.0 8.2.5.12.7 HDM Decoder Control Register */
> #define CFMWS_INTERLEAVE_WAYS(x) (1 << (x)->interleave_ways)
> -#define CFMWS_INTERLEAVE_GRANULARITY(x) ((x)->granularity + 8)
> +#define CFMWS_INTERLEAVE_GRANULARITY(x) (1 << ((x)->granularity + 8))
Looks good.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/4] cxl/core: Add more decoder attributes to sysfs
2022-01-27 21:29 [PATCH 0/4] Unify meaning of interleave attributes Ben Widawsky
2022-01-27 21:29 ` [PATCH 1/4] cxl/acpi: Store interleave granularity absolutely Ben Widawsky
@ 2022-01-27 21:29 ` Ben Widawsky
2022-01-27 22:45 ` Dan Williams
2022-01-27 21:29 ` [PATCH 3/4] cxl/core: Extract IW/IG decoding Ben Widawsky
` (2 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Ben Widawsky @ 2022-01-27 21:29 UTC (permalink / raw)
To: linux-cxl
Cc: patches, Ben Widawsky, Alison Schofield, Dan Williams, Ira Weiny,
Jonathan Cameron, Vishal Verma
The decoder attributes are consumed by userspace in order to program and
verify the CXL topology.
An example of the new attributes is with x1 and 8192 granularity:
archlinux ~ # cat /sys/bus/cxl/devices/decoder0.0/interleave_{ways,granularity}
1
8192
Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
---
drivers/cxl/core/port.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index f49783d8c845..631dec0fa79e 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -99,6 +99,25 @@ static ssize_t size_show(struct device *dev, struct device_attribute *attr,
}
static DEVICE_ATTR_RO(size);
+static ssize_t interleave_ways_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct cxl_decoder *cxld = to_cxl_decoder(dev);
+
+ return sysfs_emit(buf, "%d\n", cxld->interleave_ways);
+}
+static DEVICE_ATTR_RO(interleave_ways);
+
+static ssize_t interleave_granularity_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct cxl_decoder *cxld = to_cxl_decoder(dev);
+
+ return sysfs_emit(buf, "%d\n", cxld->interleave_granularity);
+}
+static DEVICE_ATTR_RO(interleave_granularity);
+
#define CXL_DECODER_FLAG_ATTR(name, flag) \
static ssize_t name##_show(struct device *dev, \
struct device_attribute *attr, char *buf) \
@@ -186,6 +205,8 @@ static struct attribute *cxl_decoder_base_attrs[] = {
&dev_attr_start.attr,
&dev_attr_size.attr,
&dev_attr_locked.attr,
+ &dev_attr_interleave_ways.attr,
+ &dev_attr_interleave_granularity.attr,
NULL,
};
--
2.35.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/4] cxl/core: Add more decoder attributes to sysfs
2022-01-27 21:29 ` [PATCH 2/4] cxl/core: Add more decoder attributes to sysfs Ben Widawsky
@ 2022-01-27 22:45 ` Dan Williams
0 siblings, 0 replies; 9+ messages in thread
From: Dan Williams @ 2022-01-27 22:45 UTC (permalink / raw)
To: Ben Widawsky
Cc: linux-cxl, patches, Alison Schofield, Ira Weiny,
Jonathan Cameron, Vishal Verma
On Thu, Jan 27, 2022 at 1:29 PM Ben Widawsky <ben.widawsky@intel.com> wrote:
>
> The decoder attributes are consumed by userspace in order to program and
> verify the CXL topology.
>
> An example of the new attributes is with x1 and 8192 granularity:
> archlinux ~ # cat /sys/bus/cxl/devices/decoder0.0/interleave_{ways,granularity}
> 1
> 8192
>
> Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
> ---
> drivers/cxl/core/port.c | 21 +++++++++++++++++++++
Looks good, just missing the update to Documentation/ABI/testing/sysfs-bus-cxl
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/4] cxl/core: Extract IW/IG decoding
2022-01-27 21:29 [PATCH 0/4] Unify meaning of interleave attributes Ben Widawsky
2022-01-27 21:29 ` [PATCH 1/4] cxl/acpi: Store interleave granularity absolutely Ben Widawsky
2022-01-27 21:29 ` [PATCH 2/4] cxl/core: Add more decoder attributes to sysfs Ben Widawsky
@ 2022-01-27 21:29 ` Ben Widawsky
2022-01-27 23:01 ` Dan Williams
2022-01-27 21:29 ` [PATCH 4/4] cxl/acpi: Use common " Ben Widawsky
2022-01-28 10:15 ` [PATCH 0/4] Unify meaning of interleave attributes Jonathan Cameron
4 siblings, 1 reply; 9+ messages in thread
From: Ben Widawsky @ 2022-01-27 21:29 UTC (permalink / raw)
To: linux-cxl
Cc: patches, Ben Widawsky, Alison Schofield, Dan Williams, Ira Weiny,
Jonathan Cameron, Vishal Verma
Interleave granularity and ways have specification defined encodings.
Extracting this functionality into the common header file allows other
consumers to make use of it.
Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
---
I'm in favor of making these helps part of UABI. Having userspace and
kernel disagree (because of differing spec versions for example) would
be difficult to manage.
Thoughts?
---
drivers/cxl/core/hdm.c | 11 ++---------
drivers/cxl/cxl.h | 17 +++++++++++++++++
2 files changed, 19 insertions(+), 9 deletions(-)
diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
index 4955ba16c9c8..a28369f264da 100644
--- a/drivers/cxl/core/hdm.c
+++ b/drivers/cxl/core/hdm.c
@@ -133,21 +133,14 @@ static int to_interleave_granularity(u32 ctrl)
{
int val = FIELD_GET(CXL_HDM_DECODER0_CTRL_IG_MASK, ctrl);
- return 256 << val;
+ return cxl_to_interleave_granularity(val);
}
static int to_interleave_ways(u32 ctrl)
{
int val = FIELD_GET(CXL_HDM_DECODER0_CTRL_IW_MASK, ctrl);
- switch (val) {
- case 0 ... 4:
- return 1 << val;
- case 8 ... 10:
- return 3 << (val - 8);
- default:
- return 0;
- }
+ return cxl_to_interleave_ways(val);
}
static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 962629c5775f..13fb06849199 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -64,6 +64,23 @@ static inline int cxl_hdm_decoder_count(u32 cap_hdr)
return val ? val * 2 : 1;
}
+static inline int cxl_to_interleave_granularity(u16 ig)
+{
+ return 256 << ig;
+}
+
+static inline int cxl_to_interleave_ways(u8 eniw)
+{
+ switch (eniw) {
+ case 0 ... 4:
+ return 1 << eniw;
+ case 8 ... 10:
+ return 3 << (eniw - 8);
+ default:
+ return 0;
+ }
+}
+
/* CXL 2.0 8.2.8.1 Device Capabilities Array Register */
#define CXLDEV_CAP_ARRAY_OFFSET 0x0
#define CXLDEV_CAP_ARRAY_CAP_ID 0
--
2.35.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 3/4] cxl/core: Extract IW/IG decoding
2022-01-27 21:29 ` [PATCH 3/4] cxl/core: Extract IW/IG decoding Ben Widawsky
@ 2022-01-27 23:01 ` Dan Williams
0 siblings, 0 replies; 9+ messages in thread
From: Dan Williams @ 2022-01-27 23:01 UTC (permalink / raw)
To: Ben Widawsky
Cc: linux-cxl, patches, Alison Schofield, Ira Weiny,
Jonathan Cameron, Vishal Verma
On Thu, Jan 27, 2022 at 1:29 PM Ben Widawsky <ben.widawsky@intel.com> wrote:
>
> Interleave granularity and ways have specification defined encodings.
> Extracting this functionality into the common header file allows other
> consumers to make use of it.
>
> Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
>
> ---
>
> I'm in favor of making these helps part of UABI. Having userspace and
> kernel disagree (because of differing spec versions for example) would
> be difficult to manage.
>
> Thoughts?
I have no strong objections to adding these to the UABI, but it does
not stop disagreements about the translation if the definition ever
changes. For example, the cxl-cli user tooling is snapshotting a local
copy of include/uapi/linux/cxl_mem.h, not including it directly from
the current system header location. Even then it's relying on sysfs +
raw values to interact with IW and IG, not the encoded ones.
I guess this does help tools like lspci that may want to decode HDM
decoder registers, but there's not a strong pull for that
functionality either as evidenced by the response to:
https://github.com/pciutils/pciutils/pull/59
So, keeping this in drivers/cxl/cxl.h instead of
include/uapi/linux/cxl_mem.h is fine for now.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 4/4] cxl/acpi: Use common IW/IG decoding
2022-01-27 21:29 [PATCH 0/4] Unify meaning of interleave attributes Ben Widawsky
` (2 preceding siblings ...)
2022-01-27 21:29 ` [PATCH 3/4] cxl/core: Extract IW/IG decoding Ben Widawsky
@ 2022-01-27 21:29 ` Ben Widawsky
2022-01-28 10:15 ` [PATCH 0/4] Unify meaning of interleave attributes Jonathan Cameron
4 siblings, 0 replies; 9+ messages in thread
From: Ben Widawsky @ 2022-01-27 21:29 UTC (permalink / raw)
To: linux-cxl
Cc: patches, Ben Widawsky, Alison Schofield, Dan Williams, Ira Weiny,
Jonathan Cameron, Vishal Verma
Now that functionality to decode interleave ways and granularity is in
a common place, use that functionality in the cxl_acpi driver.
Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
---
drivers/cxl/acpi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
index f99cad8350fd..d6dcb2b6af48 100644
--- a/drivers/cxl/acpi.c
+++ b/drivers/cxl/acpi.c
@@ -10,8 +10,8 @@
#include "cxl.h"
/* Encode defined in CXL 2.0 8.2.5.12.7 HDM Decoder Control Register */
-#define CFMWS_INTERLEAVE_WAYS(x) (1 << (x)->interleave_ways)
-#define CFMWS_INTERLEAVE_GRANULARITY(x) (1 << ((x)->granularity + 8))
+#define CFMWS_INTERLEAVE_WAYS(x) (cxl_to_interleave_ways((x)->interleave_ways))
+#define CFMWS_INTERLEAVE_GRANULARITY(x) (cxl_to_interleave_granularity((x)->granularity))
static unsigned long cfmws_to_decoder_flags(int restrictions)
{
--
2.35.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 0/4] Unify meaning of interleave attributes
2022-01-27 21:29 [PATCH 0/4] Unify meaning of interleave attributes Ben Widawsky
` (3 preceding siblings ...)
2022-01-27 21:29 ` [PATCH 4/4] cxl/acpi: Use common " Ben Widawsky
@ 2022-01-28 10:15 ` Jonathan Cameron
4 siblings, 0 replies; 9+ messages in thread
From: Jonathan Cameron @ 2022-01-28 10:15 UTC (permalink / raw)
To: Ben Widawsky
Cc: linux-cxl, patches, Alison Schofield, Dan Williams, Ira Weiny,
Vishal Verma
On Thu, 27 Jan 2022 13:29:07 -0800
Ben Widawsky <ben.widawsky@intel.com> wrote:
> Interleave granularity and interleave ways can be represented either as
> absolute numbers, or as their encoded values from the CXL 2.0 specification.
> When region configuration and programming is created, it becomes important to
> differentiate these two things concretely.
Other than Dan's point about sysfs docs, this looks good to me so
with the assumption that those will be fine:
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Great to tidy this up so we have one 'obvious' answer for how
these are stored.
Thanks,
Jonathan
>
> Ben Widawsky (4):
> cxl/acpi: Store interleave granularity absolutely
> cxl/core: Add more decoder attributes to sysfs
> cxl/core: Extract IW/IG decoding
> cxl/acpi: Use common IW/IG decoding
>
> drivers/cxl/acpi.c | 4 ++--
> drivers/cxl/core/hdm.c | 11 ++---------
> drivers/cxl/core/port.c | 21 +++++++++++++++++++++
> drivers/cxl/cxl.h | 17 +++++++++++++++++
> 4 files changed, 42 insertions(+), 11 deletions(-)
>
>
> base-commit: e783362eb54cd99b2cac8b3a9aeac942e6f6ac07
> prerequisite-patch-id: 90de8aefc2999f55c7534fefa971d95653c4220c
> prerequisite-patch-id: 32a5b56d83bf3372b6ed4b40f621eafb33a7201b
> prerequisite-patch-id: f827831bb7a23e0789d16d7b8979b165253c6301
> prerequisite-patch-id: 08b8febd42d3ab508b618937473807e553589e36
> prerequisite-patch-id: 18049f47c948582c1dc26348d9765c934eb82a75
> prerequisite-patch-id: 8f66d52af297449fa007a0ba963c5239b153ef5b
> prerequisite-patch-id: 3e2e86cbc2631b99c1b5c0179f35799d3df31f91
> prerequisite-patch-id: b88becd4997320a34e918cdef1b620e6dea14917
> prerequisite-patch-id: c61df81018f2a93b87d10965b418afa659d9d6d6
> prerequisite-patch-id: 73b31df62e00bb7af7082e2ca4d40023a7962abd
> prerequisite-patch-id: 207abfcd5028c41df8875ee795a8ab697cd7c688
> prerequisite-patch-id: 26978f021b3b0f4a6734ef8c0100c724dc88742e
> prerequisite-patch-id: bf229ca5aab5c5dffe69ba5b9380749a66cf20ba
> prerequisite-patch-id: 20ebefe1acfdecf184d048cb605368e1863646c1
> prerequisite-patch-id: f34c26e902dd868dc1c3ef8ba8246cc063cf991a
> prerequisite-patch-id: bcc59db1c6528244b649ced35eab015699c410fa
> prerequisite-patch-id: 2f9f6cfbd6b73a563498c6b6d721bbc169a0a414
> prerequisite-patch-id: dc8fb216dc8ff4f813bfc689273d9c5f5124e789
> prerequisite-patch-id: da83e8074d339426c886c481070366afb189b561
> prerequisite-patch-id: 501fe71f19065ba9f31cabd86756fedda853c414
> prerequisite-patch-id: ceeef31c2ca85a426d507563b886347d28acc322
> prerequisite-patch-id: f876c09942ae5a3223a36329c23262a05b2669f4
> prerequisite-patch-id: 44fa61c5569614c8d9df854cde6fedfc2bc78c12
> prerequisite-patch-id: 04ad90e1bbb5646125c4633fbe5341f572bc9548
> prerequisite-patch-id: f4dbf89d99917f50c30e1ee56bfeff8d8dd6b0f3
> prerequisite-patch-id: 2d7c3aacefcb8133897e3256ed6f76952555c2f1
> prerequisite-patch-id: 7454df4bdb07381f02717845eb3b17011a89ab18
> prerequisite-patch-id: 52ec0dfd506bb6a3f8d11a914cfc7320193a6445
> prerequisite-patch-id: 9de14fa54cfba412e09d7b41f392c0f6d55d6a01
> prerequisite-patch-id: ae39a482c2067a1f04baee5ce9131901e6d359ec
> prerequisite-patch-id: 446240d2ed24d9e55ac9edfc65b511495659464a
> prerequisite-patch-id: ba6bf6450e47df5e95e2fb1780d9edd126bc0eb2
> prerequisite-patch-id: 3c0865b6dd062e677ef8e160e14f823622eafb9f
> prerequisite-patch-id: 4503f5507cbdeb0770b420b4c26d87be2b173813
> prerequisite-patch-id: c5a8cbda77c95b052040770eca0dc5b99876dc66
> prerequisite-patch-id: e064003a6c48131fac401d9a48d4d6204fea6123
> prerequisite-patch-id: b4c7213971c981dd5ca0fda992643a7c61548fef
> prerequisite-patch-id: 2bd09e27f8a8df144a8ad386822390c87ef46ec5
> prerequisite-patch-id: 60b3fafbd3bfa225405a6762bdb6b89c044b0b86
> prerequisite-patch-id: 620068ae417bf0784809107e0dae3ec9793632df
> prerequisite-patch-id: c3415fe92e29cd4afc508f8caf31cb914be09261
> prerequisite-patch-id: 4c01f305244036afa9aaa918c8215659327dd0f3
^ permalink raw reply [flat|nested] 9+ messages in thread