Re: [RESEND PATCH 2/2] ARM: kexec: Check segment memory addresses

Re: [RESEND PATCH 2/2] ARM: kexec: Check segment memory addresses

[Aaro Koskinen]

Hi,

On Thu, Sep 20, 2012 at 05:03:38PM +0100, Will Deacon wrote:
> From: Matthew Leach <matthew.leach@arm.com>
> 
> Ensure that the memory regions that are set within the segments
> correspond to physical contiguous memory regions.

This patch seems to break kexec for me in 3.7-rc1 (commit
c564df4db85aac8d1d65a56176a0a25f46138064, bisected).

kexec -l fails with the following output:

kexec_load failed: Invalid argument
entry       = 0x80008000 flags = 280000
nr_segments = 2
segment[0].buf   = 0xc48008
segment[0].bufsz = 230
segment[0].mem   = 0x80001000
segment[0].memsz = 1000
segment[1].buf   = 0xb6a9b008
segment[1].bufsz = 3a4668
segment[1].mem   = 0x80008000
segment[1].memsz = 3a5000
Could not load the kexec kernel: n800-zImage

The board is OMAP2420 / Nokia N800.

A.

Re: [RESEND PATCH 2/2] ARM: kexec: Check segment memory addresses

[Will Deacon]

On Tue, Oct 16, 2012 at 05:10:21PM +0100, Aaro Koskinen wrote:
> Hi,

Hi Aaro,

> On Thu, Sep 20, 2012 at 05:03:38PM +0100, Will Deacon wrote:
> > From: Matthew Leach <matthew.leach@arm.com>
> > 
> > Ensure that the memory regions that are set within the segments
> > correspond to physical contiguous memory regions.
> 
> This patch seems to break kexec for me in 3.7-rc1 (commit
> c564df4db85aac8d1d65a56176a0a25f46138064, bisected).
> 
> kexec -l fails with the following output:
> 
> kexec_load failed: Invalid argument
> entry       = 0x80008000 flags = 280000
> nr_segments = 2
> segment[0].buf   = 0xc48008
> segment[0].bufsz = 230
> segment[0].mem   = 0x80001000
> segment[0].memsz = 1000
> segment[1].buf   = 0xb6a9b008
> segment[1].bufsz = 3a4668
> segment[1].mem   = 0x80008000
> segment[1].memsz = 3a5000
> Could not load the kexec kernel: n800-zImage
> 
> The board is OMAP2420 / Nokia N800.

Interesting, it sounds like kexec thinks that you don't have contiguous
memory from 0x80008000 to 0x803ad000. Can you provide some more information
about your physical memory map please?

Cheers,

Will

Re: [RESEND PATCH 2/2] ARM: kexec: Check segment memory addresses

[Aaro Koskinen]

On Tue, Oct 16, 2012 at 05:32:26PM +0100, Will Deacon wrote:
> On Tue, Oct 16, 2012 at 05:10:21PM +0100, Aaro Koskinen wrote:
> > On Thu, Sep 20, 2012 at 05:03:38PM +0100, Will Deacon wrote:
> > > From: Matthew Leach <matthew.leach@arm.com>
> > > 
> > > Ensure that the memory regions that are set within the segments
> > > correspond to physical contiguous memory regions.
> > 
> > This patch seems to break kexec for me in 3.7-rc1 (commit
> > c564df4db85aac8d1d65a56176a0a25f46138064, bisected).
> > 
> > kexec -l fails with the following output:
> > 
> > kexec_load failed: Invalid argument
> > entry       = 0x80008000 flags = 280000
> > nr_segments = 2
> > segment[0].buf   = 0xc48008
> > segment[0].bufsz = 230
> > segment[0].mem   = 0x80001000
> > segment[0].memsz = 1000
> > segment[1].buf   = 0xb6a9b008
> > segment[1].bufsz = 3a4668
> > segment[1].mem   = 0x80008000
> > segment[1].memsz = 3a5000
> > Could not load the kexec kernel: n800-zImage
> > 
> > The board is OMAP2420 / Nokia N800.
> 
> Interesting, it sounds like kexec thinks that you don't have contiguous
> memory from 0x80008000 to 0x803ad000. Can you provide some more information
> about your physical memory map please?

Well, I think it's because the patch is wrong. Shouldn't it be:

diff --git a/arch/arm/kernel/machine_kexec.c b/arch/arm/kernel/machine_kexec.c
index e29c333..a80192e 100644
--- a/arch/arm/kernel/machine_kexec.c
+++ b/arch/arm/kernel/machine_kexec.c
@@ -47,7 +47,7 @@ int machine_kexec_prepare(struct kimage *image)
 
 		err = memblock_is_region_memory(current_segment->mem,
 						current_segment->memsz);
-		if (err)
+		if (!err)
 			return - EINVAL;
 
 		err = get_user(header, (__be32*)current_segment->buf);

A.

Re: [RESEND PATCH 2/2] ARM: kexec: Check segment memory addresses

[Will Deacon]

On Tue, Oct 16, 2012 at 06:09:00PM +0100, Aaro Koskinen wrote:
> On Tue, Oct 16, 2012 at 05:32:26PM +0100, Will Deacon wrote:
> > Interesting, it sounds like kexec thinks that you don't have contiguous
> > memory from 0x80008000 to 0x803ad000. Can you provide some more information
> > about your physical memory map please?
> 
> Well, I think it's because the patch is wrong. Shouldn't it be:
> 
> diff --git a/arch/arm/kernel/machine_kexec.c b/arch/arm/kernel/machine_kexec.c
> index e29c333..a80192e 100644
> --- a/arch/arm/kernel/machine_kexec.c
> +++ b/arch/arm/kernel/machine_kexec.c
> @@ -47,7 +47,7 @@ int machine_kexec_prepare(struct kimage *image)
>  
>  		err = memblock_is_region_memory(current_segment->mem,
>  						current_segment->memsz);
> -		if (err)
> +		if (!err)
>  			return - EINVAL;
>  
>  		err = get_user(header, (__be32*)current_segment->buf);

Oops, that's a howler! Thanks for spotting it. We should probably reflow the
code a bit because !err sounds like everything should be ok.

Fancy reworking the patch or do you want me to take care of this?

Cheers,

Will