RE: [PATCH V3] multiboot2: Implement quirk-modules-after-kernel

[Chen, Zide]

> -----Original Message-----
> From: Lukasz Hawrylko <address@hidden>
> Sent: Tuesday, April 7, 2020 2:20 AM
> To: The development of GNU GRUB <address@hidden>
> Cc: Chen, Zide <address@hidden>
> Subject: Re: [PATCH V3] multiboot2: Implement quirk-modules-after-kernel
> 
> On Mon, 2020-04-06 at 23:30 -0700, Zide Chen wrote:
> > @@ -392,11 +395,9 @@ grub_cmd_module (grub_command_t cmd __attribute__ 
> > ((unused)),
> >    if (! file)
> >      return grub_errno;
> >
> > -#ifndef GRUB_USE_MULTIBOOT2
> >    lowest_addr = 0x100000;
> > -  if (grub_multiboot_quirks & GRUB_MULTIBOOT_QUIRK_MODULES_AFTER_KERNEL)
> > -    lowest_addr = ALIGN_UP (highest_load + 1048576, 4096);
> > -#endif
> > +  if (GRUB_MULTIBOOT (quirks) & GRUB_MULTIBOOT_QUIRK_MODULES_AFTER_KERNEL)
> > +    lowest_addr = ALIGN_UP (GRUB_MULTIBOOT (highest_load) + 1048576, 4096);
> >
> >    size = grub_file_size (file);
> >    if (size)
> >
> 
> Hi Zide
> 
> Above modification changes another behavior. Before your patch, lowest
> address for multiboot2 modules allocation was 0x0, now, when you removed
> #ifndef, it is set to 1MB even if quirk is not enabled. I think that it
> is worth to mention than in commit message.
> 
> To be honest, I was going to submit patch that sets lowest_addr to
> 0x1000 by default, so from my perspective your 'undocumented' change is
> good, 1MB works for my project too.
> 
> Thanks,
> Lukasz

Hi Lukasz,

I will abandon this patch for now and go to the path to add a new module 
relocatable tag.
If you want you can go ahead to submit your patch to set lowest_addr, which is
independent to my new approach. 

-Zide