Re: Issues with OF_DYNAMIC PCI bridge node generation (kmemleak/interrupt-map IC reg property)

[Lizhi Hou <lizhi.hou@amd.com>]

On 8/11/25 01:42, Lorenzo Pieralisi wrote:

> Hi Lizhi, Rob,
>
> while debugging something unrelated I noticed two issues
> (related) caused by the automatic generation of device nodes
> for PCI bridges.
>
> GICv5 interrupt controller DT top level node [1] does not have a "reg"
> property, because it represents the top level node, children (IRSes and ITSs)
> are nested.
>
> It does provide #address-cells since it has child nodes, so it has to
> have a "ranges" property as well.
>
> You have added code to automatically generate properties for PCI bridges
> and in particular this code [2] creates an interrupt-map property for
> the PCI bridges (other than the host bridge if it has got an OF node
> already).
>
> That code fails on GICv5, because the interrupt controller node does not
> have a "reg" property (and AFAIU it does not have to - as a matter of
> fact, INTx mapping works on GICv5 with the interrupt-map in the
> host bridge node containing zeros in the parent unit interrupt
> specifier #address-cells).
Does GICv5 have 'interrupt-controller' but not 'interrupt-map'? I think 
of_irq_parse_raw will not check its parent in this case.
>
> It is not clear to me why, to create an interrupt-map property, we
> are reading the "reg" value of the parent IC node to create the
> interrupt-map unit interrupt specifier address bits (could not we
> just copy the address in the parent unit interrupt specifier reported
> in the host bridge interrupt-map property ?).
>
> - #address-cells of the parent describes the number of address cells of
>    parent's child nodes not the parent itself, again, AFAIK, so parsing "reg"
>    using #address-cells of the parent node is not entirely correct, is it ?
> - It is unclear to me, from an OF spec perspective what the address value
>    in the parent unit interrupt specifier ought to be. I think that, at
>    least for dts including a GICv3 IC, the address values are always 0,
>    regardless of the GICv3 reg property.
>
> I need your feedback on this because the automatic generation must
> work seamlessly for GICv5 as well (as well as all other ICs with no "reg"
> property) and I could not find anything in the OF specs describing
> how the address cells in the unit interrupt specifier must be computed.

Please see: 
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html

2.4.3.1 mentions:

"Both the child node and the interrupt parent node are required to have 
#address-cells and #interrupt-cells properties defined. If a unit 
address component is not required, #address-cells shall be explicitly 
defined to be zero."

>
> I found this [3] link where in section 7 there is an interrupt mapping
> algorithm; I don't understand it fully and I think it is possibly misleading.
>
> Now, the failure in [2] (caused by the lack of a "reg" property in the
> IC node) triggers an interrupt-map property generation failure for PCI
> bridges that are upstream devices that need INTx swizzling.
>
> In turn, that leads to a kmemleak detection:
>
> unreferenced object 0xffff000800368780 (size 128):
>    comm "swapper/0", pid 1, jiffies 4294892824
>    hex dump (first 32 bytes):
>      f0 b8 34 00 08 00 ff ff 04 00 00 00 00 00 00 00  ..4.............
>      70 c2 30 00 08 00 ff ff 00 00 00 00 00 00 00 00  p.0.............
>    backtrace (crc 1652b62a):
>      kmemleak_alloc+0x30/0x3c
>      __kmalloc_cache_noprof+0x1fc/0x360
>      __of_prop_dup+0x68/0x110
>      of_changeset_add_prop_helper+0x28/0xac
>      of_changeset_add_prop_string+0x74/0xa4
>      of_pci_add_properties+0xa0/0x4e0
>      of_pci_make_dev_node+0x198/0x230
>      pci_bus_add_device+0x44/0x13c
>      pci_bus_add_devices+0x40/0x80
>      pci_host_probe+0x138/0x1b0
>      pci_host_common_probe+0x8c/0xb0
>      platform_probe+0x5c/0x9c
>      really_probe+0x134/0x2d8
>      __driver_probe_device+0x98/0xd0
>      driver_probe_device+0x3c/0x1f8
>      __driver_attach+0xd8/0x1a0
>
> I have not grokked it yet but it seems genuine, so whatever we decide
> in relation to "reg" above, this ought to be addressed too, if it
> is indeed a memleak.

Not sure what is the leak. I will look into more.


Lizhi

>
> Please let me know if something is unclear I can provide further
> details.
>
> Thanks,
> Lorenzo
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v5.yaml?h=v6.17-rc1
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/of_property.c?h=v6.17-rc1#n283
> [3] https://www.devicetree.org/open-firmware/practice/imap/imap0_9d.html