All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] multiboot2: Add module relocatable tag to support modules relocation
@ 2020-04-16 22:56 Zide Chen
  2020-04-16 22:56 ` [PATCH] multiboot2: Implement the new module_relocatable_tag Zide Chen
  2020-05-07 12:53 ` [PATCH] multiboot2: Add module relocatable tag to support modules relocation Daniel Kiper
  0 siblings, 2 replies; 5+ messages in thread
From: Zide Chen @ 2020-04-16 22:56 UTC (permalink / raw)
  To: grub-devel; +Cc: Zide Chen

Also change the name from "relocatable header tag" to "kernel relocatable
tag" to make it less confusing. These two tags are independent from each
other.

Signed-off-by: Zide Chen <zide.chen@intel.com>
---
 doc/multiboot.texi | 51 +++++++++++++++++++++++++++++++++++++++++-----
 doc/multiboot2.h   | 11 ++++++++++
 2 files changed, 57 insertions(+), 5 deletions(-)

diff --git a/doc/multiboot.texi b/doc/multiboot.texi
index df8a0d056e76..bf0150dc86a0 100644
--- a/doc/multiboot.texi
+++ b/doc/multiboot.texi
@@ -355,7 +355,8 @@ executable header.
 * Console header tags::
 * Module alignment tag::
 * EFI boot services tag::
-* Relocatable header tag::
+* Kernel relocatable tag::
+* Module relocatable tag::
 
 @end menu
 
@@ -691,8 +692,8 @@ u32     | size = 8          |
 This tag indicates that payload supports starting without
 terminating boot services.
 
-@node Relocatable header tag
-@subsection Relocatable header tag
+@node Kernel relocatable tag
+@subsection Kernel relocatable tag
 
 @example
 @group
@@ -708,7 +709,7 @@ u32     | preference        |
 @end group
 @end example
 
-This tag indicates that image is relocatable.
+This tag indicates that kernel image is relocatable.
 
 The meaning of each field is as follows:
 
@@ -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 relocatable tag
+@subsection Module relocatable tag
+
+@example
+@group
+        +-------------------+
+u16     | type = 11         |
+u16     | flags             |
+u32     | size = 20         |
+u32     | min_addr          |
+u32     | max_addr          |
+u32     | preference        |
+        +-------------------+
+@end group
+@end example
+
+This tag is independent to the kernel relocatable tag. All of the
+address fields in this tag are physical addresses.
+
+The meaning of each field is as follows:
+
+@table @code
+@item min_addr
+Lowest possible physical address at which any modules should be
+loaded. The bootloader cannot load any part of any modules below
+this address.
+
+@item max_addr
+Highest possible physical address at which any loaded modules should
+end. The bootloader cannot load any part of any modules above this
+address.
+
+@item preference
+It contains load address placement suggestion for Multiboot2 modules
+Boot loader should follow it. @samp{0} means load modules not lower
+than min_addr, and not higher than max_addr, but no preference on either
+lower or higher address; @samp{1} means load modules at lowest possible
+address but not lower than min_addr; @samp{2} means load modules at
+highest possible address but not higher than max_addr.
 @end table
 
 @node Machine state
@@ -1399,7 +1440,7 @@ u32     | load_base_addr    |
 @end example
 
 This tag contains image load base physical address. It is
-provided only if image has relocatable header tag.
+provided only if image has kernel relocatable tag.
 
 @node Examples
 @chapter Examples
diff --git a/doc/multiboot2.h b/doc/multiboot2.h
index b181607075b2..d97d10c2691a 100644
--- a/doc/multiboot2.h
+++ b/doc/multiboot2.h
@@ -75,6 +75,7 @@
 #define MULTIBOOT_HEADER_TAG_ENTRY_ADDRESS_EFI32  8
 #define MULTIBOOT_HEADER_TAG_ENTRY_ADDRESS_EFI64  9
 #define MULTIBOOT_HEADER_TAG_RELOCATABLE  10
+#define MULTIBOOT_HEADER_TAG_MODULE_RELOCATABLE  11
 
 #define MULTIBOOT_ARCHITECTURE_I386  0
 #define MULTIBOOT_ARCHITECTURE_MIPS32  4
@@ -179,6 +180,16 @@ struct multiboot_header_tag_relocatable
   multiboot_uint32_t preference;
 };
 
+struct multiboot_header_tag_module_relocatable
+{
+  multiboot_uint16_t type;
+  multiboot_uint16_t flags;
+  multiboot_uint32_t size;
+  multiboot_uint32_t min_addr;
+  multiboot_uint32_t max_addr;
+  multiboot_uint32_t preference;
+};
+
 struct multiboot_color
 {
   multiboot_uint8_t red;
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH] multiboot2: Implement the new module_relocatable_tag
  2020-04-16 22:56 [PATCH] multiboot2: Add module relocatable tag to support modules relocation Zide Chen
@ 2020-04-16 22:56 ` Zide Chen
  2020-05-07 12:53 ` [PATCH] multiboot2: Add module relocatable tag to support modules relocation Daniel Kiper
  1 sibling, 0 replies; 5+ messages in thread
From: Zide Chen @ 2020-04-16 22:56 UTC (permalink / raw)
  To: grub-devel; +Cc: Zide Chen

In contrast to Mulitboot, in Mulitboot2, there is currently no way to
control where to load the modules to. This could be a problem, e.g., the
OS loaded by Multiboot2 needs to reserve low address for some particular
purposes and not for loading modules.

This new tag gives it flexibility to control the modules load addresses,
which is independent to whether the kernel relocatable tag is present or
not.

Signed-off-by: Zide Chen <zide.chen@intel.com>
---
 grub-core/loader/multiboot.c      | 22 ++++++++++++++++++++--
 grub-core/loader/multiboot_mbi2.c | 23 +++++++++++++++++++++++
 include/grub/multiboot2.h         |  9 +++++++++
 include/multiboot2.h              | 11 +++++++++++
 4 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/grub-core/loader/multiboot.c b/grub-core/loader/multiboot.c
index 4a98d7082598..099a8e981126 100644
--- a/grub-core/loader/multiboot.c
+++ b/grub-core/loader/multiboot.c
@@ -369,6 +369,8 @@ grub_cmd_module (grub_command_t cmd __attribute__ ((unused)),
   grub_err_t err;
   int nounzip = 0;
   grub_uint64_t lowest_addr = 0;
+  grub_uint64_t max_addr;
+  int preference = GRUB_RELOCATOR_PREFERENCE_NONE;
 
   if (argc == 0)
     return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("filename expected"));
@@ -401,11 +403,27 @@ grub_cmd_module (grub_command_t cmd __attribute__ ((unused)),
   size = grub_file_size (file);
   if (size)
   {
+    max_addr = (0xffffffff - size) + 1;
+
+#ifdef GRUB_USE_MULTIBOOT2
+    if (grub_multiboot2_mri.relocatable)
+      {
+        lowest_addr = grub_multiboot2_mri.min_addr;
+        max_addr = grub_multiboot2_mri.max_addr;
+        preference = grub_multiboot2_mri.preference;
+        if (size > max_addr || lowest_addr > max_addr - size)
+          {
+            grub_file_close (file);
+            return grub_error (GRUB_ERR_BAD_ARGUMENT, "invalid min/max address and/or load size for modules");
+          }
+      }
+#endif
+
     grub_relocator_chunk_t ch;
     err = grub_relocator_alloc_chunk_align (GRUB_MULTIBOOT (relocator), &ch,
-					    lowest_addr, (0xffffffff - size) + 1,
+					    lowest_addr, max_addr,
 					    size, MULTIBOOT_MOD_ALIGN,
-					    GRUB_RELOCATOR_PREFERENCE_NONE, 1);
+					    preference, 1);
     if (err)
       {
 	grub_file_close (file);
diff --git a/grub-core/loader/multiboot_mbi2.c b/grub-core/loader/multiboot_mbi2.c
index 18e766c7b31c..c2639623228e 100644
--- a/grub-core/loader/multiboot_mbi2.c
+++ b/grub-core/loader/multiboot_mbi2.c
@@ -75,6 +75,7 @@ static unsigned elf_sec_shstrndx;
 static void *elf_sections;
 static int keep_bs = 0;
 static grub_uint32_t load_base_addr;
+struct module_relocatable_info grub_multiboot2_mri;
 
 void
 grub_multiboot2_add_elfsyms (grub_size_t num, grub_size_t entsize,
@@ -114,6 +115,7 @@ grub_multiboot2_load (grub_file_t file, const char *filename)
   struct multiboot_header_tag *tag;
   struct multiboot_header_tag_address *addr_tag = NULL;
   struct multiboot_header_tag_relocatable *rel_tag;
+  struct multiboot_header_tag_module_relocatable *mod_rel_tag;
   int entry_specified = 0, efi_entry_specified = 0;
   grub_addr_t entry = 0, efi_entry = 0;
   grub_uint32_t console_required = 0;
@@ -123,6 +125,7 @@ grub_multiboot2_load (grub_file_t file, const char *filename)
 
   mld.mbi_ver = 2;
   mld.relocatable = 0;
+  grub_multiboot2_mri.relocatable = 0;
 
   mld.buffer = grub_malloc (MULTIBOOT_SEARCH);
   if (!mld.buffer)
@@ -248,6 +251,26 @@ grub_multiboot2_load (grub_file_t file, const char *filename)
 	  }
 	break;
 
+      case MULTIBOOT_HEADER_TAG_MODULE_RELOCATABLE:
+	mod_rel_tag = (struct multiboot_header_tag_module_relocatable *) tag;
+	grub_multiboot2_mri.relocatable = 1;
+	grub_multiboot2_mri.min_addr = mod_rel_tag->min_addr;
+	grub_multiboot2_mri.max_addr = mod_rel_tag->max_addr;
+	switch (mod_rel_tag->preference)
+	  {
+	  case MULTIBOOT_LOAD_PREFERENCE_LOW:
+	    grub_multiboot2_mri.preference = GRUB_RELOCATOR_PREFERENCE_LOW;
+	    break;
+
+	  case MULTIBOOT_LOAD_PREFERENCE_HIGH:
+	    grub_multiboot2_mri.preference = GRUB_RELOCATOR_PREFERENCE_HIGH;
+	    break;
+
+	  default:
+	    grub_multiboot2_mri.preference = GRUB_RELOCATOR_PREFERENCE_NONE;
+	  }
+	break;
+
 	/* GRUB always page-aligns modules.  */
       case MULTIBOOT_HEADER_TAG_MODULE_ALIGN:
 	break;
diff --git a/include/grub/multiboot2.h b/include/grub/multiboot2.h
index 502d34ef1804..6d7598eeffcc 100644
--- a/include/grub/multiboot2.h
+++ b/include/grub/multiboot2.h
@@ -92,6 +92,14 @@ struct mbi_load_data
 };
 typedef struct mbi_load_data mbi_load_data_t;
 
+struct module_relocatable_info
+{
+  int relocatable;
+  grub_uint32_t min_addr;
+  grub_uint32_t max_addr;
+  grub_uint32_t preference;
+};
+
 /* Load ELF32 or ELF64.  */
 grub_err_t
 grub_multiboot2_load_elf (mbi_load_data_t *mld);
@@ -99,6 +107,7 @@ grub_multiboot2_load_elf (mbi_load_data_t *mld);
 extern grub_size_t grub_multiboot2_pure_size;
 extern grub_size_t grub_multiboot2_alloc_mbi;
 extern grub_uint32_t grub_multiboot2_payload_eip;
+extern struct module_relocatable_info grub_multiboot2_mri;
 
 
 #endif /* ! GRUB_MULTIBOOT_HEADER */
diff --git a/include/multiboot2.h b/include/multiboot2.h
index 5693923c014f..41b973aed193 100644
--- a/include/multiboot2.h
+++ b/include/multiboot2.h
@@ -74,6 +74,7 @@
 #define MULTIBOOT_HEADER_TAG_EFI_BS  7
 #define MULTIBOOT_HEADER_TAG_ENTRY_ADDRESS_EFI64  9
 #define MULTIBOOT_HEADER_TAG_RELOCATABLE  10
+#define MULTIBOOT_HEADER_TAG_MODULE_RELOCATABLE  11
 
 #define MULTIBOOT2_ARCHITECTURE_I386  0
 #define MULTIBOOT2_ARCHITECTURE_MIPS32  4
@@ -178,6 +179,16 @@ struct multiboot_header_tag_relocatable
   multiboot_uint32_t preference;
 };
 
+struct multiboot_header_tag_module_relocatable
+{
+  multiboot_uint16_t type;
+  multiboot_uint16_t flags;
+  multiboot_uint32_t size;
+  multiboot_uint32_t min_addr;
+  multiboot_uint32_t max_addr;
+  multiboot_uint32_t preference;
+};
+
 struct multiboot_color
 {
   multiboot_uint8_t red;
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] multiboot2: Add module relocatable tag to support modules relocation
  2020-04-16 22:56 [PATCH] multiboot2: Add module relocatable tag to support modules relocation Zide Chen
  2020-04-16 22:56 ` [PATCH] multiboot2: Implement the new module_relocatable_tag Zide Chen
@ 2020-05-07 12:53 ` Daniel Kiper
  2020-05-07 21:31   ` Chen, Zide
  1 sibling, 1 reply; 5+ messages in thread
From: Daniel Kiper @ 2020-05-07 12:53 UTC (permalink / raw)
  To: Zide Chen; +Cc: grub-devel

On Thu, Apr 16, 2020 at 03:56:08PM -0700, Zide Chen wrote:
> Also change the name from "relocatable header tag" to "kernel relocatable
> tag" to make it less confusing. These two tags are independent from each
> other.

First of all, the commit message should say what the patch does and why.
Just in a few words. You do not need to repeat everything from below.
Though it have to be clear what happens without looking at the patch content.

Additionally, may I ask you to send new patch for Multiboo2 spec with
"[MULTIBOOT2 SPEC PATCH]" subject prefix instead of normal "[PATCH]" one?

> Signed-off-by: Zide Chen <zide.chen@intel.com>
> ---
>  doc/multiboot.texi | 51 +++++++++++++++++++++++++++++++++++++++++-----
>  doc/multiboot2.h   | 11 ++++++++++
>  2 files changed, 57 insertions(+), 5 deletions(-)
>
> diff --git a/doc/multiboot.texi b/doc/multiboot.texi
> index df8a0d056e76..bf0150dc86a0 100644
> --- a/doc/multiboot.texi
> +++ b/doc/multiboot.texi
> @@ -355,7 +355,8 @@ executable header.
>  * Console header tags::
>  * Module alignment tag::
>  * EFI boot services tag::
> -* Relocatable header tag::
> +* Kernel relocatable tag::
> +* Module relocatable tag::

IMO this is not "Module relocatable tag". It is rather "Module load
preferences tag".

>  @end menu
>
> @@ -691,8 +692,8 @@ u32     | size = 8          |
>  This tag indicates that payload supports starting without
>  terminating boot services.
>
> -@node Relocatable header tag
> -@subsection Relocatable header tag
> +@node Kernel relocatable tag
> +@subsection Kernel relocatable tag

I am OK with this change. However, it should be done in separate patch.

>  @example
>  @group
> @@ -708,7 +709,7 @@ u32     | preference        |
>  @end group
>  @end example
>
> -This tag indicates that image is relocatable.
> +This tag indicates that kernel image is relocatable.

Ditto. However, if you want to change this then whole description of
relocatable header tag requires more editing too.

>  The meaning of each field is as follows:
>
> @@ -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 relocatable tag
> +@subsection Module relocatable tag

Please take a look above...

> +@example
> +@group
> +        +-------------------+
> +u16     | type = 11         |
> +u16     | flags             |
> +u32     | size = 20         |
> +u32     | min_addr          |
> +u32     | max_addr          |
> +u32     | preference        |
> +        +-------------------+
> +@end group
> +@end example
> +
> +This tag is independent to the kernel relocatable tag. All of the
> +address fields in this tag are physical addresses.
> +
> +The meaning of each field is as follows:
> +
> +@table @code
> +@item min_addr
> +Lowest possible physical address at which any modules should be
> +loaded. The bootloader cannot load any part of any modules below
> +this address.
> +
> +@item max_addr
> +Highest possible physical address at which any loaded modules should
> +end. The bootloader cannot load any part of any modules above this
> +address.
> +
> +@item preference
> +It contains load address placement suggestion for Multiboot2 modules

s/Multiboot2 modules/the boot loader./

> +Boot loader should follow it. @samp{0} means load modules not lower
> +than min_addr, and not higher than max_addr, but no preference on either

s/,//g

> +lower or higher address; @samp{1} means load modules at lowest possible

s/;/./

> +address but not lower than min_addr; @samp{2} means load modules at

s/;/./

> +highest possible address but not higher than max_addr.

After reading all threads related to this change somehow I got an
impression that you want to use this tag to ask the booloader to load
the modules above the kernel. Am I right? If this is true then we should
have another value, 4, which says: please load modules above the kernel.
...and maybe 8 for below the kernel... Or vice versa would be better.
Otherwise IMO you will be playing games with relocatable tag and this
new tag to gain what you want. This will be not nice and maybe not very
reliable.

Daniel


^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH] multiboot2: Add module relocatable tag to support modules relocation
  2020-05-07 12:53 ` [PATCH] multiboot2: Add module relocatable tag to support modules relocation Daniel Kiper
@ 2020-05-07 21:31   ` Chen, Zide
  2020-05-11 14:23     ` Daniel Kiper
  0 siblings, 1 reply; 5+ messages in thread
From: Chen, Zide @ 2020-05-07 21:31 UTC (permalink / raw)
  To: Daniel Kiper; +Cc: grub-devel

Hi Daniel,

Thank you very much for your review! Comments inline:

Best Regards,
Zide

> -----Original Message-----
> From: Daniel Kiper <dkiper@net-space.pl>
> Sent: Thursday, May 7, 2020 5:54 AM
> To: Chen, Zide <zide.chen@intel.com>
> Cc: grub-devel@gnu.org
> Subject: Re: [PATCH] multiboot2: Add module relocatable tag to support modules relocation
> 
> On Thu, Apr 16, 2020 at 03:56:08PM -0700, Zide Chen wrote:
> > Also change the name from "relocatable header tag" to "kernel relocatable
> > tag" to make it less confusing. These two tags are independent from each
> > other.
> 
> First of all, the commit message should say what the patch does and why.
> Just in a few words. You do not need to repeat everything from below.
> Though it have to be clear what happens without looking at the patch content.

Yes, will do.

> Additionally, may I ask you to send new patch for Multiboo2 spec with
> "[MULTIBOOT2 SPEC PATCH]" subject prefix instead of normal "[PATCH]" one?

Yes, will do.
 
> > Signed-off-by: Zide Chen <zide.chen@intel.com>
> > ---
> >  doc/multiboot.texi | 51 +++++++++++++++++++++++++++++++++++++++++-----
> >  doc/multiboot2.h   | 11 ++++++++++
> >  2 files changed, 57 insertions(+), 5 deletions(-)
> >
> > diff --git a/doc/multiboot.texi b/doc/multiboot.texi
> > index df8a0d056e76..bf0150dc86a0 100644
> > --- a/doc/multiboot.texi
> > +++ b/doc/multiboot.texi
> > @@ -355,7 +355,8 @@ executable header.
> >  * Console header tags::
> >  * Module alignment tag::
> >  * EFI boot services tag::
> > -* Relocatable header tag::
> > +* Kernel relocatable tag::
> > +* Module relocatable tag::
> 
> IMO this is not "Module relocatable tag". It is rather "Module load
> preferences tag".

Sure, I can change the name. I chose this name because the contain of this new tag is almost identical to the
relocatable header tag. 

> >  @end menu
> >
> > @@ -691,8 +692,8 @@ u32     | size = 8          |
> >  This tag indicates that payload supports starting without
> >  terminating boot services.
> >
> > -@node Relocatable header tag
> > -@subsection Relocatable header tag
> > +@node Kernel relocatable tag
> > +@subsection Kernel relocatable tag
> 
> I am OK with this change. However, it should be done in separate patch.

If this new tag is not named as "module relocatable tag", then I don't think it's needed to do this name changing any more,
since now these two names are not confusing any more.

> 
> >  @example
> >  @group
> > @@ -708,7 +709,7 @@ u32     | preference        |
> >  @end group
> >  @end example
> >
> > -This tag indicates that image is relocatable.
> > +This tag indicates that kernel image is relocatable.
> 
> Ditto. However, if you want to change this then whole description of
> relocatable header tag requires more editing too.

Ditto. Seems no need to change name for relocatable header tag any more.

> > +@table @code
> > +@item min_addr
> > +Lowest possible physical address at which any modules should be
> > +loaded. The bootloader cannot load any part of any modules below
> > +this address.
> > +
> > +@item max_addr
> > +Highest possible physical address at which any loaded modules should
> > +end. The bootloader cannot load any part of any modules above this
> > +address.
> > +
> > +@item preference
> > +It contains load address placement suggestion for Multiboot2 modules
> 
> s/Multiboot2 modules/the boot loader./

Yes, will do.
 
> > +Boot loader should follow it. @samp{0} means load modules not lower
> > +than min_addr, and not higher than max_addr, but no preference on either
> 
> s/,//g

Yes, will do.

> 
> > +lower or higher address; @samp{1} means load modules at lowest possible
> 
> s/;/./

Yes, will do.

> > +address but not lower than min_addr; @samp{2} means load modules at
> 
> s/;/./

Yes, will do.

> > +highest possible address but not higher than max_addr.
> 
> After reading all threads related to this change somehow I got an
> impression that you want to use this tag to ask the booloader to load
> the modules above the kernel. Am I right? If this is true then we should
> have another value, 4, which says: please load modules above the kernel.
> ...and maybe 8 for below the kernel... Or vice versa would be better.
> Otherwise IMO you will be playing games with relocatable tag and this
> new tag to gain what you want. This will be not nice and maybe not very
> reliable.

Not really. My purpose is to load modules not in the lower address and it doesn't
Matter whether it's loaded before or after kernel image.

I chose quirk-modules-after-kernel just because this quirk is supported in multiboot,
and it seems naturally to bring it to multiboot2 as well.

> Daniel


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] multiboot2: Add module relocatable tag to support modules relocation
  2020-05-07 21:31   ` Chen, Zide
@ 2020-05-11 14:23     ` Daniel Kiper
  0 siblings, 0 replies; 5+ messages in thread
From: Daniel Kiper @ 2020-05-11 14:23 UTC (permalink / raw)
  To: Chen, Zide; +Cc: grub-devel

On Thu, May 07, 2020 at 09:31:24PM +0000, Chen, Zide wrote:
> Hi Daniel,
>
> Thank you very much for your review! Comments inline:
>
> Best Regards,
> Zide
>
> > -----Original Message-----
> > From: Daniel Kiper <dkiper@net-space.pl>
> > Sent: Thursday, May 7, 2020 5:54 AM
> > To: Chen, Zide <zide.chen@intel.com>
> > Cc: grub-devel@gnu.org
> > Subject: Re: [PATCH] multiboot2: Add module relocatable tag to support modules relocation
> >
> > On Thu, Apr 16, 2020 at 03:56:08PM -0700, Zide Chen wrote:
> > > Also change the name from "relocatable header tag" to "kernel relocatable
> > > tag" to make it less confusing. These two tags are independent from each
> > > other.
> >
> > First of all, the commit message should say what the patch does and why.
> > Just in a few words. You do not need to repeat everything from below.
> > Though it have to be clear what happens without looking at the patch content.
>
> Yes, will do.
>
> > Additionally, may I ask you to send new patch for Multiboo2 spec with
> > "[MULTIBOOT2 SPEC PATCH]" subject prefix instead of normal "[PATCH]" one?
>
> Yes, will do.
>
> > > Signed-off-by: Zide Chen <zide.chen@intel.com>
> > > ---
> > >  doc/multiboot.texi | 51 +++++++++++++++++++++++++++++++++++++++++-----
> > >  doc/multiboot2.h   | 11 ++++++++++
> > >  2 files changed, 57 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/doc/multiboot.texi b/doc/multiboot.texi
> > > index df8a0d056e76..bf0150dc86a0 100644
> > > --- a/doc/multiboot.texi
> > > +++ b/doc/multiboot.texi
> > > @@ -355,7 +355,8 @@ executable header.
> > >  * Console header tags::
> > >  * Module alignment tag::
> > >  * EFI boot services tag::
> > > -* Relocatable header tag::
> > > +* Kernel relocatable tag::
> > > +* Module relocatable tag::
> >
> > IMO this is not "Module relocatable tag". It is rather "Module load
> > preferences tag".
>
> Sure, I can change the name. I chose this name because the contain of this new tag is almost identical to the
> relocatable header tag.
>
> > >  @end menu
> > >
> > > @@ -691,8 +692,8 @@ u32     | size = 8          |
> > >  This tag indicates that payload supports starting without
> > >  terminating boot services.
> > >
> > > -@node Relocatable header tag
> > > -@subsection Relocatable header tag
> > > +@node Kernel relocatable tag
> > > +@subsection Kernel relocatable tag
> >
> > I am OK with this change. However, it should be done in separate patch.
>
> If this new tag is not named as "module relocatable tag", then I don't think it's needed to do this name changing any more,
> since now these two names are not confusing any more.

OK.

[...]

> > > +highest possible address but not higher than max_addr.
> >
> > After reading all threads related to this change somehow I got an
> > impression that you want to use this tag to ask the booloader to load
> > the modules above the kernel. Am I right? If this is true then we should
> > have another value, 4, which says: please load modules above the kernel.
> > ...and maybe 8 for below the kernel... Or vice versa would be better.
> > Otherwise IMO you will be playing games with relocatable tag and this
> > new tag to gain what you want. This will be not nice and maybe not very
> > reliable.
>
> Not really. My purpose is to load modules not in the lower address and it doesn't
> Matter whether it's loaded before or after kernel image.

Then you can ignore my comment above.

Daniel


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-05-11 14:24 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-16 22:56 [PATCH] multiboot2: Add module relocatable tag to support modules relocation Zide Chen
2020-04-16 22:56 ` [PATCH] multiboot2: Implement the new module_relocatable_tag Zide Chen
2020-05-07 12:53 ` [PATCH] multiboot2: Add module relocatable tag to support modules relocation Daniel Kiper
2020-05-07 21:31   ` Chen, Zide
2020-05-11 14:23     ` Daniel Kiper

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.