[PATCH 0/5] Spectre big.Little updates

[PATCH 0/5] Spectre big.Little updates

[Russell King - ARM Linux]

This series of patches allows Spectre to be worked around on
big.Little systems, and as far as I'm aware, completes the Spectre
workarounds as were known at the beginning of the year for 32-bit
ARM.

In order to solve Spectre for such systems, we need to introduce
per-CPU indirection of the "processor" vtable, but we are unable to
use the kernel's per-CPU infrastructure as this is not initialised
early enough.

To work around that, we fall back to using a static array of
processor vtables indexed by the logical CPU number.  This should
not be a problem as 32-bit ARM systems are limited to 8 logical
CPUs.

 arch/arm/include/asm/cputype.h  |  2 ++
 arch/arm/include/asm/proc-fns.h | 49 ++++++++++++++++++++++++++++++++---------
 arch/arm/kernel/bugs.c          |  4 ++--
 arch/arm/kernel/head-common.S   |  6 ++---
 arch/arm/kernel/setup.c         | 40 ++++++++++++++++++++-------------
 arch/arm/kernel/smp.c           | 31 ++++++++++++++++++++++++++
 arch/arm/mm/proc-v7-bugs.c      | 17 ++------------
 7 files changed, 103 insertions(+), 46 deletions(-)

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 13.8Mbps down 630kbps up
According to speedtest.net: 13Mbps down 490kbps up

[PATCH 1/5] ARM: make lookup_processor_type() non-__init

[Russell King]

Move lookup_processor_type() out of the __init section so it is callable
from (eg) the secondary startup code during hotplug.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 arch/arm/kernel/head-common.S | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/kernel/head-common.S b/arch/arm/kernel/head-common.S
index 6e0375e7db05..997b02302c31 100644
--- a/arch/arm/kernel/head-common.S
+++ b/arch/arm/kernel/head-common.S
@@ -145,6 +145,9 @@ ENDPROC(__mmap_switched)
 #endif
 	.size	__mmap_switched_data, . - __mmap_switched_data
 
+	__FINIT
+	.text
+
 /*
  * This provides a C-API version of __lookup_processor_type
  */
@@ -156,9 +159,6 @@ ENTRY(lookup_processor_type)
 	ldmfd	sp!, {r4 - r6, r9, pc}
 ENDPROC(lookup_processor_type)
 
-	__FINIT
-	.text
-
 /*
  * Read processor ID register (CP#15, CR0), and look up in the linker-built
  * supported processor list.  Note that we can't use the absolute addresses
-- 
2.7.4

[PATCH 2/5] ARM: split out processor lookup

[Russell King]

Split out the lookup of the processor type and associated error handling
from the rest of setup_processor() - we will need to use this in the
secondary CPU bringup path for big.Little Spectre variant 2 mitigation.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 arch/arm/include/asm/cputype.h |  1 +
 arch/arm/kernel/setup.c        | 31 +++++++++++++++++++------------
 2 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/arch/arm/include/asm/cputype.h b/arch/arm/include/asm/cputype.h
index 26021980504d..f6df4bb4e543 100644
--- a/arch/arm/include/asm/cputype.h
+++ b/arch/arm/include/asm/cputype.h
@@ -107,6 +107,7 @@
 #define ARM_CPU_PART_SCORPION		0x510002d0
 
 extern unsigned int processor_id;
+struct proc_info_list *lookup_processor(u32 midr);
 
 #ifdef CONFIG_CPU_CP15
 #define read_cpuid(reg)							\
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index fc40a2b40595..05a4eb6b0d01 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -667,22 +667,29 @@ static void __init smp_build_mpidr_hash(void)
 }
 #endif
 
-static void __init setup_processor(void)
+/*
+ * locate processor in the list of supported processor types.  The linker
+ * builds this table for us from the entries in arch/arm/mm/proc-*.S
+ */
+struct proc_info_list *lookup_processor(u32 midr)
 {
-	struct proc_info_list *list;
+	struct proc_info_list *list = lookup_processor_type(midr);
 
-	/*
-	 * locate processor in the list of supported processor
-	 * types.  The linker builds this table for us from the
-	 * entries in arch/arm/mm/proc-*.S
-	 */
-	list = lookup_processor_type(read_cpuid_id());
 	if (!list) {
-		pr_err("CPU configuration botched (ID %08x), unable to continue.\n",
-		       read_cpuid_id());
-		while (1);
+		pr_err("CPU%u: configuration botched (ID %08x), CPU halted\n",
+		       smp_processor_id(), midr);
+		while (1)
+		/* can't use cpu_relax() here as it may require MMU setup */;
 	}
 
+	return list;
+}
+
+static void __init setup_processor(void)
+{
+	unsigned int midr = read_cpuid_id();
+	struct proc_info_list *list = lookup_processor(midr);
+
 	cpu_name = list->cpu_name;
 	__cpu_architecture = __get_cpu_architecture();
 
@@ -700,7 +707,7 @@ static void __init setup_processor(void)
 #endif
 
 	pr_info("CPU: %s [%08x] revision %d (ARMv%s), cr=%08lx\n",
-		cpu_name, read_cpuid_id(), read_cpuid_id() & 15,
+		list->cpu_name, midr, midr & 15,
 		proc_arch[cpu_architecture()], get_cr());
 
 	snprintf(init_utsname()->machine, __NEW_UTS_LEN + 1, "%s%c",
-- 
2.7.4

[PATCH 3/5] ARM: clean up per-processor check_bugs method call

[Russell King]

Call the per-processor type check_bugs() method in the same way as we
do other per-processor functions - move the "processor." detail into
proc-fns.h.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 arch/arm/include/asm/proc-fns.h | 1 +
 arch/arm/kernel/bugs.c          | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/proc-fns.h b/arch/arm/include/asm/proc-fns.h
index e25f4392e1b2..30c499146320 100644
--- a/arch/arm/include/asm/proc-fns.h
+++ b/arch/arm/include/asm/proc-fns.h
@@ -99,6 +99,7 @@ extern void cpu_do_suspend(void *);
 extern void cpu_do_resume(void *);
 #else
 #define cpu_proc_init			processor._proc_init
+#define cpu_check_bugs			processor.check_bugs
 #define cpu_proc_fin			processor._proc_fin
 #define cpu_reset			processor.reset
 #define cpu_do_idle			processor._do_idle
diff --git a/arch/arm/kernel/bugs.c b/arch/arm/kernel/bugs.c
index 7be511310191..d41d3598e5e5 100644
--- a/arch/arm/kernel/bugs.c
+++ b/arch/arm/kernel/bugs.c
@@ -6,8 +6,8 @@
 void check_other_bugs(void)
 {
 #ifdef MULTI_CPU
-	if (processor.check_bugs)
-		processor.check_bugs();
+	if (cpu_check_bugs)
+		cpu_check_bugs();
 #endif
 }
 
-- 
2.7.4

[PATCH 4/5] ARM: add PROC_VTABLE macro

[Russell King]

Allow the way we access members of the processor vtable to be changed
at compile time.  We will need to move to per-CPU vtables to fix the
Spectre variant 2 issues on big.Little systems.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 arch/arm/include/asm/proc-fns.h | 36 ++++++++++++++++++++++++------------
 arch/arm/kernel/setup.c         |  4 +---
 2 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/arch/arm/include/asm/proc-fns.h b/arch/arm/include/asm/proc-fns.h
index 30c499146320..571a1346245b 100644
--- a/arch/arm/include/asm/proc-fns.h
+++ b/arch/arm/include/asm/proc-fns.h
@@ -23,7 +23,7 @@ struct mm_struct;
 /*
  * Don't change this structure - ASM code relies on it.
  */
-extern struct processor {
+struct processor {
 	/* MISC
 	 * get data abort address/flags
 	 */
@@ -79,9 +79,13 @@ extern struct processor {
 	unsigned int suspend_size;
 	void (*do_suspend)(void *);
 	void (*do_resume)(void *);
-} processor;
+};
 
 #ifndef MULTI_CPU
+static inline void init_proc_vtable(const struct processor *p)
+{
+}
+
 extern void cpu_proc_init(void);
 extern void cpu_proc_fin(void);
 extern int cpu_do_idle(void);
@@ -98,18 +102,26 @@ extern void cpu_reset(unsigned long addr, bool hvc) __attribute__((noreturn));
 extern void cpu_do_suspend(void *);
 extern void cpu_do_resume(void *);
 #else
-#define cpu_proc_init			processor._proc_init
-#define cpu_check_bugs			processor.check_bugs
-#define cpu_proc_fin			processor._proc_fin
-#define cpu_reset			processor.reset
-#define cpu_do_idle			processor._do_idle
-#define cpu_dcache_clean_area		processor.dcache_clean_area
-#define cpu_set_pte_ext			processor.set_pte_ext
-#define cpu_do_switch_mm		processor.switch_mm
+
+extern struct processor processor;
+#define PROC_VTABLE(f)			processor.f
+static inline void init_proc_vtable(const struct processor *p)
+{
+	processor = *p;
+}
+
+#define cpu_proc_init			PROC_VTABLE(_proc_init)
+#define cpu_check_bugs			PROC_VTABLE(check_bugs)
+#define cpu_proc_fin			PROC_VTABLE(_proc_fin)
+#define cpu_reset			PROC_VTABLE(reset)
+#define cpu_do_idle			PROC_VTABLE(_do_idle)
+#define cpu_dcache_clean_area		PROC_VTABLE(dcache_clean_area)
+#define cpu_set_pte_ext			PROC_VTABLE(set_pte_ext)
+#define cpu_do_switch_mm		PROC_VTABLE(switch_mm)
 
 /* These three are private to arch/arm/kernel/suspend.c */
-#define cpu_do_suspend			processor.do_suspend
-#define cpu_do_resume			processor.do_resume
+#define cpu_do_suspend			PROC_VTABLE(do_suspend)
+#define cpu_do_resume			PROC_VTABLE(do_resume)
 #endif
 
 extern void cpu_resume(void);
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 05a4eb6b0d01..c214bd14a1fe 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -693,9 +693,7 @@ static void __init setup_processor(void)
 	cpu_name = list->cpu_name;
 	__cpu_architecture = __get_cpu_architecture();
 
-#ifdef MULTI_CPU
-	processor = *list->proc;
-#endif
+	init_proc_vtable(list->proc);
 #ifdef MULTI_TLB
 	cpu_tlb = *list->tlb;
 #endif
-- 
2.7.4

[PATCH 5/5] ARM: spectre-v2: per-CPU vtables to work around big.Little systems

[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.

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.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 arch/arm/include/asm/proc-fns.h | 14 ++++++++++++++
 arch/arm/kernel/setup.c         |  5 +++++
 arch/arm/kernel/smp.c           | 31 +++++++++++++++++++++++++++++++
 arch/arm/mm/proc-v7-bugs.c      | 17 ++---------------
 4 files changed, 52 insertions(+), 15 deletions(-)

diff --git a/arch/arm/include/asm/proc-fns.h b/arch/arm/include/asm/proc-fns.h
index 571a1346245b..ddbb25cb8968 100644
--- a/arch/arm/include/asm/proc-fns.h
+++ b/arch/arm/include/asm/proc-fns.h
@@ -104,11 +104,25 @@ 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
+static inline void init_proc_vtable(const struct processor *p)
+{
+	*cpu_vtable[smp_processor_id()] = *p;
+}
+#else
 #define PROC_VTABLE(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)
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index c214bd14a1fe..cd46a595422c 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -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;
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index 5ad0b67b9e33..82b879db32ee 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -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.
diff --git a/arch/arm/mm/proc-v7-bugs.c b/arch/arm/mm/proc-v7-bugs.c
index 5544b82a2e7a..9a07916af8dd 100644
--- a/arch/arm/mm/proc-v7-bugs.c
+++ b/arch/arm/mm/proc-v7-bugs.c
@@ -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)
-- 
2.7.4

[PATCH 5/5] ARM: spectre-v2: per-CPU vtables to work around big.Little systems

[Julien Thierry]

Hi Russell,

On 19/09/18 10:49, Russell King wrote:
> 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.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>   arch/arm/include/asm/proc-fns.h | 14 ++++++++++++++
>   arch/arm/kernel/setup.c         |  5 +++++
>   arch/arm/kernel/smp.c           | 31 +++++++++++++++++++++++++++++++
>   arch/arm/mm/proc-v7-bugs.c      | 17 ++---------------
>   4 files changed, 52 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/arm/include/asm/proc-fns.h b/arch/arm/include/asm/proc-fns.h
> index 571a1346245b..ddbb25cb8968 100644
> --- a/arch/arm/include/asm/proc-fns.h
> +++ b/arch/arm/include/asm/proc-fns.h
> @@ -104,11 +104,25 @@ 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
> +static inline void init_proc_vtable(const struct processor *p)
> +{
> +	*cpu_vtable[smp_processor_id()] = *p;
> +}

As you stated, 32-bit ARM can only have up to 8 logical CPUs. So, do we 
save much by having an added level of indirection? (i.e. having a table 
of pointers instead of directly having a table of struct processor)

Otherwise the patch looks good to me.

Thanks,

> +#else
>   #define PROC_VTABLE(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)
> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> index c214bd14a1fe..cd46a595422c 100644
> --- a/arch/arm/kernel/setup.c
> +++ b/arch/arm/kernel/setup.c
> @@ -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;
> diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
> index 5ad0b67b9e33..82b879db32ee 100644
> --- a/arch/arm/kernel/smp.c
> +++ b/arch/arm/kernel/smp.c
> @@ -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.
> diff --git a/arch/arm/mm/proc-v7-bugs.c b/arch/arm/mm/proc-v7-bugs.c
> index 5544b82a2e7a..9a07916af8dd 100644
> --- a/arch/arm/mm/proc-v7-bugs.c
> +++ b/arch/arm/mm/proc-v7-bugs.c
> @@ -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)
> 

-- 
Julien Thierry

[PATCH 2/5] ARM: split out processor lookup

[Julien Thierry]

Hi Russell,

On 19/09/18 10:48, Russell King wrote:
> Split out the lookup of the processor type and associated error handling
> from the rest of setup_processor() - we will need to use this in the
> secondary CPU bringup path for big.Little Spectre variant 2 mitigation.
> 

Reviewed-by: Julien Thierry <julien.thierry@arm.com>

> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>   arch/arm/include/asm/cputype.h |  1 +
>   arch/arm/kernel/setup.c        | 31 +++++++++++++++++++------------
>   2 files changed, 20 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/arm/include/asm/cputype.h b/arch/arm/include/asm/cputype.h
> index 26021980504d..f6df4bb4e543 100644
> --- a/arch/arm/include/asm/cputype.h
> +++ b/arch/arm/include/asm/cputype.h
> @@ -107,6 +107,7 @@
>   #define ARM_CPU_PART_SCORPION		0x510002d0
>   
>   extern unsigned int processor_id;
> +struct proc_info_list *lookup_processor(u32 midr);
>   
>   #ifdef CONFIG_CPU_CP15
>   #define read_cpuid(reg)							\
> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> index fc40a2b40595..05a4eb6b0d01 100644
> --- a/arch/arm/kernel/setup.c
> +++ b/arch/arm/kernel/setup.c
> @@ -667,22 +667,29 @@ static void __init smp_build_mpidr_hash(void)
>   }
>   #endif
>   
> -static void __init setup_processor(void)
> +/*
> + * locate processor in the list of supported processor types.  The linker
> + * builds this table for us from the entries in arch/arm/mm/proc-*.S
> + */
> +struct proc_info_list *lookup_processor(u32 midr)
>   {
> -	struct proc_info_list *list;
> +	struct proc_info_list *list = lookup_processor_type(midr);
>   
> -	/*
> -	 * locate processor in the list of supported processor
> -	 * types.  The linker builds this table for us from the
> -	 * entries in arch/arm/mm/proc-*.S
> -	 */
> -	list = lookup_processor_type(read_cpuid_id());
>   	if (!list) {
> -		pr_err("CPU configuration botched (ID %08x), unable to continue.\n",
> -		       read_cpuid_id());
> -		while (1);
> +		pr_err("CPU%u: configuration botched (ID %08x), CPU halted\n",
> +		       smp_processor_id(), midr);
> +		while (1)
> +		/* can't use cpu_relax() here as it may require MMU setup */;
>   	}
>   
> +	return list;
> +}
> +
> +static void __init setup_processor(void)
> +{
> +	unsigned int midr = read_cpuid_id();
> +	struct proc_info_list *list = lookup_processor(midr);
> +
>   	cpu_name = list->cpu_name;
>   	__cpu_architecture = __get_cpu_architecture();
>   
> @@ -700,7 +707,7 @@ static void __init setup_processor(void)
>   #endif
>   
>   	pr_info("CPU: %s [%08x] revision %d (ARMv%s), cr=%08lx\n",
> -		cpu_name, read_cpuid_id(), read_cpuid_id() & 15,
> +		list->cpu_name, midr, midr & 15,
>   		proc_arch[cpu_architecture()], get_cr());
>   
>   	snprintf(init_utsname()->machine, __NEW_UTS_LEN + 1, "%s%c",
> 

-- 
Julien Thierry

[PATCH 3/5] ARM: clean up per-processor check_bugs method call

[Julien Thierry]

Hi Russell,

On 19/09/18 10:49, Russell King wrote:
> Call the per-processor type check_bugs() method in the same way as we
> do other per-processor functions - move the "processor." detail into
> proc-fns.h.
> 

Reviewed-by: Julien Thierry <julien.thierry@arm.com>

> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>   arch/arm/include/asm/proc-fns.h | 1 +
>   arch/arm/kernel/bugs.c          | 4 ++--
>   2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/include/asm/proc-fns.h b/arch/arm/include/asm/proc-fns.h
> index e25f4392e1b2..30c499146320 100644
> --- a/arch/arm/include/asm/proc-fns.h
> +++ b/arch/arm/include/asm/proc-fns.h
> @@ -99,6 +99,7 @@ extern void cpu_do_suspend(void *);
>   extern void cpu_do_resume(void *);
>   #else
>   #define cpu_proc_init			processor._proc_init
> +#define cpu_check_bugs			processor.check_bugs
>   #define cpu_proc_fin			processor._proc_fin
>   #define cpu_reset			processor.reset
>   #define cpu_do_idle			processor._do_idle
> diff --git a/arch/arm/kernel/bugs.c b/arch/arm/kernel/bugs.c
> index 7be511310191..d41d3598e5e5 100644
> --- a/arch/arm/kernel/bugs.c
> +++ b/arch/arm/kernel/bugs.c
> @@ -6,8 +6,8 @@
>   void check_other_bugs(void)
>   {
>   #ifdef MULTI_CPU
> -	if (processor.check_bugs)
> -		processor.check_bugs();
> +	if (cpu_check_bugs)
> +		cpu_check_bugs();
>   #endif
>   }
>   
> 

-- 
Julien Thierry

[PATCH 1/5] ARM: make lookup_processor_type() non-__init

[Julien Thierry]

Hi Russell,

On 19/09/18 10:48, Russell King wrote:
> Move lookup_processor_type() out of the __init section so it is callable
> from (eg) the secondary startup code during hotplug.
> 

Reviewed-by: Julien Thierry <julien.thierry@arm.com>

> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>   arch/arm/kernel/head-common.S | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/kernel/head-common.S b/arch/arm/kernel/head-common.S
> index 6e0375e7db05..997b02302c31 100644
> --- a/arch/arm/kernel/head-common.S
> +++ b/arch/arm/kernel/head-common.S
> @@ -145,6 +145,9 @@ ENDPROC(__mmap_switched)
>   #endif
>   	.size	__mmap_switched_data, . - __mmap_switched_data
>   
> +	__FINIT
> +	.text
> +
>   /*
>    * This provides a C-API version of __lookup_processor_type
>    */
> @@ -156,9 +159,6 @@ ENTRY(lookup_processor_type)
>   	ldmfd	sp!, {r4 - r6, r9, pc}
>   ENDPROC(lookup_processor_type)
>   
> -	__FINIT
> -	.text
> -
>   /*
>    * Read processor ID register (CP#15, CR0), and look up in the linker-built
>    * supported processor list.  Note that we can't use the absolute addresses
> 

-- 
Julien Thierry

[PATCH 4/5] ARM: add PROC_VTABLE macro

[Julien Thierry]

Hi Russell,

On 19/09/18 10:49, Russell King wrote:
> Allow the way we access members of the processor vtable to be changed
> at compile time.  We will need to move to per-CPU vtables to fix the
> Spectre variant 2 issues on big.Little systems.
> 

Reviewed-by: Julien Thierry <julien.thierry@arm.com>

> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>   arch/arm/include/asm/proc-fns.h | 36 ++++++++++++++++++++++++------------
>   arch/arm/kernel/setup.c         |  4 +---
>   2 files changed, 25 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/arm/include/asm/proc-fns.h b/arch/arm/include/asm/proc-fns.h
> index 30c499146320..571a1346245b 100644
> --- a/arch/arm/include/asm/proc-fns.h
> +++ b/arch/arm/include/asm/proc-fns.h
> @@ -23,7 +23,7 @@ struct mm_struct;
>   /*
>    * Don't change this structure - ASM code relies on it.
>    */
> -extern struct processor {
> +struct processor {
>   	/* MISC
>   	 * get data abort address/flags
>   	 */
> @@ -79,9 +79,13 @@ extern struct processor {
>   	unsigned int suspend_size;
>   	void (*do_suspend)(void *);
>   	void (*do_resume)(void *);
> -} processor;
> +};
>   
>   #ifndef MULTI_CPU
> +static inline void init_proc_vtable(const struct processor *p)
> +{
> +}
> +
>   extern void cpu_proc_init(void);
>   extern void cpu_proc_fin(void);
>   extern int cpu_do_idle(void);
> @@ -98,18 +102,26 @@ extern void cpu_reset(unsigned long addr, bool hvc) __attribute__((noreturn));
>   extern void cpu_do_suspend(void *);
>   extern void cpu_do_resume(void *);
>   #else
> -#define cpu_proc_init			processor._proc_init
> -#define cpu_check_bugs			processor.check_bugs
> -#define cpu_proc_fin			processor._proc_fin
> -#define cpu_reset			processor.reset
> -#define cpu_do_idle			processor._do_idle
> -#define cpu_dcache_clean_area		processor.dcache_clean_area
> -#define cpu_set_pte_ext			processor.set_pte_ext
> -#define cpu_do_switch_mm		processor.switch_mm
> +
> +extern struct processor processor;
> +#define PROC_VTABLE(f)			processor.f
> +static inline void init_proc_vtable(const struct processor *p)
> +{
> +	processor = *p;
> +}
> +
> +#define cpu_proc_init			PROC_VTABLE(_proc_init)
> +#define cpu_check_bugs			PROC_VTABLE(check_bugs)
> +#define cpu_proc_fin			PROC_VTABLE(_proc_fin)
> +#define cpu_reset			PROC_VTABLE(reset)
> +#define cpu_do_idle			PROC_VTABLE(_do_idle)
> +#define cpu_dcache_clean_area		PROC_VTABLE(dcache_clean_area)
> +#define cpu_set_pte_ext			PROC_VTABLE(set_pte_ext)
> +#define cpu_do_switch_mm		PROC_VTABLE(switch_mm)
>   
>   /* These three are private to arch/arm/kernel/suspend.c */
> -#define cpu_do_suspend			processor.do_suspend
> -#define cpu_do_resume			processor.do_resume
> +#define cpu_do_suspend			PROC_VTABLE(do_suspend)
> +#define cpu_do_resume			PROC_VTABLE(do_resume)
>   #endif
>   
>   extern void cpu_resume(void);
> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> index 05a4eb6b0d01..c214bd14a1fe 100644
> --- a/arch/arm/kernel/setup.c
> +++ b/arch/arm/kernel/setup.c
> @@ -693,9 +693,7 @@ static void __init setup_processor(void)
>   	cpu_name = list->cpu_name;
>   	__cpu_architecture = __get_cpu_architecture();
>   
> -#ifdef MULTI_CPU
> -	processor = *list->proc;
> -#endif
> +	init_proc_vtable(list->proc);
>   #ifdef MULTI_TLB
>   	cpu_tlb = *list->tlb;
>   #endif
> 

-- 
Julien Thierry

[PATCH 5/5] ARM: spectre-v2: per-CPU vtables to work around big.Little systems

[Russell King - ARM Linux]

On Thu, Sep 20, 2018 at 10:04:44AM +0100, Julien Thierry wrote:
> Hi Russell,
> 
> On 19/09/18 10:49, Russell King wrote:
> >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.
> >
> >Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> >---
> >  arch/arm/include/asm/proc-fns.h | 14 ++++++++++++++
> >  arch/arm/kernel/setup.c         |  5 +++++
> >  arch/arm/kernel/smp.c           | 31 +++++++++++++++++++++++++++++++
> >  arch/arm/mm/proc-v7-bugs.c      | 17 ++---------------
> >  4 files changed, 52 insertions(+), 15 deletions(-)
> >
> >diff --git a/arch/arm/include/asm/proc-fns.h b/arch/arm/include/asm/proc-fns.h
> >index 571a1346245b..ddbb25cb8968 100644
> >--- a/arch/arm/include/asm/proc-fns.h
> >+++ b/arch/arm/include/asm/proc-fns.h
> >@@ -104,11 +104,25 @@ 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
> >+static inline void init_proc_vtable(const struct processor *p)
> >+{
> >+	*cpu_vtable[smp_processor_id()] = *p;
> >+}
> 
> As you stated, 32-bit ARM can only have up to 8 logical CPUs. So, do we save
> much by having an added level of indirection? (i.e. having a table of
> pointers instead of directly having a table of struct processor)

Yes we do - in a word, security.

If we made this a table of struct processor, because it is written to
after boot, it would need to be writable for the lifetime of the kernel.
Since it is in the kernel's static data section, it will be at a well-
known offset, which means it's a target for attackers to change the
function pointers.

Exactly this was true of the 'processor' variable, which gained a
__ro_after_init annotation a while back - and going to a table of
struct processor would mean that we end up back in that situation.

Having a table of pointers to struct processor means there is an
additional level of complexity for attackers to get around - they would
first need to copy an existing table, change its contents, and then
change the appropriate CPU's cpu_vtable pointer to point to their table.
Having a table of struct processor just means an attacker would only
need to poke one 32-bit word in the array for the appropriate CPU to
gain control.

So yes, this is slightly more complex, but it's that way to avoid
undoing the benefits that have previously been gained through
facilities such as read-only data after kernel initialisation.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 13.8Mbps down 630kbps up
According to speedtest.net: 13Mbps down 490kbps up

[PATCH 5/5] ARM: spectre-v2: per-CPU vtables to work around big.Little systems

[Julien Thierry]

On 20/09/18 10:21, Russell King - ARM Linux wrote:
> On Thu, Sep 20, 2018 at 10:04:44AM +0100, Julien Thierry wrote:
>> Hi Russell,
>>
>> On 19/09/18 10:49, Russell King wrote:
>>> 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.
>>>
>>> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
>>> ---
>>>   arch/arm/include/asm/proc-fns.h | 14 ++++++++++++++
>>>   arch/arm/kernel/setup.c         |  5 +++++
>>>   arch/arm/kernel/smp.c           | 31 +++++++++++++++++++++++++++++++
>>>   arch/arm/mm/proc-v7-bugs.c      | 17 ++---------------
>>>   4 files changed, 52 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/arch/arm/include/asm/proc-fns.h b/arch/arm/include/asm/proc-fns.h
>>> index 571a1346245b..ddbb25cb8968 100644
>>> --- a/arch/arm/include/asm/proc-fns.h
>>> +++ b/arch/arm/include/asm/proc-fns.h
>>> @@ -104,11 +104,25 @@ 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
>>> +static inline void init_proc_vtable(const struct processor *p)
>>> +{
>>> +	*cpu_vtable[smp_processor_id()] = *p;
>>> +}
>>
>> As you stated, 32-bit ARM can only have up to 8 logical CPUs. So, do we save
>> much by having an added level of indirection? (i.e. having a table of
>> pointers instead of directly having a table of struct processor)
> 
> Yes we do - in a word, security.
> 
> If we made this a table of struct processor, because it is written to
> after boot, it would need to be writable for the lifetime of the kernel.
> Since it is in the kernel's static data section, it will be at a well-
> known offset, which means it's a target for attackers to change the
> function pointers.
> 
> Exactly this was true of the 'processor' variable, which gained a
> __ro_after_init annotation a while back - and going to a table of
> struct processor would mean that we end up back in that situation.
> 
> Having a table of pointers to struct processor means there is an
> additional level of complexity for attackers to get around - they would
> first need to copy an existing table, change its contents, and then
> change the appropriate CPU's cpu_vtable pointer to point to their table.
> Having a table of struct processor just means an attacker would only
> need to poke one 32-bit word in the array for the appropriate CPU to
> gain control.
> 
> So yes, this is slightly more complex, but it's that way to avoid
> undoing the benefits that have previously been gained through
> facilities such as read-only data after kernel initialisation.
> 

I see, thanks for explaining. A very good reason for it then :) .

Reviewed-by: Julien Thierry <julien.thierry@arm.com>

Thanks,

-- 
Julien Thierry

Re: [PATCH 5/5] ARM: spectre-v2: per-CPU vtables to work around big.Little systems

[Marek Szyprowski]

Hi All,

On 2018-09-19 11:49, Russell King wrote:
> 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.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

This patch causes lots of kernel 'BUG' messages on all Samsung Exynos
boards.  It started to appear since it has been merged to linux-next
on 20181002.  I wonder if this issue is Exynos specific or there are
some patches missing in linux-next, which should fix those 'BUGS'.
If this is Exynos specific, please let us know what should be changed
in Exynos platform code to avoid this issue.

Here are some examples:

BUG: using smp_processor_id() in preemptible [00000000] code: startpar/117
caller is pgd_alloc+0x54/0x160
CPU: 3 PID: 117 Comm: startpar Not tainted 4.16.0-00029-gc6eb8a2e9190 #862
Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[<c0111074>] (unwind_backtrace) from [<c010d64c>] (show_stack+0x10/0x14)
[<c010d64c>] (show_stack) from [<c09b5c50>] (dump_stack+0x90/0xc8)
[<c09b5c50>] (dump_stack) from [<c044b76c>]
(check_preemption_disabled+0x118/0x130)
[<c044b76c>] (check_preemption_disabled) from [<c0119edc>]
(pgd_alloc+0x54/0x160)
[<c0119edc>] (pgd_alloc) from [<c0121570>] (mm_init+0xe4/0x174)
[<c0121570>] (mm_init) from [<c01234a4>] (copy_process.part.5+0x1480/0x1a9c)
[<c01234a4>] (copy_process.part.5) from [<c0123c24>] (_do_fork+0xc4/0x7ec)
[<c0123c24>] (_do_fork) from [<c012441c>] (SyS_clone+0x24/0x2c)
[<c012441c>] (SyS_clone) from [<c0101000>] (ret_fast_syscall+0x0/0x28)

BUG: using smp_processor_id() in preemptible [00000000] code:
checkroot-bootc/686
[<c010d64c>] (show_stack) from [<c09b5c50>] (dump_stack+0x90/0xc8)
caller is pgd_alloc+0x54/0x160
[<c09b5c50>] (dump_stack) from [<c044b76c>]
(check_preemption_disabled+0x118/0x130)
[<c044b76c>] (check_preemption_disabled) from [<c025a594>]
(vmap_page_range_noflush+0x114/0x1dc)
[<c025a594>] (vmap_page_range_noflush) from [<c025a68c>]
(map_vm_area+0x30/0x64)
[<c025a68c>] (map_vm_area) from [<c025b9bc>]
(__vmalloc_node_range+0x14c/0x220)
[<c025b9bc>] (__vmalloc_node_range) from [<c025bad8>]
(__vmalloc_node+0x48/0x50)
[<c025bad8>] (__vmalloc_node) from [<c025bb14>] (vmalloc+0x34/0x3c)
[<c025bb14>] (vmalloc) from [<c04b2960>] (n_tty_open+0x10/0xc8)
[<c04b2960>] (n_tty_open) from [<c04b66d8>] (tty_ldisc_open+0x3c/0x78)
[<c04b66d8>] (tty_ldisc_open) from [<c04b6ebc>] (tty_ldisc_setup+0x14/0x58)
[<c04b6ebc>] (tty_ldisc_setup) from [<c04b11ec>] (tty_init_dev+0xa4/0x1b0)
[<c04b11ec>] (tty_init_dev) from [<c04bab78>] (ptmx_open+0x90/0x160)
[<c04bab78>] (ptmx_open) from [<c02792d8>] (chrdev_open+0x9c/0x174)
[<c02792d8>] (chrdev_open) from [<c0271524>] (do_dentry_open+0xfc/0x30c)
[<c0271524>] (do_dentry_open) from [<c02833e4>] (path_openat+0x454/0xed0)
[<c02833e4>] (path_openat) from [<c0284968>] (do_filp_open+0x60/0xb4)
[<c0284968>] (do_filp_open) from [<c0272cb8>] (do_sys_open+0x118/0x1c4)
[<c0272cb8>] (do_sys_open) from [<c0101000>] (ret_fast_syscall+0x0/0x28)


> ---
>  arch/arm/include/asm/proc-fns.h | 14 ++++++++++++++
>  arch/arm/kernel/setup.c         |  5 +++++
>  arch/arm/kernel/smp.c           | 31 +++++++++++++++++++++++++++++++
>  arch/arm/mm/proc-v7-bugs.c      | 17 ++---------------
>  4 files changed, 52 insertions(+), 15 deletions(-)
>
> diff --git a/arch/arm/include/asm/proc-fns.h b/arch/arm/include/asm/proc-fns.h
> index 571a1346245b..ddbb25cb8968 100644
> --- a/arch/arm/include/asm/proc-fns.h
> +++ b/arch/arm/include/asm/proc-fns.h
> @@ -104,11 +104,25 @@ 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
> +static inline void init_proc_vtable(const struct processor *p)
> +{
> +	*cpu_vtable[smp_processor_id()] = *p;
> +}
> +#else
>  #define PROC_VTABLE(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)
> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> index c214bd14a1fe..cd46a595422c 100644
> --- a/arch/arm/kernel/setup.c
> +++ b/arch/arm/kernel/setup.c
> @@ -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;
> diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
> index 5ad0b67b9e33..82b879db32ee 100644
> --- a/arch/arm/kernel/smp.c
> +++ b/arch/arm/kernel/smp.c
> @@ -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.
> diff --git a/arch/arm/mm/proc-v7-bugs.c b/arch/arm/mm/proc-v7-bugs.c
> index 5544b82a2e7a..9a07916af8dd 100644
> --- a/arch/arm/mm/proc-v7-bugs.c
> +++ b/arch/arm/mm/proc-v7-bugs.c
> @@ -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)

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 5/5] ARM: spectre-v2: per-CPU vtables to work around big.Little systems

[Marek Szyprowski]

Hi All,

On 2018-09-19 11:49, Russell King wrote:
> 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.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

This patch causes lots of kernel 'BUG' messages on all Samsung Exynos
boards.? It started to appear since it has been merged to linux-next
on 20181002.? I wonder if this issue is Exynos specific or there are
some patches missing in linux-next, which should fix those 'BUGS'.
If this is Exynos specific, please let us know what should be changed
in Exynos platform code to avoid this issue.

Here are some examples:

BUG: using smp_processor_id() in preemptible [00000000] code: startpar/117
caller is pgd_alloc+0x54/0x160
CPU: 3 PID: 117 Comm: startpar Not tainted 4.16.0-00029-gc6eb8a2e9190 #862
Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[<c0111074>] (unwind_backtrace) from [<c010d64c>] (show_stack+0x10/0x14)
[<c010d64c>] (show_stack) from [<c09b5c50>] (dump_stack+0x90/0xc8)
[<c09b5c50>] (dump_stack) from [<c044b76c>]
(check_preemption_disabled+0x118/0x130)
[<c044b76c>] (check_preemption_disabled) from [<c0119edc>]
(pgd_alloc+0x54/0x160)
[<c0119edc>] (pgd_alloc) from [<c0121570>] (mm_init+0xe4/0x174)
[<c0121570>] (mm_init) from [<c01234a4>] (copy_process.part.5+0x1480/0x1a9c)
[<c01234a4>] (copy_process.part.5) from [<c0123c24>] (_do_fork+0xc4/0x7ec)
[<c0123c24>] (_do_fork) from [<c012441c>] (SyS_clone+0x24/0x2c)
[<c012441c>] (SyS_clone) from [<c0101000>] (ret_fast_syscall+0x0/0x28)

BUG: using smp_processor_id() in preemptible [00000000] code:
checkroot-bootc/686
[<c010d64c>] (show_stack) from [<c09b5c50>] (dump_stack+0x90/0xc8)
caller is pgd_alloc+0x54/0x160
[<c09b5c50>] (dump_stack) from [<c044b76c>]
(check_preemption_disabled+0x118/0x130)
[<c044b76c>] (check_preemption_disabled) from [<c025a594>]
(vmap_page_range_noflush+0x114/0x1dc)
[<c025a594>] (vmap_page_range_noflush) from [<c025a68c>]
(map_vm_area+0x30/0x64)
[<c025a68c>] (map_vm_area) from [<c025b9bc>]
(__vmalloc_node_range+0x14c/0x220)
[<c025b9bc>] (__vmalloc_node_range) from [<c025bad8>]
(__vmalloc_node+0x48/0x50)
[<c025bad8>] (__vmalloc_node) from [<c025bb14>] (vmalloc+0x34/0x3c)
[<c025bb14>] (vmalloc) from [<c04b2960>] (n_tty_open+0x10/0xc8)
[<c04b2960>] (n_tty_open) from [<c04b66d8>] (tty_ldisc_open+0x3c/0x78)
[<c04b66d8>] (tty_ldisc_open) from [<c04b6ebc>] (tty_ldisc_setup+0x14/0x58)
[<c04b6ebc>] (tty_ldisc_setup) from [<c04b11ec>] (tty_init_dev+0xa4/0x1b0)
[<c04b11ec>] (tty_init_dev) from [<c04bab78>] (ptmx_open+0x90/0x160)
[<c04bab78>] (ptmx_open) from [<c02792d8>] (chrdev_open+0x9c/0x174)
[<c02792d8>] (chrdev_open) from [<c0271524>] (do_dentry_open+0xfc/0x30c)
[<c0271524>] (do_dentry_open) from [<c02833e4>] (path_openat+0x454/0xed0)
[<c02833e4>] (path_openat) from [<c0284968>] (do_filp_open+0x60/0xb4)
[<c0284968>] (do_filp_open) from [<c0272cb8>] (do_sys_open+0x118/0x1c4)
[<c0272cb8>] (do_sys_open) from [<c0101000>] (ret_fast_syscall+0x0/0x28)


> ---
>  arch/arm/include/asm/proc-fns.h | 14 ++++++++++++++
>  arch/arm/kernel/setup.c         |  5 +++++
>  arch/arm/kernel/smp.c           | 31 +++++++++++++++++++++++++++++++
>  arch/arm/mm/proc-v7-bugs.c      | 17 ++---------------
>  4 files changed, 52 insertions(+), 15 deletions(-)
>
> diff --git a/arch/arm/include/asm/proc-fns.h b/arch/arm/include/asm/proc-fns.h
> index 571a1346245b..ddbb25cb8968 100644
> --- a/arch/arm/include/asm/proc-fns.h
> +++ b/arch/arm/include/asm/proc-fns.h
> @@ -104,11 +104,25 @@ 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
> +static inline void init_proc_vtable(const struct processor *p)
> +{
> +	*cpu_vtable[smp_processor_id()] = *p;
> +}
> +#else
>  #define PROC_VTABLE(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)
> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> index c214bd14a1fe..cd46a595422c 100644
> --- a/arch/arm/kernel/setup.c
> +++ b/arch/arm/kernel/setup.c
> @@ -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;
> diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
> index 5ad0b67b9e33..82b879db32ee 100644
> --- a/arch/arm/kernel/smp.c
> +++ b/arch/arm/kernel/smp.c
> @@ -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.
> diff --git a/arch/arm/mm/proc-v7-bugs.c b/arch/arm/mm/proc-v7-bugs.c
> index 5544b82a2e7a..9a07916af8dd 100644
> --- a/arch/arm/mm/proc-v7-bugs.c
> +++ b/arch/arm/mm/proc-v7-bugs.c
> @@ -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)

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH 5/5] ARM: spectre-v2: per-CPU vtables to work around big.Little systems

[Krzysztof Kozlowski]

On Fri, 5 Oct 2018 at 11:09, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>
> Hi All,
>
> On 2018-09-19 11:49, Russell King wrote:
> > 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.
> >
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
>
> This patch causes lots of kernel 'BUG' messages on all Samsung Exynos
> boards.  It started to appear since it has been merged to linux-next
> on 20181002.  I wonder if this issue is Exynos specific or there are
> some patches missing in linux-next, which should fix those 'BUGS'.
> If this is Exynos specific, please let us know what should be changed
> in Exynos platform code to avoid this issue.

Thanks Marek for narrowing this down.

I confirm. All my Exynos boards experience this. Full logs:
https://krzk.eu/#/builders/1/builds/2811/steps/12/logs/serial0

Best regards,
Krzysztof

[PATCH 5/5] ARM: spectre-v2: per-CPU vtables to work around big.Little systems

[Krzysztof Kozlowski]

On Fri, 5 Oct 2018 at 11:09, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>
> Hi All,
>
> On 2018-09-19 11:49, Russell King wrote:
> > 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.
> >
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
>
> This patch causes lots of kernel 'BUG' messages on all Samsung Exynos
> boards.  It started to appear since it has been merged to linux-next
> on 20181002.  I wonder if this issue is Exynos specific or there are
> some patches missing in linux-next, which should fix those 'BUGS'.
> If this is Exynos specific, please let us know what should be changed
> in Exynos platform code to avoid this issue.

Thanks Marek for narrowing this down.

I confirm. All my Exynos boards experience this. Full logs:
https://krzk.eu/#/builders/1/builds/2811/steps/12/logs/serial0

Best regards,
Krzysztof

Re: [PATCH 5/5] ARM: spectre-v2: per-CPU vtables to work around big.Little systems

[Russell King - ARM Linux]

On Fri, Oct 05, 2018 at 11:09:40AM +0200, Marek Szyprowski wrote:
> This patch causes lots of kernel 'BUG' messages on all Samsung Exynos
> boards.  It started to appear since it has been merged to linux-next
> on 20181002.  I wonder if this issue is Exynos specific or there are
> some patches missing in linux-next, which should fix those 'BUGS'.
> If this is Exynos specific, please let us know what should be changed
> in Exynos platform code to avoid this issue.

Thanks for the report.

It looks like my solution for big.Little isn't possible... back to
the drawing board, and big.Little will have to remain vulnerable to
Spectre for another release cycle.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

[PATCH 5/5] ARM: spectre-v2: per-CPU vtables to work around big.Little systems

[Russell King - ARM Linux]

On Fri, Oct 05, 2018 at 11:09:40AM +0200, Marek Szyprowski wrote:
> This patch causes lots of kernel 'BUG' messages on all Samsung Exynos
> boards.? It started to appear since it has been merged to linux-next
> on 20181002.? I wonder if this issue is Exynos specific or there are
> some patches missing in linux-next, which should fix those 'BUGS'.
> If this is Exynos specific, please let us know what should be changed
> in Exynos platform code to avoid this issue.

Thanks for the report.

It looks like my solution for big.Little isn't possible... back to
the drawing board, and big.Little will have to remain vulnerable to
Spectre for another release cycle.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: [PATCH 5/5] ARM: spectre-v2: per-CPU vtables to work around big.Little systems

[Russell King - ARM Linux]

On Fri, Oct 05, 2018 at 10:46:19AM +0100, Russell King - ARM Linux wrote:
> On Fri, Oct 05, 2018 at 11:09:40AM +0200, Marek Szyprowski wrote:
> > This patch causes lots of kernel 'BUG' messages on all Samsung Exynos
> > boards.  It started to appear since it has been merged to linux-next
> > on 20181002.  I wonder if this issue is Exynos specific or there are
> > some patches missing in linux-next, which should fix those 'BUGS'.
> > If this is Exynos specific, please let us know what should be changed
> > in Exynos platform code to avoid this issue.
> 
> Thanks for the report.
> 
> It looks like my solution for big.Little isn't possible... back to
> the drawing board, and big.Little will have to remain vulnerable to
> Spectre for another release cycle.

I've pushed out a new version in my build branch for the autobuilders
to chew on, but I've little confidence in validating that the problem
is fixed because the boot results are completely unreliable.

It really doesn't help that kernelci.org flags boot logs as "green"
and "successful" when they contain such stuff as:

01:08:40.181846  [    9.309984] Unable to handle kernel paging request at virtual address e7fddef0

which is the kernel hitting a BUG() - for the full log, see:

https://storage.kernelci.org/rmk/to-build/v4.16-38-g9fa10446d304/arm/multi_v7_defconfig/lab-collabora/boot-exynos5800-peach-pi.html

This means the only way to check is to _manually_ go through reading
each and every boot log - to see if your reported BUG: messages are
there - no thanks.

If kernelci thinks that a boot which hits a kernel BUG(), but still
manages to get to a shell prompt is successful, it's giving very
misleading boot results.  What about a WARN_ON() or an oops that
still allows it to reach a shell prompt.

Yes, these may be "successful" in so far as reaching the shell prompt,
but they should at least be flagged for further inspection, not
effectively marked as "there is nothing wrong here".

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

[PATCH 5/5] ARM: spectre-v2: per-CPU vtables to work around big.Little systems

[Russell King - ARM Linux]

On Fri, Oct 05, 2018 at 10:46:19AM +0100, Russell King - ARM Linux wrote:
> On Fri, Oct 05, 2018 at 11:09:40AM +0200, Marek Szyprowski wrote:
> > This patch causes lots of kernel 'BUG' messages on all Samsung Exynos
> > boards.? It started to appear since it has been merged to linux-next
> > on 20181002.? I wonder if this issue is Exynos specific or there are
> > some patches missing in linux-next, which should fix those 'BUGS'.
> > If this is Exynos specific, please let us know what should be changed
> > in Exynos platform code to avoid this issue.
> 
> Thanks for the report.
> 
> It looks like my solution for big.Little isn't possible... back to
> the drawing board, and big.Little will have to remain vulnerable to
> Spectre for another release cycle.

I've pushed out a new version in my build branch for the autobuilders
to chew on, but I've little confidence in validating that the problem
is fixed because the boot results are completely unreliable.

It really doesn't help that kernelci.org flags boot logs as "green"
and "successful" when they contain such stuff as:

01:08:40.181846  [    9.309984] Unable to handle kernel paging request at virtual address e7fddef0

which is the kernel hitting a BUG() - for the full log, see:

https://storage.kernelci.org/rmk/to-build/v4.16-38-g9fa10446d304/arm/multi_v7_defconfig/lab-collabora/boot-exynos5800-peach-pi.html

This means the only way to check is to _manually_ go through reading
each and every boot log - to see if your reported BUG: messages are
there - no thanks.

If kernelci thinks that a boot which hits a kernel BUG(), but still
manages to get to a shell prompt is successful, it's giving very
misleading boot results.  What about a WARN_ON() or an oops that
still allows it to reach a shell prompt.

Yes, these may be "successful" in so far as reaching the shell prompt,
but they should at least be flagged for further inspection, not
effectively marked as "there is nothing wrong here".

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: [PATCH 5/5] ARM: spectre-v2: per-CPU vtables to work around big.Little systems

[Marek Szyprowski]

Hi Russell,

On 2018-10-30 11:50, Russell King - ARM Linux wrote:
> On Fri, Oct 05, 2018 at 10:46:19AM +0100, Russell King - ARM Linux wrote:
>> On Fri, Oct 05, 2018 at 11:09:40AM +0200, Marek Szyprowski wrote:
>>> This patch causes lots of kernel 'BUG' messages on all Samsung Exynos
>>> boards.  It started to appear since it has been merged to linux-next
>>> on 20181002.  I wonder if this issue is Exynos specific or there are
>>> some patches missing in linux-next, which should fix those 'BUGS'.
>>> If this is Exynos specific, please let us know what should be changed
>>> in Exynos platform code to avoid this issue.
>> Thanks for the report.
>>
>> It looks like my solution for big.Little isn't possible... back to
>> the drawing board, and big.Little will have to remain vulnerable to
>> Spectre for another release cycle.
> I've pushed out a new version in my build branch for the autobuilders
> to chew on, but I've little confidence in validating that the problem
> is fixed because the boot results are completely unreliable.
>
> It really doesn't help that kernelci.org flags boot logs as "green"
> and "successful" when they contain such stuff as:
>
> 01:08:40.181846  [    9.309984] Unable to handle kernel paging request at virtual address e7fddef0
>
> which is the kernel hitting a BUG() - for the full log, see:
>
> https://storage.kernelci.org/rmk/to-build/v4.16-38-g9fa10446d304/arm/multi_v7_defconfig/lab-collabora/boot-exynos5800-peach-pi.html
>
> This means the only way to check is to _manually_ go through reading
> each and every boot log - to see if your reported BUG: messages are
> there - no thanks.
>
> If kernelci thinks that a boot which hits a kernel BUG(), but still
> manages to get to a shell prompt is successful, it's giving very
> misleading boot results.  What about a WARN_ON() or an oops that
> still allows it to reach a shell prompt.
>
> Yes, these may be "successful" in so far as reaching the shell prompt,
> but they should at least be flagged for further inspection, not
> effectively marked as "there is nothing wrong here".

I've run my own tests on various Exynos SoC based boards and your
'for-next' branch works fine and don't cause any regressions.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 5/5] ARM: spectre-v2: per-CPU vtables to work around big.Little systems

[Marek Szyprowski]

Hi Russell,

On 2018-10-30 11:50, Russell King - ARM Linux wrote:
> On Fri, Oct 05, 2018 at 10:46:19AM +0100, Russell King - ARM Linux wrote:
>> On Fri, Oct 05, 2018 at 11:09:40AM +0200, Marek Szyprowski wrote:
>>> This patch causes lots of kernel 'BUG' messages on all Samsung Exynos
>>> boards.? It started to appear since it has been merged to linux-next
>>> on 20181002.? I wonder if this issue is Exynos specific or there are
>>> some patches missing in linux-next, which should fix those 'BUGS'.
>>> If this is Exynos specific, please let us know what should be changed
>>> in Exynos platform code to avoid this issue.
>> Thanks for the report.
>>
>> It looks like my solution for big.Little isn't possible... back to
>> the drawing board, and big.Little will have to remain vulnerable to
>> Spectre for another release cycle.
> I've pushed out a new version in my build branch for the autobuilders
> to chew on, but I've little confidence in validating that the problem
> is fixed because the boot results are completely unreliable.
>
> It really doesn't help that kernelci.org flags boot logs as "green"
> and "successful" when they contain such stuff as:
>
> 01:08:40.181846  [    9.309984] Unable to handle kernel paging request at virtual address e7fddef0
>
> which is the kernel hitting a BUG() - for the full log, see:
>
> https://storage.kernelci.org/rmk/to-build/v4.16-38-g9fa10446d304/arm/multi_v7_defconfig/lab-collabora/boot-exynos5800-peach-pi.html
>
> This means the only way to check is to _manually_ go through reading
> each and every boot log - to see if your reported BUG: messages are
> there - no thanks.
>
> If kernelci thinks that a boot which hits a kernel BUG(), but still
> manages to get to a shell prompt is successful, it's giving very
> misleading boot results.  What about a WARN_ON() or an oops that
> still allows it to reach a shell prompt.
>
> Yes, these may be "successful" in so far as reaching the shell prompt,
> but they should at least be flagged for further inspection, not
> effectively marked as "there is nothing wrong here".

I've run my own tests on various Exynos SoC based boards and your
'for-next' branch works fine and don't cause any regressions.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH 5/5] ARM: spectre-v2: per-CPU vtables to work around big.Little systems

[Russell King - ARM Linux]

On Wed, Oct 31, 2018 at 04:21:30PM +0100, Marek Szyprowski wrote:
> Hi Russell,
> 
> On 2018-10-30 11:50, Russell King - ARM Linux wrote:
> > On Fri, Oct 05, 2018 at 10:46:19AM +0100, Russell King - ARM Linux wrote:
> >> On Fri, Oct 05, 2018 at 11:09:40AM +0200, Marek Szyprowski wrote:
> >>> This patch causes lots of kernel 'BUG' messages on all Samsung Exynos
> >>> boards.  It started to appear since it has been merged to linux-next
> >>> on 20181002.  I wonder if this issue is Exynos specific or there are
> >>> some patches missing in linux-next, which should fix those 'BUGS'.
> >>> If this is Exynos specific, please let us know what should be changed
> >>> in Exynos platform code to avoid this issue.
> >> Thanks for the report.
> >>
> >> It looks like my solution for big.Little isn't possible... back to
> >> the drawing board, and big.Little will have to remain vulnerable to
> >> Spectre for another release cycle.
> > I've pushed out a new version in my build branch for the autobuilders
> > to chew on, but I've little confidence in validating that the problem
> > is fixed because the boot results are completely unreliable.
> >
> > It really doesn't help that kernelci.org flags boot logs as "green"
> > and "successful" when they contain such stuff as:
> >
> > 01:08:40.181846  [    9.309984] Unable to handle kernel paging request at virtual address e7fddef0
> >
> > which is the kernel hitting a BUG() - for the full log, see:
> >
> > https://storage.kernelci.org/rmk/to-build/v4.16-38-g9fa10446d304/arm/multi_v7_defconfig/lab-collabora/boot-exynos5800-peach-pi.html
> >
> > This means the only way to check is to _manually_ go through reading
> > each and every boot log - to see if your reported BUG: messages are
> > there - no thanks.
> >
> > If kernelci thinks that a boot which hits a kernel BUG(), but still
> > manages to get to a shell prompt is successful, it's giving very
> > misleading boot results.  What about a WARN_ON() or an oops that
> > still allows it to reach a shell prompt.
> >
> > Yes, these may be "successful" in so far as reaching the shell prompt,
> > but they should at least be flagged for further inspection, not
> > effectively marked as "there is nothing wrong here".
> 
> I've run my own tests on various Exynos SoC based boards and your
> 'for-next' branch works fine and don't cause any regressions.

The code isn't in for-next because we're in a merge window, and
linux-next is supposed to be stable for the duration of that, only
accepting bug fixes.

It was added briefly to for-next (carefully timed and discussed
with Stephen to ensure it didn't appear in linux-next), to try
and get fuller builder coverage, which is when the above issue was
found.

If you want to test, please use its separate branch, 'spectre'.
Thanks.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

[PATCH 5/5] ARM: spectre-v2: per-CPU vtables to work around big.Little systems

[Russell King - ARM Linux]

On Wed, Oct 31, 2018 at 04:21:30PM +0100, Marek Szyprowski wrote:
> Hi Russell,
> 
> On 2018-10-30 11:50, Russell King - ARM Linux wrote:
> > On Fri, Oct 05, 2018 at 10:46:19AM +0100, Russell King - ARM Linux wrote:
> >> On Fri, Oct 05, 2018 at 11:09:40AM +0200, Marek Szyprowski wrote:
> >>> This patch causes lots of kernel 'BUG' messages on all Samsung Exynos
> >>> boards.? It started to appear since it has been merged to linux-next
> >>> on 20181002.? I wonder if this issue is Exynos specific or there are
> >>> some patches missing in linux-next, which should fix those 'BUGS'.
> >>> If this is Exynos specific, please let us know what should be changed
> >>> in Exynos platform code to avoid this issue.
> >> Thanks for the report.
> >>
> >> It looks like my solution for big.Little isn't possible... back to
> >> the drawing board, and big.Little will have to remain vulnerable to
> >> Spectre for another release cycle.
> > I've pushed out a new version in my build branch for the autobuilders
> > to chew on, but I've little confidence in validating that the problem
> > is fixed because the boot results are completely unreliable.
> >
> > It really doesn't help that kernelci.org flags boot logs as "green"
> > and "successful" when they contain such stuff as:
> >
> > 01:08:40.181846  [    9.309984] Unable to handle kernel paging request at virtual address e7fddef0
> >
> > which is the kernel hitting a BUG() - for the full log, see:
> >
> > https://storage.kernelci.org/rmk/to-build/v4.16-38-g9fa10446d304/arm/multi_v7_defconfig/lab-collabora/boot-exynos5800-peach-pi.html
> >
> > This means the only way to check is to _manually_ go through reading
> > each and every boot log - to see if your reported BUG: messages are
> > there - no thanks.
> >
> > If kernelci thinks that a boot which hits a kernel BUG(), but still
> > manages to get to a shell prompt is successful, it's giving very
> > misleading boot results.  What about a WARN_ON() or an oops that
> > still allows it to reach a shell prompt.
> >
> > Yes, these may be "successful" in so far as reaching the shell prompt,
> > but they should at least be flagged for further inspection, not
> > effectively marked as "there is nothing wrong here".
> 
> I've run my own tests on various Exynos SoC based boards and your
> 'for-next' branch works fine and don't cause any regressions.

The code isn't in for-next because we're in a merge window, and
linux-next is supposed to be stable for the duration of that, only
accepting bug fixes.

It was added briefly to for-next (carefully timed and discussed
with Stephen to ensure it didn't appear in linux-next), to try
and get fuller builder coverage, which is when the above issue was
found.

If you want to test, please use its separate branch, 'spectre'.
Thanks.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: [PATCH 5/5] ARM: spectre-v2: per-CPU vtables to work around big.Little systems

[Kevin Hilman]

Hi Russell,

Russell King - ARM Linux writes:

> On Fri, Oct 05, 2018 at 10:46:19AM +0100, Russell King - ARM Linux wrote:
>> On Fri, Oct 05, 2018 at 11:09:40AM +0200, Marek Szyprowski wrote:
>> > This patch causes lots of kernel 'BUG' messages on all Samsung Exynos
>> > boards. It started to appear since it has been merged to linux-next
>> > on 20181002. I wonder if this issue is Exynos specific or there are
>> > some patches missing in linux-next, which should fix those 'BUGS'.
>> > If this is Exynos specific, please let us know what should be changed
>> > in Exynos platform code to avoid this issue.
>>
>> Thanks for the report.
>>
>> It looks like my solution for big.Little isn't possible... back to
>> the drawing board, and big.Little will have to remain vulnerable to
>> Spectre for another release cycle.
>
> I've pushed out a new version in my build branch for the autobuilders
> to chew on, but I've little confidence in validating that the problem
> is fixed because the boot results are completely unreliable.
>
> It really doesn't help that kernelci.org flags boot logs as "green"
> and "successful" when they contain such stuff as:
>
> 01:08:40.181846  [    9.309984] Unable to handle kernel paging request at virtual address e7fddef0
>
> which is the kernel hitting a BUG() - for the full log, see:
>
> https://storage.kernelci.org/rmk/to-build/v4.16-38-g9fa10446d304/arm/multi_v7_defconfig/lab-collabora/boot-exynos5800-peach-pi.html
>
> This means the only way to check is to _manually_ go through reading
> each and every boot log - to see if your reported BUG: messages are
> there - no thanks.

You're right, we should be flagging these as boot failures.

> If kernelci thinks that a boot which hits a kernel BUG(), but still
> manages to get to a shell prompt is successful, it's giving very
> misleading boot results.  What about a WARN_ON() or an oops that
> still allows it to reach a shell prompt.
>
> Yes, these may be "successful" in so far as reaching the shell prompt,
> but they should at least be flagged for further inspection, not
> effectively marked as "there is nothing wrong here".

We currently catch stack bactraces like this, and flag them as boot
warnings, and they are shown in the overview:
https://kernelci.org/boot/id/5bd7af1b59b514ece774be08/

But, as you pointed out, that still requires knowing what you're looking
for and manually digging for them as we con't have a good way of
flagging them currently.

What we need to do is catch both warnings (e.g. WARN_ON) etc. as well as
real errors (e.g. BUG/panic/etc.).  In the case of errors, we should
mark the boot test as FAIL.

Kevin

[PATCH 5/5] ARM: spectre-v2: per-CPU vtables to work around big.Little systems

[Kevin Hilman]

Hi Russell,

Russell King - ARM Linux writes:

> On Fri, Oct 05, 2018 at 10:46:19AM +0100, Russell King - ARM Linux wrote:
>> On Fri, Oct 05, 2018 at 11:09:40AM +0200, Marek Szyprowski wrote:
>> > This patch causes lots of kernel 'BUG' messages on all Samsung Exynos
>> > boards. It started to appear since it has been merged to linux-next
>> > on 20181002. I wonder if this issue is Exynos specific or there are
>> > some patches missing in linux-next, which should fix those 'BUGS'.
>> > If this is Exynos specific, please let us know what should be changed
>> > in Exynos platform code to avoid this issue.
>>
>> Thanks for the report.
>>
>> It looks like my solution for big.Little isn't possible... back to
>> the drawing board, and big.Little will have to remain vulnerable to
>> Spectre for another release cycle.
>
> I've pushed out a new version in my build branch for the autobuilders
> to chew on, but I've little confidence in validating that the problem
> is fixed because the boot results are completely unreliable.
>
> It really doesn't help that kernelci.org flags boot logs as "green"
> and "successful" when they contain such stuff as:
>
> 01:08:40.181846  [    9.309984] Unable to handle kernel paging request at virtual address e7fddef0
>
> which is the kernel hitting a BUG() - for the full log, see:
>
> https://storage.kernelci.org/rmk/to-build/v4.16-38-g9fa10446d304/arm/multi_v7_defconfig/lab-collabora/boot-exynos5800-peach-pi.html
>
> This means the only way to check is to _manually_ go through reading
> each and every boot log - to see if your reported BUG: messages are
> there - no thanks.

You're right, we should be flagging these as boot failures.

> If kernelci thinks that a boot which hits a kernel BUG(), but still
> manages to get to a shell prompt is successful, it's giving very
> misleading boot results.  What about a WARN_ON() or an oops that
> still allows it to reach a shell prompt.
>
> Yes, these may be "successful" in so far as reaching the shell prompt,
> but they should at least be flagged for further inspection, not
> effectively marked as "there is nothing wrong here".

We currently catch stack bactraces like this, and flag them as boot
warnings, and they are shown in the overview:
https://kernelci.org/boot/id/5bd7af1b59b514ece774be08/

But, as you pointed out, that still requires knowing what you're looking
for and manually digging for them as we con't have a good way of
flagging them currently.

What we need to do is catch both warnings (e.g. WARN_ON) etc. as well as
real errors (e.g. BUG/panic/etc.).  In the case of errors, we should
mark the boot test as FAIL.

Kevin