Re: [PATCHv2] multiboot2: enable quirk-modules-after-kernel

grub-devel

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCHv2] multiboot2: enable quirk-modules-after-kernel

From:	Paul Menzel
Subject:	Re: [PATCHv2] multiboot2: enable quirk-modules-after-kernel
Date:	Mon, 6 Apr 2020 22:56:36 +0200
User-agent:	Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.6.0

Dear Zide,

Thank you for posting version 2 of your patch. (Please add v2 to thepatch tag next time.)


Maybe use the commit message summary below.

multiboot2: Implement quirk-modules-after-kernel


Am 03.04.20 um 09:35 schrieb Zide Chen:

In multiboot2, currently there is no way to control where to load the

Maybe start with: In contrast to Mulitboot (v1), in Mulitboot 2, thereis currently …

modules. In case of user wants to reserve low address for particular
usage, this quirk is useful to load the modules to higher address.

This patch makes the quirk to multiboot2 by:

So, implement the quirk quirk-modules-after-kernel to load modules afterthe kernel for Multiboot 2 by reusing the implementation for Multiboot (v1).

- remove "ifndef GRUB_USE_MULTIBOOT2" that is related to this logic.
- define separate variables for both multiboot and multiboot2, e.g.
   grub_multiboot_quirks, highest_load, etc.
- in grub_multiboot2_load(): calculate the highest load address of raw
   image and assign it to grub_multiboot2_highest_load.
- the existing code in multiboot_elfxx.c is able to calculate the
   highest_load for mutiboot2 already.

At the end, the adjusted GRUB_MULTIBOOT(highest_load) is used as the
lowest address to allocate memory for loading modules.


Tested with VMware. ;-)

Signed-off-by: Zide Chen <address@hidden>
---
  grub-core/loader/i386/multiboot_mbi.c |  1 -
  grub-core/loader/multiboot.c          | 23 ++++++++++++-----------
  grub-core/loader/multiboot_elfxx.c    |  7 ++++---
  grub-core/loader/multiboot_mbi2.c     |  2 ++
  include/grub/multiboot.h              |  1 +
  include/grub/multiboot2.h             |  8 ++++++++
  6 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/grub-core/loader/i386/multiboot_mbi.c 
b/grub-core/loader/i386/multiboot_mbi.c
index ad3cc292fd18..18aaac847011 100644
--- a/grub-core/loader/i386/multiboot_mbi.c
+++ b/grub-core/loader/i386/multiboot_mbi.c
@@ -63,7 +63,6 @@ static int bootdev_set;
  static grub_size_t elf_sec_num, elf_sec_entsize;
  static unsigned elf_sec_shstrndx;
  static void *elf_sections;
-grub_multiboot_quirks_t grub_multiboot_quirks;

static grub_err_t

  load_kernel (grub_file_t file, const char *filename,
diff --git a/grub-core/loader/multiboot.c b/grub-core/loader/multiboot.c
index 4a98d7082598..f9f66378dfb2 100644
--- a/grub-core/loader/multiboot.c
+++ b/grub-core/loader/multiboot.c
@@ -32,6 +32,8 @@
  #include <grub/multiboot2.h>
  #define GRUB_MULTIBOOT_CONSOLE_FRAMEBUFFER GRUB_MULTIBOOT2_CONSOLE_FRAMEBUFFER
  #define GRUB_MULTIBOOT_CONSOLE_EGA_TEXT GRUB_MULTIBOOT2_CONSOLE_EGA_TEXT
+#define GRUB_MULTIBOOT_QUIRKS_NONE GRUB_MULTIBOOT2_QUIRKS_NONE
+#define GRUB_MULTIBOOT_QUIRK_MODULES_AFTER_KERNEL 
GRUB_MULTIBOOT2_QUIRK_MODULES_AFTER_KERNEL
  #define GRUB_MULTIBOOT(x) grub_multiboot2_ ## x
  #else
  #include <grub/multiboot.h>
@@ -210,7 +212,8 @@ grub_multiboot_unload (void)
    return GRUB_ERR_NONE;
  }

-static grub_uint64_t highest_load;

+GRUB_MULTIBOOT (quirks_t) GRUB_MULTIBOOT (quirks);
+grub_uint64_t GRUB_MULTIBOOT (highest_load);

#define MULTIBOOT_LOAD_ELF64

  #include "multiboot_elfxx.c"
@@ -291,32 +294,32 @@ grub_cmd_multiboot (grub_command_t cmd __attribute__ 
((unused)),

grub_loader_unset ();- highest_load = 0;

+  GRUB_MULTIBOOT (highest_load) = 0;

-#ifndef GRUB_USE_MULTIBOOT2

-  grub_multiboot_quirks = GRUB_MULTIBOOT_QUIRKS_NONE;
+  GRUB_MULTIBOOT (quirks) = GRUB_MULTIBOOT_QUIRKS_NONE;
    int option_found = 0;

do

      {
        option_found = 0;
+#ifndef GRUB_USE_MULTIBOOT2
        if (argc != 0 && grub_strcmp (argv[0], "--quirk-bad-kludge") == 0)
        {
          argc--;
          argv++;
          option_found = 1;
-         grub_multiboot_quirks |= GRUB_MULTIBOOT_QUIRK_BAD_KLUDGE;
+         GRUB_MULTIBOOT (quirks) |= GRUB_MULTIBOOT_QUIRK_BAD_KLUDGE;
        }
+#endif

if (argc != 0 && grub_strcmp (argv[0], "--quirk-modules-after-kernel") == 0)

        {
          argc--;
          argv++;
          option_found = 1;
-         grub_multiboot_quirks |= GRUB_MULTIBOOT_QUIRK_MODULES_AFTER_KERNEL;
+         GRUB_MULTIBOOT (quirks) |= GRUB_MULTIBOOT_QUIRK_MODULES_AFTER_KERNEL;
        }
      } while (option_found);
-#endif

if (argc == 0)

      return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("filename expected"));
@@ -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)
diff --git a/grub-core/loader/multiboot_elfxx.c 
b/grub-core/loader/multiboot_elfxx.c
index 70cd1db513e6..8adce7b3489b 100644
--- a/grub-core/loader/multiboot_elfxx.c
+++ b/grub-core/loader/multiboot_elfxx.c
@@ -89,17 +89,18 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t *mld)
      if (phdr(i)->p_type == PT_LOAD)
        {
        mld->link_base_addr = grub_min (mld->link_base_addr, phdr(i)->p_paddr);
-       highest_load = grub_max (highest_load, phdr(i)->p_paddr + 
phdr(i)->p_memsz);
+       GRUB_MULTIBOOT (highest_load) = grub_max (GRUB_MULTIBOOT (highest_load),
+                       phdr(i)->p_paddr + phdr(i)->p_memsz);
        }

#ifdef MULTIBOOT_LOAD_ELF64

-  if (highest_load >= 0x100000000)
+  if (GRUB_MULTIBOOT(highest_load) >= 0x100000000)
      return grub_error (GRUB_ERR_BAD_OS, "segment crosses 4 GiB border");
  #endif

if (mld->relocatable)

      {
-      load_size = highest_load - mld->link_base_addr;
+      load_size = GRUB_MULTIBOOT(highest_load) - mld->link_base_addr;

grub_dprintf ("multiboot_loader", "align=0x%lx, preference=0x%x, "

                    "load_size=0x%x, avoid_efi_boot_services=%d\n",
diff --git a/grub-core/loader/multiboot_mbi2.c 
b/grub-core/loader/multiboot_mbi2.c
index 18e766c7b31c..3883732df531 100644
--- a/grub-core/loader/multiboot_mbi2.c
+++ b/grub-core/loader/multiboot_mbi2.c
@@ -315,8 +315,10 @@ grub_multiboot2_load (grub_file_t file, const char 
*filename)
          grub_free (mld.buffer);
          return err;
        }
+
        mld.link_base_addr = load_addr;
        mld.load_base_addr = get_physical_target_address (ch);
+      grub_multiboot2_highest_load = mld.load_base_addr + code_size;
        source = get_virtual_current_address (ch);

grub_dprintf ("multiboot_loader", "link_base_addr=0x%x, load_base_addr=0x%x, "

diff --git a/include/grub/multiboot.h b/include/grub/multiboot.h
index bd0a9873e6c1..3d7c5ad29318 100644
--- a/include/grub/multiboot.h
+++ b/include/grub/multiboot.h
@@ -107,6 +107,7 @@ grub_multiboot_load_elf (mbi_load_data_t *mld);
  extern grub_size_t grub_multiboot_pure_size;
  extern grub_size_t grub_multiboot_alloc_mbi;
  extern grub_uint32_t grub_multiboot_payload_eip;
+extern grub_uint64_t grub_multiboot_highest_load;

#endif /* ! GRUB_MULTIBOOT_HEADER */

diff --git a/include/grub/multiboot2.h b/include/grub/multiboot2.h
index 502d34ef1804..fa74508afcda 100644
--- a/include/grub/multiboot2.h
+++ b/include/grub/multiboot2.h
@@ -27,6 +27,13 @@
  #include <grub/types.h>
  #include <grub/err.h>

+typedef enum

+  {
+    GRUB_MULTIBOOT2_QUIRKS_NONE = 0,
+    GRUB_MULTIBOOT2_QUIRK_MODULES_AFTER_KERNEL = 1
+  } grub_multiboot2_quirks_t;
+extern grub_multiboot2_quirks_t grub_multiboot2_quirks;
+
  extern struct grub_relocator *grub_multiboot2_relocator;

void grub_multiboot2 (int argc, char *argv[]);

@@ -99,6 +106,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 grub_uint64_t grub_multiboot2_highest_load;

#endif /* ! GRUB_MULTIBOOT_HEADER */

Does this need to be documented somewhere? (Multiboot 2 does not seem tobe separately documented in `docs/grub.texi`, so maybe not.

It’d be great if you added a separate second patch on top, adding aMultiboot 2 example with this quirk to the documentation.

Please post a v3 patch with the improved commit message, and add theline below to it.


Reviewed-by: Paul Menzel <address@hidden>


Kind regards,

Paul

[Prev in Thread]

Current Thread

[Next in Thread]

Re: [PATCH] multiboot2: enable quirk-modules-after-kernel, Paul Menzel, 2020/04/02
- RE: [PATCH] multiboot2: enable quirk-modules-after-kernel, Chen, Zide, 2020/04/03
- [PATCH] multiboot2: enable quirk-modules-after-kernel, Zide Chen, 2020/04/03
  - Re: [PATCHv2] multiboot2: enable quirk-modules-after-kernel, Paul Menzel <=
    - RE: [PATCHv2] multiboot2: enable quirk-modules-after-kernel, Chen, Zide, 2020/04/06
    - RE: [PATCHv2] multiboot2: enable quirk-modules-after-kernel, Chen, Zide, 2020/04/07

Prev by Date: Re: [PATCH 00/10] Another set of patches from Fedora
Next by Date: RE: [PATCHv2] multiboot2: enable quirk-modules-after-kernel
Previous by thread: [PATCH] multiboot2: enable quirk-modules-after-kernel
Next by thread: RE: [PATCHv2] multiboot2: enable quirk-modules-after-kernel
Index(es):
- Date
- Thread