Re: [PATCH] dm: pci: fix uninitialized fdt_pci_addr fall-through in pci_get_devfn

Hi Dustin,

On 2026-08-05T20:31:13, Scott Moser <[email protected]> wrote:
> dm: pci: fix uninitialized fdt_pci_addr fall-through in pci_get_devfn

Thanks for tracking this down.

>
> pci_get_devfn() reads addr.phys_hi and returns the low bits of it as the
> requested devfn even on the -ENOENT branch, where ofnode_read_pci_addr()
> has NOT written *addr. The C standard leaves that read undefined; in
> practice the returned value depends on the compilers stack layout and on
> -ftrivial-auto-var-init. That value ends up in pplat->devfn (set by
> pci_uclass_child_post_bind()) and is later compared for equality in
> pci_bus_find_devfn() during PCI enumeration, so an under-determined
> value produces an under-determined driver-binding outcome.
>
> Concretely, this fires on Raspberry Pi 5 (BCM2712) with vendor U-Boot
> v2026.07 when the tree is compiled with GCC -ftrivial-auto-var-init=zero.
> In that build addr.phys_hi is zeroed rather than left as stack junk, so
> pci_get_devfn() returns 0 for every DT-declared non-PCI child of a PCI
> bus. The Pi 5 device tree includes one such child under the second root
> complex -- the rp1 simple-bus node representing the on-SoC RP1 south
> bridge as seen from the OS side. Under zero-init:
>
>   * pci_uclass_child_post_bind(rp1) sets pplat->devfn = 0
> [...]
>
> drivers/core/util.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)

>     Reported-by: Dustin Kirkland <[email protected]>
>     Fixes: b52142004fbd ("pci: Add pci_get_devfn() to extract devfn from the fdt_pci_addr")
>     Signed-off-by: Dustin Kirkland <[email protected]>
>     Investigated-by: Claude Opus 4.7 <[email protected]>
>     Co-authored-by: Claude Opus 4.7 <[email protected]>

This is a good find!

Please drop Reported-by when the reporter is also the Signed-off-by -
also U-Boot doesn't accept AI contributions, so probably best to just
remove those tags. Also not that useful to mention a vendor tree - we
mostly just worry about mainline here.

> diff --git a/drivers/core/util.c b/drivers/core/util.c
> @@ -20,10 +20,8 @@ int pci_get_devfn(struct udevice *dev)
>       /* Extract the devfn from fdt_pci_addr */
>       ret = ofnode_read_pci_addr(dev_ofnode(dev), FDT_PCI_SPACE_CONFIG,
>                                  "reg", &addr, NULL);
> -     if (ret) {
> -             if (ret != -ENOENT)
> -                     return -EINVAL;
> -     }
> +     if (ret)
> +             return -EINVAL;
>
>       return addr.phys_hi & 0xff00;
>  }

The fix is right, but the kerneldoc in include/dm/pci.h still says:

    Return: devfn in bits 15...8 if found (pci_dev_t format), or -ENODEV if not
        found

Since this patch makes the error path observable, please either update
that comment to match (-EINVAL on any lookup failure) or return
-ENODEV for the ret == -ENOENT case so the documented contract holds.
I'd lean towards -ENODEV for the not-present case and -EINVAL only for
a malformed reg - that also reads more naturally at the callers in
arch/x86/cpu/apollolake/{pmc,hostbridge}.c and intel_common/p2sb.c,
which only test < 0. What do you think?

Regards,
Simon

[1] https://lore.kernel.org/u-boot/20251113133038.GF6688@bill-the-cat/