On Mon, Mar 23, 2009 at 01:29:55PM +0100, phcoder wrote:
> Hello. Here is an initial version of patch for booting multiboot kernels
> on i386-efi. No Changelog yet because it's not for inclusion yet.
Very nice!
Would it be hard to split the patch and make it more granular? I see it
implements base mmap / lsmmap support on efi, then ports the *BSD loaders
and the Multiboot loader too, and the uppermem facility.
> pkglib_MODULES = kernel.mod normal.mod _chain.mod chain.mod appleldr.mod \
> _linux.mod linux.mod halt.mod reboot.mod pci.mod lspci.mod \
> - datetime.mod date.mod datehook.mod
> + datetime.mod date.mod datehook.mod _multiboot.mod multiboot.mod
> +
> +# For _multiboot.mod.
> +_multiboot_mod_SOURCES = loader/i386/multiboot.c \
> + loader/i386/multiboot_helper.S \
> + loader/multiboot_loader.c
> +_multiboot_mod_CFLAGS = $(COMMON_CFLAGS)
> +_multiboot_mod_LDFLAGS = $(COMMON_LDFLAGS)
> +_multiboot_mod_ASFLAGS = $(COMMON_ASFLAGS)
If everybody's fine with it, I'd like to suggest adding stuff to pkglib_MODULES
in the same place as its corresponding variables. I've done this already a few
times, and I think it makes the build system a bit more maintainable. What do
you all think about this?
> diff --git a/include/grub/i386/pc/memory.h b/include/grub/i386/pc/memory.h
> index 08e92a9..e69ff77 100644
> --- a/include/grub/i386/pc/memory.h
> +++ b/include/grub/i386/pc/memory.h
> @@ -92,6 +92,8 @@ struct grub_machine_mmap_entry
> grub_uint64_t len;
> #define GRUB_MACHINE_MEMORY_AVAILABLE 1
> #define GRUB_MACHINE_MEMORY_RESERVED 2
> +#define GRUB_MACHINE_MEMORY_ACPI 3
> +#define GRUB_MACHINE_MEMORY_NVS 4
> grub_uint32_t type;
> } __attribute__((packed));
Do we need specific knowledge of these two on i386-pc ?
> /* The minimum and maximum heap size for GRUB itself. */
> #define MIN_HEAP_SIZE 0x100000
> -#define MAX_HEAP_SIZE (16 * 0x100000)
> +#define MAX_HEAP_SIZE (1600 * 0x100000)
Is 1600 MB what we want, or to remove the limit?
> + /* Bubble-sort the memory map */
> + while (done)
> + {
> + done = 0;
> + for (i = 0; i < count - 1; i++)
> + if (regions[i].start > regions[i + 1].start)
> + {
> + done = 1;
> + t = regions[i];
> + regions[i] = regions[i + 1];
> + regions[i + 1] = t;
> + }
> + }
Do we need the memory map to be sorted? AFAIK loadees can cope with unsorted
maps fine; is there an exception?
> +#ifdef GRUB_MACHINE_PCBIOS
> + grub_stop_floppy ();
> +#endif
grub_stop_floppy() doesn't do any BIOS-specific stuff. Wouldn't __i386__
be more appropiate?
--
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."
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
