[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 6/6] multiboot2: Do not pass memory maps to image if EFI b
From: |
Konrad Rzeszutek Wilk |
Subject: |
Re: [PATCH v2 6/6] multiboot2: Do not pass memory maps to image if EFI boot services are enabled |
Date: |
Tue, 11 Aug 2015 14:59:56 -0400 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
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 <address@hidden>
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 <address@hidden>
>
> Signed-off-by: Daniel Kiper <address@hidden>
> ---
> 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
>
- Re: [PATCH v2 6/6] multiboot2: Do not pass memory maps to image if EFI boot services are enabled,
Konrad Rzeszutek Wilk <=