[v2,2/7] ARM: virt: allow the kernel to be entered in HYP mode

Commit Message

Marc Zyngier Oct. 6, 2012, 10:18 a.m. UTC

Hi Tony,

On Fri, 5 Oct 2012 13:08:22 -0700, Tony Lindgren <[e-mail]> wrote:
> Hi,
> 
> * Marc Zyngier <[e-mail]> [120907 10:04]:
>> From: Dave Martin <[e-mail]>
>> 
>> This patch does two things:
>> 
>>   * Ensure that asynchronous aborts are masked at kernel entry.
>>     The bootloader should be masking these anyway, but this reduces
>>     the damage window just in case it doesn't.
>> 
>>   * Enter svc mode via exception return to ensure that CPU state is
>>     properly serialised.  This does not matter when switching from
>>     an ordinary privileged mode ("PL1" modes in ARMv7-AR rev C
>>     parlance), but it potentially does matter when switching from a
>>     another privileged mode such as hyp mode.
>> 
>> This should allow the kernel to boot safely either from svc mode or
>> hyp mode, even if no support for use of the ARM Virtualization
>> Extensions is built into the kernel.
>> 
>> Signed-off-by: Dave Martin <[e-mail]>
>> Signed-off-by: Marc Zyngier <[e-mail]>
> 
> Just bisected this down in linux-next for breaking booting of
> my omap2420 ARMv6 based n8x0..
> 
>> --- a/arch/arm/kernel/head.S
>> +++ b/arch/arm/kernel/head.S
>> @@ -83,8 +83,12 @@ ENTRY(stext)
>>   THUMB(	.thumb			)	@ switch to Thumb now.
>>   THUMB(1:			)
>>  
>> -	setmode	PSR_F_BIT | PSR_I_BIT | SVC_MODE, r9 @ ensure svc mode
>> -						@ and irqs disabled
>> +#ifdef CONFIG_ARM_VIRT_EXT
>> +	bl	__hyp_stub_install
>> +#endif
>> +	@ ensure svc mode and all interrupts masked
>> +	safe_svcmode_maskall r9
>> +
>>  	mrc	p15, 0, r9, c0, c0		@ get processor id
>>  	bl	__lookup_processor_type		@ r5=procinfo r9=cpuid
>>  	movs	r10, r5				@ invalid processor (r5=0)?
> 
> ..and looks like undoing this part fixes it. Any ideas?
> 
> I quickly tried disabling ARCH_OMAP3 and ARCH_OMAP4 so it's
> ARMv6 but that does not help.

If you compiled for v6 only, we can safely exclude __hyp_stub_install, and
I assume that you get past the decompressor.

If so, that indicates some side effect of the safe_svcmode_maskall macro,
and I suspect the "movs pc, lr" bit.

Can you try the attached patch? It basically falls back to the previous
behaviour if not entered in HYP mode.

Thanks,

        M.

Comments

Tony Lindgren Oct. 6, 2012, 2:42 p.m. UTC | #1

Hi,

* Marc Zyngier <[e-mail]> [121006 03:19]:
> 
> If you compiled for v6 only, we can safely exclude __hyp_stub_install, and
> I assume that you get past the decompressor.

Yes, by default it's v6 + v7, but making it v6 only did not help.
 
> If so, that indicates some side effect of the safe_svcmode_maskall macro,
> and I suspect the "movs pc, lr" bit.
> 
> Can you try the attached patch? It basically falls back to the previous
> behaviour if not entered in HYP mode.

Yes, with this it boots OK.

Regards,

Tony

> diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
> index 658a15d..b21b97f 100644
> --- a/arch/arm/include/asm/assembler.h
> +++ b/arch/arm/include/asm/assembler.h
> @@ -254,16 +254,17 @@
>  	mov	lr , \reg
>  	and	lr , lr , #MODE_MASK
>  	cmp	lr , #HYP_MODE
> -	orr	\reg , \reg , #PSR_A_BIT | PSR_I_BIT | PSR_F_BIT
> +	orr	\reg , \reg , #PSR_I_BIT | PSR_F_BIT
>  	bic	\reg , \reg , #MODE_MASK
>  	orr	\reg , \reg , #SVC_MODE
>  THUMB(	orr	\reg , \reg , #PSR_T_BIT	)
> -	msr	spsr_cxsf, \reg
> -	adr	lr, BSYM(2f)
>  	bne	1f
> +	orr	\reg, \reg, #PSR_A_BIT
> +	adr	lr, BSYM(2f)
> +	msr	spsr_cxsf, \reg
>  	__MSR_ELR_HYP(14)
>  	__ERET
> -1:	movs	pc, lr
> +1:	msr	cpsr_c, \reg
>  2:
>  .endm
>

Nicolas Pitre Oct. 6, 2012, 3:32 p.m. UTC | #2

On Sat, 6 Oct 2012, Tony Lindgren wrote:

> * Marc Zyngier <[e-mail]> [121006 03:19]:
> 
> > If so, that indicates some side effect of the safe_svcmode_maskall macro,
> > and I suspect the "movs pc, lr" bit.
> > 
> > Can you try the attached patch? It basically falls back to the previous
> > behaviour if not entered in HYP mode.
> 
> Yes, with this it boots OK.

OK. In that case, I suggest this patch be sent to Russell to fix this 
issue so he could push the ARM stuff to Linus ASAP.

Acked-by: Nicolas Pitre <[e-mail]>


Nicolas

Tony Lindgren Oct. 6, 2012, 3:40 p.m. UTC | #3

* Nicolas Pitre <[e-mail]> [121006 08:33]:
> On Sat, 6 Oct 2012, Tony Lindgren wrote:
> 
> > * Marc Zyngier <[e-mail]> [121006 03:19]:
> > 
> > > If so, that indicates some side effect of the safe_svcmode_maskall macro,
> > > and I suspect the "movs pc, lr" bit.
> > > 
> > > Can you try the attached patch? It basically falls back to the previous
> > > behaviour if not entered in HYP mode.
> > 
> > Yes, with this it boots OK.
> 
> OK. In that case, I suggest this patch be sent to Russell to fix this 
> issue so he could push the ARM stuff to Linus ASAP.
> 
> Acked-by: Nicolas Pitre <[e-mail]>

Yes we can come back to this:

Tested-by: Tony Lindgren <[e-mail]>

Russell King - ARM Linux Oct. 6, 2012, 3:42 p.m. UTC | #4

On Sat, Oct 06, 2012 at 11:32:16AM -0400, Nicolas Pitre wrote:
> On Sat, 6 Oct 2012, Tony Lindgren wrote:
> > * Marc Zyngier <[e-mail]> [121006 03:19]:
> > 
> > > If so, that indicates some side effect of the safe_svcmode_maskall macro,
> > > and I suspect the "movs pc, lr" bit.
> > > 
> > > Can you try the attached patch? It basically falls back to the previous
> > > behaviour if not entered in HYP mode.
> > 
> > Yes, with this it boots OK.
> 
> OK. In that case, I suggest this patch be sent to Russell to fix this 
> issue so he could push the ARM stuff to Linus ASAP.
> 
> Acked-by: Nicolas Pitre <[e-mail]>

I've already sent the pull request (it's late enough already for the
first bunch of stuff... Linus' truely brilliant timing for the N'th
time sees the merge window open when I'm away - why does he keep doing
that to me?) for everything _except_ stuff in my 'devel-stable' branch,
which includes the opcodes, virt, and new cache maintanence stuff.

I won't be sending Linus another pull request until at least three
days after he merges the current request, so we'll just have to hope
that this doesn't turn out to be a short merge window...

Marc Zyngier Oct. 6, 2012, 4:06 p.m. UTC | #5

On Sat, 6 Oct 2012 08:40:52 -0700, Tony Lindgren <[e-mail]> wrote:
> * Nicolas Pitre <[e-mail]> [121006 08:33]:
>> On Sat, 6 Oct 2012, Tony Lindgren wrote:
>> 
>> > * Marc Zyngier <[e-mail]> [121006 03:19]:
>> > 
>> > > If so, that indicates some side effect of the safe_svcmode_maskall
>> > > macro,
>> > > and I suspect the "movs pc, lr" bit.
>> > > 
>> > > Can you try the attached patch? It basically falls back to the
>> > > previous
>> > > behaviour if not entered in HYP mode.
>> > 
>> > Yes, with this it boots OK.
>> 
>> OK. In that case, I suggest this patch be sent to Russell to fix this 
>> issue so he could push the ARM stuff to Linus ASAP.
>> 
>> Acked-by: Nicolas Pitre <[e-mail]>
> 
> Yes we can come back to this:
> 
> Tested-by: Tony Lindgren <[e-mail]>

Now in Russell's patch system as #7549/1.

Thanks a lot for your help on this both of you.

        M.

Patch

diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
index 658a15d..b21b97f 100644
--- a/arch/arm/include/asm/assembler.h
+++ b/arch/arm/include/asm/assembler.h
@@ -254,16 +254,17 @@ 
 	mov	lr , \reg
 	and	lr , lr , #MODE_MASK
 	cmp	lr , #HYP_MODE
-	orr	\reg , \reg , #PSR_A_BIT | PSR_I_BIT | PSR_F_BIT
+	orr	\reg , \reg , #PSR_I_BIT | PSR_F_BIT
 	bic	\reg , \reg , #MODE_MASK
 	orr	\reg , \reg , #SVC_MODE
 THUMB(	orr	\reg , \reg , #PSR_T_BIT	)
-	msr	spsr_cxsf, \reg
-	adr	lr, BSYM(2f)
 	bne	1f
+	orr	\reg, \reg, #PSR_A_BIT
+	adr	lr, BSYM(2f)
+	msr	spsr_cxsf, \reg
 	__MSR_ELR_HYP(14)
 	__ERET
-1:	movs	pc, lr
+1:	msr	cpsr_c, \reg
 2:
 .endm