grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Fwd: Re: [PATCH v2 4/6] multiboot2: Add tags used to pass ImageHandle to


From: Vladimir 'phcoder' Serbinenko
Subject: Fwd: Re: [PATCH v2 4/6] multiboot2: Add tags used to pass ImageHandle to loaded image
Date: Mon, 9 Nov 2015 20:12:52 +0100

---------- Message transféré ----------
De : "Vladimir 'phcoder' Serbinenko" <address@hidden>
Date : 9 nov. 2015 8:12 PM
Objet : Re: [PATCH v2 4/6] multiboot2: Add tags used to pass ImageHandle to loaded image
À : "Konrad Rzeszutek Wilk" <address@hidden>
Cc :


Le 11 août 2015 8:38 PM, "Konrad Rzeszutek Wilk" <address@hidden> a écrit :
>
> On Mon, Jul 20, 2015 at 04:35:52PM +0200, Daniel Kiper wrote:
> > Add tags used to pass ImageHandle to loaded image. It is used
>
> s/loaded image/loaded image if requested./
> > by at least ExitBootServices() function.
> >
> > Signed-off-by: Daniel Kiper <address@hidden>
> > ---
> >  grub-core/loader/multiboot_mbi2.c |   46 +++++++++++++++++++++++++++++--------
> >  include/multiboot2.h              |   16 +++++++++++++
> >  2 files changed, 53 insertions(+), 9 deletions(-)
> >
> > diff --git a/grub-core/loader/multiboot_mbi2.c b/grub-core/loader/multiboot_mbi2.c
> > index 8d66e3f..dc9c709 100644
> > --- a/grub-core/loader/multiboot_mbi2.c
> > +++ b/grub-core/loader/multiboot_mbi2.c
> > @@ -172,6 +172,8 @@ grub_multiboot_load (grub_file_t file, const char *filename)
> >             case MULTIBOOT_TAG_TYPE_NETWORK:
> >             case MULTIBOOT_TAG_TYPE_EFI_MMAP:
> >             case MULTIBOOT_TAG_TYPE_EFI_BS:
> > +           case MULTIBOOT_TAG_TYPE_EFI32_IH:
> > +           case MULTIBOOT_TAG_TYPE_EFI64_IH:
> >               break;
> >
> >             default:
> > @@ -407,16 +409,18 @@ grub_multiboot_get_mbi_size (void)
> >                + grub_get_multiboot_mmap_count ()
> >                * sizeof (struct multiboot_mmap_entry)), MULTIBOOT_TAG_ALIGN)
> >      + ALIGN_UP (sizeof (struct multiboot_tag_framebuffer), MULTIBOOT_TAG_ALIGN)
> > +#ifdef GRUB_MACHINE_EFI
> >      + ALIGN_UP (sizeof (struct multiboot_tag_efi32), MULTIBOOT_TAG_ALIGN)
> >      + ALIGN_UP (sizeof (struct multiboot_tag_efi64), MULTIBOOT_TAG_ALIGN)
> > +    + ALIGN_UP (sizeof (struct multiboot_tag_efi32_ih), MULTIBOOT_TAG_ALIGN)
>
> Shouldn't they be wrapped with #ifdef __x86_64__ (for efi64_ih) and #ifdef __i386__
> for the other?
>
No need. It's just few bytes but not adding ifdef makes it more readable
> Or two functions (in the header?) wrapped with proper #ifdef machinery?
>
> > +    + ALIGN_UP (sizeof (struct multiboot_tag_efi64_ih), MULTIBOOT_TAG_ALIGN)
> > +    + ALIGN_UP (sizeof (struct multiboot_tag_efi_mmap)
> > +             + efi_mmap_size, MULTIBOOT_TAG_ALIGN)
> > +#endif
> >      + ALIGN_UP (sizeof (struct multiboot_tag_old_acpi)
> >               + sizeof (struct grub_acpi_rsdp_v10), MULTIBOOT_TAG_ALIGN)
> >      + acpiv2_size ()
> >      + net_size ()
> > -#ifdef GRUB_MACHINE_EFI
> > -    + ALIGN_UP (sizeof (struct multiboot_tag_efi_mmap)
> > -             + efi_mmap_size, MULTIBOOT_TAG_ALIGN)
> > -#endif
> >      + sizeof (struct multiboot_tag_vbe) + MULTIBOOT_TAG_ALIGN - 1
> >      + sizeof (struct multiboot_tag_apm) + MULTIBOOT_TAG_ALIGN - 1;
> >  }
> > @@ -906,11 +910,35 @@ grub_multiboot_make_mbi (grub_uint32_t *target)
> >
> >    if (keep_bs)
> >      {
> > -      struct multiboot_tag *tag = (struct multiboot_tag *) ptrorig;
> > -      tag->type = MULTIBOOT_TAG_TYPE_EFI_BS;
> > -      tag->size = sizeof (struct multiboot_tag);
> > -      ptrorig += ALIGN_UP (tag->size, MULTIBOOT_TAG_ALIGN)
> > -     / sizeof (grub_properly_aligned_t);
> > +      {
> > +     struct multiboot_tag *tag = (struct multiboot_tag *) ptrorig;
> > +     tag->type = MULTIBOOT_TAG_TYPE_EFI_BS;
> > +     tag->size = sizeof (struct multiboot_tag);
> > +     ptrorig += ALIGN_UP (tag->size, MULTIBOOT_TAG_ALIGN)
> > +       / sizeof (grub_properly_aligned_t);
> > +      }
> > +
> > +#ifdef __x86_64__
> > +      {
> > +     struct multiboot_tag_efi64_ih *tag = (struct multiboot_tag_efi64_ih *) ptrorig;
> > +     tag->type = MULTIBOOT_TAG_TYPE_EFI64_IH;
> > +     tag->size = sizeof (*tag);
>
> The other places where sizeof is used ends up using the full structure so it :
>
> sizeof (struct multiboot_tag_efi64_ih)
>
sizeof (*tag) is fine and is actually better
> > +     tag->pointer = (grub_addr_t) grub_efi_image_handle;
> > +     ptrorig += ALIGN_UP (tag->size, MULTIBOOT_TAG_ALIGN)
> > +       / sizeof (grub_properly_aligned_t);
> > +      }
> > +#endif
> > +
> > +#ifdef __i386__
> > +      {
> > +     struct multiboot_tag_efi32_ih *tag = (struct multiboot_tag_efi32_ih *) ptrorig;
> > +     tag->type = MULTIBOOT_TAG_TYPE_EFI32_IH;
> > +     tag->size = sizeof (*tag);
>
> Ditto.
> > +     tag->pointer = (grub_addr_t) grub_efi_image_handle;
> > +     ptrorig += ALIGN_UP (tag->size, MULTIBOOT_TAG_ALIGN)
> > +       / sizeof (grub_properly_aligned_t);
> > +      }
> > +#endif
> >      }
> >  #endif
> >
> > diff --git a/include/multiboot2.h b/include/multiboot2.h
> > index b3977e3..9f97ddc 100644
> > --- a/include/multiboot2.h
> > +++ b/include/multiboot2.h
> > @@ -60,6 +60,8 @@
> >  #define MULTIBOOT_TAG_TYPE_NETWORK           16
> >  #define MULTIBOOT_TAG_TYPE_EFI_MMAP          17
> >  #define MULTIBOOT_TAG_TYPE_EFI_BS            18
> > +#define MULTIBOOT_TAG_TYPE_EFI32_IH          19
> > +#define MULTIBOOT_TAG_TYPE_EFI64_IH          20
> >
> >  #define MULTIBOOT_HEADER_TAG_END  0
> >  #define MULTIBOOT_HEADER_TAG_INFORMATION_REQUEST  1
> > @@ -379,6 +381,20 @@ struct multiboot_tag_efi_mmap
> >    multiboot_uint8_t efi_mmap[0];
> >  };
> >
> > +struct multiboot_tag_efi32_ih
> > +{
> > +  multiboot_uint32_t type;
> > +  multiboot_uint32_t size;
> > +  multiboot_uint32_t pointer;
> > +};
> > +
> > +struct multiboot_tag_efi64_ih
> > +{
> > +  multiboot_uint32_t type;
> > +  multiboot_uint32_t size;
> > +  multiboot_uint64_t pointer;
> > +};
> > +
> >  #endif /* ! ASM_FILE */
> >
> >  #endif /* ! MULTIBOOT_HEADER */
> > --
> > 1.7.10.4
> >


reply via email to

[Prev in Thread] Current Thread [Next in Thread]