Now that .msi_prepare() gets called at the right time and not
with semi-random parameters, remove the ugly hack that tried
to fix up the number of allocated vectors.
It is now correct by construction.
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
drivers/irqchip/irq-gic-v3-its-msi-parent.c | 19 -------------------
1 file changed, 19 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v3-its-msi-parent.c b/drivers/irqchip/irq-gic-v3-its-msi-parent.c
index 958736622fa57..6a5f64f120d4a 100644
--- a/drivers/irqchip/irq-gic-v3-its-msi-parent.c
+++ b/drivers/irqchip/irq-gic-v3-its-msi-parent.c
@@ -67,17 +67,6 @@ static int its_pci_msi_prepare(struct irq_domain *domain, struct device *dev,
/* ITS specific DeviceID, as the core ITS ignores dev. */
info->scratchpad[0].ul = pci_msi_domain_get_msi_rid(domain->parent, pdev);
- /*
- * @domain->msi_domain_info->hwsize contains the size of the
- * MSI[-X] domain, but vector allocation happens one by one. This
- * needs some thought when MSI comes into play as the size of MSI
- * might be unknown at domain creation time and therefore set to
- * MSI_MAX_INDEX.
- */
- msi_info = msi_get_domain_info(domain);
- if (msi_info->hwsize > nvec)
- nvec = msi_info->hwsize;
-
/*
* Always allocate a power of 2, and special case device 0 for
* broken systems where the DevID is not wired (and all devices
@@ -151,14 +140,6 @@ static int its_pmsi_prepare(struct irq_domain *domain, struct device *dev,
/* ITS specific DeviceID, as the core ITS ignores dev. */
info->scratchpad[0].ul = dev_id;
- /*
- * @domain->msi_domain_info->hwsize contains the size of the device
- * domain, but vector allocation happens one by one.
- */
- msi_info = msi_get_domain_info(domain);
- if (msi_info->hwsize > nvec)
- nvec = msi_info->hwsize;
-
/* Allocate at least 32 MSIs, and always as a power of 2 */
nvec = max_t(int, 32, roundup_pow_of_two(nvec));
--
2.39.2On Tue, May 13 2025 at 17:31, Marc Zyngier wrote:
> Now that .msi_prepare() gets called at the right time and not
> with semi-random parameters, remove the ugly hack that tried
> to fix up the number of allocated vectors.
>
> It is now correct by construction.
FWIW, while looking at something related, it occured to me that with
this change you can enable MSI_FLAG_PCI_MSIX_ALLOC_DYN now on GIC ITS.
Thanks,
tglx
On Sun, 18 May 2025 19:53:42 +0100, Thomas Gleixner <tglx@linutronix.de> wrote: > > On Tue, May 13 2025 at 17:31, Marc Zyngier wrote: > > > Now that .msi_prepare() gets called at the right time and not > > with semi-random parameters, remove the ugly hack that tried > > to fix up the number of allocated vectors. > > > > It is now correct by construction. > > FWIW, while looking at something related, it occured to me that with > this change you can enable MSI_FLAG_PCI_MSIX_ALLOC_DYN now on GIC ITS. Maybe. It is rather unclear to me what this "dynamic allocation" actually provides in terms of guarantees to the endpoint driver. M. -- Without deviation from the norm, progress is not possible.
On Mon, May 19 2025 at 11:15, Marc Zyngier wrote:
> On Sun, 18 May 2025 19:53:42 +0100,
> Thomas Gleixner <tglx@linutronix.de> wrote:
>>
>> On Tue, May 13 2025 at 17:31, Marc Zyngier wrote:
>>
>> > Now that .msi_prepare() gets called at the right time and not
>> > with semi-random parameters, remove the ugly hack that tried
>> > to fix up the number of allocated vectors.
>> >
>> > It is now correct by construction.
>>
>> FWIW, while looking at something related, it occured to me that with
>> this change you can enable MSI_FLAG_PCI_MSIX_ALLOC_DYN now on GIC ITS.
>
> Maybe. It is rather unclear to me what this "dynamic allocation"
> actually provides in terms of guarantees to the endpoint driver.
It allows the driver to avoid allocating a gazillion of interrupts
upfront during initialization. Instead it can allocate them on demand,
when e.g. a queue is initialized. Of course that means that such an
allocation can fail, but so can request_irq() and other things. I'm not
sure what you mean with guarantees here.
Thanks
tglx
On Mon, 19 May 2025 13:16:58 +0100, Thomas Gleixner <tglx@linutronix.de> wrote: > > On Mon, May 19 2025 at 11:15, Marc Zyngier wrote: > > On Sun, 18 May 2025 19:53:42 +0100, > > Thomas Gleixner <tglx@linutronix.de> wrote: > >> > >> On Tue, May 13 2025 at 17:31, Marc Zyngier wrote: > >> > >> > Now that .msi_prepare() gets called at the right time and not > >> > with semi-random parameters, remove the ugly hack that tried > >> > to fix up the number of allocated vectors. > >> > > >> > It is now correct by construction. > >> > >> FWIW, while looking at something related, it occured to me that with > >> this change you can enable MSI_FLAG_PCI_MSIX_ALLOC_DYN now on GIC ITS. > > > > Maybe. It is rather unclear to me what this "dynamic allocation" > > actually provides in terms of guarantees to the endpoint driver. > > It allows the driver to avoid allocating a gazillion of interrupts > upfront during initialization. Instead it can allocate them on demand, > when e.g. a queue is initialized. Of course that means that such an > allocation can fail, but so can request_irq() and other things. I'm not > sure what you mean with guarantees here. What is the endpoint driver allowed to expect in terms of continuity of allocation in the IRQ space? If this is solely limited to MSI-X, then the answer probably is "none whatsoever", and the driver should only manage the MSI descriptor index. Can any other MSI-like mechanism end-up with multiple allocations and require extra alignment/contiguity guarantees in the hwirq space, more or less similar to what MultiMSI requires? Because that'd be much harder to provide. M. -- Without deviation from the norm, progress is not possible.
On Mon, May 19 2025 at 15:28, Marc Zyngier wrote:
> On Mon, 19 May 2025 13:16:58 +0100,
> Thomas Gleixner <tglx@linutronix.de> wrote:
>> > Maybe. It is rather unclear to me what this "dynamic allocation"
>> > actually provides in terms of guarantees to the endpoint driver.
>>
>> It allows the driver to avoid allocating a gazillion of interrupts
>> upfront during initialization. Instead it can allocate them on demand,
>> when e.g. a queue is initialized. Of course that means that such an
>> allocation can fail, but so can request_irq() and other things. I'm not
>> sure what you mean with guarantees here.
>
> What is the endpoint driver allowed to expect in terms of continuity
> of allocation in the IRQ space? If this is solely limited to MSI-X,
> then the answer probably is "none whatsoever", and the driver should
> only manage the MSI descriptor index.
>
> Can any other MSI-like mechanism end-up with multiple allocations and
> require extra alignment/contiguity guarantees in the hwirq space, more
> or less similar to what MultiMSI requires? Because that'd be much
> harder to provide.
It's only relevant to MSI-X today. That's the only facility, which
actually provides an interface _if_ the underlying parent supports it.
static const struct msi_domain_template pci_msix_template = {
....
.info = {
.flags = MSI_COMMON_FLAGS | MSI_FLAG_PCI_MSIX |
MSI_FLAG_PCI_MSIX_ALLOC_DYN,
.bus_token = DOMAIN_BUS_PCI_DEVICE_MSIX,
},
};
That's the device domain template, which requests the functionality and
the core then checks whether the parent domain supports it. If so the
functionality is enabled.
Thanks,
tglx
The following commit has been merged into the irq/msi branch of tip:
Commit-ID: 7dd20bf2f010430b75b109e4d4101cd1616afba3
Gitweb: https://git.kernel.org/tip/7dd20bf2f010430b75b109e4d4101cd1616afba3
Author: Marc Zyngier <maz@kernel.org>
AuthorDate: Tue, 13 May 2025 17:31:44 +01:00
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Wed, 14 May 2025 12:36:42 +02:00
irqchip/gic-v3-its: Use allocation size from the prepare call
Now that .msi_prepare() gets called at the right time and not
with semi-random parameters, remove the ugly hack that tried
to fix up the number of allocated vectors.
It is now correct by construction.
Signed-off-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lore.kernel.org/all/20250513163144.2215824-6-maz@kernel.org
---
drivers/irqchip/irq-gic-v3-its-msi-parent.c | 19 -------------------
1 file changed, 19 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v3-its-msi-parent.c b/drivers/irqchip/irq-gic-v3-its-msi-parent.c
index 9587366..6a5f64f 100644
--- a/drivers/irqchip/irq-gic-v3-its-msi-parent.c
+++ b/drivers/irqchip/irq-gic-v3-its-msi-parent.c
@@ -68,17 +68,6 @@ static int its_pci_msi_prepare(struct irq_domain *domain, struct device *dev,
info->scratchpad[0].ul = pci_msi_domain_get_msi_rid(domain->parent, pdev);
/*
- * @domain->msi_domain_info->hwsize contains the size of the
- * MSI[-X] domain, but vector allocation happens one by one. This
- * needs some thought when MSI comes into play as the size of MSI
- * might be unknown at domain creation time and therefore set to
- * MSI_MAX_INDEX.
- */
- msi_info = msi_get_domain_info(domain);
- if (msi_info->hwsize > nvec)
- nvec = msi_info->hwsize;
-
- /*
* Always allocate a power of 2, and special case device 0 for
* broken systems where the DevID is not wired (and all devices
* appear as DevID 0). For that reason, we generously allocate a
@@ -151,14 +140,6 @@ static int its_pmsi_prepare(struct irq_domain *domain, struct device *dev,
/* ITS specific DeviceID, as the core ITS ignores dev. */
info->scratchpad[0].ul = dev_id;
- /*
- * @domain->msi_domain_info->hwsize contains the size of the device
- * domain, but vector allocation happens one by one.
- */
- msi_info = msi_get_domain_info(domain);
- if (msi_info->hwsize > nvec)
- nvec = msi_info->hwsize;
-
/* Allocate at least 32 MSIs, and always as a power of 2 */
nvec = max_t(int, 32, roundup_pow_of_two(nvec));© 2016 - 2025 Red Hat, Inc.