---------- Message transféré ----------
De : "Vladimir 'phcoder' Serbinenko" <phco...@gmail.com>
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" <konrad.w...@oracle.com>
Cc :
Le 11 août 2015 8:38 PM, "Konrad Rzeszutek Wilk" <konrad.w...@oracle.com> 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 <daniel.ki...@oracle.com>
> > ---
> > 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
> >
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
