Re: [PATCH v2 1/2] PCI: Initialize RCB from pci_configure_device

[Haakon Bugge <haakon.bugge@oracle.com>]

> On Wed, Jan 21, 2026 at 12:35:40PM +0100, Håkon Bugge wrote:
> [snip]

> RCB isn't meaningful for switches, so we'll read their LNKCTL
> unnecessarily.  I propose something like this, which also clears RCB
> if it's set when it shouldn't be (I think this would indicate a
> firmware defect):
> 
>        /*
>         * Per PCIe r7.0, sec 7.5.3.7, RCB is only meaningful in Root Ports
>         * (where it is read-only), Endpoints, and Bridges.  It may only be
>         * set for Endpoints and Bridges if it is set in the Root Port.
>         */
>        if (!pci_is_pcie(dev) ||
>            pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT ||
>            pci_pcie_type(dev) == PCI_EXP_TYPE_UPSTREAM ||
>            pci_pcie_type(dev) == PCI_EXP_TYPE_DOWNSTREAM ||
>            dev->is_virtfn)

I see that sec 1.3.2 defines "Endpoints are classified as either legacy, PCI Express, or Root Complex Integrated Endpoints (RCiEPs)." But, shouldn't we also exclude Root Complex Event Collectors (PCI_EXP_TYPE_RC_EC)?

>                return;
> 
>        rcb = pcie_root_rcb_set(dev);
> 
>        pcie_capability_read_word(dev, PCI_EXP_LNKCTL, &lnkctl);
>        if (rcb) {
>                if (lnkctl & PCI_EXP_LNKCTL_RCB)
>                        return;
> 
>                lnkctl |= PCI_EXP_LNKCTL_RCB;
>        } else {

I was thinking that since sec 7.5.3.7 states (for Endpoints and Bridges): "Default value of this bit is 0b.", that this case didn't happen. But of course, a defect BIOS should be considered.

A v3 is on its way.

I really appreciate your diligent review, Bjorn!


Thxs, Håkon


>                if (!(lnkctl & PCI_EXP_LNKCTL_RCB))
>                        return;
> 
>                pci_info(FW_INFO "clearing RCB (RCB not set in Root Port)\n");
>                lnkctl &= ~PCI_EXP_LNKCTL_RCB;
>        }
> 
>        pcie_capability_write_word(dev, PCI_EXP_LNKCTL, lnkctl);
> 
>> +}
>> +
>> static void pci_configure_device(struct pci_dev *dev)
>> {
>> 	pci_configure_mps(dev);
>> @@ -2419,6 +2454,7 @@ static void pci_configure_device(struct pci_dev *dev)
>> 	pci_configure_aspm_l1ss(dev);
>> 	pci_configure_eetlp_prefix(dev);
>> 	pci_configure_serr(dev);
>> +	pci_configure_rcb(dev);
>> 
>> 	pci_acpi_program_hp_params(dev);
>> }
>> --
>> 2.43.5