All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Kiper <daniel.kiper@oracle.com>
To: xen-devel@lists.xenproject.org, grub-devel@gnu.org
Cc: jgross@suse.com, eric.snowberg@oracle.com, arvidjaar@gmail.com,
	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,
	phcoder@gmail.com, qiaowei.ren@intel.com,
	richard.l.maliszewski@intel.com, gang.wei@intel.com,
	fu.wei@linaro.org, seth.goldberg@oracle.com
Subject: [GRUB2 PATCH v4 3/4 - FOR REVIEW ONLY] multiboot2: Do not pass memory maps to image if EFI boot services are enabled
Date: Tue, 15 Mar 2016 16:26:00 +0100	[thread overview]
Message-ID: <1458055562-24950-4-git-send-email-daniel.kiper@oracle.com> (raw)
In-Reply-To: <1458055562-24950-1-git-send-email-daniel.kiper@oracle.com>

Do not pass memory maps to image if it asked for EFI boot services.
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.
If we keep boot services then it is easier to not provide maps. However,
if image needs memory maps and they are not provided by bootloader then
it should get them itself just before ExitBootServices() call.

Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
v3 - suggestions/fixes:
   - improve commit message
     (suggested by Konrad Rzeszutek Wilk and Vladimir 'phcoder' Serbinenko).
---
 grub-core/loader/multiboot_mbi2.c |   17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/grub-core/loader/multiboot_mbi2.c b/grub-core/loader/multiboot_mbi2.c
index 6c04402..ad1553b 100644
--- a/grub-core/loader/multiboot_mbi2.c
+++ b/grub-core/loader/multiboot_mbi2.c
@@ -390,7 +390,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)
@@ -755,6 +755,7 @@ grub_multiboot_make_mbi (grub_uint32_t *target)
       }
   }
 
+  if (!keep_bs)
     {
       struct multiboot_tag_mmap *tag = (struct multiboot_tag_mmap *) ptrorig;
       grub_fill_multiboot_mmap (tag);
@@ -776,6 +777,7 @@ grub_multiboot_make_mbi (grub_uint32_t *target)
       / sizeof (grub_properly_aligned_t);
   }
 
+  if (!keep_bs)
     {
       struct multiboot_tag_basic_meminfo *tag
 	= (struct multiboot_tag_basic_meminfo *) ptrorig;
@@ -886,21 +888,17 @@ grub_multiboot_make_mbi (grub_uint32_t *target)
     grub_efi_uintn_t efi_desc_size;
     grub_efi_uint32_t efi_desc_version;
 
+    if (!keep_bs)
+      {
 	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");
-      }
+
 	if (err)
 	  return err;
+
 	tag->descr_size = efi_desc_size;
 	tag->descr_vers = efi_desc_version;
 	tag->size = sizeof (*tag) + efi_mmap_size;
@@ -908,6 +906,7 @@ grub_multiboot_make_mbi (grub_uint32_t *target)
 	ptrorig += ALIGN_UP (tag->size, MULTIBOOT_TAG_ALIGN)
 	  / sizeof (grub_properly_aligned_t);
       }
+  }
 
   if (keep_bs)
     {
-- 
1.7.10.4


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

WARNING: multiple messages have this Message-ID (diff)
From: Daniel Kiper <daniel.kiper@oracle.com>
To: xen-devel@lists.xenproject.org, grub-devel@gnu.org
Cc: jgross@suse.com, eric.snowberg@oracle.com, arvidjaar@gmail.com,
	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,
	phcoder@gmail.com, qiaowei.ren@intel.com,
	richard.l.maliszewski@intel.com, gang.wei@intel.com,
	fu.wei@linaro.org, seth.goldberg@oracle.com
Subject: [GRUB2 PATCH v4 3/4 - FOR REVIEW ONLY] multiboot2: Do not pass memory maps to image if EFI boot services are enabled
Date: Tue, 15 Mar 2016 16:26:00 +0100	[thread overview]
Message-ID: <1458055562-24950-4-git-send-email-daniel.kiper@oracle.com> (raw)
In-Reply-To: <1458055562-24950-1-git-send-email-daniel.kiper@oracle.com>

Do not pass memory maps to image if it asked for EFI boot services.
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.
If we keep boot services then it is easier to not provide maps. However,
if image needs memory maps and they are not provided by bootloader then
it should get them itself just before ExitBootServices() call.

Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
v3 - suggestions/fixes:
   - improve commit message
     (suggested by Konrad Rzeszutek Wilk and Vladimir 'phcoder' Serbinenko).
---
 grub-core/loader/multiboot_mbi2.c |   17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/grub-core/loader/multiboot_mbi2.c b/grub-core/loader/multiboot_mbi2.c
index 6c04402..ad1553b 100644
--- a/grub-core/loader/multiboot_mbi2.c
+++ b/grub-core/loader/multiboot_mbi2.c
@@ -390,7 +390,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)
@@ -755,6 +755,7 @@ grub_multiboot_make_mbi (grub_uint32_t *target)
       }
   }
 
+  if (!keep_bs)
     {
       struct multiboot_tag_mmap *tag = (struct multiboot_tag_mmap *) ptrorig;
       grub_fill_multiboot_mmap (tag);
@@ -776,6 +777,7 @@ grub_multiboot_make_mbi (grub_uint32_t *target)
       / sizeof (grub_properly_aligned_t);
   }
 
+  if (!keep_bs)
     {
       struct multiboot_tag_basic_meminfo *tag
 	= (struct multiboot_tag_basic_meminfo *) ptrorig;
@@ -886,21 +888,17 @@ grub_multiboot_make_mbi (grub_uint32_t *target)
     grub_efi_uintn_t efi_desc_size;
     grub_efi_uint32_t efi_desc_version;
 
+    if (!keep_bs)
+      {
 	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");
-      }
+
 	if (err)
 	  return err;
+
 	tag->descr_size = efi_desc_size;
 	tag->descr_vers = efi_desc_version;
 	tag->size = sizeof (*tag) + efi_mmap_size;
@@ -908,6 +906,7 @@ grub_multiboot_make_mbi (grub_uint32_t *target)
 	ptrorig += ALIGN_UP (tag->size, MULTIBOOT_TAG_ALIGN)
 	  / sizeof (grub_properly_aligned_t);
       }
+  }
 
   if (keep_bs)
     {
-- 
1.7.10.4



  parent reply	other threads:[~2016-03-15 15:27 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-15 15:25 [GRUB2 PATCH v4 0/4] multiboot2: Add two extensions Daniel Kiper
2016-03-15 15:25 ` Daniel Kiper
2016-03-15 15:25 ` [GRUB2 PATCH v4 1/4] i386/relocator: Add grub_relocator64_efi relocator Daniel Kiper
2016-03-15 15:25   ` Daniel Kiper
2016-03-15 16:00   ` Vladimir 'phcoder' Serbinenko
2016-03-15 19:59     ` Daniel Kiper
2016-03-15 19:59     ` Daniel Kiper
2016-03-15 16:00   ` Vladimir 'phcoder' Serbinenko
2016-03-15 15:25 ` [GRUB2 PATCH v4 2/4] multiboot2: Add tags used to pass ImageHandle to loaded image Daniel Kiper
2016-03-15 15:25   ` Daniel Kiper
2016-03-15 16:03   ` Vladimir 'phcoder' Serbinenko
2016-03-15 16:03   ` Vladimir 'phcoder' Serbinenko
2016-03-15 23:39   ` Konrad Rzeszutek Wilk
2016-03-15 23:39   ` Konrad Rzeszutek Wilk
2016-03-15 15:26 ` Daniel Kiper [this message]
2016-03-15 15:26   ` [GRUB2 PATCH v4 3/4 - FOR REVIEW ONLY] multiboot2: Do not pass memory maps to image if EFI boot services are enabled Daniel Kiper
2016-03-15 23:46   ` Konrad Rzeszutek Wilk
2016-03-15 23:46   ` Konrad Rzeszutek Wilk
2016-03-16 10:02     ` Daniel Kiper
2016-03-16 10:02     ` Daniel Kiper
2016-03-16 10:14       ` Toomas Soome
2016-03-16 10:14       ` Toomas Soome
2016-03-16 10:39         ` Vladimir 'phcoder' Serbinenko
2016-03-16 10:39           ` Vladimir 'phcoder' Serbinenko
2016-03-16 13:06           ` Konrad Rzeszutek Wilk
2016-03-16 13:06           ` Konrad Rzeszutek Wilk
2016-03-15 15:26 ` [GRUB2 PATCH v4 3/4 - FOR COMMIT] " Daniel Kiper
2016-03-15 15:26   ` Daniel Kiper
2016-03-15 16:07   ` [GRUB2 PATCH v4 3/4 - FOR REVIEW ONLY] " Vladimir 'phcoder' Serbinenko
2016-03-15 16:07   ` Vladimir 'phcoder' Serbinenko
2016-03-15 18:06     ` Andrei Borzenkov
2016-03-15 18:10       ` Vladimir 'phcoder' Serbinenko
2016-03-15 18:10       ` Vladimir 'phcoder' Serbinenko
2016-03-15 20:59         ` Daniel Kiper
2016-03-15 20:59         ` Daniel Kiper
2016-03-15 18:06     ` Andrei Borzenkov
2016-03-15 20:01     ` Daniel Kiper
2016-03-15 20:01     ` Daniel Kiper
2016-03-15 15:26 ` [GRUB2 PATCH v4 4/4] multiboot2: Add support for relocatable images Daniel Kiper
2016-03-15 15:26   ` Daniel Kiper
2016-03-15 16:27   ` Vladimir 'phcoder' Serbinenko
2016-03-15 16:30     ` Vladimir 'phcoder' Serbinenko
2016-03-15 21:42       ` Daniel Kiper
2016-03-15 23:54         ` Konrad Rzeszutek Wilk
2016-03-16 10:34           ` Daniel Kiper
2016-03-16 10:34           ` Daniel Kiper
2016-03-16 10:41         ` Vladimir 'phcoder' Serbinenko
2016-03-16 10:41           ` Vladimir 'phcoder' Serbinenko
2016-03-15 21:42       ` Daniel Kiper
2016-03-15 16:30     ` Vladimir 'phcoder' Serbinenko
2016-03-15 16:27   ` Vladimir 'phcoder' Serbinenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1458055562-24950-4-git-send-email-daniel.kiper@oracle.com \
    --to=daniel.kiper@oracle.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=arvidjaar@gmail.com \
    --cc=cardoe@cardoe.com \
    --cc=david.vrabel@citrix.com \
    --cc=eric.snowberg@oracle.com \
    --cc=fu.wei@linaro.org \
    --cc=gang.wei@intel.com \
    --cc=grub-devel@gnu.org \
    --cc=jbeulich@suse.com \
    --cc=jgross@suse.com \
    --cc=ning.sun@intel.com \
    --cc=pgnet.dev@gmail.com \
    --cc=phcoder@gmail.com \
    --cc=qiaowei.ren@intel.com \
    --cc=richard.l.maliszewski@intel.com \
    --cc=roy.franz@linaro.org \
    --cc=seth.goldberg@oracle.com \
    --cc=stefano.stabellini@eu.citrix.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.