From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1ZPEmk-0004cb-QV for mharc-grub-devel@gnu.org; Tue, 11 Aug 2015 15:00:30 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49015) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZPEmh-0004cR-98 for grub-devel@gnu.org; Tue, 11 Aug 2015 15:00:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZPEmc-0007Fk-8x for grub-devel@gnu.org; Tue, 11 Aug 2015 15:00:27 -0400 Received: from userp1040.oracle.com ([156.151.31.81]:23753) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZPEmc-0007FN-1C for grub-devel@gnu.org; Tue, 11 Aug 2015 15:00:22 -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 t7BJ01Zm010706 (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 11 Aug 2015 19:00:01 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 t7BJ00vP030698 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=FAIL); Tue, 11 Aug 2015 19:00:00 GMT Received: from abhmp0020.oracle.com (abhmp0020.oracle.com [141.146.116.26]) by aserv0121.oracle.com (8.13.8/8.13.8) with ESMTP id t7BJ006u015036; Tue, 11 Aug 2015 19:00:00 GMT Received: from l.oracle.com (/10.137.176.158) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Tue, 11 Aug 2015 11:59:58 -0700 Received: by l.oracle.com (Postfix, from userid 1000) id 40D926A3C73; Tue, 11 Aug 2015 14:59:56 -0400 (EDT) Date: Tue, 11 Aug 2015 14:59:56 -0400 From: Konrad Rzeszutek Wilk To: Daniel Kiper Subject: Re: [PATCH v2 6/6] multiboot2: Do not pass memory maps to image if EFI boot services are enabled Message-ID: <20150811185956.GE1292@l.oracle.com> References: <1437402954-7375-1-git-send-email-daniel.kiper@oracle.com> <1437402954-7375-7-git-send-email-daniel.kiper@oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1437402954-7375-7-git-send-email-daniel.kiper@oracle.com> User-Agent: Mutt/1.5.23 (2014-03-12) 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, grub-devel@gnu.org, wei.liu2@citrix.com, keir@xen.org, ian.campbell@citrix.com, stefano.stabellini@eu.citrix.com, andrew.cooper3@citrix.com, roy.franz@linaro.org, ning.sun@intel.com, david.vrabel@citrix.com, jbeulich@suse.com, 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 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: Tue, 11 Aug 2015 19:00:28 -0000 On Mon, Jul 20, 2015 at 04:35:54PM +0200, Daniel Kiper wrote: > Do not pass memory maps to image if it asked for EFI boot services. Maps are > usually invalid in that case and they can confuse potential user. Image should > get memory map itself just before ExitBootServices() call. Can we point to some commits in Linux or Xen in which these situations arose? Wait, I think there even was one commit in grub. Aha: ommit e75fdee420a7ad95e9a465c9699adc2e2e970440 Author: Vladimir 'phcoder' Serbinenko Date: Tue Mar 26 11:34:56 2013 +0100 * grub-core/kern/efi/mm.c (grub_efi_finish_boot_services): Try terminating EFI services several times due to quirks in some implementations. Otherwise: Reviewed-by: Konrad Rzeszutek Wilk > > Signed-off-by: Daniel Kiper > --- > grub-core/loader/multiboot_mbi2.c | 71 ++++++++++++++++++------------------- > 1 file changed, 35 insertions(+), 36 deletions(-) > > diff --git a/grub-core/loader/multiboot_mbi2.c b/grub-core/loader/multiboot_mbi2.c > index 7ac64ec..26e955c 100644 > --- a/grub-core/loader/multiboot_mbi2.c > +++ b/grub-core/loader/multiboot_mbi2.c > @@ -431,7 +431,7 @@ static grub_size_t > grub_multiboot_get_mbi_size (void) > { > #ifdef GRUB_MACHINE_EFI > - if (!efi_mmap_size) > + if (!keep_bs && !efi_mmap_size) > find_efi_mmap_size (); > #endif > return 2 * sizeof (grub_uint32_t) + sizeof (struct multiboot_tag) > @@ -805,12 +805,13 @@ grub_multiboot_make_mbi (grub_uint32_t *target) > } > } > > - { > - struct multiboot_tag_mmap *tag = (struct multiboot_tag_mmap *) ptrorig; > - grub_fill_multiboot_mmap (tag); > - ptrorig += ALIGN_UP (tag->size, MULTIBOOT_TAG_ALIGN) > - / sizeof (grub_properly_aligned_t); > - } > + if (!keep_bs) > + { > + struct multiboot_tag_mmap *tag = (struct multiboot_tag_mmap *) ptrorig; > + grub_fill_multiboot_mmap (tag); > + ptrorig += ALIGN_UP (tag->size, MULTIBOOT_TAG_ALIGN) > + / sizeof (grub_properly_aligned_t); > + } > > { > struct multiboot_tag_elf_sections *tag > @@ -826,18 +827,19 @@ grub_multiboot_make_mbi (grub_uint32_t *target) > / sizeof (grub_properly_aligned_t); > } > > - { > - struct multiboot_tag_basic_meminfo *tag > - = (struct multiboot_tag_basic_meminfo *) ptrorig; > - tag->type = MULTIBOOT_TAG_TYPE_BASIC_MEMINFO; > - tag->size = sizeof (struct multiboot_tag_basic_meminfo); > + if (!keep_bs) > + { > + struct multiboot_tag_basic_meminfo *tag > + = (struct multiboot_tag_basic_meminfo *) ptrorig; > + tag->type = MULTIBOOT_TAG_TYPE_BASIC_MEMINFO; > + tag->size = sizeof (struct multiboot_tag_basic_meminfo); > > - /* Convert from bytes to kilobytes. */ > - tag->mem_lower = grub_mmap_get_lower () / 1024; > - tag->mem_upper = grub_mmap_get_upper () / 1024; > - ptrorig += ALIGN_UP (tag->size, MULTIBOOT_TAG_ALIGN) > - / sizeof (grub_properly_aligned_t); > - } > + /* Convert from bytes to kilobytes. */ > + tag->mem_lower = grub_mmap_get_lower () / 1024; > + tag->mem_upper = grub_mmap_get_upper () / 1024; > + ptrorig += ALIGN_UP (tag->size, MULTIBOOT_TAG_ALIGN) > + / sizeof (grub_properly_aligned_t); > + } > > { > struct grub_net_network_level_interface *net; > @@ -936,27 +938,24 @@ grub_multiboot_make_mbi (grub_uint32_t *target) > grub_efi_uintn_t efi_desc_size; > grub_efi_uint32_t efi_desc_version; > > - tag->type = MULTIBOOT_TAG_TYPE_EFI_MMAP; > - tag->size = sizeof (*tag) + efi_mmap_size; > - > if (!keep_bs) > - err = grub_efi_finish_boot_services (&efi_mmap_size, tag->efi_mmap, NULL, > - &efi_desc_size, &efi_desc_version); > - else > { > - if (grub_efi_get_memory_map (&efi_mmap_size, (void *) tag->efi_mmap, > - NULL, > - &efi_desc_size, &efi_desc_version) <= 0) > - err = grub_error (GRUB_ERR_IO, "couldn't retrieve memory map"); > + tag->type = MULTIBOOT_TAG_TYPE_EFI_MMAP; > + tag->size = sizeof (*tag) + efi_mmap_size; > + > + err = grub_efi_finish_boot_services (&efi_mmap_size, tag->efi_mmap, NULL, > + &efi_desc_size, &efi_desc_version); > + > + if (err) > + return err; > + > + tag->descr_size = efi_desc_size; > + tag->descr_vers = efi_desc_version; > + tag->size = sizeof (*tag) + efi_mmap_size; > + > + ptrorig += ALIGN_UP (tag->size, MULTIBOOT_TAG_ALIGN) > + / sizeof (grub_properly_aligned_t); > } > - if (err) > - return err; > - tag->descr_size = efi_desc_size; > - tag->descr_vers = efi_desc_version; > - tag->size = sizeof (*tag) + efi_mmap_size; > - > - ptrorig += ALIGN_UP (tag->size, MULTIBOOT_TAG_ALIGN) > - / sizeof (grub_properly_aligned_t); > } > > if (keep_bs) > -- > 1.7.10.4 >