[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: [PATCH V3] multiboot2: Implement quirk-modules-after-kernel
From: |
Chen, Zide |
Subject: |
RE: [PATCH V3] multiboot2: Implement quirk-modules-after-kernel |
Date: |
Tue, 7 Apr 2020 17:26:05 +0000 |
Hi Lukasz,
> 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.
Sure, will update the 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.
This reminds me that I may set the lowest_addr in the following way. It seems
to me that the ALIGN_UP to 1MB + 4KB
is unnecessary since grub_relocator_alloc_chunk_align() is called with PAGE
alignment, and the additional 1MB distance
is not needed. But I keep the Multiboot behavior unchanged because I don’t know
the background of adding this statement
in the first place.
diff --git a/grub-core/loader/multiboot.c b/grub-core/loader/multiboot.c
index 4a98d7082598..23fd46008aca 100644
--- a/grub-core/loader/multiboot.c
+++ b/grub-core/loader/multiboot.c
@@ -392,11 +392,14 @@ 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)
+ if (grub_multiboot_quirks & GRUB_MULTIBOOT_QUIRK_MODULES_AFTER_KERNEL) {
+#ifdef GRUB_USE_MULTIBOOT2
+ lowest_addr = highest_load;
+#else
lowest_addr = ALIGN_UP (highest_load + 1048576, 4096);
#endif
+ }
Thanks,
Zide