[PATCH v3] multiboot2: Implement the new module load and preferences tag

[PATCH v3] multiboot2: Implement the new module load and preferences tag

[Lukasz Hawrylko]

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>
Signed-off-by: Lukasz Hawrylko <lukasz.hawrylko@intel.com>

diff -r 7fb546da47c0 -r 94ff59f2d22c grub-core/loader/multiboot.c
--- a/grub-core/loader/multiboot.c	Thu Jul 15 17:35:28 2021 +0200
+++ b/grub-core/loader/multiboot.c	Mon Aug 16 12:02:52 2021 +0200
@@ -363,12 +363,15 @@
 		 int argc, char *argv[])
 {
   grub_file_t file = 0;
-  grub_ssize_t size;
+  grub_size_t size;
   void *module = NULL;
   grub_addr_t target;
   grub_err_t err;
   int nounzip = 0;
   grub_uint64_t lowest_addr = 0;
+  grub_uint64_t max_addr;
+  grub_size_t align = MULTIBOOT_MOD_ALIGN;
+  int preference = GRUB_RELOCATOR_PREFERENCE_NONE;
 
   if (argc == 0)
     return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("filename expected"));
@@ -401,11 +404,33 @@
   size = grub_file_size (file);
   if (size)
   {
+    max_addr = UP_TO_TOP32 (size);
+
+#ifdef GRUB_USE_MULTIBOOT2
+    if (grub_multiboot2_mlp.relocatable)
+      {
+        lowest_addr = grub_multiboot2_mlp.min_addr;
+        max_addr = grub_multiboot2_mlp.max_addr;
+        preference = grub_multiboot2_mlp.preference;
+
+        if (grub_multiboot2_mlp.align)
+          {
+            align = grub_max(grub_multiboot2_mlp.align, align);
+          }
+
+        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, UP_TO_TOP32 (size),
-					    size, MULTIBOOT_MOD_ALIGN,
-					    GRUB_RELOCATOR_PREFERENCE_NONE, 1);
+					    lowest_addr, max_addr,
+					    size, align,
+					    preference, 1);
     if (err)
       {
 	grub_file_close (file);
@@ -427,7 +452,7 @@
       return err;
     }
 
-  if (size && grub_file_read (file, module, size) != size)
+  if (size && grub_file_read (file, module, size) != (grub_ssize_t)size)
     {
       grub_file_close (file);
       if (!grub_errno)
diff -r 7fb546da47c0 -r 94ff59f2d22c grub-core/loader/multiboot_mbi2.c
--- a/grub-core/loader/multiboot_mbi2.c	Thu Jul 15 17:35:28 2021 +0200
+++ b/grub-core/loader/multiboot_mbi2.c	Mon Aug 16 12:02:52 2021 +0200
@@ -75,6 +75,7 @@
 static void *elf_sections;
 static int keep_bs = 0;
 static grub_uint32_t load_base_addr;
+struct module_load_preferences grub_multiboot2_mlp;
 
 void
 grub_multiboot2_add_elfsyms (grub_size_t num, grub_size_t entsize,
@@ -114,6 +115,7 @@
   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_load_preferences *mod_load_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 @@
 
   mld.mbi_ver = 2;
   mld.relocatable = 0;
+  grub_multiboot2_mlp.relocatable = 0;
 
   mld.buffer = grub_malloc (MULTIBOOT_SEARCH);
   if (!mld.buffer)
@@ -248,6 +251,27 @@
 	  }
 	break;
 
+      case MULTIBOOT_HEADER_TAG_MODULE_LOAD_PREFERENCES:
+	grub_multiboot2_mlp.relocatable = 1;
+	mod_load_tag = (struct multiboot_header_tag_module_load_preferences *) tag;
+	grub_multiboot2_mlp.min_addr = mod_load_tag->min_addr;
+	grub_multiboot2_mlp.max_addr = mod_load_tag->max_addr;
+	grub_multiboot2_mlp.align = mod_load_tag->align;
+	switch (mod_load_tag->preference)
+	  {
+	  case MULTIBOOT_LOAD_PREFERENCE_LOW:
+	    grub_multiboot2_mlp.preference = GRUB_RELOCATOR_PREFERENCE_LOW;
+	    break;
+
+	  case MULTIBOOT_LOAD_PREFERENCE_HIGH:
+	    grub_multiboot2_mlp.preference = GRUB_RELOCATOR_PREFERENCE_HIGH;
+	    break;
+
+	  default:
+	    grub_multiboot2_mlp.preference = GRUB_RELOCATOR_PREFERENCE_NONE;
+	  }
+	break;
+
 	/* GRUB always page-aligns modules.  */
       case MULTIBOOT_HEADER_TAG_MODULE_ALIGN:
 	break;
diff -r 7fb546da47c0 -r 94ff59f2d22c include/grub/multiboot2.h
--- a/include/grub/multiboot2.h	Thu Jul 15 17:35:28 2021 +0200
+++ b/include/grub/multiboot2.h	Mon Aug 16 12:02:52 2021 +0200
@@ -92,6 +92,15 @@
 };
 typedef struct mbi_load_data mbi_load_data_t;
 
+struct module_load_preferences
+{
+  int relocatable;
+  grub_uint32_t min_addr;
+  grub_uint32_t max_addr;
+  grub_uint32_t align;
+  grub_uint32_t preference;
+};
+
 /* Load ELF32 or ELF64.  */
 grub_err_t
 grub_multiboot2_load_elf (mbi_load_data_t *mld);
@@ -99,6 +108,7 @@
 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_load_preferences grub_multiboot2_mlp;
 
 
 #endif /* ! GRUB_MULTIBOOT_HEADER */
diff -r 7fb546da47c0 -r 94ff59f2d22c include/multiboot2.h
--- a/include/multiboot2.h	Thu Jul 15 17:35:28 2021 +0200
+++ b/include/multiboot2.h	Mon Aug 16 12:02:52 2021 +0200
@@ -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_LOAD_PREFERENCES  11
 
 #define MULTIBOOT2_ARCHITECTURE_I386  0
 #define MULTIBOOT2_ARCHITECTURE_MIPS32  4
@@ -178,6 +179,17 @@
   multiboot_uint32_t preference;
 };
 
+struct multiboot_header_tag_module_load_preferences
+{
+  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 align;
+  multiboot_uint32_t preference;
+};
+
 struct multiboot_color
 {
   multiboot_uint8_t red;

Re: [PATCH v3] multiboot2: Implement the new module load and preferences tag

[Daniel Kiper]

On Mon, Aug 16, 2021 at 04:34:31PM +0200, Lukasz Hawrylko wrote:
> 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>
> Signed-off-by: Lukasz Hawrylko <lukasz.hawrylko@intel.com>
>
> diff -r 7fb546da47c0 -r 94ff59f2d22c grub-core/loader/multiboot.c
> --- a/grub-core/loader/multiboot.c	Thu Jul 15 17:35:28 2021 +0200
> +++ b/grub-core/loader/multiboot.c	Mon Aug 16 12:02:52 2021 +0200
> @@ -363,12 +363,15 @@
>  		 int argc, char *argv[])
>  {
>    grub_file_t file = 0;
> -  grub_ssize_t size;
> +  grub_size_t size;

Why do you change this? If you need that change it requires an
explanation and probably begs for separate patch.

>    void *module = NULL;
>    grub_addr_t target;
>    grub_err_t err;
>    int nounzip = 0;
>    grub_uint64_t lowest_addr = 0;
> +  grub_uint64_t max_addr;

grub_uint64_t lowest_addr = 0, max_addr;

> +  grub_size_t align = MULTIBOOT_MOD_ALIGN;

s/grub_size_t/grub_uint32_t/?

> +  int preference = GRUB_RELOCATOR_PREFERENCE_NONE;
>
>    if (argc == 0)
>      return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("filename expected"));
> @@ -401,11 +404,33 @@
>    size = grub_file_size (file);
>    if (size)
>    {
> +    max_addr = UP_TO_TOP32 (size);
> +
> +#ifdef GRUB_USE_MULTIBOOT2
> +    if (grub_multiboot2_mlp.relocatable)
> +      {
> +        lowest_addr = grub_multiboot2_mlp.min_addr;
> +        max_addr = grub_multiboot2_mlp.max_addr;
> +        preference = grub_multiboot2_mlp.preference;
> +
> +        if (grub_multiboot2_mlp.align)
> +          {
> +            align = grub_max(grub_multiboot2_mlp.align, align);

Missing space between grub_max and "(".

> +          }

I would drop these curly brackets.

> +        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, UP_TO_TOP32 (size),
> -					    size, MULTIBOOT_MOD_ALIGN,
> -					    GRUB_RELOCATOR_PREFERENCE_NONE, 1);
> +					    lowest_addr, max_addr,
> +					    size, align,
> +					    preference, 1);
>      if (err)
>        {
>  	grub_file_close (file);
> @@ -427,7 +452,7 @@
>        return err;
>      }
>
> -  if (size && grub_file_read (file, module, size) != size)
> +  if (size && grub_file_read (file, module, size) != (grub_ssize_t)size)

???

>      {
>        grub_file_close (file);
>        if (!grub_errno)
> diff -r 7fb546da47c0 -r 94ff59f2d22c grub-core/loader/multiboot_mbi2.c
> --- a/grub-core/loader/multiboot_mbi2.c	Thu Jul 15 17:35:28 2021 +0200
> +++ b/grub-core/loader/multiboot_mbi2.c	Mon Aug 16 12:02:52 2021 +0200
> @@ -75,6 +75,7 @@
>  static void *elf_sections;
>  static int keep_bs = 0;
>  static grub_uint32_t load_base_addr;
> +struct module_load_preferences grub_multiboot2_mlp;
>
>  void
>  grub_multiboot2_add_elfsyms (grub_size_t num, grub_size_t entsize,
> @@ -114,6 +115,7 @@
>    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_load_preferences *mod_load_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 @@
>
>    mld.mbi_ver = 2;
>    mld.relocatable = 0;
> +  grub_multiboot2_mlp.relocatable = 0;

I would prefer if you use mld for this. Of course you have to rename
members then, e.g. mld.mod_relocatable.

Daniel

Re: [PATCH v3] multiboot2: Implement the new module load and preferences tag

[Łukasz Hawryłko]

Hi Daniel

On Thu, 2021-09-09 at 18:40 +0200, Daniel Kiper wrote:
> > --- a/grub-core/loader/multiboot.c      Thu Jul 15 17:35:28 2021
> > +0200
> > +++ b/grub-core/loader/multiboot.c      Mon Aug 16 12:02:52 2021
> > +0200
> > @@ -363,12 +363,15 @@
> >                  int argc, char *argv[])
> >  {
> >    grub_file_t file = 0;
> > -  grub_ssize_t size;
> > +  grub_size_t size;
> 
> Why do you change this? If you need that change it requires an
> explanation and probably begs for separate patch.

This variable is initialised later by grub_file_size() that returns
uint64_t. Functions, where the variable is passed, also expect unsigned
value. I don't see any reason to keep it as signed and casts to
unsigned when it is used.

If you prefer to have this change is separate patch, I will do it.

> >  grub-core/loader/multiboot_mbi2.c
> > --- a/grub-core/loader/multiboot_mbi2.c Thu Jul 15 17:35:28 2021
> > +0200
> > +++ b/grub-core/loader/multiboot_mbi2.c Mon Aug 16 12:02:52 2021
> > +0200
> > @@ -75,6 +75,7 @@
> >  static void *elf_sections;
> >  static int keep_bs = 0;
> >  static grub_uint32_t load_base_addr;
> > +struct module_load_preferences grub_multiboot2_mlp;
> > 
> >  void
> >  grub_multiboot2_add_elfsyms (grub_size_t num, grub_size_t entsize,
> > @@ -114,6 +115,7 @@
> >    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_load_preferences
> > *mod_load_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 @@
> > 
> >    mld.mbi_ver = 2;
> >    mld.relocatable = 0;
> > +  grub_multiboot2_mlp.relocatable = 0;
> 
> I would prefer if you use mld for this. Of course you have to rename
> members then, e.g. mld.mod_relocatable.

MLD is not a good place for this as it is not exposed to
grub_cmd_module(). It is allocated on stack inside
grub_multiboot2_load() and used only when processing multiboot2
command. I need a place where values obtained by parsing multiboot2
kernel header will be available during multiboot2 modules load.

Lukasz

[PATCH V3] multiboot2: Implement the new module load and preferences tag

[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      | 35 ++++++++++++++++++++++++++-----
 grub-core/loader/multiboot_mbi2.c | 24 +++++++++++++++++++++
 include/grub/multiboot2.h         | 10 +++++++++
 include/multiboot2.h              | 12 +++++++++++
 4 files changed, 76 insertions(+), 5 deletions(-)

diff --git a/grub-core/loader/multiboot.c b/grub-core/loader/multiboot.c
index 4a98d7082598..71a306498943 100644
--- a/grub-core/loader/multiboot.c
+++ b/grub-core/loader/multiboot.c
@@ -363,12 +363,15 @@ grub_cmd_module (grub_command_t cmd __attribute__ ((unused)),
 		 int argc, char *argv[])
 {
   grub_file_t file = 0;
-  grub_ssize_t size;
+  grub_size_t size;
   void *module = NULL;
   grub_addr_t target;
   grub_err_t err;
   int nounzip = 0;
   grub_uint64_t lowest_addr = 0;
+  grub_uint64_t max_addr;
+  grub_size_t align = MULTIBOOT_MOD_ALIGN;
+  int preference = GRUB_RELOCATOR_PREFERENCE_NONE;
 
   if (argc == 0)
     return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("filename expected"));
@@ -401,11 +404,33 @@ 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_mlp.relocatable)
+      {
+        lowest_addr = grub_multiboot2_mlp.min_addr;
+        max_addr = grub_multiboot2_mlp.max_addr;
+        preference = grub_multiboot2_mlp.preference;
+
+        if (grub_multiboot2_mlp.align)
+          {
+            align = grub_max(grub_multiboot2_mlp.align, align);
+          }
+
+        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,
-					    size, MULTIBOOT_MOD_ALIGN,
-					    GRUB_RELOCATOR_PREFERENCE_NONE, 1);
+					    lowest_addr, max_addr,
+					    size, align,
+					    preference, 1);
     if (err)
       {
 	grub_file_close (file);
@@ -427,7 +452,7 @@ grub_cmd_module (grub_command_t cmd __attribute__ ((unused)),
       return err;
     }
 
-  if (size && grub_file_read (file, module, size) != size)
+  if (size && grub_file_read (file, module, size) != (grub_ssize_t)size)
     {
       grub_file_close (file);
       if (!grub_errno)
diff --git a/grub-core/loader/multiboot_mbi2.c b/grub-core/loader/multiboot_mbi2.c
index 18e766c7b31c..f46c1233f5e0 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_load_preferences grub_multiboot2_mlp;
 
 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_load_preferences *mod_load_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_mlp.relocatable = 0;
 
   mld.buffer = grub_malloc (MULTIBOOT_SEARCH);
   if (!mld.buffer)
@@ -248,6 +251,27 @@ grub_multiboot2_load (grub_file_t file, const char *filename)
 	  }
 	break;
 
+      case MULTIBOOT_HEADER_TAG_MODULE_LOAD_PREFERENCES:
+	grub_multiboot2_mlp.relocatable = 1;
+	mod_load_tag = (struct multiboot_header_tag_module_load_preferences *) tag;
+	grub_multiboot2_mlp.min_addr = mod_load_tag->min_addr;
+	grub_multiboot2_mlp.max_addr = mod_load_tag->max_addr;
+	grub_multiboot2_mlp.align = mod_load_tag->align;
+	switch (mod_load_tag->preference)
+	  {
+	  case MULTIBOOT_LOAD_PREFERENCE_LOW:
+	    grub_multiboot2_mlp.preference = GRUB_RELOCATOR_PREFERENCE_LOW;
+	    break;
+
+	  case MULTIBOOT_LOAD_PREFERENCE_HIGH:
+	    grub_multiboot2_mlp.preference = GRUB_RELOCATOR_PREFERENCE_HIGH;
+	    break;
+
+	  default:
+	    grub_multiboot2_mlp.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..cddf64228c28 100644
--- a/include/grub/multiboot2.h
+++ b/include/grub/multiboot2.h
@@ -92,6 +92,15 @@ struct mbi_load_data
 };
 typedef struct mbi_load_data mbi_load_data_t;
 
+struct module_load_preferences
+{
+  int relocatable;
+  grub_uint32_t min_addr;
+  grub_uint32_t max_addr;
+  grub_uint32_t align;
+  grub_uint32_t preference;
+};
+
 /* Load ELF32 or ELF64.  */
 grub_err_t
 grub_multiboot2_load_elf (mbi_load_data_t *mld);
@@ -99,6 +108,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_load_preferences grub_multiboot2_mlp;
 
 
 #endif /* ! GRUB_MULTIBOOT_HEADER */
diff --git a/include/multiboot2.h b/include/multiboot2.h
index 5693923c014f..0f491a0451b9 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_LOAD_PREFERENCES  11
 
 #define MULTIBOOT2_ARCHITECTURE_I386  0
 #define MULTIBOOT2_ARCHITECTURE_MIPS32  4
@@ -178,6 +179,17 @@ struct multiboot_header_tag_relocatable
   multiboot_uint32_t preference;
 };
 
+struct multiboot_header_tag_module_load_preferences
+{
+  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 align;
+  multiboot_uint32_t preference;
+};
+
 struct multiboot_color
 {
   multiboot_uint8_t red;
-- 
2.17.1