On Thu, Feb 29, 2024 at 2:01 AM Peng Fan <peng.fan@xxxxxxx> wrote:
>
> > Subject: Re: [PATCH] of: dynamic: notify before revert
> >
> > On Wed, Feb 28, 2024 at 12:13 AM Peng Fan (OSS) <peng.fan@xxxxxxxxxxx>
> > wrote:
> > >
> > > From: Peng Fan <peng.fan@xxxxxxx>
> > >
> > > When PCI node was created using an overlay and the overlay is
> > > reverted/destroyed, the "linux,pci-domain" property no longer exists,
> > > so of_get_pci_domain_nr will return failure. Then
> > > of_pci_bus_release_domain_nr will actually use the dynamic IDA, even
> > > if the IDA was allocated in static IDA.
> >
> > That usage is broken to begin with unless there is a guarantee that static and
> > dynamic domain numbers don't overlap. For example, a dynamic number is
> > assigned and then you load an overlay with the same number defined in it.
>
> I may not describe it clear, the code is here, because overlay property
> removed, of_get_pci_domain_nr will return failure, so the code path
> will goest into free a dynamic ida. But actually there is no such dynamic
> ida, so dump.
I understood the problem.
Your usage of this is broken on applying your overlay. You just got lucky.
> So the problem is overlay was removed, but the notify callback may
> still use the property to do some work.
>
> static void of_pci_bus_release_domain_nr(struct pci_bus *bus, struct device *parent)
> {
> if (bus->domain_nr < 0)
> return;
>
> /* Release domain from IDA where it was allocated. */
> if (of_get_pci_domain_nr(parent->of_node) == bus->domain_nr)
> ida_free(&pci_domain_nr_static_ida, bus->domain_nr);
> else
> ida_free(&pci_domain_nr_dynamic_ida, bus->domain_nr);
> }
> >
> > > So move the notify before revert to fix the issue.
> >
> > You can't just change the timing. Something might require notify to be after
> > the revert.
Again ^^^
> >
> > > Fixes: c14f7ccc9f5d ("PCI: Assign PCI domain IDs by ida_alloc()")
> >
> > I don't see where the notifier is even used.
>
> The stack is as below:
>
> [ 595.150529] CPU: 1 PID: 582 Comm: jailhouse Tainted: G O 6.5.0-rc4-next-20230804-05021-g3d4cc14b42ef-dirty #355
> [ 595.161998] Hardware name: NXP i.MX93 11X11 EVK board (DT)
> [ 595.167475] Call trace:
> [ 595.169908] dump_backtrace+0x94/0xec
> [ 595.173573] show_stack+0x18/0x24
> [ 595.176884] dump_stack_lvl+0x48/0x60
> [ 595.180541] dump_stack+0x18/0x24
> [ 595.183843] pci_bus_release_domain_nr+0x34/0x84
> [ 595.188453] pci_remove_root_bus+0xa0/0xa4
> [ 595.192544] pci_host_common_remove+0x28/0x40
> [ 595.196895] platform_remove+0x54/0x6c
> [ 595.200641] device_remove+0x4c/0x80
> [ 595.204209] device_release_driver_internal+0x1d4/0x230
> [ 595.209430] device_release_driver+0x18/0x24
> [ 595.213691] bus_remove_device+0xcc/0x10c
> [ 595.217686] device_del+0x15c/0x41c
> [ 595.221170] platform_device_del.part.0+0x1c/0x88
> [ 595.225861] platform_device_unregister+0x24/0x40
> [ 595.230557] of_platform_device_destroy+0xfc/0x10c
> [ 595.235344] of_platform_notify+0x13c/0x178
> [ 595.239518] blocking_notifier_call_chain+0x6c/0xa0
> [ 595.244389] __of_changeset_entry_notify+0x148/0x16c
> [ 595.249346] of_changeset_revert+0xa8/0xcc
> [ 595.253437] jailhouse_pci_virtual_root_devices_remove+0x50/0x74 [jailhouse]
$ git grep jailhouse_pci_virtual_root_devices_remove
(END)
Another out of tree overlay user. I have little interest in worrying
about them. No one wants to step up and solve the problems with
overlays, so I'm not going to worry about them either.
Rob
