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

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

[Tony Lindgren]

Hi,

* Marc Zyngier <marc.zyngier@arm.com> [120907 10:04]:
> From: Dave Martin <dave.martin@linaro.org>
> 
> 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 <dave.martin@linaro.org>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>

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.

Regards,

Tony

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

[Russell King - ARM Linux]

On Fri, Oct 05, 2012 at 01:08:22PM -0700, Tony Lindgren wrote:
> 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.

You really should Cc me when you hit something like this.  I was
thinking about sending my tree (which contains these changes) this
evening but if they're breaking stuff, I'd prefer to delay that
stuff at least for a while.

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

[Tony Lindgren]

* Russell King - ARM Linux <linux@arm.linux.org.uk> [121005 16:10]:
> On Fri, Oct 05, 2012 at 01:08:22PM -0700, Tony Lindgren wrote:
> > 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.
> 
> You really should Cc me when you hit something like this.  I was
> thinking about sending my tree (which contains these changes) this
> evening but if they're breaking stuff, I'd prefer to delay that
> stuff at least for a while.

Sorry was planning to cc you for sure but forgot. Got any ideas
what to try to fix this?

Tony

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

[Tony Lindgren]

* Tony Lindgren <tony@atomide.com> [121005 16:27]:
> * Russell King - ARM Linux <linux@arm.linux.org.uk> [121005 16:10]:
> > On Fri, Oct 05, 2012 at 01:08:22PM -0700, Tony Lindgren wrote:
> > > 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.

The same kernel boots on 2430sdp, which is the same ARMv6 core
as 2430 if I remember correctly. So this hints that it has something
to do with the bits set differently by the bootloader?

Regards,

Tony

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

[Nicolas Pitre]

On Fri, 5 Oct 2012, Tony Lindgren wrote:

> * Tony Lindgren <tony@atomide.com> [121005 16:27]:
> > * Russell King - ARM Linux <linux@arm.linux.org.uk> [121005 16:10]:
> > > On Fri, Oct 05, 2012 at 01:08:22PM -0700, Tony Lindgren wrote:
> > > > 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.
> 
> The same kernel boots on 2430sdp, which is the same ARMv6 core
> as 2430 if I remember correctly. So this hints that it has something
> to do with the bits set differently by the bootloader?

Possibly.

What if you apply this on top:

diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
index 683a1e6b60..b276c26e19 100644
--- a/arch/arm/include/asm/assembler.h
+++ b/arch/arm/include/asm/assembler.h
@@ -254,8 +254,7 @@
 	mov	lr , \reg
 	and	lr , lr , #MODE_MASK
 	cmp	lr , #HYP_MODE
-	orr	\reg , \reg , #PSR_A_BIT | PSR_I_BIT | PSR_F_BIT
-	bic	\reg , \reg , #MODE_MASK
+	mov	\reg , #PSR_A_BIT | PSR_I_BIT | PSR_F_BIT
 	orr	\reg , \reg , #SVC_MODE
 THUMB(	orr	\reg , \reg , #PSR_T_BIT	)
 	msr	spsr_cxsf, \reg



Nicolas

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

[Tony Lindgren]

* Nicolas Pitre <nicolas.pitre@linaro.org> [121005 18:33]:
> On Fri, 5 Oct 2012, Tony Lindgren wrote:
> 
> > * Tony Lindgren <tony@atomide.com> [121005 16:27]:
> > > * Russell King - ARM Linux <linux@arm.linux.org.uk> [121005 16:10]:
> > > > On Fri, Oct 05, 2012 at 01:08:22PM -0700, Tony Lindgren wrote:
> > > > > 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.
> > 
> > The same kernel boots on 2430sdp, which is the same ARMv6 core
> > as 2430 if I remember correctly. So this hints that it has something
> > to do with the bits set differently by the bootloader?
> 
> Possibly.
> 
> What if you apply this on top:
> 
> diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
> index 683a1e6b60..b276c26e19 100644
> --- a/arch/arm/include/asm/assembler.h
> +++ b/arch/arm/include/asm/assembler.h
> @@ -254,8 +254,7 @@
>  	mov	lr , \reg
>  	and	lr , lr , #MODE_MASK
>  	cmp	lr , #HYP_MODE
> -	orr	\reg , \reg , #PSR_A_BIT | PSR_I_BIT | PSR_F_BIT
> -	bic	\reg , \reg , #MODE_MASK
> +	mov	\reg , #PSR_A_BIT | PSR_I_BIT | PSR_F_BIT
>  	orr	\reg , \reg , #SVC_MODE
>  THUMB(	orr	\reg , \reg , #PSR_T_BIT	)
>  	msr	spsr_cxsf, \reg
> 
> 
> 

Thanks but that does not seem to help.

Regards,

Tony

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

[Marc Zyngier]

[-- Attachment #1: Type: text/plain, Size: 2281 bytes --]

Hi Tony,

On Fri, 5 Oct 2012 13:08:22 -0700, Tony Lindgren <tony@atomide.com> wrote:
> Hi,
> 
> * Marc Zyngier <marc.zyngier@arm.com> [120907 10:04]:
>> From: Dave Martin <dave.martin@linaro.org>
>> 
>> 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 <dave.martin@linaro.org>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> 
> 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.
-- 
Fast, cheap, reliable. Pick two.

[-- Attachment #2: setmode.patch --]
[-- Type: text/plain, Size: 672 bytes --]

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
 

[-- Attachment #3: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Nicolas Pitre]

On Sat, 6 Oct 2012, Marc Zyngier wrote:

> Hi Tony,
> 
> On Fri, 5 Oct 2012 13:08:22 -0700, Tony Lindgren <tony@atomide.com> wrote:
> > Hi,
> > 
> > * Marc Zyngier <marc.zyngier@arm.com> [120907 10:04]:
> >> From: Dave Martin <dave.martin@linaro.org>
> >> 
> >> 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 <dave.martin@linaro.org>
> >> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> > 
> > 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.

That would be surprizing if the "movs pc, lr" was to blame.  This should 
work on all architectures.

However the A bit might be to blame.

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

This is likely to work of course.  However I think we should try to 
pinpoint the exact problem i.e. whether it is the A bit or the "movs pc, 
lr" which makes a difference (it is unlikely to be both).

So I was about to suggest to test this patch as well:

diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
index 683a1e6b60..118e22ee46 100644
--- a/arch/arm/include/asm/assembler.h
+++ b/arch/arm/include/asm/assembler.h
@@ -254,8 +254,7 @@
 	mov	lr , \reg
 	and	lr , lr , #MODE_MASK
 	cmp	lr , #HYP_MODE
-	orr	\reg , \reg , #PSR_A_BIT | PSR_I_BIT | PSR_F_BIT
-	bic	\reg , \reg , #MODE_MASK
+	mov	\reg , #PSR_I_BIT | PSR_F_BIT
 	orr	\reg , \reg , #SVC_MODE
 THUMB(	orr	\reg , \reg , #PSR_T_BIT	)
 	msr	spsr_cxsf, \reg


Nicolas

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

[Tony Lindgren]

* Nicolas Pitre <nicolas.pitre@linaro.org> [121006 07:07]:
> On Sat, 6 Oct 2012, Marc Zyngier wrote:
> > 
> > If so, that indicates some side effect of the safe_svcmode_maskall macro,
> > and I suspect the "movs pc, lr" bit.
> 
> That would be surprizing if the "movs pc, lr" was to blame.  This should 
> work on all architectures.
> 
> However the A bit might be to blame.
> 
> > Can you try the attached patch? It basically falls back to the previous
> > behaviour if not entered in HYP mode.
> 
> This is likely to work of course.  However I think we should try to 
> pinpoint the exact problem i.e. whether it is the A bit or the "movs pc, 
> lr" which makes a difference (it is unlikely to be both).
> 
> So I was about to suggest to test this patch as well:

No luck with this patch alone.

Regards,

Tony
 
> diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
> index 683a1e6b60..118e22ee46 100644
> --- a/arch/arm/include/asm/assembler.h
> +++ b/arch/arm/include/asm/assembler.h
> @@ -254,8 +254,7 @@
>  	mov	lr , \reg
>  	and	lr , lr , #MODE_MASK
>  	cmp	lr , #HYP_MODE
> -	orr	\reg , \reg , #PSR_A_BIT | PSR_I_BIT | PSR_F_BIT
> -	bic	\reg , \reg , #MODE_MASK
> +	mov	\reg , #PSR_I_BIT | PSR_F_BIT
>  	orr	\reg , \reg , #SVC_MODE
>  THUMB(	orr	\reg , \reg , #PSR_T_BIT	)
>  	msr	spsr_cxsf, \reg
> 
> 
> Nicolas

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

[Marc Zyngier]

On Sat, 6 Oct 2012 10:06:00 -0400 (EDT), Nicolas Pitre
<nicolas.pitre@linaro.org> wrote:
> On Sat, 6 Oct 2012, Marc Zyngier wrote:
> 
>> Hi Tony,
>> 
>> On Fri, 5 Oct 2012 13:08:22 -0700, Tony Lindgren <tony@atomide.com>
>> wrote:
>> > Hi,
>> > 
>> > * Marc Zyngier <marc.zyngier@arm.com> [120907 10:04]:
>> >> From: Dave Martin <dave.martin@linaro.org>
>> >> 
>> >> 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 <dave.martin@linaro.org>
>> >> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> > 
>> > 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.
> 
> That would be surprizing if the "movs pc, lr" was to blame.  This should

> work on all architectures.
> 
> However the A bit might be to blame.
> 
>> Can you try the attached patch? It basically falls back to the previous
>> behaviour if not entered in HYP mode.
> 
> This is likely to work of course.  However I think we should try to 
> pinpoint the exact problem i.e. whether it is the A bit or the "movs pc,

> lr" which makes a difference (it is unlikely to be both).

Agreed. We need to get to the bottom of this. Also, finding out which
revisions of the 1136 are present on both of Tony's platforms could help
understanding why this works on OMAP 2430 and not 2420.

> So I was about to suggest to test this patch as well:
> 
> diff --git a/arch/arm/include/asm/assembler.h
> b/arch/arm/include/asm/assembler.h
> index 683a1e6b60..118e22ee46 100644
> --- a/arch/arm/include/asm/assembler.h
> +++ b/arch/arm/include/asm/assembler.h
> @@ -254,8 +254,7 @@
>  	mov	lr , \reg
>  	and	lr , lr , #MODE_MASK
>  	cmp	lr , #HYP_MODE
> -	orr	\reg , \reg , #PSR_A_BIT | PSR_I_BIT | PSR_F_BIT
> -	bic	\reg , \reg , #MODE_MASK
> +	mov	\reg , #PSR_I_BIT | PSR_F_BIT
>  	orr	\reg , \reg , #SVC_MODE
>  THUMB(	orr	\reg , \reg , #PSR_T_BIT	)
>  	msr	spsr_cxsf, \reg

Yup, that should give us all the information we need.

Thanks Nico.

        M.
-- 
Fast, cheap, reliable. Pick two.

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

[Tony Lindgren]

Hi,

* Marc Zyngier <marc.zyngier@arm.com> [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
>  

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

[Nicolas Pitre]

On Sat, 6 Oct 2012, Tony Lindgren wrote:

> * Marc Zyngier <marc.zyngier@arm.com> [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 <nico@linaro.org>


Nicolas

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

[Tony Lindgren]

* Nicolas Pitre <nicolas.pitre@linaro.org> [121006 08:33]:
> On Sat, 6 Oct 2012, Tony Lindgren wrote:
> 
> > * Marc Zyngier <marc.zyngier@arm.com> [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 <nico@linaro.org>

Yes we can come back to this:

Tested-by: Tony Lindgren <tony@atomide.com>

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

[Marc Zyngier]

On Sat, 6 Oct 2012 08:40:52 -0700, Tony Lindgren <tony@atomide.com> wrote:
> * Nicolas Pitre <nicolas.pitre@linaro.org> [121006 08:33]:
>> On Sat, 6 Oct 2012, Tony Lindgren wrote:
>> 
>> > * Marc Zyngier <marc.zyngier@arm.com> [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 <nico@linaro.org>
> 
> Yes we can come back to this:
> 
> Tested-by: Tony Lindgren <tony@atomide.com>

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

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

        M.
-- 
Fast, cheap, reliable. Pick two.

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

[Russell King - ARM Linux]

On Sat, Oct 06, 2012 at 11:32:16AM -0400, Nicolas Pitre wrote:
> On Sat, 6 Oct 2012, Tony Lindgren wrote:
> > * Marc Zyngier <marc.zyngier@arm.com> [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 <nico@linaro.org>

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...

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

[Tony Lindgren]

* Marc Zyngier <marc.zyngier@arm.com> [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
 

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

[Dave Martin]

On Sat, Oct 06, 2012 at 09:00:32AM -0700, Tony Lindgren wrote:
> * Marc Zyngier <marc.zyngier@arm.com> [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.


Cheers
---Dave

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

[Marc Zyngier]

[-- Attachment #1: Type: text/plain, Size: 2964 bytes --]

On 08/10/12 12:01, Dave Martin wrote:
> On Sat, Oct 06, 2012 at 09:00:32AM -0700, Tony Lindgren wrote:
>> * Marc Zyngier <marc.zyngier@arm.com> [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.
-- 
Jazz is not dead. It just smells funny...

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: sysmode.patch --]
[-- Type: text/x-diff; name=sysmode.patch, Size: 613 bytes --]

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
 

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

[Tony Lindgren]

* Marc Zyngier <marc.zyngier@arm.com> [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
>  

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

[Dave Martin]

On Mon, Oct 08, 2012 at 12:01:09PM +0100, Dave Martin wrote:
> On Sat, Oct 06, 2012 at 09:00:32AM -0700, Tony Lindgren wrote:
> > * Marc Zyngier <marc.zyngier@arm.com> [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.


Marc Z also just pointed out to me that there is one architecturally
valid explanation for why the movs route could fail: if the kernel is
entered in System mode for some reason -- this causes msr spsr and movs
pc to become UNPREDICTABLE.  If this is the explanation, then
(a), (b) and (c) should all work, provided the CPU has already been forced
out of System mode.

Of course, we're not supposed to be entered in System mode ... but since
the whole purpose of this code is to force us into a sane state, we should
work around it anyway.  I think Marc is busy rolling a patch for that.

Cheers
---Dave