From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1agBAI-000704-GS for mharc-grub-devel@gnu.org; Wed, 16 Mar 2016 09:07:06 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53233) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1agBAG-0006zA-CC for grub-devel@gnu.org; Wed, 16 Mar 2016 09:07:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1agBAA-0002TU-9p for grub-devel@gnu.org; Wed, 16 Mar 2016 09:07:04 -0400 Received: from userp1040.oracle.com ([156.151.31.81]:23754) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1agBAA-0002TF-1t for grub-devel@gnu.org; Wed, 16 Mar 2016 09:06:58 -0400 Received: from aserv0022.oracle.com (aserv0022.oracle.com [141.146.126.234]) by userp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with ESMTP id u2GD6ifk003272 (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 16 Mar 2016 13:06:45 GMT Received: from aserv0121.oracle.com (aserv0121.oracle.com [141.146.126.235]) by aserv0022.oracle.com (8.13.8/8.13.8) with ESMTP id u2GD6i05010672 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=FAIL); Wed, 16 Mar 2016 13:06:44 GMT Received: from abhmp0002.oracle.com (abhmp0002.oracle.com [141.146.116.8]) by aserv0121.oracle.com (8.13.8/8.13.8) with ESMTP id u2GD6g3j028158; Wed, 16 Mar 2016 13:06:42 GMT Received: from char.us.oracle.com (/10.137.176.158) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Wed, 16 Mar 2016 06:06:42 -0700 Received: by char.us.oracle.com (Postfix, from userid 1000) id 50A126A00B6; Wed, 16 Mar 2016 09:06:40 -0400 (EDT) Date: Wed, 16 Mar 2016 09:06:40 -0400 From: Konrad Rzeszutek Wilk To: "Vladimir 'phcoder' Serbinenko" Subject: Re: [GRUB2 PATCH v4 3/4 - FOR REVIEW ONLY] multiboot2: Do not pass memory maps to image if EFI boot services are enabled Message-ID: <20160316130640.GA1460@char.us.oracle.com> References: <1458055562-24950-1-git-send-email-daniel.kiper@oracle.com> <1458055562-24950-4-git-send-email-daniel.kiper@oracle.com> <20160315234646.GE29495@char.us.oracle.com> <20160316100214.GF31771@olila.local.net-space.pl> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Content-Transfer-Encoding: quoted-printable X-Source-IP: aserv0022.oracle.com [141.146.126.234] 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: "jgross@suse.com" , The development of GNU GRUB , "eric.snowberg@oracle.com" , Andrei Borzenkov , "andrew.cooper3@citrix.com" , "stefano.stabellini@eu.citrix.com" , "cardoe@cardoe.com" , "pgnet.dev@gmail.com" , "roy.franz@linaro.org" , "ning.sun@intel.com" , "david.vrabel@citrix.com" , "jbeulich@suse.com" , Toomas Soome , "xen-devel@lists.xenproject.org" , "qiaowei.ren@intel.com" , "richard.l.maliszewski@intel.com" , "gang.wei@intel.com" , "fu.wei@linaro.org" , "seth.goldberg@oracle.com" 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: Wed, 16 Mar 2016 13:07:05 -0000 On Wed, Mar 16, 2016 at 11:39:55AM +0100, Vladimir 'phcoder' Serbinenko w= rote: > On Wednesday, March 16, 2016, Toomas Soome wrote: >=20 > > > > > On 16. m=E4rts 2016, at 12:02, Daniel Kiper > > wrote: > > > > > > On Tue, Mar 15, 2016 at 07:46:46PM -0400, Konrad Rzeszutek Wilk wro= te: > > >> On Tue, Mar 15, 2016 at 04:26:00PM +0100, Daniel Kiper wrote: > > >>> Do not pass memory maps to image if it asked for EFI boot service= s. > > >> > > >> .. I would change this sentence a bit. > > >> > > >> If image requested EFI boot services then skip multiboot2 memory m= aps. > > >> > > >>> Main reason for not providing maps is because they will likely be > > >>> invalid. We do a few allocations after filling them, e.g. for rel= ocator > > >>> needs. Usually we do not care as we would already finish boot ser= vices. > > >> > > >> s/would already finish/would have finished/ ? > > >> > > >>> If we keep boot services then it is easier to not provide maps. > > However, > > >> > > >> s/easier/safer?/ > > >> > > >>> if image needs memory maps and they are not provided by bootloade= r then > > >>> it should get them itself just before ExitBootServices() call. > > >> > > >> s/them// ? > > >> > > >> That is making an assumption that the user of Multiboot2 + EFI wil= l > > >> do this. Which is OK since only Xen is using it.. but is this > > >> inline with the spec? Can you ignore not providing this informatio= n? > > > > > > AIUI, spec does not require that anything must be provided. Of cour= se > > > on every platform GRUB2 should provide minimal set of system inform= ation > > > to properly boot loaded image. However, docs does not say which set= make > > > sense or not. This is role of image to know what it requires to pro= perly > > > run on a given platform. And I think that it make sense. > > > > > >> That aside - why not sync the multiboot memory map with what > > >> the EFI one will be? Or is it too to complex to move the memory ma= p > > >> creation to a later phase of the bootup? > > > > > > IIRC, GRUB2 does some allocations after getting memory map and it i= s > > > quite complicated to change that. > > > > > >> Wish there was some multboot memory map flag indicating > > 'STALE-CHECK-EFI'.. > > > > > > Why provide map which is invalid and must be verified with somethin= g > > else? > > > Let's ignore (do not provide) invalid data and use correct one with= out > > > any additional (unneeded) checks. > > > > > > > If you are *not* calling exit efi boot services from grub, there is n= o > > sense to provide EFI memory map at all, because to exit boot services= [from > > OS], you *have to* fetch current memory map to get the map key to exi= t boot > > services. > > > But this doesn't apply for e820-like and simple maps which are unaffect= ed > by bootloader allocations Right. I think we are all on the same page then. If EFI and payload (OS) has requested to be the one executing ExitBootSer= vices then don't provide the memory map? But if the payload hasn't specified what to do with ExitBootServices, GRU= B is free to do so and provide the map. >=20 > > > > basically it means, if BS is not disabled and grub is still providing= EFI > > memory map, the OS has to assume this map is not valid - which is bad= and > > better not to provide (potentially) invalid data in first place. Right. And the description of the patch (to my reading) was not exactly clear on= this. Perhaps just updating the description of the patch and repositing as repl= y here would suffice? > > > > rgds, > > toomas >=20 >=20 >=20 > --=20 > Regards > Vladimir 'phcoder' Serbinenko