Re: [PATCH 03/14] ARM: bugs: hook processor bug checking into SMP and suspend paths

2018-05-25 Thread Russell King - ARM Linux
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

2018-05-24 Thread Florian Fainelli
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

2018-05-21 Thread Russell King
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 
---
 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

2018-05-19 Thread Russell King - ARM Linux
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

2018-05-17 Thread Florian Fainelli
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 King 

Reviewed-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

2018-05-16 Thread Russell King
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