Re: [GRUB2 PATCH v4 3/4 - FOR REVIEW ONLY] multiboot2: Do not pass memory maps to image if EFI boot services are enabled

From: Toomas Soome <tsoome@me.com>
To: The development of GNU GRUB <grub-devel@gnu.org>
Cc: jgross@suse.com, eric.snowberg@oracle.com,
	stefano.stabellini@eu.citrix.com,
	"Andrei Borzenkov" <arvidjaar@gmail.com>,
	andrew.cooper3@citrix.com, cardoe@cardoe.com,
	pgnet.dev@gmail.com, roy.franz@linaro.org, ning.sun@intel.com,
	david.vrabel@citrix.com, jbeulich@suse.com,
	"Vladimir 'φ-coder/phcoder' Serbinenko" <phcoder@gmail.com>,
	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
Subject: Re: [GRUB2 PATCH v4 3/4 - FOR REVIEW ONLY] multiboot2: Do not pass memory maps to image if EFI boot services are enabled
Date: Wed, 16 Mar 2016 12:14:37 +0200	[thread overview]
Message-ID: <D5F25A44-6157-46FB-B717-A7ED06FD01C8__23011.7420004264$1458124493$gmane$org@me.com> (raw)
In-Reply-To: <20160316100214.GF31771@olila.local.net-space.pl>

> On 16. märts 2016, at 12:02, Daniel Kiper <daniel.kiper@oracle.com> wrote:
> 
> On Tue, Mar 15, 2016 at 07:46:46PM -0400, Konrad Rzeszutek Wilk wrote:
>> 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 services.
>> 
>> .. I would change this sentence a bit.
>> 
>> If image requested EFI boot services then skip multiboot2 memory maps.
>> 
>>> Main reason for not providing maps is because they will likely be
>>> invalid. We do a few allocations after filling them, e.g. for relocator
>>> needs. Usually we do not care as we would already finish boot services.
>> 
>> 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 bootloader then
>>> it should get them itself just before ExitBootServices() call.
>> 
>> s/them// ?
>> 
>> That is making an assumption that the user of Multiboot2 + EFI will
>> do this. Which is OK since only Xen is using it.. but is this
>> inline with the spec? Can you ignore not providing this information?
> 
> AIUI, spec does not require that anything must be provided. Of course
> on every platform GRUB2 should provide minimal set of system information
> 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 properly
> 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 map
>> creation to a later phase of the bootup?
> 
> IIRC, GRUB2 does some allocations after getting memory map and it is
> 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 something else?
> Let's ignore (do not provide) invalid data and use correct one without
> any additional (unneeded) checks.
> 

If you are *not* calling exit efi boot services from grub, there is no 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 exit boot services.

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.

rgds, 
toomas
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel