From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1aTs0E-0002qN-2N for mharc-grub-devel@gnu.org; Thu, 11 Feb 2016 09:13:50 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35862) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aTs0A-0002pt-Ij for grub-devel@gnu.org; Thu, 11 Feb 2016 09:13:48 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aTs06-0004Ne-E0 for grub-devel@gnu.org; Thu, 11 Feb 2016 09:13:46 -0500 Received: from mx2.suse.de ([195.135.220.15]:60607) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aTs06-0004NY-3q for grub-devel@gnu.org; Thu, 11 Feb 2016 09:13:42 -0500 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id A2B8DACAD; Thu, 11 Feb 2016 14:13:40 +0000 (UTC) Subject: Re: [PATCH v2 4/6] xen: add capability to load initrd outside of initial mapping To: Daniel Kiper References: <1455177206-8974-1-git-send-email-jgross@suse.com> <1455177206-8974-5-git-send-email-jgross@suse.com> <20160211123352.GD3482@olila.local.net-space.pl> From: Juergen Gross X-Enigmail-Draft-Status: N1110 Message-ID: <56BC9714.7020101@suse.com> Date: Thu, 11 Feb 2016 15:13:40 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.0 MIME-Version: 1.0 In-Reply-To: <20160211123352.GD3482@olila.local.net-space.pl> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x (no timestamps) [generic] X-Received-From: 195.135.220.15 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, 11 Feb 2016 14:13:48 -0000 On 11/02/16 13:33, Daniel Kiper wrote: > On Thu, Feb 11, 2016 at 08:53:24AM +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 >> --- >> grub-core/loader/i386/xen.c | 56 ++++++++++++++++++++++++++++++-------- >> grub-core/loader/i386/xen_fileXX.c | 3 ++ >> include/grub/xen_file.h | 1 + >> 3 files changed, 49 insertions(+), 11 deletions(-) >> >> diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c >> index 65cec27..0f41048 100644 >> --- a/grub-core/loader/i386/xen.c >> +++ b/grub-core/loader/i386/xen.c >> @@ -307,15 +307,14 @@ grub_xen_pt_alloc (void) >> } >> >> static grub_err_t >> -grub_xen_boot (void) >> +grub_xen_alloc_end (void) >> { >> grub_err_t err; >> - grub_uint64_t nr_pages; >> - struct gnttab_set_version gnttab_setver; >> - grub_size_t i; >> + static int called = 0; >> >> - if (grub_xen_n_allocated_shared_pages) >> - return grub_error (GRUB_ERR_BUG, "active grants"); >> + if (called) >> + return GRUB_ERR_NONE; > > Why? Did you look at the function? grub_xen_alloc_end() (some parts of the original grub_xen_boot()) is new and may be called multiple times. It was much easier to just call it and let it do what must be done only at the first time called. > >> + called = 1; >> >> err = grub_xen_p2m_alloc (); >> if (err) >> @@ -327,6 +326,24 @@ grub_xen_boot (void) >> if (err) >> return err; >> >> + return GRUB_ERR_NONE; >> +} >> + >> +static grub_err_t >> +grub_xen_boot (void) >> +{ >> + grub_err_t err; >> + grub_uint64_t nr_pages; >> + struct gnttab_set_version gnttab_setver; >> + grub_size_t i; >> + >> + if (grub_xen_n_allocated_shared_pages) >> + return grub_error (GRUB_ERR_BUG, "active grants"); > > Why? That's how it has been before. git just decided to generate the diff that way. > >> + err = grub_xen_alloc_end (); >> + if (err) >> + return err; >> + >> err = set_mfns (console_pfn); >> if (err) >> return err; >> @@ -587,6 +604,13 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ ((unused)), >> goto fail; >> } >> >> + if (xen_inf.unmapped_initrd) >> + { >> + err = grub_xen_alloc_end (); >> + if (err) >> + goto fail; >> + } >> + >> if (grub_initrd_init (argc, argv, &initrd_ctx)) >> goto fail; >> >> @@ -603,13 +627,22 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ ((unused)), >> goto fail; >> } >> >> - next_start.mod_start = max_addr + xen_inf.virt_base; >> - next_start.mod_len = size; >> - >> - max_addr = ALIGN_UP (max_addr + size, PAGE_SIZE); >> + if (xen_inf.unmapped_initrd) >> + { >> + next_start.flags |= SIF_MOD_START_PFN; >> + next_start.mod_start = max_addr >> PAGE_SHIFT; >> + next_start.mod_len = size; >> + } >> + else >> + { >> + next_start.mod_start = max_addr + xen_inf.virt_base; >> + next_start.mod_len = size; >> + } >> >> grub_dprintf ("xen", "Initrd, addr=0x%x, size=0x%x\n", >> - (unsigned) next_start.mod_start, (unsigned) size); >> + (unsigned) (max_addr + xen_inf.virt_base), (unsigned) size); >> + >> + max_addr = ALIGN_UP (max_addr + size, PAGE_SIZE); >> >> fail: >> grub_initrd_close (&initrd_ctx); >> @@ -660,6 +693,7 @@ grub_cmd_module (grub_command_t cmd __attribute__ ((unused)), >> >> if (!xen_module_info_page) >> { >> + xen_inf.unmapped_initrd = 0; >> n_modules = 0; >> max_addr = ALIGN_UP (max_addr, PAGE_SIZE); >> modules_target_start = max_addr; >> diff --git a/grub-core/loader/i386/xen_fileXX.c b/grub-core/loader/i386/xen_fileXX.c >> index 1ba5649..69fccd2 100644 >> --- a/grub-core/loader/i386/xen_fileXX.c >> +++ b/grub-core/loader/i386/xen_fileXX.c >> @@ -253,6 +253,9 @@ parse_note (grub_elf_t elf, struct grub_xen_file_info *xi, >> descsz == 2 ? 2 : 3) == 0) >> xi->arch = GRUB_XEN_FILE_I386; >> break; >> + case 16: > > Could you define this a constant and use it here? This would be the only case with a constant. All others are numeric as well. Juergen