Re: Which partitioning schemes should be supported by GRUB?

[Grégoire Sutre]

Hi,

I made several tests, and the patch works fine with standard boot.  When
multibooting core.img, the command-line is taken into account correctly.
However, if no multiboot command-line is given, the prefix is set as
before (old partition naming style).

This comes from the fact that the modifications to the prefix done in
grub-setup only apply to the embedded image, and not to the core.img
file.  The grub-mkimage command used by grub-install is:

grub-mkimage -O i386-pc --output=/path/to/core.img --prefix=/boot/grub

Would it make sense to put the full prefix here?


> > - couldn't the complete processing of the MBI be performed in the same
> >    place, i.e. grub_machine_init()?  The assembly multiboot part would
> >    only check whether GRUB was booted by multiboot, and set (or copy)
> >    the MBI in that case.  Then the procedure to set grub_prefix would be
> >    coded in one place.  This would require though, for multiboot, to get
> >    grub_boot_drive from the boot_device field (the current assembly code
> >    takes care of this).
>
> My investigations suggest that this is very difficult.  Relocating the
> GRUB kernel, which is almost the first thing multiboot_entry does, is
> liable to overwrite the MBI, and you can't get at C code until you've
> done that.  That's why multiboot_entry has to copy out the boot device
> field even before it relocates the kernel.

What I meant is: the assembly part could be restricted to the copy of
the relevant MBI information, i.e. flags, boot_device and cmdline.  And
leave the actual decisions regarding the contents of those fields to
grub_machine_init().  This would mean that the part dealing with
grub_install_dos/bsd_part (multiboot_trampoline) would be part of the C
code (probably in make_install_device()).


>   grub_machine_set_prefix (void)
>   {
>     /* Initialize the prefix.  */
> -  grub_env_set ("prefix", make_install_device ());
> +  if (!found_multiboot_prefix)
> +    grub_env_set ("prefix", make_install_device ());

You could avoid the variable found_multiboot_prefix and use
grub_env_find("prefix").  The intention would be that if someone
(multiboot or someone else) has already set the prefix in grub_env,
then we shouldn't override this choice here.  This would simplify
a bit grub_parse_multiboot_cmdline(), which would become purely
generic.  But this is essentially cosmetic...


 === modified file 'kern/i386/pc/startup.S'
> --- kern/i386/pc/startup.S      2010-04-05 13:59:32 +0000
> +++ kern/i386/pc/startup.S      2010-06-16 12:55:05 +0000
> @@ -145,6 +145,32 @@ multiboot_entry:
>         /* obtain the boot device */
>         movl    12(%ebx), %edx

The boot_device field should be used only if the MULTIBOOT_INFO_BOOTDEV
flag is set.  This problem is already present in trunk.

> +       /* if so, copy it to a safe place; do this before relocating code to
> +          make sure that we don't lose it */
> +       movl    16(%ebx), %edi
> +       pushl   %edi

Is it safe to push?  Maybe we should start by setting %esp as is done a
few lines below.

I honestly do not understand all the details regarding the assembly code
or the memory management, so I can't provide useful feedback on that.
I just noticed that GRUB_MEMORY_MACHINE_MULTIBOOT_FLAGS is copied into a
``regular'' variable, but GRUB_MEMORY_MACHINE_MULTIBOOT_CMDLINE is not.

Grégoire