[PATCH V2 0/2] MIPS: BCM63XX: add SMP support

[PATCH V2 0/2] 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 has its second core only in a few variants enabled, but this can
be probed at runtime.

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/2, never had a problem with it so I left it
in place as to have it documented.

Changes V1 -> V2:
 * removed already applied patches
 * added a check for SMP availability on BCM6328
 * changed #ifdef FOO to if (IS_ENABLED(FOO))

Jonas Gorski (1):
  MIPS: BCM63XX: Enable second core SMP on BCM6328 if available

Kevin Cernekee (1):
  MIPS: BCM63XX: Add SMP support to prom.c

 arch/mips/bcm63xx/prom.c                          |   45 +++++++++++++++++++++
 arch/mips/include/asm/mach-bcm63xx/bcm63xx_cpu.h  |    2 +
 arch/mips/include/asm/mach-bcm63xx/bcm63xx_regs.h |    7 ++++
 3 files changed, 54 insertions(+)

-- 
1.7.10.4

[PATCH V2 1/2] 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,
 changed ifdef guards to if (IS_ENABLED())]
Signed-off-by: Jonas Gorski <jogo@openwrt.org>
---
V1 -> V2:
 * changed ifdef guards to if (IS_ENABLED())

 arch/mips/bcm63xx/prom.c |   41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/arch/mips/bcm63xx/prom.c b/arch/mips/bcm63xx/prom.c
index fd69808..33ddc78 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,43 @@ void __init prom_init(void)
 
 	/* do low level board init */
 	board_prom_init();
+
+	if (IS_ENABLED(CONFIG_CPU_BMIPS4350) && IS_ENABLED(CONFIG_SMP)) {
+		/* set up SMP */
+		register_smp_ops(&bmips_smp_ops);
+
+		/*
+		 * BCM6328 might not have its second CPU enabled, while BCM6358
+		 * needs special handling for its shared TLB, so disable SMP
+		 * for now.
+		 */
+		if (BCMCPU_IS_6328()) {
+			bmips_smp_enabled = 0;
+		} else if (BCMCPU_IS_6358()) {
+			bmips_smp_enabled = 0;
+		}
+
+		if (!bmips_smp_enabled)
+			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
+		 */
+	}
 }
 
 void __init prom_free_prom_memory(void)
-- 
1.7.10.4

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

[Sergei Shtylyov]

Hello.

On 18-06-2013 13:34, Jonas Gorski wrote:

> 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,
>   changed ifdef guards to if (IS_ENABLED())]
> Signed-off-by: Jonas Gorski <jogo@openwrt.org>
> ---
> V1 -> V2:
>   * changed ifdef guards to if (IS_ENABLED())

>   arch/mips/bcm63xx/prom.c |   41 +++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 41 insertions(+)

> diff --git a/arch/mips/bcm63xx/prom.c b/arch/mips/bcm63xx/prom.c
> index fd69808..33ddc78 100644
> --- a/arch/mips/bcm63xx/prom.c
> +++ b/arch/mips/bcm63xx/prom.c
[...]
> @@ -52,6 +56,43 @@ void __init prom_init(void)
>
>   	/* do low level board init */
>   	board_prom_init();
> +
> +	if (IS_ENABLED(CONFIG_CPU_BMIPS4350) && IS_ENABLED(CONFIG_SMP)) {
> +		/* set up SMP */
> +		register_smp_ops(&bmips_smp_ops);
> +
> +		/*
> +		 * BCM6328 might not have its second CPU enabled, while BCM6358
> +		 * needs special handling for its shared TLB, so disable SMP
> +		 * for now.
> +		 */
> +		if (BCMCPU_IS_6328()) {
> +			bmips_smp_enabled = 0;
> +		} else if (BCMCPU_IS_6358()) {
> +			bmips_smp_enabled = 0;
> +		}

     Doesn't scripts/checkpatch.pl complain here? You should not use {} 
on the single statement branches.

WBR, Sergei

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

[Jonas Gorski]

Hi Sergei,

On Tue, Jun 18, 2013 at 1:48 PM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> Hello.
>
>
> On 18-06-2013 13:34, Jonas Gorski wrote:
>
>> 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,
>>   changed ifdef guards to if (IS_ENABLED())]
>> Signed-off-by: Jonas Gorski <jogo@openwrt.org>
>> ---
>> V1 -> V2:
>>   * changed ifdef guards to if (IS_ENABLED())
>
>
>>   arch/mips/bcm63xx/prom.c |   41
>> +++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 41 insertions(+)
>
>
>> diff --git a/arch/mips/bcm63xx/prom.c b/arch/mips/bcm63xx/prom.c
>> index fd69808..33ddc78 100644
>> --- a/arch/mips/bcm63xx/prom.c
>> +++ b/arch/mips/bcm63xx/prom.c
>
> [...]
>
>> @@ -52,6 +56,43 @@ void __init prom_init(void)
>>
>>         /* do low level board init */
>>         board_prom_init();
>> +
>> +       if (IS_ENABLED(CONFIG_CPU_BMIPS4350) && IS_ENABLED(CONFIG_SMP)) {
>> +               /* set up SMP */
>> +               register_smp_ops(&bmips_smp_ops);
>> +
>> +               /*
>> +                * BCM6328 might not have its second CPU enabled, while
>> BCM6358
>> +                * needs special handling for its shared TLB, so disable
>> SMP
>> +                * for now.
>> +                */
>> +               if (BCMCPU_IS_6328()) {
>> +                       bmips_smp_enabled = 0;
>> +               } else if (BCMCPU_IS_6358()) {
>> +                       bmips_smp_enabled = 0;
>> +               }
>
>
>     Doesn't scripts/checkpatch.pl complain here? You should not use {} on
> the single statement branches.

I left the braces intentionally there because Patch 2 adds code to the
first branch, so I would have to add them in the next patch anyway.
This way the changes of Patch 2 stays smaller and easier to review
(and are limited to actual code changes).


Jonas

[PATCH V2 2/2] MIPS: BCM63XX: Enable second core SMP on BCM6328 if available

[Jonas Gorski]

BCM6328 has a OTP which tells us if the second core is available.

Signed-off-by: Jonas Gorski <jogo@openwrt.org>
---
 arch/mips/bcm63xx/prom.c                          |    6 +++++-
 arch/mips/include/asm/mach-bcm63xx/bcm63xx_cpu.h  |    2 ++
 arch/mips/include/asm/mach-bcm63xx/bcm63xx_regs.h |    7 +++++++
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/arch/mips/bcm63xx/prom.c b/arch/mips/bcm63xx/prom.c
index 33ddc78..f9ef7f7 100644
--- a/arch/mips/bcm63xx/prom.c
+++ b/arch/mips/bcm63xx/prom.c
@@ -67,7 +67,11 @@ void __init prom_init(void)
 		 * for now.
 		 */
 		if (BCMCPU_IS_6328()) {
-			bmips_smp_enabled = 0;
+			reg = bcm_readl(BCM_6328_OTP_BASE +
+					OTP_USER_BITS_6328_REG(3));
+
+			if (reg & OTP_6328_REG3_TP1_DISABLED)
+				bmips_smp_enabled = 0;
 		} else if (BCMCPU_IS_6358()) {
 			bmips_smp_enabled = 0;
 		}
diff --git a/arch/mips/include/asm/mach-bcm63xx/bcm63xx_cpu.h b/arch/mips/include/asm/mach-bcm63xx/bcm63xx_cpu.h
index 3362289..6a3020d 100644
--- a/arch/mips/include/asm/mach-bcm63xx/bcm63xx_cpu.h
+++ b/arch/mips/include/asm/mach-bcm63xx/bcm63xx_cpu.h
@@ -235,6 +235,8 @@ enum bcm63xx_regs_set {
 #define BCM_6328_PCMDMAS_BASE		(0xdeadbeef)
 #define BCM_6328_RNG_BASE		(0xdeadbeef)
 #define BCM_6328_MISC_BASE		(0xb0001800)
+#define BCM_6328_OTP_BASE		(0xb0000600)
+
 /*
  * 6338 register sets base address
  */
diff --git a/arch/mips/include/asm/mach-bcm63xx/bcm63xx_regs.h b/arch/mips/include/asm/mach-bcm63xx/bcm63xx_regs.h
index 3203fe4..ec97aa8 100644
--- a/arch/mips/include/asm/mach-bcm63xx/bcm63xx_regs.h
+++ b/arch/mips/include/asm/mach-bcm63xx/bcm63xx_regs.h
@@ -1434,4 +1434,11 @@
 
 #define PCIE_DEVICE_OFFSET		0x8000
 
+/*************************************************************************
+ * _REG relative to RSET_OTP
+ *************************************************************************/
+
+#define OTP_USER_BITS_6328_REG(i)	(0x20 + (i) * 4)
+#define   OTP_6328_REG3_TP1_DISABLED	BIT(9)
+
 #endif /* BCM63XX_REGS_H_ */
-- 
1.7.10.4

Re: [PATCH V2 2/2] MIPS: BCM63XX: Enable second core SMP on BCM6328 if available

[Ralf Baechle]

On Tue, Jun 18, 2013 at 11:34:32AM +0200, Jonas Gorski wrote:

> BCM6328 has a OTP which tells us if the second core is available.

Patch doesn't apply on top of my latest tree (more exactly, commit
426fe4812d09283a53b8294ae38c5a239eeee8ef) any more.  Can you respin?

Thanks,

  Ralf

Re: [PATCH V2 2/2] MIPS: BCM63XX: Enable second core SMP on BCM6328 if available

[Ralf Baechle]

On Thu, Jun 27, 2013 at 04:05:33PM +0200, Ralf Baechle wrote:

> > BCM6328 has a OTP which tells us if the second core is available.
> 
> Patch doesn't apply on top of my latest tree (more exactly, commit
> 426fe4812d09283a53b8294ae38c5a239eeee8ef) any more.  Can you respin?

Seems that was a patch ordering problem.  I can apply both patches of
this series if I drop https://patchwork.linux-mips.org/patch/5356/.

  Ralf