linux-kernel - Re: [PATCH v2 1/2] PCI: Initialize RCB from pci_configure

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <58E63B30-484E-4BE7-81E1-638B72F3C16C@oracle.com>
Date: Thu, 22 Jan 2026 13:25:02 +0000
From: Haakon Bugge <haakon.bugge@...cle.com>
To: Niklas Schnelle <schnelle@...ux.ibm.com>
CC: Bjorn Helgaas <helgaas@...nel.org>, Bjorn Helgaas <bhelgaas@...gle.com>,
        Johannes Thumshirn <morbidrsa@...il.com>,
        "linux-pci@...r.kernel.org"
	<linux-pci@...r.kernel.org>,
        "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>,
        "linux-acpi@...r.kernel.org"
	<linux-acpi@...r.kernel.org>,
        Alex Williamson <alex@...zbot.org>
Subject: Re: [PATCH v2 1/2] PCI: Initialize RCB from pci_configure_device



> On 22 Jan 2026, at 12:35, Niklas Schnelle <schnelle@...ux.ibm.com> wrote:
> 
> On Thu, 2026-01-22 at 04:36 -0600, Bjorn Helgaas wrote:
>> [+cc Alex, Niklas]
>> 
>> On Wed, Jan 21, 2026 at 04:40:10PM -0600, Bjorn Helgaas wrote:
>>> 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@...cle.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)
>>>> +{
>>>> 
> --- snip ---
>>> 
>>>        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;
>> 
>> On second thought, I think this is too aggressive.  I think VM guests
>> will often see endpoints but not the Root Port.  In that case,
>> pcie_root_rcb_set() would return false because it couldn't find the
>> RP, but the RP might actually have RCB set.  Then we would clear the
>> endpoint RCB unnecessarily, which should be safe but would reduce
>> performance and will result in annoying misleading warnings.
>> 
>> Could either ignore this case (as in your original patch) or bring
>> pcie_root_rcb_set() inline here and return early if we can't find the
>> RP.
>>> 
> 
> Thanks Bjorn for looping me in. If I'm reading later comments correctly
> we're already in agreement that if the root port isn't found the
> function should bail and leave the setting as is which sounds good to
> me. Still, feel free to directly add me in To on the next version and
> I'll be happy to test it and take a look at the code.
> 
> Nevertheless I'd like to confirm that yes on s390 we definitely have
> the case where PFs are passed-through to guests without the guest
> having access to / seeing the root port as a PCIe device. This is true
> on both our machine hypervisor guests (LPARs) and in KVM guests. And
> yes I think this would potentially incorrectly clear the RCB which
> could have been set by firmware or platform PCI code based on its
> knowledge of the actual root port. That said from a quick look we
> currently seem to keep the RCB at 64 bytes in endpoints.

Hi Niklas,

Thanks for chiming in. Just out a v3 [1].


Thxs, Håkon

[1] https://lore.kernel.org/linux-pci/20260122130957.68757-1-haakon.bugge@oracle.com/

> 
> Thanks,
> Niklas