Re: [PATCH] multiboot on EFI

[Robert Millan]

On Sun, Aug 02, 2009 at 09:53:32PM +0200, Vladimir 'phcoder' Serbinenko wrote:
> Was successfully tested with qemu-tianocore with example multiboot
> kernel from multiboot specification. Since real EFI has no VGA video
> fields play a crucial feature and need to be supported. Patch is
> coming but it will surely rely on gfxsplit patch. I also have plans to
> extend it to x86-64 too

Very nice.

>  static grub_err_t
>  grub_multiboot_boot (void)
>  {
> +#ifdef GRUB_MACHINE_EFI
> +  if (! grub_efi_finish_boot_services ())
> +     grub_fatal ("cannot exit boot services");
> +#endif

grub_fatal() terminates GRUB.  I suggest you just error out instead, so that
user can try other things.

> -      mmap_entry->type = type;
> +      switch (type)
> +        {
> +        case GRUB_MACHINE_MEMORY_AVAILABLE:
> +       mmap_entry->type = GRUB_MULTIBOOT_MEMORY_AVAILABLE;
> +       break;
> +
> +     default:
> +       mmap_entry->type = GRUB_MULTIBOOT_MEMORY_RESERVED;
> +       break;

Is this going to be extended with more `case' stanzas later?  If not,
it'd be more readable with an if/else, or ?:.

> -  grub_loader_set (grub_multiboot_boot, grub_multiboot_unload, 1);
> +  grub_loader_set (grub_multiboot_boot, grub_multiboot_unload, 0);

This breaks text-mode payloads when they're loaded from gfxterm.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."