linux-kernel - RE: [PATCH 1/1] pci: Add subordinate check before pci_add_new

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:
 <DS4PPFD24E991EC33DB3627FC112C84080B963EA@DS4PPFD24E991EC.namprd11.prod.outlook.com>
Date: Mon, 25 Aug 2025 08:47:56 +0000
From: "He, Rui" <Rui.He@...driver.com>
To: Ethan Zhao <etzhao1900@...il.com>, Bjorn Helgaas <bhelgaas@...gle.com>
CC: "linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Chikhalkar,
 Prashant" <Prashant.Chikhalkar@...driver.com>,
        "Xiao, Jiguang"
	<Jiguang.Xiao@...driver.com>
Subject: RE: [PATCH 1/1] pci: Add subordinate check before pci_add_new_bus()



> -----Original Message-----
> From: Ethan Zhao <etzhao1900@...il.com>
> Sent: 2025年8月17日 10:46
> To: He, Rui <Rui.He@...driver.com>; Bjorn Helgaas <bhelgaas@...gle.com>
> Cc: linux-pci@...r.kernel.org; linux-kernel@...r.kernel.org; Chikhalkar,
> Prashant <Prashant.Chikhalkar@...driver.com>; Xiao, Jiguang
> <Jiguang.Xiao@...driver.com>
> Subject: Re: [PATCH 1/1] pci: Add subordinate check before
> pci_add_new_bus()
> 
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and
> know the content is safe.
> 
> On 8/14/2025 5:39 PM, Rui He wrote:
> > For preconfigured PCI bridge, child bus created on the first scan.
> > While for some reasons(e.g register mutation), the secondary, and
> > subordiante register reset to 0 on the second scan, which caused to
> > create PCI bus twice for the same PCI device.
> >
> > Following is the related log:
> > [Wed May 28 20:38:36 CST 2025] pci 0000:0b:01.0: PCI bridge to [bus
> > 0d] [Wed May 28 20:38:36 CST 2025] pci 0000:0b:05.0: bridge
> > configuration invalid ([bus 00-00]), reconfiguring [Wed May 28
> > 20:38:36 CST 2025] pci 0000:0b:01.0: PCI bridge to [bus 0e-10] [Wed
> > May 28 20:38:36 CST 2025] pci 0000:0b:05.0: PCI bridge to [bus 0f-10]
> Could you help to attach a 'lspci -t' about the topology ?
> bridge 0000:0b:01.0 and 0000:0b:05.0 have the same subordinate bus
> number, that is weird seems they aren't connected as upstream and
> downstream, but siblings.

Follwing is the related lspci logs.
# lspci -tv
    ......
    \-[0000:00]-+-00.0  Intel Corporation Xeon E7 v3/Xeon E5 v3/Core i7 DMI2
             +-03.1-[08-73]----00.0-[09-73]--+-00.0  Microsemi / PMC / IDT PES24NT24G2 PCI Express Switch
             |                        +-02.0-[0a-10]----00.0-[0b-10]--+-01.0-[0d]----00.0  Device xxxx
             |                        |                        +-04.0  PLX Technology, Inc. PEX 8606 6 Lane, 6 Port PCI Express Gen 2 (5.0 GT/s) Switch
             |                        |                        +-05.0-[00]--
             |                        |                        +-07.0-[00]--
             |                        |                        \-09.0-[00]--
             |                        +-03.0-[11-17]----00.0-[12-17]--+-01.0-[14]----00.0  Device xxxx
             |                        |                        +-04.0  PLX Technology, Inc. PEX 8606 6 Lane, 6 Port PCI Express Gen 2 (5.0 GT/s) Switch
             |                        |                        +-05.0-[00]--
             |                        |                        +-07.0-[00]--
             |                        |                        \-09.0-[00]--
    ......

Yes, you are right. 0000:0b:01.0 and 0000:0b:05.0 are siblings. 

I added 000:0b:05.0 to indicate that [bus 0d] is created during the first scan, while [bus 0e-10] is created during the second scan.

Here, 0000:0b:01.0 is pre-assigned to bus 0d. 0000:05[07, 09].0 is not configured.

> 
> Does the device behind the bridge 0000:0b:05.0 work after the second scan
> (TLP are forwarded) ?>
> > Here PCI device 000:0b:01.0 assigend to bus 0d and 0e.
> >
> > This patch checks if child PCI bus has been created on the second scan
> > of bridge. If yes, return directly instead of create a new one.
> >
> > Signed-off-by: Rui He <rui.he@...driver.com>
> > ---
> >   drivers/pci/probe.c | 3 +++
> >   1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index
> > f41128f91ca76..ec67adbf31738 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -1444,6 +1444,9 @@ static int pci_scan_bridge_extend(struct pci_bus
> *bus, struct pci_dev *dev,
> >                       goto out;
> >               }
> >
> The bridge should was marked as broken=1 already, bailed out earlier,
> wouldn't get here with bridge forwarding was disabled. no further
> configuration anymore. what is your kernel number ?
> 
> 
> Thanks,
> Ethan

My kernel version is v5.2.60 (https://git.yoctoproject.org/linux-yocto/tree/Makefile?h=v5.2/standard/base).

The bridge can be marked to broken=1 on the first scan, while this error happens on the second scan, here pass=1, (!pass) always be false, 
broken is impossible to set to 1.

[bus 0e-10] was created on 0000:0b:01.0 on the second scan, which means that the if condition is false.

  ->  if ((secondary || subordinate) && !pcibios_assign_all_busses() &&
  -> 	    !is_cardbus && !broken) {

pcibios_assign_all_busses() always be false as "pci=assign-busses" not added to cmdline.

Is_cardbus always be false as 0000:0b.01.0 is a bridge.

Broken always be false on the second scan as pass=1.

The only possible is that (secondary || subordinate) is false.

Thanks,
Rui

> +                if(pci_has_subordinate(dev))
> > +                     goto out;
> > +
> >               /* Clear errors */
> >               pci_write_config_word(dev, PCI_STATUS, 0xffff);
> >