Copying a function with memcpy() and then trying to execute the result isn't trivially portable to Thumb.
This patch modifies the kexec soft restart code to copy its assembler trampoline relocate_new_kernel() using fncpy() instead, so that relocate_new_kernel can be in the same ISA as the rest of the kernel without problems.
Signed-off-by: Dave Martin Dave.Martin@arm.com --- Changes since v1:
* Move ENDPROC() after relocate_new_kernel's literals, to be consistent the location of relocate_new_kernel_end and with the way GCC sets ELF symbol sizes for functions. This is just a tidyup, with no functional impact.
arch/arm/kernel/machine_kexec.c | 17 ++++++++++------- arch/arm/kernel/relocate_kernel.S | 8 ++++++-- 2 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/arch/arm/kernel/machine_kexec.c b/arch/arm/kernel/machine_kexec.c index 57221e3..f0d180d 100644 --- a/arch/arm/kernel/machine_kexec.c +++ b/arch/arm/kernel/machine_kexec.c @@ -14,11 +14,12 @@ #include <asm/pgalloc.h> #include <asm/mmu_context.h> #include <asm/cacheflush.h> +#include <asm/fncpy.h> #include <asm/mach-types.h> #include <asm/smp_plat.h> #include <asm/system_misc.h>
-extern const unsigned char relocate_new_kernel[]; +extern void relocate_new_kernel(void); extern const unsigned int relocate_new_kernel_size;
extern unsigned long kexec_start_address; @@ -142,6 +143,8 @@ void machine_kexec(struct kimage *image) { unsigned long page_list; unsigned long reboot_code_buffer_phys; + unsigned long reboot_entry = (unsigned long)relocate_new_kernel; + unsigned long reboot_entry_phys; void *reboot_code_buffer;
/* @@ -168,16 +171,16 @@ void machine_kexec(struct kimage *image)
/* copy our kernel relocation code to the control code page */ - memcpy(reboot_code_buffer, - relocate_new_kernel, relocate_new_kernel_size); + reboot_entry = fncpy(reboot_code_buffer, + reboot_entry, + relocate_new_kernel_size); + reboot_entry_phys = (unsigned long)reboot_entry + + (reboot_code_buffer_phys - (unsigned long)reboot_code_buffer);
- - flush_icache_range((unsigned long) reboot_code_buffer, - (unsigned long) reboot_code_buffer + KEXEC_CONTROL_PAGE_SIZE); printk(KERN_INFO "Bye!\n");
if (kexec_reinit) kexec_reinit();
- soft_restart(reboot_code_buffer_phys); + soft_restart(reboot_entry_phys); } diff --git a/arch/arm/kernel/relocate_kernel.S b/arch/arm/kernel/relocate_kernel.S index d0cdedf..9585896 100644 --- a/arch/arm/kernel/relocate_kernel.S +++ b/arch/arm/kernel/relocate_kernel.S @@ -2,10 +2,12 @@ * relocate_kernel.S - put the kernel image in place to boot */
+#include <linux/linkage.h> #include <asm/kexec.h>
- .globl relocate_new_kernel -relocate_new_kernel: + .align 3 /* not needed for this code, but keeps fncpy() happy */ + +ENTRY(relocate_new_kernel)
ldr r0,kexec_indirection_page ldr r1,kexec_start_address @@ -79,6 +81,8 @@ kexec_mach_type: kexec_boot_atags: .long 0x0
+ENDPROC(relocate_new_kernel) + relocate_new_kernel_end:
.globl relocate_new_kernel_size
On Fri, Nov 08, 2013 at 12:24:04PM +0000, Dave Martin wrote:
Copying a function with memcpy() and then trying to execute the result isn't trivially portable to Thumb.
This patch modifies the kexec soft restart code to copy its assembler trampoline relocate_new_kernel() using fncpy() instead, so that relocate_new_kernel can be in the same ISA as the rest of the kernel without problems.
Signed-off-by: Dave Martin Dave.Martin@arm.com
Changes since v1:
- Move ENDPROC() after relocate_new_kernel's literals, to be consistent the location of relocate_new_kernel_end and with the way GCC sets ELF symbol sizes for functions. This is just a tidyup, with no functional impact.
Cheers Dave:
Acked-by: Will Deacon will.deacon@arm.com
Will
On Fri, Nov 08, 2013 at 01:34:27PM +0000, Will Deacon wrote:
On Fri, Nov 08, 2013 at 12:24:04PM +0000, Dave Martin wrote:
Copying a function with memcpy() and then trying to execute the result isn't trivially portable to Thumb.
This patch modifies the kexec soft restart code to copy its assembler trampoline relocate_new_kernel() using fncpy() instead, so that relocate_new_kernel can be in the same ISA as the rest of the kernel without problems.
Signed-off-by: Dave Martin Dave.Martin@arm.com
Changes since v1:
- Move ENDPROC() after relocate_new_kernel's literals, to be consistent the location of relocate_new_kernel_end and with the way GCC sets ELF symbol sizes for functions. This is just a tidyup, with no functional impact.
Cheers Dave:
Acked-by: Will Deacon will.deacon@arm.com
Thanks
Are you still in a position to test this, Taras?
Cheers ---Dave
Hi Dave
Yes. I've tested it on Pandaboard and results are quite weird. ARM->ARM, Thumb->Thumb and Thumb->ARM kernel transition works fine for both kexec and kdump ways. But ARM->Thumb works for only kdump via kernel panic. In case of "kexec -e" the second Thumb kernel doesn't come up.
I don't have JTAG now. I will check this tomorrow morning.
On 8 November 2013 20:46, Dave Martin Dave.Martin@arm.com wrote:
On Fri, Nov 08, 2013 at 01:34:27PM +0000, Will Deacon wrote:
On Fri, Nov 08, 2013 at 12:24:04PM +0000, Dave Martin wrote:
Copying a function with memcpy() and then trying to execute the result isn't trivially portable to Thumb.
This patch modifies the kexec soft restart code to copy its assembler trampoline relocate_new_kernel() using fncpy() instead, so that relocate_new_kernel can be in the same ISA as the rest of the kernel without problems.
Signed-off-by: Dave Martin Dave.Martin@arm.com
Changes since v1:
- Move ENDPROC() after relocate_new_kernel's literals, to be consistent the location of relocate_new_kernel_end and with the way GCC sets ELF symbol sizes for functions. This is just a tidyup, with no functional impact.
Cheers Dave:
Acked-by: Will Deacon will.deacon@arm.com
Thanks
Are you still in a position to test this, Taras?
Cheers ---Dave
On 11/12/2013 09:29 PM, Taras Kondratiuk wrote:
Hi Dave
Yes. I've tested it on Pandaboard and results are quite weird. ARM->ARM, Thumb->Thumb and Thumb->ARM kernel transition works fine for both kexec and kdump ways. But ARM->Thumb works for only kdump via kernel panic. In case of "kexec -e" the second Thumb kernel doesn't come up.
I don't have JTAG now. I will check this tomorrow morning.
Hi Dave, Will
The issue I observed is not caused by this patch. I was able to reproduce it with my initial simple patch.
So for this one: Reported-and-Tested-by: Taras Kondratiuk taras.kondratiuk@linaro.org
And the issue I'm frequently facing in reloaded kernel (Thumb from ARM) is random crashes caused by undefined instructions.
My observation summary: - Before starting a second kernel I'm dumping loaded zImage and then unpacked Image at final location and they are correct, so no issue with loading. - I observe two types of crash: 1) Undefined instruction in the middle of kernel code. After a crash I check failing address and there is always a *valid* Thumb instruction (CPU is in Thumb mode). 2) Jump to a wrong address which consequently causes undefined instruction exception. A trace of one example of a wrong jump is captured in [1]. Instead of jumping to 0xC049097C code gets executed at 0xED85E008. BTW the wrong address suspiciously looks like an ARM instruction. - If second kernel is placed at different address (like in kdump case), then it boots fine and I don't observe any crashes. - If I check failing address in the first kernel (ARM) the code there is really undefined instruction if executed as Thumb. - Looks like pieces of old ARM kernel gets executed instead of new Thumb kernel. But as I've mentioned I'm reading physical memory via JTAG before starting second kernel and memory is matching a compiled Thumb 'Image'. Icache also gets cleaned... - Once when stopped on breakpoint I've seen a piece of ARM code in Thumb kernel. Interesting that I was looking at the same memory location via physical and virtual addresses simultaneously and only virtual address showed an old code. After a few memory browsing operations, data at both addresses got synced to correct Thumb code. Sure it could be a debugger lag, but it fits nicely with other observations.
Do you have some ideas what could cause such behavior?
Unfortunately I don't have more time now to debug it further, but I will try to return to this later.
[1] https://drive.google.com/file/d/0ByfnRzd5ZYtdQWJKc1k0VmxrZlE/edit?usp=sharin...
On Fri, Nov 15, 2013 at 01:28:21PM +0200, Taras Kondratiuk wrote:
On 11/12/2013 09:29 PM, Taras Kondratiuk wrote:
Hi Dave
Yes. I've tested it on Pandaboard and results are quite weird. ARM->ARM, Thumb->Thumb and Thumb->ARM kernel transition works fine for both kexec and kdump ways. But ARM->Thumb works for only kdump via kernel panic. In case of "kexec -e" the second Thumb kernel doesn't come up.
I don't have JTAG now. I will check this tomorrow morning.
Hi Dave, Will
The issue I observed is not caused by this patch. I was able to reproduce it with my initial simple patch.
So for this one: Reported-and-Tested-by: Taras Kondratiuk taras.kondratiuk@linaro.org
Thanks for that.
And the issue I'm frequently facing in reloaded kernel (Thumb from ARM) is random crashes caused by undefined instructions.
My observation summary:
- Before starting a second kernel I'm dumping loaded zImage and then unpacked Image at final location and they are correct, so no issue with loading.
- I observe two types of crash:
- Undefined instruction in the middle of kernel code. After a crash I check failing address and there is always a *valid* Thumb instruction (CPU is in Thumb mode).
- Jump to a wrong address which consequently causes undefined instruction exception. A trace of one example of a wrong jump is captured in [1]. Instead of jumping to 0xC049097C code gets executed at 0xED85E008. BTW the wrong address suspiciously looks like an ARM instruction.
That jump to 0xED85E008 certainly looks strange ... I wonder whether there could be some instructions missing from the trace.
How early do these crashes happen?
Is this happening on SMP, and if so, what is the state of secondary CPUs across kexec?
If secondary CPUs are not safely parked, or their caches are not drained before the kexec occurs, this can cause corruption of the new kernel or unpredictable behaviour of the secondary CPUs.
- If second kernel is placed at different address (like in kdump case), then it boots fine and I don't observe any crashes.
- If I check failing address in the first kernel (ARM) the code there is really undefined instruction if executed as Thumb.
- Looks like pieces of old ARM kernel gets executed instead of new Thumb kernel. But as I've mentioned I'm reading physical memory via JTAG before starting second kernel and memory is matching a compiled Thumb 'Image'. Icache also gets cleaned...
- Once when stopped on breakpoint I've seen a piece of ARM code in Thumb kernel. Interesting that I was looking at the same memory
Thumb kernels do contain a small amount of ARM code, in the vectors page for example. But it's possible you were also looking at stale data.
location via physical and virtual addresses simultaneously and only virtual address showed an old code. After a few memory browsing
It's possible that those views could be inconsistent either due to the behaviour of the debugger, or because inconsistent memory types are used to construct the two views.
operations, data at both addresses got synced to correct Thumb code. Sure it could be a debugger lag, but it fits nicely with other observations.
Do you have some ideas what could cause such behavior?
Not really, apart from the above ideas.
Unfortunately I don't have more time now to debug it further, but I will try to return to this later.
OK ... let me know if you see this again or get any more clues.
Cheers ---Dave
[1] https://drive.google.com/file/d/0ByfnRzd5ZYtdQWJKc1k0VmxrZlE/edit?usp=sharin...
-- Taras Kondratiuk
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 11/15/2013 07:38 PM, Dave Martin wrote:
On Fri, Nov 15, 2013 at 01:28:21PM +0200, Taras Kondratiuk wrote:
And the issue I'm frequently facing in reloaded kernel (Thumb from ARM) is random crashes caused by undefined instructions.
My observation summary:
- Before starting a second kernel I'm dumping loaded zImage and then unpacked Image at final location and they are correct, so no issue with loading.
- I observe two types of crash:
- Undefined instruction in the middle of kernel code. After a crash I check failing address and there is always a *valid* Thumb instruction (CPU is in Thumb mode).
- Jump to a wrong address which consequently causes undefined instruction exception. A trace of one example of a wrong jump is captured in [1]. Instead of jumping to 0xC049097C code gets executed at 0xED85E008. BTW the wrong address suspiciously looks like an ARM instruction.
That jump to 0xED85E008 certainly looks strange ... I wonder whether there could be some instructions missing from the trace.
How early do these crashes happen?
At very early stages starting from setup_arch() up to early initcalls.
Is this happening on SMP, and if so, what is the state of secondary CPUs across kexec?
I have disabled CONFIG_SMP. Second CPU is busy-looping in ROM code and shouldn't cause any issues.
If secondary CPUs are not safely parked, or their caches are not drained before the kexec occurs, this can cause corruption of the new kernel or unpredictable behaviour of the secondary CPUs.
- If second kernel is placed at different address (like in kdump case), then it boots fine and I don't observe any crashes.
- If I check failing address in the first kernel (ARM) the code there is really undefined instruction if executed as Thumb.
- Looks like pieces of old ARM kernel gets executed instead of new Thumb kernel. But as I've mentioned I'm reading physical memory via JTAG before starting second kernel and memory is matching a compiled Thumb 'Image'. Icache also gets cleaned...
- Once when stopped on breakpoint I've seen a piece of ARM code in Thumb kernel. Interesting that I was looking at the same memory
Thumb kernels do contain a small amount of ARM code, in the vectors page for example. But it's possible you were also looking at stale data.
Right, but I mean there was an ARM code in place where definitely a Thumb code should be.
location via physical and virtual addresses simultaneously and only virtual address showed an old code. After a few memory browsing
It's possible that those views could be inconsistent either due to the behaviour of the debugger, or because inconsistent memory types are used to construct the two views.
operations, data at both addresses got synced to correct Thumb code. Sure it could be a debugger lag, but it fits nicely with other observations.
Do you have some ideas what could cause such behavior?
Not really, apart from the above ideas.
Unfortunately I don't have more time now to debug it further, but I will try to return to this later.
OK ... let me know if you see this again or get any more clues.
Cheers ---Dave
[1] https://drive.google.com/file/d/0ByfnRzd5ZYtdQWJKc1k0VmxrZlE/edit?usp=sharin...
-- Taras Kondratiuk
On Fri, Nov 15, 2013 at 06:11:43PM +0000, Taras Kondratiuk wrote:
On 11/15/2013 07:38 PM, Dave Martin wrote:
On Fri, Nov 15, 2013 at 01:28:21PM +0200, Taras Kondratiuk wrote:
And the issue I'm frequently facing in reloaded kernel (Thumb from ARM) is random crashes caused by undefined instructions.
My observation summary:
- Before starting a second kernel I'm dumping loaded zImage and then unpacked Image at final location and they are correct, so no issue with loading.
- I observe two types of crash:
- Undefined instruction in the middle of kernel code. After a crash I check failing address and there is always a *valid* Thumb instruction (CPU is in Thumb mode).
- Jump to a wrong address which consequently causes undefined instruction exception. A trace of one example of a wrong jump is captured in [1]. Instead of jumping to 0xC049097C code gets executed at 0xED85E008. BTW the wrong address suspiciously looks like an ARM instruction.
That jump to 0xED85E008 certainly looks strange ... I wonder whether there could be some instructions missing from the trace.
How early do these crashes happen?
At very early stages starting from setup_arch() up to early initcalls.
Is this happening on SMP, and if so, what is the state of secondary CPUs across kexec?
I have disabled CONFIG_SMP. Second CPU is busy-looping in ROM code and shouldn't cause any issues.
OK, that sounds reasonable.
If secondary CPUs are not safely parked, or their caches are not drained before the kexec occurs, this can cause corruption of the new kernel or unpredictable behaviour of the secondary CPUs.
- If second kernel is placed at different address (like in kdump case), then it boots fine and I don't observe any crashes.
- If I check failing address in the first kernel (ARM) the code there is really undefined instruction if executed as Thumb.
- Looks like pieces of old ARM kernel gets executed instead of new Thumb kernel. But as I've mentioned I'm reading physical memory via JTAG before starting second kernel and memory is matching a compiled Thumb 'Image'. Icache also gets cleaned...
- Once when stopped on breakpoint I've seen a piece of ARM code in Thumb kernel. Interesting that I was looking at the same memory
Thumb kernels do contain a small amount of ARM code, in the vectors page for example. But it's possible you were also looking at stale data.
Right, but I mean there was an ARM code in place where definitely a Thumb code should be.
Sure. Well, I guess this remains unexplained for now, but keep me posted.
Cheers ---Dave
linaro-kernel@lists.linaro.org
-
Dave Martin -
Dave P Martin
-
Taras Kondratiuk -
Will Deacon