| Message ID | 20250103125926.54203-1-luxu.kernel@bytedance.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | irqchip/riscv-imsic: Support allocating multiple msi-x vectors | expand |
On Fri, Jan 3, 2025 at 6:30 PM Xu Lu <luxu.kernel@bytedance.com> wrote: > > This commit modifies alloc and free callback of imsic_base_domain to > support allocating and freeing multiple msi-x vectors at once. There are two separate concepts here: "multiple MSI-X vectors" and "multi MSI vector". For multiple MSI-X vectors, we don't need to do anything special since each MSI-X vector will be a separate MSI message (aka MSI address and MSI data). We have tested the IMSIC driver for PCIe devices with multiple MSI-X vectors and it works fine because the PCI framework sets up a separate MSI-X descriptor for each MSI-X vector [1]. For multi MSI, there will be a single MSI address and multiple consecutive MSI data values for multiple interrupts. This is not properly supported by the AIA v1.0 specification because changing the MSI address of a single interrupt would change the MSI address of all interrupts. The currently the Linux IMSIC driver does not support multi MSI. In fact, there was a partial multi MSI support in the v11 of Linux AIA driver series [2] which was later dropped due to past reasoning provided in the past by Thomas [3]. AFAIK, most PCIe devices use MSI-X and even Linux arch/x86 does not support multi MSI so we really need a strong reason to support multi MSI in future AIA specification or Linux IMSIC driver. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/msi/msi.c?h=v6.13-rc5#n622 [2] https://www.spinics.net/lists/devicetree/msg643772.html [3] https://lore.kernel.org/all/877d7yhve7.ffs@tglx/ > > Signed-off-by: Xu Lu <luxu.kernel@bytedance.com> > --- > drivers/irqchip/irq-riscv-imsic-platform.c | 54 +++++++++++++--------- > 1 file changed, 32 insertions(+), 22 deletions(-) > > diff --git a/drivers/irqchip/irq-riscv-imsic-platform.c b/drivers/irqchip/irq-riscv-imsic-platform.c > index c708780e8760..d0ea0eb22e2e 100644 > --- a/drivers/irqchip/irq-riscv-imsic-platform.c > +++ b/drivers/irqchip/irq-riscv-imsic-platform.c > @@ -141,35 +141,45 @@ static struct irq_chip imsic_irq_base_chip = { > IRQCHIP_MASK_ON_SUSPEND, > }; > > +static void imsic_irq_domain_free(struct irq_domain *domain, unsigned int virq, > + unsigned int nr_irqs) > +{ > + struct irq_data *d; > + int i; > + > + for (i = 0; i < nr_irqs; i++) { > + d = irq_domain_get_irq_data(domain, virq + i); > + imsic_vector_free(irq_data_get_irq_chip_data(d)); > + } > + irq_domain_free_irqs_top(domain, virq, nr_irqs); > +} > + > static int imsic_irq_domain_alloc(struct irq_domain *domain, unsigned int virq, > unsigned int nr_irqs, void *args) > { > struct imsic_vector *vec; > - > - /* Multi-MSI is not supported yet. */ > - if (nr_irqs > 1) > - return -EOPNOTSUPP; I don't see why we need to handle "nr_irqs > 1" over here because PCI framework already creates a separate MSI-X descriptor for each MSI-X vector. If we do support "nr_irqs > 1" over here then we still need a check which prevents multi MSI (as explained above). > - > - vec = imsic_vector_alloc(virq, cpu_online_mask); > - if (!vec) > - return -ENOSPC; > - > - irq_domain_set_info(domain, virq, virq, &imsic_irq_base_chip, vec, > - handle_simple_irq, NULL, NULL); > - irq_set_noprobe(virq); > - irq_set_affinity(virq, cpu_online_mask); > - irq_data_update_effective_affinity(irq_get_irq_data(virq), cpumask_of(vec->cpu)); > + int i, err; > + > + for (i = 0; i < nr_irqs; i++) { > + vec = imsic_vector_alloc(virq + i, cpu_online_mask); > + if (!vec) { > + err = -ENOSPC; > + goto error; > + } > + > + irq_domain_set_info(domain, virq + i, virq + i, &imsic_irq_base_chip, > + vec, handle_simple_irq, NULL, NULL); > + irq_set_noprobe(virq + i); > + irq_set_affinity(virq + i, cpu_online_mask); > + irq_data_update_effective_affinity(irq_get_irq_data(virq + i), > + cpumask_of(vec->cpu)); > + } > > return 0; > -} > - > -static void imsic_irq_domain_free(struct irq_domain *domain, unsigned int virq, > - unsigned int nr_irqs) > -{ > - struct irq_data *d = irq_domain_get_irq_data(domain, virq); > > - imsic_vector_free(irq_data_get_irq_chip_data(d)); > - irq_domain_free_irqs_parent(domain, virq, nr_irqs); > +error: > + imsic_irq_domain_free(domain, virq, i); > + return err; > } > > static int imsic_irq_domain_select(struct irq_domain *domain, struct irq_fwspec *fwspec, > -- > 2.20.1 > > Regards, Anup
diff --git a/drivers/irqchip/irq-riscv-imsic-platform.c b/drivers/irqchip/irq-riscv-imsic-platform.c index c708780e8760..d0ea0eb22e2e 100644 --- a/drivers/irqchip/irq-riscv-imsic-platform.c +++ b/drivers/irqchip/irq-riscv-imsic-platform.c @@ -141,35 +141,45 @@ static struct irq_chip imsic_irq_base_chip = { IRQCHIP_MASK_ON_SUSPEND, }; +static void imsic_irq_domain_free(struct irq_domain *domain, unsigned int virq, + unsigned int nr_irqs) +{ + struct irq_data *d; + int i; + + for (i = 0; i < nr_irqs; i++) { + d = irq_domain_get_irq_data(domain, virq + i); + imsic_vector_free(irq_data_get_irq_chip_data(d)); + } + irq_domain_free_irqs_top(domain, virq, nr_irqs); +} + static int imsic_irq_domain_alloc(struct irq_domain *domain, unsigned int virq, unsigned int nr_irqs, void *args) { struct imsic_vector *vec; - - /* Multi-MSI is not supported yet. */ - if (nr_irqs > 1) - return -EOPNOTSUPP; - - vec = imsic_vector_alloc(virq, cpu_online_mask); - if (!vec) - return -ENOSPC; - - irq_domain_set_info(domain, virq, virq, &imsic_irq_base_chip, vec, - handle_simple_irq, NULL, NULL); - irq_set_noprobe(virq); - irq_set_affinity(virq, cpu_online_mask); - irq_data_update_effective_affinity(irq_get_irq_data(virq), cpumask_of(vec->cpu)); + int i, err; + + for (i = 0; i < nr_irqs; i++) { + vec = imsic_vector_alloc(virq + i, cpu_online_mask); + if (!vec) { + err = -ENOSPC; + goto error; + } + + irq_domain_set_info(domain, virq + i, virq + i, &imsic_irq_base_chip, + vec, handle_simple_irq, NULL, NULL); + irq_set_noprobe(virq + i); + irq_set_affinity(virq + i, cpu_online_mask); + irq_data_update_effective_affinity(irq_get_irq_data(virq + i), + cpumask_of(vec->cpu)); + } return 0; -} - -static void imsic_irq_domain_free(struct irq_domain *domain, unsigned int virq, - unsigned int nr_irqs) -{ - struct irq_data *d = irq_domain_get_irq_data(domain, virq); - imsic_vector_free(irq_data_get_irq_chip_data(d)); - irq_domain_free_irqs_parent(domain, virq, nr_irqs); +error: + imsic_irq_domain_free(domain, virq, i); + return err; } static int imsic_irq_domain_select(struct irq_domain *domain, struct irq_fwspec *fwspec,
This commit modifies alloc and free callback of imsic_base_domain to support allocating and freeing multiple msi-x vectors at once. Signed-off-by: Xu Lu <luxu.kernel@bytedance.com> --- drivers/irqchip/irq-riscv-imsic-platform.c | 54 +++++++++++++--------- 1 file changed, 32 insertions(+), 22 deletions(-)