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] [thread-next>] [day] [month] [year] [list]

Message-ID: <82b50e37070e617786dccf84056183e70c7cb538.camel@linux.ibm.com>
Date: Thu, 22 Jan 2026 12:35:17 +0100
From: Niklas Schnelle <schnelle@...ux.ibm.com>
To: Bjorn Helgaas <helgaas@...nel.org>,
        Håkon Bugge
	 <haakon.bugge@...cle.com>
Cc: Bjorn Helgaas <bhelgaas@...gle.com>,
        Johannes Thumshirn	
 <morbidrsa@...il.com>, linux-pci@...r.kernel.org,
        linux-kernel@...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 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.

Thanks,
Niklas