| Message ID | 1381352223-17721-1-git-send-email-taras.kondratiuk@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Oct 09, 2013 at 09:57:03PM +0100, Taras Kondratiuk wrote: > In Thumb2 kernel (CONFIG_THUMB2_KERNEL) kexec's relocate code is assembled > in Thumb2 mode, but cpu_v7_reset() jumps to this code in ARM state, > because its address is page aligned and has 0 in LSB. This used to work, but Dave broken it when he fixed the reset code in 153cd8e839b5 ("ARM: 7553/1: proc-v7: Ensure correct instruction set after cpu_reset"). > Assemble this code in ARM mode to fix the issue. In the interest of keeping kexec a possibility for v7m, it might be better to do something similar to head.S (i.e. switch back to thumb if we're a thumb-2 kernel). Cheers, Will > Signed-off-by: Taras Kondratiuk <taras.kondratiuk@linaro.org> > --- > Based on v3.12-rc4 > > Cc: Dave Martin <dave.martin@linaro.org> > Cc: Will Deacon <will.deacon@arm.com> > Cc: Russell King <linux@arm.linux.org.uk> > Cc: linaro-kernel@lists.linaro.org > Cc: linux-arm-kernel@lists.infradead.org > --- > arch/arm/kernel/relocate_kernel.S | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm/kernel/relocate_kernel.S b/arch/arm/kernel/relocate_kernel.S > index d0cdedf..a3af323 100644 > --- a/arch/arm/kernel/relocate_kernel.S > +++ b/arch/arm/kernel/relocate_kernel.S > @@ -5,6 +5,7 @@ > #include <asm/kexec.h> > > .globl relocate_new_kernel > + .arm > relocate_new_kernel: > > ldr r0,kexec_indirection_page > -- > 1.7.9.5 > >
On 10 October 2013 12:02, Will Deacon <will.deacon@arm.com> wrote: > On Wed, Oct 09, 2013 at 09:57:03PM +0100, Taras Kondratiuk wrote: >> In Thumb2 kernel (CONFIG_THUMB2_KERNEL) kexec's relocate code is assembled >> in Thumb2 mode, but cpu_v7_reset() jumps to this code in ARM state, >> because its address is page aligned and has 0 in LSB. > > This used to work, but Dave broken it when he fixed the reset code in > 153cd8e839b5 ("ARM: 7553/1: proc-v7: Ensure correct instruction set after > cpu_reset"). > >> Assemble this code in ARM mode to fix the issue. > > In the interest of keeping kexec a possibility for v7m, it might be better > to do something similar to head.S (i.e. switch back to thumb if we're a > thumb-2 kernel). > Actually only v7 jumps here in ARM, so maybe it worth to wrap the patch with CONFIG_CPU_V7? In this case v7m won't be touched. I'll update patch with a stub that switches back to Thumb, but honestly I don't see a bit sense of it here. Anyway at the end of this function instruction set is switched again.
On Wed, Oct 09, 2013 at 11:57:03PM +0300, Taras Kondratiuk wrote: > In Thumb2 kernel (CONFIG_THUMB2_KERNEL) kexec's relocate code is assembled > in Thumb2 mode, but cpu_v7_reset() jumps to this code in ARM state, > because its address is page aligned and has 0 in LSB. > > Assemble this code in ARM mode to fix the issue. I think the actual issue here is that relocate_new_kernel is not properly annotated as a function symbol. Can you remove the explicit label declaration and try the following: #include <linux/linkage.h> ENTRY(relocate_new_kernel) /* body of relocate_new_kernel */ ENDPROC(relocate_new_kernel) Without this, the linker will treat it as a random pointer to data and never set the Thumb bit. This fails in precisely the same was as an ordinary function call would fail if the destination function doesn't have the needed annotation. There should be no need to switch to ARM if the kernel is just jumping to itself... Cheers ---Dave > > Signed-off-by: Taras Kondratiuk <taras.kondratiuk@linaro.org> > --- > Based on v3.12-rc4 > > Cc: Dave Martin <dave.martin@linaro.org> > Cc: Will Deacon <will.deacon@arm.com> > Cc: Russell King <linux@arm.linux.org.uk> > Cc: linaro-kernel@lists.linaro.org > Cc: linux-arm-kernel@lists.infradead.org > --- > arch/arm/kernel/relocate_kernel.S | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm/kernel/relocate_kernel.S b/arch/arm/kernel/relocate_kernel.S > index d0cdedf..a3af323 100644 > --- a/arch/arm/kernel/relocate_kernel.S > +++ b/arch/arm/kernel/relocate_kernel.S > @@ -5,6 +5,7 @@ > #include <asm/kexec.h> > > .globl relocate_new_kernel > + .arm > relocate_new_kernel: > > ldr r0,kexec_indirection_page > -- > 1.7.9.5 >
On 10 October 2013 17:12, Dave Martin <Dave.Martin@arm.com> wrote: > On Wed, Oct 09, 2013 at 11:57:03PM +0300, Taras Kondratiuk wrote: >> In Thumb2 kernel (CONFIG_THUMB2_KERNEL) kexec's relocate code is assembled >> in Thumb2 mode, but cpu_v7_reset() jumps to this code in ARM state, >> because its address is page aligned and has 0 in LSB. >> >> Assemble this code in ARM mode to fix the issue. > > I think the actual issue here is that relocate_new_kernel is not properly > annotated as a function symbol. > > Can you remove the explicit label declaration and try the following: > > #include <linux/linkage.h> > > ENTRY(relocate_new_kernel) > > /* body of relocate_new_kernel */ > > ENDPROC(relocate_new_kernel) > > > Without this, the linker will treat it as a random pointer to data and > never set the Thumb bit. > > This fails in precisely the same was as an ordinary function call > would fail if the destination function doesn't have the needed > annotation. > > > There should be no need to switch to ARM if the kernel is just jumping > to itself... I think it won't help, because here is no direct jump to this label. This code gets copied to a new page and jump is done to the beginning of that page.
diff --git a/arch/arm/kernel/relocate_kernel.S b/arch/arm/kernel/relocate_kernel.S index d0cdedf..a3af323 100644 --- a/arch/arm/kernel/relocate_kernel.S +++ b/arch/arm/kernel/relocate_kernel.S @@ -5,6 +5,7 @@ #include <asm/kexec.h> .globl relocate_new_kernel + .arm relocate_new_kernel: ldr r0,kexec_indirection_page
In Thumb2 kernel (CONFIG_THUMB2_KERNEL) kexec's relocate code is assembled in Thumb2 mode, but cpu_v7_reset() jumps to this code in ARM state, because its address is page aligned and has 0 in LSB. Assemble this code in ARM mode to fix the issue. Signed-off-by: Taras Kondratiuk <taras.kondratiuk@linaro.org> --- Based on v3.12-rc4 Cc: Dave Martin <dave.martin@linaro.org> Cc: Will Deacon <will.deacon@arm.com> Cc: Russell King <linux@arm.linux.org.uk> Cc: linaro-kernel@lists.linaro.org Cc: linux-arm-kernel@lists.infradead.org --- arch/arm/kernel/relocate_kernel.S | 1 + 1 file changed, 1 insertion(+)