Re: [Xen-devel] [RFC PATCH 2/3] x86/boot: Reserve live update boot memory

[Jan Beulich <jbeulich@xxxxxxxx>]

On 08.01.2020 18:24, David Woodhouse wrote:
> @@ -980,6 +1015,22 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>      set_kexec_crash_area_size((u64)nr_pages << PAGE_SHIFT);
>      kexec_reserve_area(&boot_e820);
>  
> +    if ( lu_bootmem_start )
> +    {
> +        /* XX: Check it's in usable memory first */
> +        reserve_e820_ram(&boot_e820, lu_bootmem_start, lu_bootmem_start + 
> lu_bootmem_size);
> +
> +        /* Since it will already be out of the e820 map by the time the first
> +         * loop over physical memory, map it manually already. */
> +        set_pdx_range(lu_bootmem_start >> PAGE_SHIFT,
> +                      (lu_bootmem_start + lu_bootmem_size) >> PAGE_SHIFT);
> +        map_pages_to_xen((unsigned long)__va(lu_bootmem_start),
> +                         maddr_to_mfn(lu_bootmem_start),
> +                         PFN_DOWN(lu_bootmem_size), PAGE_HYPERVISOR);

Doesn't this require the range to be a multiple of 2Mb and below
4Gb? I don't see this enforced anywhere.

> @@ -1278,8 +1348,8 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>          xenheap_max_mfn(PFN_DOWN(highmem_start - 1));
>  
>      /*
> -     * Walk every RAM region and map it in its entirety (on x86/64, at least)
> -     * and notify it to the boot allocator.
> +     * Walk every RAM region and map it in its entirety and (unless in
> +     * live update mode) notify it to the boot allocator.
>       */
>      for ( i = 0; i < boot_e820.nr_map; i++ )
>      {
> @@ -1329,6 +1399,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>                  printk(XENLOG_WARNING "Ignoring inaccessible memory range"
>                                        " %013"PRIx64"-%013"PRIx64"\n",
>                         s, e);
> +                reserve_e820_ram(&boot_e820, s, e);
>                  continue;
>              }
>              map_e = e;
> @@ -1336,6 +1407,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>              printk(XENLOG_WARNING "Ignoring inaccessible memory range"
>                                    " %013"PRIx64"-%013"PRIx64"\n",
>                     e, map_e);
> +            reserve_e820_ram(&boot_e820, e, map_e);
>          }
>  
>          set_pdx_range(s >> PAGE_SHIFT, e >> PAGE_SHIFT);

What are these two hunks needed for? The comment you change further up
relates to ...

> @@ -1346,7 +1418,9 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>                        ARRAY_SIZE(l2_identmap) << L2_PAGETABLE_SHIFT);
>  
>          /* Pass mapped memory to allocator /before/ creating new mappings. */
> -        init_boot_pages(s, min(map_s, e));
> +        if ( !lu_reserved)
> +            init_boot_pages(s, min(map_s, e));

... this afaict.

Apart from this, also applicable to patch 3 - where I have no other
comments - there's quite a bit of style cleanup to b done here. And
of course the new command line option wants documenting. I can't
e.g. guess yet what lu_data is about, and hence why this is
apparently an address without an accompanying size.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel