Re: [PATCH v2 21/22] irqchip/gic-v5: Add GICv5 IWB support

[Lorenzo Pieralisi <lpieralisi@kernel.org>]

On Fri, May 02, 2025 at 04:43:57PM +0100, Marc Zyngier wrote:
> On Fri, 02 May 2025 08:59:42 +0100,
> Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
> > 
> > It looks like the msi_prepare() ITS callback (ie where the its_device is
> > allocated) is called everytime an endpoint device driver requests a
> > wired IRQ through:
> > 
> > gicv5_its_msi_prepare+0x68c/0x6f8
> > its_pmsi_prepare+0x16c/0x1b8
> > __msi_domain_alloc_irqs+0x70/0x448
> > __msi_domain_alloc_irq_at+0xf8/0x194
> > msi_device_domain_alloc_wired+0x88/0x10c
> > irq_create_fwspec_mapping+0x3a0/0x4c0
> > irq_create_of_mapping+0xc0/0xe8
> > of_irq_get+0xa0/0xe4
> > platform_get_irq_optional+0x54/0x1c4
> > platform_get_irq+0x1c/0x50
> > 
> > so it becomes "shared" if multiple IWB wires are requested by endpoint
> > drivers.
> 
> Right, I've reproduced on D05 with MBIGEN:
> 
> [    5.505530] Reusing ITT for devID 40000
> [    5.505532] CPU: 36 UID: 0 PID: 557 Comm: (udev-worker) Not tainted 6.15.0-rc4-00079-geef147df4841-dirty #4403 PREEMPT 
> [    5.505535] Hardware name: Huawei Taishan 2280 /D05, BIOS Hisilicon D05 IT21 Nemo 2.0 RC0 04/18/2018
> [    5.505536] Call trace:
> [    5.505537]  show_stack+0x20/0x38 (C)
> [    5.505540]  dump_stack_lvl+0x80/0xf8
> [    5.505543]  dump_stack+0x18/0x28
> [    5.505546]  its_msi_prepare+0xe4/0x1d0
> [    5.505549]  its_pmsi_prepare+0x15c/0x1d0
> [    5.505552]  __msi_domain_alloc_irqs+0x80/0x398
> [    5.505556]  __msi_domain_alloc_irq_at+0x100/0x168
> [    5.505560]  msi_device_domain_alloc_wired+0x9c/0x128
> [    5.505564]  irq_create_fwspec_mapping+0x180/0x388
> [    5.505567]  acpi_irq_get+0xac/0xe8
> [    5.505570]  platform_get_irq_optional+0x1e8/0x208
> [    5.505574]  devm_platform_get_irqs_affinity+0x58/0x298
> [    5.505578]  hisi_sas_v2_interrupt_preinit+0x60/0xb0 [hisi_sas_v2_hw]
> [    5.505582]  hisi_sas_probe+0x164/0x278 [hisi_sas_main]
> [    5.505588]  hisi_sas_v2_probe+0x20/0x38 [hisi_sas_v2_hw]
> [    5.505591]  platform_probe+0x70/0xd0
> [    5.505595]  really_probe+0xc8/0x3a0
> [    5.505598]  __driver_probe_device+0x84/0x170
> [    5.505600]  driver_probe_device+0x44/0x120
> [    5.505603]  __driver_attach+0xfc/0x210
> [    5.505606]  bus_for_each_dev+0x7c/0xe8
> [    5.505608]  driver_attach+0x2c/0x40
> [    5.505611]  bus_add_driver+0x118/0x248
> [    5.505613]  driver_register+0x68/0x138
> [    5.505616]  __platform_driver_register+0x2c/0x40
> [    5.505619]  hisi_sas_v2_driver_init+0x28/0xff8 [hisi_sas_v2_hw]
> [    5.505623]  do_one_initcall+0x4c/0x2c0
> [    5.505626]  do_init_module+0x60/0x230
> [    5.505629]  load_module+0xa64/0xb30
> [    5.505631]  init_module_from_file+0x8c/0xd8
> [    5.505634]  idempotent_init_module+0x1c4/0x2b8
> [    5.505637]  __arm64_sys_finit_module+0x74/0xe8
> [    5.505640]  invoke_syscall+0x50/0x120
> [    5.505642]  el0_svc_common.constprop.0+0x48/0xf0
> [    5.505644]  do_el0_svc+0x24/0x38
> [    5.505646]  el0_svc+0x34/0xf0
> [    5.505650]  el0t_64_sync_handler+0x10c/0x138
> [    5.505654]  el0t_64_sync+0x1ac/0x1b0
> [    5.505681] ID:78 pID:8382 vID:143
> 
> And that a few dozen times.

Yep that matches my expectations, thanks a lot for testing it.

> I'll have a think at how to unfsck this. This was previously avoided
> by (IIRC) populating the domain upfront and letting the domain
> matching code do its job. That behaviour seems to have been lost. On
> the other hand, as long as you don't expect the ITT to *grow*, nothing
> horrible should happen.

Yes - I can remove the "shared" ITS device flag but should keep the
logic preventing an ITS device with same deviceID to be allocated
if found.

> But I also get an interesting crash in msi_domain_debig_show(), so
> there is more than just this corner case that is screwed.

That I can try on my side too to try to help you untangle it.

Possibly this was introduced when the MBIgen switched to MSI parent
with 752e021f5b9b ? It is pure speculation at this stage just noticed
that's a point in time where the domain code changed.

Is MBIgen the only example of an IC relying on the ITS as MSI parent ?

Thanks,
Lorenzo