From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1aYvL6-0004mX-D4 for mharc-grub-devel@gnu.org; Thu, 25 Feb 2016 07:48:16 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55931) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aYvL4-0004jk-6R for grub-devel@gnu.org; Thu, 25 Feb 2016 07:48:15 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aYvL0-0005Aa-QM for grub-devel@gnu.org; Thu, 25 Feb 2016 07:48:14 -0500 Received: from userp1040.oracle.com ([156.151.31.81]:24877) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aYvL0-0005AU-IE for grub-devel@gnu.org; Thu, 25 Feb 2016 07:48:10 -0500 Received: from userv0022.oracle.com (userv0022.oracle.com [156.151.31.74]) by userp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with ESMTP id u1PCm8jc006030 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 25 Feb 2016 12:48:09 GMT Received: from aserv0121.oracle.com (aserv0121.oracle.com [141.146.126.235]) by userv0022.oracle.com (8.14.4/8.13.8) with ESMTP id u1PCm7I1015876 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=FAIL); Thu, 25 Feb 2016 12:48:07 GMT Received: from abhmp0010.oracle.com (abhmp0010.oracle.com [141.146.116.16]) by aserv0121.oracle.com (8.13.8/8.13.8) with ESMTP id u1PCm6lk026524; Thu, 25 Feb 2016 12:48:07 GMT Received: from olila.local.net-space.pl (/10.175.171.82) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Thu, 25 Feb 2016 04:48:06 -0800 Date: Thu, 25 Feb 2016 13:47:57 +0100 From: Daniel Kiper To: Juergen Gross Subject: Re: [PATCH v5 09/11] xen: add capability to load initrd outside of initial mapping Message-ID: <20160225124757.GB3482@olila.local.net-space.pl> References: <1456400017-5789-1-git-send-email-jgross@suse.com> <1456400017-5789-10-git-send-email-jgross@suse.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1456400017-5789-10-git-send-email-jgross@suse.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-Source-IP: userv0022.oracle.com [156.151.31.74] X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.4.x-2.6.x [generic] X-Received-From: 156.151.31.81 Cc: grub-devel@gnu.org, phcoder@gmail.com, mchang@suse.com, xen-devel@lists.xen.org X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 25 Feb 2016 12:48:15 -0000 On Thu, Feb 25, 2016 at 12:33: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 > --- > 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 | 65 ++++++++++++++++++++++++++++++-------- > grub-core/loader/i386/xen_fileXX.c | 3 ++ > include/grub/xen_file.h | 1 + > 3 files changed, 56 insertions(+), 13 deletions(-) > > diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c > index 2e12763..466f0c0 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; > @@ -320,6 +329,24 @@ grub_xen_pt_alloc (void) > } > > static grub_err_t > +grub_xen_alloc_final (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; Could you move grub_xen_p2m_alloc() here? This way grub_xen_alloc_final() will be real final in patch 11 and you do not need an extra condition around grub_xen_p2m_alloc(). > + > + return GRUB_ERR_NONE; > +} > + > +static grub_err_t > grub_xen_boot (void) > { > grub_err_t err; > @@ -330,13 +357,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_final (); if (!xen_state.xen_inf.unmapped_initrd) { err = grub_xen_alloc_final (); if (err) goto fail; } This way you avoid checks in grub_xen_p2m_alloc(), grub_xen_special_alloc() and grub_xen_pt_alloc(). > if (err) > return err; > > @@ -610,6 +631,13 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ ((unused)), > goto fail; > } > > + if (xen_state.xen_inf.unmapped_initrd) > + { > + err = grub_xen_alloc_final (); > + if (err) > + goto fail; > + } > + > if (grub_initrd_init (argc, argv, &initrd_ctx)) > goto fail; > > @@ -627,14 +655,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; Leave "xen_state.next_start.mod_len = size;" as is here and... > - > - 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; ... do not add it here... > + } > + else > + { > + xen_state.next_start.mod_start = > + xen_state.max_addr + xen_state.xen_inf.virt_base; > + xen_state.next_start.mod_len = size; ... and here... > + } ... and you do not need curly brackets then. > grub_dprintf ("xen", "Initrd, addr=0x%x, size=0x%x\n", > - (unsigned) xen_state.next_start.mod_start, (unsigned) size); > + (unsigned) (xen_state.max_addr + xen_state.xen_inf.virt_base), > + (unsigned) size); This should be printed only if !xen_state.xen_inf.unmapped_initrd. Why do not use xen_state.next_start.mod_start here? Daniel