Re: [PATCH 11/13] arm64: Panic when VHE and non VHE CPUs coexist

2015-08-06 Thread Catalin Marinas 
On Wed, Jul 08, 2015 at 05:19:14PM +0100, Marc Zyngier wrote:
 Having both VHE and non-VHE capable CPUs in the same system
 is likely to be a recipe for disaster.
 
 If the boot CPU has VHE, but a secondary is not, we won't be
 able to downgrade and run the kernel at EL1. Add CPU hotplug
 to the mix, and this produces a terrifying mess.
 
 Let's solve the problem once and for all. If you mix VHE and
 non-VHE CPUs in the same system, you deserve to loose, and this
 patch makes sure you don't get a chance.
 
 This is implemented by storing the kernel execution level in
 a global variable. Secondaries will park themselves in a
 WFI loop if they observe a mismatch. Also, the primary CPU
 will detect that the secondary CPU has died on a mismatched
 execution level. Panic will follow.

We don't have such system yet but we could try it on the model. Do you
know how far the secondary CPU goes? If it gets to the cpuid sanity
checking, we could do the check together with the rest of the features
(we could add a new CHECK_FATAL macro to make this even more obvious):

diff --git a/arch/arm64/include/asm/cpu.h b/arch/arm64/include/asm/cpu.h
index 8e797b2fcc01..36cc81c9b9df 100644
--- a/arch/arm64/include/asm/cpu.h
+++ b/arch/arm64/include/asm/cpu.h
@@ -29,6 +29,7 @@ struct cpuinfo_arm64 {
u32 reg_cntfrq;
u32 reg_dczid;
u32 reg_midr;
+   u32 reg_currentel;
 
u64 reg_id_aa64dfr0;
u64 reg_id_aa64dfr1;
diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
index ee6403df9fe4..de3e8903b5a8 100644
--- a/arch/arm64/include/asm/cputype.h
+++ b/arch/arm64/include/asm/cputype.h
@@ -118,6 +118,14 @@ static inline bool id_aa64mmfr0_mixed_endian_el0(u64 mmfr0)
return (ID_AA64MMFR0_BIGEND(mmfr0) == 0x1) ||
(ID_AA64MMFR0_BIGENDEL0(mmfr0) == 0x1);
 }
+
+static inline unsigned int read_currentel(void)
+{
+   u64 el;
+   asm(mrs%0, CurrentEL : =r (el));
+   return el;
+}
+
 #endif /* __ASSEMBLY__ */
 
 #endif
diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
index 75d5a867e7fb..c0269210fd54 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -132,6 +132,9 @@ static void cpuinfo_sanity_check(struct cpuinfo_arm64 *cur)
/* If different, timekeeping will be broken (especially with KVM) */
diff |= CHECK(cntfrq, boot, cur, cpu);
 
+   /* Same EL for all CPUs */
+   diff |= CHECK(currentel, boot, cur, cpu);
+
/*
 * The kernel uses self-hosted debug features and expects CPUs to
 * support identical debug features. We presently need CTX_CMPs, WRPs,
@@ -205,6 +208,7 @@ static void __cpuinfo_store_cpu(struct cpuinfo_arm64 *info)
info-reg_ctr = read_cpuid_cachetype();
info-reg_dczid = read_cpuid(DCZID_EL0);
info-reg_midr = read_cpuid_id();
+   info-reg_currentel = read_currentel();
 
info-reg_id_aa64dfr0 = read_cpuid(ID_AA64DFR0_EL1);
info-reg_id_aa64dfr1 = read_cpuid(ID_AA64DFR1_EL1);
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 11/13] arm64: Panic when VHE and non VHE CPUs coexist

2015-07-16 Thread Will Deacon 
On Wed, Jul 08, 2015 at 05:19:14PM +0100, Marc Zyngier wrote:
 Having both VHE and non-VHE capable CPUs in the same system
 is likely to be a recipe for disaster.
 
 If the boot CPU has VHE, but a secondary is not, we won't be
 able to downgrade and run the kernel at EL1. Add CPU hotplug
 to the mix, and this produces a terrifying mess.
 
 Let's solve the problem once and for all. If you mix VHE and
 non-VHE CPUs in the same system, you deserve to loose, and this
 patch makes sure you don't get a chance.
 
 This is implemented by storing the kernel execution level in
 a global variable. Secondaries will park themselves in a
 WFI loop if they observe a mismatch. Also, the primary CPU
 will detect that the secondary CPU has died on a mismatched
 execution level. Panic will follow.
 
 Signed-off-by: Marc Zyngier marc.zyng...@arm.com
 ---
  arch/arm64/include/asm/virt.h | 15 +++
  arch/arm64/kernel/head.S  | 15 +++
  arch/arm64/kernel/smp.c   |  4 
  3 files changed, 34 insertions(+)
 
 diff --git a/arch/arm64/include/asm/virt.h b/arch/arm64/include/asm/virt.h
 index 9f22dd6..8e246f7 100644
 --- a/arch/arm64/include/asm/virt.h
 +++ b/arch/arm64/include/asm/virt.h
 @@ -36,6 +36,11 @@
   */
  extern u32 __boot_cpu_mode[2];
  
 +/*
 + * __run_cpu_mode records the mode the boot CPU uses for the kernel.
 + */
 +extern u32 __run_cpu_mode;
 +
  void __hyp_set_vectors(phys_addr_t phys_vector_base);
  phys_addr_t __hyp_get_vectors(void);
  
 @@ -60,6 +65,16 @@ static inline bool is_kernel_in_hyp_mode(void)
   return el == CurrentEL_EL2;
  }
  
 +static inline bool is_kernel_mode_mismatched(void)
 +{
 + u64 el;
 + u32 mode;
 +
 + asm(mrs %0, CurrentEL : =r (el));
 + mode = ACCESS_ONCE(__run_cpu_mode);
 + return el != mode;

Why the temporary 'mode' variable?

 +}
 +
  /* The section containing the hypervisor text */
  extern char __hyp_text_start[];
  extern char __hyp_text_end[];
 diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
 index a179747..318e69f 100644
 --- a/arch/arm64/kernel/head.S
 +++ b/arch/arm64/kernel/head.S
 @@ -578,7 +578,20 @@ ENTRY(set_cpu_boot_mode_flag)
  1:   str w20, [x1]   // This CPU has booted in EL1
   dmb sy
   dc  ivac, x1// Invalidate potentially stale 
 cache line
 + adr_l   x1, __run_cpu_mode
 + ldr w0, [x1]
 + mrs x20, CurrentEL

Why x20?

 + str w20, [x1]
 + dmb sy
 + dc  ivac, x1// Invalidate potentially stale 
 cache line

Can we stick __run_cpu_mode and __boot_cpu_mode into a struct in the same
cacheline and avoid the extra maintenance?

 + cbz x0, skip_el_check

w0?

 + cmp x0, x20

w0, w20?

 + bne mismatched_el
 +skip_el_check:
   ret
 +mismatched_el:
 + wfi
 + b   mismatched_el
  ENDPROC(set_cpu_boot_mode_flag)
  
  /*
 @@ -593,6 +606,8 @@ ENDPROC(set_cpu_boot_mode_flag)
  ENTRY(__boot_cpu_mode)
   .long   BOOT_CPU_MODE_EL2
   .long   BOOT_CPU_MODE_EL1
 +ENTRY(__run_cpu_mode)
 + .long   0
   .popsection
  
  #ifdef CONFIG_SMP
 diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
 index 695801a..b467f51 100644
 --- a/arch/arm64/kernel/smp.c
 +++ b/arch/arm64/kernel/smp.c
 @@ -52,6 +52,7 @@
  #include asm/sections.h
  #include asm/tlbflush.h
  #include asm/ptrace.h
 +#include asm/virt.h
  
  #define CREATE_TRACE_POINTS
  #include trace/events/ipi.h
 @@ -112,6 +113,9 @@ int __cpu_up(unsigned int cpu, struct task_struct *idle)
   pr_crit(CPU%u: failed to come online\n, cpu);
   ret = -EIO;
   }
 +
 + if (is_kernel_mode_mismatched())
 + panic(CPU%u: incompatible execution level, cpu);

Might be useful to print the incompatible values, if possible.

Will
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 11/13] arm64: Panic when VHE and non VHE CPUs coexist

2015-07-08 Thread Marc Zyngier 
Having both VHE and non-VHE capable CPUs in the same system
is likely to be a recipe for disaster.

If the boot CPU has VHE, but a secondary is not, we won't be
able to downgrade and run the kernel at EL1. Add CPU hotplug
to the mix, and this produces a terrifying mess.

Let's solve the problem once and for all. If you mix VHE and
non-VHE CPUs in the same system, you deserve to loose, and this
patch makes sure you don't get a chance.

This is implemented by storing the kernel execution level in
a global variable. Secondaries will park themselves in a
WFI loop if they observe a mismatch. Also, the primary CPU
will detect that the secondary CPU has died on a mismatched
execution level. Panic will follow.

Signed-off-by: Marc Zyngier marc.zyng...@arm.com
---
 arch/arm64/include/asm/virt.h | 15 +++
 arch/arm64/kernel/head.S  | 15 +++
 arch/arm64/kernel/smp.c   |  4 
 3 files changed, 34 insertions(+)

diff --git a/arch/arm64/include/asm/virt.h b/arch/arm64/include/asm/virt.h
index 9f22dd6..8e246f7 100644
--- a/arch/arm64/include/asm/virt.h
+++ b/arch/arm64/include/asm/virt.h
@@ -36,6 +36,11 @@
  */
 extern u32 __boot_cpu_mode[2];
 
+/*
+ * __run_cpu_mode records the mode the boot CPU uses for the kernel.
+ */
+extern u32 __run_cpu_mode;
+
 void __hyp_set_vectors(phys_addr_t phys_vector_base);
 phys_addr_t __hyp_get_vectors(void);
 
@@ -60,6 +65,16 @@ static inline bool is_kernel_in_hyp_mode(void)
return el == CurrentEL_EL2;
 }
 
+static inline bool is_kernel_mode_mismatched(void)
+{
+   u64 el;
+   u32 mode;
+
+   asm(mrs %0, CurrentEL : =r (el));
+   mode = ACCESS_ONCE(__run_cpu_mode);
+   return el != mode;
+}
+
 /* The section containing the hypervisor text */
 extern char __hyp_text_start[];
 extern char __hyp_text_end[];
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index a179747..318e69f 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -578,7 +578,20 @@ ENTRY(set_cpu_boot_mode_flag)
 1: str w20, [x1]   // This CPU has booted in EL1
dmb sy
dc  ivac, x1// Invalidate potentially stale 
cache line
+   adr_l   x1, __run_cpu_mode
+   ldr w0, [x1]
+   mrs x20, CurrentEL
+   str w20, [x1]
+   dmb sy
+   dc  ivac, x1// Invalidate potentially stale 
cache line
+   cbz x0, skip_el_check
+   cmp x0, x20
+   bne mismatched_el
+skip_el_check:
ret
+mismatched_el:
+   wfi
+   b   mismatched_el
 ENDPROC(set_cpu_boot_mode_flag)
 
 /*
@@ -593,6 +606,8 @@ ENDPROC(set_cpu_boot_mode_flag)
 ENTRY(__boot_cpu_mode)
.long   BOOT_CPU_MODE_EL2
.long   BOOT_CPU_MODE_EL1
+ENTRY(__run_cpu_mode)
+   .long   0
.popsection
 
 #ifdef CONFIG_SMP
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 695801a..b467f51 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -52,6 +52,7 @@
 #include asm/sections.h
 #include asm/tlbflush.h
 #include asm/ptrace.h
+#include asm/virt.h
 
 #define CREATE_TRACE_POINTS
 #include trace/events/ipi.h
@@ -112,6 +113,9 @@ int __cpu_up(unsigned int cpu, struct task_struct *idle)
pr_crit(CPU%u: failed to come online\n, cpu);
ret = -EIO;
}
+
+   if (is_kernel_mode_mismatched())
+   panic(CPU%u: incompatible execution level, cpu);
} else {
pr_err(CPU%u: failed to boot: %d\n, cpu, ret);
}
-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html