[PATCH] Load crash kernel high on x86

[PATCH] Load crash kernel high on x86

[Petr Tesarik]

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@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;
 }
-- 
2.1.4

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] Load crash kernel high on x86

[Petr Tesarik]

On Fri, 18 Sep 2015 08:03:24 +0200
Petr Tesarik <ptesarik@suse.com> 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

I have just re-checked with kexec_file(2), and it is also affected. At
this point, you may think that it would be better to fix the check in
sanity_check_segment_list() instead.

I don't think so.

Low memory reservation is intended for swiotlb and DMA buffers, i.e. it
is somehow precious. The kernel can run above 4G, initrd can also be
located above 4G, so why should I place it in the _precious_ low memory?

If you agree, I'll also post a kernel patch for kexec_file(2).

Petr T

> Signed-off-by: Petr Tesarik <ptesarik@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;
>  }


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] Load crash kernel high on x86

[Baoquan He]

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:

For system with more than 4G memory, usually people won't reserve
crashkernel memory like that. But it's truly a bug and could happen in
some cases or people intend to make it happen. Sound a reasonable fix.

Ack.

> 
> kexec_load failed: Cannot assign requested address
> 
> Signed-off-by: Petr Tesarik <ptesarik@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;
>  }
> -- 
> 2.1.4
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] Load crash kernel high on x86

[Minfei Huang]

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@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
about get_reserved_mem_limit()? Also it is appreciative, if you can
comment it more detail.

Thanks
Minfei

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] Load crash kernel high on x86

[Petr Tesarik]

On Fri, 18 Sep 2015 19:39:02 +0800
Minfei Huang <mhuang@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@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?

Regards,
Petr Tesarik

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [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@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@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

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] Load crash kernel high on x86

[Minfei Huang]

On 09/18/15 at 10:40am, Petr Tesarik wrote:
> On Fri, 18 Sep 2015 08:03:24 +0200
> Petr Tesarik <ptesarik@suse.com> 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
> 
> I have just re-checked with kexec_file(2), and it is also affected. At
> this point, you may think that it would be better to fix the check in
> sanity_check_segment_list() instead.
> 
> I don't think so.
> 
> Low memory reservation is intended for swiotlb and DMA buffers, i.e. it
> is somehow precious. The kernel can run above 4G, initrd can also be
> located above 4G, so why should I place it in the _precious_ low memory?
> 
> If you agree, I'll also post a kernel patch for kexec_file(2).

KEXEC always use a pair of crashk_res.start and crashk_res.end as crash
memory region in kernel. And this struct is assigned in function
reserve_crashkernel during booting. In order to solve low memory issue,
there is another struct crashk_low_res is used.

So, the code flow deals with the above case correctly.

Thanks
Minfei

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] Load crash kernel high on x86

[Baoquan He]

On 09/18/15 at 10:40am, Petr Tesarik wrote:
> On Fri, 18 Sep 2015 08:03:24 +0200
> Petr Tesarik <ptesarik@suse.com> 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
> 
> I have just re-checked with kexec_file(2), and it is also affected. At
> this point, you may think that it would be better to fix the check in
> sanity_check_segment_list() instead.
> 
> I don't think so.
> 
> Low memory reservation is intended for swiotlb and DMA buffers, i.e. it
> is somehow precious. The kernel can run above 4G, initrd can also be
> located above 4G, so why should I place it in the _precious_ low memory?
> 
> If you agree, I'll also post a kernel patch for kexec_file(2).

It does not need a fix in kernel for kexec_file. Since it uses below code
to get memory in kexec_add_buffer(), this is the same as it does in
locate_hole() using mem_min and mem_max in kexec_tools.

        /* Walk the RAM ranges and allocate a suitable range for the buffer */
        if (image->type == KEXEC_TYPE_CRASH)
                ret = walk_iomem_res("Crash kernel",
                                     IORESOURCE_MEM | IORESOURCE_BUSY,
                                     crashk_res.start, crashk_res.end, kbuf,
                                     locate_mem_hole_callback);
	else
		...


Thanks
Baoquan

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec