| Message ID | 165999282258.493131.2782730417677035484.stgit@djiang5-desk4.jf.intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add sanity check for interleave setup | expand |
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 | 4 ++++ > drivers/cxl/cxl.h | 2 ++ > drivers/cxl/cxlmem.h | 29 +++++++++++++++++++++++++++++ > 3 files changed, 35 insertions(+) > > diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c > index 8143e2615957..50ff7387e425 100644 > --- a/drivers/cxl/core/hdm.c > +++ b/drivers/cxl/core/hdm.c > @@ -80,6 +80,10 @@ 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); > + if (FIELD_GET(CXL_HDM_DECODER_INTERLEAVE_3_6_12_WAY, hdm_cap)) > + cxlhdm->interleave_3_6_12 = true; > + if (FIELD_GET(CXL_HDM_DECODER_INTERLEAVE_16_WAY, hdm_cap)) > + cxlhdm->interleave_16 = true; > } > > static void __iomem *map_hdm_decoder_regs(struct cxl_port *port, > diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h > index 275979fbd15a..db9631d09dd0 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 d5f872ca62f9..9b4b23b3b78a 100644 > --- a/drivers/cxl/cxlmem.h > +++ b/drivers/cxl/cxlmem.h > @@ -398,9 +398,35 @@ struct cxl_hdm { > unsigned int decoder_count; > unsigned int target_count; > unsigned int interleave_mask; > + bool interleave_3_6_12; > + bool interleave_16; > struct cxl_port *port; > }; > > +static inline bool valid_interleave(struct cxl_hdm *cxlhdm, u8 iw) > +{ > + switch (iw) { > + case CXL_INTERLEAVE_1_WAY: > + case CXL_INTERLEAVE_2_WAY: > + case CXL_INTERLEAVE_4_WAY: > + case CXL_INTERLEAVE_8_WAY: > + return true; > + case CXL_INTERLEAVE_16_WAY: > + if (!cxlhdm->interleave_16) > + return false; > + return true; > + case CXL_INTERLEAVE_3_WAY: > + case CXL_INTERLEAVE_6_WAY: > + case CXL_INTERLEAVE_12_WAY: > + if (!cxlhdm->interleave_3_6_12) > + return false; > + return true; > + default: > + }; > + > + return false; I'd prefer something more compact like test_bit(iw, &port->interleave_cap) ...where interleave_cap is just a mask with bits 1,2,3,4,6,8,12,16 optionally set.
On 8/9/2022 9:20 AM, Dan Williams wrote: > 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 | 4 ++++ >> drivers/cxl/cxl.h | 2 ++ >> drivers/cxl/cxlmem.h | 29 +++++++++++++++++++++++++++++ >> 3 files changed, 35 insertions(+) >> >> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c >> index 8143e2615957..50ff7387e425 100644 >> --- a/drivers/cxl/core/hdm.c >> +++ b/drivers/cxl/core/hdm.c >> @@ -80,6 +80,10 @@ 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); >> + if (FIELD_GET(CXL_HDM_DECODER_INTERLEAVE_3_6_12_WAY, hdm_cap)) >> + cxlhdm->interleave_3_6_12 = true; >> + if (FIELD_GET(CXL_HDM_DECODER_INTERLEAVE_16_WAY, hdm_cap)) >> + cxlhdm->interleave_16 = true; >> } >> >> static void __iomem *map_hdm_decoder_regs(struct cxl_port *port, >> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h >> index 275979fbd15a..db9631d09dd0 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 d5f872ca62f9..9b4b23b3b78a 100644 >> --- a/drivers/cxl/cxlmem.h >> +++ b/drivers/cxl/cxlmem.h >> @@ -398,9 +398,35 @@ struct cxl_hdm { >> unsigned int decoder_count; >> unsigned int target_count; >> unsigned int interleave_mask; >> + bool interleave_3_6_12; >> + bool interleave_16; >> struct cxl_port *port; >> }; >> >> +static inline bool valid_interleave(struct cxl_hdm *cxlhdm, u8 iw) >> +{ >> + switch (iw) { >> + case CXL_INTERLEAVE_1_WAY: >> + case CXL_INTERLEAVE_2_WAY: >> + case CXL_INTERLEAVE_4_WAY: >> + case CXL_INTERLEAVE_8_WAY: >> + return true; >> + case CXL_INTERLEAVE_16_WAY: >> + if (!cxlhdm->interleave_16) >> + return false; >> + return true; >> + case CXL_INTERLEAVE_3_WAY: >> + case CXL_INTERLEAVE_6_WAY: >> + case CXL_INTERLEAVE_12_WAY: >> + if (!cxlhdm->interleave_3_6_12) >> + return false; >> + return true; >> + default: >> + }; >> + >> + return false; > I'd prefer something more compact like > > test_bit(iw, &port->interleave_cap) > > ...where interleave_cap is just a mask with bits 1,2,3,4,6,8,12,16 > optionally set. ok
diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c index 8143e2615957..50ff7387e425 100644 --- a/drivers/cxl/core/hdm.c +++ b/drivers/cxl/core/hdm.c @@ -80,6 +80,10 @@ 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); + if (FIELD_GET(CXL_HDM_DECODER_INTERLEAVE_3_6_12_WAY, hdm_cap)) + cxlhdm->interleave_3_6_12 = true; + if (FIELD_GET(CXL_HDM_DECODER_INTERLEAVE_16_WAY, hdm_cap)) + cxlhdm->interleave_16 = true; } static void __iomem *map_hdm_decoder_regs(struct cxl_port *port, diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h index 275979fbd15a..db9631d09dd0 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 d5f872ca62f9..9b4b23b3b78a 100644 --- a/drivers/cxl/cxlmem.h +++ b/drivers/cxl/cxlmem.h @@ -398,9 +398,35 @@ struct cxl_hdm { unsigned int decoder_count; unsigned int target_count; unsigned int interleave_mask; + bool interleave_3_6_12; + bool interleave_16; struct cxl_port *port; }; +static inline bool valid_interleave(struct cxl_hdm *cxlhdm, u8 iw) +{ + switch (iw) { + case CXL_INTERLEAVE_1_WAY: + case CXL_INTERLEAVE_2_WAY: + case CXL_INTERLEAVE_4_WAY: + case CXL_INTERLEAVE_8_WAY: + return true; + case CXL_INTERLEAVE_16_WAY: + if (!cxlhdm->interleave_16) + return false; + return true; + case CXL_INTERLEAVE_3_WAY: + case CXL_INTERLEAVE_6_WAY: + case CXL_INTERLEAVE_12_WAY: + if (!cxlhdm->interleave_3_6_12) + return false; + return true; + default: + }; + + return false; +} + static inline int cxl_interleave_verify(struct cxl_port *port, int ways, int granularity) { @@ -421,6 +447,9 @@ static inline int cxl_interleave_verify(struct cxl_port *port, if (iw == 0) return 0; + if (!valid_interleave(cxlhdm, iw)) + return -EINVAL; + if (iw < CXL_INTERLEAVE_3_WAY) addr_mask = GENMASK(ig + 8 + iw - 1, ig + 8); else
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 | 4 ++++ drivers/cxl/cxl.h | 2 ++ drivers/cxl/cxlmem.h | 29 +++++++++++++++++++++++++++++ 3 files changed, 35 insertions(+)