ARM: tegra: Remove redundant change to the CPSR in headsmp.S
Subsystems (3)
PATCH 1/1 ARM: tegra: Remove redundant change to the CPSR in headsmp.S
At secondary_startup, the MSR CPSR_cxsf, #0xd3 is not compatible with Thumb-2 and also unmasks asynchronous aborts (CPSR.A bit forced to zero -- this is probably unintentional).
Any remotely sane bootloader should be putting each secondary CPU in the appropriate state _before_ entering the kernel anyway. Otherwise, disabling interrupts on entry to the kernel isn't going to fix it.
Therefore this patch just removes the MSR instruction.
---
I make assumptions about the bootloader in this patch. If someone with Tegra knowledge can please comment and/or test, that would be much appreciated, thanks.
arch/arm/mach-tegra/headsmp.S | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)
arch/arm/mach-tegra/headsmp.S +0 -1
@@ -48,7 +48,6 @@ ENTRY(v7_invalidate_l1) ENDPROC(v7_invalidate_l1) ENTRY(tegra_secondary_startup) - msr cpsr_fsxc, #0xd3 bl v7_invalidate_l1 mrc p15, 0, r0, c0, c0, 5 and r0, r0, #15
Signature · 1 line
1.7.4.1
PATCH 1/1 ARM: tegra: Remove redundant change to the CPSR in headsmp.S
Colin, can you comment on this?
Quoted 26 lines · Dave Martin
At
secondary_startup, the MSRCPSR_cxsf, #0xd3 is not compatible with Thumb-2 and also unmasks asynchronous aborts (CPSR.A bit forced to zero -- this is probably unintentional).Any remotely sane bootloader should be putting each secondary CPU in the appropriate state _before_ entering the kernel anyway. Otherwise, disabling interrupts on entry to the kernel isn't going to fix it.
Therefore this patch just removes the MSR instruction.
Signed-off-by Dave Martin <[email protected]>---
I make assumptions about the bootloader in this patch. If someone with Tegra knowledge can please comment and/or test, that would be much appreciated, thanks.
arch/arm/mach-tegra/headsmp.S | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)arch/arm/mach-tegra/headsmp.S +0
-0
arch/arm/mach-tegra/headsmp.S ENTRY(v7_invalidate_l1) · line 53
Quoted 7 lines
arch/arm/mach-tegra/headsmp.S +0
-1@@ -48,7 +48,6 @@ ENTRY(v7_invalidate_l1) ENDPROC(v7_invalidate_l1) ENTRY(tegra_secondary_startup) - msr cpsr_fsxc, #0xd3 bl v7_invalidate_l1 mrc p15, 0, r0, c0, c0, 5 and r0, r0, #15
My rationale here is that the CPU boots straight into the correct mode -- if there is any boot code before we get here, then it should already have established the correct mode, otherwise we're likely to get problems which "MSR" by itself isn't going to fix -- such as stray interrupts for example.
If that feels unsafe however, we should still to be able to write
cpsid aif, #SVC_MODE
(which is the compact v6/v7-compatible way to set all the interrupt mask bits and get into a specific mode)
Cheers
---Dave
PATCH 1/1 ARM: tegra: Remove redundant change to the CPSR in headsmp.S
Quoted 43 lines · Dave Martin
Colin, can you comment on this?
Quoted 30 lines · Dave Martin
At
secondary_startup, the MSRCPSR_cxsf, #0xd3 is not compatible with Thumb-2 and also unmasks asynchronous aborts (CPSR.A bit forced to zero -- this is probably unintentional).Any remotely sane bootloader should be putting each secondary CPU in the appropriate state _before_ entering the kernel anyway. Otherwise, disabling interrupts on entry to the kernel isn't going to fix it.
Therefore this patch just removes the MSR instruction.
Signed-off-by Dave Martin <[email protected]>---
I make assumptions about the bootloader in this patch. ?If someone with Tegra knowledge can please comment and/or test, that would be much appreciated, thanks.
?arch/arm/mach-tegra/headsmp.S | ? ?1 -
?1 files changed, 0 insertions(+), 1 deletions(-)arch/arm/mach-tegra/headsmp.S +0
-1@@ -48,7 +48,6 @@ ENTRY(v7_invalidate_l1) ?ENDPROC(v7_invalidate_l1) ?ENTRY(tegra_secondary_startup) - ? ? msr ? ? cpsr_fsxc, #0xd3Quoted 3 lines
? ? ? ? ?bl ? ? ?v7_invalidate_l1
? ? ? mrc ? ? p15, 0, r0, c0, c0, 5
? ? ? ? ?and ?r0, r0, #15My rationale here is that the CPU boots straight into the correct mode -- if there is any boot code before we get here, then it should already have established the correct mode, otherwise we're likely to get problems which "MSR" by itself isn't going to fix -- such as stray interrupts for example.
Quoted 6 lines
If that feels unsafe however, we should still to be able to write
? ? ? ?cpsid ? aif, #SVC_MODE
(which is the compact v6/v7-compatible way to set all the interrupt mask bits and get into a specific mode)
Tegra boots with one cpu enabled, and the other in reset. The cpu needs to be initialized from scratch by the kernel. Does cpsid aif, #SVC_MODE compile to the same instruction as the msr instruction?
PATCH 1/1 ARM: tegra: Remove redundant change to the CPSR in headsmp.S
Quoted 46 lines · Colin Cross
Quoted 45 lines · Dave Martin
Colin, can you comment on this?
Quoted 30 lines · Dave Martin
At
secondary_startup, the MSRCPSR_cxsf, #0xd3 is not compatible with Thumb-2 and also unmasks asynchronous aborts (CPSR.A bit forced to zero -- this is probably unintentional).Any remotely sane bootloader should be putting each secondary CPU in the appropriate state _before_ entering the kernel anyway. Otherwise, disabling interrupts on entry to the kernel isn't going to fix it.
Therefore this patch just removes the MSR instruction.
Signed-off-by Dave Martin <[email protected]>---
I make assumptions about the bootloader in this patch. ?If someone with Tegra knowledge can please comment and/or test, that would be much appreciated, thanks.
?arch/arm/mach-tegra/headsmp.S | ? ?1 -
?1 files changed, 0 insertions(+), 1 deletions(-)arch/arm/mach-tegra/headsmp.S +0
-1@@ -48,7 +48,6 @@ ENTRY(v7_invalidate_l1) ?ENDPROC(v7_invalidate_l1) ?ENTRY(tegra_secondary_startup) - ? ? msr ? ? cpsr_fsxc, #0xd3Quoted 3 lines
? ? ? ? ?bl ? ? ?v7_invalidate_l1
? ? ? mrc ? ? p15, 0, r0, c0, c0, 5
? ? ? ? ?and ?r0, r0, #15My rationale here is that the CPU boots straight into the correct mode -- if there is any boot code before we get here, then it should already have established the correct mode, otherwise we're likely to get problems which "MSR" by itself isn't going to fix -- such as stray interrupts for example.
If that feels unsafe however, we should still to be able to write
Quoted 8 lines
Quoted 4 lines
? ? ? ?cpsid ? aif, #SVC_MODE
(which is the compact v6/v7-compatible way to set all the interrupt mask bits and get into a specific mode)
Tegra boots with one cpu enabled, and the other in reset. The cpu needs to be initialized from scratch by the kernel. Does cpsid aif, #SVC_MODE compile to the same instruction as the msr instruction?
CPUs power on in SVC mode with all interrupts masked, so it should be safe to assume that the CPU is already in this state (this is actually _required_ by Documentation/arm/Booting). If the CPU booted with a random mode and random interrupt masks, no system could boot reliably...
However, it's harmless to set the state explicitly if we want to be extra safe.
My assumption is that the unmasking of asynchronous aborts done by msr CPSR_cxsf, #0xd3 is unintentional -- i.e., this is a copy-paste of code which would have been correct prior to ARMv6 (when there was no separate 'A' interrupt class).
cps is a distinct instruction, introduced in ARMv6. The advantages are that it can be used to control the A bit, and doesn't clobber the T bit (or other CPSR bits).
Since mach-tegra doesn't apply to any platform older than ARMv6, use of this instruction seems reasonable.
cpsid aif disables all three kinds of interrupt.
With regard to Thumb-2 kernels, what code is responsible for performing the jump to tegra_secondary_startup?
If the jump is done by making an exception return from some firmware, you may find you hit tegra_secondary_starup in ARM state, and everything will go horribly wrong...
It could be worth making the secondary kernel entry point ARM code unconditionally (just as is now done for the primary kernel entry point).
Cheers
---Dave
PATCH 1/1 ARM: tegra: Remove redundant change to the CPSR in headsmp.S
On Wed, Sep 14, 2011 at 2:15 AM, Dave Martin <[email protected]> wrote:
<snip>
Quoted 2 lines
With regard to Thumb-2 kernels, what code is responsible for performing the jump to
tegra_secondary_startup?
arch/arm/mach-tegra/platsmp.c puts the physical address of tegra_secondary_startup into the reset vector, and then brings the cpu out of reset. As far as I know, it is always in ARM mode, but it is possible that it comes through ROM code that ends with a bx, allowing it to be entered in Thumb mode.
Quoted 7 lines
If the jump is done by making an exception return from some firmware, you may find you hit
tegra_secondary_starupin ARM state, and everything will go horribly wrong...It could be worth making the secondary kernel entry point ARM code unconditionally (just as is now done for the primary kernel entry point).
That seems safest.
Quoted 8 lines
Cheers
---Dave_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel