Re: [PATCH v3 4/4] x86: Pass memory range via E820 for kdump

[WANG Chao <chaowang@redhat.com>]

On 03/13/14 at 04:24pm, Linn Crosetto wrote:
> Wang Chao,
> 
> On Mon, Mar 10, 2014 at 11:09:09PM +0800, WANG Chao wrote:
> > command line size is restricted by kernel, sometimes memmap=exactmap has
> > too many memory ranges to pass to cmdline. A better approach, to pass the
> > memory ranges for crash kernel to boot into, is filling the memory
> > ranges into E820.
> > 
> > boot_params only got 128 slots for E820 map to fit in, when the number of
> > memory map exceeds 128, use setup_data to pass the rest as extended E820
> > memory map.
> > 
> > kexec boot could also benefit from setup_data in case E820 memory map
> > exceeds 128.
> > 
> > Now this new approach becomes default instead of memmap=exactmap.
> > saved_max_pfn users can specify --pass-memmap-cmdline to use the
> > exactmap approach.
> > 
> > Signed-off-by: WANG Chao <chaowang@redhat.com>
> > ---
> >  kexec/arch/i386/crashdump-x86.c   |  25 +++---
> >  kexec/arch/i386/crashdump-x86.h   |   1 +
> >  kexec/arch/i386/x86-linux-setup.c | 171 +++++++++++++++++++++++++-------------
> >  3 files changed, 130 insertions(+), 67 deletions(-)
> > 
> 
> [..]
> 
> > +static void setup_e820_ext(struct kexec_info *info, struct x86_linux_param_header *real_mode,
> > +			   struct memory_range *range, int nr_range)
> > +{
> > +	struct setup_data *sd;
> > +	struct e820entry *e820;
> > +	int nr_range_ext;
> > +
> > +	nr_range_ext = nr_range - E820MAX;
> > +	sd = malloc(sizeof(struct setup_data) + nr_range_ext * sizeof(struct e820entry));
> 
> You might check the return value from malloc().

Right.
I'll use xmalloc(), defined in kexec/kexec.h.

> 
> > +	sd->next = 0;
> > +	sd->len = nr_range_ext * sizeof(struct e820entry);
> > +	sd->type = SETUP_E820_EXT;
> > +
> > +	e820 = (struct e820entry *) sd->data;
> > +	dbgprintf("Extended E820 via setup_data:\n");
> > +	add_e820_map_from_mr(real_mode, e820, range + E820MAX, nr_range_ext);
> > +	add_setup_data(info, real_mode, sd);
> > +}
> 
> [..]
> 
> I tested on a prototype system with 231 entries in the map with good results.
> Everything succeeds when using kexec to initiate a fast reboot. For crash, it
> works with and without --pass-memmap-cmdline when using noefi. I hit the
> following panic when initiating a crash leaving EFI enabled in the crash kernel:

Good to know!

> 
>  ? __unmap_pmd_range+0x77/0x190
>  unmap_pmd_range+0xcf/0x1c0
>  populate_pgd+0x16d/0x250
>  __cpa_process_fault+0x15/0xb0
>  __change_page_attr+0x15e/0x2a0
>  __change_page_attr_set_clr+0x59/0xc0
>  kernel_map_pages_in_pgd+0x7a/0xb0
>  __map_region+0x46/0x64
>  ? early_idt_handlers+0x117/0x120
>  efi_map_region_fixed+0xd/0xf
>  efi_enter_virtual_mode+0x4c/0x476
>  ? early_idt_handlers+0x117/0x120
>  ? early_idt_handlers+0x117/0x120
>  start_kernel+0x2e5/0x376
>  ? repair_env_string+0x5b/0x5b
>  ? memblock_reserve+0x49/0x4e
>  x86_64_start_reservations+0x2a/0x2c
>  x86_64_start_kernel+0x19f/0x1ae
> 
> However, I was able to reproduce this panic using an earlier version of
> kexec-tools, so I believe it is unrelated to this patch.
> 
> For this patch:
> 
> Reviewed-by: Linn Crosetto <linn@hp.com>
> 
> For the series:
> 
> Tested-by: Linn Crosetto <linn@hp.com>

Thank you for your review and test. I really appreciate it.

WANG Chao

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