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

Commit Message

Marc Zyngier Oct. 8, 2012, 11:33 a.m. UTC

On 08/10/12 12:01, Dave Martin wrote:
> On Sat, Oct 06, 2012 at 09:00:32AM -0700, 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.
>> ...
>>
>>> 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
>>>  
>>
>> The minimal version of this that still boots on my n800 is just
>> the last change of the above patch:
>>
>> --- a/arch/arm/include/asm/assembler.h
>> +++ b/arch/arm/include/asm/assembler.h
>> @@ -263,7 +263,7 @@ THUMB(	orr	\reg , \reg , #PSR_T_BIT	)
>>  	bne	1f
>>  	__MSR_ELR_HYP(14)
>>  	__ERET
>> -1:	movs	pc, lr
>> +1:	msr	cpsr_c, \reg
>>  2:
>>  .endm
>>  
> 
> In an attempt to narrow this down...
> 
> Can you follow this (i.e., _after_ a known successful switch to SVC mode)
> 
> (a)
> 	mrs	\reg, cpsr
> 	msr	spsr_cxsf, \reg
> 	adr	\reg, 3f
> 	movs	pc, lr
> 3:
> 
> and (b)
> 
> 	mrs	\reg, cpsr
> 	orr	\reg, \reg, #CPSR_A_BIT
> 	msr	cpsr_cxsf, \reg
> 
> and (c)
> 
> 	mrs	\reg, cpsr
> 	orr	\reg, \reg, #CPSR_A_BIT
> 	msr	spsr_cxsf, \reg
> 	adr	\reg, 3f
> 	movs	pc, lr
> 3:
> 
> 
> 
> 
> If only (a) works, this would suggest that the attempt to set the A bit
> is causing the problem.
> 
> If only (b) works, this suggests that the A bit is OK but that some
> invalid hardware state, or something else we don't understand, is causing
> exception returns to fail in general.
> 
> If (a) and (b) work but (c) fails, this suggests that specifically
> trying to set the A bit via an exception return is problematic.
> 
> If all of them work then this suggests some invalid hardware state or
> something else we don't understand, but which is cleared by the initial
> msr cpsr_c which clobbers the processor mode.

Playing with an Integrator-CP with a 1136 tile, the only way I could
cause the thing to fail was to boot the thing in SYSTEM mode.

Tony, can you possibly also try the attached patch?

Thanks,

	M.

Comments

Tony Lindgren Oct. 8, 2012, 8:36 p.m. UTC | #1

* Marc Zyngier <[e-mail]> [121008 04:34]:
> On 08/10/12 12:01, Dave Martin wrote:
> > 
> > In an attempt to narrow this down...
> > 
> > Can you follow this (i.e., _after_ a known successful switch to SVC mode)
> > 
> > (a)
> > 	mrs	\reg, cpsr
> > 	msr	spsr_cxsf, \reg
> > 	adr	\reg, 3f
> > 	movs	pc, lr
> > 3:
> > 
> > and (b)
> > 
> > 	mrs	\reg, cpsr
> > 	orr	\reg, \reg, #CPSR_A_BIT
> > 	msr	cpsr_cxsf, \reg
> > 
> > and (c)
> > 
> > 	mrs	\reg, cpsr
> > 	orr	\reg, \reg, #CPSR_A_BIT
> > 	msr	spsr_cxsf, \reg
> > 	adr	\reg, 3f
> > 	movs	pc, lr
> > 3:
> > 
> > 
> > 
> > 
> > If only (a) works, this would suggest that the attempt to set the A bit
> > is causing the problem.
> > 
> > If only (b) works, this suggests that the A bit is OK but that some
> > invalid hardware state, or something else we don't understand, is causing
> > exception returns to fail in general.
> > 
> > If (a) and (b) work but (c) fails, this suggests that specifically
> > trying to set the A bit via an exception return is problematic.
> > 
> > If all of them work then this suggests some invalid hardware state or
> > something else we don't understand, but which is cleared by the initial
> > msr cpsr_c which clobbers the processor mode.

Thanks it does not seem to be the A bit..
 
> Playing with an Integrator-CP with a 1136 tile, the only way I could
> cause the thing to fail was to boot the thing in SYSTEM mode.
> 
> Tony, can you possibly also try the attached patch?

Yup you figured it out n800 is in system mode while 2430sdp is not..
I verified it with printhex2.

Your patch fails to compile with:

arch/arm/kernel/head.S: Assembler messages:
arch/arm/kernel/head.S:336: Error: symbol `omgsys' is already defined

Probably because it's included twice. Also, shouldn't it
be bne omgsys instead of beq?

Regards,

Tony

> diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
> index 683a1e6..7714ec7 100644
> --- a/arch/arm/include/asm/assembler.h
> +++ b/arch/arm/include/asm/assembler.h
> @@ -253,6 +253,8 @@
>  	mrs	\reg , cpsr
>  	mov	lr , \reg
>  	and	lr , lr , #MODE_MASK
> +	cmp	lr , #SYSTEM_MODE	/* Yet another braindead platform? */
> +	beq	omgsys
>  	cmp	lr , #HYP_MODE
>  	orr	\reg , \reg , #PSR_A_BIT | PSR_I_BIT | PSR_F_BIT
>  	bic	\reg , \reg , #MODE_MASK
> @@ -264,6 +266,7 @@ THUMB(	orr	\reg , \reg , #PSR_T_BIT	)
>  	__MSR_ELR_HYP(14)
>  	__ERET
>  1:	movs	pc, lr
> +omgsys: msr	cpsr_c, \reg
>  2:
>  .endm
>

Patch

diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
index 683a1e6..7714ec7 100644
--- a/arch/arm/include/asm/assembler.h
+++ b/arch/arm/include/asm/assembler.h
@@ -253,6 +253,8 @@ 
 	mrs	\reg , cpsr
 	mov	lr , \reg
 	and	lr , lr , #MODE_MASK
+	cmp	lr , #SYSTEM_MODE	/* Yet another braindead platform? */
+	beq	omgsys
 	cmp	lr , #HYP_MODE
 	orr	\reg , \reg , #PSR_A_BIT | PSR_I_BIT | PSR_F_BIT
 	bic	\reg , \reg , #MODE_MASK
@@ -264,6 +266,7 @@  THUMB(	orr	\reg , \reg , #PSR_T_BIT	)
 	__MSR_ELR_HYP(14)
 	__ERET
 1:	movs	pc, lr
+omgsys: msr	cpsr_c, \reg
 2:
 .endm