Re: [PATCH 03/14] ARM: bugs: hook processor bug checking into SMP and suspend paths
On Fri, May 25, 2018 at 11:03:59AM +0100, Russell King - ARM Linux wrote: > On Thu, May 24, 2018 at 04:30:40PM -0700, Florian Fainelli wrote: > > On 05/21/2018 04:44 AM, Russell King wrote: > > > Check for CPU bugs when secondary processors are being brought online, > > > and also when CPUs are resuming from a low power mode. This gives an > > > opportunity to check that processor specific bug workarounds are > > > correctly enabled for all paths that a CPU re-enters the kernel. > > > > > > Signed-off-by: Russell King> > > Reviewed-by: Florian Fainelli > > > > Something I missed, is that this correctly warns about e.g: missing the > > IBE bit for secondary cores, but it seems to be missing it for the boot CPU: > > Are you sure that the boot CPU has the IBE bit clear? Here's what I get on TI Keystone 2, which doesn't set the IBE bit for any CPU: CPU: Testing write buffer coherency: ok CPU0: Spectre v2: firmware did not set auxiliary control register IBE bit, system vulnerable CPU0: Spectre v2: using ICIALLU workaround /cpus/cpu@0 missing clock-frequency property /cpus/cpu@1 missing clock-frequency property /cpus/cpu@2 missing clock-frequency property /cpus/cpu@3 missing clock-frequency property CPU0: thread -1, cpu 0, socket 0, mpidr 8000 Setting up static identity map for 0x80008300 - 0x80008438 Hierarchical SRCU implementation. smp: Bringing up secondary CPUs ... CPU1: thread -1, cpu 1, socket 0, mpidr 8001 CPU1: Spectre v2: firmware did not set auxiliary control register IBE bit, system vulnerable CPU1: Spectre v2: using ICIALLU workaround CPU2: thread -1, cpu 2, socket 0, mpidr 8002 CPU2: Spectre v2: firmware did not set auxiliary control register IBE bit, system vulnerable CPU2: Spectre v2: using ICIALLU workaround CPU3: thread -1, cpu 3, socket 0, mpidr 8003 CPU3: Spectre v2: firmware did not set auxiliary control register IBE bit, system vulnerable CPU3: Spectre v2: using ICIALLU workaround It should be noted that, if we implement a "bugs" bit exported to userspace (as I think someone requested) that booting on a system where only the boot CPU initially comes up (which has the IBE bit set) and then bringing the secondary CPUs online after userspace has started (where those CPUs don't have the IBE bit set) will result in the system initially not being vulnerable, but then becoming vulnerable when running on those other CPUs. If the bugs bit had already been checked by userspace, then it would think that there's no system level vulnerability. Userspace would need to check the status each time a CPU comes online. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up According to speedtest.net: 8.21Mbps down 510kbps up ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 03/14] ARM: bugs: hook processor bug checking into SMP and suspend paths
On 05/21/2018 04:44 AM, Russell King wrote: > Check for CPU bugs when secondary processors are being brought online, > and also when CPUs are resuming from a low power mode. This gives an > opportunity to check that processor specific bug workarounds are > correctly enabled for all paths that a CPU re-enters the kernel. > > Signed-off-by: Russell King> Reviewed-by: Florian Fainelli Something I missed, is that this correctly warns about e.g: missing the IBE bit for secondary cores, but it seems to be missing it for the boot CPU: [0.001053] CPU: Testing write buffer coherency: ok [0.001086] CPU: Spectre v2: using ICIALLU workaround [0.001304] CPU0: update cpu_capacity 1024 [0.001316] CPU0: thread -1, cpu 0, socket 0, mpidr 8000 [0.001693] Setting up static identity map for 0x20 - 0x200060 [0.001769] Hierarchical SRCU implementation. [0.003951] brcmstb: biuctrl: MCP: Write pairing already disabled [0.004224] smp: Bringing up secondary CPUs ... [0.004874] CPU1: update cpu_capacity 1024 [0.004877] CPU1: thread -1, cpu 1, socket 0, mpidr 8001 [0.004881] CPU1: Spectre v2: firmware did not set auxiliary control register IBE bit, system vulnerable [0.005604] CPU2: update cpu_capacity 1024 [0.005607] CPU2: thread -1, cpu 2, socket 0, mpidr 8002 [0.005610] CPU2: Spectre v2: firmware did not set auxiliary control register IBE bit, system vulnerable [0.006295] CPU3: update cpu_capacity 1024 [0.006299] CPU3: thread -1, cpu 3, socket 0, mpidr 8003 [0.006302] CPU3: Spectre v2: firmware did not set auxiliary control register IBE bit, system vulnerable [0.006377] smp: Brought up 1 node, 4 CPUs [0.006389] SMP: Total of 4 processors activated (216.00 BogoMIPS). [0.006398] CPU: All CPU(s) started in SVC mode. Which could be confusing if you intentionally restricted a SMP system to UP with maxcpus=1 or smp=off: [0.001043] CPU: Testing write buffer coherency: ok [0.001077] CPU: Spectre v2: using ICIALLU workaround [0.001291] CPU0: update cpu_capacity 1024 [0.001302] CPU0: thread -1, cpu 0, socket 0, mpidr 8000 [0.001516] Setting up static identity map for 0x20 - 0x200060 [0.001593] Hierarchical SRCU implementation. [0.003829] brcmstb: biuctrl: MCP: Write pairing already disabled [0.004097] smp: Bringing up secondary CPUs ... [0.004108] smp: Brought up 1 node, 1 CPU [0.004117] SMP: Total of 1 processors activated (54.00 BogoMIPS). [0.004126] CPU: All CPU(s) started in SVC mode. > --- > arch/arm/include/asm/bugs.h | 2 ++ > arch/arm/kernel/bugs.c | 5 + > arch/arm/kernel/smp.c | 4 > arch/arm/kernel/suspend.c | 2 ++ > 4 files changed, 13 insertions(+) > > diff --git a/arch/arm/include/asm/bugs.h b/arch/arm/include/asm/bugs.h > index ed122d294f3f..73a99c72a930 100644 > --- a/arch/arm/include/asm/bugs.h > +++ b/arch/arm/include/asm/bugs.h > @@ -14,8 +14,10 @@ extern void check_writebuffer_bugs(void); > > #ifdef CONFIG_MMU > extern void check_bugs(void); > +extern void check_other_bugs(void); > #else > #define check_bugs() do { } while (0) > +#define check_other_bugs() do { } while (0) > #endif > > #endif > diff --git a/arch/arm/kernel/bugs.c b/arch/arm/kernel/bugs.c > index 88024028bb70..16e7ba2a9cc4 100644 > --- a/arch/arm/kernel/bugs.c > +++ b/arch/arm/kernel/bugs.c > @@ -3,7 +3,12 @@ > #include > #include > > +void check_other_bugs(void) > +{ > +} > + > void __init check_bugs(void) > { > check_writebuffer_bugs(); > + check_other_bugs(); > } > diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c > index 2da087926ebe..5ad0b67b9e33 100644 > --- a/arch/arm/kernel/smp.c > +++ b/arch/arm/kernel/smp.c > @@ -31,6 +31,7 @@ > #include > > #include > +#include > #include > #include > #include > @@ -405,6 +406,9 @@ asmlinkage void secondary_start_kernel(void) >* before we continue - which happens after __cpu_up returns. >*/ > set_cpu_online(cpu, true); > + > + check_other_bugs(); > + > complete(_running); > > local_irq_enable(); > diff --git a/arch/arm/kernel/suspend.c b/arch/arm/kernel/suspend.c > index a40ebb7c0896..d08099269e35 100644 > --- a/arch/arm/kernel/suspend.c > +++ b/arch/arm/kernel/suspend.c > @@ -3,6 +3,7 @@ > #include > #include > > +#include > #include > #include > #include > @@ -36,6 +37,7 @@ int cpu_suspend(unsigned long arg, int (*fn)(unsigned long)) > cpu_switch_mm(mm->pgd, mm); > local_flush_bp_all(); > local_flush_tlb_all(); > + check_other_bugs(); > } > > return ret; > -- Florian ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH 03/14] ARM: bugs: hook processor bug checking into SMP and suspend paths
Check for CPU bugs when secondary processors are being brought online, and also when CPUs are resuming from a low power mode. This gives an opportunity to check that processor specific bug workarounds are correctly enabled for all paths that a CPU re-enters the kernel. Signed-off-by: Russell KingReviewed-by: Florian Fainelli --- arch/arm/include/asm/bugs.h | 2 ++ arch/arm/kernel/bugs.c | 5 + arch/arm/kernel/smp.c | 4 arch/arm/kernel/suspend.c | 2 ++ 4 files changed, 13 insertions(+) diff --git a/arch/arm/include/asm/bugs.h b/arch/arm/include/asm/bugs.h index ed122d294f3f..73a99c72a930 100644 --- a/arch/arm/include/asm/bugs.h +++ b/arch/arm/include/asm/bugs.h @@ -14,8 +14,10 @@ extern void check_writebuffer_bugs(void); #ifdef CONFIG_MMU extern void check_bugs(void); +extern void check_other_bugs(void); #else #define check_bugs() do { } while (0) +#define check_other_bugs() do { } while (0) #endif #endif diff --git a/arch/arm/kernel/bugs.c b/arch/arm/kernel/bugs.c index 88024028bb70..16e7ba2a9cc4 100644 --- a/arch/arm/kernel/bugs.c +++ b/arch/arm/kernel/bugs.c @@ -3,7 +3,12 @@ #include #include +void check_other_bugs(void) +{ +} + void __init check_bugs(void) { check_writebuffer_bugs(); + check_other_bugs(); } diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c index 2da087926ebe..5ad0b67b9e33 100644 --- a/arch/arm/kernel/smp.c +++ b/arch/arm/kernel/smp.c @@ -31,6 +31,7 @@ #include #include +#include #include #include #include @@ -405,6 +406,9 @@ asmlinkage void secondary_start_kernel(void) * before we continue - which happens after __cpu_up returns. */ set_cpu_online(cpu, true); + + check_other_bugs(); + complete(_running); local_irq_enable(); diff --git a/arch/arm/kernel/suspend.c b/arch/arm/kernel/suspend.c index a40ebb7c0896..d08099269e35 100644 --- a/arch/arm/kernel/suspend.c +++ b/arch/arm/kernel/suspend.c @@ -3,6 +3,7 @@ #include #include +#include #include #include #include @@ -36,6 +37,7 @@ int cpu_suspend(unsigned long arg, int (*fn)(unsigned long)) cpu_switch_mm(mm->pgd, mm); local_flush_bp_all(); local_flush_tlb_all(); + check_other_bugs(); } return ret; -- 2.7.4 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 03/14] ARM: bugs: hook processor bug checking into SMP and suspend paths
On Wed, May 16, 2018 at 09:23:01AM -0700, Florian Fainelli wrote: > On 05/16/2018 04:00 AM, Russell King wrote: > > diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c > > index 2da087926ebe..5ad0b67b9e33 100644 > > --- a/arch/arm/kernel/smp.c > > +++ b/arch/arm/kernel/smp.c > > @@ -31,6 +31,7 @@ > > #include > > > > #include > > +#include > > #include > > #include > > #include > > @@ -405,6 +406,9 @@ asmlinkage void secondary_start_kernel(void) > > * before we continue - which happens after __cpu_up returns. > > */ > > set_cpu_online(cpu, true); > > + > > + check_other_bugs(); > > Given what is currently implemented, I don't think the location of > check_other_bugs() matters too much, but we might have to move this > after the local_irq_enable() at some point if we need to check for e.g: > a bogus local timer or whatever? We could move it later if we need to. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up According to speedtest.net: 8.21Mbps down 510kbps up ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 03/14] ARM: bugs: hook processor bug checking into SMP and suspend paths
On 05/16/2018 04:00 AM, Russell King wrote: > Check for CPU bugs when secondary processors are being brought online, > and also when CPUs are resuming from a low power mode. This gives an > opportunity to check that processor specific bug workarounds are > correctly enabled for all paths that a CPU re-enters the kernel. > > Signed-off-by: Russell KingReviewed-by: Florian Fainelli > --- > arch/arm/include/asm/bugs.h | 2 ++ > arch/arm/kernel/bugs.c | 5 + > arch/arm/kernel/smp.c | 4 > arch/arm/kernel/suspend.c | 2 ++ > 4 files changed, 13 insertions(+) > > diff --git a/arch/arm/include/asm/bugs.h b/arch/arm/include/asm/bugs.h > index ed122d294f3f..73a99c72a930 100644 > --- a/arch/arm/include/asm/bugs.h > +++ b/arch/arm/include/asm/bugs.h > @@ -14,8 +14,10 @@ extern void check_writebuffer_bugs(void); > > #ifdef CONFIG_MMU > extern void check_bugs(void); > +extern void check_other_bugs(void); > #else > #define check_bugs() do { } while (0) > +#define check_other_bugs() do { } while (0) > #endif > > #endif > diff --git a/arch/arm/kernel/bugs.c b/arch/arm/kernel/bugs.c > index 88024028bb70..16e7ba2a9cc4 100644 > --- a/arch/arm/kernel/bugs.c > +++ b/arch/arm/kernel/bugs.c > @@ -3,7 +3,12 @@ > #include > #include > > +void check_other_bugs(void) > +{ > +} > + > void __init check_bugs(void) > { > check_writebuffer_bugs(); > + check_other_bugs(); > } > diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c > index 2da087926ebe..5ad0b67b9e33 100644 > --- a/arch/arm/kernel/smp.c > +++ b/arch/arm/kernel/smp.c > @@ -31,6 +31,7 @@ > #include > > #include > +#include > #include > #include > #include > @@ -405,6 +406,9 @@ asmlinkage void secondary_start_kernel(void) >* before we continue - which happens after __cpu_up returns. >*/ > set_cpu_online(cpu, true); > + > + check_other_bugs(); Given what is currently implemented, I don't think the location of check_other_bugs() matters too much, but we might have to move this after the local_irq_enable() at some point if we need to check for e.g: a bogus local timer or whatever? -- Florian ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH 03/14] ARM: bugs: hook processor bug checking into SMP and suspend paths
Check for CPU bugs when secondary processors are being brought online, and also when CPUs are resuming from a low power mode. This gives an opportunity to check that processor specific bug workarounds are correctly enabled for all paths that a CPU re-enters the kernel. Signed-off-by: Russell King--- arch/arm/include/asm/bugs.h | 2 ++ arch/arm/kernel/bugs.c | 5 + arch/arm/kernel/smp.c | 4 arch/arm/kernel/suspend.c | 2 ++ 4 files changed, 13 insertions(+) diff --git a/arch/arm/include/asm/bugs.h b/arch/arm/include/asm/bugs.h index ed122d294f3f..73a99c72a930 100644 --- a/arch/arm/include/asm/bugs.h +++ b/arch/arm/include/asm/bugs.h @@ -14,8 +14,10 @@ extern void check_writebuffer_bugs(void); #ifdef CONFIG_MMU extern void check_bugs(void); +extern void check_other_bugs(void); #else #define check_bugs() do { } while (0) +#define check_other_bugs() do { } while (0) #endif #endif diff --git a/arch/arm/kernel/bugs.c b/arch/arm/kernel/bugs.c index 88024028bb70..16e7ba2a9cc4 100644 --- a/arch/arm/kernel/bugs.c +++ b/arch/arm/kernel/bugs.c @@ -3,7 +3,12 @@ #include #include +void check_other_bugs(void) +{ +} + void __init check_bugs(void) { check_writebuffer_bugs(); + check_other_bugs(); } diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c index 2da087926ebe..5ad0b67b9e33 100644 --- a/arch/arm/kernel/smp.c +++ b/arch/arm/kernel/smp.c @@ -31,6 +31,7 @@ #include #include +#include #include #include #include @@ -405,6 +406,9 @@ asmlinkage void secondary_start_kernel(void) * before we continue - which happens after __cpu_up returns. */ set_cpu_online(cpu, true); + + check_other_bugs(); + complete(_running); local_irq_enable(); diff --git a/arch/arm/kernel/suspend.c b/arch/arm/kernel/suspend.c index a40ebb7c0896..d08099269e35 100644 --- a/arch/arm/kernel/suspend.c +++ b/arch/arm/kernel/suspend.c @@ -3,6 +3,7 @@ #include #include +#include #include #include #include @@ -36,6 +37,7 @@ int cpu_suspend(unsigned long arg, int (*fn)(unsigned long)) cpu_switch_mm(mm->pgd, mm); local_flush_bp_all(); local_flush_tlb_all(); + check_other_bugs(); } return ret; -- 2.7.4 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm