linux-kernel - Re: [PATCH v2] PCI: of: Downgrade error message on missing of

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: <aQyApy8lcadd-1se@apocalypse>
Date: Thu, 6 Nov 2025 12:04:07 +0100
From: Andrea della Porta <andrea.porta@...e.com>
To: Bjorn Helgaas <helgaas@...nel.org>
Cc: Andrea della Porta <andrea.porta@...e.com>,
	Bjorn Helgaas <bhelgaas@...gle.com>, linux-pci@...r.kernel.org,
	linux-kernel@...r.kernel.org, mbrugger@...e.com,
	guillaume.gardet@....com, tiwai@...e.com,
	Lizhi Hou <lizhi.hou@....com>, herve.codina@...tlin.com
Subject: Re: [PATCH v2] PCI: of: Downgrade error message on missing of_root
 node

[+cc Herve]

Hi Bjorn,

On 18:23 Wed 05 Nov     , Bjorn Helgaas wrote:
> [+cc Lizhi]
> 
> On Wed, Nov 05, 2025 at 07:33:40PM +0100, Andrea della Porta wrote:
> > When CONFIG_PCI_DYNAMIC_OF_NODES is enabled, an error message
> > is generated if no 'of_root' node is defined.
> > 
> > On DT-based systems, this cannot happen as a root DT node is
> > always present. On ACPI-based systems, this is not a true error
> > because a DT is not used.
> > 
> > Downgrade the pr_err() to pr_info() and reword the message text
> > to be less context specific.
> 
> of_pci_make_host_bridge_node() is called in the very generic
> pci_register_host_bridge() path.  Does that mean every boot of a
> kernel with CONFIG_PCI_DYNAMIC_OF_NODES on a non-DT system will see
> this message?

This is the case, indeed. That's why downgrading to info seems sensible.

> 
> This message seems like something that will generate user questions.
> Or is this really an error, and we were supposed to have created
> of_root somewhere but it failed?  If so, I would expect a message
> where the of_root creation failed.

Not really an error per se: on ACPI system we usually don't have DT, so
this message just warns you that there will be no pci nodes created on it.
Which, again, should be of no importance on ACPI.

The only scenario in which this message is actually an error would be on
ACPI system that use DT as a complement to make runtime overlay work,
i.e. the overlay approach for RP1 on RPi5 with ACPI fw. AFAIK this fw is
more a PoC that something really widespread and currntly the overlay
approach is in stand-by anyway (meaning no one will use it unless some
major changes will be made to make it work). But there may be other
situations in which this scenario could arise, I'm thinking about Bootlin's
LAN966x driver which also uses runtime overlay to describe thw hw.
On ACPI system the root DT node is not created because unflatten_device_tree()
is not called.
One possible (easy) solution would be calling unflatten_device_tree() also
in case IS_ENABLED(PCI_DYNAMIC_OF_NODES), but this of course requires some
investigation against side effects.
In this case the roto DT node is always created (on both ACPI and non ACPI
systems) and the info message will not be printed.

> 
> I guess I'm confused about what the point of this message is.  If it's
> just a hint that loading an overlay in the future will fail, I assume
> we would emit a message at that time, connected with the user action
> of trying to load the overlay.

For a functional standpoint, it basically is, if we neglect the fact that
you won't have PCI nodes described in the DT, of course, which I guess it's
just informationali (but I may be mistaken on this point). Loading an overlay
in the future will fail anyway so no need to take further action there. Plus,
the systems on which this could happen (ACPI system + runtime overlay) would
be probably rare, if ever exist.

> 
> What badness would ensue if we downgraded this message even further
> and removed it completely?

As stated above, I would expect no major issue but other opinions
would be very welcome. 

Many thanks,
Andrea

> 
> > Signed-off-by: Andrea della Porta <andrea.porta@...e.com>
> > ---
> > CHANGES in V2:
> > 
> > * message text reworded to be less context specific (Bjorn)
> > ---
> >  drivers/pci/of.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> > index 3579265f1198..872c36b195e3 100644
> > --- a/drivers/pci/of.c
> > +++ b/drivers/pci/of.c
> > @@ -775,7 +775,7 @@ void of_pci_make_host_bridge_node(struct pci_host_bridge *bridge)
> >  
> >  	/* Check if there is a DT root node to attach the created node */
> >  	if (!of_root) {
> > -		pr_err("of_root node is NULL, cannot create PCI host bridge node\n");
> > +		pr_info("Missing DeviceTree, cannot create PCI host bridge node\n");
> >  		return;
> >  	}
> >  
> > -- 
> > 2.35.3
> >