Sendkey patch

Sendkey patch

[phcoder]

[-- Attachment #1: Type: text/plain, Size: 2340 bytes --]

Hello. I wrote a patch to add sendkey functionality to grub2. After
loading sendkey module some new environments variables appear with which
one may control the sequence of keys that would be sent to OS or next
bootloader. This functionality is useful in different cases. One example
 is entering "safe mode" in some OS when without this feature you
desperately try many times to press "magic key" in exactly good moment.
Another usage example is chainloading another bootloader. Then you can
send someting like "down down enter" to it and avoid another delay. The
implementation as environment variables has its downside as I don't know
where to put help about it. Perhaps possibility of adding help notice to
environment variables would be a good idea. Then we could do like
>help root
root is a variable containing root device.
For implementing this functionality I needed my function to be executed
right before booting. To do it I added a simple interface for adding
"preboot" functions:
struct grub_preboot_t *grub_loader_add_preboot (grub_err_t
(*preboot_func) (int));
As parameter this function recieves a callback (integer parameter to
callback is noreturn variable). This function returns a handle with
which the preboot function may be removed using
void grub_loader_remove_preboot (struct grub_preboot_t *p);
Implementation uses linked lists and tries to implement it in as few
commands as possible since this code is a part of core image. This
interface can later be reused for adding different hooks to bios ("map",
"memdisk",...). Now the questions are:
Whether such interface is OK for grub2?
Whether we need also interface for adding "postboot" commands? (in case
boot_function returns)
Variables made available after loading sendkey module:
sendkey - a space-separated list of keys to be sent
kb_num, kb_caps, kb_scroll, kb_insert, kb_wait - [keep|on|off] - set
numlock, capslock, scrollock, insert or wait state
kb_noled - [0|1] When set to 1 the LED state is not changed to match
num-/caps-/scrolllock state
"kb_lshift", "kb_rshift", "kb_sysreq", "kb_numkey", "kb_capskey",
"kb_scrollkey", "kb_insertkey", "kb_lalt", "kb_ralt", "kb_lctrl",
"kb_rctrl" - [keep|on|off] When set to on emulate pressing and holding
of corresponding key. To undo just press the corresponding key on your
keyboard

Regards
Vladimir Serbinenko

[-- Attachment #2: sendkey.patch --]
[-- Type: text/x-diff, Size: 21463 bytes --]

Index: conf/i386-pc.rmk
===================================================================
--- conf/i386-pc.rmk	(revision 1845)
+++ conf/i386-pc.rmk	(working copy)
@@ -117,7 +117,7 @@
 	commands/configfile.c commands/echo.c commands/help.c		\
 	commands/terminal.c commands/ls.c commands/test.c 		\
 	commands/search.c commands/blocklist.c commands/hexdump.c	\
-	lib/hexdump.c commands/i386/pc/halt.c commands/reboot.c		\
+	lib/hexdump.c commands/i386/pc/halt.c  commands/reboot.c		\
 	commands/i386/cpuid.c						\
 	disk/host.c disk/loopback.c					\
 	fs/fshelp.c 	\
@@ -161,7 +161,7 @@
 
 # Modules.
 pkglib_MODULES = biosdisk.mod _chain.mod _linux.mod linux.mod normal.mod \
-	_multiboot.mod chain.mod multiboot.mod reboot.mod halt.mod	\
+	_multiboot.mod chain.mod multiboot.mod reboot.mod halt.mod sendkey.mod	\
 	vbe.mod vbetest.mod vbeinfo.mod video.mod gfxterm.mod \
 	videotest.mod play.mod bitmap.mod tga.mod cpuid.mod serial.mod	\
 	ata.mod vga.mod memdisk.mod jpeg.mod png.mod pci.mod lspci.mod \
@@ -214,6 +214,11 @@
 halt_mod_CFLAGS = $(COMMON_CFLAGS)
 halt_mod_LDFLAGS = $(COMMON_LDFLAGS)
 
+# For sendkey.mod.
+sendkey_mod_SOURCES = commands/i386/pc/sendkey.c
+sendkey_mod_CFLAGS = $(COMMON_CFLAGS)
+sendkey_mod_LDFLAGS = $(COMMON_LDFLAGS)
+
 # For serial.mod.
 serial_mod_SOURCES = term/i386/pc/serial.c
 serial_mod_CFLAGS = $(COMMON_CFLAGS)
Index: conf/i386-pc.mk
===================================================================
--- conf/i386-pc.mk	(revision 1845)
+++ conf/i386-pc.mk	(working copy)
@@ -520,7 +520,7 @@
 	commands/configfile.c commands/echo.c commands/help.c		\
 	commands/terminal.c commands/ls.c commands/test.c 		\
 	commands/search.c commands/blocklist.c commands/hexdump.c	\
-	lib/hexdump.c commands/i386/pc/halt.c commands/reboot.c		\
+	lib/hexdump.c commands/i386/pc/halt.c  commands/reboot.c		\
 	commands/i386/cpuid.c						\
 	disk/host.c disk/loopback.c					\
 	fs/fshelp.c 	\
@@ -926,7 +926,7 @@
 
 # Modules.
 pkglib_MODULES = biosdisk.mod _chain.mod _linux.mod linux.mod normal.mod \
-	_multiboot.mod chain.mod multiboot.mod reboot.mod halt.mod	\
+	_multiboot.mod chain.mod multiboot.mod reboot.mod halt.mod sendkey.mod	\
 	vbe.mod vbetest.mod vbeinfo.mod video.mod gfxterm.mod \
 	videotest.mod play.mod bitmap.mod tga.mod cpuid.mod serial.mod	\
 	ata.mod vga.mod memdisk.mod jpeg.mod png.mod pci.mod lspci.mod \
@@ -1661,6 +1661,63 @@
 halt_mod_CFLAGS = $(COMMON_CFLAGS)
 halt_mod_LDFLAGS = $(COMMON_LDFLAGS)
 
+# For sendkey.mod.
+sendkey_mod_SOURCES = commands/i386/pc/sendkey.c
+CLEANFILES += sendkey.mod mod-sendkey.o mod-sendkey.c pre-sendkey.o sendkey_mod-commands_i386_pc_sendkey.o und-sendkey.lst
+ifneq ($(sendkey_mod_EXPORTS),no)
+CLEANFILES += def-sendkey.lst
+DEFSYMFILES += def-sendkey.lst
+endif
+MOSTLYCLEANFILES += sendkey_mod-commands_i386_pc_sendkey.d
+UNDSYMFILES += und-sendkey.lst
+
+sendkey.mod: pre-sendkey.o mod-sendkey.o $(TARGET_OBJ2ELF)
+	-rm -f $@
+	$(TARGET_CC) $(sendkey_mod_LDFLAGS) $(TARGET_LDFLAGS) $(MODULE_LDFLAGS) -Wl,-r,-d -o $@ pre-sendkey.o mod-sendkey.o
+	if test ! -z $(TARGET_OBJ2ELF); then ./$(TARGET_OBJ2ELF) $@ || (rm -f $@; exit 1); fi
+	$(STRIP) --strip-unneeded -K grub_mod_init -K grub_mod_fini -K _grub_mod_init -K _grub_mod_fini -R .note -R .comment $@
+
+pre-sendkey.o: $(sendkey_mod_DEPENDENCIES) sendkey_mod-commands_i386_pc_sendkey.o
+	-rm -f $@
+	$(TARGET_CC) $(sendkey_mod_LDFLAGS) $(TARGET_LDFLAGS) -Wl,-r,-d -o $@ sendkey_mod-commands_i386_pc_sendkey.o
+
+mod-sendkey.o: mod-sendkey.c
+	$(TARGET_CC) $(TARGET_CPPFLAGS) $(TARGET_CFLAGS) $(sendkey_mod_CFLAGS) -c -o $@ $<
+
+mod-sendkey.c: moddep.lst genmodsrc.sh
+	sh $(srcdir)/genmodsrc.sh 'sendkey' $< > $@ || (rm -f $@; exit 1)
+
+ifneq ($(sendkey_mod_EXPORTS),no)
+def-sendkey.lst: pre-sendkey.o
+	$(NM) -g --defined-only -P -p $< | sed 's/^\([^ ]*\).*/\1 sendkey/' > $@
+endif
+
+und-sendkey.lst: pre-sendkey.o
+	echo 'sendkey' > $@
+	$(NM) -u -P -p $< | cut -f1 -d' ' >> $@
+
+sendkey_mod-commands_i386_pc_sendkey.o: commands/i386/pc/sendkey.c $(commands/i386/pc/sendkey.c_DEPENDENCIES)
+	$(TARGET_CC) -Icommands/i386/pc -I$(srcdir)/commands/i386/pc $(TARGET_CPPFLAGS)  $(TARGET_CFLAGS) $(sendkey_mod_CFLAGS) -MD -c -o $@ $<
+-include sendkey_mod-commands_i386_pc_sendkey.d
+
+CLEANFILES += cmd-sendkey_mod-commands_i386_pc_sendkey.lst fs-sendkey_mod-commands_i386_pc_sendkey.lst partmap-sendkey_mod-commands_i386_pc_sendkey.lst
+COMMANDFILES += cmd-sendkey_mod-commands_i386_pc_sendkey.lst
+FSFILES += fs-sendkey_mod-commands_i386_pc_sendkey.lst
+PARTMAPFILES += partmap-sendkey_mod-commands_i386_pc_sendkey.lst
+
+cmd-sendkey_mod-commands_i386_pc_sendkey.lst: commands/i386/pc/sendkey.c $(commands/i386/pc/sendkey.c_DEPENDENCIES) gencmdlist.sh
+	set -e; 	  $(TARGET_CC) -Icommands/i386/pc -I$(srcdir)/commands/i386/pc $(TARGET_CPPFLAGS) $(TARGET_CFLAGS) $(sendkey_mod_CFLAGS) -E $< 	  | sh $(srcdir)/gencmdlist.sh sendkey > $@ || (rm -f $@; exit 1)
+
+fs-sendkey_mod-commands_i386_pc_sendkey.lst: commands/i386/pc/sendkey.c $(commands/i386/pc/sendkey.c_DEPENDENCIES) genfslist.sh
+	set -e; 	  $(TARGET_CC) -Icommands/i386/pc -I$(srcdir)/commands/i386/pc $(TARGET_CPPFLAGS) $(TARGET_CFLAGS) $(sendkey_mod_CFLAGS) -E $< 	  | sh $(srcdir)/genfslist.sh sendkey > $@ || (rm -f $@; exit 1)
+
+partmap-sendkey_mod-commands_i386_pc_sendkey.lst: commands/i386/pc/sendkey.c $(commands/i386/pc/sendkey.c_DEPENDENCIES) genpartmaplist.sh
+	set -e; 	  $(TARGET_CC) -Icommands/i386/pc -I$(srcdir)/commands/i386/pc $(TARGET_CPPFLAGS) $(TARGET_CFLAGS) $(sendkey_mod_CFLAGS) -E $< 	  | sh $(srcdir)/genpartmaplist.sh sendkey > $@ || (rm -f $@; exit 1)
+
+
+sendkey_mod_CFLAGS = $(COMMON_CFLAGS)
+sendkey_mod_LDFLAGS = $(COMMON_LDFLAGS)
+
 # For serial.mod.
 serial_mod_SOURCES = term/i386/pc/serial.c
 CLEANFILES += serial.mod mod-serial.o mod-serial.c pre-serial.o serial_mod-term_i386_pc_serial.o und-serial.lst
Index: kern/loader.c
===================================================================
--- kern/loader.c	(revision 1845)
+++ kern/loader.c	(working copy)
@@ -22,12 +22,41 @@
 #include <grub/err.h>
 #include <grub/kernel.h>
 
+
 static grub_err_t (*grub_loader_boot_func) (void);
 static grub_err_t (*grub_loader_unload_func) (void);
 static int grub_loader_noreturn;
 
 static int grub_loader_loaded;
 
+static struct grub_preboot_t *grub_loader_preboots=0;
+
+struct grub_preboot_t *
+grub_loader_add_preboot (grub_err_t (*preboot_func) (int))
+{
+  struct grub_preboot_t **cur=&grub_loader_preboots;
+  if (!preboot_func)
+    return 0;
+  while (*cur)    
+    cur=&((*cur)->next);
+  *cur=(struct grub_preboot_t *)grub_malloc (sizeof (struct grub_preboot_t));
+  (*cur)->prev_pointer=cur;
+  (*cur)->next=0;
+  (*cur)->preboot_func=preboot_func;
+  return *cur;
+}
+
+void
+grub_loader_remove_preboot (struct grub_preboot_t *p)
+{
+  if (!p)
+    return;
+  *(p->prev_pointer)=p->next;
+  if (p->next)
+    (p->next)->prev_pointer=p->prev_pointer;
+  grub_free (p);
+}
+
 int
 grub_loader_is_loaded (void)
 {
@@ -64,11 +93,19 @@
 grub_err_t
 grub_loader_boot (void)
 {
+  struct grub_preboot_t *iter=grub_loader_preboots;
   if (! grub_loader_loaded)
     return grub_error (GRUB_ERR_NO_KERNEL, "no loaded kernel");
 
   if (grub_loader_noreturn)
     grub_machine_fini ();
+
+  while (iter)
+    {
+      if (iter->preboot_func)
+	iter->preboot_func (grub_loader_noreturn);
+      iter=iter->next;
+    }
   
   return (grub_loader_boot_func) ();
 }
Index: include/grub/loader.h
===================================================================
--- include/grub/loader.h	(revision 1845)
+++ include/grub/loader.h	(working copy)
@@ -25,6 +25,14 @@
 #include <grub/err.h>
 #include <grub/types.h>
 
+struct grub_preboot_t
+{
+  grub_err_t (*preboot_func) (int);  
+  struct grub_preboot_t *next;
+  struct grub_preboot_t **prev_pointer;
+};
+
+
 /* Check if a loader is loaded.  */
 int EXPORT_FUNC(grub_loader_is_loaded) (void);
 
@@ -37,6 +45,12 @@
 /* Unset current loader, if any.  */
 void EXPORT_FUNC(grub_loader_unset) (void);
 
+/*Add a preboot function*/
+struct grub_preboot_t *EXPORT_FUNC(grub_loader_add_preboot) (grub_err_t (*preboot_func) (int noreturn));
+
+/*Remove given preboot function*/
+void EXPORT_FUNC(grub_loader_remove_preboot) (struct grub_preboot_t *p);
+
 /* Call the boot hook in current loader. This may or may not return,
    depending on the setting by grub_loader_set.  */
 grub_err_t EXPORT_FUNC(grub_loader_boot) (void);
Index: commands/i386/pc/sendkey.c
===================================================================
--- commands/i386/pc/sendkey.c	(revision 0)
+++ commands/i386/pc/sendkey.c	(revision 0)
@@ -0,0 +1,392 @@
+/* sendkey.c - test module for dynamic loading */
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2005  Free Software Foundation, Inc.
+ *  Copyright (C) 2005  Vladimir Serbinenko serbinenko.vova@bk.ru
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include <grub/normal.h>
+#include <grub/dl.h>
+#include <grub/arg.h>
+#include <grub/misc.h>
+#include <grub/loader.h>
+
+#define RAW_ADDR(a) ((void *)(a))
+
+static struct grub_preboot_t *grub_sendkey_preboot_handle=0;
+struct 
+keysym
+{
+  char *unshifted_name;			/* the name in unshifted state */
+  char *shifted_name;			/* the name in shifted state */
+  unsigned char unshifted_ascii;	/* the ascii code in unshifted state */
+  unsigned char shifted_ascii;		/* the ascii code in shifted state */
+  unsigned char keycode;		/* keyboard scancode */
+};
+
+/* The table for key symbols. If the "shifted" member of an entry is
+   NULL, the entry does not have shifted state. Copied from GRUB Legacy setkey fuction  */
+static struct keysym keysym_table[] =
+{
+  {"escape",		0,		0x1b,	0,	0x01},
+  {"1",			"exclam",	'1',	'!',	0x02},
+  {"2",			"at",		'2',	'@',	0x03},
+  {"3",			"numbersign",	'3',	'#',	0x04},
+  {"4",			"dollar",	'4',	'$',	0x05},
+  {"5",			"percent",	'5',	'%',	0x06},
+  {"6",			"caret",	'6',	'^',	0x07},
+  {"7",			"ampersand",	'7',	'&',	0x08},
+  {"8",			"asterisk",	'8',	'*',	0x09},
+  {"9",			"parenleft",	'9',	'(',	0x0a},
+  {"0",			"parenright",	'0',	')',	0x0b},
+  {"minus",		"underscore",	'-',	'_',	0x0c},
+  {"equal",		"plus",		'=',	'+',	0x0d},
+  {"backspace",		0,		'\b',	0,	0x0e},
+  {"tab",		0,		'\t',	0,	0x0f},
+  {"q",			"Q",		'q',	'Q',	0x10},
+  {"w",			"W",		'w',	'W',	0x11},
+  {"e",			"E",		'e',	'E',	0x12},
+  {"r",			"R",		'r',	'R',	0x13},
+  {"t",			"T",		't',	'T',	0x14},
+  {"y",			"Y",		'y',	'Y',	0x15},
+  {"u",			"U",		'u',	'U',	0x16},
+  {"i",			"I",		'i',	'I',	0x17},
+  {"o",			"O",		'o',	'O',	0x18},
+  {"p",			"P",		'p',	'P',	0x19},
+  {"bracketleft",	"braceleft",	'[',	'{',	0x1a},
+  {"bracketright",	"braceright",	']',	'}',	0x1b},
+  {"enter",		0,		'\r',	0,	0x1c},
+  {"control",		0,		0,	0,	0x1d},
+  {"a",			"A",		'a',	'A',	0x1e},
+  {"s",			"S",		's',	'S',	0x1f},
+  {"d",			"D",		'd',	'D',	0x20},
+  {"f",			"F",		'f',	'F',	0x21},
+  {"g",			"G",		'g',	'G',	0x22},
+  {"h",			"H",		'h',	'H',	0x23},
+  {"j",			"J",		'j',	'J',	0x24},
+  {"k",			"K",		'k',	'K',	0x25},
+  {"l",			"L",		'l',	'L',	0x26},
+  {"semicolon",		"colon",	';',	':',	0x27},
+  {"quote",		"doublequote",	'\'',	'"',	0x28},
+  {"backquote",		"tilde",	'`',	'~',	0x29},
+  {"shift",		0,		0,	0,	0x2a},
+  {"backslash",		"bar",		'\\',	'|',	0x2b},
+  {"z",			"Z",		'z',	'Z',	0x2c},
+  {"x",			"X",		'x',	'X',	0x2d},
+  {"c",			"C",		'c',	'C',	0x2e},
+  {"v",			"V",		'v',	'V',	0x2f},
+  {"b",			"B",		'b',	'B',	0x30},
+  {"n",			"N",		'n',	'N',	0x31},
+  {"m",			"M",		'm',	'M',	0x32},
+  {"comma",		"less",		',',	'<',	0x33},
+  {"period",		"greater",	'.',	'>',	0x34},
+  {"slash",		"question",	'/',	'?',	0x35},
+  {"rshift",		0,		0,	0,	0x36},
+  {"numasterisk",		0,		'*',	0,	0x37},
+  {"alt",		0,		0,	0,	0x38},
+  {"space",		0,		' ',	0,	0x39},
+  {"capslock",		0,		0,	0,	0x3a},
+  {"F1",		0,		0,	0,	0x3b},
+  {"F2",		0,		0,	0,	0x3c},
+  {"F3",		0,		0,	0,	0x3d},
+  {"F4",		0,		0,	0,	0x3e},
+  {"F5",		0,		0,	0,	0x3f},
+  {"F6",	 	0,		0,	0,	0x40},
+  {"F7",		0,		0,	0,	0x41},
+  {"F8",		0,		0,	0,	0x42},
+  {"F9",		0,		0,	0,	0x43},
+  {"F10",		0,		0,	0,	0x44},
+  {"num7",		"numhome",		'7',	0,	0x47},
+  {"num8",		"numup",		'8',	0,	0x48},
+  {"num9",		"numpgup",		'9',	0,	0x49},
+  {"numminus",		0,		'-',	0,	0x4a},
+  {"num4",		"numleft",		'4',	0,	0x4b},
+  {"num5",		"num5numlock",		'5',	0,	0x4c},
+  {"num6",		"numright",		'6',	0,	0x4d},
+  {"numplus",		0,		'-',	0,	0x4e},
+  {"num1",		"numend",		'1',	0,	0x4f},
+  {"num2",		"numdown",		'2',	0,	0x50},
+  {"num3",		"numpgdown",		'3',	0,	0x51},
+  {"num0",		"numinsert",		'0',	0,	0x52},
+  {"numperiod",	"numdelete", 0,	0x7f,		0x53},
+  {"F11",		0,		0,	0,	0x57},
+  {"F12",		0,		0,	0,	0x58},
+  {"numenter",		0,		'\r',	0,	0xe0},
+  {"numslash",		0,		'/',	0,	0xe0},
+  {"delete",		0,		0x7f,	0,	0xe0},
+  {"insert",		0,		0xe0,	0,	0x52},
+  {"home",		0,		0xe0,	0,	0x47},
+  {"end",		0,		0xe0,	0,	0x4f},
+  {"pgdown",		0,		0xe0,	0,	0x51},
+  {"pgup",		0,		0xe0,	0,	0x49},
+  {"down",		0,		0xe0,	0,	0x50},
+  {"up",		0,		0xe0,	0,	0x48},
+  {"left",		0,		0xe0,	0,	0x4b},
+  {"right",		0,		0xe0,	0,	0x4d}
+};
+
+/* Send a character VALUE to port PORT  */
+static void 
+outportb (char value, int port) {
+  asm volatile ("outb %%al,%%dx": :"a" (value),"d" (port));
+  return;
+}
+
+/* Read a byte from port PORT  */
+static unsigned char 
+inb (unsigned int port)
+{
+  unsigned char ret;
+  asm volatile ("inb %%dx,%%al":"=a" (ret):"d" (port));
+  return ret;
+
+}
+
+/* Set a simple flag in flags variable  
+   FLAGS - where to set,
+   OUTOFFSET - offset of flag in FLAGS,
+   OP - action id
+*/
+static void
+grub_sendkey_set_simple_flag (unsigned long *flags, int outoffset, int op)
+{
+  /* previous state of flag  */
+  int prevstat = (*flags >> outoffset) & 1;
+  /* new state */
+  int newstat = (op == 1) || (op == 2 && prevstat);
+  /* Set new state  */
+  *flags = (*flags & (~(1 << outoffset))) | (newstat << outoffset);
+}
+
+/* Set a double flag (ctrl/alt) in flags variable  
+   FLAGS - where to set,
+   OUTOFFSETR - offset of common flag in FLAGS,
+   OUTOFFSETL - offset of "left" flag in FLAGS,
+   OPR - operation for "right" flag,
+   OPL - operation for  "left" flag
+*/
+static void
+grub_sendkey_set_double_flag (unsigned long *flags, int outoffsetc, int outoffsetl, int opr, int opl)
+{
+  /* previous state of flag  */
+  int prevstatc = (*flags >> outoffsetc) & 1;
+  int prevstatl = (*flags >> outoffsetl) & 1;
+  int prevstatr = prevstatc && (!prevstatl);
+  /* new state */
+  int newstatl = (opl == 1) || (opl == 2 && prevstatl);
+  int newstatr = (opr == 1) || (opr == 2 && prevstatr);
+  int newstatc = newstatr || newstatr;
+  /* Set new state  */
+  *flags = (*flags & (~(1 << outoffsetl))) | (newstatl << outoffsetl);
+  *flags = (*flags & (~(1 << outoffsetc))) | (newstatc << outoffsetc);
+}
+
+static int
+grub_sendkey_parse_op (char *name)
+{
+  char *var;
+
+  var = grub_env_get (name);
+
+  if (!var)
+    return 2;
+
+  if (!grub_strcmp (var, "off") || !grub_strcmp (var, "0") || !grub_strcmp (var, "unpress"))
+    return 0;
+
+  if (!grub_strcmp (var, "on") || !grub_strcmp (var, "1") || !grub_strcmp (var, "press"))
+    return 1;
+
+  return 2;
+}
+
+/* Set keyboard buffer to our sendkey  */
+static grub_err_t
+grub_sendkey_preboot (int noreturn __attribute__ ((unused)))
+{
+
+  /* Length of sendkey  */
+  int keylen = 0;
+  char sendkey[0x20];
+  /* For convenion: pointer to flags  */
+  unsigned long *flags = (unsigned long *) RAW_ADDR (0x417);
+  char *next, ch=0, *sendkeyvar;
+  int noled = 0;
+
+  auto int find_key_code (char *key); 
+  auto int find_ascii_code (char *key);
+
+  auto int find_key_code (char *key)
+    {
+      unsigned i;
+
+      for (i = 0; i < sizeof (keysym_table) / sizeof (keysym_table[0]); i++)
+	{
+	  if (keysym_table[i].unshifted_name && grub_strcmp (key, keysym_table[i].unshifted_name) == 0)
+	    return keysym_table[i].keycode;
+	  else if (keysym_table[i].shifted_name && grub_strcmp (key, keysym_table[i].shifted_name) == 0)
+	    return keysym_table[i].keycode;
+	}
+
+      return 0;
+    }
+
+  auto int find_ascii_code (char *key)
+    {
+      unsigned i;
+
+      for (i = 0; i < sizeof (keysym_table) / sizeof (keysym_table[0]); i++)
+	{
+	  if (keysym_table[i].unshifted_name && grub_strcmp (key, keysym_table[i].unshifted_name) == 0)
+	    return keysym_table[i].unshifted_ascii;
+	  else if (keysym_table[i].shifted_name && grub_strcmp (key, keysym_table[i].shifted_name) == 0)
+	    return keysym_table[i].shifted_ascii;
+	}
+
+      return 0;
+    }
+
+  sendkeyvar = grub_env_get ("sendkey");
+
+  if (sendkeyvar)
+    do
+      {
+	next = grub_strchr (sendkeyvar, ' ');
+	if (next)
+	  {
+	    ch = *next;
+	    *next = 0;
+	  }
+	if (find_key_code (sendkeyvar))
+	  {
+	    sendkey[keylen++] = find_ascii_code (sendkeyvar);
+	    sendkey[keylen++] = find_key_code (sendkeyvar);
+	  }
+	if (next)
+	  {
+	    *next = ch;
+	    sendkeyvar = next + 1;
+	  }
+      }
+    while (next && keylen < 0x20);
+  
+  {
+    int i;
+    /* Set the sendkey  */
+    *((char *) RAW_ADDR (0x41a)) = 0x1e;
+    *((char *) RAW_ADDR (0x41c)) = keylen + 0x1e;
+    for(i = 0; i < 0x20; i++)
+      ((char *) RAW_ADDR (0x41e))[i] = sendkey[i];
+  }
+
+  /* Set the flags. For more information reffer to technical specification*/
+  grub_sendkey_set_simple_flag (flags,  5, grub_sendkey_parse_op("kb_num")); // numlock mode
+  grub_sendkey_set_simple_flag (flags,  6, grub_sendkey_parse_op("kb_caps")); // capslock mode
+  grub_sendkey_set_simple_flag (flags,  4, grub_sendkey_parse_op("kb_scroll")); // scrolllock mode
+  grub_sendkey_set_simple_flag (flags,  7, grub_sendkey_parse_op("kb_insert")); // insert mode
+  grub_sendkey_set_simple_flag (flags, 11, grub_sendkey_parse_op("kb_wait")); // wait mode
+  grub_sendkey_set_simple_flag (flags,  1, grub_sendkey_parse_op("kb_lshift")); // left shift
+  grub_sendkey_set_simple_flag (flags,  0, grub_sendkey_parse_op("kb_rshift")); // right shift
+  grub_sendkey_set_simple_flag (flags, 10, grub_sendkey_parse_op("kb_sysreq")); // sysreq
+  grub_sendkey_set_simple_flag (flags, 13, grub_sendkey_parse_op("kb_numkey")); // numlock key
+  grub_sendkey_set_simple_flag (flags, 14, grub_sendkey_parse_op("kb_capskey")); // capslock key
+  grub_sendkey_set_simple_flag (flags, 12, grub_sendkey_parse_op("kb_scrollkey")); // scrolllock key
+  grub_sendkey_set_simple_flag (flags, 15, grub_sendkey_parse_op("kb_insertkey")); // insert key
+
+  /*Set ctrl and alt*/
+  grub_sendkey_set_double_flag (flags, 2, 8, grub_sendkey_parse_op("kb_rctrl"), grub_sendkey_parse_op("kb_lctrl")); //Ctrl
+  grub_sendkey_set_double_flag (flags, 3, 9, grub_sendkey_parse_op("kb_ralt"),  grub_sendkey_parse_op("kb_lalt")); //Alt
+
+
+  /* Set noled */
+  {
+    char *var;
+
+    /* set 1 if set explicitely  */
+    if ((var = grub_env_get ("kb_noled")) && grub_strcmp (var, "0"))
+      noled = 1;
+    
+    /* implicit: when LEDs haven't changed  */
+    if (!var && grub_sendkey_parse_op("kb_num") == 2 && grub_sendkey_parse_op("kb_caps") == 2
+	&& grub_sendkey_parse_op("kb_scroll") == 2)
+      noled = 1;
+  }
+
+  /* Write new LED state  */
+  if (!noled)
+    {
+      int value = 0;
+      int failed;
+      /* Try 5 times  */
+      for (failed = 0; failed < 5; failed++)
+	{
+	  value = 0;
+	  /* Send command change LEDs  */
+	  outportb (0xed, 0x60);
+
+	  /* Wait */
+	  while ((value != 0xfa) && (value != 0xfe))
+	    value = inb (0x60);
+
+	  if (value == 0xfa)
+	    {
+	      /* Set new LEDs*/
+	      outportb ((flags[0] >> 4) & 7, 0x60);
+	      break;
+	    }
+	}
+    }
+  return 0;
+}
+
+GRUB_MOD_INIT(sendkey)
+{
+  (void)mod;			/* To stop warning. */
+  unsigned i;
+  /* List of variables to set to "keep"  */
+  static char list[16][12] = 
+    {
+      "kb_num", "kb_caps", "kb_scroll", "kb_insert", "kb_wait", "kb_lshift", "kb_rshift", "kb_sysreq", 
+      "kb_numkey", "kb_capskey", "kb_scrollkey", "kb_insertkey", "kb_lalt", "kb_ralt", "kb_lctrl", "kb_rctrl"
+    };
+
+  grub_env_set ("sendkey", "");
+  grub_env_set ("kb_noled", "0");
+
+  for (i = 0; i < sizeof (list) / sizeof (list[0]); i++)
+    grub_env_set (list[i], "keep");
+
+  grub_sendkey_preboot_handle=grub_loader_add_preboot (grub_sendkey_preboot);
+}
+
+GRUB_MOD_FINI(sendkey)
+{
+
+  unsigned i;
+  /* List of variables to unset  */
+  static char list[19][12] = 
+    {
+      "kb_num", "kb_caps", "kb_scroll", "kb_insert", "kb_wait", "kb_lshift", "kb_rshift", "kb_sysreq", 
+      "kb_numkey", "kb_capskey", "kb_scrollkey", "kb_insertkey", "kb_lalt", "kb_ralt", "kb_lctrl", "kb_rctrl",
+      "sendkey", "kb_noled"
+    };
+
+  for (i = 0; i < sizeof (list) / sizeof (list[0]); i++)
+    grub_env_unset (list[i]);  
+  
+  grub_loader_remove_preboot (grub_sendkey_preboot_handle);
+}

Re: Sendkey patch

[Javier Martín]

[-- Attachment #1: Type: text/plain, Size: 1887 bytes --]

El mar, 02-09-2008 a las 16:23 +0200, phcoder escribió:
> For implementing this functionality I needed my function to be executed
> right before booting. To do it I added a simple interface for adding
> "preboot" functions:
> struct grub_preboot_t *grub_loader_add_preboot (grub_err_t
> (*preboot_func) (int));
> As parameter this function recieves a callback (integer parameter to
> callback is noreturn variable). This function returns a handle with
> which the preboot function may be removed using
> void grub_loader_remove_preboot (struct grub_preboot_t *p);
> Implementation uses linked lists and tries to implement it in as few
> commands as possible since this code is a part of core image. This
> interface can later be reused for adding different hooks to bios ("map",
> "memdisk",...). Now the questions are:
> Whether such interface is OK for grub2?
An interface like this is implemented in another patch in "discussion",
my drivemap patch (see the August list archives). As yours, it is
linked-list based and very similar in the prototypes. I haven't checked
if your code has any kind of errors/corner cases, but it seems terser
than mine even though it's a bit more difficult to understand because
you use double pointers to avoid my handling of the head case. I don't
understand the purpose of doubly-linking the list though...

> Whether we need also interface for adding "postboot" commands? (in case
> boot_function returns)
I don't think it would offer a lot of functionality because most loaders
don't return on failure, they just get stuck or their payload
triple-faults and reboots. The same question applies for the failure of
one of the preboot routines. Sure, things like an INT13 handler are easy
to remove and restore the old machine state, but I really think the best
option here would be panic and offer to reboot.

-Habbit

[-- Attachment #2: Esta parte del mensaje está firmada digitalmente --]
[-- Type: application/pgp-signature, Size: 827 bytes --]

Re: Sendkey patch

[phcoder]

Hello, again
I had a look at your patch. In some mail it was suggested that kernel
patch should be split from module that uses it. So I resend the kernel
part of my patch. Another question is whether we need some kind of
abortion procedure (like in your patch) if a preboot hook fails.
Javier Martín wrote:
> An interface like this is implemented in another patch in "discussion",
> my drivemap patch (see the August list archives). As yours, it is
> linked-list based and very similar in the prototypes. I haven't checked
> if your code has any kind of errors/corner cases,

I checked and found that I didn't check if grub_malloc succeeded

> but it seems terser
> than mine even though it's a bit more difficult to understand because
> you use double pointers to avoid my handling of the head case. I don't
> understand the purpose of doubly-linking the list though...
> 
The goal is to avoid walking through list when deleting an entry from it.
>> Whether we need also interface for adding "postboot" commands? (in case
>> boot_function returns)
> I don't think it would offer a lot of functionality because most loaders
> don't return on failure, they just get stuck or their payload
> triple-faults and reboots.

It's the case for i386-pc loaders but not the case of some other targets
(e.g. EFI). So the question remains.

Vladimir Serbinenko

Re: Sendkey patch

[phcoder]

[-- Attachment #1: Type: text/plain, Size: 1433 bytes --]

Sorry, forgot to attach
phcoder wrote:
> Hello, again
> I had a look at your patch. In some mail it was suggested that kernel
> patch should be split from module that uses it. So I resend the kernel
> part of my patch. Another question is whether we need some kind of
> abortion procedure (like in your patch) if a preboot hook fails.
> Javier Martín wrote:
>> An interface like this is implemented in another patch in "discussion",
>> my drivemap patch (see the August list archives). As yours, it is
>> linked-list based and very similar in the prototypes. I haven't checked
>> if your code has any kind of errors/corner cases,
> 
> I checked and found that I didn't check if grub_malloc succeeded
> 
>> but it seems terser
>> than mine even though it's a bit more difficult to understand because
>> you use double pointers to avoid my handling of the head case. I don't
>> understand the purpose of doubly-linking the list though...
>>
> The goal is to avoid walking through list when deleting an entry from it.
>>> Whether we need also interface for adding "postboot" commands? (in case
>>> boot_function returns)
>> I don't think it would offer a lot of functionality because most loaders
>> don't return on failure, they just get stuck or their payload
>> triple-faults and reboots.
> 
> It's the case for i386-pc loaders but not the case of some other targets
> (e.g. EFI). So the question remains.
> 
> Vladimir Serbinenko


[-- Attachment #2: preboot.patch --]
[-- Type: text/x-diff, Size: 2670 bytes --]

Index: kern/loader.c
===================================================================
--- kern/loader.c	(revision 1845)
+++ kern/loader.c	(working copy)
@@ -22,12 +22,46 @@
 #include <grub/err.h>
 #include <grub/kernel.h>
 
+
 static grub_err_t (*grub_loader_boot_func) (void);
 static grub_err_t (*grub_loader_unload_func) (void);
 static int grub_loader_noreturn;
 
 static int grub_loader_loaded;
 
+static struct grub_preboot_t *grub_loader_preboots=0;
+
+struct grub_preboot_t *
+grub_loader_add_preboot (grub_err_t (*preboot_func) (int))
+{
+  struct grub_preboot_t **cur=&grub_loader_preboots;
+  if (!preboot_func)
+    return 0;
+  while (*cur)    
+    cur=&((*cur)->next);
+  *cur=(struct grub_preboot_t *)grub_malloc (sizeof (struct grub_preboot_t));
+  if (!*cur)
+    {
+      grub_error (GRUB_ERR_OUT_OF_MEMORY, "hook not added");
+      return 0;
+    }
+  (*cur)->prev_pointer=cur;
+  (*cur)->next=0;
+  (*cur)->preboot_func=preboot_func;
+  return *cur;
+}
+
+void
+grub_loader_remove_preboot (struct grub_preboot_t *p)
+{
+  if (!p)
+    return;
+  *(p->prev_pointer)=p->next;
+  if (p->next)
+    (p->next)->prev_pointer=p->prev_pointer;
+  grub_free (p);
+}
+
 int
 grub_loader_is_loaded (void)
 {
@@ -64,11 +98,19 @@
 grub_err_t
 grub_loader_boot (void)
 {
+  struct grub_preboot_t *iter=grub_loader_preboots;
   if (! grub_loader_loaded)
     return grub_error (GRUB_ERR_NO_KERNEL, "no loaded kernel");
 
   if (grub_loader_noreturn)
     grub_machine_fini ();
+
+  while (iter)
+    {
+      if (iter->preboot_func)
+	iter->preboot_func (grub_loader_noreturn);
+      iter=iter->next;
+    }
   
   return (grub_loader_boot_func) ();
 }
Index: include/grub/loader.h
===================================================================
--- include/grub/loader.h	(revision 1845)
+++ include/grub/loader.h	(working copy)
@@ -25,6 +25,14 @@
 #include <grub/err.h>
 #include <grub/types.h>
 
+struct grub_preboot_t
+{
+  grub_err_t (*preboot_func) (int);  
+  struct grub_preboot_t *next;
+  struct grub_preboot_t **prev_pointer;
+};
+
+
 /* Check if a loader is loaded.  */
 int EXPORT_FUNC(grub_loader_is_loaded) (void);
 
@@ -37,6 +45,12 @@
 /* Unset current loader, if any.  */
 void EXPORT_FUNC(grub_loader_unset) (void);
 
+/*Add a preboot function*/
+struct grub_preboot_t *EXPORT_FUNC(grub_loader_add_preboot) (grub_err_t (*preboot_func) (int noreturn));
+
+/*Remove given preboot function*/
+void EXPORT_FUNC(grub_loader_remove_preboot) (struct grub_preboot_t *p);
+
 /* Call the boot hook in current loader. This may or may not return,
    depending on the setting by grub_loader_set.  */
 grub_err_t EXPORT_FUNC(grub_loader_boot) (void);

Re: Sendkey patch

[Vesa Jääskeläinen]

Hi,

Now that we have two use cases for this. I would propose that we first
fine tune interface and then use common one. This is the critical part
of the design and needs to be solved. I wasn't really convinced with the
other proposed interface.

So if you two could propose improved one that can handle following:

- BIOS interrupt service hooks
  - drivemap
  - eltorito emulation
  - custom int13h handler for unsupported controllers
- custom GRUB code to be executed
  - sendkey feature

This way it would be also easier to incorporate patches as there is
already skeleton that can be used easily.

What we don't want is to increase kernel size too much. So please try to
think alternatives that reside on loaded module. There might be some
other options too, but I'll let you guys think a bit first :)

Thanks,
Vesa Jääskeläinen

Re: Sendkey patch

[Javier Martín]

[-- Attachment #1: Type: text/plain, Size: 1942 bytes --]

El mar, 02-09-2008 a las 17:58 +0200, phcoder escribió:
> > but it seems terser
> > than mine even though it's a bit more difficult to understand because
> > you use double pointers to avoid my handling of the head case. I don't
> > understand the purpose of doubly-linking the list though...
> > 
> The goal is to avoid walking through list when deleting an entry from it.
But you negate any performance gain when you _do_ traverse the list to
add an entry to it instead of just make it the new head as I do.
Besides, even for that, double indirection should be avoided in the
structure previous pointer because it makes things oh-so-incredibly
confusing.

Besides, I think the "user" (i.e. module) visible type returned by _add
and taken by _remove should be a "blank hidden type", i.e. you don't
need to declare "struct grub_preboot_t" in loader.h because the public
interface only uses _pointers_ to it, whose size is known. This is all
the C compiler requires and you avoid polluting the namespace with
internal implementation details. I recommend the following typedefs:

typedef struct grub_preboot_t* grub_preboot_hnd;
typedef grub_err_t *(grub_preboot_func)(int noreturn);

So that the prototypes would look

grub_preboot_hnd add(grub_preboot_func f);
void remove(grub_preboot_hnd handle);

> >> Whether we need also interface for adding "postboot" commands? (in case
> >> boot_function returns)
> > I don't think it would offer a lot of functionality because most loaders
> > don't return on failure, they just get stuck or their payload
> > triple-faults and reboots.
> 
> It's the case for i386-pc loaders but not the case of some other targets
> (e.g. EFI). So the question remains.
And same question for abortion procedures. This is a spinous matter
because clean handling is difficult, not always possible and requires
registering "undo" functions, i.e. more bloat in kernel.

-Habbit

[-- Attachment #2: Esta parte del mensaje está firmada digitalmente --]
[-- Type: application/pgp-signature, Size: 827 bytes --]

Re: Sendkey patch

[phcoder]

[-- Attachment #1: Type: text/plain, Size: 1397 bytes --]

Javier Martín wrote:
> But you negate any performance gain when you _do_ traverse the list to
> add an entry to it instead of just make it the new head as I do.
> Besides, even for that, double indirection should be avoided in the
> structure previous pointer because it makes things oh-so-incredibly
> confusing.
I was thinking about code size then about performance or easy of
understanding. On my system this code results in 91 addition bytes in
core image (63 if I remove grub_error). Do you have any idea how it
would be possible to do an error message without description or put some
generic description?
> 
> Besides, I think the "user" (i.e. module) visible type returned by _add
> and taken by _remove should be a "blank hidden type", i.e. you don't
> need to declare "struct grub_preboot_t" in loader.h because the public
> interface only uses _pointers_ to it, whose size is known. This is all
> the C compiler requires and you avoid polluting the namespace with
> internal implementation details. I recommend the following typedefs:
> 
> typedef struct grub_preboot_t* grub_preboot_hnd;
> typedef grub_err_t *(grub_preboot_func)(int noreturn);
> 
> So that the prototypes would look
> 
> grub_preboot_hnd add(grub_preboot_func f);
> void remove(grub_preboot_hnd handle);
I noticed that actually no code needs the size of grub_preoot_hnd. So I
changed it to void *
Vladimir Serbinenko

[-- Attachment #2: preboot.patch --]
[-- Type: text/x-diff, Size: 2730 bytes --]

Index: kern/loader.c
===================================================================
--- kern/loader.c	(revision 1845)
+++ kern/loader.c	(working copy)
@@ -22,12 +22,54 @@
 #include <grub/err.h>
 #include <grub/kernel.h>
 
+#define PREBOOT_HND(x) (((struct grub_preboot_t *)x))
+
+struct grub_preboot_t
+{
+  grub_err_t (*preboot_func) (int);  
+  struct grub_preboot_t *next;
+  struct grub_preboot_t **prev_pointer;
+};
+
 static grub_err_t (*grub_loader_boot_func) (void);
 static grub_err_t (*grub_loader_unload_func) (void);
 static int grub_loader_noreturn;
 
 static int grub_loader_loaded;
 
+static struct grub_preboot_t *grub_loader_preboots=0;
+
+void *
+grub_loader_add_preboot (grub_err_t (*preboot_func) (int))
+{
+  struct grub_preboot_t *cur;
+  if (!preboot_func)
+    return 0;
+  cur=(struct grub_preboot_t *)grub_malloc (sizeof (struct grub_preboot_t));
+  if (!cur)
+    {
+      grub_error (GRUB_ERR_OUT_OF_MEMORY, "hook not added");
+      return 0;
+    }
+  cur->next=grub_loader_preboots;
+  cur->prev_pointer=0;
+  cur->next->prev_pointer=&(cur->next);
+  cur->preboot_func=preboot_func;
+  grub_loader_preboots=cur;
+  return cur;
+}
+
+void
+grub_loader_remove_preboot (void *p)
+{
+  if (!p)
+    return;
+  *(PREBOOT_HND(p)->prev_pointer)=PREBOOT_HND(p)->next;
+  if (PREBOOT_HND(p)->next)
+    PREBOOT_HND(p)->next->prev_pointer=PREBOOT_HND(p)->prev_pointer;
+    grub_free (p);
+}
+
 int
 grub_loader_is_loaded (void)
 {
@@ -64,12 +106,18 @@
 grub_err_t
 grub_loader_boot (void)
 {
+  struct grub_preboot_t *iter=grub_loader_preboots;
   if (! grub_loader_loaded)
     return grub_error (GRUB_ERR_NO_KERNEL, "no loaded kernel");
 
   if (grub_loader_noreturn)
     grub_machine_fini ();
-  
+  while (iter)
+    {
+      if (iter->preboot_func)
+	iter->preboot_func (grub_loader_noreturn);
+      iter=iter->next;
+    }
   return (grub_loader_boot_func) ();
 }
 
Index: include/grub/loader.h
===================================================================
--- include/grub/loader.h	(revision 1845)
+++ include/grub/loader.h	(working copy)
@@ -25,6 +25,7 @@
 #include <grub/err.h>
 #include <grub/types.h>
 
+
 /* Check if a loader is loaded.  */
 int EXPORT_FUNC(grub_loader_is_loaded) (void);
 
@@ -37,6 +38,12 @@
 /* Unset current loader, if any.  */
 void EXPORT_FUNC(grub_loader_unset) (void);
 
+/*Add a preboot function*/
+void *EXPORT_FUNC(grub_loader_add_preboot) (grub_err_t (*preboot_func) (int noreturn));
+
+/*Remove given preboot function*/
+void EXPORT_FUNC(grub_loader_remove_preboot) (void *hnd);
+
 /* Call the boot hook in current loader. This may or may not return,
    depending on the setting by grub_loader_set.  */
 grub_err_t EXPORT_FUNC(grub_loader_boot) (void);

Re: Sendkey patch

[phcoder]

Well the interface is as we described: the module gives a callback
function which will be called before launching boot function. This
interface is enough for both (and probaly many other) needs. The only
problem is that callback functions can conflict with each other and with
boot function. E.g. if callback sets an INT13h hook and then boot
function reads harddrive then it could get wrong data. In my opinion the
ese callback and boot functions shouldn't use device access at all. This
is especially true because theese functions are after grub_machine_fini.


> 
> This way it would be also easier to incorporate patches as there is
> already skeleton that can be used easily.
> 
There could several templates for using such feature because it's quite
universal


Vladimir Serbinenko

Re: Sendkey patch

[Vesa Jääskeläinen]

phcoder wrote:
> Well the interface is as we described: the module gives a callback
> function which will be called before launching boot function. This
> interface is enough for both (and probaly many other) needs. The only
> problem is that callback functions can conflict with each other and with
> boot function. E.g. if callback sets an INT13h hook and then boot
> function reads harddrive then it could get wrong data. In my opinion the
> ese callback and boot functions shouldn't use device access at all. This
> is especially true because theese functions are after grub_machine_fini.

That is more like a textual description of the issues not an interface
description. It has to be more concrete than that. And mechanism should
support hook installation. And I do not see reason why that same thing
cannot support also this send key stuff.

>> This way it would be also easier to incorporate patches as there is
>> already skeleton that can be used easily.
>>
> There could several templates for using such feature because it's quite
> universal

I am not really delighted about that... Just one or two connection
points and that should be able to handle those cases. If not then we
have something wrong somewhere...

Re: Sendkey patch

[Javier Martín]

[-- Attachment #1.1: Type: text/plain, Size: 1269 bytes --]

El mar, 02-09-2008 a las 20:39 +0200, phcoder escribió:
> +void
> +grub_loader_remove_preboot (void *p)
> +{
> +  if (!p)
> +    return;
> +  *(PREBOOT_HND(p)->prev_pointer)=PREBOOT_HND(p)->next;
This line will "crash" if p is the head of the list (with prev_pointer
being 0). I quote crash because a crash is what happens under an OS:
under GRUB you just overwrite address 0x0 which in i386-pc is the start
of the real mode IVT.

> +  if (PREBOOT_HND(p)->next)
> +    PREBOOT_HND(p)->next->prev_pointer=PREBOOT_HND(p)->prev_pointer;
> +    grub_free (p);
> +}
All these macro plays are nonsense and a hindrance to readability just
because you did not want to add a local variable and do the cast once.

Here is my "version" of your patch, without the double indirection and
the strange plays. The overhead is 103 bytes without the error line
against 63 of yours, but I really think that the symmetric and
understandable handling of previous and next is worth 40 bytes.

PS:
> +void *EXPORT_FUNC(grub_loader_add_preboot) (grub_err_t ...
I think that (only) in function declarations it's better to write "void*
f()" than "void *f()" because otherwise the * can be easily overlooked.
However, this is my word and does not come from the GCS.

[-- Attachment #1.2: preboot.patch --]
[-- Type: text/x-patch, Size: 2862 bytes --]

Index: kern/loader.c
===================================================================
--- kern/loader.c	(revisión: 1845)
+++ kern/loader.c	(copia de trabajo)
@@ -22,12 +22,54 @@
 #include <grub/err.h>
 #include <grub/kernel.h>
 
+struct grub_preboot_t
+{
+  grub_err_t (*preboot_func) (int);  
+  struct grub_preboot_t *next;
+  struct grub_preboot_t *previous;
+};
+
 static grub_err_t (*grub_loader_boot_func) (void);
 static grub_err_t (*grub_loader_unload_func) (void);
 static int grub_loader_noreturn;
 
 static int grub_loader_loaded;
 
+static struct grub_preboot_t *grub_loader_preboots=0;
+
+void *
+grub_loader_add_preboot (grub_err_t (*preboot_func) (int))
+{
+  struct grub_preboot_t *cur;
+  if (!preboot_func)
+    return 0;
+  cur = (struct grub_preboot_t *) grub_malloc (sizeof (struct grub_preboot_t));
+  if (!cur)
+    {
+      grub_error (GRUB_ERR_OUT_OF_MEMORY, "no memory left to register hook");
+      return 0;
+    }
+  cur->next = grub_loader_preboots;
+  cur->previous = 0;
+  cur->next->previous = cur;
+  cur->preboot_func = preboot_func;
+  grub_loader_preboots = cur;
+  return cur;
+}
+
+void
+grub_loader_remove_preboot (void *p)
+{
+  struct grub_preboot_t *hnd = (struct grub_preboot_t *)p;
+  if (!hnd)
+    return;
+  if (hnd->previous)
+    hnd->previous->next = hnd->next;
+  if (hnd->next)
+    hnd->next->previous = hnd->previous;
+  grub_free (hnd);
+}
+
 int
 grub_loader_is_loaded (void)
 {
@@ -64,12 +106,18 @@
 grub_err_t
 grub_loader_boot (void)
 {
+  struct grub_preboot_t *iter=grub_loader_preboots;
   if (! grub_loader_loaded)
     return grub_error (GRUB_ERR_NO_KERNEL, "no loaded kernel");
 
   if (grub_loader_noreturn)
     grub_machine_fini ();
-  
+  while (iter)
+    {
+      if (iter->preboot_func)
+	iter->preboot_func (grub_loader_noreturn);
+      iter=iter->next;
+    }
   return (grub_loader_boot_func) ();
 }
 
Index: include/grub/loader.h
===================================================================
--- include/grub/loader.h	(revisión: 1845)
+++ include/grub/loader.h	(copia de trabajo)
@@ -25,6 +25,8 @@
 #include <grub/err.h>
 #include <grub/types.h>
 
+typedef grub_err_t (*grub_preboot_func) (int noreturn);
+
 /* Check if a loader is loaded.  */
 int EXPORT_FUNC(grub_loader_is_loaded) (void);
 
@@ -37,6 +39,12 @@
 /* Unset current loader, if any.  */
 void EXPORT_FUNC(grub_loader_unset) (void);
 
+/*Add a preboot function*/
+void* EXPORT_FUNC(grub_loader_add_preboot) (grub_preboot_func func);
+
+/*Remove given preboot function*/
+void EXPORT_FUNC(grub_loader_remove_preboot) (void *hnd);
+
 /* Call the boot hook in current loader. This may or may not return,
    depending on the setting by grub_loader_set.  */
 grub_err_t EXPORT_FUNC(grub_loader_boot) (void);

[-- Attachment #2: Esta parte del mensaje está firmada digitalmente --]
[-- Type: application/pgp-signature, Size: 827 bytes --]

Re: Sendkey patch

[phcoder]

[-- Attachment #1: Type: text/plain, Size: 2238 bytes --]


Javier Martín wrote:
> El mar, 02-09-2008 a las 20:39 +0200, phcoder escribió:
>> +void
>> +grub_loader_remove_preboot (void *p)
>> +{
>> +  if (!p)
>> +    return;
>> +  *(PREBOOT_HND(p)->prev_pointer)=PREBOOT_HND(p)->next;
> This line will "crash" if p is the head of the list (with prev_pointer
> being 0). I quote crash because a crash is what happens under an OS:
> under GRUB you just overwrite address 0x0 which in i386-pc is the start
> of the real mode IVT.
Thank you for pointing at this mistake. Corrected. I should really go to
bed now.
> 
>> +  if (PREBOOT_HND(p)->next)
>> +    PREBOOT_HND(p)->next->prev_pointer=PREBOOT_HND(p)->prev_pointer;
>> +    grub_free (p);
>> +}
> All these macro plays are nonsense and a hindrance to readability just
> because you did not want to add a local variable and do the cast once.
> 
> Here is my "version" of your patch, without the double indirection and
> the strange plays. The overhead is 103 bytes without the error line
> against 63 of yours, but I really think that the symmetric and
> understandable handling of previous and next is worth 40 bytes.
> 
Well let's summ up what we have:
-my version in 63 bytes but difficult to read (could some comments help
with it?)
-your version much easier to read but in 103 bytes
Neither of versions is right or wrong. It's a question of priorities. I
had some experiences that past some point it's difficult to decrease
size past some point. Core image has to be embed in first cylinder.
There we have 62 sectors=31744 bytes. And now our core image (my version
with error reporting) with ata,pc  and reiserfs is 29654 bytes. This
leaves us 2090 bytes. This combination is not something completely out
of the ordinary. So I suggest to give the priority to the size of the
kernel over readability (perhaps adding some comments to my version).
> PS:
>> +void *EXPORT_FUNC(grub_loader_add_preboot) (grub_err_t ...
> I think that (only) in function declarations it's better to write "void*
> f()" than "void *f()" because otherwise the * can be easily overlooked.
> However, this is my word and does not come from the GCS.
> 
I've just made the same that I see in include/grub/mm.h . But for me it
doesn't really matter
Vladimir Serbinenko


[-- Attachment #2: preboot.patch --]
[-- Type: text/x-diff, Size: 2750 bytes --]

Index: kern/loader.c
===================================================================
--- kern/loader.c	(revision 1845)
+++ kern/loader.c	(working copy)
@@ -22,12 +22,54 @@
 #include <grub/err.h>
 #include <grub/kernel.h>
 
+#define PREBOOT_HND(x) (((struct grub_preboot_t *)x))
+
+struct grub_preboot_t
+{
+  grub_err_t (*preboot_func) (int);  
+  struct grub_preboot_t *next;
+  struct grub_preboot_t **prev_pointer;
+};
+
 static grub_err_t (*grub_loader_boot_func) (void);
 static grub_err_t (*grub_loader_unload_func) (void);
 static int grub_loader_noreturn;
 
 static int grub_loader_loaded;
 
+static struct grub_preboot_t *grub_loader_preboots=0;
+
+void *
+grub_loader_add_preboot (grub_err_t (*preboot_func) (int))
+{
+  struct grub_preboot_t *cur;
+  if (!preboot_func)
+    return 0;
+  cur=(struct grub_preboot_t *)grub_malloc (sizeof (struct grub_preboot_t));
+  if (!cur)
+    {
+      grub_error (GRUB_ERR_OUT_OF_MEMORY, "hook not added");
+      return 0;
+    }
+  cur->next=grub_loader_preboots;
+  cur->prev_pointer=&grub_loader_preboots;
+  cur->next->prev_pointer=&(cur->next);
+  cur->preboot_func=preboot_func;
+  grub_loader_preboots=cur;
+  return cur;
+}
+
+void
+grub_loader_remove_preboot (void *p)
+{
+  if (!p)
+    return;
+  *(PREBOOT_HND(p)->prev_pointer)=PREBOOT_HND(p)->next;
+  if (PREBOOT_HND(p)->next)
+    PREBOOT_HND(p)->next->prev_pointer=PREBOOT_HND(p)->prev_pointer;
+    grub_free (p);
+}
+
 int
 grub_loader_is_loaded (void)
 {
@@ -64,12 +106,18 @@
 grub_err_t
 grub_loader_boot (void)
 {
+  struct grub_preboot_t *iter=grub_loader_preboots;
   if (! grub_loader_loaded)
     return grub_error (GRUB_ERR_NO_KERNEL, "no loaded kernel");
 
   if (grub_loader_noreturn)
     grub_machine_fini ();
-  
+  while (iter)
+    {
+      if (iter->preboot_func)
+	iter->preboot_func (grub_loader_noreturn);
+      iter=iter->next;
+    }
   return (grub_loader_boot_func) ();
 }
 
Index: include/grub/loader.h
===================================================================
--- include/grub/loader.h	(revision 1845)
+++ include/grub/loader.h	(working copy)
@@ -25,6 +25,7 @@
 #include <grub/err.h>
 #include <grub/types.h>
 
+
 /* Check if a loader is loaded.  */
 int EXPORT_FUNC(grub_loader_is_loaded) (void);
 
@@ -37,6 +38,12 @@
 /* Unset current loader, if any.  */
 void EXPORT_FUNC(grub_loader_unset) (void);
 
+/*Add a preboot function*/
+void *EXPORT_FUNC(grub_loader_add_preboot) (grub_err_t (*preboot_func) (int noreturn));
+
+/*Remove given preboot function*/
+void EXPORT_FUNC(grub_loader_remove_preboot) (void *hnd);
+
 /* Call the boot hook in current loader. This may or may not return,
    depending on the setting by grub_loader_set.  */
 grub_err_t EXPORT_FUNC(grub_loader_boot) (void);

Re: Sendkey patch

[Javier Martín]

[-- Attachment #1: Type: text/plain, Size: 2061 bytes --]

El mié, 03-09-2008 a las 00:22 +0200, phcoder escribió:
> > Here is my "version" of your patch, without the double indirection and
> > the strange plays. The overhead is 103 bytes without the error line
> > against 63 of yours, but I really think that the symmetric and
> > understandable handling of previous and next is worth 40 bytes.
> > 
> Well let's summ up what we have:
> -my version in 63 bytes but difficult to read (could some comments help
> with it?)
> -your version much easier to read but in 103 bytes
> Neither of versions is right or wrong. It's a question of priorities. I
> had some experiences that past some point it's difficult to decrease
> size past some point. Core image has to be embed in first cylinder.
> There we have 62 sectors=31744 bytes.
We have 63 sectors = 32256 bytes (sectors range from 0 to 63 and the
first is used by the MBR).

> And now our core image (my version
> with error reporting) with ata,pc  and reiserfs is 29654 bytes. This
> leaves us 2090 bytes. This combination is not something completely out
> of the ordinary. So I suggest to give the priority to the size of the
> kernel over readability (perhaps adding some comments to my version).
I was wondering why my data did not match yours, and then I realized I
was using my usual "extreme" combination of modules: "biosdisk pc ext2
lvm raid" yields 29298 bytes, "plenty" of space. But ata and reiserfs
are quite the bloaters... The reiserfs case is particularly strange: in
the linux kernel, the reiserfs module is 50% bigger than ext3.ko; while
in GRUB, reiserfs.mod (without journaling) is twice the size of ext2.mod
(which includes full support for ext2, partial journal support in ext3
and extents in ext4).

Thus, while you are right in prioritizing kernel size; why not optimize
reiserfs a bit instead of killing our (and future maintainers') eyes and
brains to shave less than 40 bytes from kernel? I suppose the story
would be similar with ata, because it is a new module that is yet in
development.

-Habbit

[-- Attachment #2: Esta parte del mensaje está firmada digitalmente --]
[-- Type: application/pgp-signature, Size: 827 bytes --]

Re: Sendkey patch

[phcoder]

Hello, again.
Javier Martín wrote:
> We have 63 sectors = 32256 bytes (sectors range from 0 to 63 and the
> first is used by the MBR).
> 
I've just rechecked on my system. My first partition begins at sector
number 63. This is the value I've seen at most systems. So last usable
sector is 62. Sector 0 is MBR. So we have 62 sectors
>> And now our core image (my version
>> with error reporting) with ata,pc  and reiserfs is 29654 bytes. This
>> leaves us 2090 bytes. This combination is not something completely out
>> of the ordinary. So I suggest to give the priority to the size of the
>> kernel over readability (perhaps adding some comments to my version).
> I was wondering why my data did not match yours, and then I realized I
> was using my usual "extreme" combination of modules: "biosdisk pc ext2
> lvm raid" yields 29298 bytes, "plenty" of space. But ata and reiserfs
> are quite the bloaters... The reiserfs case is particularly strange: in
> the linux kernel, the reiserfs module is 50% bigger than ext3.ko; while
> in GRUB, reiserfs.mod (without journaling) is twice the size of ext2.mod
> (which includes full support for ext2, partial journal support in ext3
> and extents in ext4).
> 
I'll have a look at it but not sure to find anything since I'm not
familiar with either ata or reiserfs internal structure.
> Thus, while you are right in prioritizing kernel size; why not optimize
> reiserfs a bit instead of killing our (and future maintainers') eyes and
> brains to shave less than 40 bytes from kernel? I suppose the story
> would be similar with ata, because it is a new module that is yet in
> development.
Well the point is that if we don't do it now and then one day we'll have
to squeeze the core it will be very difficult to find places like this.
> 
> -Habbit
> 
Vladimir 'phcoder' Serbinenko

Re: Sendkey patch

[Javier Martín]

[-- Attachment #1: Type: text/plain, Size: 2795 bytes --]

El mié, 03-09-2008 a las 02:08 +0200, phcoder escribió:
> Hello, again.
> Javier Martín wrote:
> > We have 63 sectors = 32256 bytes (sectors range from 0 to 63 and the
> > first is used by the MBR).
> > 
> I've just rechecked on my system. My first partition begins at sector
> number 63. This is the value I've seen at most systems. So last usable
> sector is 62. Sector 0 is MBR. So we have 62 sectors
Oops, true! How strange. So there was no sector 63 in the CHS model and
that is why the first sector of cyl 1 (the start of the first partition)
is LBA 63... Does anyone know the historical reasons for this?

> >> And now our core image (my version
> >> with error reporting) with ata,pc  and reiserfs is 29654 bytes. This
> >> leaves us 2090 bytes. This combination is not something completely out
> >> of the ordinary. So I suggest to give the priority to the size of the
> >> kernel over readability (perhaps adding some comments to my version).
> > I was wondering why my data did not match yours, and then I realized I
> > was using my usual "extreme" combination of modules: "biosdisk pc ext2
> > lvm raid" yields 29298 bytes, "plenty" of space. But ata and reiserfs
> > are quite the bloaters... The reiserfs case is particularly strange: in
> > the linux kernel, the reiserfs module is 50% bigger than ext3.ko; while
> > in GRUB, reiserfs.mod (without journaling) is twice the size of ext2.mod
> > (which includes full support for ext2, partial journal support in ext3
> > and extents in ext4).
> > 
> I'll have a look at it but not sure to find anything since I'm not
> familiar with either ata or reiserfs internal structure.
I was not suggesting that it was you or me who did it; it was a general
"call" for GRUB devs... (ahem xD)

> > Thus, while you are right in prioritizing kernel size; why not optimize
> > reiserfs a bit instead of killing our (and future maintainers') eyes and
> > brains to shave less than 40 bytes from kernel? I suppose the story
> > would be similar with ata, because it is a new module that is yet in
> > development.
> Well the point is that if we don't do it now and then one day we'll have
> to squeeze the core it will be very difficult to find places like this.
Yes, but my point is that 40 bytes is so small a difference that it can
offset by simply rewriting error strings in kernel and other smallish
non-intrusive changes and thus we should prioritize future
maintainability. However, eventually this is not our decision to make:
the Overlords here (mainly Marco, but also Robert, Vesa and Bean) are
the ones who should, likely only once the interface is established,
decide how to conjugate the Prime Directive of "keep kernel small" with
code complexity in this particular case.

-Habbit

[-- Attachment #2: Esta parte del mensaje está firmada digitalmente --]
[-- Type: application/pgp-signature, Size: 827 bytes --]

Re: Sendkey patch

[Felix Zielcke]

Am Mittwoch, den 03.09.2008, 01:38 +0200 schrieb Javier Martín:
> But ata and reiserfs
> are quite the bloaters... The reiserfs case is particularly strange: in
> the linux kernel, the reiserfs module is 50% bigger than ext3.ko; while
> in GRUB, reiserfs.mod (without journaling) is twice the size of ext2.mod
> (which includes full support for ext2, partial journal support in ext3
> and extents in ext4).

ata.mod needs work anyway to be ready to end up in core.img
fs/ext2.c doestn't support the journaling at all.
The code was reverted, just not 100% complete.
The structures are still defined but not used, just use your favourite
editor and search for `journal'.
But comparing the size difference between ext2.ko and reiserfs.ko (or
however exactly it's called, I used it ages ago last)
with the size difference of grub2's ext2.mod and reiserfs.mod is
probable like apples and oranges.
Kernel has full support for everything, wheres grub2 only needs small
read support.

> Thus, while you are right in prioritizing kernel size; why not optimize
> reiserfs a bit instead of killing our (and future maintainers') eyes and
> brains to shave less than 40 bytes from kernel? I suppose the story
> would be similar with ata, because it is a new module that is yet in
> development.

If you can reduce the size of the modules, without making the code ugly
then feel free to do so, but please keep things consistently to the
current commited code.
If you want to introduce new ideas/concepts then I suggest you do a
[RFC] topic on the list to discuss it first.

-- 
Felix Zielcke

Re: Sendkey patch

[phcoder]

Javier Martín wrote:
> El mié, 03-09-2008 a las 02:08 +0200, phcoder escribió:
>> Hello, again.
>> Javier Martín wrote:
>>> We have 63 sectors = 32256 bytes (sectors range from 0 to 63 and the
>>> first is used by the MBR).
>>>
>> I've just rechecked on my system. My first partition begins at sector
>> number 63. This is the value I've seen at most systems. So last usable
>> sector is 62. Sector 0 is MBR. So we have 62 sectors
> Oops, true! How strange. So there was no sector 63 in the CHS model and
> that is why the first sector of cyl 1 (the start of the first partition)
> is LBA 63... Does anyone know the historical reasons for this?
> 
int 0x13:0x08 can't return more than 63 sectors per track.
http://en.wikipedia.org/wiki/Int_13h#INT_13h_AH.3D08h:_Read_Drive_Parameters
>> I'll have a look at it but not sure to find anything since I'm not
>> familiar with either ata or reiserfs internal structure.
> I was not suggesting that it was you or me who did it; it was a general
> "call" for GRUB devs... (ahem xD)
> 
I understand. I was saying that I'll just have a look at least to know
what I'm talking about. But even if we manage to decrease the size of
reiserfs module there are still other FS which could result in big
modules e.g. ZFS.
>>> Thus, while you are right in prioritizing kernel size; why not optimize
>>> reiserfs a bit instead of killing our (and future maintainers') eyes and
>>> brains to shave less than 40 bytes from kernel? I suppose the story
>>> would be similar with ata, because it is a new module that is yet in
>>> development.
>> Well the point is that if we don't do it now and then one day we'll have
>> to squeeze the core it will be very difficult to find places like this.
> Yes, but my point is that 40 bytes is so small a difference

40 bytes are small but 40 bytes per small interface is a big difference

> that it can
> offset by simply rewriting error strings in kernel and other smallish
> non-intrusive changes and thus we should prioritize future
> maintainability. However, eventually this is not our decision to make:
> the Overlords here (mainly Marco, but also Robert, Vesa and Bean) are
> the ones who should, likely only once the interface is established,
> decide how to conjugate the Prime Directive of "keep kernel small" with
> code complexity in this particular case.
> 
You have the point. But if we don't discuss this matter the choice made
by maintainers can be random.
> -Habbit

Vladimir 'phcoder' Serbinenko

Re: Sendkey patch

[bitbucket]

On Wednesday 03 September 2008, Javier Martín wrote:
> El mié, 03-09-2008 a las 02:08 +0200, phcoder escribió:
> > Hello, again.
> >
> > Javier Martín wrote:
> > > We have 63 sectors = 32256 bytes (sectors range from 0 to 63 and the
> > > first is used by the MBR).
> >
> > I've just rechecked on my system. My first partition begins at sector
> > number 63. This is the value I've seen at most systems. So last usable
> > sector is 62. Sector 0 is MBR. So we have 62 sectors
>
> Oops, true! How strange. So there was no sector 63 in the CHS model and
> that is why the first sector of cyl 1 (the start of the first partition)
> is LBA 63... Does anyone know the historical reasons for this?

Well, the point is, in CHS addressing sector numbering starts with 1, not 0, 
and since there are only 6 bits available for the sector number (the other 2 
being used for cylinder number bits #8 and #9), the maximum number of sectors 
per track in CHS in 63.

Regards,
Niels

Re: Sendkey patch

[Javier Martín]

[-- Attachment #1: Type: text/plain, Size: 1152 bytes --]

Hi again,

El mié, 03-09-2008 a las 11:10 +0200, phcoder escribió:
> >> I'll have a look at it but not sure to find anything since I'm not
> >> familiar with either ata or reiserfs internal structure.
> > I was not suggesting that it was you or me who did it; it was a general
> > "call" for GRUB devs... (ahem xD)
> > 
> I understand. I was saying that I'll just have a look at least to know
> what I'm talking about. But even if we manage to decrease the size of
> reiserfs module there are still other FS which could result in big
> modules e.g. ZFS.
Of course, but even though we should strive to support te widest module
configurations possible, GRUB is no panacea. Even when an initialized
GRUB could cope with a ZFS partition in a LVM+RAID on SATA drives not
supported by the BIOS, one should not really expect to be able to boot
directly off it - get a simpler boot scheme, man! extN, FAT or even SFS
are way simpler filesystems which should be used for boot partitions.
More complex filesystems such as ReiserFS or NTFS require _less_ complex
schemes such as no LVM+RAID in order to keep the total core size
manageable.

[-- Attachment #2: Esta parte del mensaje está firmada digitalmente --]
[-- Type: application/pgp-signature, Size: 827 bytes --]

Re: Sendkey patch

[Vesa Jääskeläinen]

Hi,

How about following (may not be syntaxically correct):

In kernel:

/* Variable holding pointer to preboot hook function.  */
preboot_hook:
	.long	0

...
	/* Check if there is preboot hook installed.  */
	movl	preboot_hook, %eax
	testl	%eax, %eax
	jne	1f
	call	%eax
1:
	/* Continue boot.  */


Then in module code:

void grub_preboot_hook_handler(void)
{
	/* Process list of registered preboot hooks.  */
}

void grub_preboot_register_hook(...);
void grub_preboot_unregister_hook(...);

GRUB_MOD_INIT(preboot)
{
	preboot_hook = grub_preboot_hook_handler;
}

GRUB_MOD_FINI(preboot)
{
	preboot_hook = 0;
}

If preboot.mod gets loaded then it goes and registers preboot handler to
kernel to do its tricks. Other modules then would use preboot.mod to
register their own handlers.

This would have minimal impact for kernel.

Thanks,
Vesa Jääskeläinen

Re: Sendkey patch

[Javier Martín]

[-- Attachment #1: Type: text/plain, Size: 1566 bytes --]

El mié, 03-09-2008 a las 20:07 +0300, Vesa Jääskeläinen escribió:
> Hi,
> 
> How about following (may not be syntaxically correct):
> 
> In kernel:
> 
> /* Variable holding pointer to preboot hook function.  */
> preboot_hook:
> 	.long	0
> 
> ...
> 	/* Check if there is preboot hook installed.  */
> 	movl	preboot_hook, %eax
> 	testl	%eax, %eax
> 	jne	1f
> 	call	%eax
> 1:
> 	/* Continue boot.  */
> 
> 
> Then in module code:
> 
> void grub_preboot_hook_handler(void)
> {
> 	/* Process list of registered preboot hooks.  */
> }
> 
> void grub_preboot_register_hook(...);
> void grub_preboot_unregister_hook(...);
> 
> GRUB_MOD_INIT(preboot)
> {
> 	preboot_hook = grub_preboot_hook_handler;
> }
> 
> GRUB_MOD_FINI(preboot)
> {
> 	preboot_hook = 0;
> }
> 
> If preboot.mod gets loaded then it goes and registers preboot handler to
> kernel to do its tricks. Other modules then would use preboot.mod to
> register their own handlers.
This is certainly a good solution, and would probably add less than 10
bytes to the kernel. I only have to add two things:
 * This is not as elegant as it could be, as it forces us to expose
preboot_hook in the kernel, thus breaking encapsulation a bit. But it is
a price that I'm more than willing to pay.
 * I doubt: is the current DL system in GRUB able to hand dependencies
directly? In other words, is our "insmod" more like modprobe or Linux
insmod? This affects the handling of "insmod drivemap" or "insmod
sendkey" if preboot.mod is not loaded beforehand.

-Habbit

[-- Attachment #2: Esta parte del mensaje está firmada digitalmente --]
[-- Type: application/pgp-signature, Size: 827 bytes --]

Re: Sendkey patch

[Felix Zielcke]

Am Mittwoch, den 03.09.2008, 19:23 +0200 schrieb Javier Martín:
>  * I doubt: is the current DL system in GRUB able to hand dependencies
> directly? In other words, is our "insmod" more like modprobe or Linux
> insmod? This affects the handling of "insmod drivemap" or "insmod
> sendkey" if preboot.mod is not loaded beforehand.

Urm /boot/grub/moddep.lst ?
I think grub2 is able to handle module dependencys.
Though I haven't bothered to check the code ;)

-- 
Felix Zielcke

Re: Sendkey patch

[phcoder]

Hello. In this case we can transfer the whole functionality located in
kern/loader.c to a dedicated module boot.mod. This module will also
register "boot" command. In this way the encapsulation won't be broken
and kernel will become even smaller.
Vladimir 'phcoder' Serbinenko
Vesa Jääskeläinen wrote:
> Hi,
> 
> How about following (may not be syntaxically correct):
> 
> In kernel:
> 
> /* Variable holding pointer to preboot hook function.  */
> preboot_hook:
> 	.long	0
> 
> ...
> 	/* Check if there is preboot hook installed.  */
> 	movl	preboot_hook, %eax
> 	testl	%eax, %eax
> 	jne	1f
> 	call	%eax
> 1:
> 	/* Continue boot.  */
> 
> 
> Then in module code:
> 
> void grub_preboot_hook_handler(void)
> {
> 	/* Process list of registered preboot hooks.  */
> }
> 
> void grub_preboot_register_hook(...);
> void grub_preboot_unregister_hook(...);
> 
> GRUB_MOD_INIT(preboot)
> {
> 	preboot_hook = grub_preboot_hook_handler;
> }
> 
> GRUB_MOD_FINI(preboot)
> {
> 	preboot_hook = 0;
> }
> 
> If preboot.mod gets loaded then it goes and registers preboot handler to
> kernel to do its tricks. Other modules then would use preboot.mod to
> register their own handlers.
> 
> This would have minimal impact for kernel.
> 
> Thanks,
> Vesa Jääskeläinen
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Sendkey patch

[Vesa Jääskeläinen]

phcoder wrote:
> Hello. In this case we can transfer the whole functionality located in
> kern/loader.c to a dedicated module boot.mod. This module will also
> register "boot" command. In this way the encapsulation won't be broken
> and kernel will become even smaller.

Remember that realmode code needs to reside below < 1 MiB. That is the
reason realmode code is embedded in kernel. In there you only see jumps
to RM and back to PM.

Re: Sendkey patch

[phcoder]

Vesa Jääskeläinen wrote:
> phcoder wrote:
>> Hello. In this case we can transfer the whole functionality located in
>> kern/loader.c to a dedicated module boot.mod. This module will also
>> register "boot" command. In this way the encapsulation won't be broken
>> and kernel will become even smaller.
> 
> Remember that realmode code needs to reside below < 1 MiB. That is the
> reason realmode code is embedded in kernel. In there you only see jumps
> to RM and back to PM.
> 
kern/loader.c is always executed in PM. It just chooses which
kernel-specific function to call next. RM helpers are found in
kern/i386/loader.S and kern/i386/pc/startup.S (for chainloader).

Vladimir 'phcoder' Serbinenko

Re: Sendkey patch

[Javier Martín]

[-- Attachment #1: Type: text/plain, Size: 897 bytes --]

El mié, 03-09-2008 a las 20:53 +0300, Vesa Jääskeläinen escribió:
> phcoder wrote:
> > Hello. In this case we can transfer the whole functionality located in
> > kern/loader.c to a dedicated module boot.mod. This module will also
> > register "boot" command. In this way the encapsulation won't be broken
> > and kernel will become even smaller.
> 
> Remember that realmode code needs to reside below < 1 MiB. That is the
> reason realmode code is embedded in kernel. In there you only see jumps
> to RM and back to PM.
We could use the relocator functionality that was once discussed here (I
don't know if it was finally committed) so that modules could declare
"bundles" of code+data to be deployed to RM area. Or make sure every
single instruction in there uses 32-bit addressing, together with
artificial EIP-relativization of addresses like in drivemap_int13.S

-Habbit

[-- Attachment #2: Esta parte del mensaje está firmada digitalmente --]
[-- Type: application/pgp-signature, Size: 827 bytes --]

[PATCH] Move kern/loader.c to boot.mod and add preboot_support (was Re: Sendkey patch)

[phcoder]

[-- Attachment #1: Type: text/plain, Size: 1264 bytes --]

Hello. As I said in another email there is no need for it. I send a
patch for it.
Vladimir 'phcoder' Serbinenko
Javier Martín wrote:
> El mié, 03-09-2008 a las 20:53 +0300, Vesa Jääskeläinen escribió:
>> phcoder wrote:
>>> Hello. In this case we can transfer the whole functionality located in
>>> kern/loader.c to a dedicated module boot.mod. This module will also
>>> register "boot" command. In this way the encapsulation won't be broken
>>> and kernel will become even smaller.
>> Remember that realmode code needs to reside below < 1 MiB. That is the
>> reason realmode code is embedded in kernel. In there you only see jumps
>> to RM and back to PM.
> We could use the relocator functionality that was once discussed here (I
> don't know if it was finally committed) so that modules could declare
> "bundles" of code+data to be deployed to RM area. Or make sure every
> single instruction in there uses 32-bit addressing, together with
> artificial EIP-relativization of addresses like in drivemap_int13.S
> 
> -Habbit
> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel


[-- Attachment #2: boot.patch --]
[-- Type: text/x-diff, Size: 10840 bytes --]

Index: conf/common.rmk
===================================================================
--- conf/common.rmk	(revision 1845)
+++ conf/common.rmk	(working copy)
@@ -311,7 +311,7 @@
 scsi_mod_LDFLAGS = $(COMMON_LDFLAGS)
 
 # Commands.
-pkglib_MODULES += hello.mod boot.mod terminal.mod ls.mod	\
+pkglib_MODULES += hello.mod _boot.mod boot.mod terminal.mod ls.mod	\
 	cmp.mod cat.mod help.mod font.mod search.mod		\
 	loopback.mod fs_uuid.mod configfile.mod echo.mod	\
 	terminfo.mod test.mod blocklist.mod hexdump.mod		\
@@ -322,8 +322,13 @@
 hello_mod_CFLAGS = $(COMMON_CFLAGS)
 hello_mod_LDFLAGS = $(COMMON_LDFLAGS)
 
+# For _boot.mod.
+_boot_mod_SOURCES = commands/boot.c
+_boot_mod_CFLAGS = $(COMMON_CFLAGS)
+_boot_mod_LDFLAGS = $(COMMON_LDFLAGS)
+
 # For boot.mod.
-boot_mod_SOURCES = commands/boot.c
+boot_mod_SOURCES = commands/boot_normal.c
 boot_mod_CFLAGS = $(COMMON_CFLAGS)
 boot_mod_LDFLAGS = $(COMMON_LDFLAGS)
 
Index: conf/i386-pc.rmk
===================================================================
--- conf/i386-pc.rmk	(revision 1845)
+++ conf/i386-pc.rmk	(working copy)
@@ -41,7 +41,7 @@
 # For kernel.img.
 kernel_img_SOURCES = kern/i386/pc/startup.S kern/main.c kern/device.c \
 	kern/disk.c kern/dl.c kern/file.c kern/fs.c kern/err.c \
-	kern/misc.c kern/mm.c kern/loader.c kern/rescue.c kern/term.c \
+	kern/misc.c kern/mm.c kern/rescue.c kern/term.c \
 	kern/time.c \
 	kern/i386/dl.c kern/i386/pc/init.c kern/i386/pc/mmap.c \
 	kern/parser.c kern/partition.c \
@@ -113,11 +113,11 @@
 
 # For grub-emu.
 util/grub-emu.c_DEPENDENCIES = grub_emu_init.h
-grub_emu_SOURCES = commands/boot.c commands/cat.c commands/cmp.c	\
+grub_emu_SOURCES = commands/boot.c commands/boot_normal.c commands/cat.c commands/cmp.c	\
 	commands/configfile.c commands/echo.c commands/help.c		\
 	commands/terminal.c commands/ls.c commands/test.c 		\
 	commands/search.c commands/blocklist.c commands/hexdump.c	\
-	lib/hexdump.c commands/i386/pc/halt.c commands/reboot.c		\
+	lib/hexdump.c commands/i386/pc/halt.c  commands/reboot.c		\
 	commands/i386/cpuid.c						\
 	disk/host.c disk/loopback.c					\
 	fs/fshelp.c 	\
@@ -126,7 +126,7 @@
 	kern/device.c kern/disk.c kern/dl.c kern/elf.c kern/env.c	\
 	kern/err.c							\
 	normal/execute.c kern/file.c kern/fs.c normal/lexer.c 		\
-	kern/loader.c kern/main.c kern/misc.c kern/parser.c		\
+	kern/main.c kern/misc.c kern/parser.c		\
 	grub_script.tab.c kern/partition.c kern/rescue.c kern/term.c	\
 	normal/arg.c normal/cmdline.c normal/command.c normal/function.c\
 	normal/completion.c normal/main.c normal/color.c		\
Index: kern/loader.c
===================================================================
--- kern/loader.c	(revision 1845)
+++ kern/loader.c	(working copy)
@@ -1,75 +0,0 @@
-/*
- *  GRUB  --  GRand Unified Bootloader
- *  Copyright (C) 2002,2003,2004,2006,2007  Free Software Foundation, Inc.
- *
- *  GRUB is free software: you can redistribute it and/or modify
- *  it under the terms of the GNU General Public License as published by
- *  the Free Software Foundation, either version 3 of the License, or
- *  (at your option) any later version.
- *
- *  GRUB is distributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *  GNU General Public License for more details.
- *
- *  You should have received a copy of the GNU General Public License
- *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
- */
-
-#include <grub/loader.h>
-#include <grub/misc.h>
-#include <grub/mm.h>
-#include <grub/err.h>
-#include <grub/kernel.h>
-
-static grub_err_t (*grub_loader_boot_func) (void);
-static grub_err_t (*grub_loader_unload_func) (void);
-static int grub_loader_noreturn;
-
-static int grub_loader_loaded;
-
-int
-grub_loader_is_loaded (void)
-{
-  return grub_loader_loaded;
-}
-
-void
-grub_loader_set (grub_err_t (*boot) (void),
-		 grub_err_t (*unload) (void),
-		 int noreturn)
-{
-  if (grub_loader_loaded && grub_loader_unload_func)
-    grub_loader_unload_func ();
-  
-  grub_loader_boot_func = boot;
-  grub_loader_unload_func = unload;
-  grub_loader_noreturn = noreturn;
-  
-  grub_loader_loaded = 1;
-}
-
-void
-grub_loader_unset(void)
-{
-  if (grub_loader_loaded && grub_loader_unload_func)
-    grub_loader_unload_func ();
-  
-  grub_loader_boot_func = 0;
-  grub_loader_unload_func = 0;
-
-  grub_loader_loaded = 0;
-}
-
-grub_err_t
-grub_loader_boot (void)
-{
-  if (! grub_loader_loaded)
-    return grub_error (GRUB_ERR_NO_KERNEL, "no loaded kernel");
-
-  if (grub_loader_noreturn)
-    grub_machine_fini ();
-  
-  return (grub_loader_boot_func) ();
-}
-
Index: kern/rescue.c
===================================================================
--- kern/rescue.c	(revision 1845)
+++ kern/rescue.c	(working copy)
@@ -117,14 +117,6 @@
   grub_refresh ();
 }
 
-/* boot */
-static void
-grub_rescue_cmd_boot (int argc __attribute__ ((unused)),
-		      char *argv[] __attribute__ ((unused)))
-{
-  grub_loader_boot ();
-}
-
 /* cat FILE */
 static void
 grub_rescue_cmd_cat (int argc, char *argv[])
@@ -620,8 +612,6 @@
 
   grub_printf ("Entering rescue mode...\n");
   
-  grub_rescue_register_command ("boot", grub_rescue_cmd_boot,
-				"boot an operating system");
   grub_rescue_register_command ("cat", grub_rescue_cmd_cat,
 				"show the contents of a file");
   grub_rescue_register_command ("help", grub_rescue_cmd_help,
Index: include/grub/loader.h
===================================================================
--- include/grub/loader.h	(revision 1845)
+++ include/grub/loader.h	(working copy)
@@ -25,20 +25,26 @@
 #include <grub/err.h>
 #include <grub/types.h>
 
+
 /* Check if a loader is loaded.  */
-int EXPORT_FUNC(grub_loader_is_loaded) (void);
+int grub_loader_is_loaded (void);
 
 /* Set loader functions. NORETURN must be set to true, if BOOT won't return
    to the original state.  */
-void EXPORT_FUNC(grub_loader_set) (grub_err_t (*boot) (void),
+void grub_loader_set (grub_err_t (*boot) (void),
 				   grub_err_t (*unload) (void),
 				   int noreturn);
 
 /* Unset current loader, if any.  */
-void EXPORT_FUNC(grub_loader_unset) (void);
+void grub_loader_unset (void);
 
-/* Call the boot hook in current loader. This may or may not return,
-   depending on the setting by grub_loader_set.  */
-grub_err_t EXPORT_FUNC(grub_loader_boot) (void);
+/*Add a preboot function*/
+void *grub_loader_add_preboot (grub_err_t (*preboot_func) (int noreturn));
 
+/*Remove given preboot function*/
+void grub_loader_remove_preboot (void *hnd);
+
+void grub_rescue_cmd_boot (int argc, char *argv[]);
+
+
 #endif /* ! GRUB_LOADER_HEADER */
Index: include/grub/kernel.h
===================================================================
--- include/grub/kernel.h	(revision 1845)
+++ include/grub/kernel.h	(working copy)
@@ -64,7 +64,7 @@
 void grub_machine_init (void);
 
 /* The machine-specific finalization.  */
-void grub_machine_fini (void);
+void EXPORT_FUNC(grub_machine_fini) (void);
 
 /* The machine-specific prefix initialization.  */
 void grub_machine_set_prefix (void);
Index: commands/boot.c
===================================================================
--- commands/boot.c	(revision 1845)
+++ commands/boot.c	(working copy)
@@ -17,34 +17,140 @@
  *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include <grub/normal.h>
+#include <grub/loader.h>
+#include <grub/misc.h>
+#include <grub/mm.h>
+#include <grub/err.h>
+#include <grub/kernel.h>
+#include <grub/rescue.h>
 #include <grub/dl.h>
-#include <grub/arg.h>
-#include <grub/misc.h>
-#include <grub/loader.h>
 
-static grub_err_t
-grub_cmd_boot (struct grub_arg_list *state __attribute__ ((unused)),
-	       int argc, char **args __attribute__ ((unused)))
+struct grub_loader_preboot_t
 {
-  if (argc)
-    return grub_error (GRUB_ERR_BAD_ARGUMENT, "too many arguments");
+  struct grub_loader_preboot_t *prev;
+  struct grub_loader_preboot_t *next;
+  grub_err_t (*preboot_func) (int noreturn);
+};
+
+static grub_dl_t my_mod;
+
+static grub_err_t (*grub_loader_boot_func) (void);
+static grub_err_t (*grub_loader_unload_func) (void);
+static int grub_loader_noreturn;
+static int grub_loader_loaded;
+static struct grub_loader_preboot_t *grub_loader_preboots=0;
+
+void *
+grub_loader_add_preboot (grub_err_t (*preboot_func) (int noreturn))
+{
+  struct grub_loader_preboot_t *cur;
+
+  if (!preboot_func)
+    return 0;
+
+  cur=(struct grub_loader_preboot_t *)
+    grub_malloc (sizeof (struct grub_loader_preboot_t));
+  if (!cur)
+    {
+      grub_error (GRUB_ERR_OUT_OF_MEMORY, "hook not added");
+      return 0;
+    }
+
+  cur->preboot_func=preboot_func;
+  cur->prev=0;
+  if (grub_loader_preboots)
+    {
+      cur->next=grub_loader_preboots;
+      cur->next->prev=cur;
+    }
+  else
+    cur->next=0;    
+  grub_loader_preboots=cur;
+
+  return cur;
+}
+
+void 
+grub_loader_remove_preboot (void *hnd)
+{
+  struct grub_loader_preboot_t *cur=(struct grub_loader_preboot_t *)hnd;
+
+  if (cur->next)
+    cur->next->prev=cur->prev;
+
+  if (cur->prev)
+    cur->prev->next=cur->next;
+  else
+    grub_loader_preboots=cur->next;
+
+  grub_free (cur);
+}
+
+int
+grub_loader_is_loaded (void)
+{
+  return grub_loader_loaded;
+}
+
+void
+grub_loader_set (grub_err_t (*boot) (void),
+		 grub_err_t (*unload) (void),
+		 int noreturn)
+{
+  if (grub_loader_loaded && grub_loader_unload_func)
+    grub_loader_unload_func ();
   
-  grub_loader_boot ();
+  grub_loader_boot_func = boot;
+  grub_loader_unload_func = unload;
+  grub_loader_noreturn = noreturn;
   
-  return 0;
+  grub_loader_loaded = 1;
 }
 
-\f
+void
+grub_loader_unset(void)
+{
+  if (grub_loader_loaded && grub_loader_unload_func)
+    grub_loader_unload_func ();
+  
+  grub_loader_boot_func = 0;
+  grub_loader_unload_func = 0;
 
-GRUB_MOD_INIT(boot)
+  grub_loader_loaded = 0;
+}
+
+
+void
+grub_rescue_cmd_boot (int argc __attribute__ ((unused)), 
+		      char *argv[] __attribute__ ((unused)))
 {
-  (void) mod;			/* To stop warning. */
-  grub_register_command ("boot", grub_cmd_boot, GRUB_COMMAND_FLAG_BOTH,
-			 "boot", "Boot an operating system.", 0);
+  struct grub_loader_preboot_t *cur;
+  if (! grub_loader_loaded)
+    {
+      grub_error (GRUB_ERR_NO_KERNEL, "no loaded kernel");
+      return;
+    }
+
+  if (grub_loader_noreturn)
+    grub_machine_fini ();
+
+  for (cur=grub_loader_preboots; cur; cur=cur->next)
+    if (cur->preboot_func)
+      cur->preboot_func (grub_loader_noreturn);
+  
+  (grub_loader_boot_func) ();
 }
 
-GRUB_MOD_FINI(boot)
+
+GRUB_MOD_INIT(_boot)
 {
-  grub_unregister_command ("boot");
+  grub_rescue_register_command ("boot",
+				grub_rescue_cmd_boot,
+				"boot OS");
+  my_mod = mod;
 }
+
+GRUB_MOD_FINI(_boot)
+{
+  grub_rescue_unregister_command ("boot");
+}

Re: [PATCH] Move kern/loader.c to boot.mod and add preboot_support (was Re: Sendkey patch)

[Javier Martín]

[-- Attachment #1: Type: text/plain, Size: 2074 bytes --]

El vie, 05-09-2008 a las 18:13 +0200, phcoder escribió:
> Hello. As I said in another email there is no need for it. I send a
> patch for it.
Well, I won't deny the patch is clever and looks good (I haven't tested
it yet). However, taking the generic "boot" command out of kernel and
leaving the several loader-specific assembly routines (which would be
the parts needing relocation) there is as elegant as vogon poetry.
However, if there is no opposition and your patch works fine - I'm about
to test it right now - we can first commit it (thus making core.img
smaller) and then work on getting the loader asm routines out of kernel.

-Habbit

> Vladimir 'phcoder' Serbinenko
> Javier Martín wrote:
> > El mié, 03-09-2008 a las 20:53 +0300, Vesa Jääskeläinen escribió:
> >> phcoder wrote:
> >>> Hello. In this case we can transfer the whole functionality located in
> >>> kern/loader.c to a dedicated module boot.mod. This module will also
> >>> register "boot" command. In this way the encapsulation won't be broken
> >>> and kernel will become even smaller.
> >> Remember that realmode code needs to reside below < 1 MiB. That is the
> >> reason realmode code is embedded in kernel. In there you only see jumps
> >> to RM and back to PM.
> > We could use the relocator functionality that was once discussed here (I
> > don't know if it was finally committed) so that modules could declare
> > "bundles" of code+data to be deployed to RM area. Or make sure every
> > single instruction in there uses 32-bit addressing, together with
> > artificial EIP-relativization of addresses like in drivemap_int13.S
> > 
> > -Habbit
> > 
> > 
> > ------------------------------------------------------------------------
> > 
> > _______________________________________________
> > Grub-devel mailing list
> > Grub-devel@gnu.org
> > http://lists.gnu.org/mailman/listinfo/grub-devel
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel

[-- Attachment #2: Esta parte del mensaje está firmada digitalmente --]
[-- Type: application/pgp-signature, Size: 827 bytes --]

Re: [PATCH] Move kern/loader.c to boot.mod and add preboot_support (was Re: Sendkey patch)

[Vesa Jääskeläinen]

phcoder wrote:
> Hello. As I said in another email there is no need for it. I send a
> patch for it.

Doesn't this break our rescue mode ?

Re: [PATCH] Move kern/loader.c to boot.mod and add preboot_support (was Re: Sendkey patch)

[Javier Martín]

[-- Attachment #1: Type: text/plain, Size: 469 bytes --]

El lun, 08-09-2008 a las 22:48 +0300, Vesa Jääskeläinen escribió:
> phcoder wrote:
> > Hello. As I said in another email there is no need for it. I send a
> > patch for it.
> 
> Doesn't this break our rescue mode ?
How would it? As with other commands, there would be two versions of it,
one for rescue and one for normal. Besides, what is the point in having
the "boot" command in kernel -and thus always available- if you can't
load a kernel to boot?


[-- Attachment #2: Esta parte del mensaje está firmada digitalmente --]
[-- Type: application/pgp-signature, Size: 827 bytes --]

Re: [PATCH] Move kern/loader.c to boot.mod and add preboot_support (was Re: Sendkey patch)

[Vesa Jääskeläinen]

Javier Martín wrote:
> El lun, 08-09-2008 a las 22:48 +0300, Vesa Jääskeläinen escribió:
>> phcoder wrote:
>>> Hello. As I said in another email there is no need for it. I send a
>>> patch for it.
>> Doesn't this break our rescue mode ?
> How would it? As with other commands, there would be two versions of it,
> one for rescue and one for normal. Besides, what is the point in having
> the "boot" command in kernel -and thus always available- if you can't
> load a kernel to boot?

You may want to chainload something?

Re: [PATCH] Move kern/loader.c to boot.mod and add preboot_support (was Re: Sendkey patch)

[Javier Martín]

[-- Attachment #1: Type: text/plain, Size: 829 bytes --]

El lun, 08-09-2008 a las 23:25 +0300, Vesa Jääskeläinen escribió:
> Javier Martín wrote:
> > El lun, 08-09-2008 a las 22:48 +0300, Vesa Jääskeläinen escribió:
> >> phcoder wrote:
> >>> Hello. As I said in another email there is no need for it. I send a
> >>> patch for it.
> >> Doesn't this break our rescue mode ?
> > How would it? As with other commands, there would be two versions of it,
> > one for rescue and one for normal. Besides, what is the point in having
> > the "boot" command in kernel -and thus always available- if you can't
> > load a kernel to boot?
> 
> You may want to chainload something?
Isn't "chainloader" also a command external to the kernel? I recall
seeing _chain.mod (rescue) and chain.mod (normal). So, as long as those
don't go with core.img, you won't chainload a thing.

[-- Attachment #2: Esta parte del mensaje está firmada digitalmente --]
[-- Type: application/pgp-signature, Size: 827 bytes --]

Re: [PATCH] Move kern/loader.c to boot.mod and add preboot_support (was Re: Sendkey patch)

[phcoder]

Hello,all. I was busy studying so wasn't watching the list. Is there a 
particular reason why my patch still isn't incorporated?
Thanks
phcoder
Javier Martín wrote:
> El lun, 08-09-2008 a las 23:25 +0300, Vesa Jääskeläinen escribió:
>> Javier Martín wrote:
>>> El lun, 08-09-2008 a las 22:48 +0300, Vesa Jääskeläinen escribió:
>>>> phcoder wrote:
>>>>> Hello. As I said in another email there is no need for it. I send a
>>>>> patch for it.
>>>> Doesn't this break our rescue mode ?
>>> How would it? As with other commands, there would be two versions of it,
>>> one for rescue and one for normal. Besides, what is the point in having
>>> the "boot" command in kernel -and thus always available- if you can't
>>> load a kernel to boot?
>> You may want to chainload something?
> Isn't "chainloader" also a command external to the kernel? I recall
> seeing _chain.mod (rescue) and chain.mod (normal). So, as long as those
> don't go with core.img, you won't chainload a thing.
> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Move kern/loader.c to boot.mod and add preboot_support (was Re: Sendkey patch)

[Vesa Jääskeläinen]

phcoder wrote:
> Hello,all. I was busy studying so wasn't watching the list. Is there a
> particular reason why my patch still isn't incorporated?

Perhaps we didn't like the move of loader/boot core functionality out of
kernel?

And I do not remember you describing in detail about interfaces and
functional design of your idea.

Tip: use threaded view to browse messages.

Re: [PATCH] Move kern/loader.c to boot.mod and add preboot_support (was Re: Sendkey patch)

[phcoder]

I did. The discussion seems to simply have died out without any official 
response
Thanks
Vladimir 'phcoder' Serbinenko
Vesa Jääskeläinen wrote:
> phcoder wrote:
>> Hello,all. I was busy studying so wasn't watching the list. Is there a
>> particular reason why my patch still isn't incorporated?
> 
> Perhaps we didn't like the move of loader/boot core functionality out of
> kernel?
> 
> And I do not remember you describing in detail about interfaces and
> functional design of your idea.
> 
> Tip: use threaded view to browse messages.
> 
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Move kern/loader.c to boot.mod and add preboot_support (was Re: Sendkey patch)

[Robert Millan]

On Mon, Sep 08, 2008 at 10:59:02PM +0200, Javier Martín wrote:
> El lun, 08-09-2008 a las 23:25 +0300, Vesa Jääskeläinen escribió:
> > Javier Martín wrote:
> > > El lun, 08-09-2008 a las 22:48 +0300, Vesa Jääskeläinen escribió:
> > >> phcoder wrote:
> > >>> Hello. As I said in another email there is no need for it. I send a
> > >>> patch for it.
> > >> Doesn't this break our rescue mode ?
> > > How would it? As with other commands, there would be two versions of it,
> > > one for rescue and one for normal. Besides, what is the point in having
> > > the "boot" command in kernel -and thus always available- if you can't
> > > load a kernel to boot?
> > 
> > You may want to chainload something?
> Isn't "chainloader" also a command external to the kernel? I recall
> seeing _chain.mod (rescue) and chain.mod (normal). So, as long as those
> don't go with core.img, you won't chainload a thing.

It is.  We just happen to load it by default on i386-pc.

-- 
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."

Re: [PATCH] Move kern/loader.c to boot.mod and add preboot_support (was Re: Sendkey patch)

[Robert Millan]

On Mon, Sep 08, 2008 at 10:11:33PM +0200, Javier Martín wrote:
> El lun, 08-09-2008 a las 22:48 +0300, Vesa Jääskeläinen escribió:
> > phcoder wrote:
> > > Hello. As I said in another email there is no need for it. I send a
> > > patch for it.
> > 
> > Doesn't this break our rescue mode ?
> How would it? As with other commands, there would be two versions of it,
> one for rescue and one for normal. Besides, what is the point in having
> the "boot" command in kernel -and thus always available- if you can't
> load a kernel to boot?

Vladimir, you're proposing an important change because it modifies the goals
of the rescue mode, but I see there's neither significant support for or
against your proposal.

I think it would help if you provide numbers on how much size do kernel and
boot.mod have before and after the proposed change (on i386-pc).

-- 
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."

Re: [PATCH] Move kern/loader.c to boot.mod and add preboot_support (was Re: Sendkey patch)

[phcoder]

[-- Attachment #1: Type: text/plain, Size: 1492 bytes --]

A version with biosdisk, pc and ext2
before: 24800
after: 24687
But another matter is expandability - preboot hooks are important. 
Without them implementation of sendkey or map is impossible. And adding 
preboot hooks to kernel would increase its size by 150-200 bytes.
Also I don't see how this change would prevent integrating e.g. _linux 
in the core. But modules for loading kernels are often reltively big 
since they have to treat multiple versions of boot protocol.
It also seems that some parts of my patch were missing. I send a 
completed version.
Thanks
Vladimir 'phcoder' Serbinenko
Robert Millan wrote:
> On Mon, Sep 08, 2008 at 10:11:33PM +0200, Javier Martín wrote:
>> El lun, 08-09-2008 a las 22:48 +0300, Vesa Jääskeläinen escribió:
>>> phcoder wrote:
>>>> Hello. As I said in another email there is no need for it. I send a
>>>> patch for it.
>>> Doesn't this break our rescue mode ?
>> How would it? As with other commands, there would be two versions of it,
>> one for rescue and one for normal. Besides, what is the point in having
>> the "boot" command in kernel -and thus always available- if you can't
>> load a kernel to boot?
> 
> Vladimir, you're proposing an important change because it modifies the goals
> of the rescue mode, but I see there's neither significant support for or
> against your proposal.
> 
> I think it would help if you provide numbers on how much size do kernel and
> boot.mod have before and after the proposed change (on i386-pc).
> 


[-- Attachment #2: boot.patch --]
[-- Type: text/x-diff, Size: 22327 bytes --]

Index: conf/common.rmk
===================================================================
--- conf/common.rmk	(revision 1967)
+++ conf/common.rmk	(working copy)
@@ -325,7 +325,7 @@
 scsi_mod_LDFLAGS = $(COMMON_LDFLAGS)
 
 # Commands.
-pkglib_MODULES += hello.mod boot.mod terminal.mod ls.mod	\
+pkglib_MODULES += hello.mod _boot.mod boot.mod terminal.mod ls.mod	\
 	cmp.mod cat.mod help.mod search.mod					\
 	loopback.mod fs_uuid.mod configfile.mod echo.mod	\
 	terminfo.mod test.mod blocklist.mod hexdump.mod		\
@@ -336,8 +336,13 @@
 hello_mod_CFLAGS = $(COMMON_CFLAGS)
 hello_mod_LDFLAGS = $(COMMON_LDFLAGS)
 
+# For _boot.mod.
+_boot_mod_SOURCES = commands/boot.c
+_boot_mod_CFLAGS = $(COMMON_CFLAGS)
+_boot_mod_LDFLAGS = $(COMMON_LDFLAGS)
+
 # For boot.mod.
-boot_mod_SOURCES = commands/boot.c
+boot_mod_SOURCES = commands/boot_normal.c
 boot_mod_CFLAGS = $(COMMON_CFLAGS)
 boot_mod_LDFLAGS = $(COMMON_LDFLAGS)
 
Index: conf/i386-pc.rmk
===================================================================
--- conf/i386-pc.rmk	(revision 1967)
+++ conf/i386-pc.rmk	(working copy)
@@ -41,7 +41,7 @@
 # For kernel.img.
 kernel_img_SOURCES = kern/i386/pc/startup.S kern/main.c kern/device.c \
 	kern/disk.c kern/dl.c kern/file.c kern/fs.c kern/err.c \
-	kern/misc.c kern/mm.c kern/loader.c kern/rescue.c kern/term.c \
+	kern/misc.c kern/mm.c kern/rescue.c kern/term.c \
 	kern/time.c \
 	kern/i386/dl.c kern/i386/pc/init.c kern/i386/pc/mmap.c \
 	kern/parser.c kern/partition.c \
@@ -52,7 +52,7 @@
 	term/i386/pc/console.c term/i386/vga_common.c \
 	symlist.c
 kernel_img_HEADERS = arg.h boot.h cache.h device.h disk.h dl.h elf.h elfload.h \
-	env.h err.h file.h fs.h kernel.h loader.h misc.h mm.h net.h parser.h \
+	env.h err.h file.h fs.h kernel.h misc.h mm.h net.h parser.h \
 	partition.h pc_partition.h rescue.h symbol.h term.h time.h types.h \
 	machine/biosdisk.h machine/boot.h machine/console.h machine/init.h \
 	machine/memory.h machine/loader.h machine/vga.h machine/vbe.h \
@@ -112,7 +112,7 @@
 
 # For grub-emu.
 util/grub-emu.c_DEPENDENCIES = grub_emu_init.h
-grub_emu_SOURCES = commands/boot.c commands/cat.c commands/cmp.c	\
+grub_emu_SOURCES = commands/boot.c commands/boot_normal.c commands/cat.c commands/cmp.c	\
 	commands/configfile.c commands/echo.c commands/help.c		\
 	commands/terminal.c commands/ls.c commands/test.c 		\
 	commands/search.c commands/blocklist.c commands/hexdump.c	\
@@ -125,7 +125,7 @@
 	kern/device.c kern/disk.c kern/dl.c kern/elf.c kern/env.c	\
 	kern/err.c							\
 	normal/execute.c kern/file.c kern/fs.c normal/lexer.c 		\
-	kern/loader.c kern/main.c kern/misc.c kern/parser.c		\
+	kern/main.c kern/misc.c kern/parser.c		\
 	grub_script.tab.c kern/partition.c kern/rescue.c kern/term.c	\
 	normal/arg.c normal/cmdline.c normal/command.c normal/function.c\
 	normal/completion.c normal/main.c normal/color.c		\
Index: conf/i386-efi.rmk
===================================================================
--- conf/i386-efi.rmk	(revision 1967)
+++ conf/i386-efi.rmk	(working copy)
@@ -49,7 +49,7 @@
 	kern/device.c kern/disk.c kern/dl.c kern/elf.c kern/env.c	\
 	kern/err.c							\
 	normal/execute.c kern/file.c kern/fs.c normal/lexer.c 		\
-	kern/loader.c kern/main.c kern/misc.c kern/parser.c		\
+	kern/main.c kern/misc.c kern/parser.c		\
 	grub_script.tab.c kern/partition.c kern/rescue.c kern/term.c	\
 	normal/arg.c normal/cmdline.c normal/command.c normal/function.c\
 	normal/completion.c normal/context.c normal/main.c		\
@@ -82,7 +82,7 @@
 kernel_mod_EXPORTS = no
 kernel_mod_SOURCES = kern/i386/efi/startup.S kern/main.c kern/device.c \
 	kern/disk.c kern/dl.c kern/file.c kern/fs.c kern/err.c \
-	kern/misc.c kern/mm.c kern/loader.c kern/rescue.c kern/term.c \
+	kern/misc.c kern/mm.c kern/rescue.c kern/term.c \
 	kern/i386/dl.c kern/i386/efi/init.c kern/parser.c kern/partition.c \
 	kern/env.c symlist.c kern/efi/efi.c kern/efi/init.c kern/efi/mm.c \
 	term/efi/console.c disk/efi/efidisk.c \
@@ -91,7 +91,7 @@
 	kern/generic/rtc_get_time_ms.c \
 	kern/generic/millisleep.c
 kernel_mod_HEADERS = arg.h boot.h cache.h device.h disk.h dl.h elf.h elfload.h \
-	env.h err.h file.h fs.h kernel.h loader.h misc.h mm.h net.h parser.h \
+	env.h err.h file.h fs.h kernel.h misc.h mm.h net.h parser.h \
 	partition.h pc_partition.h rescue.h symbol.h term.h time.h types.h \
 	efi/efi.h efi/time.h efi/disk.h
 kernel_mod_CFLAGS = $(COMMON_CFLAGS)
Index: conf/x86_64-efi.rmk
===================================================================
--- conf/x86_64-efi.rmk	(revision 1967)
+++ conf/x86_64-efi.rmk	(working copy)
@@ -51,7 +51,7 @@
 	kern/device.c kern/disk.c kern/dl.c kern/elf.c kern/env.c	\
 	kern/err.c							\
 	normal/execute.c kern/file.c kern/fs.c normal/lexer.c 		\
-	kern/loader.c kern/main.c kern/misc.c kern/parser.c		\
+	kern/main.c kern/misc.c kern/parser.c		\
 	grub_script.tab.c kern/partition.c kern/rescue.c kern/term.c	\
 	normal/arg.c normal/cmdline.c normal/command.c normal/function.c\
 	normal/completion.c normal/context.c normal/main.c		\
@@ -85,7 +85,7 @@
 kernel_mod_SOURCES = kern/x86_64/efi/startup.S kern/x86_64/efi/callwrap.S \
 	kern/main.c kern/device.c \
 	kern/disk.c kern/dl.c kern/file.c kern/fs.c kern/err.c \
-	kern/misc.c kern/mm.c kern/loader.c kern/rescue.c kern/term.c \
+	kern/misc.c kern/mm.c kern/rescue.c kern/term.c \
 	kern/x86_64/dl.c kern/i386/efi/init.c kern/parser.c kern/partition.c \
 	kern/env.c symlist.c kern/efi/efi.c kern/efi/init.c kern/efi/mm.c \
 	kern/time.c \
@@ -93,7 +93,7 @@
 	kern/generic/millisleep.c kern/generic/rtc_get_time_ms.c \
 	term/efi/console.c disk/efi/efidisk.c
 kernel_mod_HEADERS = arg.h boot.h cache.h device.h disk.h dl.h elf.h elfload.h \
-	env.h err.h file.h fs.h kernel.h loader.h misc.h mm.h net.h parser.h \
+	env.h err.h file.h fs.h kernel.h misc.h mm.h net.h parser.h \
 	partition.h pc_partition.h rescue.h symbol.h term.h time.h types.h \
 	efi/efi.h efi/time.h efi/disk.h machine/loader.h
 kernel_mod_CFLAGS = $(COMMON_CFLAGS)
Index: conf/powerpc-ieee1275.rmk
===================================================================
--- conf/powerpc-ieee1275.rmk	(revision 1967)
+++ conf/powerpc-ieee1275.rmk	(working copy)
@@ -15,7 +15,7 @@
 
 kernel_elf_HEADERS = arg.h boot.h cache.h device.h disk.h dl.h elf.h elfload.h \
 	env.h err.h file.h fs.h kernel.h misc.h mm.h net.h parser.h rescue.h \
-	symbol.h term.h time.h types.h powerpc/libgcc.h loader.h partition.h \
+	symbol.h term.h time.h types.h powerpc/libgcc.h partition.h \
 	pc_partition.h ieee1275/ieee1275.h machine/kernel.h
 
 symlist.c: $(addprefix include/grub/,$(kernel_elf_HEADERS)) config.h gensymlist.sh
@@ -52,7 +52,7 @@
 	\
 	io/gzio.c							\
 	kern/device.c kern/disk.c kern/dl.c kern/elf.c kern/env.c	\
-	kern/err.c kern/file.c kern/fs.c kern/loader.c kern/main.c	\
+	kern/err.c kern/file.c kern/fs.c kern/main.c	\
 	kern/misc.c kern/parser.c kern/partition.c kern/rescue.c	\
 	kern/term.c fs/fshelp.c						\
 	normal/arg.c normal/cmdline.c normal/command.c			\
@@ -75,7 +75,7 @@
 kernel_elf_SOURCES = kern/powerpc/ieee1275/startup.S kern/ieee1275/cmain.c \
 	kern/ieee1275/ieee1275.c kern/main.c kern/device.c 		\
 	kern/disk.c kern/dl.c kern/err.c kern/file.c kern/fs.c 		\
-	kern/misc.c kern/mm.c kern/loader.c kern/rescue.c kern/term.c 	\
+	kern/misc.c kern/mm.c kern/rescue.c kern/term.c 	\
 	kern/ieee1275/init.c 						\
 	kern/ieee1275/mmap.c						\
 	term/ieee1275/ofconsole.c 		\
Index: conf/i386-coreboot.rmk
===================================================================
--- conf/i386-coreboot.rmk	(revision 1967)
+++ conf/i386-coreboot.rmk	(working copy)
@@ -16,7 +16,7 @@
 	kern/i386/multiboot_mmap.c \
 	kern/main.c kern/device.c \
 	kern/disk.c kern/dl.c kern/file.c kern/fs.c kern/err.c \
-	kern/misc.c kern/mm.c kern/loader.c kern/rescue.c kern/term.c \
+	kern/misc.c kern/mm.c kern/rescue.c kern/term.c \
 	kern/time.c \
 	kern/i386/dl.c kern/parser.c kern/partition.c \
 	kern/i386/tsc.c kern/i386/pit.c \
@@ -27,7 +27,7 @@
 	term/i386/pc/at_keyboard.c \
 	symlist.c
 kernel_elf_HEADERS = arg.h boot.h cache.h device.h disk.h dl.h elf.h elfload.h \
-	env.h err.h file.h fs.h kernel.h loader.h misc.h mm.h net.h parser.h \
+	env.h err.h file.h fs.h kernel.h misc.h mm.h net.h parser.h \
 	partition.h pc_partition.h rescue.h symbol.h term.h time.h types.h \
 	machine/boot.h machine/console.h machine/init.h \
 	machine/memory.h machine/loader.h
Index: conf/sparc64-ieee1275.rmk
===================================================================
--- conf/sparc64-ieee1275.rmk	(revision 1967)
+++ conf/sparc64-ieee1275.rmk	(working copy)
@@ -15,7 +15,7 @@
 
 kernel_elf_HEADERS = arg.h boot.h cache.h device.h disk.h dl.h elf.h elfload.h \
 	env.h err.h file.h fs.h kernel.h misc.h mm.h net.h parser.h rescue.h \
-	symbol.h term.h time.h types.h sparc64/libgcc.h loader.h partition.h \
+	symbol.h term.h time.h types.h sparc64/libgcc.h partition.h \
 	pc_partition.h ieee1275/ieee1275.h machine/kernel.h
 
 symlist.c: $(addprefix include/grub/,$(kernel_elf_HEADERS)) config.h gensymlist.sh
@@ -53,7 +53,7 @@
 #	grub_script.tab.c						\
 #	io/gzio.c                                                       \
 #	kern/device.c kern/disk.c kern/dl.c kern/env.c kern/err.c 	\
-#	kern/file.c kern/fs.c kern/loader.c kern/main.c kern/misc.c	\
+#	kern/file.c kern/fs.c kern/main.c kern/misc.c	\
 #	kern/parser.c kern/partition.c kern/rescue.c kern/term.c	\
 #	normal/arg.c normal/cmdline.c normal/command.c			\
 #	normal/completion.c normal/context.c normal/execute.c		\
@@ -69,7 +69,7 @@
 
 kernel_elf_SOURCES = kern/sparc64/ieee1275/init.c kern/ieee1275/ieee1275.c \
 	kern/main.c kern/device.c kern/disk.c kern/dl.c kern/file.c \
-	kern/fs.c kern/err.c kern/misc.c kern/mm.c kern/loader.c \
+	kern/fs.c kern/err.c kern/misc.c kern/mm.c \
 	kern/rescue.c kern/term.c term/ieee1275/ofconsole.c \
 	kern/sparc64/ieee1275/openfw.c disk/ieee1275/ofdisk.c \
 	kern/partition.c kern/env.c kern/sparc64/dl.c symlist.c \
Index: conf/i386-ieee1275.rmk
===================================================================
--- conf/i386-ieee1275.rmk	(revision 1967)
+++ conf/i386-ieee1275.rmk	(working copy)
@@ -17,7 +17,7 @@
 	kern/ieee1275/cmain.c kern/ieee1275/openfw.c \
 	kern/main.c kern/device.c \
 	kern/disk.c kern/dl.c kern/file.c kern/fs.c kern/err.c \
-	kern/misc.c kern/mm.c kern/loader.c kern/rescue.c kern/term.c \
+	kern/misc.c kern/mm.c kern/rescue.c kern/term.c \
 	kern/i386/dl.c kern/parser.c kern/partition.c \
 	kern/env.c \
 	kern/time.c \
@@ -27,7 +27,7 @@
 	disk/ieee1275/ofdisk.c \
 	symlist.c
 kernel_elf_HEADERS = arg.h cache.h device.h disk.h dl.h elf.h elfload.h \
-	env.h err.h file.h fs.h kernel.h loader.h misc.h mm.h net.h parser.h \
+	env.h err.h file.h fs.h kernel.h misc.h mm.h net.h parser.h \
 	partition.h pc_partition.h rescue.h symbol.h term.h time.h types.h \
 	ieee1275/ieee1275.h machine/kernel.h machine/loader.h machine/memory.h
 kernel_elf_CFLAGS = $(COMMON_CFLAGS)
@@ -71,7 +71,7 @@
 	kern/device.c kern/disk.c kern/dl.c kern/elf.c kern/env.c	\
 	kern/err.c							\
 	normal/execute.c kern/file.c kern/fs.c normal/lexer.c 		\
-	kern/loader.c kern/main.c kern/misc.c kern/parser.c		\
+	kern/main.c kern/misc.c kern/parser.c		\
 	grub_script.tab.c kern/partition.c kern/rescue.c kern/term.c	\
 	normal/arg.c normal/cmdline.c normal/command.c normal/function.c\
 	normal/completion.c normal/main.c				\
Index: kern/loader.c
===================================================================
--- kern/loader.c	(revision 1967)
+++ kern/loader.c	(working copy)
@@ -1,75 +0,0 @@
-/*
- *  GRUB  --  GRand Unified Bootloader
- *  Copyright (C) 2002,2003,2004,2006,2007  Free Software Foundation, Inc.
- *
- *  GRUB is free software: you can redistribute it and/or modify
- *  it under the terms of the GNU General Public License as published by
- *  the Free Software Foundation, either version 3 of the License, or
- *  (at your option) any later version.
- *
- *  GRUB is distributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *  GNU General Public License for more details.
- *
- *  You should have received a copy of the GNU General Public License
- *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
- */
-
-#include <grub/loader.h>
-#include <grub/misc.h>
-#include <grub/mm.h>
-#include <grub/err.h>
-#include <grub/kernel.h>
-
-static grub_err_t (*grub_loader_boot_func) (void);
-static grub_err_t (*grub_loader_unload_func) (void);
-static int grub_loader_noreturn;
-
-static int grub_loader_loaded;
-
-int
-grub_loader_is_loaded (void)
-{
-  return grub_loader_loaded;
-}
-
-void
-grub_loader_set (grub_err_t (*boot) (void),
-		 grub_err_t (*unload) (void),
-		 int noreturn)
-{
-  if (grub_loader_loaded && grub_loader_unload_func)
-    grub_loader_unload_func ();
-  
-  grub_loader_boot_func = boot;
-  grub_loader_unload_func = unload;
-  grub_loader_noreturn = noreturn;
-  
-  grub_loader_loaded = 1;
-}
-
-void
-grub_loader_unset(void)
-{
-  if (grub_loader_loaded && grub_loader_unload_func)
-    grub_loader_unload_func ();
-  
-  grub_loader_boot_func = 0;
-  grub_loader_unload_func = 0;
-
-  grub_loader_loaded = 0;
-}
-
-grub_err_t
-grub_loader_boot (void)
-{
-  if (! grub_loader_loaded)
-    return grub_error (GRUB_ERR_NO_KERNEL, "no loaded kernel");
-
-  if (grub_loader_noreturn)
-    grub_machine_fini ();
-  
-  return (grub_loader_boot_func) ();
-}
-
Index: kern/rescue.c
===================================================================
--- kern/rescue.c	(revision 1967)
+++ kern/rescue.c	(working copy)
@@ -117,14 +117,6 @@
   grub_refresh ();
 }
 
-/* boot */
-static void
-grub_rescue_cmd_boot (int argc __attribute__ ((unused)),
-		      char *argv[] __attribute__ ((unused)))
-{
-  grub_loader_boot ();
-}
-
 /* cat FILE */
 static void
 grub_rescue_cmd_cat (int argc, char *argv[])
@@ -620,8 +612,6 @@
 
   grub_printf ("Entering rescue mode...\n");
   
-  grub_rescue_register_command ("boot", grub_rescue_cmd_boot,
-				"boot an operating system");
   grub_rescue_register_command ("cat", grub_rescue_cmd_cat,
 				"show the contents of a file");
   grub_rescue_register_command ("help", grub_rescue_cmd_help,
Index: include/grub/loader.h
===================================================================
--- include/grub/loader.h	(revision 1967)
+++ include/grub/loader.h	(working copy)
@@ -25,20 +25,26 @@
 #include <grub/err.h>
 #include <grub/types.h>
 
+
 /* Check if a loader is loaded.  */
-int EXPORT_FUNC(grub_loader_is_loaded) (void);
+int grub_loader_is_loaded (void);
 
 /* Set loader functions. NORETURN must be set to true, if BOOT won't return
    to the original state.  */
-void EXPORT_FUNC(grub_loader_set) (grub_err_t (*boot) (void),
+void grub_loader_set (grub_err_t (*boot) (void),
 				   grub_err_t (*unload) (void),
 				   int noreturn);
 
 /* Unset current loader, if any.  */
-void EXPORT_FUNC(grub_loader_unset) (void);
+void grub_loader_unset (void);
 
-/* Call the boot hook in current loader. This may or may not return,
-   depending on the setting by grub_loader_set.  */
-grub_err_t EXPORT_FUNC(grub_loader_boot) (void);
+/*Add a preboot function*/
+void *grub_loader_add_preboot (grub_err_t (*preboot_func) (int noreturn));
 
+/*Remove given preboot function*/
+void grub_loader_remove_preboot (void *hnd);
+
+void grub_rescue_cmd_boot (int argc, char *argv[]);
+
+
 #endif /* ! GRUB_LOADER_HEADER */
Index: include/grub/kernel.h
===================================================================
--- include/grub/kernel.h	(revision 1967)
+++ include/grub/kernel.h	(working copy)
@@ -64,7 +64,7 @@
 void grub_machine_init (void);
 
 /* The machine-specific finalization.  */
-void grub_machine_fini (void);
+void EXPORT_FUNC(grub_machine_fini) (void);
 
 /* The machine-specific prefix initialization.  */
 void grub_machine_set_prefix (void);
Index: ChangeLog
===================================================================
--- ChangeLog	(revision 1967)
+++ ChangeLog	(working copy)
@@ -1,3 +1,24 @@
+2009-02-07  Vladimir Serbinenko  <phcoder@gmail.com>
+
+	Add preboot hooks and move boot out of kernel
+	
+	* conf/common.rmk: Added _boot.mod
+	* conf/i386-pc.rmk: Removed loader.c and loader.h from 
+	kernel and grub-emu sources
+	* conf/i386-coreboot.rmk: Likewise
+	* conf/i386-efi.rmk: Likewise
+	* conf/i386-ieee1275.rmk: Likewise
+	* conf/powerpc-ieee1275.rmk: Likewise
+	* conf/sparc64-ieee1275.rmk: Likewise
+	* commands/boot.c: Moved from here...
+	* commands/boot_normal.c: ...moved here
+	* kern/loader.c: Moved to ..
+	* commands/boot.c: ... moved here
+	* commands/boot.c: preboot hook support
+	* kern/rescue.c: removed grub_rescue_cmd_boot
+	* include/grub/loader.h: removed EXPORT_FUNC
+	* include/grub/kernel.h: added EXPORT_FUNC to grub_machine_init
+
 2009-02-01  Felix Zielcke  <fzielcke@z-51.de>
 
 	* INSTALL: Note that we now require at least autconf 2.59 and that LZO
Index: commands/boot.c
===================================================================
--- commands/boot.c	(revision 1967)
+++ commands/boot.c	(working copy)
@@ -17,34 +17,140 @@
  *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include <grub/normal.h>
+#include <grub/loader.h>
+#include <grub/misc.h>
+#include <grub/mm.h>
+#include <grub/err.h>
+#include <grub/kernel.h>
+#include <grub/rescue.h>
 #include <grub/dl.h>
-#include <grub/arg.h>
-#include <grub/misc.h>
-#include <grub/loader.h>
 
-static grub_err_t
-grub_cmd_boot (struct grub_arg_list *state __attribute__ ((unused)),
-	       int argc, char **args __attribute__ ((unused)))
+struct grub_loader_preboot_t
 {
-  if (argc)
-    return grub_error (GRUB_ERR_BAD_ARGUMENT, "too many arguments");
+  struct grub_loader_preboot_t *prev;
+  struct grub_loader_preboot_t *next;
+  grub_err_t (*preboot_func) (int noreturn);
+};
+
+static grub_dl_t my_mod;
+
+static grub_err_t (*grub_loader_boot_func) (void);
+static grub_err_t (*grub_loader_unload_func) (void);
+static int grub_loader_noreturn;
+static int grub_loader_loaded;
+static struct grub_loader_preboot_t *grub_loader_preboots=0;
+
+void *
+grub_loader_add_preboot (grub_err_t (*preboot_func) (int noreturn))
+{
+  struct grub_loader_preboot_t *cur;
+
+  if (!preboot_func)
+    return 0;
+
+  cur=(struct grub_loader_preboot_t *)
+    grub_malloc (sizeof (struct grub_loader_preboot_t));
+  if (!cur)
+    {
+      grub_error (GRUB_ERR_OUT_OF_MEMORY, "hook not added");
+      return 0;
+    }
+
+  cur->preboot_func=preboot_func;
+  cur->prev=0;
+  if (grub_loader_preboots)
+    {
+      cur->next=grub_loader_preboots;
+      cur->next->prev=cur;
+    }
+  else
+    cur->next=0;    
+  grub_loader_preboots=cur;
+
+  return cur;
+}
+
+void 
+grub_loader_remove_preboot (void *hnd)
+{
+  struct grub_loader_preboot_t *cur=(struct grub_loader_preboot_t *)hnd;
+
+  if (cur->next)
+    cur->next->prev=cur->prev;
+
+  if (cur->prev)
+    cur->prev->next=cur->next;
+  else
+    grub_loader_preboots=cur->next;
+
+  grub_free (cur);
+}
+
+int
+grub_loader_is_loaded (void)
+{
+  return grub_loader_loaded;
+}
+
+void
+grub_loader_set (grub_err_t (*boot) (void),
+		 grub_err_t (*unload) (void),
+		 int noreturn)
+{
+  if (grub_loader_loaded && grub_loader_unload_func)
+    grub_loader_unload_func ();
   
-  grub_loader_boot ();
+  grub_loader_boot_func = boot;
+  grub_loader_unload_func = unload;
+  grub_loader_noreturn = noreturn;
   
-  return 0;
+  grub_loader_loaded = 1;
 }
 
-\f
+void
+grub_loader_unset(void)
+{
+  if (grub_loader_loaded && grub_loader_unload_func)
+    grub_loader_unload_func ();
+  
+  grub_loader_boot_func = 0;
+  grub_loader_unload_func = 0;
 
-GRUB_MOD_INIT(boot)
+  grub_loader_loaded = 0;
+}
+
+
+void
+grub_rescue_cmd_boot (int argc __attribute__ ((unused)), 
+		      char *argv[] __attribute__ ((unused)))
 {
-  (void) mod;			/* To stop warning. */
-  grub_register_command ("boot", grub_cmd_boot, GRUB_COMMAND_FLAG_BOTH,
-			 "boot", "Boot an operating system.", 0);
+  struct grub_loader_preboot_t *cur;
+  if (! grub_loader_loaded)
+    {
+      grub_error (GRUB_ERR_NO_KERNEL, "no loaded kernel");
+      return;
+    }
+
+  if (grub_loader_noreturn)
+    grub_machine_fini ();
+
+  for (cur=grub_loader_preboots; cur; cur=cur->next)
+    if (cur->preboot_func)
+      cur->preboot_func (grub_loader_noreturn);
+  
+  (grub_loader_boot_func) ();
 }
 
-GRUB_MOD_FINI(boot)
+
+GRUB_MOD_INIT(_boot)
 {
-  grub_unregister_command ("boot");
+  grub_rescue_register_command ("boot",
+				grub_rescue_cmd_boot,
+				"boot OS");
+  my_mod = mod;
 }
+
+GRUB_MOD_FINI(_boot)
+{
+  grub_rescue_unregister_command ("boot");
+}
Index: commands/boot_normal.c
===================================================================
--- commands/boot_normal.c	(revision 0)
+++ commands/boot_normal.c	(revision 0)
@@ -0,0 +1,48 @@
+/* boot_normal.c - wraper of boot command for normal mode */
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2003,2005,2007  Free Software Foundation, Inc.
+ *
+ *  GRUB is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <grub/normal.h>
+#include <grub/dl.h>
+#include <grub/arg.h>
+#include <grub/misc.h>
+#include <grub/loader.h>
+
+static grub_err_t
+grub_cmd_boot (struct grub_arg_list *state __attribute__ ((unused)),
+	       int argc, char **args __attribute__ ((unused)))
+{
+
+  grub_rescue_cmd_boot (argc, args);
+  
+  return grub_errno;
+}
+
+\f
+
+GRUB_MOD_INIT(boot)
+{
+  (void) mod;			/* To stop warning. */
+  grub_register_command ("boot", grub_cmd_boot, GRUB_COMMAND_FLAG_BOTH,
+			 "boot", "Boot an operating system.", 0);
+}
+
+GRUB_MOD_FINI(boot)
+{
+  grub_unregister_command ("boot");
+}