Thank you all for the fast response.
8-byte alignment makes sense. I was looking at another implementation of multiboot2:
https://github.com/todorez/tummiboot
I thought this implementation was using a 2-byte alignment. On reviewing the code, I realized I misread it.
Thank you for the clarification.
Safayet
From: Vladimir 'phcoder' Serbinenko [mailto:address@hidden
Sent: Thursday, March 09, 2017 1:37 AM
To: Andrey Borzenkov <address@hidden>; Ahmed, Safayet (GE Global Research, US) <address@hidden>; Daniel Kiper <address@hidden>
Cc: address@hidden
Subject: EXT: Re: Tag-alignment in multiboot2 image headers
On Thu, Mar 9, 2017 at 1:17 AM, Ahmed, Safayet (GE Global Research,
US) <address@hidden> wrote:
> Hello,
>
> I'm seeing an inconsistency in the multiboot2 specification and the implementation of the multiboot2 loader code in GRUB. It may be my understanding that's incorrect. A clarification would be appreciated.
>
> This concerns the alignment requirements for tags in OS image headers. The specification states 4 bytes, but the code uses 8 bytes.
>
> The specification states (Section 3.1.3) that "Tags constitutes a buffer of structures following each other padded on 'u32' size."
>
This is ambiguous and needs better wording as well (it is not clear
whether "padded" here applies to individual tag or all tags block).
> The "for" loop for parsing tags uses the following "increment" statement (grub/grub_core/loader/multiboot_mbi2.c: line 148):
> tag = (struct multiboot_header_tag *) ((grub_uint32_t *) tag + ALIGN_UP (tag->size, MULTIBOOT_TAG_ALIGN) / 4))
>
> The macro MULTIBOOT_TAG_ALIGN is defined in (include/multiboot2.h) as 8. This alignment value is consistent with the specification for tags in the multiboot2 information structure, but not for tags in an OS image header.
>
Yes, it sure looks wrong. Thanks for making us aware!
@Vladimir, @Daniel - I think this is 2.02 material, we do not want
release with such inconsistency. The question is what needs fixing
though - about half of all tags are not multiple of 8 bytes, so I
expect people to hit it in real life. What is current implementation
in MB2 compliant kernels?
The intention was 8, so that we can have u64 naturally aligned in the header even on non-x86. I believe all current kernels are compatible with grub, so I think we should update the docs. Daniel, your opinion?