linux-kernel - Re: [PATCH v2] PCI/pwrctrl: Skip creating pwrctrl device unless CONFIG_PCI

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: <20250722191506.GA2810550@bhelgaas>
Date: Tue, 22 Jul 2025 14:15:06 -0500
From: Bjorn Helgaas <helgaas@...nel.org>
To: Manivannan Sadhasivam <mani@...nel.org>
Cc: bhelgaas@...gle.com, lukas@...ner.de, linux-pci@...r.kernel.org,
	linux-kernel@...r.kernel.org, stable@...r.kernel.org,
	Jim Quinlan <james.quinlan@...adcom.com>
Subject: Re: [PATCH v2] PCI/pwrctrl: Skip creating pwrctrl device unless
 CONFIG_PCI_PWRCTRL is enabled

[use Mani's new email addr]

On Tue, Jul 22, 2025 at 01:58:11PM -0500, Bjorn Helgaas wrote:
> On Tue, Jul 01, 2025 at 12:17:31PM +0530, Manivannan Sadhasivam wrote:
> > If devicetree describes power supplies related to a PCI device, we
> > previously created a pwrctrl device even if CONFIG_PCI_PWRCTL was
> > not enabled.
> > 
> > When pci_pwrctrl_create_device() creates and returns a pwrctrl device,
> > pci_scan_device() doesn't enumerate the PCI device. It assumes the pwrctrl
> > core will rescan the bus after turning on the power. However, if
> > CONFIG_PCI_PWRCTL is not enabled, the rescan never happens.
> > 
> > This may break PCI enumeration on any system that describes power supplies
> > in devicetree but does not use pwrctrl. Jim reported that some brcmstb
> > platforms break this way.
> > 
> > While the actual fix would be to convert all the platforms to use pwrctrl
> > framework, we also need to skip creating the pwrctrl device if
> > CONFIG_PCI_PWRCTL is not enabled and let the PCI core scan the device
> > normally (assuming it is already powered on or by the controller driver).
> > 
> > Cc: stable@...r.kernel.org # 6.15
> > Fixes: 957f40d039a9 ("PCI/pwrctrl: Move creation of pwrctrl devices to pci_scan_device()")
> > Reported-by: Jim Quinlan <james.quinlan@...adcom.com>
> > Closes: https://lore.kernel.org/r/CA+-6iNwgaByXEYD3j=-+H_PKAxXRU78svPMRHDKKci8AGXAUPg@mail.gmail.com
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
> 
> I (finally) applied this to for-linus for v6.16 with the following
> commit log:
> 
>     PCI/pwrctrl: Create pwrctrl devices only when CONFIG_PCI_PWRCTRL is enabled
>     
>     If devicetree describes power supplies related to a PCI device, we
>     unnecessarily created a pwrctrl device even if CONFIG_PCI_PWRCTL was not
>     enabled.
>     
>     We only need pci_pwrctrl_create_device() when CONFIG_PCI_PWRCTRL is
>     enabled.  Compile it out when CONFIG_PCI_PWRCTRL is not enabled.
>     
>     When pci_pwrctrl_create_device() creates and returns a pwrctrl device,
>     pci_scan_device() doesn't enumerate the PCI device. It assumes the pwrctrl
>     core will rescan the bus after turning on the power. However, if
>     CONFIG_PCI_PWRCTRL is not enabled, the rescan never happens, which breaks
>     PCI enumeration on any system that describes power supplies in devicetree
>     but does not use pwrctrl.
>     
>     Jim reported that some brcmstb platforms break this way.  The brcmstb
>     driver is still broken if CONFIG_PCI_PWRCTRL is enabled, but this commit at
>     least allows brcmstb to work when it's NOT enabled.
>     
>     Fixes: 957f40d039a9 ("PCI/pwrctrl: Move creation of pwrctrl devices to pci_scan_device()")
>     Reported-by: Jim Quinlan <james.quinlan@...adcom.com>
>     Link: https://lore.kernel.org/r/CA+-6iNwgaByXEYD3j=-+H_PKAxXRU78svPMRHDKKci8AGXAUPg@mail.gmail.com
>     Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
>     [bhelgaas: commit log]
>     Signed-off-by: Bjorn Helgaas <bhelgaas@...gle.com>
>     Reviewed-by: Lukas Wunner <lukas@...ner.de>
>     Cc: stable@...r.kernel.org  # v6.15
>     Link: https://patch.msgid.link/20250701064731.52901-1-manivannan.sadhasivam@linaro.org
> 
> > ---
> > 
> > Changes in v2:
> > 
> > * Used the stub instead of returning NULL inside the function
> > 
> >  drivers/pci/probe.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > index 4b8693ec9e4c..e6a34db77826 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -2508,6 +2508,7 @@ bool pci_bus_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *l,
> >  }
> >  EXPORT_SYMBOL(pci_bus_read_dev_vendor_id);
> >  
> > +#if IS_ENABLED(CONFIG_PCI_PWRCTRL)
> >  static struct platform_device *pci_pwrctrl_create_device(struct pci_bus *bus, int devfn)
> >  {
> >  	struct pci_host_bridge *host = pci_find_host_bridge(bus);
> > @@ -2537,6 +2538,12 @@ static struct platform_device *pci_pwrctrl_create_device(struct pci_bus *bus, in
> >  
> >  	return pdev;
> >  }
> > +#else
> > +static struct platform_device *pci_pwrctrl_create_device(struct pci_bus *bus, int devfn)
> > +{
> > +	return NULL;
> > +}
> > +#endif
> >  
> >  /*
> >   * Read the config data for a PCI device, sanity-check it,
> > -- 
> > 2.43.0
> >