[PATCH 0/6] Random collection of hyp-boot fixes and updates

[PATCH 0/6] Random collection of hyp-boot fixes and updates

[Will Deacon]

Hello,

I've been collecting random patches to the hyp-boot code, but it's
reached the point where much of it should go in as fixes for 3.8, CC'd
to stable. I've included the full series here, with the fixes marked
for stable.

I plan to send the fixes to Russell next week and then the three updates
later for 3.9.

All feedback welcome,

Will


Dave Martin (1):
  ARM: virt: Avoid bx instruction for compatibility with <=ARMv4

Marc Zyngier (2):
  ARM: virt: boot secondary CPUs through the right entry point
  ARM: virt: simplify __hyp_stub_install epilog

Russell King (1):
  ARM: virt: avoid clobbering lr when forcing svc mode

Will Deacon (2):
  ARM: virt: use PSR_N_BIT for detecting boot CPU mode mismatch
  ARM: virt: hide CONFIG_ARM_VIRT_EXT from user

 arch/arm/include/asm/assembler.h | 10 +++-------
 arch/arm/include/asm/virt.h      |  4 ++--
 arch/arm/kernel/head.S           |  2 +-
 arch/arm/kernel/hyp-stub.S       | 18 ++++++------------
 arch/arm/mm/Kconfig              | 10 +++-------
 5 files changed, 15 insertions(+), 29 deletions(-)

-- 
1.8.0

[PATCH 1/6] ARM: virt: Avoid bx instruction for compatibility with <=ARMv4

[Will Deacon]

From: Dave Martin <dave.martin@linaro.org>

Non-T variants of ARMv4 do not support the bx instruction.

However, __hyp_stub_install is always called from the same
instruction set used to build the bulk of the kernel, so bx should
not be necessary.

This patch uses the traditional "mov pc" instead of bx.

Cc: <stable@vger.kernel.org>
Signed-off-by: Dave Martin <dave.martin@linaro.org>
[will: fixed up remaining bx instruction]
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/kernel/hyp-stub.S | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/kernel/hyp-stub.S b/arch/arm/kernel/hyp-stub.S
index 65b2417..3c60256 100644
--- a/arch/arm/kernel/hyp-stub.S
+++ b/arch/arm/kernel/hyp-stub.S
@@ -99,7 +99,7 @@ ENTRY(__hyp_stub_install_secondary)
 	 * immediately.
 	 */
 	compare_cpu_mode_with_primary	r4, r5, r6, r7
-	bxne	lr
+	movne	pc, lr
 
 	/*
 	 * Once we have given up on one CPU, we do not try to install the
@@ -111,7 +111,7 @@ ENTRY(__hyp_stub_install_secondary)
 	 */
 
 	cmp	r4, #HYP_MODE
-	bxne	lr			@ give up if the CPU is not in HYP mode
+	movne	pc, lr			@ give up if the CPU is not in HYP mode
 
 /*
  * Configure HSCTLR to set correct exception endianness/instruction set
@@ -200,7 +200,7 @@ ENDPROC(__hyp_get_vectors)
 	@ fall through
 ENTRY(__hyp_set_vectors)
 	__HVC(0)
-	bx	lr
+	mov	pc, lr
 ENDPROC(__hyp_set_vectors)
 
 #ifndef ZIMAGE
-- 
1.8.0

[PATCH 2/6] ARM: virt: boot secondary CPUs through the right entry point

[Will Deacon]

From: Marc Zyngier <marc.zyngier@arm.com>

Secondary CPUs should use the __hyp_stub_install_secondary entry
point, so boot mode inconsistencies can be detected.

Cc: <stable@vger.kernel.org>
Acked-by: Dave Martin <dave.martin@linaro.org>
Reported-by: Ian Molton <ian.molton@collabora.co.uk>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/kernel/head.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
index 4eee351..16abc83 100644
--- a/arch/arm/kernel/head.S
+++ b/arch/arm/kernel/head.S
@@ -331,7 +331,7 @@ ENTRY(secondary_startup)
 	 * as it has already been validated by the primary processor.
 	 */
 #ifdef CONFIG_ARM_VIRT_EXT
-	bl	__hyp_stub_install
+	bl	__hyp_stub_install_secondary
 #endif
 	safe_svcmode_maskall r9
 
-- 
1.8.0

[PATCH 3/6] ARM: virt: simplify __hyp_stub_install epilog

[Will Deacon]

From: Marc Zyngier <marc.zyngier@arm.com>

__hyp_stub_install duplicates quite a bit of safe_svcmode_maskall
by forcing the CPU back to SVC. This is unnecessary, as
safe_svcmode_maskall is called just after.

Furthermore, the way we build SPSR_hyp is buggy as we fail to mask
the interrupts, leading to interesting behaviours on TC2 + UEFI.

The fix is to simply remove this code and rely on safe_svcmode_maskall
to do the right thing.

Cc: <stable@vger.kernel.org>
Reviewed-by: Dave Martin <dave.martin@linaro.org>
Reported-by: Harry Liebel <harry.liebel@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/kernel/hyp-stub.S | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/arch/arm/kernel/hyp-stub.S b/arch/arm/kernel/hyp-stub.S
index 3c60256..1315c4c 100644
--- a/arch/arm/kernel/hyp-stub.S
+++ b/arch/arm/kernel/hyp-stub.S
@@ -120,7 +120,8 @@ ENTRY(__hyp_stub_install_secondary)
  * Eventually, CPU-specific code might be needed -- assume not for now
  *
  * This code relies on the "eret" instruction to synchronize the
- * various coprocessor accesses.
+ * various coprocessor accesses. This is done when we switch to SVC
+ * (see safe_svcmode_maskall).
  */
 	@ Now install the hypervisor stub:
 	adr	r7, __hyp_stub_vectors
@@ -155,14 +156,7 @@ THUMB(	orr	r7, #(1 << 30)	)	@ HSCTLR.TE
 1:
 #endif
 
-	bic	r7, r4, #MODE_MASK
-	orr	r7, r7, #SVC_MODE
-THUMB(	orr	r7, r7, #PSR_T_BIT	)
-	msr	spsr_cxsf, r7		@ This is SPSR_hyp.
-
-	__MSR_ELR_HYP(14)		@ msr elr_hyp, lr
-	__ERET				@ return, switching to SVC mode
-					@ The boot CPU mode is left in r4.
+	bx	lr			@ The boot CPU mode is left in r4.
 ENDPROC(__hyp_stub_install_secondary)
 
 __hyp_stub_do_trap:
-- 
1.8.0

[PATCH 4/6] ARM: virt: avoid clobbering lr when forcing svc mode

[Will Deacon]

From: Russell King <rmk+kernel@arm.linux.org.uk>

The safe_svcmode_maskall macro is used to ensure that we are running in
svc mode, causing an exception return from hvc mode if required.

This patch removes the unneeded lr clobber from the macro and operates
entirely on the temporary parameter register instead.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
[will: updated comment]
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/include/asm/assembler.h | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
index eb87200..05ee9ee 100644
--- a/arch/arm/include/asm/assembler.h
+++ b/arch/arm/include/asm/assembler.h
@@ -246,18 +246,14 @@
  *
  * This macro is intended for forcing the CPU into SVC mode at boot time.
  * you cannot return to the original mode.
- *
- * Beware, it also clobers LR.
  */
 .macro safe_svcmode_maskall reg:req
 #if __LINUX_ARM_ARCH__ >= 6
 	mrs	\reg , cpsr
-	mov	lr , \reg
-	and	lr , lr , #MODE_MASK
-	cmp	lr , #HYP_MODE
-	orr	\reg , \reg , #PSR_I_BIT | PSR_F_BIT
+	eor	\reg, \reg, #HYP_MODE
+	tst	\reg, #MODE_MASK
 	bic	\reg , \reg , #MODE_MASK
-	orr	\reg , \reg , #SVC_MODE
+	orr	\reg , \reg , #PSR_I_BIT | PSR_F_BIT | SVC_MODE
 THUMB(	orr	\reg , \reg , #PSR_T_BIT	)
 	bne	1f
 	orr	\reg, \reg, #PSR_A_BIT
-- 
1.8.0

[PATCH 5/6] ARM: virt: use PSR_N_BIT for detecting boot CPU mode mismatch

[Will Deacon]

During boot, we detect whether or not all CPUs are brought up in the
same mode and signal this to the kernel using the N bit in the SPSR.

This patch tidies up the checking code to use the PSR_N_BIT macro,
rather than hardcoding the bit field and commenting it as such.

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/include/asm/virt.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/virt.h b/arch/arm/include/asm/virt.h
index 86164df..50af92b 100644
--- a/arch/arm/include/asm/virt.h
+++ b/arch/arm/include/asm/virt.h
@@ -24,9 +24,9 @@
 /*
  * Flag indicating that the kernel was not entered in the same mode on every
  * CPU.  The zImage loader stashes this value in an SPSR, so we need an
- * architecturally defined flag bit here (the N flag, as it happens)
+ * architecturally defined flag bit here.
  */
-#define BOOT_CPU_MODE_MISMATCH (1<<31)
+#define BOOT_CPU_MODE_MISMATCH	PSR_N_BIT
 
 #ifndef __ASSEMBLY__
 
-- 
1.8.0

[PATCH 6/6] ARM: virt: hide CONFIG_ARM_VIRT_EXT from user

[Will Deacon]

ARM_VIRT_EXT is a property of CPU_V7, but does not adversely affect
other CPUs that can be built into the same kernel image (i.e. ARMv6+).

This patch defaults ARM_VIRT_EXT to y if CPU_V7, allowing hypervisors
such as KVM to make better use of the option and being able to rely
on hyp-mode boot support.

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/mm/Kconfig | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
index 3fd629d..025d173 100644
--- a/arch/arm/mm/Kconfig
+++ b/arch/arm/mm/Kconfig
@@ -629,8 +629,9 @@ config ARM_THUMBEE
 	  make use of it. Say N for code that can run on CPUs without ThumbEE.
 
 config ARM_VIRT_EXT
-	bool "Native support for the ARM Virtualization Extensions"
-	depends on MMU && CPU_V7
+	bool
+	depends on MMU
+	default y if CPU_V7
 	help
 	  Enable the kernel to make use of the ARM Virtualization
 	  Extensions to install hypervisors without run-time firmware
@@ -640,11 +641,6 @@ config ARM_VIRT_EXT
 	  use of this feature.  Refer to Documentation/arm/Booting for
 	  details.
 
-	  It is safe to enable this option even if the kernel may not be
-	  booted in HYP mode, may not have support for the
-	  virtualization extensions, or may be booted with a
-	  non-compliant bootloader.
-
 config SWP_EMULATE
 	bool "Emulate SWP/SWPB instructions"
 	depends on !CPU_USE_DOMAINS && CPU_V7
-- 
1.8.0

[PATCH 6/6] ARM: virt: hide CONFIG_ARM_VIRT_EXT from user

[Christopher Covington]

Hi Will,

On 01/11/2013 10:34 AM, Will Deacon wrote:
> ARM_VIRT_EXT is a property of CPU_V7, but does not adversely affect
> other CPUs that can be built into the same kernel image (i.e. ARMv6+).
> 
> This patch defaults ARM_VIRT_EXT to y if CPU_V7, allowing hypervisors
> such as KVM to make better use of the option and being able to rely
> on hyp-mode boot support.

[...]

> @@ -640,11 +641,6 @@ config ARM_VIRT_EXT
>  	  use of this feature.  Refer to Documentation/arm/Booting for
>  	  details.
>  
> -	  It is safe to enable this option even if the kernel may not be
> -	  booted in HYP mode, may not have support for the
> -	  virtualization extensions, or may be booted with a
> -	  non-compliant bootloader.

Why take this out?

[...]

Regards,
Christopher

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by
the Linux Foundation

[PATCH 6/6] ARM: virt: hide CONFIG_ARM_VIRT_EXT from user

[Will Deacon]

On Fri, Jan 11, 2013 at 03:42:40PM +0000, Christopher Covington wrote:
> Hi Will,

Hello,

> On 01/11/2013 10:34 AM, Will Deacon wrote:
> > ARM_VIRT_EXT is a property of CPU_V7, but does not adversely affect
> > other CPUs that can be built into the same kernel image (i.e. ARMv6+).
> > 
> > This patch defaults ARM_VIRT_EXT to y if CPU_V7, allowing hypervisors
> > such as KVM to make better use of the option and being able to rely
> > on hyp-mode boot support.
> 
> [...]
> 
> > @@ -640,11 +641,6 @@ config ARM_VIRT_EXT
> >  	  use of this feature.  Refer to Documentation/arm/Booting for
> >  	  details.
> >  
> > -	  It is safe to enable this option even if the kernel may not be
> > -	  booted in HYP mode, may not have support for the
> > -	  virtualization extensions, or may be booted with a
> > -	  non-compliant bootloader.
> 
> Why take this out?

I just removed that last paragraph because it's not user-selectable anymore,
so this comment about enabling the option is redundant. I could remove all
of the help text, but we have it for some other options that are selected
automatically and it has a pointer to some documentation too.

Will

[PATCH 6/6] ARM: virt: hide CONFIG_ARM_VIRT_EXT from user

[Christopher Covington]

On 01/11/2013 11:43 AM, Will Deacon wrote:
> On Fri, Jan 11, 2013 at 03:42:40PM +0000, Christopher Covington wrote:
>> Hi Will,
> 
> Hello,
> 
>> On 01/11/2013 10:34 AM, Will Deacon wrote:
>>> ARM_VIRT_EXT is a property of CPU_V7, but does not adversely affect
>>> other CPUs that can be built into the same kernel image (i.e. ARMv6+).
>>>
>>> This patch defaults ARM_VIRT_EXT to y if CPU_V7, allowing hypervisors
>>> such as KVM to make better use of the option and being able to rely
>>> on hyp-mode boot support.
>>
>> [...]
>>
>>> @@ -640,11 +641,6 @@ config ARM_VIRT_EXT
>>>  	  use of this feature.  Refer to Documentation/arm/Booting for
>>>  	  details.
>>>  
>>> -	  It is safe to enable this option even if the kernel may not be
>>> -	  booted in HYP mode, may not have support for the
>>> -	  virtualization extensions, or may be booted with a
>>> -	  non-compliant bootloader.
>>
>> Why take this out?
> 
> I just removed that last paragraph because it's not user-selectable anymore,
> so this comment about enabling the option is redundant. I could remove all
> of the help text, but we have it for some other options that are selected
> automatically and it has a pointer to some documentation too.

It's not a big deal, but that paragraph helped me understand the (lack of)
ramifications for v7 CPU's without the virtualization extensions, and I figure
it might still be helpful for others as well.

Christopher

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by
the Linux Foundation