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

[Bjorn Helgaas <helgaas@kernel.org>]

On Wed, Jan 21, 2026 at 12:35:40PM +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.
> 
> 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(). RCB
> is RO in Root Ports, and in VFs, the bit is RsvdP, so for these two
> cases we skip programming it. Then, if the Root Port has RCB set and
> it is not set in the EP, we set 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.
> ---
>  drivers/pci/probe.c | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 41183aed8f5d9..347af29868124 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -2410,6 +2410,41 @@ static void pci_configure_serr(struct pci_dev *dev)
>  	}
>  }
>  
> +static bool pcie_root_rcb_set(struct pci_dev *dev)
> +{
> +	struct pci_dev *rp = pcie_find_root_port(dev);
> +	u16 lnkctl;
> +
> +	if (!rp)
> +		return false;
> +
> +	pcie_capability_read_word(rp, PCI_EXP_LNKCTL, &lnkctl);
> +
> +	return !!(lnkctl & PCI_EXP_LNKCTL_RCB);
> +}
> +
> +static void pci_configure_rcb(struct pci_dev *dev)
> +{
> +	/*
> +	 * Obviously, we need a Link Control register. The RCB is RO
> +	 * in Root Ports, so no need to attempt to set it for
> +	 * them. For VFs, the RCB is RsvdP, so, no need to set it.
> +	 * Then, if the Root Port has RCB set, then we set for the EP
> +	 * unless already set.
> +	 */
> +	if (pcie_cap_has_lnkctl(dev) &&
> +	    (pci_pcie_type(dev) != PCI_EXP_TYPE_ROOT_PORT) &&
> +	    !dev->is_virtfn && pcie_root_rcb_set(dev)) {
> +		u16 lnkctl;
> +
> +		pcie_capability_read_word(dev, PCI_EXP_LNKCTL, &lnkctl);
> +		if (lnkctl & PCI_EXP_LNKCTL_RCB)
> +			return;
> +
> +		pcie_capability_write_word(dev, PCI_EXP_LNKCTL, lnkctl | PCI_EXP_LNKCTL_RCB);
> +	}

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)
                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 {
                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
>