Re: [PATCH v7 8/8] cxl: Allow auto-committed BI hdm decoders

[Davidlohr Bueso <dave@stgolabs.net>]

On Fri, 07 Aug 2026, Richard Cheng wrote:

>On Tue, Jul 28, 2026 at 07:41:36AM +0800, Davidlohr Bueso wrote:
>>  /* Enable or dealloc BI-ID changes in the given level of the topology. */
>>  static int __cxl_bi_ctrl_dport(struct cxl_dport *dport, bool enable)
>>  {
>> @@ -1127,6 +1139,16 @@ static int __cxl_bi_ctrl_dport(struct cxl_dport *dport, bool enable)
>>  		return 0;
>>  	case PCI_EXP_TYPE_DOWNSTREAM:
>>  		if (enable) {
>> +			/*
>> +			 * Adopt a dport that was already programmed and
>> +			 * committed by firmware: nothing new is enabled
>> +			 * below it, so no commit is due.
>> +			 */
>> +			if (FIELD_GET(CXL_BI_DECODER_CTRL_BI_ENABLE, ctrl) &&
>> +			    !FIELD_GET(CXL_BI_DECODER_CTRL_BI_FW, ctrl) &&
>> +			    cxl_bi_decoder_committed(bi))
>> +				return 0;
>> +
>
>Small question, when DSP's BI related registers are committed by FW, does it guaranteed
>that all the intermediate switch on the path are also committed with BI enabled ?

No, there are no guarantees at all for any of this. Please see below for details on the
most flexible way I could come up with.

...

>> @@ -3806,6 +3831,14 @@ static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd,
>>  	if (part < 0)
>>  		return ERR_PTR(-EBUSY);
>>
>> +	/* avoid UB */
>> +	if (cxled_committed_bi(cxled) && !cxl_root_decoder_is_bi(cxlrd)) {
>> +		dev_err(cxlmd->dev.parent,
>> +			"%s:%s BI decoder in a non-BI window\n",
>> +			dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev));
>> +		return ERR_PTR(-ENXIO);
>> +	}
>> +
>
>Should we also check the inverse mismatch case here ?
>
>if (!cxled_committed_bi() && cxl_root_decoder_is_bi()) {}
>
>thoughts ?

Yep, and the whole thing can be turned into:

if (cxled_committed_bi(cxled) != cxl_root_decoder_is_bi(cxlrd)) { }

...

>
>Hmmm just curious are intermediate switch are guaranteed to be valid in AUTO discovery
>scenario , seems that the implementation only checks DSP.

So overall I had been ignoring nested switches because all this
is quite unsupported in Linux.

But for v8 I went ahead and added support for this. Basically
for patch 2 I redid the dport ctrl to handle nr_bi with nested
switches (as opposed to just for root port).

Now, for this auto discovery, when an endpoint device detects
the BI Enable already there it will "adopt" already configured
switch(es) and rp as it goes up the hierarchy. If at any level
things are not already configured, this adoption is broken and
it will do the respective BI decoder programming/committing as
it always does. For all level(s) above, every DSP that requires
explicit commits gets one, even with the decoder already configured
and previously committed because the new device's BI-ID may or
may not be accounted for.

Thanks,
Davidlohr