Re: [PATCH v4 1/4] cxl/hdm: Reject switch decoder interleave ways that overflow targets

On Tue, Jul 21, 2026 at 02:27:27PM +0800, Li Ming wrote:
> 
> 在 2026/7/21 09:59, Alison Schofield 写道:

Thanks for the review Ming,

snip

> > diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> > index 0c80b76a5f9b..d81df45d8005 100644
> > --- a/drivers/cxl/core/hdm.c
> > +++ b/drivers/cxl/core/hdm.c
> > @@ -76,6 +76,8 @@ static void parse_hdm_decoder_caps(struct cxl_hdm *cxlhdm)
> >   	hdm_cap = readl(cxlhdm->regs.hdm_decoder + CXL_HDM_DECODER_CAP_OFFSET);
> >   	cxlhdm->decoder_count = cxl_hdm_decoder_count(hdm_cap);
> > +
> > +	/* target_count is a direct count (1h..8h), not 0-based like 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))
> > @@ -1084,6 +1086,37 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
> >   		cxld->interleave_ways, cxld->interleave_granularity);
> >   	if (!cxled) {
> > +		struct cxl_switch_decoder *cxlsd =
> > +			to_cxl_switch_decoder(&cxld->dev);
> > +
> > +		if (cxlsd->nr_targets < 1) {
> > +			dev_err(&port->dev,
> > +				"decoder%d.%d: reports zero targets\n",
> > +				port->id, cxld->id);
> > +			return -ENXIO;
> > +		}
> I noticed cxl_switch_decoder_init() has a check for the value of given
> nr_targets before setting cxlsd->nr_targets, maybe we can move this check
> into cxl_switch_decoder_init()?

Done in v5.

Jonathan asked for same treatment of the target list capacity check,
so I moved that up to the port in devm_cxl_enumerate_decoders().


> > +
> > +		if (!committed) {
> > +			/* Ignore interleave ways until commit */
> > +			cxld->interleave_ways = 1;
> > +			return 0;
> > +		}
> The following part is related to a committed decoder( including target_map
> array updating), like my comment in patch #2, maybe it is worth a cleanup.
> > +

No, that part is not committed-only. Enumeration reads the interleave
fields for every decoder, and interleave_ways and interleave_granularity
report them to userspace. Gating the read on committed changes what
those attributes show, so patch 2 is dropped in v5 and the code stays
where it is. See my reply on patch 2.


> > +		if (cxld->interleave_ways > CXL_HDM_DECODER0_TL_TARGETS) {
> > +			dev_err(&port->dev,
> > +				"decoder%d.%d: interleave ways: %d exceeds target list capacity: %d\n",
> > +				port->id, cxld->id, cxld->interleave_ways,
> > +				CXL_HDM_DECODER0_TL_TARGETS);
> > +			return -ENXIO;
> > +		}
> > +		if (cxld->interleave_ways > cxlsd->nr_targets) {
> > +			dev_err(&port->dev,
> > +				"decoder%d.%d: interleave ways: %d exceeds targets: %d\n",
> > +				port->id, cxld->id, cxld->interleave_ways,
> > +				cxlsd->nr_targets);
> > +			return -ENXIO;
> > +		}
> > +
> >   		lo = readl(hdm + CXL_HDM_DECODER0_TL_LOW(which));
> >   		hi = readl(hdm + CXL_HDM_DECODER0_TL_HIGH(which));
> >   		target_list.value = (hi << 32) + lo;


snip