[PATCH] powerpc/64s: support nospectre_v2 cmdline option

Add support for disabling the kernel implemented spectre v2 mitigation
(count cache flush on context switch) via the nospectre_v2 cmdline
option.

Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Christopher M. Riedl <redacted>
---
reference: https://github.com/linuxppc/issues/issues/236

 arch/powerpc/kernel/security.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/security.c b/arch/powerpc/kernel/security.c
index b33bafb8fcea..f7c34745cd0f 100644
--- a/arch/powerpc/kernel/security.c
+++ b/arch/powerpc/kernel/security.c

@@ -28,7 +28,7 @@ static enum count_cache_flush_type count_cache_flush_type = COUNT_CACHE_FLUSH_NO
 bool barrier_nospec_enabled;
 static bool no_nospec;
 static bool btb_flush_enabled;
-#ifdef CONFIG_PPC_FSL_BOOK3E
+#if defined(CONFIG_PPC_FSL_BOOK3E) || defined(CONFIG_PPC_BOOK3S_64)
 static bool no_spectrev2;
 #endif
 

@@ -106,7 +106,7 @@ static __init int barrier_nospec_debugfs_init(void)
 device_initcall(barrier_nospec_debugfs_init);
 #endif /* CONFIG_DEBUG_FS */
 
-#ifdef CONFIG_PPC_FSL_BOOK3E
+#if defined(CONFIG_PPC_FSL_BOOK3E) || defined(CONFIG_PPC_BOOK3S_64)
 static int __init handle_nospectre_v2(char *p)
 {
 	no_spectrev2 = true;

@@ -114,6 +114,9 @@ static int __init handle_nospectre_v2(char *p)
 	return 0;
 }
 early_param("nospectre_v2", handle_nospectre_v2);
+#endif /* CONFIG_PPC_FSL_BOOK3E || CONFIG_PPC_BOOK3S_64 */
+
+#ifdef CONFIG_PPC_FSL_BOOK3E
 void setup_spectre_v2(void)
 {
 	if (no_spectrev2)

@@ -391,6 +394,13 @@ static void toggle_count_cache_flush(bool enable)
 
 void setup_count_cache_flush(void)
 {
+	if (no_spectrev2) {
+		if (security_ftr_enabled(SEC_FTR_BCCTRL_SERIALISED)
+		    || security_ftr_enabled(SEC_FTR_COUNT_CACHE_DISABLED))
+			pr_warn("Spectre v2 mitigations not under software control, can't disable\n");
+		return;
+	}
+
 	toggle_count_cache_flush(true);
 }
 

-- 
2.21.0

On 6/5/19 8:10 am, Christopher M. Riedl wrote:

If one of those ftrs is set, what's the impact of not calling 
toggle_count_cache_flush()?

+		return;
+	}
+
  	toggle_count_cache_flush(true);
  }
  

-- 
Andrew Donnellan              OzLabs, ADL Canberra
ajd@linux.ibm.com             IBM Australia Limited

On May 5, 2019 at 9:32 PM Andrew Donnellan [off-list ref] wrote:


On 6/5/19 8:10 am, Christopher M. Riedl wrote:

quoted

Add support for disabling the kernel implemented spectre v2 mitigation
(count cache flush on context switch) via the nospectre_v2 cmdline
option.

Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Christopher M. Riedl <redacted>
---

  arch/powerpc/kernel/security.c | 14 ++++++++++++--
  1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/security.c b/arch/powerpc/kernel/security.c
index b33bafb8fcea..f7c34745cd0f 100644
--- a/arch/powerpc/kernel/security.c
+++ b/arch/powerpc/kernel/security.c

@@ -28,7 +28,7 @@ static enum count_cache_flush_type count_cache_flush_type = COUNT_CACHE_FLUSH_NO
  bool barrier_nospec_enabled;
  static bool no_nospec;
  static bool btb_flush_enabled;
-#ifdef CONFIG_PPC_FSL_BOOK3E
+#if defined(CONFIG_PPC_FSL_BOOK3E) || defined(CONFIG_PPC_BOOK3S_64)
  static bool no_spectrev2;
  #endif
  

@@ -106,7 +106,7 @@ static __init int barrier_nospec_debugfs_init(void)
  device_initcall(barrier_nospec_debugfs_init);
  #endif /* CONFIG_DEBUG_FS */
  
-#ifdef CONFIG_PPC_FSL_BOOK3E
+#if defined(CONFIG_PPC_FSL_BOOK3E) || defined(CONFIG_PPC_BOOK3S_64)
  static int __init handle_nospectre_v2(char *p)
  {
  	no_spectrev2 = true;

@@ -114,6 +114,9 @@ static int __init handle_nospectre_v2(char *p)
  	return 0;
  }
  early_param("nospectre_v2", handle_nospectre_v2);
+#endif /* CONFIG_PPC_FSL_BOOK3E || CONFIG_PPC_BOOK3S_64 */
+
+#ifdef CONFIG_PPC_FSL_BOOK3E
  void setup_spectre_v2(void)
  {
  	if (no_spectrev2)

@@ -391,6 +394,13 @@ static void toggle_count_cache_flush(bool enable)
  
  void setup_count_cache_flush(void)
  {
+	if (no_spectrev2) {
+		if (security_ftr_enabled(SEC_FTR_BCCTRL_SERIALISED)
+		    || security_ftr_enabled(SEC_FTR_COUNT_CACHE_DISABLED))
+			pr_warn("Spectre v2 mitigations not under software control, can't disable\n");

If one of those ftrs is set, what's the impact of not calling 
toggle_count_cache_flush()?

The patchsite/callsite (kernel/entry_64.S:597) for flush_count_cache
inside _switch remains a nop.

Disassembly of vmlinux after build:
c00000000000e260:       00 00 23 f8     std     r1,0(r3)
c00000000000e264:       00 00 00 60     nop
c00000000000e268:       00 60 c0 3c     lis     r6,24576

Disassembly (xmon) after boot/during runtime in qemu:
c00000000000e260  f8230000	std     r1,0(r3)
c00000000000e264  4bffdb7d	bl      c00000000000bde0	# flush_count_cache+0x0/0x2420
c00000000000e268  3cc06000	lis     r6,24576

Disassembly (xmon) after boot/during runtime in qemu w/ nospectre_v2:
c00000000000e260  f8230000	std     r1,0(r3)
c00000000000e264  60000000	nop
c00000000000e268  3cc06000	lis     r6,24576

toggle_count_cache_flush() well uhh "toggles" the patchsite to either
contain a branch to the flush procedure or a nop.

quoted

+		return;
+	}
+
  	toggle_count_cache_flush(true);
  }
  

-- 
Andrew Donnellan              OzLabs, ADL Canberra
ajd@linux.ibm.com             IBM Australia Limited

Christopher M Riedl [off-list ref] writes:

quoted

On May 5, 2019 at 9:32 PM Andrew Donnellan [off-list ref] wrote:
On 6/5/19 8:10 am, Christopher M. Riedl wrote:

quoted

Add support for disabling the kernel implemented spectre v2 mitigation
(count cache flush on context switch) via the nospectre_v2 cmdline
option.

Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Christopher M. Riedl <redacted>

diff --git a/arch/powerpc/kernel/security.c b/arch/powerpc/kernel/security.c
index b33bafb8fcea..f7c34745cd0f 100644
--- a/arch/powerpc/kernel/security.c
+++ b/arch/powerpc/kernel/security.c

@@ -391,6 +394,13 @@ static void toggle_count_cache_flush(bool enable)
  
  void setup_count_cache_flush(void)
  {
+	if (no_spectrev2) {
+		if (security_ftr_enabled(SEC_FTR_BCCTRL_SERIALISED)
+		    || security_ftr_enabled(SEC_FTR_COUNT_CACHE_DISABLED))
+			pr_warn("Spectre v2 mitigations not under software control, can't disable\n");

If one of those ftrs is set, what's the impact of not calling 
toggle_count_cache_flush()?

The patchsite/callsite (kernel/entry_64.S:597) for flush_count_cache
inside _switch remains a nop.

Disassembly of vmlinux after build:
c00000000000e260:       00 00 23 f8     std     r1,0(r3)
c00000000000e264:       00 00 00 60     nop
c00000000000e268:       00 60 c0 3c     lis     r6,24576

Disassembly (xmon) after boot/during runtime in qemu:
c00000000000e260  f8230000	std     r1,0(r3)
c00000000000e264  4bffdb7d	bl      c00000000000bde0	# flush_count_cache+0x0/0x2420
c00000000000e268  3cc06000	lis     r6,24576

Disassembly (xmon) after boot/during runtime in qemu w/ nospectre_v2:
c00000000000e260  f8230000	std     r1,0(r3)
c00000000000e264  60000000	nop
c00000000000e268  3cc06000	lis     r6,24576

Yes you're right, in this case the initial state is deactivated.

toggle_count_cache_flush() well uhh "toggles" the patchsite to either
contain a branch to the flush procedure or a nop.

Sort of. It doesn't actually know the initial state, so calling it at
boot with false will still nop out the nops.

I think I'd probably prefer it if we still call
toggle_count_cache_flush(false) when no_spectrev2 is true. The main
reason being that it means we'll print to dmesg, but it would also avoid
problems if we ever change the initial state of the count cache flush.

cheers

On May 6, 2019 at 9:29 PM Michael Ellerman [off-list ref] wrote:


Christopher M Riedl [off-list ref] writes:

quoted

quoted

On May 5, 2019 at 9:32 PM Andrew Donnellan [off-list ref] wrote:
On 6/5/19 8:10 am, Christopher M. Riedl wrote:

quoted

Add support for disabling the kernel implemented spectre v2 mitigation
(count cache flush on context switch) via the nospectre_v2 cmdline
option.

Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Christopher M. Riedl <redacted>

diff --git a/arch/powerpc/kernel/security.c b/arch/powerpc/kernel/security.c
index b33bafb8fcea..f7c34745cd0f 100644
--- a/arch/powerpc/kernel/security.c
+++ b/arch/powerpc/kernel/security.c

@@ -391,6 +394,13 @@ static void toggle_count_cache_flush(bool enable)
  
  void setup_count_cache_flush(void)
  {
+	if (no_spectrev2) {
+		if (security_ftr_enabled(SEC_FTR_BCCTRL_SERIALISED)
+		    || security_ftr_enabled(SEC_FTR_COUNT_CACHE_DISABLED))
+			pr_warn("Spectre v2 mitigations not under software control, can't disable\n");

If one of those ftrs is set, what's the impact of not calling 
toggle_count_cache_flush()?

The patchsite/callsite (kernel/entry_64.S:597) for flush_count_cache
inside _switch remains a nop.

Disassembly of vmlinux after build:
c00000000000e260:       00 00 23 f8     std     r1,0(r3)
c00000000000e264:       00 00 00 60     nop
c00000000000e268:       00 60 c0 3c     lis     r6,24576

Disassembly (xmon) after boot/during runtime in qemu:
c00000000000e260  f8230000	std     r1,0(r3)
c00000000000e264  4bffdb7d	bl      c00000000000bde0	# flush_count_cache+0x0/0x2420
c00000000000e268  3cc06000	lis     r6,24576

Disassembly (xmon) after boot/during runtime in qemu w/ nospectre_v2:
c00000000000e260  f8230000	std     r1,0(r3)
c00000000000e264  60000000	nop
c00000000000e268  3cc06000	lis     r6,24576

Yes you're right, in this case the initial state is deactivated.

quoted

toggle_count_cache_flush() well uhh "toggles" the patchsite to either
contain a branch to the flush procedure or a nop.

Sort of. It doesn't actually know the initial state, so calling it at
boot with false will still nop out the nops.

I think I'd probably prefer it if we still call
toggle_count_cache_flush(false) when no_spectrev2 is true. The main
reason being that it means we'll print to dmesg, but it would also avoid
problems if we ever change the initial state of the count cache flush.

cheers

Sounds good, I will add this to the next version. Thanks!