Re: [PATCH v7 2/8] cxl/pci: Add BI topology enable/disable

[Davidlohr Bueso <dave@stgolabs.net>]

On Wed, 05 Aug 2026, Li Ming wrote:

>On 7/28/2026 10:41 PM, 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)
>> +{
>> +	struct pci_dev *pdev = to_pci_dev(dport->dport_dev);
>> +	void __iomem *bi = dport->regs.bi_decoder;
>> +	struct cxl_port *port = dport->port;
>> +	u32 ctrl, value;
>> +	int rc;
>> +
>> +	guard(mutex)(&port->bi_lock);
>> +	if (!bi)
>> +		return -EINVAL;
>> +
>> +	ctrl = readl(bi + CXL_BI_DECODER_CTRL_OFFSET);
>> +
>> +	switch (pci_pcie_type(pdev)) {
>> +	case PCI_EXP_TYPE_ROOT_PORT:
>> +		if (enable) {
>> +			/*
>> +			 * There is no point of failure from here on,
>> +			 * BI will be enabled on the endpoint device.
>> +			 */
>> +			dport->nr_bi++;
>> +
>> +			if (FIELD_GET(CXL_BI_DECODER_CTRL_BI_FW, ctrl) &&
>> +			    !FIELD_GET(CXL_BI_DECODER_CTRL_BI_ENABLE, ctrl))
>> +				return 0;
>> +
>> +			value = ctrl | CXL_BI_DECODER_CTRL_BI_FW;
>> +			value &= ~CXL_BI_DECODER_CTRL_BI_ENABLE;
>> +		} else {
>> +			if (WARN_ON_ONCE(dport->nr_bi == 0))
>> +				return -EINVAL;
>> +			if (--dport->nr_bi > 0)
>> +				return 0;
>> +
>> +			value = ctrl & ~(CXL_BI_DECODER_CTRL_BI_FW |
>> +					 CXL_BI_DECODER_CTRL_BI_ENABLE);
>> +		}
>> +
>> +		writel(value, bi + CXL_BI_DECODER_CTRL_OFFSET);
>> +		return 0;
>> +	case PCI_EXP_TYPE_DOWNSTREAM:
>> +		if (enable) {
>> +			value = ctrl & ~CXL_BI_DECODER_CTRL_BI_FW;
>> +			value |= CXL_BI_DECODER_CTRL_BI_ENABLE;
>> +		} else {
>> +			if (!FIELD_GET(CXL_BI_DECODER_CTRL_BI_ENABLE, ctrl))
>> +				return 0;
>> +			value = ctrl & ~(CXL_BI_DECODER_CTRL_BI_FW |
>> +					 CXL_BI_DECODER_CTRL_BI_ENABLE);
>> +		}
>> +
>> +		writel(value, bi + CXL_BI_DECODER_CTRL_OFFSET);
>> +
>> +		rc = __cxl_bi_commit_decoder(dport->dport_dev, bi);
>> +		if (rc)
>> +			return rc;
>> +
>> +		if (port->regs.bi_rt)
>> +			return __cxl_bi_commit_rt(&port->dev, port->regs.bi_rt);
>> +		return 0;
>
>I think another issue here is:
>
>Per CXL r4.0 8.2.4.27.2 Table 8-157. the description of BI  Enable and BI Forward fields.
>
>My understanding is that if a DSP or RP connects to a BI-Capable device directly, the BI Enable field should be set.
>
>Which means if a BI-Capable device is connected to a root port directly, the BI Enable filed of the root port should be set.
>
>In multiple switch levels case, only the last level DSP should set BI Enable filed, other DSPs and root port should set BI Forward.
>
>But current implementation will set BI Enable on all DSPs, and set BI Forward on root port whatever the root port connects to BI-Capable device directly.
>
>If my understanding is incorrect, please let me know. thanks

Thanks for reviewing, I am just now circling back to this.

Yes, I think you are right. For v8 I have consolidated this for root ports and dsp,
the component directly attached to the device now always gets the Enable, the rest get FW.

Thanks,
Davidlohr