diff mbox series

[v3,14/24] genirq/gic-v3-mbi: Remove unused wired MSI mechanics

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

Commit Message

Shivamurthy Shastri June 14, 2024, 10:23 a.m. UTC
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(-)

Comments

Marc Zyngier June 15, 2024, 5:24 p.m. UTC | #1
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.
Thomas Gleixner June 17, 2024, 12:55 p.m. UTC | #2
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.
Marc Zyngier June 17, 2024, 1:03 p.m. UTC | #3
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.
Thomas Gleixner June 17, 2024, 2:02 p.m. UTC | #4
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.
Thomas Gleixner June 17, 2024, 2:15 p.m. UTC | #5
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
Thomas Gleixner June 21, 2024, 2:04 p.m. UTC | #6
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
Thomas Gleixner June 23, 2024, 11:26 a.m. UTC | #7
On Fri, Jun 21 2024 at 16:04, Thomas Gleixner wrote:
> Gentle ping!

Whatever. I just drop the patch and be done with it.
Marc Zyngier June 25, 2024, 7:37 a.m. UTC | #8
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.
Thomas Gleixner June 25, 2024, 7:47 a.m. UTC | #9
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 mbox series

Patch

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,
 };