* [PATCH] cxl: check decoder count for end device
@ 2022-09-15 18:11 Dave Jiang
2022-10-11 16:45 ` Jonathan Cameron
2022-10-21 21:17 ` Dan Williams
0 siblings, 2 replies; 5+ messages in thread
From: Dave Jiang @ 2022-09-15 18:11 UTC (permalink / raw)
To: linux-cxl
Cc: alison.schofield, vishal.l.verma, ira.weiny, bwidawsk, dan.j.williams
CXL spec rev3.0 8.2.4.19.1 added definition for up to 32 decoders. It also
indicates that for devices, only 10 decoders should be advertised. Add
check on number of decoders greater than 10 for devices and reset to 10 to
force spec compliance.
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
drivers/cxl/core/hdm.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
index d1d2caea5c62..1919d99d157e 100644
--- a/drivers/cxl/core/hdm.c
+++ b/drivers/cxl/core/hdm.c
@@ -71,9 +71,23 @@ EXPORT_SYMBOL_NS_GPL(devm_cxl_add_passthrough_decoder, CXL);
static void parse_hdm_decoder_caps(struct cxl_hdm *cxlhdm)
{
u32 hdm_cap;
+ int decoder_count;
+ struct device *dev = &cxlhdm->port->dev;
hdm_cap = readl(cxlhdm->regs.hdm_decoder + CXL_HDM_DECODER_CAP_OFFSET);
- cxlhdm->decoder_count = cxl_hdm_decoder_count(hdm_cap);
+ decoder_count = cxl_hdm_decoder_count(hdm_cap);
+ /*
+ * CXL spec rev3.0 8.2.4.19.1 indicates CXL devices shall not advertise
+ * more than 10 decoders. Switches and Host Bridges may advertise up to
+ * 32 decoders. Set the decoders to 10 for devices if more than 10 are
+ * found.
+ */
+ if (is_cxl_endpoint(cxlhdm->port) && decoder_count > 10) {
+ dev_warn(dev, "Reset decoders count (%d) to 10, spec violation!\n",
+ decoder_count);
+ decoder_count = 10;
+ }
+ cxlhdm->decoder_count = decoder_count;
cxlhdm->target_count =
FIELD_GET(CXL_HDM_DECODER_TARGET_COUNT_MASK, hdm_cap);
if (FIELD_GET(CXL_HDM_DECODER_INTERLEAVE_11_8, hdm_cap))
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] cxl: check decoder count for end device
2022-09-15 18:11 [PATCH] cxl: check decoder count for end device Dave Jiang
@ 2022-10-11 16:45 ` Jonathan Cameron
2022-10-11 16:56 ` Dave Jiang
2022-10-21 21:17 ` Dan Williams
1 sibling, 1 reply; 5+ messages in thread
From: Jonathan Cameron @ 2022-10-11 16:45 UTC (permalink / raw)
To: Dave Jiang
Cc: linux-cxl, alison.schofield, vishal.l.verma, ira.weiny, bwidawsk,
dan.j.williams
On Thu, 15 Sep 2022 11:11:07 -0700
Dave Jiang <dave.jiang@intel.com> wrote:
> CXL spec rev3.0 8.2.4.19.1 added definition for up to 32 decoders. It also
> indicates that for devices, only 10 decoders should be advertised. Add
> check on number of decoders greater than 10 for devices and reset to 10 to
> force spec compliance.
>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Hi Dave
Seems reasonable. I'm a bit curious to whether there is an actual problem if
a device is spec non compliant and advertises more decoders?
Either way does no harm and might let us know something fishy is happening.
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
> drivers/cxl/core/hdm.c | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index d1d2caea5c62..1919d99d157e 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -71,9 +71,23 @@ EXPORT_SYMBOL_NS_GPL(devm_cxl_add_passthrough_decoder, CXL);
> static void parse_hdm_decoder_caps(struct cxl_hdm *cxlhdm)
> {
> u32 hdm_cap;
> + int decoder_count;
> + struct device *dev = &cxlhdm->port->dev;
>
> hdm_cap = readl(cxlhdm->regs.hdm_decoder + CXL_HDM_DECODER_CAP_OFFSET);
> - cxlhdm->decoder_count = cxl_hdm_decoder_count(hdm_cap);
> + decoder_count = cxl_hdm_decoder_count(hdm_cap);
> + /*
> + * CXL spec rev3.0 8.2.4.19.1 indicates CXL devices shall not advertise
> + * more than 10 decoders. Switches and Host Bridges may advertise up to
> + * 32 decoders. Set the decoders to 10 for devices if more than 10 are
> + * found.
> + */
> + if (is_cxl_endpoint(cxlhdm->port) && decoder_count > 10) {
> + dev_warn(dev, "Reset decoders count (%d) to 10, spec violation!\n",
> + decoder_count);
> + decoder_count = 10;
> + }
> + cxlhdm->decoder_count = decoder_count;
> cxlhdm->target_count =
> FIELD_GET(CXL_HDM_DECODER_TARGET_COUNT_MASK, hdm_cap);
> if (FIELD_GET(CXL_HDM_DECODER_INTERLEAVE_11_8, hdm_cap))
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] cxl: check decoder count for end device
2022-10-11 16:45 ` Jonathan Cameron
@ 2022-10-11 16:56 ` Dave Jiang
0 siblings, 0 replies; 5+ messages in thread
From: Dave Jiang @ 2022-10-11 16:56 UTC (permalink / raw)
To: Jonathan Cameron
Cc: linux-cxl, alison.schofield, vishal.l.verma, ira.weiny, bwidawsk,
dan.j.williams
On 10/11/2022 9:45 AM, Jonathan Cameron wrote:
> On Thu, 15 Sep 2022 11:11:07 -0700
> Dave Jiang <dave.jiang@intel.com> wrote:
>
>> CXL spec rev3.0 8.2.4.19.1 added definition for up to 32 decoders. It also
>> indicates that for devices, only 10 decoders should be advertised. Add
>> check on number of decoders greater than 10 for devices and reset to 10 to
>> force spec compliance.
>>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> Hi Dave
>
> Seems reasonable. I'm a bit curious to whether there is an actual problem if
> a device is spec non compliant and advertises more decoders?
Not that I'm aware of. Just being cautious.
>
> Either way does no harm and might let us know something fishy is happening.
>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Thanks!
>
>> ---
>> drivers/cxl/core/hdm.c | 16 +++++++++++++++-
>> 1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
>> index d1d2caea5c62..1919d99d157e 100644
>> --- a/drivers/cxl/core/hdm.c
>> +++ b/drivers/cxl/core/hdm.c
>> @@ -71,9 +71,23 @@ EXPORT_SYMBOL_NS_GPL(devm_cxl_add_passthrough_decoder, CXL);
>> static void parse_hdm_decoder_caps(struct cxl_hdm *cxlhdm)
>> {
>> u32 hdm_cap;
>> + int decoder_count;
>> + struct device *dev = &cxlhdm->port->dev;
>>
>> hdm_cap = readl(cxlhdm->regs.hdm_decoder + CXL_HDM_DECODER_CAP_OFFSET);
>> - cxlhdm->decoder_count = cxl_hdm_decoder_count(hdm_cap);
>> + decoder_count = cxl_hdm_decoder_count(hdm_cap);
>> + /*
>> + * CXL spec rev3.0 8.2.4.19.1 indicates CXL devices shall not advertise
>> + * more than 10 decoders. Switches and Host Bridges may advertise up to
>> + * 32 decoders. Set the decoders to 10 for devices if more than 10 are
>> + * found.
>> + */
>> + if (is_cxl_endpoint(cxlhdm->port) && decoder_count > 10) {
>> + dev_warn(dev, "Reset decoders count (%d) to 10, spec violation!\n",
>> + decoder_count);
>> + decoder_count = 10;
>> + }
>> + cxlhdm->decoder_count = decoder_count;
>> cxlhdm->target_count =
>> FIELD_GET(CXL_HDM_DECODER_TARGET_COUNT_MASK, hdm_cap);
>> if (FIELD_GET(CXL_HDM_DECODER_INTERLEAVE_11_8, hdm_cap))
>>
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] cxl: check decoder count for end device
2022-09-15 18:11 [PATCH] cxl: check decoder count for end device Dave Jiang
2022-10-11 16:45 ` Jonathan Cameron
@ 2022-10-21 21:17 ` Dan Williams
2022-10-21 21:24 ` Dave Jiang
1 sibling, 1 reply; 5+ messages in thread
From: Dan Williams @ 2022-10-21 21:17 UTC (permalink / raw)
To: Dave Jiang, linux-cxl
Cc: alison.schofield, vishal.l.verma, ira.weiny, bwidawsk, dan.j.williams
Dave Jiang wrote:
> CXL spec rev3.0 8.2.4.19.1 added definition for up to 32 decoders. It also
> indicates that for devices, only 10 decoders should be advertised. Add
> check on number of decoders greater than 10 for devices and reset to 10 to
> force spec compliance.
>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
> drivers/cxl/core/hdm.c | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index d1d2caea5c62..1919d99d157e 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -71,9 +71,23 @@ EXPORT_SYMBOL_NS_GPL(devm_cxl_add_passthrough_decoder, CXL);
> static void parse_hdm_decoder_caps(struct cxl_hdm *cxlhdm)
> {
> u32 hdm_cap;
> + int decoder_count;
> + struct device *dev = &cxlhdm->port->dev;
>
> hdm_cap = readl(cxlhdm->regs.hdm_decoder + CXL_HDM_DECODER_CAP_OFFSET);
> - cxlhdm->decoder_count = cxl_hdm_decoder_count(hdm_cap);
> + decoder_count = cxl_hdm_decoder_count(hdm_cap);
> + /*
> + * CXL spec rev3.0 8.2.4.19.1 indicates CXL devices shall not advertise
> + * more than 10 decoders. Switches and Host Bridges may advertise up to
> + * 32 decoders. Set the decoders to 10 for devices if more than 10 are
> + * found.
> + */
> + if (is_cxl_endpoint(cxlhdm->port) && decoder_count > 10) {
> + dev_warn(dev, "Reset decoders count (%d) to 10, spec violation!\n",
> + decoder_count);
> + decoder_count = 10;
I agree with the reporting, but not the limitation. The Robustness
Principle says if allowing it does no harm, then let it through.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] cxl: check decoder count for end device
2022-10-21 21:17 ` Dan Williams
@ 2022-10-21 21:24 ` Dave Jiang
0 siblings, 0 replies; 5+ messages in thread
From: Dave Jiang @ 2022-10-21 21:24 UTC (permalink / raw)
To: Dan Williams, linux-cxl
Cc: alison.schofield, vishal.l.verma, ira.weiny, bwidawsk
On 10/21/2022 2:17 PM, Dan Williams wrote:
> Dave Jiang wrote:
>> CXL spec rev3.0 8.2.4.19.1 added definition for up to 32 decoders. It also
>> indicates that for devices, only 10 decoders should be advertised. Add
>> check on number of decoders greater than 10 for devices and reset to 10 to
>> force spec compliance.
>>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>> ---
>> drivers/cxl/core/hdm.c | 16 +++++++++++++++-
>> 1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
>> index d1d2caea5c62..1919d99d157e 100644
>> --- a/drivers/cxl/core/hdm.c
>> +++ b/drivers/cxl/core/hdm.c
>> @@ -71,9 +71,23 @@ EXPORT_SYMBOL_NS_GPL(devm_cxl_add_passthrough_decoder, CXL);
>> static void parse_hdm_decoder_caps(struct cxl_hdm *cxlhdm)
>> {
>> u32 hdm_cap;
>> + int decoder_count;
>> + struct device *dev = &cxlhdm->port->dev;
>>
>> hdm_cap = readl(cxlhdm->regs.hdm_decoder + CXL_HDM_DECODER_CAP_OFFSET);
>> - cxlhdm->decoder_count = cxl_hdm_decoder_count(hdm_cap);
>> + decoder_count = cxl_hdm_decoder_count(hdm_cap);
>> + /*
>> + * CXL spec rev3.0 8.2.4.19.1 indicates CXL devices shall not advertise
>> + * more than 10 decoders. Switches and Host Bridges may advertise up to
>> + * 32 decoders. Set the decoders to 10 for devices if more than 10 are
>> + * found.
>> + */
>> + if (is_cxl_endpoint(cxlhdm->port) && decoder_count > 10) {
>> + dev_warn(dev, "Reset decoders count (%d) to 10, spec violation!\n",
>> + decoder_count);
>> + decoder_count = 10;
> I agree with the reporting, but not the limitation. The Robustness
> Principle says if allowing it does no harm, then let it through.
Ok I'll remove the reassigment.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-10-21 21:24 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-15 18:11 [PATCH] cxl: check decoder count for end device Dave Jiang
2022-10-11 16:45 ` Jonathan Cameron
2022-10-11 16:56 ` Dave Jiang
2022-10-21 21:17 ` Dan Williams
2022-10-21 21:24 ` Dave Jiang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).