On Thu, Oct 10, 2013 at 11:36:08PM +0300, Taras Kondratiuk wrote:
> On 10 October 2013 17:12, Dave Martin <Dave.Martin(a)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.
Ah, right.
I think that the fncpy() function should work. This is for the precise
purpsoe of copying a function body from one place to another, while
reatining the Thumb bit.
I have to disappear early to today, but I'll try and follow up.
Currently, I have the following, but I've not been able to test it yet.
If you'd like to give it a try, that would be much appreciated.
Cheers
---Dave
>From 94925667e650202e1baf74e1ff223671d316401d Mon Sep 17 00:00:00 2001
From: Dave Martin <Dave.Martin(a)arm.com>
Date: Tue, 15 Oct 2013 11:48:46 +0100
Subject: [PATCH] ARM: kexec: Use the right ISA for relocate_new_kernel
Copying a function with memcpy() and then trying to execute the
result isn't 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(a)arm.com>
---
arch/arm/kernel/machine_kexec.c | 20 +++++++++-----------
arch/arm/kernel/relocate_kernel.S | 7 ++++---
2 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/arch/arm/kernel/machine_kexec.c b/arch/arm/kernel/machine_kexec.c
index 57221e3..ce135b3 100644
--- a/arch/arm/kernel/machine_kexec.c
+++ b/arch/arm/kernel/machine_kexec.c
@@ -11,6 +11,7 @@
#include <linux/memblock.h>
#include <asm/pgtable.h>
#include <linux/of_fdt.h>
+#include <asm/fncpy.h>
#include <asm/pgalloc.h>
#include <asm/mmu_context.h>
#include <asm/cacheflush.h>
@@ -18,7 +19,7 @@
#include <asm/smp_plat.h>
#include <asm/system_misc.h>
-extern const unsigned char relocate_new_kernel[];
+extern char relocate_new_kernel;
extern const unsigned int relocate_new_kernel_size;
extern unsigned long kexec_start_address;
@@ -141,8 +142,9 @@ void (*kexec_reinit)(void);
void machine_kexec(struct kimage *image)
{
unsigned long page_list;
- unsigned long reboot_code_buffer_phys;
void *reboot_code_buffer;
+ unsigned long v2p_offset;
+ void *entry_point;
/*
* This can only happen if machine_shutdown() failed to disable some
@@ -154,10 +156,9 @@ void machine_kexec(struct kimage *image)
page_list = image->head & PAGE_MASK;
- /* we need both effective and real address here */
- reboot_code_buffer_phys =
- page_to_pfn(image->control_code_page) << PAGE_SHIFT;
reboot_code_buffer = page_address(image->control_code_page);
+ v2p_offset = (page_to_pfn(image->control_code_page) << PAGE_SHIFT)
+ - (unsigned long)reboot_code_buffer;
/* Prepare parameters for reboot_code_buffer*/
kexec_start_address = image->start;
@@ -168,16 +169,13 @@ 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);
+ entry_point = fncpy(reboot_code_buffer,
+ &relocate_new_kernel, relocate_new_kernel_size);
-
- 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((unsigned long)entry_point + v2p_offset);
}
diff --git a/arch/arm/kernel/relocate_kernel.S b/arch/arm/kernel/relocate_kernel.S
index d0cdedf..1492930 100644
--- a/arch/arm/kernel/relocate_kernel.S
+++ b/arch/arm/kernel/relocate_kernel.S
@@ -2,11 +2,11 @@
* 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
+ENTRY(relocate_new_kernel)
ldr r0,kexec_indirection_page
ldr r1,kexec_start_address
@@ -59,6 +59,7 @@ relocate_new_kernel:
ldr r2,kexec_boot_atags
ARM( mov pc, lr )
THUMB( bx lr )
+ENDPROC(relocate_new_kernel)
.align
--
1.7.9.5