[PATCH 0/2] Add SMP support for the Allwinner A31 SoCs

[PATCH 0/2] Add SMP support for the Allwinner A31 SoCs

[Maxime Ripard]

Hi everyone,

The Allwinner A31 is a quad-Cortex-A7 SoC. This patchset adds support
for the bringup of the others CPUs at startup.

Thanks,
Maxime

Maxime Ripard (2):
  ARM: sun6i: dt: Add IP needed to bring up the additional cores
  ARM: sun6i: Add SMP support for the Allwinner A31

 arch/arm/boot/dts/sun6i-a31.dtsi |  10 ++++
 arch/arm/mach-sunxi/Makefile     |   1 +
 arch/arm/mach-sunxi/common.h     |  19 +++++++
 arch/arm/mach-sunxi/headsmp.S    |  11 ++++
 arch/arm/mach-sunxi/platsmp.c    | 115 +++++++++++++++++++++++++++++++++++++++
 arch/arm/mach-sunxi/sunxi.c      |   3 +
 6 files changed, 159 insertions(+)
 create mode 100644 arch/arm/mach-sunxi/common.h
 create mode 100644 arch/arm/mach-sunxi/headsmp.S
 create mode 100644 arch/arm/mach-sunxi/platsmp.c

-- 
1.8.4

[PATCH 0/2] Add SMP support for the Allwinner A31 SoCs

[Maxime Ripard]

Hi everyone,

The Allwinner A31 is a quad-Cortex-A7 SoC. This patchset adds support
for the bringup of the others CPUs at startup.

Thanks,
Maxime

Maxime Ripard (2):
  ARM: sun6i: dt: Add IP needed to bring up the additional cores
  ARM: sun6i: Add SMP support for the Allwinner A31

 arch/arm/boot/dts/sun6i-a31.dtsi |  10 ++++
 arch/arm/mach-sunxi/Makefile     |   1 +
 arch/arm/mach-sunxi/common.h     |  19 +++++++
 arch/arm/mach-sunxi/headsmp.S    |  11 ++++
 arch/arm/mach-sunxi/platsmp.c    | 115 +++++++++++++++++++++++++++++++++++++++
 arch/arm/mach-sunxi/sunxi.c      |   3 +
 6 files changed, 159 insertions(+)
 create mode 100644 arch/arm/mach-sunxi/common.h
 create mode 100644 arch/arm/mach-sunxi/headsmp.S
 create mode 100644 arch/arm/mach-sunxi/platsmp.c

-- 
1.8.4

[PATCH 1/2] ARM: sun6i: dt: Add IP needed to bring up the additional cores

[Maxime Ripard]

Add the PRCM and CPU configuration units needed for SMP in the A31 DTSI.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boot/dts/sun6i-a31.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi
index c1751a64889a..a0646877f5f2 100644
--- a/arch/arm/boot/dts/sun6i-a31.dtsi
+++ b/arch/arm/boot/dts/sun6i-a31.dtsi
@@ -295,5 +295,15 @@
 			#interrupt-cells = <3>;
 			interrupts = <1 9 0xf04>;
 		};
+
+		cpucfg@01f01c00 {
+			compatible = "allwinner,sun6i-a31-cpuconfig";
+			reg = <0x01f01c00 0x300>;
+		};
+
+		prcm@01f01c00 {
+			compatible = "allwinner,sun6i-a31-prcm";
+			reg = <0x01f01400 0x200>;
+		};
 	};
 };
-- 
1.8.4

[PATCH 1/2] ARM: sun6i: dt: Add IP needed to bring up the additional cores

[Maxime Ripard]

Add the PRCM and CPU configuration units needed for SMP in the A31 DTSI.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boot/dts/sun6i-a31.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi
index c1751a64889a..a0646877f5f2 100644
--- a/arch/arm/boot/dts/sun6i-a31.dtsi
+++ b/arch/arm/boot/dts/sun6i-a31.dtsi
@@ -295,5 +295,15 @@
 			#interrupt-cells = <3>;
 			interrupts = <1 9 0xf04>;
 		};
+
+		cpucfg at 01f01c00 {
+			compatible = "allwinner,sun6i-a31-cpuconfig";
+			reg = <0x01f01c00 0x300>;
+		};
+
+		prcm at 01f01c00 {
+			compatible = "allwinner,sun6i-a31-prcm";
+			reg = <0x01f01400 0x200>;
+		};
 	};
 };
-- 
1.8.4

[PATCH 2/2] ARM: sun6i: Add SMP support for the Allwinner A31

[Maxime Ripard]

The A31 is a quad Cortex-A7. Add the logic to use the IPs used to
control the CPU configuration and the CPU power so that we can bring up
secondary CPUs at boot.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/mach-sunxi/Makefile  |   1 +
 arch/arm/mach-sunxi/common.h  |  19 +++++++
 arch/arm/mach-sunxi/headsmp.S |  11 ++++
 arch/arm/mach-sunxi/platsmp.c | 115 ++++++++++++++++++++++++++++++++++++++++++
 arch/arm/mach-sunxi/sunxi.c   |   3 ++
 5 files changed, 149 insertions(+)
 create mode 100644 arch/arm/mach-sunxi/common.h
 create mode 100644 arch/arm/mach-sunxi/headsmp.S
 create mode 100644 arch/arm/mach-sunxi/platsmp.c

diff --git a/arch/arm/mach-sunxi/Makefile b/arch/arm/mach-sunxi/Makefile
index 93bebfc3ff9f..d9397202d6ec 100644
--- a/arch/arm/mach-sunxi/Makefile
+++ b/arch/arm/mach-sunxi/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_ARCH_SUNXI) += sunxi.o
+obj-$(CONFIG_SMP) += platsmp.o headsmp.o
diff --git a/arch/arm/mach-sunxi/common.h b/arch/arm/mach-sunxi/common.h
new file mode 100644
index 000000000000..9e5ac4756cbb
--- /dev/null
+++ b/arch/arm/mach-sunxi/common.h
@@ -0,0 +1,19 @@
+/*
+ * Core functions for Allwinner SoCs
+ *
+ * Copyright (C) 2013 Maxime Ripard
+ *
+ * Maxime Ripard <maxime.ripard@free-electrons.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#ifndef __ARCH_SUNXI_COMMON_H_
+#define __ARCH_SUNXI_COMMON_H_
+
+void sun6i_secondary_startup(void);
+extern struct smp_operations sun6i_smp_ops;
+
+#endif /* __ARCH_SUNXI_COMMON_H_ */
diff --git a/arch/arm/mach-sunxi/headsmp.S b/arch/arm/mach-sunxi/headsmp.S
new file mode 100644
index 000000000000..f5ff05e0e6a3
--- /dev/null
+++ b/arch/arm/mach-sunxi/headsmp.S
@@ -0,0 +1,11 @@
+#include <linux/linkage.h>
+#include <linux/init.h>
+
+        .section ".text.head", "ax"
+	__CPUINIT
+
+ENTRY(sun6i_secondary_startup)
+	msr	cpsr_fsxc, #0xd3
+	b	secondary_startup
+ENDPROC(sun6i_secondary_startup)
+
diff --git a/arch/arm/mach-sunxi/platsmp.c b/arch/arm/mach-sunxi/platsmp.c
new file mode 100644
index 000000000000..90045700c429
--- /dev/null
+++ b/arch/arm/mach-sunxi/platsmp.c
@@ -0,0 +1,115 @@
+/*
+ * SMP support for Allwinner SoCs
+ *
+ * Copyright (C) 2013 Maxime Ripard
+ *
+ * Maxime Ripard <maxime.ripard@free-electrons.com>
+ *
+ * Based on code
+ *  Copyright (C) 2012-2013 Allwinner Ltd.
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/memory.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/smp.h>
+
+#include "common.h"
+
+#define CPUCFG_CPU_PWR_CLAMP_STATUS_REG(cpu)	((cpu) * 0x40 + 0x64)
+#define CPUCFG_CPU_RST_CTRL_REG(cpu)		(((cpu) + 1) * 0x40)
+#define CPUCFG_CPU_CTRL_REG(cpu)		(((cpu) + 1) * 0x40 + 0x04)
+#define CPUCFG_CPU_STATUS_REG(cpu)		(((cpu) + 1) * 0x40 + 0x08)
+#define CPUCFG_GEN_CTRL_REG			0x184
+#define CPUCFG_PRIVATE0_REG			0x1a4
+#define CPUCFG_PRIVATE1_REG			0x1a8
+#define CPUCFG_DBG_CTL0_REG			0x1e0
+#define CPUCFG_DBG_CTL1_REG			0x1e4
+
+#define PRCM_CPU_PWROFF_REG			0x100
+#define PRCM_CPU_PWR_CLAMP_REG(cpu)		(((cpu) * 4) + 0x140)
+
+static void __iomem *cpucfg_membase;
+static void __iomem *prcm_membase;
+
+static DEFINE_SPINLOCK(cpu_lock);
+
+static void __init sun6i_smp_prepare_cpus(unsigned int max_cpus)
+{
+	struct device_node *node;
+
+	node = of_find_compatible_node(NULL, NULL, "allwinner,sun6i-a31-prcm");
+	if (!node)
+		panic("Missing A31 PRCM node in the device tree\n");
+
+	prcm_membase = of_iomap(node, 0);
+	if (!prcm_membase)
+		panic("Couldn't map A31 PRCM registers\n");
+
+	node = of_find_compatible_node(NULL, NULL,
+				       "allwinner,sun6i-a31-cpuconfig");
+	if (!node)
+		panic("Missing A31 CPU config node in the device tree\n");
+
+	cpucfg_membase = of_iomap(node, 0);
+	if (!cpucfg_membase)
+		panic("Couldn't map A31 CPU config registers\n");
+
+}
+
+static int sun6i_smp_boot_secondary(unsigned int cpu,
+				    struct task_struct *idle)
+{
+	u32 reg;
+	int i;
+
+	spin_lock(&cpu_lock);
+
+	/* Set CPU boot address */
+	writel(virt_to_phys(sun6i_secondary_startup),
+	       cpucfg_membase + CPUCFG_PRIVATE0_REG);
+
+	/* Assert the CPU core in reset */
+	writel(0, cpucfg_membase + CPUCFG_CPU_RST_CTRL_REG(cpu));
+
+	/* Assert the L1 cache in reset */
+	reg = readl(cpucfg_membase + CPUCFG_GEN_CTRL_REG);
+	writel(reg & ~BIT(cpu), cpucfg_membase + CPUCFG_GEN_CTRL_REG);
+
+	/* Disable external debug access */
+	reg = readl(cpucfg_membase + CPUCFG_DBG_CTL1_REG);
+	writel(reg & ~BIT(cpu), cpucfg_membase + CPUCFG_DBG_CTL1_REG);
+
+	/* Power up the CPU */
+	for (i = 0; i <= 8; i++)
+		writel(0xff >> i, prcm_membase + PRCM_CPU_PWR_CLAMP_REG(cpu));
+	mdelay(10);
+
+	/* Clear CPU power-off gating */
+	reg = readl(prcm_membase + PRCM_CPU_PWROFF_REG);
+	writel(reg & ~BIT(cpu), prcm_membase + PRCM_CPU_PWROFF_REG);
+	mdelay(1);
+
+	/* Deassert the CPU core reset */
+	writel(3, cpucfg_membase + CPUCFG_CPU_RST_CTRL_REG(cpu));
+
+	/* Enable back the external debug accesses */
+	reg = readl(cpucfg_membase + CPUCFG_DBG_CTL1_REG);
+	writel(reg | BIT(cpu), cpucfg_membase + CPUCFG_DBG_CTL1_REG);
+
+	spin_unlock(&cpu_lock);
+
+	return 0;
+}
+
+struct smp_operations sun6i_smp_ops __initdata = {
+	.smp_prepare_cpus	= sun6i_smp_prepare_cpus,
+	.smp_boot_secondary	= sun6i_smp_boot_secondary,
+};
diff --git a/arch/arm/mach-sunxi/sunxi.c b/arch/arm/mach-sunxi/sunxi.c
index f184f6c2fa33..b2e570253a8f 100644
--- a/arch/arm/mach-sunxi/sunxi.c
+++ b/arch/arm/mach-sunxi/sunxi.c
@@ -26,6 +26,8 @@
 #include <asm/mach/map.h>
 #include <asm/system_misc.h>
 
+#include "common.h"
+
 #define SUN4I_WATCHDOG_CTRL_REG		0x00
 #define SUN4I_WATCHDOG_CTRL_RESTART		BIT(0)
 #define SUN4I_WATCHDOG_MODE_REG		0x04
@@ -147,6 +149,7 @@ DT_MACHINE_START(SUN6I_DT, "Allwinner sun6i (A31) Family")
 	.init_time	= sunxi_timer_init,
 	.dt_compat	= sun6i_board_dt_compat,
 	.restart	= sun6i_restart,
+	.smp		= smp_ops(sun6i_smp_ops),
 MACHINE_END
 
 static const char * const sun7i_board_dt_compat[] = {
-- 
1.8.4

[PATCH 2/2] ARM: sun6i: Add SMP support for the Allwinner A31

[Maxime Ripard]

The A31 is a quad Cortex-A7. Add the logic to use the IPs used to
control the CPU configuration and the CPU power so that we can bring up
secondary CPUs at boot.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/mach-sunxi/Makefile  |   1 +
 arch/arm/mach-sunxi/common.h  |  19 +++++++
 arch/arm/mach-sunxi/headsmp.S |  11 ++++
 arch/arm/mach-sunxi/platsmp.c | 115 ++++++++++++++++++++++++++++++++++++++++++
 arch/arm/mach-sunxi/sunxi.c   |   3 ++
 5 files changed, 149 insertions(+)
 create mode 100644 arch/arm/mach-sunxi/common.h
 create mode 100644 arch/arm/mach-sunxi/headsmp.S
 create mode 100644 arch/arm/mach-sunxi/platsmp.c

diff --git a/arch/arm/mach-sunxi/Makefile b/arch/arm/mach-sunxi/Makefile
index 93bebfc3ff9f..d9397202d6ec 100644
--- a/arch/arm/mach-sunxi/Makefile
+++ b/arch/arm/mach-sunxi/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_ARCH_SUNXI) += sunxi.o
+obj-$(CONFIG_SMP) += platsmp.o headsmp.o
diff --git a/arch/arm/mach-sunxi/common.h b/arch/arm/mach-sunxi/common.h
new file mode 100644
index 000000000000..9e5ac4756cbb
--- /dev/null
+++ b/arch/arm/mach-sunxi/common.h
@@ -0,0 +1,19 @@
+/*
+ * Core functions for Allwinner SoCs
+ *
+ * Copyright (C) 2013 Maxime Ripard
+ *
+ * Maxime Ripard <maxime.ripard@free-electrons.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#ifndef __ARCH_SUNXI_COMMON_H_
+#define __ARCH_SUNXI_COMMON_H_
+
+void sun6i_secondary_startup(void);
+extern struct smp_operations sun6i_smp_ops;
+
+#endif /* __ARCH_SUNXI_COMMON_H_ */
diff --git a/arch/arm/mach-sunxi/headsmp.S b/arch/arm/mach-sunxi/headsmp.S
new file mode 100644
index 000000000000..f5ff05e0e6a3
--- /dev/null
+++ b/arch/arm/mach-sunxi/headsmp.S
@@ -0,0 +1,11 @@
+#include <linux/linkage.h>
+#include <linux/init.h>
+
+        .section ".text.head", "ax"
+	__CPUINIT
+
+ENTRY(sun6i_secondary_startup)
+	msr	cpsr_fsxc, #0xd3
+	b	secondary_startup
+ENDPROC(sun6i_secondary_startup)
+
diff --git a/arch/arm/mach-sunxi/platsmp.c b/arch/arm/mach-sunxi/platsmp.c
new file mode 100644
index 000000000000..90045700c429
--- /dev/null
+++ b/arch/arm/mach-sunxi/platsmp.c
@@ -0,0 +1,115 @@
+/*
+ * SMP support for Allwinner SoCs
+ *
+ * Copyright (C) 2013 Maxime Ripard
+ *
+ * Maxime Ripard <maxime.ripard@free-electrons.com>
+ *
+ * Based on code
+ *  Copyright (C) 2012-2013 Allwinner Ltd.
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/memory.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/smp.h>
+
+#include "common.h"
+
+#define CPUCFG_CPU_PWR_CLAMP_STATUS_REG(cpu)	((cpu) * 0x40 + 0x64)
+#define CPUCFG_CPU_RST_CTRL_REG(cpu)		(((cpu) + 1) * 0x40)
+#define CPUCFG_CPU_CTRL_REG(cpu)		(((cpu) + 1) * 0x40 + 0x04)
+#define CPUCFG_CPU_STATUS_REG(cpu)		(((cpu) + 1) * 0x40 + 0x08)
+#define CPUCFG_GEN_CTRL_REG			0x184
+#define CPUCFG_PRIVATE0_REG			0x1a4
+#define CPUCFG_PRIVATE1_REG			0x1a8
+#define CPUCFG_DBG_CTL0_REG			0x1e0
+#define CPUCFG_DBG_CTL1_REG			0x1e4
+
+#define PRCM_CPU_PWROFF_REG			0x100
+#define PRCM_CPU_PWR_CLAMP_REG(cpu)		(((cpu) * 4) + 0x140)
+
+static void __iomem *cpucfg_membase;
+static void __iomem *prcm_membase;
+
+static DEFINE_SPINLOCK(cpu_lock);
+
+static void __init sun6i_smp_prepare_cpus(unsigned int max_cpus)
+{
+	struct device_node *node;
+
+	node = of_find_compatible_node(NULL, NULL, "allwinner,sun6i-a31-prcm");
+	if (!node)
+		panic("Missing A31 PRCM node in the device tree\n");
+
+	prcm_membase = of_iomap(node, 0);
+	if (!prcm_membase)
+		panic("Couldn't map A31 PRCM registers\n");
+
+	node = of_find_compatible_node(NULL, NULL,
+				       "allwinner,sun6i-a31-cpuconfig");
+	if (!node)
+		panic("Missing A31 CPU config node in the device tree\n");
+
+	cpucfg_membase = of_iomap(node, 0);
+	if (!cpucfg_membase)
+		panic("Couldn't map A31 CPU config registers\n");
+
+}
+
+static int sun6i_smp_boot_secondary(unsigned int cpu,
+				    struct task_struct *idle)
+{
+	u32 reg;
+	int i;
+
+	spin_lock(&cpu_lock);
+
+	/* Set CPU boot address */
+	writel(virt_to_phys(sun6i_secondary_startup),
+	       cpucfg_membase + CPUCFG_PRIVATE0_REG);
+
+	/* Assert the CPU core in reset */
+	writel(0, cpucfg_membase + CPUCFG_CPU_RST_CTRL_REG(cpu));
+
+	/* Assert the L1 cache in reset */
+	reg = readl(cpucfg_membase + CPUCFG_GEN_CTRL_REG);
+	writel(reg & ~BIT(cpu), cpucfg_membase + CPUCFG_GEN_CTRL_REG);
+
+	/* Disable external debug access */
+	reg = readl(cpucfg_membase + CPUCFG_DBG_CTL1_REG);
+	writel(reg & ~BIT(cpu), cpucfg_membase + CPUCFG_DBG_CTL1_REG);
+
+	/* Power up the CPU */
+	for (i = 0; i <= 8; i++)
+		writel(0xff >> i, prcm_membase + PRCM_CPU_PWR_CLAMP_REG(cpu));
+	mdelay(10);
+
+	/* Clear CPU power-off gating */
+	reg = readl(prcm_membase + PRCM_CPU_PWROFF_REG);
+	writel(reg & ~BIT(cpu), prcm_membase + PRCM_CPU_PWROFF_REG);
+	mdelay(1);
+
+	/* Deassert the CPU core reset */
+	writel(3, cpucfg_membase + CPUCFG_CPU_RST_CTRL_REG(cpu));
+
+	/* Enable back the external debug accesses */
+	reg = readl(cpucfg_membase + CPUCFG_DBG_CTL1_REG);
+	writel(reg | BIT(cpu), cpucfg_membase + CPUCFG_DBG_CTL1_REG);
+
+	spin_unlock(&cpu_lock);
+
+	return 0;
+}
+
+struct smp_operations sun6i_smp_ops __initdata = {
+	.smp_prepare_cpus	= sun6i_smp_prepare_cpus,
+	.smp_boot_secondary	= sun6i_smp_boot_secondary,
+};
diff --git a/arch/arm/mach-sunxi/sunxi.c b/arch/arm/mach-sunxi/sunxi.c
index f184f6c2fa33..b2e570253a8f 100644
--- a/arch/arm/mach-sunxi/sunxi.c
+++ b/arch/arm/mach-sunxi/sunxi.c
@@ -26,6 +26,8 @@
 #include <asm/mach/map.h>
 #include <asm/system_misc.h>
 
+#include "common.h"
+
 #define SUN4I_WATCHDOG_CTRL_REG		0x00
 #define SUN4I_WATCHDOG_CTRL_RESTART		BIT(0)
 #define SUN4I_WATCHDOG_MODE_REG		0x04
@@ -147,6 +149,7 @@ DT_MACHINE_START(SUN6I_DT, "Allwinner sun6i (A31) Family")
 	.init_time	= sunxi_timer_init,
 	.dt_compat	= sun6i_board_dt_compat,
 	.restart	= sun6i_restart,
+	.smp		= smp_ops(sun6i_smp_ops),
 MACHINE_END
 
 static const char * const sun7i_board_dt_compat[] = {
-- 
1.8.4

Re: [linux-sunxi] [PATCH 2/2] ARM: sun6i: Add SMP support for the Allwinner A31

[Ian Campbell]

Not a comment on the patch, or even A31, but a question about how the
SMP boot process works on sunxi generally...

On Sun, 2013-11-03 at 10:30 +0100, Maxime Ripard wrote:
> The A31 is a quad Cortex-A7. Add the logic to use the IPs used to
> control the CPU configuration and the CPU power so that we can bring up
> secondary CPUs at boot.
> [...]
> +	/* Set CPU boot address */
> +	writel(virt_to_phys(sun6i_secondary_startup),
> +	       cpucfg_membase + CPUCFG_PRIVATE0_REG);

Does the secondary CPU jump straight here from the lowlevel firmware or
does it go through the bootloader which reads the reg and does the jump?
I can't see any reference to this reg in u-boot so I guess the former?

I'm trying to work out if we can make this work with the requirement
which both Xen and KVM have to enter the kernel in NS-HYP mode.

The way this works on e.g. vexpress is (roughly) that u-boot wakes up
the secondary CPUs from the lowlevel firmware and places them into its
own holding pen, which has the same wake up protocol as the firmware so
the kernel can just use the same code. If u-boot never gets to run on
secondary CPUs that isn't going to help much. 

My concern is that the sequence here appears to involve resetting the
secondary CPU, which I figure will probably defeat that strategy by
kicking the CPU back into the lowlevel firmware in the reset state,
meaning it can't be done by a u-boot only change.

Hrm, what to do ... perhaps a DT driven selection between this mechanism
and sev to kick a wfe loop reading the private register?

Ian.

> +
> +	/* Assert the CPU core in reset */
> +	writel(0, cpucfg_membase + CPUCFG_CPU_RST_CTRL_REG(cpu));
> +
> +	/* Assert the L1 cache in reset */
> +	reg = readl(cpucfg_membase + CPUCFG_GEN_CTRL_REG);
> +	writel(reg & ~BIT(cpu), cpucfg_membase + CPUCFG_GEN_CTRL_REG);
> +
> +	/* Disable external debug access */
> +	reg = readl(cpucfg_membase + CPUCFG_DBG_CTL1_REG);
> +	writel(reg & ~BIT(cpu), cpucfg_membase + CPUCFG_DBG_CTL1_REG);
> +
> +	/* Power up the CPU */
> +	for (i = 0; i <= 8; i++)
> +		writel(0xff >> i, prcm_membase + PRCM_CPU_PWR_CLAMP_REG(cpu));
> +	mdelay(10);
> +
> +	/* Clear CPU power-off gating */
> +	reg = readl(prcm_membase + PRCM_CPU_PWROFF_REG);
> +	writel(reg & ~BIT(cpu), prcm_membase + PRCM_CPU_PWROFF_REG);
> +	mdelay(1);
> +
> +	/* Deassert the CPU core reset */
> +	writel(3, cpucfg_membase + CPUCFG_CPU_RST_CTRL_REG(cpu));
> +
> +	/* Enable back the external debug accesses */
> +	reg = readl(cpucfg_membase + CPUCFG_DBG_CTL1_REG);
> +	writel(reg | BIT(cpu), cpucfg_membase + CPUCFG_DBG_CTL1_REG);
> +
> +	spin_unlock(&cpu_lock);
> +
> +	return 0;
> +}
> +
> +struct smp_operations sun6i_smp_ops __initdata = {
> +	.smp_prepare_cpus	= sun6i_smp_prepare_cpus,
> +	.smp_boot_secondary	= sun6i_smp_boot_secondary,
> +};
> diff --git a/arch/arm/mach-sunxi/sunxi.c b/arch/arm/mach-sunxi/sunxi.c
> index f184f6c2fa33..b2e570253a8f 100644
> --- a/arch/arm/mach-sunxi/sunxi.c
> +++ b/arch/arm/mach-sunxi/sunxi.c
> @@ -26,6 +26,8 @@
>  #include <asm/mach/map.h>
>  #include <asm/system_misc.h>
>  
> +#include "common.h"
> +
>  #define SUN4I_WATCHDOG_CTRL_REG		0x00
>  #define SUN4I_WATCHDOG_CTRL_RESTART		BIT(0)
>  #define SUN4I_WATCHDOG_MODE_REG		0x04
> @@ -147,6 +149,7 @@ DT_MACHINE_START(SUN6I_DT, "Allwinner sun6i (A31) Family")
>  	.init_time	= sunxi_timer_init,
>  	.dt_compat	= sun6i_board_dt_compat,
>  	.restart	= sun6i_restart,
> +	.smp		= smp_ops(sun6i_smp_ops),
>  MACHINE_END
>  
>  static const char * const sun7i_board_dt_compat[] = {
> -- 
> 1.8.4
> 

[linux-sunxi] [PATCH 2/2] ARM: sun6i: Add SMP support for the Allwinner A31

[Ian Campbell]

Not a comment on the patch, or even A31, but a question about how the
SMP boot process works on sunxi generally...

On Sun, 2013-11-03 at 10:30 +0100, Maxime Ripard wrote:
> The A31 is a quad Cortex-A7. Add the logic to use the IPs used to
> control the CPU configuration and the CPU power so that we can bring up
> secondary CPUs at boot.
> [...]
> +	/* Set CPU boot address */
> +	writel(virt_to_phys(sun6i_secondary_startup),
> +	       cpucfg_membase + CPUCFG_PRIVATE0_REG);

Does the secondary CPU jump straight here from the lowlevel firmware or
does it go through the bootloader which reads the reg and does the jump?
I can't see any reference to this reg in u-boot so I guess the former?

I'm trying to work out if we can make this work with the requirement
which both Xen and KVM have to enter the kernel in NS-HYP mode.

The way this works on e.g. vexpress is (roughly) that u-boot wakes up
the secondary CPUs from the lowlevel firmware and places them into its
own holding pen, which has the same wake up protocol as the firmware so
the kernel can just use the same code. If u-boot never gets to run on
secondary CPUs that isn't going to help much. 

My concern is that the sequence here appears to involve resetting the
secondary CPU, which I figure will probably defeat that strategy by
kicking the CPU back into the lowlevel firmware in the reset state,
meaning it can't be done by a u-boot only change.

Hrm, what to do ... perhaps a DT driven selection between this mechanism
and sev to kick a wfe loop reading the private register?

Ian.

> +
> +	/* Assert the CPU core in reset */
> +	writel(0, cpucfg_membase + CPUCFG_CPU_RST_CTRL_REG(cpu));
> +
> +	/* Assert the L1 cache in reset */
> +	reg = readl(cpucfg_membase + CPUCFG_GEN_CTRL_REG);
> +	writel(reg & ~BIT(cpu), cpucfg_membase + CPUCFG_GEN_CTRL_REG);
> +
> +	/* Disable external debug access */
> +	reg = readl(cpucfg_membase + CPUCFG_DBG_CTL1_REG);
> +	writel(reg & ~BIT(cpu), cpucfg_membase + CPUCFG_DBG_CTL1_REG);
> +
> +	/* Power up the CPU */
> +	for (i = 0; i <= 8; i++)
> +		writel(0xff >> i, prcm_membase + PRCM_CPU_PWR_CLAMP_REG(cpu));
> +	mdelay(10);
> +
> +	/* Clear CPU power-off gating */
> +	reg = readl(prcm_membase + PRCM_CPU_PWROFF_REG);
> +	writel(reg & ~BIT(cpu), prcm_membase + PRCM_CPU_PWROFF_REG);
> +	mdelay(1);
> +
> +	/* Deassert the CPU core reset */
> +	writel(3, cpucfg_membase + CPUCFG_CPU_RST_CTRL_REG(cpu));
> +
> +	/* Enable back the external debug accesses */
> +	reg = readl(cpucfg_membase + CPUCFG_DBG_CTL1_REG);
> +	writel(reg | BIT(cpu), cpucfg_membase + CPUCFG_DBG_CTL1_REG);
> +
> +	spin_unlock(&cpu_lock);
> +
> +	return 0;
> +}
> +
> +struct smp_operations sun6i_smp_ops __initdata = {
> +	.smp_prepare_cpus	= sun6i_smp_prepare_cpus,
> +	.smp_boot_secondary	= sun6i_smp_boot_secondary,
> +};
> diff --git a/arch/arm/mach-sunxi/sunxi.c b/arch/arm/mach-sunxi/sunxi.c
> index f184f6c2fa33..b2e570253a8f 100644
> --- a/arch/arm/mach-sunxi/sunxi.c
> +++ b/arch/arm/mach-sunxi/sunxi.c
> @@ -26,6 +26,8 @@
>  #include <asm/mach/map.h>
>  #include <asm/system_misc.h>
>  
> +#include "common.h"
> +
>  #define SUN4I_WATCHDOG_CTRL_REG		0x00
>  #define SUN4I_WATCHDOG_CTRL_RESTART		BIT(0)
>  #define SUN4I_WATCHDOG_MODE_REG		0x04
> @@ -147,6 +149,7 @@ DT_MACHINE_START(SUN6I_DT, "Allwinner sun6i (A31) Family")
>  	.init_time	= sunxi_timer_init,
>  	.dt_compat	= sun6i_board_dt_compat,
>  	.restart	= sun6i_restart,
> +	.smp		= smp_ops(sun6i_smp_ops),
>  MACHINE_END
>  
>  static const char * const sun7i_board_dt_compat[] = {
> -- 
> 1.8.4
> 

Re: [linux-sunxi] [PATCH 2/2] ARM: sun6i: Add SMP support for the Allwinner A31

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2593 bytes --]

Hi Ian,

On Mon, Nov 04, 2013 at 04:53:17PM +0000, Ian Campbell wrote:
> Not a comment on the patch, or even A31, but a question about how the
> SMP boot process works on sunxi generally...
> 
> On Sun, 2013-11-03 at 10:30 +0100, Maxime Ripard wrote:
> > The A31 is a quad Cortex-A7. Add the logic to use the IPs used to
> > control the CPU configuration and the CPU power so that we can bring up
> > secondary CPUs at boot.
> > [...]
> > +	/* Set CPU boot address */
> > +	writel(virt_to_phys(sun6i_secondary_startup),
> > +	       cpucfg_membase + CPUCFG_PRIVATE0_REG);
> 
> Does the secondary CPU jump straight here from the lowlevel firmware or
> does it go through the bootloader which reads the reg and does the jump?
> I can't see any reference to this reg in u-boot so I guess the former?

There's no reference of this either in the A31 datasheet (or at least,
none I could find), so I assume the former too.

> I'm trying to work out if we can make this work with the requirement
> which both Xen and KVM have to enter the kernel in NS-HYP mode.
> 
> The way this works on e.g. vexpress is (roughly) that u-boot wakes up
> the secondary CPUs from the lowlevel firmware and places them into its
> own holding pen, which has the same wake up protocol as the firmware so
> the kernel can just use the same code. If u-boot never gets to run on
> secondary CPUs that isn't going to help much. 
> 
> My concern is that the sequence here appears to involve resetting the
> secondary CPU, which I figure will probably defeat that strategy by
> kicking the CPU back into the lowlevel firmware in the reset state,
> meaning it can't be done by a u-boot only change.

I think this is where we're headed for the A20, Marc was interested in
doing that, since we already have pretty much this in u-boot already,
however, this is not the case for the A31.

As far as I know, the Allwinner's bootloader that we currently use
isn't bringing up the secondary CPUs, and we don't have any port of
some sort of u-boot yet that we could work on.

So, I guess we don't really have much choice in that case, even though
eventually I'd like to have this for the A31 too.

> Hrm, what to do ... perhaps a DT driven selection between this mechanism
> and sev to kick a wfe loop reading the private register?

We can discuss this whenever we will actually have that choice to
make, but maybe a kernel parameter would be better?

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[linux-sunxi] [PATCH 2/2] ARM: sun6i: Add SMP support for the Allwinner A31

[Maxime Ripard]

Hi Ian,

On Mon, Nov 04, 2013 at 04:53:17PM +0000, Ian Campbell wrote:
> Not a comment on the patch, or even A31, but a question about how the
> SMP boot process works on sunxi generally...
> 
> On Sun, 2013-11-03 at 10:30 +0100, Maxime Ripard wrote:
> > The A31 is a quad Cortex-A7. Add the logic to use the IPs used to
> > control the CPU configuration and the CPU power so that we can bring up
> > secondary CPUs at boot.
> > [...]
> > +	/* Set CPU boot address */
> > +	writel(virt_to_phys(sun6i_secondary_startup),
> > +	       cpucfg_membase + CPUCFG_PRIVATE0_REG);
> 
> Does the secondary CPU jump straight here from the lowlevel firmware or
> does it go through the bootloader which reads the reg and does the jump?
> I can't see any reference to this reg in u-boot so I guess the former?

There's no reference of this either in the A31 datasheet (or at least,
none I could find), so I assume the former too.

> I'm trying to work out if we can make this work with the requirement
> which both Xen and KVM have to enter the kernel in NS-HYP mode.
> 
> The way this works on e.g. vexpress is (roughly) that u-boot wakes up
> the secondary CPUs from the lowlevel firmware and places them into its
> own holding pen, which has the same wake up protocol as the firmware so
> the kernel can just use the same code. If u-boot never gets to run on
> secondary CPUs that isn't going to help much. 
> 
> My concern is that the sequence here appears to involve resetting the
> secondary CPU, which I figure will probably defeat that strategy by
> kicking the CPU back into the lowlevel firmware in the reset state,
> meaning it can't be done by a u-boot only change.

I think this is where we're headed for the A20, Marc was interested in
doing that, since we already have pretty much this in u-boot already,
however, this is not the case for the A31.

As far as I know, the Allwinner's bootloader that we currently use
isn't bringing up the secondary CPUs, and we don't have any port of
some sort of u-boot yet that we could work on.

So, I guess we don't really have much choice in that case, even though
eventually I'd like to have this for the A31 too.

> Hrm, what to do ... perhaps a DT driven selection between this mechanism
> and sev to kick a wfe loop reading the private register?

We can discuss this whenever we will actually have that choice to
make, but maybe a kernel parameter would be better?

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131108/cdb8410f/attachment.sig>

Re: [linux-sunxi] [PATCH 2/2] ARM: sun6i: Add SMP support for the Allwinner A31

[Ian Campbell]

On Fri, 2013-11-08 at 09:40 +0100, Maxime Ripard wrote:
> > I'm trying to work out if we can make this work with the requirement
> > which both Xen and KVM have to enter the kernel in NS-HYP mode.
> > 
> > The way this works on e.g. vexpress is (roughly) that u-boot wakes up
> > the secondary CPUs from the lowlevel firmware and places them into its
> > own holding pen, which has the same wake up protocol as the firmware so
> > the kernel can just use the same code. If u-boot never gets to run on
> > secondary CPUs that isn't going to help much. 
> > 
> > My concern is that the sequence here appears to involve resetting the
> > secondary CPU, which I figure will probably defeat that strategy by
> > kicking the CPU back into the lowlevel firmware in the reset state,
> > meaning it can't be done by a u-boot only change.
> 
> I think this is where we're headed for the A20, Marc was interested in
> doing that,

Marc Zyngier is that?

>  since we already have pretty much this in u-boot already,
> however, this is not the case for the A31.

> As far as I know, the Allwinner's bootloader that we currently use
> isn't bringing up the secondary CPUs, and we don't have any port of
> some sort of u-boot yet that we could work on.

Ah, OK. I'd assumed that A20 and A31 (indeed, most sunxi platforms) were
mostly equivalent as far as u-boot support went.

> So, I guess we don't really have much choice in that case, even though
> eventually I'd like to have this for the A31 too.

Right, I suppose it makes sense to consider what we want to do on the
A20 now and keep in mind that A31 may want to follow in the future.

> > Hrm, what to do ... perhaps a DT driven selection between this mechanism
> > and sev to kick a wfe loop reading the private register?
> 
> We can discuss this whenever we will actually have that choice to
> make, but maybe a kernel parameter would be better?

I don't think so -- u-boot would then have to munge the command line to
say that it had/had not brought up secondaries. DTB seems more natural
to me. e.g. on ARMv8 there is already a requirement to provide a per-CPU
property describing the bringup protocol ("PSCI" and "spintable" are the
options there).

Anyway, once I get to the point of being able to do something I'll
coordinate with Marc etc and figure out what to do. In the meantime I
think having the kernel do the bringup (like this patch does) is
sensible. It's very likely to be what we want to do in the absence of
any instruction to the contrary (DTB or otherwise) in the future anyway.

Thanks,

Ian.

[linux-sunxi] [PATCH 2/2] ARM: sun6i: Add SMP support for the Allwinner A31

[Ian Campbell]

On Fri, 2013-11-08 at 09:40 +0100, Maxime Ripard wrote:
> > I'm trying to work out if we can make this work with the requirement
> > which both Xen and KVM have to enter the kernel in NS-HYP mode.
> > 
> > The way this works on e.g. vexpress is (roughly) that u-boot wakes up
> > the secondary CPUs from the lowlevel firmware and places them into its
> > own holding pen, which has the same wake up protocol as the firmware so
> > the kernel can just use the same code. If u-boot never gets to run on
> > secondary CPUs that isn't going to help much. 
> > 
> > My concern is that the sequence here appears to involve resetting the
> > secondary CPU, which I figure will probably defeat that strategy by
> > kicking the CPU back into the lowlevel firmware in the reset state,
> > meaning it can't be done by a u-boot only change.
> 
> I think this is where we're headed for the A20, Marc was interested in
> doing that,

Marc Zyngier is that?

>  since we already have pretty much this in u-boot already,
> however, this is not the case for the A31.

> As far as I know, the Allwinner's bootloader that we currently use
> isn't bringing up the secondary CPUs, and we don't have any port of
> some sort of u-boot yet that we could work on.

Ah, OK. I'd assumed that A20 and A31 (indeed, most sunxi platforms) were
mostly equivalent as far as u-boot support went.

> So, I guess we don't really have much choice in that case, even though
> eventually I'd like to have this for the A31 too.

Right, I suppose it makes sense to consider what we want to do on the
A20 now and keep in mind that A31 may want to follow in the future.

> > Hrm, what to do ... perhaps a DT driven selection between this mechanism
> > and sev to kick a wfe loop reading the private register?
> 
> We can discuss this whenever we will actually have that choice to
> make, but maybe a kernel parameter would be better?

I don't think so -- u-boot would then have to munge the command line to
say that it had/had not brought up secondaries. DTB seems more natural
to me. e.g. on ARMv8 there is already a requirement to provide a per-CPU
property describing the bringup protocol ("PSCI" and "spintable" are the
options there).

Anyway, once I get to the point of being able to do something I'll
coordinate with Marc etc and figure out what to do. In the meantime I
think having the kernel do the bringup (like this patch does) is
sensible. It's very likely to be what we want to do in the absence of
any instruction to the contrary (DTB or otherwise) in the future anyway.

Thanks,

Ian.

[linux-sunxi] [PATCH 2/2] ARM: sun6i: Add SMP support for the Allwinner A31

[Marc Zyngier]

On 2013-11-08 10:25, Ian Campbell wrote:
> On Fri, 2013-11-08 at 09:40 +0100, Maxime Ripard wrote:
>> > I'm trying to work out if we can make this work with the 
>> requirement
>> > which both Xen and KVM have to enter the kernel in NS-HYP mode.
>> >
>> > The way this works on e.g. vexpress is (roughly) that u-boot wakes 
>> up
>> > the secondary CPUs from the lowlevel firmware and places them into 
>> its
>> > own holding pen, which has the same wake up protocol as the 
>> firmware so
>> > the kernel can just use the same code. If u-boot never gets to run 
>> on
>> > secondary CPUs that isn't going to help much.
>> >
>> > My concern is that the sequence here appears to involve resetting 
>> the
>> > secondary CPU, which I figure will probably defeat that strategy 
>> by
>> > kicking the CPU back into the lowlevel firmware in the reset 
>> state,
>> > meaning it can't be done by a u-boot only change.
>>
>> I think this is where we're headed for the A20, Marc was interested 
>> in
>> doing that,
>
> Marc Zyngier is that?

Yup, that's me. My cunning plan is to add PSCI support to u-boot, and 
do all the HYP stuff there.

>>  since we already have pretty much this in u-boot already,
>> however, this is not the case for the A31.
>
>> As far as I know, the Allwinner's bootloader that we currently use
>> isn't bringing up the secondary CPUs, and we don't have any port of
>> some sort of u-boot yet that we could work on.
>
> Ah, OK. I'd assumed that A20 and A31 (indeed, most sunxi platforms) 
> were
> mostly equivalent as far as u-boot support went.
>
>> So, I guess we don't really have much choice in that case, even 
>> though
>> eventually I'd like to have this for the A31 too.
>
> Right, I suppose it makes sense to consider what we want to do on the
> A20 now and keep in mind that A31 may want to follow in the future.

If we have the magic incantations to power on the A31 secondaries, it 
should be fairly easy to integrate that into PSCI, assuming we have some 
SRAM available somewhere (otherwise, we'll have to reserve some memory 
in DRAM).

>> > Hrm, what to do ... perhaps a DT driven selection between this 
>> mechanism
>> > and sev to kick a wfe loop reading the private register?
>>
>> We can discuss this whenever we will actually have that choice to
>> make, but maybe a kernel parameter would be better?
>
> I don't think so -- u-boot would then have to munge the command line 
> to
> say that it had/had not brought up secondaries. DTB seems more 
> natural
> to me. e.g. on ARMv8 there is already a requirement to provide a 
> per-CPU
> property describing the bringup protocol ("PSCI" and "spintable" are 
> the
> options there).
>
> Anyway, once I get to the point of being able to do something I'll
> coordinate with Marc etc and figure out what to do. In the meantime I
> think having the kernel do the bringup (like this patch does) is
> sensible. It's very likely to be what we want to do in the absence of
> any instruction to the contrary (DTB or otherwise) in the future 
> anyway.

I'm planning to hack something over the weekend if nothing comes 
between me and the damned board.

         M.
-- 
Who you jivin' with that Cosmik Debris?

Re: [linux-sunxi] [PATCH 2/2] ARM: sun6i: Add SMP support for the Allwinner A31

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 3350 bytes --]

Hi Ian,

On Fri, Nov 08, 2013 at 10:25:55AM +0000, Ian Campbell wrote:
> On Fri, 2013-11-08 at 09:40 +0100, Maxime Ripard wrote:
> > > I'm trying to work out if we can make this work with the requirement
> > > which both Xen and KVM have to enter the kernel in NS-HYP mode.
> > > 
> > > The way this works on e.g. vexpress is (roughly) that u-boot wakes up
> > > the secondary CPUs from the lowlevel firmware and places them into its
> > > own holding pen, which has the same wake up protocol as the firmware so
> > > the kernel can just use the same code. If u-boot never gets to run on
> > > secondary CPUs that isn't going to help much. 
> > > 
> > > My concern is that the sequence here appears to involve resetting the
> > > secondary CPU, which I figure will probably defeat that strategy by
> > > kicking the CPU back into the lowlevel firmware in the reset state,
> > > meaning it can't be done by a u-boot only change.
> > 
> > I think this is where we're headed for the A20, Marc was interested in
> > doing that,
> 
> Marc Zyngier is that?

Ah yes. I forgot to put it in CC...

> >  since we already have pretty much this in u-boot already,
> > however, this is not the case for the A31.
> 
> > As far as I know, the Allwinner's bootloader that we currently use
> > isn't bringing up the secondary CPUs, and we don't have any port of
> > some sort of u-boot yet that we could work on.
> 
> Ah, OK. I'd assumed that A20 and A31 (indeed, most sunxi platforms) were
> mostly equivalent as far as u-boot support went.

No. The A31 has no current support at all in u-boot(-sunxi, that is),
so the only bootloader we can use is Allwinner's one.

It's one my TODO list somewhere, but as usual, time is lacking :)

> > So, I guess we don't really have much choice in that case, even though
> > eventually I'd like to have this for the A31 too.
> 
> Right, I suppose it makes sense to consider what we want to do on the
> A20 now and keep in mind that A31 may want to follow in the future.
> 
> > > Hrm, what to do ... perhaps a DT driven selection between this mechanism
> > > and sev to kick a wfe loop reading the private register?
> > 
> > We can discuss this whenever we will actually have that choice to
> > make, but maybe a kernel parameter would be better?
> 
> I don't think so -- u-boot would then have to munge the command line to
> say that it had/had not brought up secondaries. DTB seems more natural
> to me. e.g. on ARMv8 there is already a requirement to provide a per-CPU
> property describing the bringup protocol ("PSCI" and "spintable" are the
> options there).

Then I guess we can assume that we have to do all the CPU bring up
work if this property is missing?

> Anyway, once I get to the point of being able to do something I'll
> coordinate with Marc etc and figure out what to do. In the meantime I
> think having the kernel do the bringup (like this patch does) is
> sensible. It's very likely to be what we want to do in the absence of
> any instruction to the contrary (DTB or otherwise) in the future anyway.

Yep.

A part from the discussion on the approach, do you have any comments
on the patches themselves?

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[linux-sunxi] [PATCH 2/2] ARM: sun6i: Add SMP support for the Allwinner A31

[Maxime Ripard]

Hi Ian,

On Fri, Nov 08, 2013 at 10:25:55AM +0000, Ian Campbell wrote:
> On Fri, 2013-11-08 at 09:40 +0100, Maxime Ripard wrote:
> > > I'm trying to work out if we can make this work with the requirement
> > > which both Xen and KVM have to enter the kernel in NS-HYP mode.
> > > 
> > > The way this works on e.g. vexpress is (roughly) that u-boot wakes up
> > > the secondary CPUs from the lowlevel firmware and places them into its
> > > own holding pen, which has the same wake up protocol as the firmware so
> > > the kernel can just use the same code. If u-boot never gets to run on
> > > secondary CPUs that isn't going to help much. 
> > > 
> > > My concern is that the sequence here appears to involve resetting the
> > > secondary CPU, which I figure will probably defeat that strategy by
> > > kicking the CPU back into the lowlevel firmware in the reset state,
> > > meaning it can't be done by a u-boot only change.
> > 
> > I think this is where we're headed for the A20, Marc was interested in
> > doing that,
> 
> Marc Zyngier is that?

Ah yes. I forgot to put it in CC...

> >  since we already have pretty much this in u-boot already,
> > however, this is not the case for the A31.
> 
> > As far as I know, the Allwinner's bootloader that we currently use
> > isn't bringing up the secondary CPUs, and we don't have any port of
> > some sort of u-boot yet that we could work on.
> 
> Ah, OK. I'd assumed that A20 and A31 (indeed, most sunxi platforms) were
> mostly equivalent as far as u-boot support went.

No. The A31 has no current support at all in u-boot(-sunxi, that is),
so the only bootloader we can use is Allwinner's one.

It's one my TODO list somewhere, but as usual, time is lacking :)

> > So, I guess we don't really have much choice in that case, even though
> > eventually I'd like to have this for the A31 too.
> 
> Right, I suppose it makes sense to consider what we want to do on the
> A20 now and keep in mind that A31 may want to follow in the future.
> 
> > > Hrm, what to do ... perhaps a DT driven selection between this mechanism
> > > and sev to kick a wfe loop reading the private register?
> > 
> > We can discuss this whenever we will actually have that choice to
> > make, but maybe a kernel parameter would be better?
> 
> I don't think so -- u-boot would then have to munge the command line to
> say that it had/had not brought up secondaries. DTB seems more natural
> to me. e.g. on ARMv8 there is already a requirement to provide a per-CPU
> property describing the bringup protocol ("PSCI" and "spintable" are the
> options there).

Then I guess we can assume that we have to do all the CPU bring up
work if this property is missing?

> Anyway, once I get to the point of being able to do something I'll
> coordinate with Marc etc and figure out what to do. In the meantime I
> think having the kernel do the bringup (like this patch does) is
> sensible. It's very likely to be what we want to do in the absence of
> any instruction to the contrary (DTB or otherwise) in the future anyway.

Yep.

A part from the discussion on the approach, do you have any comments
on the patches themselves?

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131110/875b99bf/attachment-0001.sig>

Re: [linux-sunxi] [PATCH 2/2] ARM: sun6i: Add SMP support for the Allwinner A31

[Ian Campbell]

On Sun, 2013-11-10 at 11:03 +0100, Maxime Ripard wrote:
> Hi Ian,
> 
> On Fri, Nov 08, 2013 at 10:25:55AM +0000, Ian Campbell wrote:
> > On Fri, 2013-11-08 at 09:40 +0100, Maxime Ripard wrote:
> > > > I'm trying to work out if we can make this work with the requirement
> > > > which both Xen and KVM have to enter the kernel in NS-HYP mode.
> > > > 
> > > > The way this works on e.g. vexpress is (roughly) that u-boot wakes up
> > > > the secondary CPUs from the lowlevel firmware and places them into its
> > > > own holding pen, which has the same wake up protocol as the firmware so
> > > > the kernel can just use the same code. If u-boot never gets to run on
> > > > secondary CPUs that isn't going to help much. 
> > > > 
> > > > My concern is that the sequence here appears to involve resetting the
> > > > secondary CPU, which I figure will probably defeat that strategy by
> > > > kicking the CPU back into the lowlevel firmware in the reset state,
> > > > meaning it can't be done by a u-boot only change.
> > > 
> > > I think this is where we're headed for the A20, Marc was interested in
> > > doing that,
> > 
> > Marc Zyngier is that?
> 
> Ah yes. I forgot to put it in CC...
> 
> > >  since we already have pretty much this in u-boot already,
> > > however, this is not the case for the A31.
> > 
> > > As far as I know, the Allwinner's bootloader that we currently use
> > > isn't bringing up the secondary CPUs, and we don't have any port of
> > > some sort of u-boot yet that we could work on.
> > 
> > Ah, OK. I'd assumed that A20 and A31 (indeed, most sunxi platforms) were
> > mostly equivalent as far as u-boot support went.
> 
> No. The A31 has no current support at all in u-boot(-sunxi, that is),
> so the only bootloader we can use is Allwinner's one.
> 
> It's one my TODO list somewhere, but as usual, time is lacking :)
> 
> > > So, I guess we don't really have much choice in that case, even though
> > > eventually I'd like to have this for the A31 too.
> > 
> > Right, I suppose it makes sense to consider what we want to do on the
> > A20 now and keep in mind that A31 may want to follow in the future.
> > 
> > > > Hrm, what to do ... perhaps a DT driven selection between this mechanism
> > > > and sev to kick a wfe loop reading the private register?
> > > 
> > > We can discuss this whenever we will actually have that choice to
> > > make, but maybe a kernel parameter would be better?
> > 
> > I don't think so -- u-boot would then have to munge the command line to
> > say that it had/had not brought up secondaries. DTB seems more natural
> > to me. e.g. on ARMv8 there is already a requirement to provide a per-CPU
> > property describing the bringup protocol ("PSCI" and "spintable" are the
> > options there).
> 
> Then I guess we can assume that we have to do all the CPU bring up
> work if this property is missing?

Yes, I think that is a fair enough assumption.

> > Anyway, once I get to the point of being able to do something I'll
> > coordinate with Marc etc and figure out what to do. In the meantime I
> > think having the kernel do the bringup (like this patch does) is
> > sensible. It's very likely to be what we want to do in the absence of
> > any instruction to the contrary (DTB or otherwise) in the future anyway.
> 
> Yep.
> 
> A part from the discussion on the approach, do you have any comments
> on the patches themselves?

I know approximately diddly about how one is supposed to bring up these
processors, but I did correlate what you were doing as best I could with
the A20 manual for the registers and it looked sensible to me, modulo
the fact that I was looking at the manual for a slightly different
processor ;-)

Not a terribly strong statement, sorry.

Ian.

[linux-sunxi] [PATCH 2/2] ARM: sun6i: Add SMP support for the Allwinner A31

[Ian Campbell]

On Sun, 2013-11-10 at 11:03 +0100, Maxime Ripard wrote:
> Hi Ian,
> 
> On Fri, Nov 08, 2013 at 10:25:55AM +0000, Ian Campbell wrote:
> > On Fri, 2013-11-08 at 09:40 +0100, Maxime Ripard wrote:
> > > > I'm trying to work out if we can make this work with the requirement
> > > > which both Xen and KVM have to enter the kernel in NS-HYP mode.
> > > > 
> > > > The way this works on e.g. vexpress is (roughly) that u-boot wakes up
> > > > the secondary CPUs from the lowlevel firmware and places them into its
> > > > own holding pen, which has the same wake up protocol as the firmware so
> > > > the kernel can just use the same code. If u-boot never gets to run on
> > > > secondary CPUs that isn't going to help much. 
> > > > 
> > > > My concern is that the sequence here appears to involve resetting the
> > > > secondary CPU, which I figure will probably defeat that strategy by
> > > > kicking the CPU back into the lowlevel firmware in the reset state,
> > > > meaning it can't be done by a u-boot only change.
> > > 
> > > I think this is where we're headed for the A20, Marc was interested in
> > > doing that,
> > 
> > Marc Zyngier is that?
> 
> Ah yes. I forgot to put it in CC...
> 
> > >  since we already have pretty much this in u-boot already,
> > > however, this is not the case for the A31.
> > 
> > > As far as I know, the Allwinner's bootloader that we currently use
> > > isn't bringing up the secondary CPUs, and we don't have any port of
> > > some sort of u-boot yet that we could work on.
> > 
> > Ah, OK. I'd assumed that A20 and A31 (indeed, most sunxi platforms) were
> > mostly equivalent as far as u-boot support went.
> 
> No. The A31 has no current support at all in u-boot(-sunxi, that is),
> so the only bootloader we can use is Allwinner's one.
> 
> It's one my TODO list somewhere, but as usual, time is lacking :)
> 
> > > So, I guess we don't really have much choice in that case, even though
> > > eventually I'd like to have this for the A31 too.
> > 
> > Right, I suppose it makes sense to consider what we want to do on the
> > A20 now and keep in mind that A31 may want to follow in the future.
> > 
> > > > Hrm, what to do ... perhaps a DT driven selection between this mechanism
> > > > and sev to kick a wfe loop reading the private register?
> > > 
> > > We can discuss this whenever we will actually have that choice to
> > > make, but maybe a kernel parameter would be better?
> > 
> > I don't think so -- u-boot would then have to munge the command line to
> > say that it had/had not brought up secondaries. DTB seems more natural
> > to me. e.g. on ARMv8 there is already a requirement to provide a per-CPU
> > property describing the bringup protocol ("PSCI" and "spintable" are the
> > options there).
> 
> Then I guess we can assume that we have to do all the CPU bring up
> work if this property is missing?

Yes, I think that is a fair enough assumption.

> > Anyway, once I get to the point of being able to do something I'll
> > coordinate with Marc etc and figure out what to do. In the meantime I
> > think having the kernel do the bringup (like this patch does) is
> > sensible. It's very likely to be what we want to do in the absence of
> > any instruction to the contrary (DTB or otherwise) in the future anyway.
> 
> Yep.
> 
> A part from the discussion on the approach, do you have any comments
> on the patches themselves?

I know approximately diddly about how one is supposed to bring up these
processors, but I did correlate what you were doing as best I could with
the A20 manual for the registers and it looked sensible to me, modulo
the fact that I was looking at the manual for a slightly different
processor ;-)

Not a terribly strong statement, sorry.

Ian.

Re: [linux-sunxi] [PATCH 2/2] ARM: sun6i: Add SMP support for the Allwinner A31

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1074 bytes --]

On Mon, Nov 11, 2013 at 04:48:05PM +0000, Ian Campbell wrote:
> > > Anyway, once I get to the point of being able to do something I'll
> > > coordinate with Marc etc and figure out what to do. In the meantime I
> > > think having the kernel do the bringup (like this patch does) is
> > > sensible. It's very likely to be what we want to do in the absence of
> > > any instruction to the contrary (DTB or otherwise) in the future anyway.
> > 
> > Yep.
> > 
> > A part from the discussion on the approach, do you have any comments
> > on the patches themselves?
> 
> I know approximately diddly about how one is supposed to bring up these
> processors, but I did correlate what you were doing as best I could with
> the A20 manual for the registers and it looked sensible to me, modulo
> the fact that I was looking at the manual for a slightly different
> processor ;-)
> 
> Not a terribly strong statement, sorry.

Fair enough :)

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[linux-sunxi] [PATCH 2/2] ARM: sun6i: Add SMP support for the Allwinner A31

[Maxime Ripard]

On Mon, Nov 11, 2013 at 04:48:05PM +0000, Ian Campbell wrote:
> > > Anyway, once I get to the point of being able to do something I'll
> > > coordinate with Marc etc and figure out what to do. In the meantime I
> > > think having the kernel do the bringup (like this patch does) is
> > > sensible. It's very likely to be what we want to do in the absence of
> > > any instruction to the contrary (DTB or otherwise) in the future anyway.
> > 
> > Yep.
> > 
> > A part from the discussion on the approach, do you have any comments
> > on the patches themselves?
> 
> I know approximately diddly about how one is supposed to bring up these
> processors, but I did correlate what you were doing as best I could with
> the A20 manual for the registers and it looked sensible to me, modulo
> the fact that I was looking at the manual for a slightly different
> processor ;-)
> 
> Not a terribly strong statement, sorry.

Fair enough :)

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131113/d60f51a1/attachment.sig>

Re: [PATCH 0/2] Add SMP support for the Allwinner A31 SoCs

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 400 bytes --]

On Sun, Nov 03, 2013 at 10:30:11AM +0100, Maxime Ripard wrote:
> Hi everyone,
> 
> The Allwinner A31 is a quad-Cortex-A7 SoC. This patchset adds support
> for the bringup of the others CPUs at startup.

Applied respectively to sunxi/dt-for-3.14 and sunxi/core-for-3.14.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 0/2] Add SMP support for the Allwinner A31 SoCs

[Maxime Ripard]

On Sun, Nov 03, 2013 at 10:30:11AM +0100, Maxime Ripard wrote:
> Hi everyone,
> 
> The Allwinner A31 is a quad-Cortex-A7 SoC. This patchset adds support
> for the bringup of the others CPUs at startup.

Applied respectively to sunxi/dt-for-3.14 and sunxi/core-for-3.14.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131216/e0266e0a/attachment-0001.sig>