[PATCH] Improve FreeDOS direct loading support compatibility.

["C. Masloch" <pushbx@38.de>]

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

Hello,

A bit about me first; feel free to skip this paragraph. I know 86 assembly  
language well and have been involved in FreeDOS kernel development. On the  
freedos-user list I read that GRUB 2.00 added FreeDOS direct loading  
support. I have reviewed GRUB's implementation of the FreeDOS load  
protocol.

The attached patch improves compatibility to the FreeDOS kernel and its  
original loader as well as potential compatibility to other DOS-like  
kernels and loaders. In particular, some requirements of the current  
EDR-DOS load protocol are implemented by this as well (though possibly not  
all of them) without harming the loading of FreeDOS kernels.

Regards,
Chris Masloch

[-- Attachment #2: improve-freedos-compatibility.diff --]
[-- Type: application/octet-stream, Size: 7265 bytes --]

Subject: [PATCH] Improve FreeDOS direct loading support compatibility.
From: "C. Masloch" <pushbx@38.de>
Date: Sat Jun 30 06:46:08 2012 +0200

 ChangeLog                          |  15 ++++++
 grub-core/lib/i386/relocator.c     |   2 +
 grub-core/lib/i386/relocator16.S   |   5 ++
 grub-core/loader/i386/pc/freedos.c |  83 +++++++++++++++++++++++++++++++++++--
 include/grub/i386/relocator.h      |   1 +
 5 files changed, 101 insertions(+), 5 deletions(-)

# HG changeset patch
# User C. Masloch <pushbx@38.de>
# Date 1341031568 -7200
# Node ID 100d87aefb9b101d05dc614ead36cbcbcd5f4692
# Parent  cd406008554c8411969b8b5ab13e7b58d3f084a8
Improve FreeDOS direct loading support compatibility.

diff --git a/ChangeLog b/ChangeLog
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,18 @@
+2012-06-30  C. Masloch  <pushbx@38.de>
+
+	Improve FreeDOS direct loading support compatibility.
+
+	* include/grub/i386/relocator.h (grub_relocator16_state):
+	New member ebp.
+	* grub-core/lib/i386/relocator.c (grub_relocator16_ebp): New extern
+	variable.
+	(grub_relocator16_boot): Handle %ebp.
+	* grub-core/lib/i386/relocator16.S: Likewise.
+	* grub-core/loader/i386/pc/freedos.c:
+	Load BPB to pass kernel which partition to load from.
+	Check that kernel file is not too large.
+	Set register dl to BIOS unit number as well.
+
 2012-06-27  Vladimir Serbinenko  <phcoder@gmail.com>
 
 	* configure.ac: Bump version to 2.00.
diff --git a/grub-core/lib/i386/relocator.c b/grub-core/lib/i386/relocator.c
--- a/grub-core/lib/i386/relocator.c
+++ b/grub-core/lib/i386/relocator.c
@@ -54,6 +54,7 @@
 extern grub_uint32_t grub_relocator16_edx;
 extern grub_uint32_t grub_relocator16_ebx;
 extern grub_uint32_t grub_relocator16_esi;
+extern grub_uint32_t grub_relocator16_ebp;
 
 extern grub_uint16_t grub_relocator16_keep_a20_enabled;
 
@@ -225,6 +226,7 @@
   grub_relocator16_ss = state.ss;
   grub_relocator16_sp = state.sp;
 
+  grub_relocator16_ebp = state.ebp;
   grub_relocator16_ebx = state.ebx;
   grub_relocator16_edx = state.edx;
   grub_relocator16_esi = state.esi;
diff --git a/grub-core/lib/i386/relocator16.S b/grub-core/lib/i386/relocator16.S
--- a/grub-core/lib/i386/relocator16.S
+++ b/grub-core/lib/i386/relocator16.S
@@ -259,6 +259,11 @@
 VARIABLE(grub_relocator16_ebx)
 	.long	0
 
+	/* movl imm32, %ebp.  */
+	.byte	0x66, 0xbd
+VARIABLE(grub_relocator16_ebp)
+	.long	0
+
 	/* Cleared direction flag is of no problem with any current
 	   payload and makes this implementation easier.  */
 	cld
diff --git a/grub-core/loader/i386/pc/freedos.c b/grub-core/loader/i386/pc/freedos.c
--- a/grub-core/loader/i386/pc/freedos.c
+++ b/grub-core/loader/i386/pc/freedos.c
@@ -32,6 +32,7 @@
 #include <grub/video.h>
 #include <grub/mm.h>
 #include <grub/cpu/relocator.h>
+#include <grub/machine/chainloader.h>
 
 GRUB_MOD_LICENSE ("GPLv3+");
 
@@ -40,8 +41,23 @@
 static grub_uint32_t ebx = 0xffffffff;
 
 #define GRUB_FREEDOS_SEGMENT         0x60
+#define GRUB_FREEDOS_ADDR            (GRUB_FREEDOS_SEGMENT<<4)
 #define GRUB_FREEDOS_STACK_SEGMENT         0x1fe0
-#define GRUB_FREEDOS_STACK_POINTER         0x8000
+#define GRUB_FREEDOS_STACK_BPB_POINTER     0x7c00
+#define GRUB_FREEDOS_BPB_ADDR        ((GRUB_FREEDOS_STACK_SEGMENT<<4) \
+                                      +GRUB_FREEDOS_STACK_BPB_POINTER)
+
+/* FreeDOS boot.asm passes register sp as exactly this. Importantly,
+   it must point below the BPB (to avoid overwriting any of it). */
+#define GRUB_FREEDOS_STACK_POINTER         (GRUB_FREEDOS_STACK_BPB_POINTER \
+                                            -0x60)
+
+/* In this, the additional 8192 bytes are the stack reservation; the
+   remaining parts trivially give the maximum allowed size. */
+#define GRUB_FREEDOS_MAX_SIZE        ((GRUB_FREEDOS_STACK_SEGMENT<<4) \
+                                      +GRUB_FREEDOS_STACK_POINTER \
+                                      -GRUB_FREEDOS_ADDR \
+                                      -8192)
 
 static grub_err_t
 grub_freedos_boot (void)
@@ -49,14 +65,30 @@
   struct grub_relocator16_state state = { 
     .cs = GRUB_FREEDOS_SEGMENT,
     .ip = 0,
-    .ds = 0,
+
+    /* This is not strictly necessary for the current FreeDOS kernel
+       but improves potential compatibility with the current EDR-DOS
+       load protocol.
+       There is no harm in setting this. */
+    .ds = GRUB_FREEDOS_STACK_SEGMENT,
     .es = 0,
     .fs = 0,
     .gs = 0,
     .ss = GRUB_FREEDOS_STACK_SEGMENT,
     .sp = GRUB_FREEDOS_STACK_POINTER,
+    .ebp = GRUB_FREEDOS_STACK_BPB_POINTER,
     .ebx = ebx,
-    .edx = 0,
+
+    /* This is not strictly necessary for the current FreeDOS kernel
+       but is crucial for potential compatibility with the load
+       protocols of eg the current EDR-DOS kernel, or other DOS-like
+       kernels and loaders.
+       (Among those, FreeDOS's load protocol must be considered a
+       special case in that it doesn't require register dl to pass
+       the unit number. Incidentally, the current FreeDOS boot.asm
+       does pass it in both registers.)
+       There is no harm in setting this. */
+    .edx = ebx,
     .a20 = 1
   };
   grub_video_set_mode ("text", 0, 0);
@@ -79,8 +111,9 @@
 {
   grub_file_t file = 0;
   grub_err_t err;
-  void *kernelsys;
+  void *bs, *kernelsys;
   grub_size_t kernelsyssize;
+  grub_device_t dev;
 
   if (argc == 0)
     return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("filename expected"));
@@ -95,12 +128,52 @@
   if (! file)
     goto fail;
 
+  {
+    grub_relocator_chunk_t ch;
+    err = grub_relocator_alloc_chunk_addr (rel, &ch, GRUB_FREEDOS_BPB_ADDR,
+					   GRUB_DISK_SECTOR_SIZE);
+    if (err)
+      goto fail;
+    bs = get_virtual_current_address (ch);
+  }
+
   ebx = grub_get_root_biosnumber ();
+  dev = grub_device_open (0);
+
+  if (dev && dev->disk)
+    {
+      err = grub_disk_read (dev->disk, 0, 0, GRUB_DISK_SECTOR_SIZE, bs);
+      if (err)
+	{
+	  grub_device_close (dev);
+	  goto fail;
+	}
+      grub_chainloader_patch_bpb (bs, dev, ebx);
+    }
+
+  if (dev)
+    grub_device_close (dev);
 
   kernelsyssize = grub_file_size (file);
+
+  /* This check could be considered optional, but it provides a more
+     specific error message than grub_relocator_alloc_chunk_addr would,
+     and additionally it insures that a little is set aside for the
+     initial stack as well.
+     Quirkily, because of its size constraints FreeDOS's original loader
+     doesn't perform such a check at all (and crashes instead). The file
+     size limit is documented though. */
+  if (kernelsyssize > GRUB_FREEDOS_MAX_SIZE)
+    {
+      (void)grub_error (GRUB_ERR_BAD_OS,
+		        "file `%s' is too large for a valid"
+		        " FreeDOS kernel.sys", argv[0]);
+      goto fail;
+    }
+
   {
     grub_relocator_chunk_t ch;
-    err = grub_relocator_alloc_chunk_addr (rel, &ch, GRUB_FREEDOS_SEGMENT << 4,
+    err = grub_relocator_alloc_chunk_addr (rel, &ch, GRUB_FREEDOS_ADDR,
 					   kernelsyssize);
     if (err)
       goto fail;
diff --git a/include/grub/i386/relocator.h b/include/grub/i386/relocator.h
--- a/include/grub/i386/relocator.h
+++ b/include/grub/i386/relocator.h
@@ -49,6 +49,7 @@
   grub_uint32_t ebx;
   grub_uint32_t edx;
   grub_uint32_t esi;
+  grub_uint32_t ebp;
   int a20;
 };