| Message ID | 20220127212911.127741-4-ben.widawsky@intel.com |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Unify meaning of interleave attributes | expand |
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.
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
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(-)