[PATCH] ARM: kdump: copy kernel relocation code at the kexec prepare stage

[Lei Wen]

Hi Russell,

> -----Original Message-----
> From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]
> Sent: Monday, September 19, 2011 8:21 PM
> To: Lei Wen
> Cc: Chao Xie; Yu Tang; linux-arm-kernel at lists.infradead.org;
> kexec at lists.infradead.org
> Subject: Re: [PATCH] ARM: kdump: copy kernel relocation code at the kexec prepare
> stage
> 
> On Thu, Sep 15, 2011 at 10:32:09PM -0700, Lei Wen wrote:
> > diff --git a/arch/arm/kernel/machine_kexec.c
> b/arch/arm/kernel/machine_kexec.c
> > index e59bbd4..f60fc90 100644
> > --- a/arch/arm/kernel/machine_kexec.c
> > +++ b/arch/arm/kernel/machine_kexec.c
> > @@ -32,6 +32,21 @@ static atomic_t waiting_for_crash_ipi;
> >
> >  int machine_kexec_prepare(struct kimage *image)
> >  {
> > +	unsigned long page_list;
> > +	void *reboot_code_buffer;
> > +	page_list = image->head & PAGE_MASK;
> > +
> > +	reboot_code_buffer = page_address(image->control_code_page);
> > +
> > +	/* Prepare parameters for reboot_code_buffer*/
> > +	kexec_start_address = image->start;
> > +	kexec_indirection_page = page_list;
> > +	kexec_mach_type = machine_arch_type;
> > +	kexec_boot_atags = image->start - KEXEC_ARM_ZIMAGE_OFFSET +
> KEXEC_ARM_ATAGS_OFFSET;
> > +
> > +	/* copy our kernel relocation code to the control code page */
> > +	memcpy(reboot_code_buffer,
> > +	       relocate_new_kernel, relocate_new_kernel_size);
> >  	return 0;
> >  }
> >
> > @@ -82,29 +97,14 @@ void (*kexec_reinit)(void);
> >
> >  void machine_kexec(struct kimage *image)
> >  {
> > -	unsigned long page_list;
> >  	unsigned long reboot_code_buffer_phys;
> >  	void *reboot_code_buffer;
> >
> > -
> > -	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);
> >
> > -	/* Prepare parameters for reboot_code_buffer*/
> > -	kexec_start_address = image->start;
> > -	kexec_indirection_page = page_list;
> > -	kexec_mach_type = machine_arch_type;
> > -	kexec_boot_atags = image->start - KEXEC_ARM_ZIMAGE_OFFSET +
> KEXEC_ARM_ATAGS_OFFSET;
> > -
> > -	/* copy our kernel relocation code to the control code page */
> > -	memcpy(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);
> 
> You should keep this flush with the memcpy - the two are intimately
> related - the flush is to ensure I/D cache coherency for the code
> which was copied into the page.  Splitting them into two different
> functions is asking for future bugs.

Thanks for this reminding!
Then how about add flush_icache_range at the end of machine_kexec_prepare? Could it be acceptable?

Also with this change, do we still need flush icache at the end stage of machine_kexec?

Thanks,
Lei