From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1aUF0o-0007Gw-9Y for mharc-grub-devel@gnu.org; Fri, 12 Feb 2016 09:47:58 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38688) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aUF0l-0007Bf-QP for grub-devel@gnu.org; Fri, 12 Feb 2016 09:47:56 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aUF0g-0005UI-Mk for grub-devel@gnu.org; Fri, 12 Feb 2016 09:47:55 -0500 Received: from mx2.suse.de ([195.135.220.15]:42656) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aUF0g-0005U2-FN for grub-devel@gnu.org; Fri, 12 Feb 2016 09:47:50 -0500 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 72615AB45; Fri, 12 Feb 2016 14:47:48 +0000 (UTC) Subject: Re: [PATCH v2 4/6] xen: add capability to load initrd outside of initial mapping To: =?UTF-8?Q?Vladimir_'=cf=86-coder/phcoder'_Serbinenko?= , 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> <56BC9714.7020101@suse.com> <56BDCF12.6050108@gmail.com> From: Juergen Gross X-Enigmail-Draft-Status: N1110 Message-ID: <56BDF093.3080708@suse.com> Date: Fri, 12 Feb 2016 15:47:47 +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: <56BDCF12.6050108@gmail.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable 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, 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: Fri, 12 Feb 2016 14:47:57 -0000 On 12/02/16 13:24, Vladimir '=CF=86-coder/phcoder' Serbinenko wrote: > On 11.02.2016 15:13, Juergen Gross wrote: >> 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. >>>>=20 >>>> 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. >>>>=20 >>>> Signed-off-by: Juergen Gross ---=20 >>>> grub-core/loader/i386/xen.c | 56 > ++++++++++++++++++++++++++++++-------- >>>> grub-core/loader/i386/xen_fileXX.c | 3 ++=20 >>>> include/grub/xen_file.h | 1 + 3 files changed, 49 >>>> insertions(+), 11 deletions(-) >>>>=20 >>>> diff --git a/grub-core/loader/i386/xen.c >>>> b/grub-core/loader/i386/xen.c index 65cec27..0f41048 100644=20 >>>> --- a/grub-core/loader/i386/xen.c +++ >>>> b/grub-core/loader/i386/xen.c @@ -307,15 +307,14 @@ >>>> grub_xen_pt_alloc (void) } >>>>=20 >>>> 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 =3D 0; >>>>=20 >>>> - if (grub_xen_n_allocated_shared_pages) - return >>>> grub_error (GRUB_ERR_BUG, "active grants"); + if (called) + >>>> return GRUB_ERR_NONE; >>>=20 >>> Why? >>=20 >> 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. >>=20 > What if a boot fails and then fallback kernel is loaded? As already stated before: I'll add a patch handling freeing of memory in a reset function which will reset the "called" flag as well. >>>> + if (grub_xen_n_allocated_shared_pages) + return >>>> grub_error (GRUB_ERR_BUG, "active grants"); >>>=20 >>> Why? >>=20 >> That's how it has been before. git just decided to generate the >> diff that way. >>=20 > This is also needed to avoid passing control when some drivers are > still active >>>> + case 16: >>>=20 >>> Could you define this a constant and use it here? >>=20 >> This would be the only case with a constant. All others are >> numeric as well. >>=20 > I'm ok with not insisisting on using constants given current state > but in general constants are preferable (yes, xen code isn't always > clean) Okay, thanks for the confirmation. I'll add a patch (or better: some patches) which will clean up the xen code by introducing (lots of) constants. Juergen