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

Ilpo Järvinen
2026-01-22T13:46:03Z

[-- Attachment #1: Type: text/plain, Size: 4155 bytes --]

On Thu, 22 Jan 2026, 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.
>
> e42010d8207f fixed that by qualifying the setting of the RCB in the
> endpoint with the RC supporting an 128 byte RCB.
>
> In retrospect, the program_hpx_type2() should only modify the AER
> bits, and stay away from fiddling with the Link Control Register.
>
> Hence, we explicitly program the RCB from pci_configure_device().
>
> According to PCIe r7.0, sec 7.5.3.7, RCB is only valid for Root Ports
> (where it is Read-Only), Bridges, and Endpoints. The bit is 'RsvdP'
> for Virtual Functions. Hence, for other cases than Bridges and Physical
> Endpoints, we bail out early from pci_configure_rcb().
>
> If the Root Port's RCB cannot be determined, we do nothing.
>
> If RCB is set in the Root Port and not in the device, we set it. If it
> is set in the device but not in the Root Port, we print an info
> message and reset it.
>
> Fixes: Commit e42010d8207f ("PCI: Set Read Completion Boundary to 128 iff Root Port supports it (_HPX)")
> Signed-off-by: Håkon Bugge <haakon.bugge@oracle.com>
>
> ---
>
> Note, that the current duplication of pcie_root_rcb_set() will be
> removed in the next commit.
>
> v2 -> v3:
> * Qualified the device types more strictly
> * s/pcie_root_rcb_set/pcie_read_root_rcb/ and changed signature
> * Do nothing if the RP's RCB cannot be determined
> * Reset the device's RCB if not set in the RP
> ---
> drivers/pci/probe.c | 53 +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 53 insertions(+)
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 41183aed8f5d9..7165ac4065c97 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -2410,6 +2410,58 @@ static void pci_configure_serr(struct pci_dev *dev)
> }
> }
>
> +static bool pcie_read_root_rcb(struct pci_dev *dev, bool *rcb)
> +{
> + struct pci_dev *rp = pcie_find_root_port(dev);
> + u16 lnkctl;
> +
> + if (!rp)
> + return false;
> +
> + pcie_capability_read_word(rp, PCI_EXP_LNKCTL, &lnkctl);
> +
> + *rcb = !!(lnkctl & PCI_EXP_LNKCTL_RCB);
> + return true;
> +}
> +
> +static void pci_configure_rcb(struct pci_dev *dev)
> +{
> + u16 lnkctl;
> + bool rcb;
> +
> + /*
> + * 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 the Root Port's RCB cannot be determined, we
> + * bail out.
> + */
> + 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 || !pcie_read_root_rcb(dev, &rcb))
> + return;
> +
> + 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");
> + lnkctl &= ~PCI_EXP_LNKCTL_RCB;
> + }
> +
> + pcie_capability_write_word(dev, PCI_EXP_LNKCTL, lnkctl);

So this sequence is effectively implementing this simple statement:

pcie_capability_clear_and_set_word(dev, PCI_EXP_LNKCTL,
PCI_EXP_LNKCTL_RCB,
rcb ? PCI_EXP_LNKCTL_RCB : 0);

+ the print.

Is there a good reason why you want to avoid the write by using early
returns?

I also wonder if those clear & set & clean_and_set interfaces should
implement the write avoidance if it's an useful thing (callers should be
checked they're not used for RW1C bits if that's implemented though).

--
i.