diff mbox series

[tip:,irq/msi] genirq/irqdomain: Remove the param count restriction from select()

Message ID 170802702416.398.14922976721740218856.tip-bot2@tip-bot2
State New
Headers
Series [tip:,irq/msi] genirq/irqdomain: Remove the param count restriction from select() |

Commit Message

tip-bot2 for Thomas Gleixner Feb. 15, 2024, 7:57 p.m. UTC 
  The following commit has been merged into the irq/msi branch of tip:

Commit-ID:     de1ff306dcf4546d6a8863b1f956335e0d3fbb9b
Gitweb:        https://git.kernel.org/tip/de1ff306dcf4546d6a8863b1f956335e0d3fbb9b
Author:        Thomas Gleixner <tglx@linutronix.de>
AuthorDate:    Sat, 27 Jan 2024 21:47:30 +05:30
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Thu, 15 Feb 2024 17:55:39 +01:00

genirq/irqdomain: Remove the param count restriction from select()

Now that the GIC-v3 callback can handle invocation with a fwspec parameter
count of 0 lift the restriction in the core code and invoke select()
unconditionally when the domain provides it.

Preparatory change for per device MSI domains.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lore.kernel.org/r/20240127161753.114685-3-apatel@ventanamicro.com

---
 kernel/irq/irqdomain.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Marc Zyngier Feb. 19, 2024, 3:56 p.m. UTC | #1 
On Mon, 19 Feb 2024 15:50:36 +0000,
Biju Das <biju.das.jz@bp.renesas.com> wrote:
> 
> > Now that the GIC-v3 callback can handle invocation with a fwspec parameter
> > count of 0 lift the restriction in the core code and invoke select()
> > unconditionally when the domain provides it.
> 
> This patch breaks on RZ/G2L SMARC EVK as of_phandle_args_to_fwspec count()
> is called after irq_find_matching_fwspec() is causing fwspec->param_count=0
> and this results in boot failure as the patch removes the check.
> 
> Maybe we need to revert this patch or fix the fundamental issue.
> 
> Cheers,
> Biju
> ---
>  kernel/irq/irqdomain.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index 0bdef4f..8fee379 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -448,7 +448,7 @@ struct irq_domain *irq_find_matching_fwspec(struct irq_fwspec *fwspec,
>  	 */
>  	mutex_lock(&irq_domain_mutex);
>  	list_for_each_entry(h, &irq_domain_list, link) {
> -		if (h->ops->select && fwspec->param_count)
> +		if (h->ops->select)
>  			rc = h->ops->select(h, fwspec, bus_token);
>  		else if (h->ops->match)
>  			rc = h->ops->match(h, to_of_node(fwnode), bus_token);
> 
> 

Dmitry posted his take on this at [1], and I have suggested another
possible fix in my reply.

Could you please give both patches a go?

Thanks,

	M.

[1] https://lore.kernel.org/r/20240219-gic-fix-child-domain-v1-1-09f8fd2d9a8f@linaro.org
Biju Das Feb. 19, 2024, 4:39 p.m. UTC | #2 
Hi Marc Zyngier,

Thanks for the feedback.

> -----Original Message-----
> From: Marc Zyngier <maz@kernel.org>
> Sent: Monday, February 19, 2024 3:57 PM
> Subject: Re: [tip: irq/msi] genirq/irqdomain: Remove the param count
> restriction from select()
> 
> On Mon, 19 Feb 2024 15:50:36 +0000,
> Biju Das <biju.das.jz@bp.renesas.com> wrote:
> >
> > > Now that the GIC-v3 callback can handle invocation with a fwspec
> > > parameter count of 0 lift the restriction in the core code and
> > > invoke select() unconditionally when the domain provides it.
> >
> > This patch breaks on RZ/G2L SMARC EVK as of_phandle_args_to_fwspec
> > count() is called after irq_find_matching_fwspec() is causing
> > fwspec->param_count=0 and this results in boot failure as the patch
> removes the check.
> >
> > Maybe we need to revert this patch or fix the fundamental issue.
> >
> > Cheers,
> > Biju
> > ---
> >  kernel/irq/irqdomain.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c index
> > 0bdef4f..8fee379 100644
> > --- a/kernel/irq/irqdomain.c
> > +++ b/kernel/irq/irqdomain.c
> > @@ -448,7 +448,7 @@ struct irq_domain *irq_find_matching_fwspec(struct
> irq_fwspec *fwspec,
> >  	 */
> >  	mutex_lock(&irq_domain_mutex);
> >  	list_for_each_entry(h, &irq_domain_list, link) {
> > -		if (h->ops->select && fwspec->param_count)
> > +		if (h->ops->select)
> >  			rc = h->ops->select(h, fwspec, bus_token);
> >  		else if (h->ops->match)
> >  			rc = h->ops->match(h, to_of_node(fwnode), bus_token);
> >
> >
> 
> Dmitry posted his take on this at [1], and I have suggested another
> possible fix in my reply.
> 
> Could you please give both patches a go?

I tested and confirm both the patches looks good.

Cheers,
Biju
Biju Das Feb. 19, 2024, 5:39 p.m. UTC | #3 
Hi Marc and Dmitry,

> -----Original Message-----
> From: Biju Das
> Sent: Monday, February 19, 2024 4:39 PM
> Subject: RE: [tip: irq/msi] genirq/irqdomain: Remove the param count
> restriction from select()
> 
> Hi Marc Zyngier,
> 
> Thanks for the feedback.
> 
> > -----Original Message-----
> > From: Marc Zyngier <maz@kernel.org>
> > Sent: Monday, February 19, 2024 3:57 PM
> > Subject: Re: [tip: irq/msi] genirq/irqdomain: Remove the param count
> > restriction from select()
> >
> > On Mon, 19 Feb 2024 15:50:36 +0000,
> > Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > >
> > > > Now that the GIC-v3 callback can handle invocation with a fwspec
> > > > parameter count of 0 lift the restriction in the core code and
> > > > invoke select() unconditionally when the domain provides it.
> > >
> > > This patch breaks on RZ/G2L SMARC EVK as of_phandle_args_to_fwspec
> > > count() is called after irq_find_matching_fwspec() is causing
> > > fwspec->param_count=0 and this results in boot failure as the patch
> > removes the check.
> > >
> > > Maybe we need to revert this patch or fix the fundamental issue.
> > >
> > > Cheers,
> > > Biju
> > > ---
> > >  kernel/irq/irqdomain.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c index
> > > 0bdef4f..8fee379 100644
> > > --- a/kernel/irq/irqdomain.c
> > > +++ b/kernel/irq/irqdomain.c
> > > @@ -448,7 +448,7 @@ struct irq_domain
> > > *irq_find_matching_fwspec(struct
> > irq_fwspec *fwspec,
> > >  	 */
> > >  	mutex_lock(&irq_domain_mutex);
> > >  	list_for_each_entry(h, &irq_domain_list, link) {
> > > -		if (h->ops->select && fwspec->param_count)
> > > +		if (h->ops->select)
> > >  			rc = h->ops->select(h, fwspec, bus_token);
> > >  		else if (h->ops->match)
> > >  			rc = h->ops->match(h, to_of_node(fwnode), bus_token);
> > >
> > >
> >
> > Dmitry posted his take on this at [1], and I have suggested another
> > possible fix in my reply.
> >
> > Could you please give both patches a go?
> 
> I tested and confirm both the patches looks good.

Please let me know the details of the final patch, so that I can test and 
add tested-by tag for Renesas RZ/G2L platforms.

Cheers,
Biju
Thomas Gleixner Feb. 20, 2024, 8:50 a.m. UTC | #4 
On Mon, Feb 19 2024 at 15:56, Marc Zyngier wrote:
>> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
>> index 0bdef4f..8fee379 100644
>> --- a/kernel/irq/irqdomain.c
>> +++ b/kernel/irq/irqdomain.c
>> @@ -448,7 +448,7 @@ struct irq_domain *irq_find_matching_fwspec(struct irq_fwspec *fwspec,
>>  	 */
>>  	mutex_lock(&irq_domain_mutex);
>>  	list_for_each_entry(h, &irq_domain_list, link) {
>> -		if (h->ops->select && fwspec->param_count)
>> +		if (h->ops->select)
>>  			rc = h->ops->select(h, fwspec, bus_token);
>>  		else if (h->ops->match)
>>  			rc = h->ops->match(h, to_of_node(fwnode), bus_token);
>> 
>> 
>
> Dmitry posted his take on this at [1], and I have suggested another
> possible fix in my reply.

Your core side DOMAIN_BUS_ANY variant makes a lot of sense. Can you
please post a proper patch for that?

Aside of this I just noticed that we need the below too.

Thanks,

        tglx
---
Subject: irqchip/imx-intmux: Handle pure domain searches correctly
From: Thomas Gleixner <tglx@linutronix.de>
Date: Tue, 20 Feb 2024 09:46:19 +0100

The removal of the paremeter count restriction in the core code to allow
pure domain token based select() decisions broke the IMX intmux select
callback as that unconditioally expects that there is a parameter.

Add the missing check for zero parameter count and the token match.

Fixes: de1ff306dcf4 ("genirq/irqdomain: Remove the param count restriction from select()")
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/irqchip/irq-imx-intmux.c |    4 ++++
 1 file changed, 4 insertions(+)

--- a/drivers/irqchip/irq-imx-intmux.c
+++ b/drivers/irqchip/irq-imx-intmux.c
@@ -166,6 +166,10 @@ static int imx_intmux_irq_select(struct
 	if (fwspec->fwnode != d->fwnode)
 		return false;
 
+	/* Handle pure domain searches */
+	if (!fwspec->param_count)
+		return d->bus_token == bus_token;
+
 	return irqchip_data->chanidx == fwspec->param[1];
 }
diff mbox series

Patch

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 0bdef4f..8fee379 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -448,7 +448,7 @@  struct irq_domain *irq_find_matching_fwspec(struct irq_fwspec *fwspec,
 	 */
 	mutex_lock(&irq_domain_mutex);
 	list_for_each_entry(h, &irq_domain_list, link) {
-		if (h->ops->select && fwspec->param_count)
+		if (h->ops->select)
 			rc = h->ops->select(h, fwspec, bus_token);
 		else if (h->ops->match)
 			rc = h->ops->match(h, to_of_node(fwnode), bus_token);