[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <34a30d8d-f4e2-4c3b-8386-d07f7b8cfaa1@intel.com>
Date: Thu, 27 Feb 2025 08:21:57 -0700
From: Dave Jiang <dave.jiang@...el.com>
To: Li Ming <ming.li@...omail.com>, dave@...olabs.net,
jonathan.cameron@...wei.com, alison.schofield@...el.com,
vishal.l.verma@...el.com, ira.weiny@...el.com, dan.j.williams@...el.com
Cc: linux-cxl@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 1/1] cxl/hdm: Verify HDM decoder capabilities after
parsing
On 2/27/25 3:32 AM, Li Ming wrote:
> devm_cxl_setup_hdm() only checks if decoder_count is 0 after parsing HDM
> decoder capability, But according to the implementation of
> cxl_hdm_decoder_count(), cxlhdm->decoder_count will never be 0.
>
> Per CXL specification, the values ranges of decoder_count and
> target_count are limited. Adding a checking for the values of them
> in case hardware initialized them with wrong values.
>
> Signed-off-by: Li Ming <ming.li@...omail.com>
> ---
> base-commit: 22eea823f69ae39dc060c4027e8d1470803d2e49 cxl/next
> ---
> drivers/cxl/core/hdm.c | 31 ++++++++++++++++++++++++++++++-
> 1 file changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 70cae4ebf8a4..a98191867c22 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -138,6 +138,34 @@ static bool should_emulate_decoders(struct cxl_endpoint_dvsec_info *info)
> return true;
> }
>
> +static int cxlhdm_decoder_caps_verify(struct cxl_hdm *cxlhdm)
> +{
> + /*
> + * CXL r3.2 section 8.2.4.20.1
> + * CXL devices shall not advertise more than 10 decoders,
> + * CXL switches and HBs may advertise up to 32 decoders.
> + */
> + if (is_cxl_endpoint(cxlhdm->port) && cxlhdm->decoder_count > 10)
#define the limit please
> + return -EINVAL;
> + else if (cxlhdm->decoder_count > 32)
same here
DJ
> + return -EINVAL;
> +
> + /*
> + * CXL r3.2 section 8.2.4.20.1
> + * target count is applicable only to CXL upstream port and HB.
> + * The number of target ports each decoder supports should be
> + * one of the numbers 1, 2, 4 or 8.
> + */
> + if (!is_cxl_endpoint(cxlhdm->port) &&
> + cxlhdm->target_count != 1 &&
> + cxlhdm->target_count != 2 &&
> + cxlhdm->target_count != 4 &&
> + cxlhdm->target_count != 8)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> /**
> * devm_cxl_setup_hdm - map HDM decoder component registers
> * @port: cxl_port to map
> @@ -182,7 +210,8 @@ struct cxl_hdm *devm_cxl_setup_hdm(struct cxl_port *port,
> }
>
> parse_hdm_decoder_caps(cxlhdm);
> - if (cxlhdm->decoder_count == 0) {
> + rc = cxlhdm_decoder_caps_verify(cxlhdm);
> + if (rc) {
> dev_err(dev, "Spec violation. Caps invalid\n");
> return ERR_PTR(-ENXIO);
> }
Powered by blists - more mailing lists