Message ID | 20240614102403.13610-15-shivamurthy.shastri@linutronix.de (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | genirq, irqchip: Convert ARM MSI handling to | expand |
On Fri, 14 Jun 2024 11:23:53 +0100, Shivamurthy Shastri <shivamurthy.shastri@linutronix.de> wrote: > > From: Thomas Gleixner <tglx@linutronix.de> > > Nothing builds a platform_device MSI domain for wire to MSI on top of > this. The "regular" users of the platform MSI domain just provide their own > irq_write_msi_msg() callback. > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > Signed-off-by: Anna-Maria Behnsen <anna-maria@linutronix.de> > Signed-off-by: Shivamurthy Shastri <shivamurthy.shastri@linutronix.de> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Marc Zyngier <maz@kernel.org> > --- > drivers/irqchip/irq-gic-v3-mbi.c | 17 +---------------- > 1 file changed, 1 insertion(+), 16 deletions(-) > > diff --git a/drivers/irqchip/irq-gic-v3-mbi.c b/drivers/irqchip/irq-gic-v3-mbi.c > index dbb8b1efda44..19298cc6c2ee 100644 > --- a/drivers/irqchip/irq-gic-v3-mbi.c > +++ b/drivers/irqchip/irq-gic-v3-mbi.c > @@ -199,31 +199,16 @@ static int mbi_allocate_pci_domain(struct irq_domain *nexus_domain, > } > #endif > > -static void mbi_compose_mbi_msg(struct irq_data *data, struct msi_msg *msg) > -{ > - mbi_compose_msi_msg(data, msg); > - > - msg[1].address_hi = upper_32_bits(mbi_phys_base + GICD_CLRSPI_NSR); > - msg[1].address_lo = lower_32_bits(mbi_phys_base + GICD_CLRSPI_NSR); > - msg[1].data = data->parent_data->hwirq; > - > - iommu_dma_compose_msi_msg(irq_data_get_msi_desc(data), &msg[1]); > -} > - > /* Platform-MSI specific irqchip */ > static struct irq_chip mbi_pmsi_irq_chip = { > .name = "pMSI", > - .irq_set_type = irq_chip_set_type_parent, > - .irq_compose_msi_msg = mbi_compose_mbi_msg, > - .flags = IRQCHIP_SUPPORTS_LEVEL_MSI, > }; > > static struct msi_domain_ops mbi_pmsi_ops = { > }; > > static struct msi_domain_info mbi_pmsi_domain_info = { > - .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS | > - MSI_FLAG_LEVEL_CAPABLE), > + .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS), > .ops = &mbi_pmsi_ops, > .chip = &mbi_pmsi_irq_chip, > }; This patch doesn't do what it says. It simply kills any form of level MSI support for *endpoints*, and has nothing to do with any sort of "wire to MSI". What replaces it? M.
On Sat, Jun 15 2024 at 18:24, Marc Zyngier wrote: > On Fri, 14 Jun 2024 11:23:53 +0100, > Shivamurthy Shastri <shivamurthy.shastri@linutronix.de> wrote: >> static struct msi_domain_info mbi_pmsi_domain_info = { >> - .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS | >> - MSI_FLAG_LEVEL_CAPABLE), >> + .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS), >> .ops = &mbi_pmsi_ops, >> .chip = &mbi_pmsi_irq_chip, >> }; > > This patch doesn't do what it says. It simply kills any form of level > MSI support for *endpoints*, and has nothing to do with any sort of > "wire to MSI". > > What replaces it? Patch 9/24 switches the wire to MSI with level support over. This just removes the leftovers.
On Mon, 17 Jun 2024 13:55:24 +0100, Thomas Gleixner <tglx@linutronix.de> wrote: > > On Sat, Jun 15 2024 at 18:24, Marc Zyngier wrote: > > On Fri, 14 Jun 2024 11:23:53 +0100, > > Shivamurthy Shastri <shivamurthy.shastri@linutronix.de> wrote: > >> static struct msi_domain_info mbi_pmsi_domain_info = { > >> - .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS | > >> - MSI_FLAG_LEVEL_CAPABLE), > >> + .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS), > >> .ops = &mbi_pmsi_ops, > >> .chip = &mbi_pmsi_irq_chip, > >> }; > > > > This patch doesn't do what it says. It simply kills any form of level > > MSI support for *endpoints*, and has nothing to do with any sort of > > "wire to MSI". > > > > What replaces it? > > Patch 9/24 switches the wire to MSI with level support over. This just > removes the leftovers. That's not what I read. Patch 9/24 rewrites the mbigen driver. Which has nothing to do with what the gic-v3-mbi code does. They are different blocks, and the sole machine that has the mbigen IP doesn't have any gic-v3-mbi support. All they have in common are 3 random letters. What you are doing here is to kill any support for *devices* that need to signal level-triggered MSIs in that driver, and nothing to do with wire-MSI translation. So what replaces it? M.
On Mon, Jun 17 2024 at 14:03, Marc Zyngier wrote: > On Mon, 17 Jun 2024 13:55:24 +0100, > Thomas Gleixner <tglx@linutronix.de> wrote: >> >> On Sat, Jun 15 2024 at 18:24, Marc Zyngier wrote: >> > On Fri, 14 Jun 2024 11:23:53 +0100, >> > Shivamurthy Shastri <shivamurthy.shastri@linutronix.de> wrote: >> >> static struct msi_domain_info mbi_pmsi_domain_info = { >> >> - .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS | >> >> - MSI_FLAG_LEVEL_CAPABLE), >> >> + .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS), >> >> .ops = &mbi_pmsi_ops, >> >> .chip = &mbi_pmsi_irq_chip, >> >> }; >> > >> > This patch doesn't do what it says. It simply kills any form of level >> > MSI support for *endpoints*, and has nothing to do with any sort of >> > "wire to MSI". >> > >> > What replaces it? >> >> Patch 9/24 switches the wire to MSI with level support over. This just >> removes the leftovers. > > That's not what I read. > > Patch 9/24 rewrites the mbigen driver. Which has nothing to do with > what the gic-v3-mbi code does. They are different blocks, and the sole > machine that has the mbigen IP doesn't have any gic-v3-mbi support. > All they have in common are 3 random letters. > > What you are doing here is to kill any support for *devices* that need > to signal level-triggered MSIs in that driver, and nothing to do with > wire-MSI translation. > > So what replaces it? Hrm. I must have misread this mess. Let me stare some more.
On Mon, Jun 17 2024 at 16:02, Thomas Gleixner wrote: > On Mon, Jun 17 2024 at 14:03, Marc Zyngier wrote: >> Patch 9/24 rewrites the mbigen driver. Which has nothing to do with >> what the gic-v3-mbi code does. They are different blocks, and the sole >> machine that has the mbigen IP doesn't have any gic-v3-mbi support. >> All they have in common are 3 random letters. >> >> What you are doing here is to kill any support for *devices* that need >> to signal level-triggered MSIs in that driver, and nothing to do with >> wire-MSI translation. >> >> So what replaces it? > > Hrm. I must have misread this mess. Let me stare some more. Ok. Found my old notes. AFAICT _all_ users of platform_device_msi_init_and_alloc_irqs(): ufs_qcom_config_esi() smmu_pmu_setup_msi() flexrm_mbox_probe() arm_smmu_setup_msis() hidma_request_msi() mv_xor_v2_probe() just install their special MSI write callback. I don't see any of those setting up LEVEL triggered MSIs. But then I'm might be missing something. If so can you point me please to the usage instance which actually uses level signaled MSI? Thanks, tglx
Gentle ping! On Mon, Jun 17 2024 at 16:15, Thomas Gleixner wrote: > On Mon, Jun 17 2024 at 16:02, Thomas Gleixner wrote: >> On Mon, Jun 17 2024 at 14:03, Marc Zyngier wrote: >>> Patch 9/24 rewrites the mbigen driver. Which has nothing to do with >>> what the gic-v3-mbi code does. They are different blocks, and the sole >>> machine that has the mbigen IP doesn't have any gic-v3-mbi support. >>> All they have in common are 3 random letters. >>> >>> What you are doing here is to kill any support for *devices* that need >>> to signal level-triggered MSIs in that driver, and nothing to do with >>> wire-MSI translation. >>> >>> So what replaces it? >> >> Hrm. I must have misread this mess. Let me stare some more. > > Ok. Found my old notes. > > AFAICT _all_ users of platform_device_msi_init_and_alloc_irqs(): > > ufs_qcom_config_esi() > smmu_pmu_setup_msi() > flexrm_mbox_probe() > arm_smmu_setup_msis() > hidma_request_msi() > mv_xor_v2_probe() > > just install their special MSI write callback. I don't see any of those > setting up LEVEL triggered MSIs. > > But then I'm might be missing something. If so can you point me please > to the usage instance which actually uses level signaled MSI? > > Thanks, > > tglx
On Fri, Jun 21 2024 at 16:04, Thomas Gleixner wrote:
> Gentle ping!
Whatever. I just drop the patch and be done with it.
On Mon, 17 Jun 2024 15:15:44 +0100, Thomas Gleixner <tglx@linutronix.de> wrote: > > On Mon, Jun 17 2024 at 16:02, Thomas Gleixner wrote: > > On Mon, Jun 17 2024 at 14:03, Marc Zyngier wrote: > >> Patch 9/24 rewrites the mbigen driver. Which has nothing to do with > >> what the gic-v3-mbi code does. They are different blocks, and the sole > >> machine that has the mbigen IP doesn't have any gic-v3-mbi support. > >> All they have in common are 3 random letters. > >> > >> What you are doing here is to kill any support for *devices* that need > >> to signal level-triggered MSIs in that driver, and nothing to do with > >> wire-MSI translation. > >> > >> So what replaces it? > > > > Hrm. I must have misread this mess. Let me stare some more. > > Ok. Found my old notes. > > AFAICT _all_ users of platform_device_msi_init_and_alloc_irqs(): > > ufs_qcom_config_esi() > smmu_pmu_setup_msi() > flexrm_mbox_probe() > arm_smmu_setup_msis() > hidma_request_msi() > mv_xor_v2_probe() > > just install their special MSI write callback. I don't see any of those > setting up LEVEL triggered MSIs. > > But then I'm might be missing something. If so can you point me please > to the usage instance which actually uses level signaled MSI? Good question. I'm pretty sure we had *something* at some point that used it, or that was planning on using it. I even vividly remember who was asking for this. But either that never really made it upstream, or they decided to move away from the kernel setting the MSI up and relied on firmware for that (which is fine as long as the device isn't behind an IOMMU). In the end, it begs the question of what we want to do with this feature. I don't think it is a big deal to keep it around, but maybe we should plan for it to be retired. That's independent of this series, IMO. Thanks, M.
On Tue, Jun 25 2024 at 08:37, Marc Zyngier wrote: > On Mon, 17 Jun 2024 15:15:44 +0100, > Thomas Gleixner <tglx@linutronix.de> wrote: >> just install their special MSI write callback. I don't see any of those >> setting up LEVEL triggered MSIs. >> >> But then I'm might be missing something. If so can you point me please >> to the usage instance which actually uses level signaled MSI? > > Good question. I'm pretty sure we had *something* at some point that > used it, or that was planning on using it. I even vividly remember who > was asking for this. > > But either that never really made it upstream, or they decided to move > away from the kernel setting the MSI up and relied on firmware for > that (which is fine as long as the device isn't behind an IOMMU). > > In the end, it begs the question of what we want to do with this > feature. I don't think it is a big deal to keep it around, but maybe > we should plan for it to be retired. That's independent of this > series, IMO. I reworked the conversion so that it keeps it alive. We can remove it later nevertheless :) Thanks, tglx
diff --git a/drivers/irqchip/irq-gic-v3-mbi.c b/drivers/irqchip/irq-gic-v3-mbi.c index dbb8b1efda44..19298cc6c2ee 100644 --- a/drivers/irqchip/irq-gic-v3-mbi.c +++ b/drivers/irqchip/irq-gic-v3-mbi.c @@ -199,31 +199,16 @@ static int mbi_allocate_pci_domain(struct irq_domain *nexus_domain, } #endif -static void mbi_compose_mbi_msg(struct irq_data *data, struct msi_msg *msg) -{ - mbi_compose_msi_msg(data, msg); - - msg[1].address_hi = upper_32_bits(mbi_phys_base + GICD_CLRSPI_NSR); - msg[1].address_lo = lower_32_bits(mbi_phys_base + GICD_CLRSPI_NSR); - msg[1].data = data->parent_data->hwirq; - - iommu_dma_compose_msi_msg(irq_data_get_msi_desc(data), &msg[1]); -} - /* Platform-MSI specific irqchip */ static struct irq_chip mbi_pmsi_irq_chip = { .name = "pMSI", - .irq_set_type = irq_chip_set_type_parent, - .irq_compose_msi_msg = mbi_compose_mbi_msg, - .flags = IRQCHIP_SUPPORTS_LEVEL_MSI, }; static struct msi_domain_ops mbi_pmsi_ops = { }; static struct msi_domain_info mbi_pmsi_domain_info = { - .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS | - MSI_FLAG_LEVEL_CAPABLE), + .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS), .ops = &mbi_pmsi_ops, .chip = &mbi_pmsi_irq_chip, };