On Tue, Feb 12, 2008 at 04:46:31AM +0800, Bean wrote:
> +# For _freebsd.mod
> +_bsd_mod_SOURCES = loader/i386/pc/bsd.c
You forgot to rename the commend ;-)
> +_bsd_mod_CFLAGS = $(COMMON_CFLAGS)
> +_bsd_mod_LDFLAGS = $(COMMON_LDFLAGS)
> +
> +# For freebsd.mod
> +bsd_mod_SOURCES = loader/i386/pc/bsd_normal.c
> +bsd_mod_CFLAGS = $(COMMON_CFLAGS)
> +bsd_mod_LDFLAGS = $(COMMON_LDFLAGS)
How much pc-dependant are those? Can we hope they will work in e.g.
i386-coreboot in the future, when *BSD kernels remove their BIOS
dependency?
> +#define FREEBSD_RB_ASKNAME 0x1 /* ask for file name to reboot from */
> +#define FREEBSD_RB_SINGLE 0x2 /* reboot to single user only */
> +#define FREEBSD_RB_NOSYNC 0x4 /* dont sync before reboot */
> +#define FREEBSD_RB_HALT 0x8 /* don't reboot, just halt */
> +#define FREEBSD_RB_INITNAME 0x10 /* name given for /etc/init (unused) */
> +#define FREEBSD_RB_DFLTROOT 0x20 /* use compiled-in rootdev */
> +#define FREEBSD_RB_KDB 0x40 /* give control to kernel debugger */
> +#define FREEBSD_RB_RDONLY 0x80 /* mount root fs read-only */
> +#define FREEBSD_RB_DUMP 0x100 /* dump kernel memory before
> reboot */
> +#define FREEBSD_RB_MINIROOT 0x200 /* mini-root present in memory
> at boot time */
> +#define FREEBSD_RB_CONFIG 0x400 /* invoke user configuration
> routing */
> +#define FREEBSD_RB_VERBOSE 0x800 /* print all potentially useful
> info */
> +#define FREEBSD_RB_SERIAL 0x1000 /* user serial port as console
> */
> +#define FREEBSD_RB_CDROM 0x2000 /* use cdrom as root */
> +#define FREEBSD_RB_GDB 0x8000 /* use GDB remote debugger
> instead of DDB */
> +#define FREEBSD_RB_MUTE 0x10000 /* Come up with the console
> muted */
> +#define FREEBSD_RB_PAUSE 0x100000
> +#define FREEBSD_RB_QUIET 0x200000
> +#define FREEBSD_RB_NOINTR 0x10000000
> +#define FREENSD_RB_MULTIPLE 0x20000000 /* Use multiple consoles */
> +#define FREEBSD_RB_DUAL FREENSD_RB_MULTIPLE
> +#define FREEBSD_RB_BOOTINFO 0x80000000 /* have `struct bootinfo *' arg */
Usually we have "(1 << 0)", "(1 << 1)", etc for bitmask lists.
> +FUNCTION(grub_unix_real_boot)
> + call EXT_C(grub_dl_unload_all)
> + call EXT_C(grub_stop_floppy)
> +
> + cli
> +
> + popl %eax
> + popl %eax
> + call *%eax
Why a call? Do we need the return address in the stack here? A comment would
be nice in that case.
> +#define ALIGN_DWORD(a) ((a + 3) & (~3))
> +#define ALIGN_PAGE(a) ((a + 4095) & (~4095))
We already have ALIGN_UP:
./include/grub/misc.h:#define ALIGN_UP(addr, align) ((long)((char *)addr +
align - 1) & ~(align - 1))
it'd be better to reuse that, I think.
> +static const grub_uint32_t freebsd_flags[] = {
> + FREEBSD_RB_DUAL, FREEBSD_RB_SERIAL, FREEBSD_RB_ASKNAME,
> + FREEBSD_RB_CDROM, FREEBSD_RB_CONFIG, FREEBSD_RB_KDB,
> + FREEBSD_RB_GDB, FREEBSD_RB_MUTE, FREEBSD_RB_NOINTR,
> + FREEBSD_RB_PAUSE, FREEBSD_RB_QUIET, FREEBSD_RB_DFLTROOT,
> + FREEBSD_RB_SINGLE, FREEBSD_RB_VERBOSE
> +};
We usually put a newline after the '='.
--
Robert Millan
<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call… if you are unable to speak?
(as seen on /.)
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
