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

Message-ID: <CAL_JsqJ_gCu_A2YqX4r2UMWssJ=4FprsM_Os3Zw=V8yCTGubdA@mail.gmail.com>
Date: Fri, 5 Dec 2025 07:58:10 -0600
From: Rob Herring <robh@...nel.org>
To: Herve Codina <herve.codina@...tlin.com>
Cc: Andrea della Porta <andrea.porta@...e.com>, Bjorn Helgaas <helgaas@...nel.org>, 
	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>
Subject: Re: [PATCH v2] PCI: of: Downgrade error message on missing of_root node

On Fri, Nov 7, 2025 at 5:58 AM Herve Codina <herve.codina@...tlin.com> wrote:
>
> Hi Andrea, Bjorn,
>
> On Fri, 7 Nov 2025 10:32:35 +0100
> Andrea della Porta <andrea.porta@...e.com> wrote:
>
> > Hi Bjorn,
> >
> > On 11:50 Thu 06 Nov     , Bjorn Helgaas wrote:
> > > On Thu, Nov 06, 2025 at 06:27:08PM +0100, Herve Codina wrote:
> > > > On Thu, 6 Nov 2025 16:21:47 +0100
> > > > Andrea della Porta <andrea.porta@...e.com> wrote:
> > > > > On 13:18 Thu 06 Nov     , Herve Codina wrote:
> > > > > > On Thu, 6 Nov 2025 12:04:07 +0100
> > > > > > Andrea della Porta <andrea.porta@...e.com> wrote:
> > > > > > > On 18:23 Wed 05 Nov     , Bjorn Helgaas wrote:
> > > > > > > > On Wed, Nov 05, 2025 at 07:33:40PM +0100, Andrea della Porta wrote:
> > > Patch at https://lore.kernel.org/r/955bc7a9b78678fad4b705c428e8b45aeb0cbf3c.1762367117.git.andrea.porta@suse.com,
> > > added back for reference:
> > >
> > >   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;
> > >       }
> > >
> > > > > > > > > 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.
> > >
> > > I think we should just remove the message completely.  I don't want
> > > users to enable CONFIG_PCI_DYNAMIC_OF_NODES out of curiosity or
> > > willingness to test, and then ask about this message.

No, please keep it. Anyone that enables CONFIG_PCI_DYNAMIC_OF_NODES
should know what they are doing. I was thinking it should be hidden
behind EXPERT perhaps.

> >
> > Agreed. This would be the easy solution, the other being creating the
> > empty DT so that the message will never be printed. But this require some
> > careful thought. The latter solution will be needed if we'll ever want to
> > make drivers like RP1 or lan96xx (which uses the runtime overlay) to work.
> >
> > >
> > > "You can avoid the message by also enabling CONFIG_OF_EARLY_FLATTREE"
> > > is not a very satisfactory answer.
> >
> > Unfortunately this would work on x86, but not on arm. And who knows on
> > other platforms.
> >
> > >
> > > A message at the point of *needing* this, i.e., when loading an
> > > overlay fails for lack of this dynamic DT, is fine.
> >
> > It seems fine to me.
> >
>
> Ok, even if I would prefer having an empty (or not) of_node available on all
> platforms, what is proposed makes sense especially in regards to potential
> users questions.

Having a DT always available was the original intent, but the arm64
maintainers resisted so that's a fight for another day when someone
really wants to use this on an ACPI system. Now riscv has done the
same thing unfortunately.

Rob