Re: [PATCH] support Linux' advanced crashkernel= syntax

[Keir Fraser <keir.fraser@eu.citrix.com>]

A code comment explaining the cmdline format parsed would be good. Also
perhaps a more detailed patch summary: I can't really see what is going on
here. Seems to be checking user-specified 'ranges' against 'total RAM', but
I can't work out what it's all for.

 -- Keir

On 26/04/2010 08:22, "Jan Beulich" <JBeulich@novell.com> wrote:

> For x86, other than Linux we pass the actual amount of RAM rather than
> the highest page's address (to cope with sparse physical address maps).
> 
> This still needs to be hooked up for ia64.
> 
> Signed-off-by: Jan Beulich <jbeulich@novell.com>
> 
> --- 2010-04-22.orig/xen/arch/x86/setup.c 2010-04-23 00:00:00.000000000 +0200
> +++ 2010-04-22/xen/arch/x86/setup.c 2010-04-26 08:52:06.000000000 +0200
> @@ -643,6 +643,11 @@ void __init __start_xen(unsigned long mb
>      memcpy(&boot_e820, &e820, sizeof(e820));
>  
>      /* Early kexec reservation (explicit static start address). */
> +    nr_pages = 0;
> +    for ( i = 0; i < e820.nr_map; i++ )
> +        if ( e820.map[i].type == E820_RAM )
> +            nr_pages += e820.map[i].size >> PAGE_SHIFT;
> +    set_kexec_crash_area_size((u64)nr_pages << PAGE_SHIFT);
>      kexec_reserve_area(&boot_e820);
>  
>      /*
> --- 2010-04-22.orig/xen/common/kexec.c 2010-01-13 18:58:11.000000000 +0100
> +++ 2010-04-22/xen/common/kexec.c 2010-04-26 08:44:36.000000000 +0200
> @@ -47,15 +47,96 @@ static unsigned char vmcoreinfo_data[VMC
>  static size_t vmcoreinfo_size = 0;
>  
>  xen_kexec_reserve_t kexec_crash_area;
> +static struct {
> +    u64 start, end;
> +    unsigned long size;
> +} ranges[16] __initdata;
>  
>  static void __init parse_crashkernel(const char *str)
>  {
> -    kexec_crash_area.size = parse_size_and_unit(str, &str);
> -    if ( *str == '@' )
> -        kexec_crash_area.start = parse_size_and_unit(str+1, NULL);
> +    const char *cur;
> +
> +    if ( strchr(str, ':' ) )
> +    {
> +        unsigned int idx = 0;
> +
> +        do {
> +            if ( idx >= ARRAY_SIZE(ranges) )
> +            {
> +                printk(XENLOG_WARNING "crashkernel: too many ranges\n");
> +                cur = NULL;
> +                str = strchr(str, '@');
> +                break;
> +            }
> +
> +            ranges[idx].start = parse_size_and_unit(cur = str + !!idx, &str);
> +            if ( cur == str )
> +                break;
> +
> +            if ( *str != '-' )
> +            {
> +                printk(XENLOG_WARNING "crashkernel: '-' expected\n");
> +                break;
> +            }
> +
> +            if ( *++str != ':' )
> +            {
> +                ranges[idx].end = parse_size_and_unit(cur = str, &str);
> +                if ( cur == str )
> +                    break;
> +                if ( ranges[idx].end <= ranges[idx].start )
> +                {
> +                    printk(XENLOG_WARNING "crashkernel: end <= start\n");
> +                    break;
> +                }
> +            }
> +            else
> +                ranges[idx].end = -1;
> +
> +            if ( *str != ':' )
> +            {
> +                printk(XENLOG_WARNING "crashkernel: ':' expected\n");
> +                break;
> +            }
> +
> +            ranges[idx].size = parse_size_and_unit(cur = str + 1, &str);
> +            if ( cur == str )
> +                break;
> +
> +            ++idx;
> +        } while ( *str == ',' );
> +        if ( idx < ARRAY_SIZE(ranges) )
> +            ranges[idx].size = 0;
> +    }
> +    else
> +        kexec_crash_area.size = parse_size_and_unit(cur = str, &str);
> +    if ( cur != str && *str == '@' )
> +        kexec_crash_area.start = parse_size_and_unit(cur = str + 1, &str);
> +    if ( cur == str )
> +        printk(XENLOG_WARNING "crashkernel: memory value expected\n");
>  }
>  custom_param("crashkernel", parse_crashkernel);
>  
> +void __init set_kexec_crash_area_size(u64 system_ram)
> +{
> +    unsigned int idx;
> +
> +    for ( idx = 0; idx < ARRAY_SIZE(ranges) && !kexec_crash_area.size; ++idx
> )
> +    {
> +        if ( !ranges[idx].size )
> +            break;
> +
> +        if ( ranges[idx].size >= system_ram )
> +        {
> +            printk(XENLOG_WARNING "crashkernel: invalid size\n");
> +            continue;
> +        }
> +
> +        if ( ranges[idx].start <= system_ram && ranges[idx].end > system_ram
> )
> +            kexec_crash_area.size = ranges[idx].size;
> +    }
> +}
> +
>  static void one_cpu_only(void)
>  {
>      /* Only allow the first cpu to continue - force other cpus to spin */
> --- 2010-04-22.orig/xen/include/xen/kexec.h 2010-01-13 18:58:11.000000000
> +0100
> +++ 2010-04-22/xen/include/xen/kexec.h 2010-04-26 08:43:26.000000000 +0200
> @@ -12,6 +12,8 @@ typedef struct xen_kexec_reserve {
>  
>  extern xen_kexec_reserve_t kexec_crash_area;
>  
> +void set_kexec_crash_area_size(u64 system_ram);
> +
>  /* We have space for 4 images to support atomic update
>   * of images. This is important for CRASH images since
>   * a panic can happen at any time...
> 
>