Message-ID: <4pdu25dnnqegnd67zf4ftfvwc57bn67kp7mj2gk2cywc3hdcvr@eydar5gvuwtu>
Date: Fri, 16 Jan 2026 20:33:33 +0530
From: Manivannan Sadhasivam <mani@...nel.org>
To: Marc Zyngier <maz@...nel.org>
Cc: tglx@...utronix.de, linux-kernel@...r.kernel.org, 
	Qiang Yu <qiang.yu@....qualcomm.com>
Subject: Re: MSIs not freed in GICv3 ITS driver

On Sun, Jul 21, 2024 at 02:20:32PM +0530, Manivannan Sadhasivam wrote:
> On Tue, Jul 09, 2024 at 08:24:37PM +0100, Marc Zyngier wrote:
> > On Tue, 09 Jul 2024 18:37:08 +0100,
> > Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org> wrote:
> > > 
> > > On Mon, Jul 08, 2024 at 06:31:57PM +0100, Marc Zyngier wrote:
> > > > Mani,
> > > > 
> > > > On Mon, 08 Jul 2024 16:39:33 +0100,
> > > > Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org> wrote:
> > > > > 
> > > > > Hi Marc, Thomas,
> > > > > 
> > > > > I'm seeing a weird behavior with GICv3 ITS driver while allocating MSIs from
> > > > > PCIe devices. When the PCIe driver (I'm using virtio_pci_common.c) tries to
> > > > > allocate non power of 2 MSIs (like 3), then the GICv3 MSI driver always rounds
> > > > > the MSI count to power of 2 to find the order. In this case, the order becomes 2
> > > > > in its_alloc_device_irq().
> > > > 
> > > > That's because we can only allocate EventIDs as a number of ID
> > > > bits. So you can't have *1* MSI, nor 3. You can have 2, 4, 8, or
> > > > 2^24. This is a power-of-two architecture.
> > > > 
> > > 
> > > Ah okay.
> > > 
> > > > > So 4 entries are allocated by bitmap_find_free_region().
> > > > 
> > > > Assuming you're calling about its_alloc_device_irq(), it looks like a
> > > > bug. Or rather, some laziness on my part. The thing is, this bitmap is
> > > > only dealing with sub-allocation in the pool that has been given to
> > > > the endpoint. So the power-of-two crap doesn't really matter unless
> > > > you are dealing with Multi-MSI, which has actual alignment
> > > > requirements.
> > > > 
> > > 
> > > Okay.
> > > 
> > > > >
> > > > > But since the PCIe driver has only requested 3 MSIs, its_irq_domain_alloc()
> > > > > will only allocate 3 MSIs, leaving one bitmap entry unused.
> > > > > 
> > > > > And when the driver frees the MSIs using pci_free_irq_vectors(), only 3
> > > > > allocated MSIs were freed and their bitmap entries were also released. But the
> > > > > entry for the additional bitmap was never released. Due to this,
> > > > > its_free_device() was also never called, resulting in the ITS device not getting
> > > > > freed.
> > > > > 
> > > > > So when the PCIe driver tries to request the MSIs again (PCIe device being
> > > > > removed and inserted back), because the ITS device was not freed previously,
> > > > > MSIs were again requested for the same ITS device. And due to the stale bitmap
> > > > > entry, the ITS driver refuses to allocate 4 MSIs as only 3 bitmap entries were
> > > > > available. This forces the PCIe driver to reduce the MSI count, which is sub
> > > > > optimal.
> > > > > 
> > > > > This behavior might be applicable to other irqchip drivers handling MSI as well.
> > > > > I want to know if this behavior is already known with MSI and irqchip drivers?
> > > > > 
> > > > > For fixing this issue, the PCIe drivers could always request MSIs of power of 2,
> > > > > and use a dummy MSI handler for the extra number of MSIs allocated. This could
> > > > > also be done in the generic MSI driver itself to avoid changes in the PCIe
> > > > > drivers. But I wouldn't say it is the best possible fix.
> > > > 
> > > > No, that's terrible. This is just papering over a design mistake, and
> > > > I refuse to go down that road.
> > > > 
> > > 
> > > Agree. But what about other MSI drivers? And because of the MSI design, they
> > > also round the requested MSI count to power of 2, leading to unused vectors and
> > > those also wouldn't get freed.
> > 
> > This has absolutely nothing to do with the "design" of MSIs. It has
> > everything to do with not special-casing Multi-MSI.
> > 
> > > I think this power of 2 limitation should be
> > > imposed at the API level or in the MSI driver instead of silently keeping unused
> > > vectors in irqchip drivers.
> > 
> > You really have the wrong end of the stick. The MSi API has *zero*
> > control over the payload allocation. How could it? The whole point of
> > having an MSI driver is to insulate the core code from such stuff.
> > 
> 
> Right, but because of the way most of the MSI drivers (not all?) use bitmap to
> allocate MSIs, this issue is also present in all of them. So I think the fix is
> not just applicable for the gic-its-v3 driver alone.
> 
> > >
> > > > > 
> > > > > Is there any other way to address this issue? Or am I missing something
> > > > > completely?
> > > > 
> > > > Well, since each endpoint handled by an ITS has its allocation tracked
> > > > by a bitmap, it makes more sense to precisely track the allocation.
> > > > 
> > > > Here's a quick hack that managed to survive a VM boot. It may even
> > > > work. The only problem with it is that it probably breaks a Multi-MSi
> > > > device sitting behind a non-transparent bridge that would get its MSIs
> > > > allocated after another device. In this case, we wouldn't honor the
> > > > required alignment and things would break.
> > > > 
> > > > So take this as a proof of concept. If that works, I'll think of how
> > > > to deal with this crap in a more suitable way...
> > > > 
> > > 
> > > This works fine. Now the ITS driver allocates requested number of MSIs, thanks!
> > 
> > Well, as I said, this breaks tons of other things so I'm not going to
> > merge this any time soon. Certainly not before Thomas gets his MSI
> > rework upstream. And then I need to work out how to deal with
> > Multi-MSI in the correct way.
> > 
> > So don't hold your breath.
> > 
> 
> Sure thing. Thanks for getting it around quickly. I'll wait for a proper fix.
> 

Hi Marc,

Looks like this has fallen through the cracks and my colleage internally
reported a warning during the removal of a PCI driver and it seems to be related
to the issue we were discussing in this thread:

[   54.727284] WARNING: drivers/irqchip/irq-gic-v3-its.c:3639 at its_msi_teardown+0x11c/0x13c, CPU#4: kworker/u73:1/115
[   54.738366] Modules linked in: mhi_pci_generic mhi nvme_core usb_f_fs libcomposite sm3_ce nvmem_qcom_spmi_sdam qcom_pon rtc_pm8xxx qcom_spmi_temp_alarm qcom_stats dispcc_glymur gpi llcc_qcom phy_qcom_qmp_pcie qcom_cpucp_mbox qcom_wdt socinfo
[   54.760588] CPU: 4 UID: 0 PID: 115 Comm: kworker/u73:1 Tainted: G        W           6.18.0-next-20251210-14099-gc20082c23661-dirty #2 PREEMPT
[   54.774067] Tainted: [W]=WARN
[   54.777412] Hardware name: Qualcomm MTP/Qualcomm Test Device, BIOS 7.0.251121.BOOT.OSSUEFI.3.1-00008-GLYMUR-1 11/21/2025
[   54.788849] Workqueue: async async_run_entry_fn
[   54.793791] pstate: 21400009 (nzCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
[   54.801230] pc : its_msi_teardown+0x11c/0x13c
[   54.805997] lr : its_msi_teardown+0x54/0x13c
[   54.810675] sp : ffff8000837cb710
[   54.814373] x29: ffff8000837cb710 x28: ffff00080190e410 x27: ffff0008085ba390
[   54.821985] x26: ffff000808629bf0 x25: 0000000000000000 x24: 0000000000000066
[   54.829602] x23: 0000000000000007 x22: 0000000000000020 x21: ffff000800059608
[   54.837209] x20: ffff000800059607 x19: ffff000800a4a300 x18: 00000000ffffffff
[   54.844819] x17: ffff00080ec65400 x16: ffff00080ec65200 x15: ffff00080ec65000
[   54.852429] x14: 0000000000000004 x13: ffff0008000b8810 x12: 0000000000000000
[   54.860046] x11: ffff0008007798e8 x10: 0000000000000002 x9 : 0000000000000001
[   54.867661] x8 : ffff0008007796f8 x7 : 000000000000001f x6 : ffff8000837cb640
[   54.875277] x5 : ffff000801918f40 x4 : 0000000000000007 x3 : 0000000000000000
[   54.882891] x2 : ffff000800a037c0 x1 : 0000000000000020 x0 : 0000000000000007
[   54.890509] Call trace:
[   54.893320]  its_msi_teardown+0x11c/0x13c (P)
[   54.898082]  its_msi_teardown+0x34/0x44
[   54.902316]  msi_remove_device_irq_domain+0x70/0x114
[   54.907701]  msi_device_data_release+0x20/0x64
[   54.912551]  devres_release_all+0xa4/0x104

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Powered by blists - more mailing lists