On Fri, Nov 08, 2013 at 09:40:18AM +0000, Will Deacon wrote:

Hi Dave,

On Thu, Nov 07, 2013 at 08:51:36PM +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

[...]

...

@@ -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);
  

My only slight gripe with this is that relocate_new_kernel_size also includes a bunch of data following the function (which you have now delimited with ENTRY/ENDPROC), so using fncpy for that feels a bit awkward.

ENDPROC() is pretty much a no-op apart from determining the symbol type.

However, putting it after the literaloids will be more consistent with the GCC behaviour, even if Linux does not make any real use of the ELF symbol size information.

It would be breathtakingly sensible if there was an ELF relocation to get the size of a function symbol directly so that we wouldn't need the silly relocate_new_kernel_size symbol ... but unfortunately, there isn't.

Will make the change and repost.

Cheers ---Dave