Re: [PATCH v6 09/11] xen: add capability to load initrd outside of initial mapping

[Daniel Kiper]

On Tue, Mar 01, 2016 at 08:55:09AM +0100, Juergen Gross wrote:
> Modern pvops linux kernels support an initrd not covered by the initial
> mapping. This capability is flagged by an elf-note.
>
> In case the elf-note is set by the kernel don't place the initrd into
> the initial mapping. This will allow to load larger initrds and/or
> support domains with larger memory, as the initial mapping is limited
> to 2GB and it is containing the p2m list.
>
> Signed-off-by: Juergen Gross <address@hidden>
> Reviewed-by: Daniel Kiper <address@hidden>
> ---
> V6: rename grub_xen_alloc_final() to grub_xen_alloc_boot_data()
> V5: let call grub_xen_alloc_final() all subfunctions unconditionally
>     and let them decide whether they need to do anything
> V4: rename grub_xen_alloc_end() to grub_xen_alloc_final()
> ---
>  grub-core/loader/i386/xen.c        | 66 
> ++++++++++++++++++++++++++++++--------
>  grub-core/loader/i386/xen_fileXX.c |  3 ++
>  include/grub/xen_file.h            |  1 +
>  3 files changed, 57 insertions(+), 13 deletions(-)
>
> diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
> index 2e12763..989a60b 100644
> --- a/grub-core/loader/i386/xen.c
> +++ b/grub-core/loader/i386/xen.c
> @@ -228,6 +228,9 @@ grub_xen_p2m_alloc (void)
>    grub_size_t p2msize;
>    grub_err_t err;
>
> +  if (xen_state.virt_mfn_list)
> +    return GRUB_ERR_NONE;
> +
>    xen_state.state.mfn_list = xen_state.max_addr;
>    xen_state.next_start.mfn_list =
>      xen_state.max_addr + xen_state.xen_inf.virt_base;
> @@ -250,6 +253,9 @@ grub_xen_special_alloc (void)
>    grub_relocator_chunk_t ch;
>    grub_err_t err;
>
> +  if (xen_state.virt_start_info)
> +    return GRUB_ERR_NONE;
> +
>    err = grub_relocator_alloc_chunk_addr (xen_state.relocator, &ch,
>                                        xen_state.max_addr,
>                                        sizeof (xen_state.next_start));
> @@ -281,6 +287,9 @@ grub_xen_pt_alloc (void)
>    grub_uint64_t nr_info_pages;
>    grub_uint64_t nr_pages, nr_pt_pages, nr_need_pages;
>
> +  if (xen_state.virt_pgtable)
> +    return GRUB_ERR_NONE;
> +
>    xen_state.next_start.pt_base =
>      xen_state.max_addr + xen_state.xen_inf.virt_base;
>    xen_state.state.paging_start = xen_state.max_addr >> PAGE_SHIFT;
> @@ -319,6 +328,25 @@ grub_xen_pt_alloc (void)
>    return GRUB_ERR_NONE;
>  }
>
> +/* Allocate all not yet allocated areas mapped by initial page tables. */
> +static grub_err_t
> +grub_xen_alloc_boot_data (void)
> +{
> +  grub_err_t err;
> +
> +  err = grub_xen_p2m_alloc ();
> +  if (err)
> +    return err;
> +  err = grub_xen_special_alloc ();
> +  if (err)
> +    return err;
> +  err = grub_xen_pt_alloc ();
> +  if (err)
> +    return err;
> +
> +  return GRUB_ERR_NONE;
> +}
> +
>  static grub_err_t
>  grub_xen_boot (void)
>  {
> @@ -330,13 +358,7 @@ grub_xen_boot (void)
>    if (grub_xen_n_allocated_shared_pages)
>      return grub_error (GRUB_ERR_BUG, "active grants");
>
> -  err = grub_xen_p2m_alloc ();
> -  if (err)
> -    return err;
> -  err = grub_xen_special_alloc ();
> -  if (err)
> -    return err;
> -  err = grub_xen_pt_alloc ();
> +  err = grub_xen_alloc_boot_data ();
>    if (err)
>      return err;
>
> @@ -610,6 +632,13 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ 
> ((unused)),
>        goto fail;
>      }
>
> +  if (xen_state.xen_inf.unmapped_initrd)
> +    {
> +      err = grub_xen_alloc_boot_data ();
> +      if (err)
> +     goto fail;
> +    }
> +
>    if (grub_initrd_init (argc, argv, &initrd_ctx))
>      goto fail;
>
> @@ -627,14 +656,24 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ 
> ((unused)),
>       goto fail;
>      }
>
> -  xen_state.next_start.mod_start =
> -    xen_state.max_addr + xen_state.xen_inf.virt_base;
> -  xen_state.next_start.mod_len = size;

You forgot about leaving this as is...

> -
> -  xen_state.max_addr = ALIGN_UP (xen_state.max_addr + size, PAGE_SIZE);
> +  if (xen_state.xen_inf.unmapped_initrd)
> +    {
> +      xen_state.next_start.flags |= SIF_MOD_START_PFN;
> +      xen_state.next_start.mod_start = xen_state.max_addr >> PAGE_SHIFT;
> +      xen_state.next_start.mod_len = size;

Then you do not need this...

> +    }
> +  else
> +    {
> +      xen_state.next_start.mod_start =
> +     xen_state.max_addr + xen_state.xen_inf.virt_base;
> +      xen_state.next_start.mod_len = size;

... nor this...

> +    }

... and even this curly brackets...

Daniel