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.c...@intel.com>
> Signed-off-by: Lukasz Hawrylko <lukasz.hawry...@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
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
