[PATCH v2] irqchip/gic: Allow self-SGIs for SMP on UP configurations

[PATCH v2] irqchip/gic: Allow self-SGIs for SMP on UP configurations

[Fabio Estevam]

From: Marc Zyngier <marc.zyngier@arm.com>

commit 059e232089e45b0befc9933d31209c225e08b426 upstream.

On systems where a single CPU is present, the GIC may not support
having SGIs delivered to a target list. In that case, we use the
self-SGI mechanism to allow the interrupt to be delivered locally.

Cc: <stable@vger.kernel.org> # 4.7.x
Tested-by: Fabio Estevam <fabio.estevam@nxp.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
---
Changes since v1:
- Add the 4.7.x notation

 drivers/irqchip/irq-gic.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index c2cab57..390fac5 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -769,6 +769,13 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
 	int cpu;
 	unsigned long flags, map = 0;
 
+	if (unlikely(nr_cpu_ids == 1)) {
+		/* Only one CPU? let's do a self-IPI... */
+		writel_relaxed(2 << 24 | irq,
+			       gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);
+		return;
+	}
+
 	raw_spin_lock_irqsave(&irq_controller_lock, flags);
 
 	/* Convert our logical CPU mask into a physical one. */
-- 
1.9.1

Re: [PATCH v2] irqchip/gic: Allow self-SGIs for SMP on UP configurations

[Marc Zyngier]

Hi Fabio,

On 09/09/16 14:44, Fabio Estevam wrote:
> From: Marc Zyngier <marc.zyngier@arm.com>
> 
> commit 059e232089e45b0befc9933d31209c225e08b426 upstream.
> 
> On systems where a single CPU is present, the GIC may not support
> having SGIs delivered to a target list. In that case, we use the
> self-SGI mechanism to allow the interrupt to be delivered locally.
> 
> Cc: <stable@vger.kernel.org> # 4.7.x
> Tested-by: Fabio Estevam <fabio.estevam@nxp.com>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>

I'm sorry, but what is the justification for this to be backported to a
stable kernel? It is not a regression, since we never supported this
kind of HW before. This is not a workaround for an erratum either.

If that was a kernel bug, why not backport it all the way the the oldest
stable, instead of limiting it to v4.7?

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH v2] irqchip/gic: Allow self-SGIs for SMP on UP configurations

[Fabio Estevam]

Hi Marc,

On Fri, Sep 9, 2016 at 11:40 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> Hi Fabio,
>
> On 09/09/16 14:44, Fabio Estevam wrote:
>> From: Marc Zyngier <marc.zyngier@arm.com>
>>
>> commit 059e232089e45b0befc9933d31209c225e08b426 upstream.
>>
>> On systems where a single CPU is present, the GIC may not support
>> having SGIs delivered to a target list. In that case, we use the
>> self-SGI mechanism to allow the interrupt to be delivered locally.
>>
>> Cc: <stable@vger.kernel.org> # 4.7.x
>> Tested-by: Fabio Estevam <fabio.estevam@nxp.com>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
>
> I'm sorry, but what is the justification for this to be backported to a
> stable kernel? It is not a regression, since we never supported this

Without this patch imx6ul SoC hangs after a 'reboot' command.

> kind of HW before. This is not a workaround for an erratum either.
>
> If that was a kernel bug, why not backport it all the way the the oldest
> stable, instead of limiting it to v4.7?

I can send it to 4.4 as well, if you agree.

Re: [PATCH v2] irqchip/gic: Allow self-SGIs for SMP on UP configurations

[Marc Zyngier]

On 09/09/16 16:31, Fabio Estevam wrote:
> Hi Marc,
> 
> On Fri, Sep 9, 2016 at 11:40 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> Hi Fabio,
>>
>> On 09/09/16 14:44, Fabio Estevam wrote:
>>> From: Marc Zyngier <marc.zyngier@arm.com>
>>>
>>> commit 059e232089e45b0befc9933d31209c225e08b426 upstream.
>>>
>>> On systems where a single CPU is present, the GIC may not support
>>> having SGIs delivered to a target list. In that case, we use the
>>> self-SGI mechanism to allow the interrupt to be delivered locally.
>>>
>>> Cc: <stable@vger.kernel.org> # 4.7.x
>>> Tested-by: Fabio Estevam <fabio.estevam@nxp.com>
>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>>> Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
>>
>> I'm sorry, but what is the justification for this to be backported to a
>> stable kernel? It is not a regression, since we never supported this
> 
> Without this patch imx6ul SoC hangs after a 'reboot' command.

But that's not a regression, right? It *never* worked before, as far as
I know, because we never supported such a configuration in the past.

I'm a bit concerned when I see backporting random patches to random
kernel versions for things that are not bugs. I'll leave it up to Greg
to decide, but I thought I should point out what this patch actually is.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH v2] irqchip/gic: Allow self-SGIs for SMP on UP configurations

[Fabio Estevam]

Hi Marc,

On Fri, Sep 9, 2016 at 12:42 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:

>> Without this patch imx6ul SoC hangs after a 'reboot' command.
>
> But that's not a regression, right? It *never* worked before, as far as
> I know, because we never supported such a configuration in the past.

imx6ul is supported since kernel 4.3.

> I'm a bit concerned when I see backporting random patches to random
> kernel versions for things that are not bugs. I'll leave it up to Greg
> to decide, but I thought I should point out what this patch actually is.

Not bugs? I consider the hang I get after running 'reboot' on imx6ul a bug.

According to Documentation/stable_kernel_rules.txt :

" - It must fix a problem that causes a build error (but not for things
   marked CONFIG_BROKEN), an oops, a hang, data corruption, a real
   security issue, or some "oh, that's not good" issue.  In short, something
   critical. "

, so that's why I submitted for stable inclusion.

Re: [PATCH v2] irqchip/gic: Allow self-SGIs for SMP on UP configurations

[Marc Zyngier]

On 09/09/16 16:48, Fabio Estevam wrote:
> Hi Marc,
> 
> On Fri, Sep 9, 2016 at 12:42 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> 
>>> Without this patch imx6ul SoC hangs after a 'reboot' command.
>>
>> But that's not a regression, right? It *never* worked before, as far as
>> I know, because we never supported such a configuration in the past.
> 
> imx6ul is supported since kernel 4.3.

And yet nobody reported this as an issue until the 4.8 cycle. So it
doesn't look like it was annoying anyone until then.

> 
>> I'm a bit concerned when I see backporting random patches to random
>> kernel versions for things that are not bugs. I'll leave it up to Greg
>> to decide, but I thought I should point out what this patch actually is.
> 
> Not bugs? I consider the hang I get after running 'reboot' on imx6ul a bug.
> 
> According to Documentation/stable_kernel_rules.txt :
> 
> " - It must fix a problem that causes a build error (but not for things
>    marked CONFIG_BROKEN), an oops, a hang, data corruption, a real
>    security issue, or some "oh, that's not good" issue.  In short, something
>    critical. "
> 
> , so that's why I submitted for stable inclusion.
> 

I don't, which is why I didn't tag it for stable the first place. I
consider this as an unsupported configuration (MP CPU on a UP GIC),
which we start supporting from 4.8 onward.

Anyway, I'm not going to argue any further for such a small patch, but
I've made my position clear.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH v2] irqchip/gic: Allow self-SGIs for SMP on UP configurations

[Greg KH]

On Fri, Sep 09, 2016 at 12:48:34PM -0300, Fabio Estevam wrote:
> Hi Marc,
> 
> On Fri, Sep 9, 2016 at 12:42 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> 
> >> Without this patch imx6ul SoC hangs after a 'reboot' command.
> >
> > But that's not a regression, right? It *never* worked before, as far as
> > I know, because we never supported such a configuration in the past.
> 
> imx6ul is supported since kernel 4.3.
> 
> > I'm a bit concerned when I see backporting random patches to random
> > kernel versions for things that are not bugs. I'll leave it up to Greg
> > to decide, but I thought I should point out what this patch actually is.
> 
> Not bugs? I consider the hang I get after running 'reboot' on imx6ul a bug.
> 
> According to Documentation/stable_kernel_rules.txt :
> 
> " - It must fix a problem that causes a build error (but not for things
>    marked CONFIG_BROKEN), an oops, a hang, data corruption, a real
>    security issue, or some "oh, that's not good" issue.  In short, something
>    critical. "
> 
> , so that's why I submitted for stable inclusion.

But if it never worked to start with, that's not really a regression,
that is a "make new hardware work" type thing.

Did it work properly in 4.3?  What caused support for this hardware to
break?

thanks,

greg k-h

Re: [PATCH v2] irqchip/gic: Allow self-SGIs for SMP on UP configurations

[Greg KH]

On Fri, Sep 09, 2016 at 05:06:02PM +0100, Marc Zyngier wrote:
> On 09/09/16 16:48, Fabio Estevam wrote:
> > Hi Marc,
> > 
> > On Fri, Sep 9, 2016 at 12:42 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> > 
> >>> Without this patch imx6ul SoC hangs after a 'reboot' command.
> >>
> >> But that's not a regression, right? It *never* worked before, as far as
> >> I know, because we never supported such a configuration in the past.
> > 
> > imx6ul is supported since kernel 4.3.
> 
> And yet nobody reported this as an issue until the 4.8 cycle. So it
> doesn't look like it was annoying anyone until then.
> 
> > 
> >> I'm a bit concerned when I see backporting random patches to random
> >> kernel versions for things that are not bugs. I'll leave it up to Greg
> >> to decide, but I thought I should point out what this patch actually is.
> > 
> > Not bugs? I consider the hang I get after running 'reboot' on imx6ul a bug.
> > 
> > According to Documentation/stable_kernel_rules.txt :
> > 
> > " - It must fix a problem that causes a build error (but not for things
> >    marked CONFIG_BROKEN), an oops, a hang, data corruption, a real
> >    security issue, or some "oh, that's not good" issue.  In short, something
> >    critical. "
> > 
> > , so that's why I submitted for stable inclusion.
> > 
> 
> I don't, which is why I didn't tag it for stable the first place. I
> consider this as an unsupported configuration (MP CPU on a UP GIC),
> which we start supporting from 4.8 onward.
> 
> Anyway, I'm not going to argue any further for such a small patch, but
> I've made my position clear.

As the maintainer wants this removed, I've now dropped it from the
4.7-stable queue.

Fabio, just use 4.8 on this hardware and you should be fine.

thanks,

greg k-h