On 06/15/2010 01:21 PM, Colin Watson wrote:
> On Mon, Jun 14, 2010 at 07:12:38PM +0200, Grégoire Sutre wrote:
>
>> On 06/14/2010 18:43, Colin Watson wrote:
>>
>>> Do you have any suggestions on how to deal with that? I'm not familiar
>>> with multiboot and need guidance.
>>>
>> A possible solution would be to use the multiboot-command line. AFAIK,
>> the boot_device field of the multiboot information structure is supposed
>> to pass this kind of partition information, but you cannot specify the
>> partmaps in this field, hence its interpretation is ambiguous.
>>
> That would potentially allow a user to override things, but doesn't help
> with users who don't change their configuration. Unless the user
> explicitly configures things, boot_device is all we've got.
>
>
Apparently multiboot part has revealed to be a bit more tricky due to
heterogenity. Go ahead with non-multiboot part, I'll review multiboot
part separately (need to think about it a bit)
> Thus, I guess we end up with a two-part fix:
>
> 1) Honour key=value pairs in the multiboot command line when booting
> GRUB itself as a multiboot image. These would simply become
> environment variables. Presumably this goes in grub_machine_init,
> by analogy with kern/ieee1275/init.c. This allows users to
> override the prefix on the command line as if they had changed the
> image itself.
>
> 2) If multiboot_trampoline needs to change install_dos_part or
> install_bsd_part based on the value of boot_device in the MBI, then
> we know that the drive/partition part of prefix (which was
> calculated in the same way as install_dos_part and install_bsd_part
> when grub-setup was run) is now invalid and should be ignored.
> This fact needs to be passed on to make_install_device.
>
> Something like this? I'm afraid I have no idea how to test this (GRUB's
> multiboot command doesn't seem to accept command-line arguments?), not
> to mention that this is the first time I've been anywhere near most of
> this code. I would greatly appreciate advice and review.
>
> 2010-06-15 Colin Watson <cjwat...@ubuntu.com>
>
> Fix i386-pc prefix handling with nested partitions (Debian bug
> #585068).
>
> * include/grub/i386/pc/kernel.h (grub_multiboot_change_prefix): New
> declaration.
> * kern/i386/pc/startup.S (multiboot_entry): Save a pointer to the
> MBI in startup_multiboot_info.
> (multiboot_trampoline): If the new partition numbers differ from the
> previous ones, then set grub_multiboot_change_prefix.
> (grub_multiboot_change_prefix): New definition.
> * kern/i386/pc/init.c (make_install_device): Invalidate any
> drive/partition part of the prefix if grub_multiboot_change_prefix
> is set. After that, if the prefix starts with "(,", fill the boot
> drive in between those two characters, but expect that a full
> partition specification including partition map names will follow.
> (grub_parse_multiboot_cmdline): New function.
> (grub_machine_init): If we have an MBI, copy it, then call
> grub_parse_multiboot_cmdline.
> * util/i386/pc/grub-setup.c (setup): Unless an explicit prefix was
> specified, write a prefix without the drive name but including a
> full partition specification.
>
> === modified file 'include/grub/i386/pc/kernel.h'
> --- include/grub/i386/pc/kernel.h 2010-04-26 19:11:16 +0000
> +++ include/grub/i386/pc/kernel.h 2010-06-15 11:02:34 +0000
> @@ -44,6 +44,10 @@ extern grub_int32_t grub_install_bsd_par
> /* The boot BIOS drive number. */
> extern grub_uint8_t EXPORT_VAR(grub_boot_drive);
>
> +/* Set if multiboot changed the partition numbers, so grub_prefix is now
> + invalid. */
> +extern grub_uint8_t grub_multiboot_change_prefix;
> +
> #endif /* ! ASM_FILE */
>
> #endif /* ! KERNEL_MACHINE_HEADER */
>
> === modified file 'kern/i386/pc/init.c'
> --- kern/i386/pc/init.c 2010-05-21 18:08:48 +0000
> +++ kern/i386/pc/init.c 2010-06-15 11:06:20 +0000
> @@ -32,6 +32,7 @@
> #include <grub/cache.h>
> #include <grub/time.h>
> #include <grub/cpu/tsc.h>
> +#include <grub/multiboot.h>
>
> struct mem_region
> {
> @@ -47,12 +48,29 @@ static int num_regions;
> grub_addr_t grub_os_area_addr;
> grub_size_t grub_os_area_size;
>
> +/* A pointer to the MBI in its initial location. */
> +struct multiboot_info *startup_multiboot_info = NULL;
> +
> +/* The MBI has to be copied to our BSS so that it won't be
> + overwritten. This is its final location. */
> +static struct multiboot_info kern_multiboot_info;
> +
> static char *
> make_install_device (void)
> {
> /* XXX: This should be enough. */
> char dev[100], *ptr = dev;
>
> + if (grub_prefix[0] == '(' && grub_multiboot_change_prefix)
> + {
> + /* The multiboot information invalidated our hardcoded prefix because
> + partition numbers differed. Eliminate the drive/partition part of
> + the prefix, if possible. */
> + char *ket = grub_strchr (grub_prefix, ')');
> + if (ket)
> + grub_memmove (grub_prefix, ket + 1, grub_strlen (ket));
> + }
> +
> if (grub_prefix[0] != '(')
> {
> /* No hardcoded root partition - make it from the boot drive and the
> @@ -83,6 +101,14 @@ make_install_device (void)
> grub_snprintf (ptr, sizeof (dev) - (ptr - dev), ")%s", grub_prefix);
> grub_strcpy (grub_prefix, dev);
> }
> + else if (grub_prefix[1] == ',' || grub_prefix[1] == ')')
> + {
> + /* We have a prefix, but still need to fill in the boot drive. */
> + grub_snprintf (dev, sizeof (dev),
> + "(%cd%u%s", (grub_boot_drive & 0x80) ? 'h' : 'f',
> + grub_boot_drive & 0x7f, grub_prefix + 1);
> + grub_strcpy (grub_prefix, dev);
> + }
>
> return grub_prefix;
> }
> @@ -134,6 +160,45 @@ compact_mem_regions (void)
> }
> }
>
> +static void
> +grub_parse_multiboot_cmdline (void)
> +{
> + char *cmdline;
> + char *p;
> +
> + if (! (kern_multiboot_info.flags & MULTIBOOT_INFO_CMDLINE))
> + return;
> +
> + p = cmdline = grub_strdup ((char *) kern_multiboot_info.cmdline);
> +
> + while (*p)
> + {
> + char *command = p;
> + char *end;
> + char *val;
> +
> + end = grub_strchr (command, ' ');
> + if (end)
> + {
> + *end = '\0';
> + p = end + 1;
> + while (*p == ' ')
> + ++p;
> + }
> +
> + /* Process command. */
> + val = grub_strchr (command, '=');
> + if (val)
> + {
> + *val = '\0';
> + grub_env_set (command, val + 1);
> +
> + if (grub_strcmp (command, "prefix") == 0)
> + grub_multiboot_change_prefix = 0;
> + }
> + }
> +}
> +
> void
> grub_machine_init (void)
> {
> @@ -214,6 +279,15 @@ grub_machine_init (void)
> if (! grub_os_area_addr)
> grub_fatal ("no upper memory");
>
> + if (startup_multiboot_info)
> + {
> + /* Move MBI to a safe place. */
> + grub_memmove (&kern_multiboot_info, startup_multiboot_info,
> + sizeof (struct multiboot_info));
> +
> + grub_parse_multiboot_cmdline ();
> + }
> +
> grub_tsc_init ();
> }
>
>
> === 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-15 11:02:19 +0000
> @@ -142,6 +142,8 @@ multiboot_header:
>
> multiboot_entry:
> .code32
> + movl %ebx, EXT_C(startup_multiboot_info)
> +
> /* obtain the boot device */
> movl 12(%ebx), %edx
>
> @@ -161,20 +163,38 @@ multiboot_entry:
> jmp *%eax
>
> multiboot_trampoline:
> + /* remember the original boot information */
> + movl EXT_C(grub_install_dos_part), %eax
> + orl %eax, %eax
> + jns 1f
> + movb $0xFF, %al
> +1:
> + movl EXT_C(grub_install_bsd_part), %ebx
> + orl %ebx, %ebx
> + jns 2f
> + movb $0xFF, %bl
> +2:
> + movb %al, %bh
> +
> /* fill the boot information */
> movl %edx, %eax
> shrl $8, %eax
> + cmpw %ax, %bx
> + je 3f
> + /* doesn't match the original */
> + movb $1, EXT_C(grub_multiboot_change_prefix)
> +3:
> xorl %ebx, %ebx
> cmpb $0xFF, %ah
> - je 1f
> + je 4f
> movb %ah, %bl
> movl %ebx, EXT_C(grub_install_dos_part)
> -1:
> +4:
> cmpb $0xFF, %al
> - je 2f
> + je 5f
> movb %al, %bl
> movl %ebx, EXT_C(grub_install_bsd_part)
> -2:
> +5:
> shrl $24, %edx
> movb $0xFF, %dh
> /* enter the usual booting */
> @@ -285,6 +305,8 @@ LOCAL (codestart):
>
> VARIABLE(grub_boot_drive)
> .byte 0
> +VARIABLE(grub_multiboot_change_prefix)
> + .byte 0
>
> .p2align 2 /* force 4-byte alignment */
>
>
> === modified file 'util/i386/pc/grub-setup.c'
> --- util/i386/pc/grub-setup.c 2010-06-11 20:31:16 +0000
> +++ util/i386/pc/grub-setup.c 2010-06-14 16:29:54 +0000
> @@ -99,6 +99,7 @@ setup (const char *dir,
> struct grub_boot_blocklist *first_block, *block;
> grub_int32_t *install_dos_part, *install_bsd_part;
> grub_int32_t dos_part, bsd_part;
> + char *prefix;
> char *tmp_img;
> int i;
> grub_disk_addr_t first_sector;
> @@ -230,6 +231,8 @@ setup (const char *dir,
> + GRUB_KERNEL_MACHINE_INSTALL_DOS_PART);
> install_bsd_part = (grub_int32_t *) (core_img + GRUB_DISK_SECTOR_SIZE
> + GRUB_KERNEL_MACHINE_INSTALL_BSD_PART);
> + prefix = (char *) (core_img + GRUB_DISK_SECTOR_SIZE +
> + GRUB_KERNEL_MACHINE_PREFIX);
>
> /* Open the root device and the destination device. */
> root_dev = grub_device_open (root);
> @@ -305,6 +308,18 @@ setup (const char *dir,
> dos_part = root_dev->disk->partition->number;
> bsd_part = -1;
> }
> +
> + if (prefix[0] != '(')
> + {
> + char *root_part_name, *new_prefix;
> +
> + root_part_name =
> + grub_partition_get_name (root_dev->disk->partition);
> + new_prefix = xasprintf ("(,%s)%s", root_part_name, prefix);
> + strcpy (prefix, new_prefix);
> + free (new_prefix);
> + free (root_part_name);
> + }
> }
> else
> dos_part = bsd_part = -1;
>
> Thanks,
>
> -- Regards Vladimir 'φ-coder/phcoder' Serbinenko
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
