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

Haakon Bugge
2026-01-28T17:08:38Z

> On Thu, Jan 22, 2026 at 02:09:53PM +0100, Håkon Bugge wrote:
>> Commit e42010d8207f ("PCI: Set Read Completion Boundary to 128 iff
>> Root Port supports it (_HPX)") fixed a bogus _HPX type 2 record, which
>> instructed program_hpx_type2() to set the RCB in an endpoint,
>> although it's RC did not have the RCB bit set.
>> ...
>>
>> + pcie_capability_read_word(dev, PCI_EXP_LNKCTL, &lnkctl);
>> + if (rcb) {
>> + if (lnkctl & PCI_EXP_LNKCTL_RCB)
>> + return;
>> +
>> + lnkctl |= PCI_EXP_LNKCTL_RCB;
>> + } else {
>> + if (!(lnkctl & PCI_EXP_LNKCTL_RCB))
>> + return;
>> +
>> + pci_info(dev, FW_INFO "clearing RCB (RCB not set in Root Port)\n");
>
> I know I suggested all this code and the message, but I'm not sure
> it's worth it. If the device doesn't work,

I see your point. If the situation my commit is fixing has been there, the system would have failed, and a fix to the firmware must have been applied. Hence, so need to fix it in the OS.

> that will be obvious
> anyway, so this all feels over-engineered.
>
>> + lnkctl &= ~PCI_EXP_LNKCTL_RCB;
>> + }
>> +
>> + pcie_capability_write_word(dev, PCI_EXP_LNKCTL, lnkctl);
>> +}
>
> What do you think about this?
>
> PCI: Initialize RCB from pci_configure_device()
>
> Commit e42010d8207f ("PCI: Set Read Completion Boundary to 128 iff Root
> Port supports it (_HPX)") worked around a bogus _HPX type 2 record, which
> caused program_hpx_type2() to set the RCB in an endpoint even though the
> Root Port did not have the RCB bit set.
>
> e42010d8207f fixed that by setting the RCB in the endpoint only when it was
> set in the Root Port.
>
> In retrospect, program_hpx_type2() is intended for AER-related settings,
> and the RCB should be configured elsewhere so it doesn't depend on the
> presence or contents of an _HPX record.
>
> Explicitly program the RCB from pci_configure_device() so it matches the
> Root Port's RCB. The Root Port may not be visible to virtualized guests;
> in that case, leave RCB alone.
>
> Fixes: e42010d8207f ("PCI: Set Read Completion Boundary to 128 iff Root Port supports it (_HPX)")

Crisp and clear. For this and the other commit, is it OK that I add you as a co-developer? Aka:

Co-developed-by: Bjorn Helgaas <helgaas@kernel.org>
Signed-off-by: Bjorn Helgaas <helgaas@kernel.org>
Signed-off-by: Håkon Bugge <haakon.bugge@oracle.com>

?

>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 41183aed8f5d..8571c7c6e1a0 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -2410,6 +2410,37 @@ static void pci_configure_serr(struct pci_dev *dev)
> }
> }
>
> +static void pci_configure_rcb(struct pci_dev *dev)
> +{
> + struct pci_dev *rp;
> + u16 rp_lnkctl;
> +
> + /*
> + * 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. For
> + * Endpoints, it is 'RsvdP' for Virtual Functions.
> + */
> + 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 ||
> + pci_pcie_type(dev) == PCI_EXP_TYPE_RC_EC ||
> + dev->is_virtfn)
> + return;
> +
> + /* Root Port often not visible to virtualized guests */
> + rp = pcie_find_root_port(dev);
> + if (!rp)
> + return;
> +
> + pcie_capability_read_word(rp, PCI_EXP_LNKCTL, &rp_lnkctl);
> + pcie_capability_clear_and_set_word(dev, PCI_EXP_LNKCTL,
> + PCI_EXP_LNKCTL_RCB,
> + (rp_lnkctl & PCI_EXP_LNKCTL_RCB) ?
> + PCI_EXP_LNKCTL_RCB : 0);

Looks good to me! This will enforce the locked flavour of pcie_capability_clear_and_set_word(). Is that an overkill?

Again, thank for the effort you put into this, ,Bjorn!

Thxs, Håkon

> +}
> +
> static void pci_configure_device(struct pci_dev *dev)
> {
> pci_configure_mps(dev);
> @@ -2419,6 +2450,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);
> }