RE: [MULTIBOOT2 SPEC PATCH V2] multiboot2: Add module load and preference tag

[Chen, Zide]

Hi Daniel,

> -----Original Message-----
> From: Chen, Zide
> Sent: Wednesday, May 13, 2020 3:39 PM
> To: 'Daniel Kiper' <address@hidden>
> Cc: address@hidden
> Subject: RE: [MULTIBOOT2 SPEC PATCH V2] multiboot2: Add module load and 
> preference tag
> > >
> > > @@ -730,6 +731,46 @@ Boot loader should follow it. @samp{0} means none, 
> > > @samp{1} means
> > >  load image at lowest possible address but not lower than min_addr
> > >  and @samp{2} means load image at highest possible address but not
> > >  higher than max_addr.
> > > +
> > > +@node Module load preferences tag
> > > +@subsection Module load preferences tag
> > > +
> > > +@example
> > > +@group
> > > +        +-------------------+
> > > +u16     | type = 11         |
> > > +u16     | flags             |
> > > +u32     | size = 20         |
> > > +u32     | min_addr          |
> > > +u32     | max_addr          |
> >
> > I have an itching to add "align" member here like in the relocatable
> > header tag. I can imagine that some kernels may want aligned modules. If
> > you do that then we have to resolve conflict with the module alignment
> > tag. I think if both tags are in the image header then the bootloader
> > should use max(align, PAGE_SIZE). This behavior should be described in
> > one way or another next to both tags.
> 
> Yes, I thought about the align member.
> Sure, I can add the it to the spec. Since we give user the flexibility to 
> specify alignment,
> why we would overwrite user's choice to force it to minimum PAGE_SIZE aligned?
> How about pick the maximum alignment of the two tags?
> 
> But in GRUB's implementation, since GRUB ignores the module alignment tag, 
> the behavior
> Could be these:
> 
> If align is zero in this tag: module alignment is PAGE_SIZE (
> 
> If align is none-zero, Grub will use whatever value it is to load modules.

You are right. Please ignore my previous comments. I have sent out the updated 
patch for review.

-Zide