All of lore.kernel.org
 help / color / mirror / Atom feed
* Fwd: Re: [PATCH v2 6/6] multiboot2: Do not pass memory maps to image if EFI boot services are enabled
       [not found] <CAEaD8JM6aWsJve25gfm3_8Y7JzLPxvjePD3s1DoYZpOqnzmooQ@mail.gmail.com>
@ 2015-11-09 20:05 ` Vladimir 'φ-coder/phcoder' Serbinenko
  0 siblings, 0 replies; 2+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2015-11-09 20:05 UTC (permalink / raw)
  To: The development of GRUB 2

[-- Attachment #1: Type: text/plain, Size: 5690 bytes --]




-------- Forwarded Message --------
Subject: 	Re: [PATCH v2 6/6] multiboot2: Do not pass memory maps to
image if EFI boot services are enabled
Date: 	Mon, 9 Nov 2015 20:07:23 +0100
From: 	Vladimir 'phcoder' Serbinenko <phcoder@gmail.com>
To: 	Daniel Kiper <daniel.kiper@oracle.com>



This one is good to go. But the main reason for not providing the map is
because it will likely be invalid. We do few allocations after filling
the map I.a. git relocator. Usually we don't care as we would already
finish boot services but if we keep them, it's easier not to provide
them, at least until somebody needs then. Please adjust the description
and add or to the comment

Le 20 juil. 2015 4:37 PM, "Daniel Kiper" <daniel.kiper@oracle.com
<mailto:daniel.kiper@oracle.com>> a écrit :

    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.

    Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com
    <mailto:daniel.kiper@oracle.com>>
    ---
     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





[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 213 bytes --]

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Fwd: Re: [PATCH v2 6/6] multiboot2: Do not pass memory maps to image if EFI boot services are enabled
       [not found] <CAEaD8JMqNO0ZsF-hZgccjZ4eRPRMA7xrJs7xTLM9aNcvg9hmLQ@mail.gmail.com>
@ 2015-11-09 20:06 ` Vladimir 'φ-coder/phcoder' Serbinenko
  0 siblings, 0 replies; 2+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2015-11-09 20:06 UTC (permalink / raw)
  To: The development of GRUB 2

[-- Attachment #1: Type: text/plain, Size: 6470 bytes --]




-------- Forwarded Message --------
Subject: 	Re: [PATCH v2 6/6] multiboot2: Do not pass memory maps to
image if EFI boot services are enabled
Date: 	Mon, 9 Nov 2015 20:08:38 +0100
From: 	Vladimir 'phcoder' Serbinenko <phcoder@gmail.com>
To: 	Daniel Kiper <daniel.kiper@oracle.com>



And you also need to adjust loader code to reject images which request
memory tags and bs at the same time

Le 9 nov. 2015 8:07 PM, "Vladimir 'phcoder' Serbinenko"
<phcoder@gmail.com <mailto:phcoder@gmail.com>> a écrit :

    This one is good to go. But the main reason for not providing the
    map is because it will likely be invalid. We do few allocations
    after filling the map I.a. git relocator. Usually we don't care as
    we would already finish boot services but if we keep them, it's
    easier not to provide them, at least until somebody needs then.
    Please adjust the description and add or to the comment

    Le 20 juil. 2015 4:37 PM, "Daniel Kiper" <daniel.kiper@oracle.com
    <mailto:daniel.kiper@oracle.com>> a écrit :

        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.

        Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com
        <mailto:daniel.kiper@oracle.com>>
        ---
         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





[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 213 bytes --]

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2015-11-09 20:06 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <CAEaD8JM6aWsJve25gfm3_8Y7JzLPxvjePD3s1DoYZpOqnzmooQ@mail.gmail.com>
2015-11-09 20:05 ` Fwd: Re: [PATCH v2 6/6] multiboot2: Do not pass memory maps to image if EFI boot services are enabled Vladimir 'φ-coder/phcoder' Serbinenko
     [not found] <CAEaD8JMqNO0ZsF-hZgccjZ4eRPRMA7xrJs7xTLM9aNcvg9hmLQ@mail.gmail.com>
2015-11-09 20:06 ` Vladimir 'φ-coder/phcoder' Serbinenko

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.