Spectre big.Little updates
Subsystems (2)
Russell King - ARM Linux
maintainercover Spectre big.Little updates
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(-)
Signature · 3 lines
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
Move lookup_processor_type() out of the __init section so it is callable from (eg) the secondary startup code during hotplug.
--- arch/arm/kernel/head-common.S | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
arch/arm/kernel/head-common.S +3 -3
@@ -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
Signature · 1 line
2.7.4
PATCH 1/5 ARM: make lookup_processor_type() non-__init
Hi Russell,
Quoted 3 lines · Russell King
Move
lookup_processor_type()out of the__initsection so it is callable from (eg) the secondary startup code during hotplug.
Quoted 30 lines
Signed-off-by Russell King <[email protected]>--- arch/arm/kernel/head-common.S | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)arch/arm/kernel/head-common.S +3
-3@@ -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
Signature · 1 line
Julien Thierry
PATCH 2/5 ARM: split out processor lookup
In reply to Russell King - ARM LinuxSplit 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.
--- arch/arm/include/asm/cputype.h | 1 + arch/arm/kernel/setup.c | 31 +++++++++++++++++++------------ 2 files changed, 20 insertions(+), 12 deletions(-)
arch/arm/include/asm/cputype.h +1 -0
@@ -107,6 +107,7 @@#define ARM_CPU_PART_SCORPION 0x510002d0extern unsigned int processor_id; +struct proc_info_list *lookup_processor (u32 midr);#ifdef CONFIG_CPU_CP15 #define read_cpuid (reg) \
arch/arm/kernel/setup.c +19 -12
@@ -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",
Signature · 1 line
2.7.4
PATCH 2/5 ARM: split out processor lookup
Hi Russell,
Quoted 4 lines · 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.
Quoted 73 lines
Signed-off-by Russell King <[email protected]>--- arch/arm/include/asm/cputype.h | 1 + arch/arm/kernel/setup.c | 31 +++++++++++++++++++------------ 2 files changed, 20 insertions(+), 12 deletions(-)arch/arm/include/asm/cputype.h +1
-0@@ -107,6 +107,7 @@#define ARM_CPU_PART_SCORPION 0x510002d0extern unsigned int processor_id; +struct proc_info_list *lookup_processor (u32 midr);#ifdef CONFIG_CPU_CP15 #define read_cpuid (reg) \arch/arm/kernel/setup.c +19
-12@@ -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",
Signature · 1 line
Julien Thierry
PATCH 3/5 ARM: clean up per-processor check_bugs method call
In reply to Russell King - ARM LinuxCall 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.
--- arch/arm/include/asm/proc-fns.h | 1 + arch/arm/kernel/bugs.c | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-)
arch/arm/include/asm/proc-fns.h +1 -0
@@ -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
arch/arm/kernel/bugs.c +2 -2
@@ -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 }
Signature · 1 line
2.7.4
PATCH 3/5 ARM: clean up per-processor check_bugs method call
Hi Russell,
Quoted 4 lines · 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 intoproc-fns.h.
Quoted 34 lines
Signed-off-by Russell King <[email protected]>--- arch/arm/include/asm/proc-fns.h | 1 + arch/arm/kernel/bugs.c | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-)arch/arm/include/asm/proc-fns.h +1
-0@@ -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_idlearch/arm/kernel/bugs.c +2
-2@@ -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 }
Signature · 1 line
Julien Thierry
PATCH 4/5 ARM: add PROC_VTABLE macro
In reply to Russell King - ARM LinuxAllow 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.
--- arch/arm/include/asm/proc-fns.h | 36 ++++++++++++++++++++++++------------ arch/arm/kernel/setup.c | 4 +--- 2 files changed, 25 insertions(+), 15 deletions(-)
arch/arm/include/asm/proc-fns.h +24 -12
@@ -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 );
arch/arm/kernel/setup.c +1 -3
@@ -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
Signature · 1 line
2.7.4
PATCH 4/5 ARM: add PROC_VTABLE macro
Hi Russell,
Quoted 4 lines · 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.
Quoted 87 lines
Signed-off-by Russell King <[email protected]>--- arch/arm/include/asm/proc-fns.h | 36 ++++++++++++++++++++++++------------ arch/arm/kernel/setup.c | 4 +--- 2 files changed, 25 insertions(+), 15 deletions(-)arch/arm/include/asm/proc-fns.h +24
-12@@ -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 );arch/arm/kernel/setup.c +1
-3@@ -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
Signature · 1 line
Julien Thierry
PATCH 5/5 ARM: spectre-v2: per-CPU vtables to work around big.Little systems
In reply to Russell King - ARM LinuxIn 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.
--- 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(-)
arch/arm/include/asm/proc-fns.h +14 -0
@@ -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.fstatic 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
@@ -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
@@ -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
@@ -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 )
Signature · 1 line
2.7.4
PATCH 5/5 ARM: spectre-v2: per-CPU vtables to work around big.Little systems
Hi Russell,
Quoted 26 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.
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 <[email protected]>--- 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(-)arch/arm/include/asm/proc-fns.h +2
-0@@ -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>
arch/arm/include/asm/proc-fns.h extern void cpu_do_resume(void *); · line 118
Quoted 10 lines
arch/arm/include/asm/proc-fns.h +10
-0+/* +* 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,
Quoted 150 lines
arch/arm/include/asm/proc-fns.h +2
-0+#else #define PROC_VTABLE (f) processor.fstatic 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@@ -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@@ -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@@ -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 )
Signature · 1 line
Julien Thierry
Russell King - ARM Linux
maintainerPATCH 5/5 ARM: spectre-v2: per-CPU vtables to work around big.Little systems
Quoted 34 lines · Julien Thierry
Hi Russell,
Quoted 31 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.
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 <[email protected]>--- 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(-)arch/arm/include/asm/proc-fns.h +8
-0@@ -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
Quoted 8 lines
arch/arm/include/asm/proc-fns.h extern void cpu_do_resume(void *); · line 118
Quoted 4 lines
arch/arm/include/asm/proc-fns.h +4
-0+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.
Signature · 3 lines
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
Quoted 64 lines · Russell King - ARM Linux
Quoted 34 lines · Julien Thierry
Hi Russell,
Quoted 31 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.
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 <[email protected]>--- 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(-)arch/arm/include/asm/proc-fns.h +8
-0@@ -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()]->fQuoted 8 lines
arch/arm/include/asm/proc-fns.h extern void cpu_do_resume(void *); · line 118
Quoted 4 lines
arch/arm/include/asm/proc-fns.h +4
-0+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_initannotation 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_vtablepointer 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.
Quoted 4 lines
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 :) .
Thanks,
Signature · 1 line
Julien Thierry
PATCH 5/5 ARM: spectre-v2: per-CPU vtables to work around big.Little systems
In reply to Russell KingHi All,
Quoted 9 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.
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 <[email protected]>
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)
Quoted 166 lines
--- 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(-)arch/arm/include/asm/proc-fns.h +14
-0@@ -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.fstatic 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@@ -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@@ -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@@ -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)
arch/arm/mm/proc-v7-bugs.c static void cpu_v7_spectre_init(void) · line 116
Quoted 10 lines
arch/arm/mm/proc-v7-bugs.c +0
-5pr_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
Signature · 2 lines
Marek Szyprowski, PhD Samsung R&D Institute Poland
Krzysztof Kozlowski
maintainerPATCH 5/5 ARM: spectre-v2: per-CPU vtables to work around big.Little systems
Quoted 12 lines · Marek Szyprowski
Hi All,
Quoted 8 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.
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.
Quoted 8 lines
Quoted 1 line
Signed-off-by Russell King <[email protected]>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
Russell King - ARM Linux
maintainerPATCH 5/5 ARM: spectre-v2: per-CPU vtables to work around big.Little systems
In reply to Marek SzyprowskiQuoted 6 lines · Marek Szyprowski
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.
Signature · 3 lines
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
Russell King - ARM Linux
maintainerPATCH 5/5 ARM: spectre-v2: per-CPU vtables to work around big.Little systems
Quoted 8 lines · Russell King - ARM Linux
Quoted 6 lines · Marek Szyprowski
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.
Quoted 5 lines
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:
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".
Signature · 3 lines
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
Hi Russell,
Quoted 30 lines · Russell King - ARM Linux
Quoted 7 lines · Russell King - ARM Linux
Quoted 6 lines · Marek Szyprowski
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.
Quoted 5 lines
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
e7fddef0which is the kernel hitting a
BUG()- for the full log, see: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.
Quoted 8 lines
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 aWARN_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
Signature · 2 lines
Marek Szyprowski, PhD Samsung R&D Institute Poland
Russell King - ARM Linux
maintainerPATCH 5/5 ARM: spectre-v2: per-CPU vtables to work around big.Little systems
Quoted 38 lines · Marek Szyprowski
Hi Russell,
Quoted 35 lines · Russell King - ARM Linux
Quoted 7 lines · Russell King - ARM Linux
Quoted 6 lines · Marek Szyprowski
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.
Quoted 5 lines
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
e7fddef0which is the kernel hitting a
BUG()- for the full log, see: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 aWARN_ON()or an oops that still allows it to reach a shell prompt.
Quoted 6 lines
Quoted 3 lines
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.
Signature · 3 lines
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
In reply to Russell King - ARM LinuxHi Russell,
Quoted 24 lines · Russell King - ARM Linux
Quoted 8 lines · Russell King - ARM Linux
Quoted 6 lines · Marek Szyprowski
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.
Quoted 5 lines
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
Quoted 7 lines
which is the kernel hitting a
BUG()- for the full log, see: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.
Quoted 8 lines
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 aWARN_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