Hi all, On Thu, Mar 09, 2017 at 09:34:34PM +0000, Ahmed, Safayet (GE Global Research, US) wrote: > 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:[email protected]] > Sent: Thursday, March 09, 2017 1:37 AM > To: Andrey Borzenkov <[email protected]>; Ahmed, Safayet (GE Global > Research, US) <[email protected]>; Daniel Kiper <[email protected]>
Hmmm... It is interesting. I have not received original email... So, sorry for late reply. > Cc: [email protected] > Subject: EXT: Re: Tag-alignment in multiboot2 image headers > > > On Wed, Mar 8, 2017, 22:28 Andrei Borzenkov > <[email protected]<mailto:[email protected]>> wrote: > On Thu, Mar 9, 2017 at 1:17 AM, Ahmed, Safayet (GE Global Research, > US) <[email protected]<mailto:[email protected]>> 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). We have to fix the spec. I will do that tomorrow. > > 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. As I can see GRUB2 code properly enforces alignment for both. Though spec is really unclear. So, I will fix it. > 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? I agree. I will do it tomorrow. Though, IMO, it should not delay RC2 cutting because spec changes go to separate branch. Daniel _______________________________________________ Bug-grub mailing list [email protected] https://lists.gnu.org/mailman/listinfo/bug-grub
