Re: [PATCH v3 08/10] xen: add capability to load initrd outside of initial mapping

[Daniel Kiper]

On Thu, Feb 18, 2016 at 01:43:33PM +0100, Juergen Gross wrote:
> On 18/02/16 12:18, Daniel Kiper wrote:
> > On Wed, Feb 17, 2016 at 06:19:35PM +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>
> >
> > One nitpick.
> >
> > Reviewed-by: Daniel Kiper <address@hidden>
> >
> >> ---
> >>  grub-core/loader/i386/xen.c        | 61 
> >> ++++++++++++++++++++++++++++++--------
> >>  grub-core/loader/i386/xen_fileXX.c |  3 ++
> >>  include/grub/xen_file.h            |  1 +
> >>  3 files changed, 52 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
> >> index 3bcf4c8..7ac74f6 100644
> >> --- a/grub-core/loader/i386/xen.c
> >> +++ b/grub-core/loader/i386/xen.c
> >> @@ -58,6 +58,7 @@ struct xen_loader_state {
> >>    grub_uint64_t modules_target_start;
> >>    grub_size_t n_modules;
> >>    int loaded;
> >> +  int alloc_end_called;
> >>  };
> >>
> >>  static struct xen_loader_state xen_state;
> >> @@ -320,6 +321,28 @@ grub_xen_pt_alloc (void)
> >>  }
> >>
> >>  static grub_err_t
> >> +grub_xen_alloc_end (void)
> >
> > Why is it called grub_xen_alloc_end()?
> > Could we use just grub_xen_alloc()?
>
> We could, of course. I just wanted to make clear that this function will
> do the allocations needed to be at the end of the allocation process.
> Naming it grub_xen_alloc() would make it occur to be a very basic
> allocation function, which just isn't true. What about
> grub_xen_alloc_rest()?

My order of preference is: grub_xen_alloc_final() or grub_xen_alloc_last() or
grub_xen_alloc_end() or grub_xen_alloc_rest().

Choose one which is best for you.

Daniel