| Message ID | 1458055562-24950-5-git-send-email-daniel.kiper@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Looks good. Let's give a day for others to comment. Is the second email the version for commit? On Tuesday, March 15, 2016, Daniel Kiper <daniel.kiper@oracle.com> wrote: > 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 <javascript:;>> > Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com <javascript:;>> > --- > v3 - suggestions/fixes: > - improve commit message > (suggested by Konrad Rzeszutek Wilk and Vladimir 'phcoder' > Serbinenko). > --- > 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 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,12 +755,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 > @@ -776,18 +777,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; > @@ -886,27 +888,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 > >
15.03.2016 19:07, Vladimir 'phcoder' Serbinenko ?????: > Looks good. Let's give a day for others to comment. Is the second email the > version for commit? > > On Tuesday, March 15, 2016, Daniel Kiper <daniel.kiper@oracle.com> wrote: > >> 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. >> Are there any existing users of it that rely on memory map provided by bootloader? What if image explicitly requested non-optional memory map? According to multiboot specification it is valid and bootloader must either provide requested information or fail load. >> Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com <javascript:;>> >> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com <javascript:;>> >> --- >> v3 - suggestions/fixes: >> - improve commit message >> (suggested by Konrad Rzeszutek Wilk and Vladimir 'phcoder' >> Serbinenko). >> --- >> 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 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,12 +755,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 >> @@ -776,18 +777,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; >> @@ -886,27 +888,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 >> >> >
Le mar. 15 mars 2016 19:06, Andrei Borzenkov <arvidjaar@gmail.com> a écrit : > 15.03.2016 19:07, Vladimir 'phcoder' Serbinenko ?????: > > Looks good. Let's give a day for others to comment. Is the second email > the > > version for commit? > > > > On Tuesday, March 15, 2016, Daniel Kiper <daniel.kiper@oracle.com> > wrote: > > > >> 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. > >> > > Are there any existing users of it that rely on memory map provided by > bootloader? What if image explicitly requested non-optional memory map? > According to multiboot specification it is valid and bootloader must > either provide requested information or fail load. > I think you have a point. @Daniel, is current behavior a problem for you? Don't you just ignore the memory map in this case? > > >> Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com <javascript:;>> > >> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com > <javascript:;>> > >> --- > >> v3 - suggestions/fixes: > >> - improve commit message > >> (suggested by Konrad Rzeszutek Wilk and Vladimir 'phcoder' > >> Serbinenko). > >> --- > >> 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 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,12 +755,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 > >> @@ -776,18 +777,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; > >> @@ -886,27 +888,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 > >> > >> > > > >
On Tue, Mar 15, 2016 at 05:07:43PM +0100, Vladimir 'phcoder' Serbinenko wrote: > Looks good. Let's give a day for others to comment. Is the second email the > version for commit? Yep, as you asked for. Daniel
On Tue, Mar 15, 2016 at 06:10:48PM +0000, Vladimir 'phcoder' Serbinenko wrote: > Le mar. 15 mars 2016 19:06, Andrei Borzenkov <arvidjaar@gmail.com> a écrit : > > 15.03.2016 19:07, Vladimir 'phcoder' Serbinenko ?????: > > > Looks good. Let's give a day for others to comment. Is the second email > > the > > > version for commit? > > > > > > On Tuesday, March 15, 2016, Daniel Kiper <daniel.kiper@oracle.com> > > wrote: > > > > > >> 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. > > >> > > > > Are there any existing users of it that rely on memory map provided by > > bootloader? What if image explicitly requested non-optional memory map? > > According to multiboot specification it is valid and bootloader must > > either provide requested information or fail load. > > > I think you have a point. @Daniel, is current behavior a problem for you? Do you think about MULTIBOOT_HEADER_TAG_INFORMATION_REQUEST tag? If yes it looks that multiboot2 doc is a bit imprecise and/or GRUB2 code is wrong. multiboot2 doc says: [...] If this tag is present and ‘optional’ is set to ‘0’ information conveyed by requested tag types must be present. If bootloader is unable to supply this information it must fail with an error Note: it doesn’t garantee that any tags of type ‘mbi_tag_types’ will actually be present. E.g. on a videoless system even if you requested tag ‘8’ no tags of type ‘8’ will be present in mbi. [...] However, grub_multiboot_load() just looks for MULTIBOOT_HEADER_TAG_INFORMATION_REQUEST tag. If it exists and if ‘optional’ is set to ‘0’ then grub_multiboot_load() verify list of requested tags. If unknown value is found then multiboot2 command fails with "grub_error (GRUB_ERR_UNKNOWN_OS, "unsupported information tag: 0x%x", ...". I cannot find additional checks anywhere... So, AIUI, "If bootloader is unable to supply..." should be read as "If bootloader does not support..." or even "If bootloader do not understand..." or something like that. This does not mean that bootloader must have requested data in hand and provide it to image. It means that bootloader must understand what image ask for. And in this term my patch does not violate spec implied by implementation (well, multiboot2 doc makes some confusion; see below). Image may ask/request for any memory map. However, if a given map (or set of maps) is not available or it could be provided but could make confusion in some cases then it can/should not be provided. Anyway, I think that maybe multiboot2 doc should be fixed and more precise MULTIBOOT_HEADER_TAG_INFORMATION_REQUEST tag description should be provided. Or GRUB2 implementation is wrong then... > Don't you just ignore the memory map in this case? Yes to some extent. However, I think that loader should not provide data which is known as (potentially) incorrect/faulty. Daniel
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,12 +755,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 @@ -776,18 +777,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; @@ -886,27 +888,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)