linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] MIPS: Fix broken malta qemu
@ 2016-03-17 21:08 Qais Yousef
  2016-03-18  1:17 ` Guenter Roeck
  2016-04-01 12:48 ` Paul Burton
  0 siblings, 2 replies; 12+ messages in thread
From: Qais Yousef @ 2016-03-17 21:08 UTC (permalink / raw)
  To: ralf
  Cc: Qais Yousef, Guenter Roeck, Thomas Gleixner, linux-mips, linux-kernel

Malta defconfig compiles with GIC on. Hence when compiling for SMP it causes the
new IPI code to be activated. But on qemu malta there's no GIC causing a
BUG_ON(!ipidomain) to be hit in mips_smp_ipi_init().

Since in that configuration one can only run a single core SMP (!), skip IPI
initialisation if we detect that this is the case. It is a sensible behaviour
to introduce and should keep such possible configuration to run rather than die
hard unnecessarily.

Signed-off-by: Qais Yousef <qsyousef@gmail.com>
Reported-by: Guenter Roeck <linux@roeck-us.net>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-mips@linux-mips.org
Cc: linux-kernel@vger.kernel.org
---
 arch/mips/kernel/smp.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/mips/kernel/smp.c b/arch/mips/kernel/smp.c
index 37708d9..27cb638 100644
--- a/arch/mips/kernel/smp.c
+++ b/arch/mips/kernel/smp.c
@@ -243,6 +243,18 @@ static int __init mips_smp_ipi_init(void)
 	struct irq_domain *ipidomain;
 	struct device_node *node;
 
+	/*
+	 * In some cases like qemu-malta, it is desired to try SMP with
+	 * a single core. Qemu-malta has no GIC, so an attempt to set any IPIs
+	 * would cause a BUG_ON() to be triggered since there's no ipidomain.
+	 *
+	 * Since for a single core system IPIs aren't required really, skip the
+	 * initialisation which should generally keep any such configurations
+	 * happy and only fail hard when trying to truely run SMP.
+	 */
+	if (cpumask_weight(cpu_possible_mask) == 1)
+		return 0;
+
 	node = of_irq_find_parent(of_root);
 	ipidomain = irq_find_matching_host(node, DOMAIN_BUS_IPI);
 
-- 
2.7.3

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] MIPS: Fix broken malta qemu
  2016-03-17 21:08 [PATCH] MIPS: Fix broken malta qemu Qais Yousef
@ 2016-03-18  1:17 ` Guenter Roeck
  2016-04-01 12:48 ` Paul Burton
  1 sibling, 0 replies; 12+ messages in thread
From: Guenter Roeck @ 2016-03-18  1:17 UTC (permalink / raw)
  To: Qais Yousef; +Cc: ralf, Thomas Gleixner, linux-mips, linux-kernel

On Thu, Mar 17, 2016 at 09:08:09PM +0000, Qais Yousef wrote:
> Malta defconfig compiles with GIC on. Hence when compiling for SMP it causes the
> new IPI code to be activated. But on qemu malta there's no GIC causing a
> BUG_ON(!ipidomain) to be hit in mips_smp_ipi_init().
> 
> Since in that configuration one can only run a single core SMP (!), skip IPI
> initialisation if we detect that this is the case. It is a sensible behaviour
> to introduce and should keep such possible configuration to run rather than die
> hard unnecessarily.
> 
> Signed-off-by: Qais Yousef <qsyousef@gmail.com>
> Reported-by: Guenter Roeck <linux@roeck-us.net>

Tested-by: Guenter Roeck <linux@roeck-us.net>

> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: linux-mips@linux-mips.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  arch/mips/kernel/smp.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/arch/mips/kernel/smp.c b/arch/mips/kernel/smp.c
> index 37708d9..27cb638 100644
> --- a/arch/mips/kernel/smp.c
> +++ b/arch/mips/kernel/smp.c
> @@ -243,6 +243,18 @@ static int __init mips_smp_ipi_init(void)
>  	struct irq_domain *ipidomain;
>  	struct device_node *node;
>  
> +	/*
> +	 * In some cases like qemu-malta, it is desired to try SMP with
> +	 * a single core. Qemu-malta has no GIC, so an attempt to set any IPIs
> +	 * would cause a BUG_ON() to be triggered since there's no ipidomain.
> +	 *
> +	 * Since for a single core system IPIs aren't required really, skip the
> +	 * initialisation which should generally keep any such configurations
> +	 * happy and only fail hard when trying to truely run SMP.
> +	 */
> +	if (cpumask_weight(cpu_possible_mask) == 1)
> +		return 0;
> +
>  	node = of_irq_find_parent(of_root);
>  	ipidomain = irq_find_matching_host(node, DOMAIN_BUS_IPI);
>  
> -- 
> 2.7.3
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] MIPS: Fix broken malta qemu
  2016-03-17 21:08 [PATCH] MIPS: Fix broken malta qemu Qais Yousef
  2016-03-18  1:17 ` Guenter Roeck
@ 2016-04-01 12:48 ` Paul Burton
  2016-04-02 12:19   ` Qais Yousef
  1 sibling, 1 reply; 12+ messages in thread
From: Paul Burton @ 2016-04-01 12:48 UTC (permalink / raw)
  To: Qais Yousef, ralf
  Cc: Guenter Roeck, Thomas Gleixner, linux-mips, linux-kernel

On Thu, Mar 17, 2016 at 09:08:09PM +0000, Qais Yousef wrote:
> Malta defconfig compiles with GIC on. Hence when compiling for SMP it causes the
> new IPI code to be activated. But on qemu malta there's no GIC causing a
> BUG_ON(!ipidomain) to be hit in mips_smp_ipi_init().
> 
> Since in that configuration one can only run a single core SMP (!), skip IPI
> initialisation if we detect that this is the case. It is a sensible behaviour
> to introduce and should keep such possible configuration to run rather than die
> hard unnecessarily.

Hi Qais/Ralf,

This patch is insufficient I'm afraid. It's entirely possible to use SMP
with multiple VPEs in a single core on Malta boards that don't have a
GIC - we have code handling IPIs in that case guarded by #ifdef
CONFIG_MIPS_MT_SMP in arch/mips/mti-malta/malta-int.c. I think the
BUG_ON needs to be removed entirely, unless that single-core multi-VPE
IPI code is also converted to use an IPI irqdomain.

Thanks,
    Paul

> 
> Signed-off-by: Qais Yousef <qsyousef@gmail.com>
> Reported-by: Guenter Roeck <linux@roeck-us.net>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: linux-mips@linux-mips.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  arch/mips/kernel/smp.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/arch/mips/kernel/smp.c b/arch/mips/kernel/smp.c
> index 37708d9..27cb638 100644
> --- a/arch/mips/kernel/smp.c
> +++ b/arch/mips/kernel/smp.c
> @@ -243,6 +243,18 @@ static int __init mips_smp_ipi_init(void)
>  	struct irq_domain *ipidomain;
>  	struct device_node *node;
>  
> +	/*
> +	 * In some cases like qemu-malta, it is desired to try SMP with
> +	 * a single core. Qemu-malta has no GIC, so an attempt to set any IPIs
> +	 * would cause a BUG_ON() to be triggered since there's no ipidomain.
> +	 *
> +	 * Since for a single core system IPIs aren't required really, skip the
> +	 * initialisation which should generally keep any such configurations
> +	 * happy and only fail hard when trying to truely run SMP.
> +	 */
> +	if (cpumask_weight(cpu_possible_mask) == 1)
> +		return 0;
> +
>  	node = of_irq_find_parent(of_root);
>  	ipidomain = irq_find_matching_host(node, DOMAIN_BUS_IPI);
>  
> -- 
> 2.7.3
> 
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] MIPS: Fix broken malta qemu
  2016-04-01 12:48 ` Paul Burton
@ 2016-04-02 12:19   ` Qais Yousef
  2016-04-02 14:05     ` Guenter Roeck
  2016-04-04  6:41     ` Paul Burton
  0 siblings, 2 replies; 12+ messages in thread
From: Qais Yousef @ 2016-04-02 12:19 UTC (permalink / raw)
  To: Paul Burton, ralf
  Cc: Guenter Roeck, Thomas Gleixner, linux-mips, linux-kernel

Hi Paul,

On 01/04/2016 13:48, Paul Burton wrote:
> On Thu, Mar 17, 2016 at 09:08:09PM +0000, Qais Yousef wrote:
>> Malta defconfig compiles with GIC on. Hence when compiling for SMP it causes the
>> new IPI code to be activated. But on qemu malta there's no GIC causing a
>> BUG_ON(!ipidomain) to be hit in mips_smp_ipi_init().
>>
>> Since in that configuration one can only run a single core SMP (!), skip IPI
>> initialisation if we detect that this is the case. It is a sensible behaviour
>> to introduce and should keep such possible configuration to run rather than die
>> hard unnecessarily.
> Hi Qais/Ralf,
>
> This patch is insufficient I'm afraid. It's entirely possible to use SMP
> with multiple VPEs in a single core on Malta boards that don't have a
> GIC - we have code handling IPIs in that case guarded by #ifdef
> CONFIG_MIPS_MT_SMP in arch/mips/mti-malta/malta-int.c. I think the
> BUG_ON needs to be removed entirely, unless that single-core multi-VPE
> IPI code is also converted to use an IPI irqdomain.
>

I was under the impression that SMP is only supported under GIC and 
older forms of SMP are deprecated.

I think the problem you're describing is different to the one this is 
trying to fix. The right fix for your issue is to make 
CONFIG_GENERIC_IRQ_IPI selected when CONFIG_MIPS_GIC && !CONFIG_MIPS_MT_SMP.

Would it be easy for you to create such a patch and test it?

Thanks,
Qais

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] MIPS: Fix broken malta qemu
  2016-04-02 12:19   ` Qais Yousef
@ 2016-04-02 14:05     ` Guenter Roeck
  2016-04-04  6:41     ` Paul Burton
  1 sibling, 0 replies; 12+ messages in thread
From: Guenter Roeck @ 2016-04-02 14:05 UTC (permalink / raw)
  To: Qais Yousef, Paul Burton, ralf; +Cc: Thomas Gleixner, linux-mips, linux-kernel

On 04/02/2016 05:19 AM, Qais Yousef wrote:
> Hi Paul,
>
> On 01/04/2016 13:48, Paul Burton wrote:
>> On Thu, Mar 17, 2016 at 09:08:09PM +0000, Qais Yousef wrote:
>>> Malta defconfig compiles with GIC on. Hence when compiling for SMP it causes the
>>> new IPI code to be activated. But on qemu malta there's no GIC causing a
>>> BUG_ON(!ipidomain) to be hit in mips_smp_ipi_init().
>>>
>>> Since in that configuration one can only run a single core SMP (!), skip IPI
>>> initialisation if we detect that this is the case. It is a sensible behaviour
>>> to introduce and should keep such possible configuration to run rather than die
>>> hard unnecessarily.
>> Hi Qais/Ralf,
>>
>> This patch is insufficient I'm afraid. It's entirely possible to use SMP
>> with multiple VPEs in a single core on Malta boards that don't have a
>> GIC - we have code handling IPIs in that case guarded by #ifdef
>> CONFIG_MIPS_MT_SMP in arch/mips/mti-malta/malta-int.c. I think the
>> BUG_ON needs to be removed entirely, unless that single-core multi-VPE
>> IPI code is also converted to use an IPI irqdomain.
>>
>
> I was under the impression that SMP is only supported under GIC and older forms of SMP are deprecated.
>
> I think the problem you're describing is different to the one this is trying to fix. The right fix for your issue is to make CONFIG_GENERIC_IRQ_IPI selected when CONFIG_MIPS_GIC && !CONFIG_MIPS_MT_SMP.
>
Didn't Paul say that his system doesn't have a GIC ? Are you saying that he can not
(or no longer) run an image built for SMP on his system ?

Guenter

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] MIPS: Fix broken malta qemu
  2016-04-02 12:19   ` Qais Yousef
  2016-04-02 14:05     ` Guenter Roeck
@ 2016-04-04  6:41     ` Paul Burton
  2016-04-04  7:59       ` [PATCH] MIPS: Don't BUG_ON when no IPI domain is found Paul Burton
  2016-04-04  8:02       ` [PATCH] MIPS: Fix broken malta qemu Ralf Baechle
  1 sibling, 2 replies; 12+ messages in thread
From: Paul Burton @ 2016-04-04  6:41 UTC (permalink / raw)
  To: Qais Yousef
  Cc: ralf, Guenter Roeck, Thomas Gleixner, linux-mips, linux-kernel

On Sat, Apr 02, 2016 at 01:19:03PM +0100, Qais Yousef wrote:
> Hi Paul,
> 
> On 01/04/2016 13:48, Paul Burton wrote:
> >On Thu, Mar 17, 2016 at 09:08:09PM +0000, Qais Yousef wrote:
> >>Malta defconfig compiles with GIC on. Hence when compiling for SMP it causes the
> >>new IPI code to be activated. But on qemu malta there's no GIC causing a
> >>BUG_ON(!ipidomain) to be hit in mips_smp_ipi_init().
> >>
> >>Since in that configuration one can only run a single core SMP (!), skip IPI
> >>initialisation if we detect that this is the case. It is a sensible behaviour
> >>to introduce and should keep such possible configuration to run rather than die
> >>hard unnecessarily.
> >Hi Qais/Ralf,
> >
> >This patch is insufficient I'm afraid. It's entirely possible to use SMP
> >with multiple VPEs in a single core on Malta boards that don't have a
> >GIC - we have code handling IPIs in that case guarded by #ifdef
> >CONFIG_MIPS_MT_SMP in arch/mips/mti-malta/malta-int.c. I think the
> >BUG_ON needs to be removed entirely, unless that single-core multi-VPE
> >IPI code is also converted to use an IPI irqdomain.
> >
> 
> I was under the impression that SMP is only supported under GIC and older
> forms of SMP are deprecated.

Hi Qais,

That's incorrect. We're talking about systems that don't have any GIC -
there are Malta configurations that don't. QEMU is the obvious one, and
you can break it (at least v2.5.0 which has no GIC) with this patch just
by using "-cpu 34Kf -smp 2" to show up 2 VPEs. I believe there are also
real bitfiles (though probably unused these days) that would have the
same problem.

> I think the problem you're describing is different to the one this is trying
> to fix. The right fix for your issue is to make CONFIG_GENERIC_IRQ_IPI
> selected when CONFIG_MIPS_GIC && !CONFIG_MIPS_MT_SMP.
> 
> Would it be easy for you to create such a patch and test it?

That would be insufficient, since we can have a kernel that includes GIC
support & CONFIG_MIPS_MT_SMP (SMVP) support and not know whether
there'll actually be a GIC or multiple VPEs until runtime. So Kconfig
cannot solve this at compile time.

I believe that the best thing to do would be to convert the single-core
MT IPI code to use the IRQ domain stuff you added (which I don't see any
documentation for & am currently struggling to decipher). But given that
we're post-merge-window on the way to v4.6 I think the best we can do is
to just return when there's no IPI domain, rather than BUG_ON. I'll
submit a patch to do that later this morning.

Thanks,
    Paul

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] MIPS: Don't BUG_ON when no IPI domain is found
  2016-04-04  6:41     ` Paul Burton
@ 2016-04-04  7:59       ` Paul Burton
  2016-04-04  9:04         ` [PATCH v2] " Paul Burton
  2016-04-04  8:02       ` [PATCH] MIPS: Fix broken malta qemu Ralf Baechle
  1 sibling, 1 reply; 12+ messages in thread
From: Paul Burton @ 2016-04-04  7:59 UTC (permalink / raw)
  To: linux-mips
  Cc: Paul Burton, Qais Yousef, linux-kernel, Thomas Gleixner,
	James Hogan, Markos Chandras, Ralf Baechle, Alex Smith

Commit fbde2d7d8290 ("MIPS: Add generic SMP IPI support") introduced
code that BUG_ON's in the case of a kernel that supports IPI domains but
does not have one at runtime. This case is possible on Malta where for
IPIs we may use either the GIC (which has an IPI IRQ domain
implementation) or core-local software interrupts between VPEs (which do
not currently have an IPI IRQ domain implementation). We can not know
which will be used until runtime when we know whether a GIC is actually
present, and if we run on a system with multiple VPEs and no GIC then
the BUG_ON is hit.

A simple way to reproduce this bug is by using QEMU, which partially
implements the MT ASE but does not implement the GIC as of version 2.5.
Using "-cpu 34Kf -smp 2" will present a system with 2 VPEs in one core &
no GIC, hitting the BUG_ON.

Given that we're post-merge-window on the way to v4.6, avoid this by
just returning from mips_smp_ipi_init when no IPI IRQ domain is found.
Ideally at some point all IPI implementations would be converted to the
same system & we'd be able to restore the check.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Fixes: fbde2d7d8290 ("MIPS: Add generic SMP IPI support")
---

 arch/mips/kernel/smp.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/mips/kernel/smp.c b/arch/mips/kernel/smp.c
index 8b687fe..630bcfb 100644
--- a/arch/mips/kernel/smp.c
+++ b/arch/mips/kernel/smp.c
@@ -254,7 +254,17 @@ static int __init mips_smp_ipi_init(void)
 	if (node && !ipidomain)
 		ipidomain = irq_find_matching_host(NULL, DOMAIN_BUS_IPI);
 
-	BUG_ON(!ipidomain);
+	/*
+	 * There are systems which only use IPI domains some of the time,
+	 * depending upon configuration we don't know until runtime. An
+	 * example is Malta where we may compile in support for GIC & the
+	 * MT ASE, but run on a system which has multiple VPEs in a single
+	 * core and doesn't include a GIC. Until all IPI implementations
+	 * have been converted to use IPI domains the best we can do here
+	 * is to return & hope some other code sets up the IPIs.
+	 */
+	if (!ipidomain)
+		return 0;
 
 	call_virq = irq_reserve_ipi(ipidomain, cpu_possible_mask);
 	BUG_ON(!call_virq);
-- 
2.8.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] MIPS: Fix broken malta qemu
  2016-04-04  6:41     ` Paul Burton
  2016-04-04  7:59       ` [PATCH] MIPS: Don't BUG_ON when no IPI domain is found Paul Burton
@ 2016-04-04  8:02       ` Ralf Baechle
  2016-04-04  8:06         ` Paul Burton
  1 sibling, 1 reply; 12+ messages in thread
From: Ralf Baechle @ 2016-04-04  8:02 UTC (permalink / raw)
  To: Paul Burton
  Cc: Qais Yousef, Guenter Roeck, Thomas Gleixner, linux-mips, linux-kernel

On Mon, Apr 04, 2016 at 07:41:40AM +0100, Paul Burton wrote:

> > On 01/04/2016 13:48, Paul Burton wrote:
> > >On Thu, Mar 17, 2016 at 09:08:09PM +0000, Qais Yousef wrote:
> > >>Malta defconfig compiles with GIC on. Hence when compiling for SMP it causes the
> > >>new IPI code to be activated. But on qemu malta there's no GIC causing a
> > >>BUG_ON(!ipidomain) to be hit in mips_smp_ipi_init().
> > >>
> > >>Since in that configuration one can only run a single core SMP (!), skip IPI
> > >>initialisation if we detect that this is the case. It is a sensible behaviour
> > >>to introduce and should keep such possible configuration to run rather than die
> > >>hard unnecessarily.
> > >Hi Qais/Ralf,
> > >
> > >This patch is insufficient I'm afraid. It's entirely possible to use SMP
> > >with multiple VPEs in a single core on Malta boards that don't have a
> > >GIC - we have code handling IPIs in that case guarded by #ifdef
> > >CONFIG_MIPS_MT_SMP in arch/mips/mti-malta/malta-int.c. I think the
> > >BUG_ON needs to be removed entirely, unless that single-core multi-VPE
> > >IPI code is also converted to use an IPI irqdomain.
> > >
> > 
> > I was under the impression that SMP is only supported under GIC and older
> > forms of SMP are deprecated.
> 
> Hi Qais,
> 
> That's incorrect. We're talking about systems that don't have any GIC -
> there are Malta configurations that don't. QEMU is the obvious one, and
> you can break it (at least v2.5.0 which has no GIC) with this patch just
> by using "-cpu 34Kf -smp 2" to show up 2 VPEs. I believe there are also
> real bitfiles (though probably unused these days) that would have the
> same problem.
> 
> > I think the problem you're describing is different to the one this is trying
> > to fix. The right fix for your issue is to make CONFIG_GENERIC_IRQ_IPI
> > selected when CONFIG_MIPS_GIC && !CONFIG_MIPS_MT_SMP.
> > 
> > Would it be easy for you to create such a patch and test it?
> 
> That would be insufficient, since we can have a kernel that includes GIC
> support & CONFIG_MIPS_MT_SMP (SMVP) support and not know whether
> there'll actually be a GIC or multiple VPEs until runtime. So Kconfig
> cannot solve this at compile time.
> 
> I believe that the best thing to do would be to convert the single-core
> MT IPI code to use the IRQ domain stuff you added (which I don't see any
> documentation for & am currently struggling to decipher). But given that
> we're post-merge-window on the way to v4.6 I think the best we can do is
> to just return when there's no IPI domain, rather than BUG_ON. I'll
> submit a patch to do that later this morning.

FYI, Qais' initial fix is in the pull request I sent to Linus yesterday so
any fixes please relative to that patch.

  Ralf

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] MIPS: Fix broken malta qemu
  2016-04-04  8:02       ` [PATCH] MIPS: Fix broken malta qemu Ralf Baechle
@ 2016-04-04  8:06         ` Paul Burton
  2016-04-04  8:32           ` Ralf Baechle
       [not found]           ` <CA+mqd+7YA5JH=CfLBV9S-c+0aQw=NHihT+WyPvgUF6UkmK+tEQ@mail.gmail.com>
  0 siblings, 2 replies; 12+ messages in thread
From: Paul Burton @ 2016-04-04  8:06 UTC (permalink / raw)
  To: Ralf Baechle
  Cc: Qais Yousef, Guenter Roeck, Thomas Gleixner, linux-mips, linux-kernel

On Mon, Apr 04, 2016 at 10:02:23AM +0200, Ralf Baechle wrote:
> FYI, Qais' initial fix is in the pull request I sent to Linus yesterday so
> any fixes please relative to that patch.

Hi Ralf,

To late - I already submitted:

    https://patchwork.linux-mips.org/patch/13003/

I can rebase, but it'll be a revert of Qais' workaround followed by
mine & squashed.

Thanks,
    Paul

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] MIPS: Fix broken malta qemu
  2016-04-04  8:06         ` Paul Burton
@ 2016-04-04  8:32           ` Ralf Baechle
       [not found]           ` <CA+mqd+7YA5JH=CfLBV9S-c+0aQw=NHihT+WyPvgUF6UkmK+tEQ@mail.gmail.com>
  1 sibling, 0 replies; 12+ messages in thread
From: Ralf Baechle @ 2016-04-04  8:32 UTC (permalink / raw)
  To: Paul Burton
  Cc: Qais Yousef, Guenter Roeck, Thomas Gleixner, linux-mips, linux-kernel

On Mon, Apr 04, 2016 at 09:06:54AM +0100, Paul Burton wrote:

> On Mon, Apr 04, 2016 at 10:02:23AM +0200, Ralf Baechle wrote:
> > FYI, Qais' initial fix is in the pull request I sent to Linus yesterday so
> > any fixes please relative to that patch.
> 
> Hi Ralf,
> 
> To late - I already submitted:
> 
>     https://patchwork.linux-mips.org/patch/13003/
> 
> I can rebase, but it'll be a revert of Qais' workaround followed by
> mine & squashed.

Yeah, rebase and something on top of that will be fine.

  Ralf

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] MIPS: Fix broken malta qemu
       [not found]           ` <CA+mqd+7YA5JH=CfLBV9S-c+0aQw=NHihT+WyPvgUF6UkmK+tEQ@mail.gmail.com>
@ 2016-04-04  8:40             ` Paul Burton
  0 siblings, 0 replies; 12+ messages in thread
From: Paul Burton @ 2016-04-04  8:40 UTC (permalink / raw)
  To: Qais Yousef
  Cc: Ralf Baechle, Thomas Gleixner, linux-mips, linux-kernel, Guenter Roeck

On Mon, Apr 04, 2016 at 09:28:16AM +0100, Qais Yousef wrote:
> On 4 Apr 2016 09:06, "Paul Burton" <paul.burton@imgtec.com> wrote:
> >
> > On Mon, Apr 04, 2016 at 10:02:23AM +0200, Ralf Baechle wrote:
> > > FYI, Qais' initial fix is in the pull request I sent to Linus yesterday
> so
> > > any fixes please relative to that patch.
> >
> > Hi Ralf,
> >
> > To late - I already submitted:
> >
> >     https://patchwork.linux-mips.org/patch/13003/
> >
> > I can rebase, but it'll be a revert of Qais' workaround followed by
> > mine & squashed.
> >
> > Thanks,
> >     Paul
> 
> Removing BUG_ON () is the real workaround.

Hi Qais,

They're both workarounds. I think given the point in the cycle that
we're at now it's the best option for now.

> The problem with Malta is that it always selects GIC even when it does
> not have it.

No, that's by design. GIC support can be enabled in Kconfig & the actual
presence of a GIC is detected at runtime. A kernel with GIC support can
run on a system with or without a GIC.

> If you add 2 ipi domains then you need to add a way to tell the platform
> which one to use.

Then we may need some way to do that, or perhaps it'll be OK if we only
register one of them on any given system. As mentioned I haven't really
deciphered this IPI IRQ domain code yet.

> All of the patches I sent were sent for review and were around for a long
> time. There's no need for the passive aggressiveness please because you
> found a problem now. Everything went through the formal process and you and
> everyone had a chance to comment and raise issues out.

Passive agressiveness? Really?

I'm not complaining about your patches having being merged. I haven't
said you failed to follow any process. I didn't have time to review your
patches, and clearly nobody else spotted this either, and a regression
has snuck into mainline. I simply want to fix it. I would also have
liked to find some documentation since it's difficult to figure out how
to add support for the single-core multi-VPE software interrupt case,
but that's the only criticism I've made of the patches & I think it's
just. Please don't be so personal...

> My idea of a fix is to get config dependencies sorted but I'll leave it
> with you and Ralf.

As I tried to explain before and above, we don't have all of the
required information until runtime, so Kconfig cannot solve this. That
won't work.

Thanks,
    Paul

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2] MIPS: Don't BUG_ON when no IPI domain is found
  2016-04-04  7:59       ` [PATCH] MIPS: Don't BUG_ON when no IPI domain is found Paul Burton
@ 2016-04-04  9:04         ` Paul Burton
  0 siblings, 0 replies; 12+ messages in thread
From: Paul Burton @ 2016-04-04  9:04 UTC (permalink / raw)
  To: linux-mips
  Cc: Paul Burton, Qais Yousef, linux-kernel, James Hogan,
	Markos Chandras, Ralf Baechle, Alex Smith

Commit fbde2d7d8290 ("MIPS: Add generic SMP IPI support") introduced
code that BUG_ON's in the case of a kernel that supports IPI domains but
does not have one at runtime. This case is possible on Malta where for
IPIs we may use either the GIC (which has an IPI IRQ domain
implementation) or core-local software interrupts between VPEs (which do
not currently have an IPI IRQ domain implementation). We can not know
which will be used until runtime when we know whether a GIC is actually
present, and if we run on a system with multiple VPEs and no GIC then
the BUG_ON is hit.

Commit 19fb5818ed60 ("IPS: Fix broken malta qemu") worked around this
for the single-core single-VPE case typically seen using QEMU, but does
not catch the multi-VPE case. This patch removes the insufficient CPU
presence check that was added and works around the bug differently,
effectively reverting that commit.

A simple way to reproduce this bug is by using QEMU, which partially
implements the MT ASE but does not implement the GIC as of version 2.5.
Using "-cpu 34Kf -smp 2" will present a system with 2 VPEs in one core &
no GIC, hitting the BUG_ON.

Given that we're post-merge-window on the way to v4.6, avoid this by
just returning from mips_smp_ipi_init when no IPI IRQ domain is found.
Ideally at some point all IPI implementations would be converted to the
same IPI IRQ domain interface & we'd be able to restore the check.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Cc: Qais Yousef <qsyousef@gmail.com>
Fixes: fbde2d7d8290 ("MIPS: Add generic SMP IPI support")
Fixes: 19fb5818ed60 ("IPS: Fix broken malta qemu")
Reverts: 19fb5818ed60 ("IPS: Fix broken malta qemu")

---

Changes in v2:
- Rebase this workaround atop Qais' workaround at Ralf's request.
- Update the commit message to reflect that.

 arch/mips/kernel/smp.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/arch/mips/kernel/smp.c b/arch/mips/kernel/smp.c
index c87c0da..630bcfb 100644
--- a/arch/mips/kernel/smp.c
+++ b/arch/mips/kernel/smp.c
@@ -243,18 +243,6 @@ static int __init mips_smp_ipi_init(void)
 	struct irq_domain *ipidomain;
 	struct device_node *node;
 
-	/*
-	 * In some cases like qemu-malta, it is desired to try SMP with
-	 * a single core. Qemu-malta has no GIC, so an attempt to set any IPIs
-	 * would cause a BUG_ON() to be triggered since there's no ipidomain.
-	 *
-	 * Since for a single core system IPIs aren't required really, skip the
-	 * initialisation which should generally keep any such configurations
-	 * happy and only fail hard when trying to truely run SMP.
-	 */
-	if (cpumask_weight(cpu_possible_mask) == 1)
-		return 0;
-
 	node = of_irq_find_parent(of_root);
 	ipidomain = irq_find_matching_host(node, DOMAIN_BUS_IPI);
 
@@ -266,7 +254,17 @@ static int __init mips_smp_ipi_init(void)
 	if (node && !ipidomain)
 		ipidomain = irq_find_matching_host(NULL, DOMAIN_BUS_IPI);
 
-	BUG_ON(!ipidomain);
+	/*
+	 * There are systems which only use IPI domains some of the time,
+	 * depending upon configuration we don't know until runtime. An
+	 * example is Malta where we may compile in support for GIC & the
+	 * MT ASE, but run on a system which has multiple VPEs in a single
+	 * core and doesn't include a GIC. Until all IPI implementations
+	 * have been converted to use IPI domains the best we can do here
+	 * is to return & hope some other code sets up the IPIs.
+	 */
+	if (!ipidomain)
+		return 0;
 
 	call_virq = irq_reserve_ipi(ipidomain, cpu_possible_mask);
 	BUG_ON(!call_virq);
-- 
2.8.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2016-04-04  9:05 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-17 21:08 [PATCH] MIPS: Fix broken malta qemu Qais Yousef
2016-03-18  1:17 ` Guenter Roeck
2016-04-01 12:48 ` Paul Burton
2016-04-02 12:19   ` Qais Yousef
2016-04-02 14:05     ` Guenter Roeck
2016-04-04  6:41     ` Paul Burton
2016-04-04  7:59       ` [PATCH] MIPS: Don't BUG_ON when no IPI domain is found Paul Burton
2016-04-04  9:04         ` [PATCH v2] " Paul Burton
2016-04-04  8:02       ` [PATCH] MIPS: Fix broken malta qemu Ralf Baechle
2016-04-04  8:06         ` Paul Burton
2016-04-04  8:32           ` Ralf Baechle
     [not found]           ` <CA+mqd+7YA5JH=CfLBV9S-c+0aQw=NHihT+WyPvgUF6UkmK+tEQ@mail.gmail.com>
2016-04-04  8:40             ` Paul Burton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).