[PATCH] Load crash kernel high on x86

[Minfei Huang]

On 09/18/15 at 03:11pm, Petr Tesarik wrote:
> On Fri, 18 Sep 2015 19:39:02 +0800
> Minfei Huang <mhuang at redhat.com> wrote:
> 
> > On 09/18/15 at 08:03am, Petr Tesarik wrote:
> > > Hello,
> > > 
> > > There may be more than one crash kernel regions on x86. However, the kexec
> > > syscall checks that target address is within crashk_res boundaries. Looking
> > > at the logic in arch/x86/kernel/setup.c, there are only two possible layouts:
> > > 
> > >   1. crashk_res is below 4G, and there is only one region,
> > >   2. crashk_res is above 4G, and crashk_low_res is below 4G
> > > 
> > > In either case, kexec-tools must pick the highest region.
> > > 
> > > Currently, kexec-tools picks the largest region. If high reservation is
> > > smaller than low, kexec(2) returns -EADDRNOTAVAIL, and kexec prints out
> > > this error message:
> > > 
> > > kexec_load failed: Cannot assign requested address
> > > 
> > > Signed-off-by: Petr Tesarik <ptesarik at suse.com>
> > > ---
> > >  kexec/arch/i386/crashdump-x86.c | 12 ++----------
> > >  1 file changed, 2 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/kexec/arch/i386/crashdump-x86.c b/kexec/arch/i386/crashdump-x86.c
> > > index 63959b7..2710a9e 100644
> > > --- a/kexec/arch/i386/crashdump-x86.c
> > > +++ b/kexec/arch/i386/crashdump-x86.c
> > > @@ -1034,16 +1034,8 @@ int get_max_crash_kernel_limit(uint64_t *start, uint64_t *end)
> > >  	if (!crash_reserved_mem_nr)
> > >  		return -1;
> > >  
> > > -	for (i = crash_reserved_mem_nr - 1; i >= 0; i--) {
> > > -		sz = crash_reserved_mem[i].end - crash_reserved_mem[i].start +1;
> > > -		if (sz <= sz_max)
> > > -			continue;
> > > -		sz_max = sz;
> > > -		idx = i;
> > > -	}
> > > -
> > > -	*start = crash_reserved_mem[idx].start;
> > > -	*end = crash_reserved_mem[idx].end;
> > > +	*start = crash_reserved_mem[crash_reserved_mem_nr - 1].start;
> > > +	*end = crash_reserved_mem[crash_reserved_mem_nr - 1].end;
> > >  
> > >  	return 0;
> > 
> > It is not a proper function name for what it does in your patch. How
> 
> True. It should be now called something like get_crashk_res_location().
> I'm going to change that.
> 
> > about get_reserved_mem_limit()? Also it is appreciative, if you can
> > comment it more detail.
> 
> I'm not sure what would be more detail. Regarding the motivation, I
> have already documented why it fails and what are the symptoms of
> failure. Regarding the implementation, the code speaks for itself, IMO.
> 
> Can you explain what kind of detail you're missing, please?

Sorry, What I mean is to comment above the function why we choose the
region crash_reserved_mem[crash_reserved_mem_nr - 1].

Thanks
Minfei