[PATCH 0/3] MIPS: BCM63XX: add SMP support

[PATCH 0/3] MIPS: BCM63XX: add SMP support

[Jonas Gorski]

Most newer BCM63XX SoCs after BCM6358 use a BMIPS4350 CPU with SMP
support. This patchset allows BCM6368 and BCM6362 to boot a SMP kernel
(both tested, as well as (not yet upstreamed) BCM63268).

BCM6328 is skipped because the only SMP versions will be rejected by
current code (they are BCM6329, which is treated as a totally
unsupported chip).

BCM6358 is intentionally skipped because it shares a single TLB for
both cores/threads, which requires implementing locking for TLB accesses,
and ain't nobody got time for that.

The internal interrupt controller supports routing IRQs to both CPUs,
and support will be added in a later patchset. For now all hardware
interrupts will go to CPU0.

Totally unscientific OpenSSL benchmarking shows a nice ~90% speed
increase when enabling the second core.

No idea about the FIXME in 1/3, never had a problem with it so I left it
in place as to have it documented.

Jonas Gorski (1):
  MIPS: BCM63XX: select BMIPS4350 and default to 2 CPUs for supported
    SoCs

Kevin Cernekee (2):
  MIPS: BCM63XX: Add SMP support to prom.c
  MIPS: BCM63XX: Handle SW IRQs 0-1

 arch/mips/Kconfig        |    2 ++
 arch/mips/bcm63xx/irq.c  |    4 ++++
 arch/mips/bcm63xx/prom.c |   33 +++++++++++++++++++++++++++++++++
 3 files changed, 39 insertions(+)

-- 
1.7.10.4

[PATCH 1/3] MIPS: BCM63XX: Add SMP support to prom.c

[Jonas Gorski]

From: Kevin Cernekee <cernekee@gmail.com>

This involves two changes to the BSP code:

1) register_smp_ops() for BMIPS SMP

2) The CPU1 boot vector on some of the BCM63xx platforms conflicts with
the special interrupt vector (IV).  Move it to 0x8000_0380 at boot time,
to resolve the conflict.

Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
[jogo@openwrt.org: moved SMP ops registration into ifdef guard]
Signed-off-by: Jonas Gorski <jogo@openwrt.org>
---
 arch/mips/bcm63xx/prom.c |   33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/arch/mips/bcm63xx/prom.c b/arch/mips/bcm63xx/prom.c
index fd69808..1209373 100644
--- a/arch/mips/bcm63xx/prom.c
+++ b/arch/mips/bcm63xx/prom.c
@@ -8,7 +8,11 @@
 
 #include <linux/init.h>
 #include <linux/bootmem.h>
+#include <linux/smp.h>
 #include <asm/bootinfo.h>
+#include <asm/bmips.h>
+#include <asm/smp-ops.h>
+#include <asm/mipsregs.h>
 #include <bcm63xx_board.h>
 #include <bcm63xx_cpu.h>
 #include <bcm63xx_io.h>
@@ -52,6 +56,35 @@ void __init prom_init(void)
 
 	/* do low level board init */
 	board_prom_init();
+
+#if defined(CONFIG_CPU_BMIPS4350) && defined(CONFIG_SMP)
+	/* set up SMP */
+	register_smp_ops(&bmips_smp_ops);
+
+	/*
+	 * BCM6328 does not have its second CPU enabled, while BCM6358
+	 * needs special handling for its shared TLB, so disable SMP for now.
+	 */
+	if (BCMCPU_IS_6328() || BCMCPU_IS_6358()) {
+		bmips_smp_enabled = 0;
+		return;
+	}
+
+	/*
+	 * The bootloader has set up the CPU1 reset vector at 0xa000_0200.
+	 * This conflicts with the special interrupt vector (IV).
+	 * The bootloader has also set up CPU1 to respond to the wrong
+	 * IPI interrupt.
+	 * Here we will start up CPU1 in the background and ask it to
+	 * reconfigure itself then go back to sleep.
+	 */
+	memcpy((void *)0xa0000200, &bmips_smp_movevec, 0x20);
+	__sync();
+	set_c0_cause(C_SW0);
+	cpumask_set_cpu(1, &bmips_booted_mask);
+
+	/* FIXME: we really should have some sort of hazard barrier here */
+#endif
 }
 
 void __init prom_free_prom_memory(void)
-- 
1.7.10.4

[PATCH 2/3] MIPS: BCM63XX: Handle SW IRQs 0-1

[Jonas Gorski]

From: Kevin Cernekee <cernekee@gmail.com>

MIPS software IRQs 0 and 1 are used for interprocessor signaling (IPI)
on BMIPS SMP.  Make the board support code aware of them.

Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
[jogo@openwrt.org: move sw irqs behind timer irq]
Signed-off-by: Jonas Gorski <jogo@openwrt.org>
---
 arch/mips/bcm63xx/irq.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/mips/bcm63xx/irq.c b/arch/mips/bcm63xx/irq.c
index c0ab388..d744606 100644
--- a/arch/mips/bcm63xx/irq.c
+++ b/arch/mips/bcm63xx/irq.c
@@ -294,6 +294,10 @@ asmlinkage void plat_irq_dispatch(void)
 
 		if (cause & CAUSEF_IP7)
 			do_IRQ(7);
+		if (cause & CAUSEF_IP0)
+			do_IRQ(0);
+		if (cause & CAUSEF_IP1)
+			do_IRQ(1);
 		if (cause & CAUSEF_IP2)
 			dispatch_internal();
 		if (!is_ext_irq_cascaded) {
-- 
1.7.10.4

[PATCH 3/3] MIPS: BCM63XX: select BMIPS4350 and default to 2 CPUs for supported SoCs

[Jonas Gorski]

All BCM63XX SoCs starting with BCM6358 have a BMIPS4350 instead of a
BMIPS3300, so select it unless support for any of the older SoCs is
selected.
All BMIPS4350 have only two CPUs, so select the appropriate default.

Signed-off-by: Jonas Gorski <jogo@openwrt.org>
---
 arch/mips/Kconfig |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index ade9973..dc535be 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -128,6 +128,8 @@ config BCM63XX
 	select DMA_NONCOHERENT
 	select IRQ_CPU
 	select SYS_HAS_CPU_MIPS32_R1
+	select SYS_HAS_CPU_BMIPS4350 if !BCM63XX_CPU_6338 && !BCM63XX_CPU_6345 && !BCM63XX_CPU_6348
+	select NR_CPUS_DEFAULT_2
 	select SYS_SUPPORTS_32BIT_KERNEL
 	select SYS_SUPPORTS_BIG_ENDIAN
 	select SYS_HAS_EARLY_PRINTK
-- 
1.7.10.4

Re: [PATCH 0/3] MIPS: BCM63XX: add SMP support

[Florian Fainelli]

Le lundi 03 juin 2013 16:39:32, Jonas Gorski a écrit :
> Most newer BCM63XX SoCs after BCM6358 use a BMIPS4350 CPU with SMP
> support. This patchset allows BCM6368 and BCM6362 to boot a SMP kernel
> (both tested, as well as (not yet upstreamed) BCM63268).
> 
> BCM6328 is skipped because the only SMP versions will be rejected by
> current code (they are BCM6329, which is treated as a totally
> unsupported chip).
> 
> BCM6358 is intentionally skipped because it shares a single TLB for
> both cores/threads, which requires implementing locking for TLB accesses,
> and ain't nobody got time for that.
> 
> The internal interrupt controller supports routing IRQs to both CPUs,
> and support will be added in a later patchset. For now all hardware
> interrupts will go to CPU0.
> 
> Totally unscientific OpenSSL benchmarking shows a nice ~90% speed
> increase when enabling the second core.
> 
> No idea about the FIXME in 1/3, never had a problem with it so I left it
> in place as to have it documented.

I successfully tested these on BCM6361 (6362-like) on top of the current mips-
for-linux-next, feel free to add my:

Tested-by: Florian Fainelli <florian@openwrt.org>

Thanks Jonas!
-- 
Florian

Re: [PATCH 1/3] MIPS: BCM63XX: Add SMP support to prom.c

[Ralf Baechle]

On Mon, Jun 03, 2013 at 05:39:33PM +0200, Jonas Gorski wrote:

> 
> This involves two changes to the BSP code:
> 
> 1) register_smp_ops() for BMIPS SMP
> 
> 2) The CPU1 boot vector on some of the BCM63xx platforms conflicts with
> the special interrupt vector (IV).  Move it to 0x8000_0380 at boot time,
> to resolve the conflict.
> 
> Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
> [jogo@openwrt.org: moved SMP ops registration into ifdef guard]
> Signed-off-by: Jonas Gorski <jogo@openwrt.org>
> ---
>  arch/mips/bcm63xx/prom.c |   33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/arch/mips/bcm63xx/prom.c b/arch/mips/bcm63xx/prom.c
> index fd69808..1209373 100644
> --- a/arch/mips/bcm63xx/prom.c
> +++ b/arch/mips/bcm63xx/prom.c
> @@ -8,7 +8,11 @@
>  
>  #include <linux/init.h>
>  #include <linux/bootmem.h>
> +#include <linux/smp.h>
>  #include <asm/bootinfo.h>
> +#include <asm/bmips.h>
> +#include <asm/smp-ops.h>
> +#include <asm/mipsregs.h>
>  #include <bcm63xx_board.h>
>  #include <bcm63xx_cpu.h>
>  #include <bcm63xx_io.h>
> @@ -52,6 +56,35 @@ void __init prom_init(void)
>  
>  	/* do low level board init */
>  	board_prom_init();
> +
> +#if defined(CONFIG_CPU_BMIPS4350) && defined(CONFIG_SMP)
> +	/* set up SMP */
> +	register_smp_ops(&bmips_smp_ops);

The call to register_smp_ops() can remain outside the #ifdef.  It's defined
as:

static inline void register_smp_ops(struct plat_smp_ops *ops)
{
}

so the compiler will completly discard it and the referenced SMP ops.

> +
> +	/*
> +	 * BCM6328 does not have its second CPU enabled, while BCM6358
> +	 * needs special handling for its shared TLB, so disable SMP for now.
> +	 */
> +	if (BCMCPU_IS_6328() || BCMCPU_IS_6358()) {
> +		bmips_smp_enabled = 0;
> +		return;
> +	}
> +
> +	/*
> +	 * The bootloader has set up the CPU1 reset vector at 0xa000_0200.
> +	 * This conflicts with the special interrupt vector (IV).
> +	 * The bootloader has also set up CPU1 to respond to the wrong
> +	 * IPI interrupt.
> +	 * Here we will start up CPU1 in the background and ask it to
> +	 * reconfigure itself then go back to sleep.
> +	 */
> +	memcpy((void *)0xa0000200, &bmips_smp_movevec, 0x20);
> +	__sync();
> +	set_c0_cause(C_SW0);
> +	cpumask_set_cpu(1, &bmips_booted_mask);
> +
> +	/* FIXME: we really should have some sort of hazard barrier here */

Any reason why the remainder of this code can't go into the smp_setup
method?  That then would entirely eleminate the <censored> ifdef.

  Ralf

Re: [PATCH 1/3] MIPS: BCM63XX: Add SMP support to prom.c

[Jonas Gorski]

On Fri, Jun 14, 2013 at 12:31 PM, Ralf Baechle <ralf@linux-mips.org> wrote:
> On Mon, Jun 03, 2013 at 05:39:33PM +0200, Jonas Gorski wrote:
>
>>
>> This involves two changes to the BSP code:
>>
>> 1) register_smp_ops() for BMIPS SMP
>>
>> 2) The CPU1 boot vector on some of the BCM63xx platforms conflicts with
>> the special interrupt vector (IV).  Move it to 0x8000_0380 at boot time,
>> to resolve the conflict.
>>
>> Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
>> [jogo@openwrt.org: moved SMP ops registration into ifdef guard]
>> Signed-off-by: Jonas Gorski <jogo@openwrt.org>
>> ---
>>  arch/mips/bcm63xx/prom.c |   33 +++++++++++++++++++++++++++++++++
>>  1 file changed, 33 insertions(+)
>>
>> diff --git a/arch/mips/bcm63xx/prom.c b/arch/mips/bcm63xx/prom.c
>> index fd69808..1209373 100644
>> --- a/arch/mips/bcm63xx/prom.c
>> +++ b/arch/mips/bcm63xx/prom.c
>> @@ -8,7 +8,11 @@
>>
>>  #include <linux/init.h>
>>  #include <linux/bootmem.h>
>> +#include <linux/smp.h>
>>  #include <asm/bootinfo.h>
>> +#include <asm/bmips.h>
>> +#include <asm/smp-ops.h>
>> +#include <asm/mipsregs.h>
>>  #include <bcm63xx_board.h>
>>  #include <bcm63xx_cpu.h>
>>  #include <bcm63xx_io.h>
>> @@ -52,6 +56,35 @@ void __init prom_init(void)
>>
>>       /* do low level board init */
>>       board_prom_init();
>> +
>> +#if defined(CONFIG_CPU_BMIPS4350) && defined(CONFIG_SMP)
>> +     /* set up SMP */
>> +     register_smp_ops(&bmips_smp_ops);
>
> The call to register_smp_ops() can remain outside the #ifdef.  It's defined
> as:
>
> static inline void register_smp_ops(struct plat_smp_ops *ops)
> {
> }
>
> so the compiler will completly discard it and the referenced SMP ops.

As long as it doesn't cause linking errors with -O0 or something, I'm
fine with either way.

>> +
>> +     /*
>> +      * BCM6328 does not have its second CPU enabled, while BCM6358
>> +      * needs special handling for its shared TLB, so disable SMP for now.
>> +      */
>> +     if (BCMCPU_IS_6328() || BCMCPU_IS_6358()) {
>> +             bmips_smp_enabled = 0;
>> +             return;
>> +     }
>> +
>> +     /*
>> +      * The bootloader has set up the CPU1 reset vector at 0xa000_0200.
>> +      * This conflicts with the special interrupt vector (IV).
>> +      * The bootloader has also set up CPU1 to respond to the wrong
>> +      * IPI interrupt.
>> +      * Here we will start up CPU1 in the background and ask it to
>> +      * reconfigure itself then go back to sleep.
>> +      */
>> +     memcpy((void *)0xa0000200, &bmips_smp_movevec, 0x20);
>> +     __sync();
>> +     set_c0_cause(C_SW0);
>> +     cpumask_set_cpu(1, &bmips_booted_mask);
>> +
>> +     /* FIXME: we really should have some sort of hazard barrier here */
>
> Any reason why the remainder of this code can't go into the smp_setup
> method?  That then would entirely eleminate the <censored> ifdef.

Yes, it would introduce an ifdef there, as this is (as far as I
understood Kevin) bcm63xx specific, and other platforms with BMIPS
CPUs aren't affected by it.
If you want I can replace the #ifdef with if (IS_ENABLED()) if that
suits you better.


Jonas