Spectre big.Little updates

In big.Little systems, some CPUs require the Spectre workarounds in paths such as the context switch, but other CPUs do not. In order to handle these differences, we need per-CPU vtables.

We are unable to use the kernel's per-CPU variables to support this as per-CPU is not initialised at times when we need access to the vtables, so we have to use an array indexed by logical CPU number.

We use an array-of-pointers to avoid having function pointers in the kernel's read/write .data section.

Signed-off-by Russell King <[email protected]>

---
  arch/arm/include/asm/proc-fns.h | 20 ++++++++++++++++++++
  arch/arm/kernel/setup.c         |  5 +++++
  arch/arm/kernel/smp.c           | 31 +++++++++++++++++++++++++++++++
  arch/arm/mm/proc-v7-bugs.c      | 17 ++---------------
  4 files changed, 58 insertions(+), 15 deletions(-)

arch/arm/include/asm/proc-fns.h +0 -0

index c259cc49c641..fe58fb3c4b4e 100644
@@ -104,12 +104,32 @@ extern void cpu_do_resume(void *);
  #else
 

arch/arm/include/asm/proc-fns.h +11 -0

+static inline void init_proc_vtable(const struct processor *p)
+{
+	unsigned int cpu = smp_processor_id();
+	*cpu_vtable[cpu] = *p;
+	WARN_ON_ONCE(cpu_vtable[cpu]->dcache_clean_area !=
+		     cpu_vtable[0]->dcache_clean_area);
+	WARN_ON_ONCE(cpu_vtable[cpu]->set_pte_ext !=
+		     cpu_vtable[0]->set_pte_ext);
+}
+#else
  #define PROC_VTABLE(f)			processor.f
  #define PROC_TABLE(f)			processor.f
  static inline void init_proc_vtable(const struct processor *p)
  {
  	processor = *p;
  }
+#endif
 
  #define cpu_proc_init			PROC_VTABLE(_proc_init)
  #define cpu_check_bugs			PROC_VTABLE(check_bugs)

arch/arm/kernel/setup.c +5 -0

index c214bd14a1fe..cd46a595422c 100644
@@ -115,6 +115,11 @@ EXPORT_SYMBOL(elf_hwcap2);
 
  #ifdef MULTI_CPU
  struct processor processor __ro_after_init;
+#if defined(CONFIG_BIG_LITTLE) && defined(CONFIG_HARDEN_BRANCH_PREDICTOR)
+struct processor *cpu_vtable[NR_CPUS] = {
+	[0] = &processor,
+};
+#endif
  #endif
  #ifdef MULTI_TLB
  struct cpu_tlb_fns cpu_tlb __ro_after_init;

arch/arm/kernel/smp.c +31 -0

index 5ad0b67b9e33..82b879db32ee 100644
@@ -42,6 +42,7 @@
  #include <asm/mmu_context.h>
  #include <asm/pgtable.h>
  #include <asm/pgalloc.h>
+#include <asm/procinfo.h>
  #include <asm/processor.h>
  #include <asm/sections.h>
  #include <asm/tlbflush.h>

@@ -102,6 +103,30 @@ static unsigned long get_arch_pgd(pgd_t *pgd)
  #endif
  }
 
+#if defined(CONFIG_BIG_LITTLE) && defined(CONFIG_HARDEN_BRANCH_PREDICTOR)
+static int secondary_biglittle_prepare(unsigned int cpu)
+{
+	if (!cpu_vtable[cpu])
+		cpu_vtable[cpu] = kzalloc(sizeof(*cpu_vtable[cpu]), GFP_KERNEL);
+
+	return cpu_vtable[cpu] ? 0 : -ENOMEM;
+}
+
+static void secondary_biglittle_init(void)
+{
+	init_proc_vtable(lookup_processor(read_cpuid_id())->proc);
+}
+#else
+static int secondary_biglittle_prepare(unsigned int cpu)
+{
+	return 0;
+}
+
+static void secondary_biglittle_init(void)
+{
+}
+#endif
+
  int __cpu_up(unsigned int cpu, struct task_struct *idle)
  {
  	int ret;

@@ -109,6 +134,10 @@ int __cpu_up(unsigned int cpu, struct task_struct *idle)
  	if (!smp_ops.smp_boot_secondary)
  		return -ENOSYS;
 
+	ret = secondary_biglittle_prepare(cpu);
+	if (ret)
+		return ret;
+
  	/*
  	 * We need to tell the secondary core where to find
  	 * its stack and the page tables.

@@ -360,6 +389,8 @@ asmlinkage void secondary_start_kernel(void)
  	struct mm_struct *mm = &init_mm;
  	unsigned int cpu;
 
+	secondary_biglittle_init();
+
  	/*
  	 * The identity mapping is uncached (strongly ordered), so
  	 * switch away from it before attempting any exclusive accesses.

arch/arm/mm/proc-v7-bugs.c +2 -15

index 5544b82a2e7a..9a07916af8dd 100644
@@ -52,8 +52,6 @@ static void cpu_v7_spectre_init(void)
  	case ARM_CPU_PART_CORTEX_A17:
  	case ARM_CPU_PART_CORTEX_A73:
  	case ARM_CPU_PART_CORTEX_A75:
-		if (processor.switch_mm != cpu_v7_bpiall_switch_mm)
-			goto bl_error;
  		per_cpu(harden_branch_predictor_fn, cpu) =
  			harden_branch_predictor_bpiall;
  		spectre_v2_method = "BPIALL";

@@ -61,8 +59,6 @@ static void cpu_v7_spectre_init(void)
 
  	case ARM_CPU_PART_CORTEX_A15:
  	case ARM_CPU_PART_BRAHMA_B15:
-		if (processor.switch_mm != cpu_v7_iciallu_switch_mm)
-			goto bl_error;
  		per_cpu(harden_branch_predictor_fn, cpu) =
  			harden_branch_predictor_iciallu;
  		spectre_v2_method = "ICIALLU";

@@ -88,11 +84,9 @@ static void cpu_v7_spectre_init(void)
  					  ARM_SMCCC_ARCH_WORKAROUND_1, &res);
  			if ((int)res.a0 != 0)
  				break;
-			if (processor.switch_mm != cpu_v7_hvc_switch_mm && cpu)
-				goto bl_error;
  			per_cpu(harden_branch_predictor_fn, cpu) =
  				call_hvc_arch_workaround_1;
-			processor.switch_mm = cpu_v7_hvc_switch_mm;
+			cpu_do_switch_mm = cpu_v7_hvc_switch_mm;
  			spectre_v2_method = "hypervisor";
  			break;
 

@@ -101,11 +95,9 @@ static void cpu_v7_spectre_init(void)
  					  ARM_SMCCC_ARCH_WORKAROUND_1, &res);
  			if ((int)res.a0 != 0)
  				break;
-			if (processor.switch_mm != cpu_v7_smc_switch_mm && cpu)
-				goto bl_error;
  			per_cpu(harden_branch_predictor_fn, cpu) =
  				call_smc_arch_workaround_1;
-			processor.switch_mm = cpu_v7_smc_switch_mm;
+			cpu_do_switch_mm = cpu_v7_smc_switch_mm;
  			spectre_v2_method = "firmware";
  			break;
 

@@ -119,11 +111,6 @@ static void cpu_v7_spectre_init(void)
  	if (spectre_v2_method)
  		pr_info("CPU%u: Spectre v2: using %s workaround\n",
  			smp_processor_id(), spectre_v2_method);
-	return;
-
-bl_error:
-	pr_err("CPU%u: Spectre v2: incorrect context switching function, system vulnerable\n",
-		cpu);
  }
  #else
  static void cpu_v7_spectre_init(void)

In big.Little systems, some CPUs require the Spectre workarounds in paths such as the context switch, but other CPUs do not. In order to handle these differences, we need per-CPU vtables.

We are unable to use the kernel's per-CPU variables to support this as per-CPU is not initialised at times when we need access to the vtables, so we have to use an array indexed by logical CPU number.

We use an array-of-pointers to avoid having function pointers in the kernel's read/write .data section.

Signed-off-by Russell King <[email protected]>

---
 arch/arm/include/asm/proc-fns.h | 20 ++++++++++++++++++++
 arch/arm/kernel/setup.c         |  5 +++++
 arch/arm/kernel/smp.c           | 31 +++++++++++++++++++++++++++++++
 arch/arm/mm/proc-v7-bugs.c      | 17 ++---------------
 4 files changed, 58 insertions(+), 15 deletions(-)

arch/arm/include/asm/proc-fns.h +20 -0

index c259cc49c641..fe58fb3c4b4e 100644
@@ -104,12 +104,32 @@ extern void cpu_do_resume(void *);
 #else
 
 extern struct processor processor;
+#if defined(CONFIG_BIG_LITTLE) && defined(CONFIG_HARDEN_BRANCH_PREDICTOR)
+#include <linux/smp.h>
+/*
+ * This can't be a per-cpu variable because we need to access it before
+ * per-cpu has been initialised.
+ */
+extern struct processor *cpu_vtable[];
+#define PROC_VTABLE(f)			cpu_vtable[smp_processor_id()]->f
+#define PROC_TABLE(f)			cpu_vtable[0]->f
+static inline void init_proc_vtable(const struct processor *p)
+{
+	unsigned int cpu = smp_processor_id();
+	*cpu_vtable[cpu] = *p;
+	WARN_ON_ONCE(cpu_vtable[cpu]->dcache_clean_area !=
+		     cpu_vtable[0]->dcache_clean_area);
+	WARN_ON_ONCE(cpu_vtable[cpu]->set_pte_ext !=
+		     cpu_vtable[0]->set_pte_ext);
+}
+#else
 #define PROC_VTABLE(f)			processor.f
 #define PROC_TABLE(f)			processor.f
 static inline void init_proc_vtable(const struct processor *p)
 {
 	processor = *p;
 }
+#endif
 
 #define cpu_proc_init			PROC_VTABLE(_proc_init)
 #define cpu_check_bugs			PROC_VTABLE(check_bugs)

arch/arm/kernel/setup.c +5 -0

index c214bd14a1fe..cd46a595422c 100644
@@ -115,6 +115,11 @@ EXPORT_SYMBOL(elf_hwcap2);
 
 #ifdef MULTI_CPU
 struct processor processor __ro_after_init;
+#if defined(CONFIG_BIG_LITTLE) && defined(CONFIG_HARDEN_BRANCH_PREDICTOR)
+struct processor *cpu_vtable[NR_CPUS] = {
+	[0] = &processor,
+};
+#endif
 #endif
 #ifdef MULTI_TLB
 struct cpu_tlb_fns cpu_tlb __ro_after_init;

arch/arm/kernel/smp.c +31 -0

index 5ad0b67b9e33..82b879db32ee 100644
@@ -42,6 +42,7 @@
 #include <asm/mmu_context.h>
 #include <asm/pgtable.h>
 #include <asm/pgalloc.h>
+#include <asm/procinfo.h>
 #include <asm/processor.h>
 #include <asm/sections.h>
 #include <asm/tlbflush.h>

@@ -102,6 +103,30 @@ static unsigned long get_arch_pgd(pgd_t *pgd)
 #endif
 }
 
+#if defined(CONFIG_BIG_LITTLE) && defined(CONFIG_HARDEN_BRANCH_PREDICTOR)
+static int secondary_biglittle_prepare(unsigned int cpu)
+{
+	if (!cpu_vtable[cpu])
+		cpu_vtable[cpu] = kzalloc(sizeof(*cpu_vtable[cpu]), GFP_KERNEL);
+
+	return cpu_vtable[cpu] ? 0 : -ENOMEM;
+}
+
+static void secondary_biglittle_init(void)
+{
+	init_proc_vtable(lookup_processor(read_cpuid_id())->proc);
+}
+#else
+static int secondary_biglittle_prepare(unsigned int cpu)
+{
+	return 0;
+}
+
+static void secondary_biglittle_init(void)
+{
+}
+#endif
+
 int __cpu_up(unsigned int cpu, struct task_struct *idle)
 {
 	int ret;

@@ -109,6 +134,10 @@ int __cpu_up(unsigned int cpu, struct task_struct *idle)
 	if (!smp_ops.smp_boot_secondary)
 		return -ENOSYS;
 
+	ret = secondary_biglittle_prepare(cpu);
+	if (ret)
+		return ret;
+
 	/*
 	 * We need to tell the secondary core where to find
 	 * its stack and the page tables.

@@ -360,6 +389,8 @@ asmlinkage void secondary_start_kernel(void)
 	struct mm_struct *mm = &init_mm;
 	unsigned int cpu;
 
+	secondary_biglittle_init();
+
 	/*
 	 * The identity mapping is uncached (strongly ordered), so
 	 * switch away from it before attempting any exclusive accesses.

arch/arm/mm/proc-v7-bugs.c +2 -10

index 5544b82a2e7a..9a07916af8dd 100644
@@ -52,8 +52,6 @@ static void cpu_v7_spectre_init(void)
 	case ARM_CPU_PART_CORTEX_A17:
 	case ARM_CPU_PART_CORTEX_A73:
 	case ARM_CPU_PART_CORTEX_A75:
-		if (processor.switch_mm != cpu_v7_bpiall_switch_mm)
-			goto bl_error;
 		per_cpu(harden_branch_predictor_fn, cpu) =
 			harden_branch_predictor_bpiall;
 		spectre_v2_method = "BPIALL";

@@ -61,8 +59,6 @@ static void cpu_v7_spectre_init(void)
 
 	case ARM_CPU_PART_CORTEX_A15:
 	case ARM_CPU_PART_BRAHMA_B15:
-		if (processor.switch_mm != cpu_v7_iciallu_switch_mm)
-			goto bl_error;
 		per_cpu(harden_branch_predictor_fn, cpu) =
 			harden_branch_predictor_iciallu;
 		spectre_v2_method = "ICIALLU";

@@ -88,11 +84,9 @@ static void cpu_v7_spectre_init(void)
 					  ARM_SMCCC_ARCH_WORKAROUND_1, &res);
 			if ((int)res.a0 != 0)
 				break;
-			if (processor.switch_mm != cpu_v7_hvc_switch_mm && cpu)
-				goto bl_error;
 			per_cpu(harden_branch_predictor_fn, cpu) =
 				call_hvc_arch_workaround_1;
-			processor.switch_mm = cpu_v7_hvc_switch_mm;
+			cpu_do_switch_mm = cpu_v7_hvc_switch_mm;
 			spectre_v2_method = "hypervisor";
 			break;
 

@@ -101,11 +95,9 @@ static void cpu_v7_spectre_init(void)
 					  ARM_SMCCC_ARCH_WORKAROUND_1, &res);
 			if ((int)res.a0 != 0)
 				break;
-			if (processor.switch_mm != cpu_v7_smc_switch_mm && cpu)
-				goto bl_error;
 			per_cpu(harden_branch_predictor_fn, cpu) =
 				call_smc_arch_workaround_1;
-			processor.switch_mm = cpu_v7_smc_switch_mm;
+			cpu_do_switch_mm = cpu_v7_smc_switch_mm;
 			spectre_v2_method = "firmware";
 			break;
 

@@ -119,11 +111,6 @@ static void cpu_v7_spectre_init(void)
 	if (spectre_v2_method)

Hi All,

Quoted 4 lines · Russell King

In big.Little systems, some CPUs require the Spectre workarounds in paths such as the context switch, but other CPUs do not. In order to handle these differences, we need per-CPU vtables.

Quoted 8 lines

We are unable to use the kernel's per-CPU variables to support this as per-CPU is not initialised at times when we need access to the vtables, so we have to use an array indexed by logical CPU number.

We use an array-of-pointers to avoid having function pointers in the kernel's read/write .data section.

Signed-off-by Russell King <[email protected]>

Today I've noticed that this patch (which has landed in v4.20-rc3 as commit 383fb3ee8024) causes regression in CPU hotplug and suspend/resume handling on Samsung Exynos4412 SoC (4*CortexA9 SMP). After putting any non-zero CPU offline, it is no longer possible to get it online again. Same in system suspend/resume - after s2r cycle, only CPU0 is online. The regression can be observed on Hardkernel's Odroid U3 and Samsung Trats2 boards.

Any idea how to fix this issue?

Quoted 24 lines · Marek Szyprowski

Hi All,

Quoted 4 lines · Russell King

In big.Little systems, some CPUs require the Spectre workarounds in paths such as the context switch, but other CPUs do not. In order to handle these differences, we need per-CPU vtables.

Quoted 8 lines

We are unable to use the kernel's per-CPU variables to support this as per-CPU is not initialised at times when we need access to the vtables, so we have to use an array indexed by logical CPU number.

We use an array-of-pointers to avoid having function pointers in the kernel's read/write .data section.

Signed-off-by Russell King <[email protected]>

Today I've noticed that this patch (which has landed in v4.20-rc3 as commit 383fb3ee8024) causes regression in CPU hotplug and suspend/resume handling on Samsung Exynos4412 SoC (4*CortexA9 SMP). After putting any non-zero CPU offline, it is no longer possible to get it online again. Same in system suspend/resume - after s2r cycle, only CPU0 is online. The regression can be observed on Hardkernel's Odroid U3 and Samsung Trats2 boards.

Quoted 1 line

Any idea how to fix this issue?

This is getting _really_ annoying. I send the patches out, I ask for people to test, they seem to ignore the patches, and then they later report problems. This is the _second_ time that this has happened, both times with Exynos, the first time it caused the patches to miss the merge window. Given that Spectre has to deal with every kernel entry path, wouldn't it be logical to test every form of entry - boot, CPU hotplug, suspend/resume etc?

Sorry, but at this point I have little sympathy.

I've no idea based on what you've supplied given that the SoC maintainers are responsible for writing the code to deal with hotplug etc, and Exynos's code there is something of a maze. It's not clear which bits are being used. I think you at the very least need to debug to find out whether the problem is at CPU down or CPU up.

From the ARM architecture point of view, for Cortex A9, all the processor function instances should be identical. The only difference as a result of the patch is that we'll be calling smp_processor_id() early (which should be fine), and indirecting through the cpu_vtable[] array rather than merely dereferencing the processor struct.

What about checking dmesg - messages from offline CPUs do not appear on the console(s) but are still logged in the kernel log.

You could try making PROC_VTABLE() the same as PROC_TABLE() (iow, always access cpu_vtable[0]) to see whether it's the smp_processor_id() that's causing your problem or not. If it is, then try and work out which of the processor functions is causing it by restoring PROC_VTABLE() and then switching each from PROC_VTABLE() to PROC_TABLE() until it does work.

Quoted 15 lines · Russell King - ARM Linux

I've no idea based on what you've supplied given that the SoC maintainers are responsible for writing the code to deal with hotplug etc, and Exynos's code there is something of a maze. It's not clear which bits are being used. I think you at the very least need to debug to find out whether the problem is at CPU down or CPU up.

From the ARM architecture point of view, for Cortex A9, all the processor function instances should be identical. The only difference as a result of the patch is that we'll be calling smp_processor_id() early (which should be fine), and indirecting through the cpu_vtable[] array rather than merely dereferencing the processor struct.

What about checking dmesg - messages from offline CPUs do not appear on the console(s) but are still logged in the kernel log.

Quoted 6 lines

You could try making PROC_VTABLE() the same as PROC_TABLE() (iow, always access cpu_vtable[0]) to see whether it's the smp_processor_id() that's causing your problem or not. If it is, then try and work out which of the processor functions is causing it by restoring PROC_VTABLE() and then switching each from PROC_VTABLE() to PROC_TABLE() until it does work.

Thanks for hints!

Quoted 24 lines · Krzysztof Kozlowski

Quoted 21 lines · Russell King - ARM Linux

I've no idea based on what you've supplied given that the SoC maintainers are responsible for writing the code to deal with hotplug etc, and Exynos's code there is something of a maze. It's not clear which bits are being used. I think you at the very least need to debug to find out whether the problem is at CPU down or CPU up.

From the ARM architecture point of view, for Cortex A9, all the processor function instances should be identical. The only difference as a result of the patch is that we'll be calling smp_processor_id() early (which should be fine), and indirecting through the cpu_vtable[] array rather than merely dereferencing the processor struct.

What about checking dmesg - messages from offline CPUs do not appear on the console(s) but are still logged in the kernel log.

You could try making PROC_VTABLE() the same as PROC_TABLE() (iow, always access cpu_vtable[0]) to see whether it's the smp_processor_id() that's causing your problem or not. If it is, then try and work out which of the processor functions is causing it by restoring PROC_VTABLE() and then switching each from PROC_VTABLE() to PROC_TABLE() until it does work.

Quoted 1 line

Thanks for hints!

So I can plan, how long do you think it will take to get some results from my suggestions above?

Quoted 26 lines · Russell King - ARM Linux

Quoted 24 lines · Krzysztof Kozlowski

Quoted 21 lines · Russell King - ARM Linux

I've no idea based on what you've supplied given that the SoC maintainers are responsible for writing the code to deal with hotplug etc, and Exynos's code there is something of a maze. It's not clear which bits are being used. I think you at the very least need to debug to find out whether the problem is at CPU down or CPU up.

From the ARM architecture point of view, for Cortex A9, all the processor function instances should be identical. The only difference as a result of the patch is that we'll be calling smp_processor_id() early (which should be fine), and indirecting through the cpu_vtable[] array rather than merely dereferencing the processor struct.

What about checking dmesg - messages from offline CPUs do not appear on the console(s) but are still logged in the kernel log.

You could try making PROC_VTABLE() the same as PROC_TABLE() (iow, always access cpu_vtable[0]) to see whether it's the smp_processor_id() that's causing your problem or not. If it is, then try and work out which of the processor functions is causing it by restoring PROC_VTABLE() and then switching each from PROC_VTABLE() to PROC_TABLE() until it does work.

Quoted 4 lines

Quoted 1 line

Thanks for hints!

So I can plan, how long do you think it will take to get some results from my suggestions above?

For Suspend to RAM, on v4.20-rc3, this warning appears:

[   84.046722] WARNING: CPU: 1 PID: 0 at

../arch/arm/include/asm/proc-fns.h:124
secondary_start_kernel+0x214/0x26c
(difference between dcache_clean_area)
The secondary CPUs bringup fails with ETIMEDOUT.

Quoted 33 lines · Krzysztof Kozlowski

Quoted 26 lines · Russell King - ARM Linux

Quoted 24 lines · Krzysztof Kozlowski

Quoted 21 lines · Russell King - ARM Linux

I've no idea based on what you've supplied given that the SoC maintainers are responsible for writing the code to deal with hotplug etc, and Exynos's code there is something of a maze. It's not clear which bits are being used. I think you at the very least need to debug to find out whether the problem is at CPU down or CPU up.

From the ARM architecture point of view, for Cortex A9, all the processor function instances should be identical. The only difference as a result of the patch is that we'll be calling smp_processor_id() early (which should be fine), and indirecting through the cpu_vtable[] array rather than merely dereferencing the processor struct.

What about checking dmesg - messages from offline CPUs do not appear on the console(s) but are still logged in the kernel log.

You could try making PROC_VTABLE() the same as PROC_TABLE() (iow, always access cpu_vtable[0]) to see whether it's the smp_processor_id() that's causing your problem or not. If it is, then try and work out which of the processor functions is causing it by restoring PROC_VTABLE() and then switching each from PROC_VTABLE() to PROC_TABLE() until it does work.

Quoted 4 lines

Quoted 1 line

Thanks for hints!

So I can plan, how long do you think it will take to get some results from my suggestions above?

Quoted 6 lines

For Suspend to RAM, on v4.20-rc3, this warning appears:

[   84.046722] WARNING: CPU: 1 PID: 0 at

../arch/arm/include/asm/proc-fns.h:124
secondary_start_kernel+0x214/0x26c
(difference between dcache_clean_area)
The secondary CPUs bringup fails with ETIMEDOUT.

That basically means that the dcache_clean_area method for CPU1 differs from the dcache_clean_area method for CPU0. If all your CPUs are identical, that definitely should not be happening.

Hmm. Interestingly, OMAP4430 passes hotplug tests just fine.

Please try this patch.

Quoted 46 lines · Russell King - ARM Linux

Quoted 33 lines · Krzysztof Kozlowski

Quoted 26 lines · Russell King - ARM Linux

Quoted 24 lines · Krzysztof Kozlowski

Quoted 21 lines · Russell King - ARM Linux

I've no idea based on what you've supplied given that the SoC maintainers are responsible for writing the code to deal with hotplug etc, and Exynos's code there is something of a maze. It's not clear which bits are being used. I think you at the very least need to debug to find out whether the problem is at CPU down or CPU up.

From the ARM architecture point of view, for Cortex A9, all the processor function instances should be identical. The only difference as a result of the patch is that we'll be calling smp_processor_id() early (which should be fine), and indirecting through the cpu_vtable[] array rather than merely dereferencing the processor struct.

What about checking dmesg - messages from offline CPUs do not appear on the console(s) but are still logged in the kernel log.

You could try making PROC_VTABLE() the same as PROC_TABLE() (iow, always access cpu_vtable[0]) to see whether it's the smp_processor_id() that's causing your problem or not. If it is, then try and work out which of the processor functions is causing it by restoring PROC_VTABLE() and then switching each from PROC_VTABLE() to PROC_TABLE() until it does work.

Quoted 4 lines

Quoted 1 line

Thanks for hints!

So I can plan, how long do you think it will take to get some results from my suggestions above?

Quoted 6 lines

For Suspend to RAM, on v4.20-rc3, this warning appears:

[   84.046722] WARNING: CPU: 1 PID: 0 at

../arch/arm/include/asm/proc-fns.h:124
secondary_start_kernel+0x214/0x26c
(difference between dcache_clean_area)
The secondary CPUs bringup fails with ETIMEDOUT.

That basically means that the dcache_clean_area method for CPU1 differs from the dcache_clean_area method for CPU0. If all your CPUs are identical, that definitely should not be happening.

Quoted 3 lines

Hmm. Interestingly, OMAP4430 passes hotplug tests just fine.

Please try this patch.

This fixes both hotplug and suspend to RAM. I was trying to narrow why the pointers to all processor functions differ. During first boot they were OK but it seems they were changed just before suspend.

Quoted 5 lines · Krzysztof Kozlowski

On Thu, 6 Dec 2018 at 15:07, Russell King - ARM Linux

Quoted 4 lines

That basically means that the dcache_clean_area method for CPU1 differs from the dcache_clean_area method for CPU0. If all your CPUs are identical, that definitely should not be happening.

Quoted 7 lines

Quoted 3 lines

Hmm. Interestingly, OMAP4430 passes hotplug tests just fine.

Please try this patch.

This fixes both hotplug and suspend to RAM. I was trying to narrow why the pointers to all processor functions differ. During first boot they were OK but it seems they were changed just before suspend.

Thanks for testing. I think this is probably a better patch which should end up with the same result.

I suspect no one else has noticed because most people have big.Little support disabled - that'd explain why it doesn't show up on OMAP4.

arch/arm/mm/proc-macros.S +7 -0

index 81d0efb055c6..44f9776139a8 100644
@@ -274,6 +274,13 @@
        .endm
 
 .macro define_processor_functions name:req, dabort:req, pabort:req, nommu=0, suspend=0, bugs=0
+/*
+ * If we are building for big.Little with branch predictor hardening,
+ * we need the processor function tables to remain available after boot.
+ */
+#if defined(CONFIG_BIG_LITTLE) && defined(CONFIG_HARDEN_BRANCH_PREDICTOR)
+       .rodata
+#endif
        .type   \name\()_processor_functions, #object
        .align 2
 ENTRY(\name\()_processor_functions)

Quoted 48 lines · Russell King - ARM Linux

Quoted 5 lines · Krzysztof Kozlowski

On Thu, 6 Dec 2018 at 15:07, Russell King - ARM Linux

Quoted 4 lines

That basically means that the dcache_clean_area method for CPU1 differs from the dcache_clean_area method for CPU0. If all your CPUs are identical, that definitely should not be happening.

Quoted 7 lines

Quoted 3 lines

Hmm. Interestingly, OMAP4430 passes hotplug tests just fine.

Please try this patch.

This fixes both hotplug and suspend to RAM. I was trying to narrow why the pointers to all processor functions differ. During first boot they were OK but it seems they were changed just before suspend.

Thanks for testing. I think this is probably a better patch which should end up with the same result.

I suspect no one else has noticed because most people have big.Little support disabled - that'd explain why it doesn't show up on OMAP4.

arch/arm/mm/proc-macros.S +10 -0

index 81d0efb055c6..44f9776139a8 100644
@@ -274,6 +274,13 @@
        .endm
 
 .macro define_processor_functions name:req, dabort:req, pabort:req, nommu=0, suspend=0, bugs=0
+/*
+ * If we are building for big.Little with branch predictor hardening,
+ * we need the processor function tables to remain available after boot.
+ */
+#if defined(CONFIG_BIG_LITTLE) && defined(CONFIG_HARDEN_BRANCH_PREDICTOR)
+       .rodata
+#endif
        .type   \name\()_processor_functions, #object
        .align 2
 ENTRY(\name\()_processor_functions)

@@ -309,6 +316,9 @@ ENTRY(\name\()_processor_functions)
        .endif
 
        .size   \name\()_processor_functions, . - \name\()_processor_functions
+#if defined(CONFIG_BIG_LITTLE) && defined(CONFIG_HARDEN_BRANCH_PREDICTOR)
+       .previous
+#endif
 .endm
 

arch/arm/mm/proc-macros.S ENTRY(\name\()_processor_functions) · line 324

Quoted 1 line

arch/arm/mm/proc-macros.S +0 -0

 .macro define_cache_functions name:req

Does not compile:
../arch/arm/mm/proc-v7.S: Assembler messages:
../arch/arm/mm/proc-v7.S:560: Error: unknown pseudo-op: .rodata' ../arch/arm/mm/proc-v7.S:575: Error: unknown pseudo-op: .rodata' ../arch/arm/mm/proc-v7.S:596: Error: unknown pseudo-op: .rodata' ../arch/arm/mm/proc-v7.S:611: Error: unknown pseudo-op: .rodata' ../arch/arm/mm/proc-v7.S:629: Error: unknown pseudo-op: `.rodata'

Quoted 47 lines · Krzysztof Kozlowski

Quoted 45 lines · Russell King - ARM Linux

Quoted 11 lines · Krzysztof Kozlowski

On Thu, 6 Dec 2018 at 15:07, Russell King - ARM Linux

Quoted 7 lines

That basically means that the dcache_clean_area method for CPU1 differs from the dcache_clean_area method for CPU0. If all your CPUs are identical, that definitely should not be happening.

Hmm. Interestingly, OMAP4430 passes hotplug tests just fine.

Please try this patch.

This fixes both hotplug and suspend to RAM. I was trying to narrow why the pointers to all processor functions differ. During first boot they were OK but it seems they were changed just before suspend.

Thanks for testing. I think this is probably a better patch which should end up with the same result.

I suspect no one else has noticed because most people have big.Little support disabled - that'd explain why it doesn't show up on OMAP4.

arch/arm/mm/proc-macros.S +10 -0

index 81d0efb055c6..44f9776139a8 100644
@@ -274,6 +274,13 @@
        .endm
 
 .macro define_processor_functions name:req, dabort:req, pabort:req, nommu=0, suspend=0, bugs=0
+/*
+ * If we are building for big.Little with branch predictor hardening,
+ * we need the processor function tables to remain available after boot.
+ */
+#if defined(CONFIG_BIG_LITTLE) && defined(CONFIG_HARDEN_BRANCH_PREDICTOR)
+       .rodata
+#endif
        .type   \name\()_processor_functions, #object
        .align 2
 ENTRY(\name\()_processor_functions)

@@ -309,6 +316,9 @@ ENTRY(\name\()_processor_functions)
        .endif
 
        .size   \name\()_processor_functions, . - \name\()_processor_functions
+#if defined(CONFIG_BIG_LITTLE) && defined(CONFIG_HARDEN_BRANCH_PREDICTOR)
+       .previous
+#endif
 .endm
 

Quoted 8 lines

arch/arm/mm/proc-macros.S ENTRY(\name\()_processor_functions) · line 324

Quoted 1 line

arch/arm/mm/proc-macros.S +0 -0

 .macro define_cache_functions name:req

Does not compile:
../arch/arm/mm/proc-v7.S: Assembler messages:
../arch/arm/mm/proc-v7.S:560: Error: unknown pseudo-op: .rodata' ../arch/arm/mm/proc-v7.S:575: Error: unknown pseudo-op: .rodata' ../arch/arm/mm/proc-v7.S:596: Error: unknown pseudo-op: .rodata' ../arch/arm/mm/proc-v7.S:611: Error: unknown pseudo-op: .rodata' ../arch/arm/mm/proc-v7.S:629: Error: unknown pseudo-op: `.rodata'

It should be .section ".rodata", sorry.

Quoted 50 lines · Krzysztof Kozlowski

Quoted 48 lines · Russell King - ARM Linux

Quoted 5 lines · Krzysztof Kozlowski

On Thu, 6 Dec 2018 at 15:07, Russell King - ARM Linux

Quoted 4 lines

That basically means that the dcache_clean_area method for CPU1 differs from the dcache_clean_area method for CPU0. If all your CPUs are identical, that definitely should not be happening.

Quoted 7 lines

Quoted 3 lines

Hmm. Interestingly, OMAP4430 passes hotplug tests just fine.

Please try this patch.

This fixes both hotplug and suspend to RAM. I was trying to narrow why the pointers to all processor functions differ. During first boot they were OK but it seems they were changed just before suspend.

Thanks for testing. I think this is probably a better patch which should end up with the same result.

I suspect no one else has noticed because most people have big.Little support disabled - that'd explain why it doesn't show up on OMAP4.

arch/arm/mm/proc-macros.S +10 -0

index 81d0efb055c6..44f9776139a8 100644
@@ -274,6 +274,13 @@
        .endm
 
 .macro define_processor_functions name:req, dabort:req, pabort:req, nommu=0, suspend=0, bugs=0
+/*
+ * If we are building for big.Little with branch predictor hardening,
+ * we need the processor function tables to remain available after boot.
+ */
+#if defined(CONFIG_BIG_LITTLE) && defined(CONFIG_HARDEN_BRANCH_PREDICTOR)
+       .rodata
+#endif
        .type   \name\()_processor_functions, #object
        .align 2
 ENTRY(\name\()_processor_functions)

@@ -309,6 +316,9 @@ ENTRY(\name\()_processor_functions)
        .endif
 
        .size   \name\()_processor_functions, . - \name\()_processor_functions
+#if defined(CONFIG_BIG_LITTLE) && defined(CONFIG_HARDEN_BRANCH_PREDICTOR)
+       .previous
+#endif
 .endm
 

Quoted 9 lines

arch/arm/mm/proc-macros.S ENTRY(\name\()_processor_functions) · line 324

Quoted 1 line

arch/arm/mm/proc-macros.S +0 -0

 .macro define_cache_functions name:req

Does not compile:
../arch/arm/mm/proc-v7.S: Assembler messages:
../arch/arm/mm/proc-v7.S:560: Error: unknown pseudo-op: .rodata' ../arch/arm/mm/proc-v7.S:575: Error: unknown pseudo-op: .rodata' ../arch/arm/mm/proc-v7.S:596: Error: unknown pseudo-op: .rodata' ../arch/arm/mm/proc-v7.S:611: Error: unknown pseudo-op: .rodata' ../arch/arm/mm/proc-v7.S:629: Error: unknown pseudo-op: `.rodata'

It should be .section ".rodata", sorry.

Quoted 46 lines · Russell King - ARM Linux

Quoted 44 lines · Krzysztof Kozlowski

Quoted 42 lines · Russell King - ARM Linux

Quoted 5 lines · Krzysztof Kozlowski

On Thu, 6 Dec 2018 at 15:07, Russell King - ARM Linux

Quoted 4 lines

That basically means that the dcache_clean_area method for CPU1 differs from the dcache_clean_area method for CPU0. If all your CPUs are identical, that definitely should not be happening.

Quoted 7 lines

Quoted 3 lines

Hmm. Interestingly, OMAP4430 passes hotplug tests just fine.

Please try this patch.

This fixes both hotplug and suspend to RAM. I was trying to narrow why the pointers to all processor functions differ. During first boot they were OK but it seems they were changed just before suspend.

Thanks for testing. I think this is probably a better patch which should end up with the same result.

I suspect no one else has noticed because most people have big.Little support disabled - that'd explain why it doesn't show up on OMAP4.

arch/arm/mm/proc-macros.S +7 -0

index 81d0efb055c6..44f9776139a8 100644
@@ -274,6 +274,13 @@
        .endm
 
 .macro define_processor_functions name:req, dabort:req, pabort:req, nommu=0, suspend=0, bugs=0
+/*
+ * If we are building for big.Little with branch predictor hardening,
+ * we need the processor function tables to remain available after boot.
+ */
+#if defined(CONFIG_BIG_LITTLE) && defined(CONFIG_HARDEN_BRANCH_PREDICTOR)
+       .rodata
+#endif
        .type   \name\()_processor_functions, #object
        .align 2
 ENTRY(\name\()_processor_functions)

@@ -309,6 +316,9 @@ ENTRY(\name\()_processor_functions)
        .endif
 

Quoted 7 lines

Quoted 7 lines

arch/arm/mm/proc-macros.S ENTRY(\name\()_processor_functions) · line 324

Quoted 7 lines

arch/arm/mm/proc-macros.S +3 -0

        .size   \name\()_processor_functions, . - \name\()_processor_functions
+#if defined(CONFIG_BIG_LITTLE) && defined(CONFIG_HARDEN_BRANCH_PREDICTOR)
+       .previous
+#endif
 .endm
 
 .macro define_cache_functions name:req

Russell,

I noticed that the patch merged by Linus has

#if 1 // defined(CONFIG_BIG_LITTLE) && defined(CONFIG_HARDEN_BRANCH_PREDICTOR)

Was that what you intended?