[KJ PATCH] Replacing memcpy(dest,src,PAGE_SIZE) with copy_page(dest,src) in arch/i386/kernel/machine_kexec.c

[KJ PATCH] Replacing memcpy(dest,src,PAGE_SIZE) with copy_page(dest,src) in arch/i386/kernel/machine_kexec.c

[Shani Moideen]

Hi,
Replacing memcpy(dest,src,PAGE_SIZE) with copy_page(dest,src) in arch/i386/kernel/machine_kexec.c.

Signed-off-by: Shani Moideen <shani.moideen@wipro.com>
----

diff --git a/arch/i386/kernel/machine_kexec.c b/arch/i386/kernel/machine_kexec.c
index 91966ba..ce79a44 100644
--- a/arch/i386/kernel/machine_kexec.c
+++ b/arch/i386/kernel/machine_kexec.c
@@ -110,7 +110,7 @@ NORET_TYPE void machine_kexec(struct kimage *image)
 	local_irq_disable();
 
 	control_page = page_address(image->control_code_page);
-	memcpy(control_page, relocate_kernel, PAGE_SIZE);
+	copy_page(control_page, relocate_kernel);
 
 	page_list[PA_CONTROL_PAGE] = __pa(control_page);
 	page_list[VA_CONTROL_PAGE] = (unsigned long)relocate_kernel;

-- 
Shani 

Re: [KJ PATCH] Replacing memcpy(dest,src,PAGE_SIZE) with copy_page(dest,src) in arch/i386/kernel/machine_kexec.c

[Eric W. Biederman]

Shani Moideen <shani.moideen@wipro.com> writes:

> Hi,
> Replacing memcpy(dest,src,PAGE_SIZE) with copy_page(dest,src) in
> arch/i386/kernel/machine_kexec.c.

Please no.

People get creative in copy_page (especially mmx_copy_page),
and this code path need something simple and stupid, that
will work every time, especially when things are messed up
elsewhere.

Ideally we would actually do all of the setup before this point.
but that is another issue entirely.

Eric

Re: [KJ PATCH] Replacing memcpy(dest,src,PAGE_SIZE) with copy_page(dest,src) in arch/i386/kernel/machine_kexec.c

[H. Peter Anvin]

Eric W. Biederman wrote:
> Shani Moideen <shani.moideen@wipro.com> writes:
> 
>> Hi,
>> Replacing memcpy(dest,src,PAGE_SIZE) with copy_page(dest,src) in
>> arch/i386/kernel/machine_kexec.c.
> 
> Please no.
> 
> People get creative in copy_page (especially mmx_copy_page),
> and this code path need something simple and stupid, that
> will work every time, especially when things are messed up
> elsewhere.
> 

memcpy() is pretty creative, too.  If you want something really dumb and
robust, you should probably create an inline:

/*
 * Eric sez: his code path need something simple and stupid, that
 * will work every time, especially when things are messed up
 * elsewhere.
 */
static inline void braindead_copy_page(void *dst, void *src)
{
	unsigned long ctr = PAGE_SIZE >> 2;

	asm volatile("cld; rep; movsl"
			: "+D" (dst), "+S" (src), "+c" (ctr)
			: : "memory");
}

(FWIW, the "cld" is supposed to be redundant, as DF=0 is supposed to be
guaranteed by the ABI.)

	-hpa

Re: [KJ PATCH] Replacing memcpy(dest,src,PAGE_SIZE) with copy_page(dest,src) in arch/i386/kernel/machine_kexec.c

[Pavel Machek]

Hi!

> >> Replacing memcpy(dest,src,PAGE_SIZE) with copy_page(dest,src) in
> >> arch/i386/kernel/machine_kexec.c.
> > 
> > Please no.
> > 
> > People get creative in copy_page (especially mmx_copy_page),
> > and this code path need something simple and stupid, that
> > will work every time, especially when things are messed up
> > elsewhere.
> 
> memcpy() is pretty creative, too.  If you want something really dumb and
> robust, you should probably create an inline:
> 
> /*
>  * Eric sez: his code path need something simple and stupid, that
>  * will work every time, especially when things are messed up
>  * elsewhere.
>  */
> static inline void braindead_copy_page(void *dst, void *src)
> {
> 	unsigned long ctr = PAGE_SIZE >> 2;
> 
> 	asm volatile("cld; rep; movsl"
> 			: "+D" (dst), "+S" (src), "+c" (ctr)
> 			: : "memory");
> }
> 
> (FWIW, the "cld" is supposed to be redundant, as DF=0 is supposed to be
> guaranteed by the ABI.)

Actually, swsusp would be second user for that. We currently do it
with for loop in C, but...

...well, we'd probably need it for i386, ppc, x86-64, ppc64 and maybe
arm...

(But you should probably add a comment into kexec )
							Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html