* [PATCH] Improve FreeDOS direct loading support compatibility.
@ 2012-06-30 5:43 C. Masloch
2012-07-07 12:23 ` Vladimir 'φ-coder/phcoder' Serbinenko
0 siblings, 1 reply; 6+ messages in thread
From: C. Masloch @ 2012-06-30 5:43 UTC (permalink / raw)
To: grub-devel; +Cc: freedos-kernel
[-- 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;
};
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Improve FreeDOS direct loading support compatibility.
2012-06-30 5:43 [PATCH] Improve FreeDOS direct loading support compatibility C. Masloch
@ 2012-07-07 12:23 ` Vladimir 'φ-coder/phcoder' Serbinenko
2012-07-08 8:28 ` C. Masloch
0 siblings, 1 reply; 6+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2012-07-07 12:23 UTC (permalink / raw)
To: The development of GNU GRUB
[-- Attachment #1: Type: text/plain, Size: 1620 bytes --]
On 30.06.2012 07:43, C. Masloch wrote:
> #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)
> +
Please put spaces around operators.
> +
> + /* This is not strictly necessary for the current FreeDOS kernel
> + but improves potential compatibility with the current EDR-DOS
> + load protocol.
DR-DOS is less-known proprietary software which we usually don't support
as policy. While it's ok to set those registers, please avoid
referencing DR-DOS directly, even in comments.
> + /* 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]);
No need of void cast here.
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Improve FreeDOS direct loading support compatibility.
2012-07-07 12:23 ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2012-07-08 8:28 ` C. Masloch
2012-09-09 19:49 ` Vladimir 'φ-coder/phcoder' Serbinenko
2013-01-27 15:08 ` Vladimir 'φ-coder/phcoder' Serbinenko
0 siblings, 2 replies; 6+ messages in thread
From: C. Masloch @ 2012-07-08 8:28 UTC (permalink / raw)
To: The development of GNU GRUB
[-- Attachment #1: Type: text/plain, Size: 432 bytes --]
Thanks for the help. The attached patch includes spaces around all
operators, doesn't include the void cast, and doesn't mention (E)DR-DOS
any longer. I recently learned that the dl register requirement is present
in prior releases of the FreeDOS kernel too, still called DOS-C. These
DOS-C releases already are under GPLv2+ so I hope it is okay to mention
them in the comment, as the attached patch does.
Regards,
Chris
[-- Attachment #2: improve-freedos-compatibility-2.diff --]
[-- Type: application/octet-stream, Size: 7257 bytes --]
Subject: [PATCH] Improve FreeDOS direct loading support compatibility
Date: Sun, 08 Jul 2012 10:08:29 +0200
From: "C. Masloch" <pushbx@38.de>
ChangeLog | 15 ++++++
grub-core/lib/i386/relocator.c | 2 +
grub-core/lib/i386/relocator16.S | 5 ++
grub-core/loader/i386/pc/freedos.c | 82 +++++++++++++++++++++++++++++++++++--
include/grub/i386/relocator.h | 1 +
5 files changed, 100 insertions(+), 5 deletions(-)
# HG changeset patch
# User C. Masloch <pushbx@38.de>
# Date 1341734778 -7200
# Node ID 3c587f7d6d1e80c73ec0c4a664feebb788f00ed1
# Parent 73948d0f69334ef6b60e60c16389742c402c2f3e
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,29 @@
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 others.
+ 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 other DOS-like kernels and loaders (including
+ the older FreeDOS kernel releases called DOS-C).
+ (Among those, FreeDOS's new 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 +110,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 +127,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)
+ {
+ 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;
};
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Improve FreeDOS direct loading support compatibility.
2012-07-08 8:28 ` C. Masloch
@ 2012-09-09 19:49 ` Vladimir 'φ-coder/phcoder' Serbinenko
2012-09-09 21:37 ` C. Masloch
2013-01-27 15:08 ` Vladimir 'φ-coder/phcoder' Serbinenko
1 sibling, 1 reply; 6+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2012-09-09 19:49 UTC (permalink / raw)
To: grub-devel
[-- Attachment #1: Type: text/plain, Size: 6935 bytes --]
On 08.07.2012 10:28, C. Masloch wrote:
>
> Thanks for the help. The attached patch includes spaces around all
> operators, doesn't include the void cast, and doesn't mention (E)DR-DOS
> any longer. I recently learned that the dl register requirement is
> present in prior releases of the FreeDOS kernel too, still called DOS-C.
> These DOS-C releases already are under GPLv2+ so I hope it is okay to
> mention them in the comment, as the attached patch does.
>
> Regards,
> Chris
>
> improve-freedos-compatibility-2.diff
>
>
> Subject: [PATCH] Improve FreeDOS direct loading support compatibility
> Date: Sun, 08 Jul 2012 10:08:29 +0200
> From: "C. Masloch" <pushbx@38.de>
>
> ChangeLog | 15 ++++++
> grub-core/lib/i386/relocator.c | 2 +
> grub-core/lib/i386/relocator16.S | 5 ++
> grub-core/loader/i386/pc/freedos.c | 82 +++++++++++++++++++++++++++++++++++--
> include/grub/i386/relocator.h | 1 +
> 5 files changed, 100 insertions(+), 5 deletions(-)
>
> # HG changeset patch
> # User C. Masloch <pushbx@38.de>
> # Date 1341734778 -7200
> # Node ID 3c587f7d6d1e80c73ec0c4a664feebb788f00ed1
> # Parent 73948d0f69334ef6b60e60c16389742c402c2f3e
> 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,29 @@
> 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 others.
> + 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 other DOS-like kernels and loaders (including
> + the older FreeDOS kernel releases called DOS-C).
> + (Among those, FreeDOS's new 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 +110,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 +127,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);
I don't understand this. Shouldn't it be at 0x7c00 ?
Do you plan to contribute more? If so I'd prefer to make you sign a
copyright assignment, otherwise this patch can go in, once it's clear
with addresses.
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Improve FreeDOS direct loading support compatibility.
2012-09-09 19:49 ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2012-09-09 21:37 ` C. Masloch
0 siblings, 0 replies; 6+ messages in thread
From: C. Masloch @ 2012-09-09 21:37 UTC (permalink / raw)
To: The development of GNU GRUB
On 2012-09-09 21:49 +0200, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
>> + {
>> + grub_relocator_chunk_t ch;
>> + err = grub_relocator_alloc_chunk_addr (rel, &ch,
>> GRUB_FREEDOS_BPB_ADDR,
>> + GRUB_DISK_SECTOR_SIZE);
>
> I don't understand this. Shouldn't it be at 0x7c00 ?
This is intended to be at 0x27a00, not 0x7c00. GRUB_FREEDOS_BPB_ADDR
should evaluate to 0x27a00. I'll try to explain the reasoning for this -
if any of the following is unclear, please feel free to ask for
clarification. Sorry that it's so verbose; I suggest you read the summary
first if you prefer.
-
The FreeDOS kernel's original boot loader (boot/boot.asm in the kernel's
repo) first relocates itself to linear address 2_7A00h (formed as
segmented destination address 1FE0h:7C00h, the segmented source address
being 0000h:7C00h). This is an adjustment from other/earlier loaders that
left the (boot sector) loader at (linear) 0_7C00h and thus restricted the
next stage* to be loaded from disk to about 29 KiB, if it is to be loaded
into contiguous space starting at a lower address (used to be 0_0700h, now
0_0600h for current FreeDOS kernel releases). With this adjustment, the
limit is increased to about 157 KiB.
[* Earlier DOS-C (ie FreeDOS kernel) releases loaded a subsequent IPL
stage first, possibly motivated by this ca. 29 KiB limit as well. Current
FreeDOS kernel releases typically directly load the whole kernel.]
In principle, GRUB could of course write the BPB anywhere up to the top of
"conventional memory" (below A_0000h, or below an EBDA if present there)
and incidentally raise the file size limit. (Or potentially the kernel
could be loaded to eg 0_8000h with the BPB placed somewhere below that,
also raising the file size limit - however 0_0600h is clearly documented
as the canonical load address of the current protocol and the kernel might
rely on it.)
However, various programs might assume that the FreeDOS load protocol
places the BPB at exactly 2_7A00h (with ss:bp and ds:bp specifying
1FE0h:7C00h). I don't know whether the current kernel itself relies on
this. I'm aware that at least MetaBoot** does search at that particular
(linear) address, so placing the BPB/sector there is necessary (though not
sufficient) for it. (MetaBoot's only purpose is to reload a different
next-stage file, hence loading it from GRUB isn't important because GRUB
can instead directly load the desired next-stage file. I merely mentioned
MetaBoot for its assumption that this particular position is implied by
the load protocol.)
[** MetaBoot belongs to the MetaKern loader for FreeDOS, developed by
FreeDOS kernel maintainer Eric Auer. It is covered by the GPL 2 with an
additional loading exception/permission, and distributed as part of
FreeDOS.]
In addition, arguably there is little use in raising the file size limit
above what the original loader can work with. This differs from my
opinion, but might be a relevant view for GRUB to consider.
-
So, summing it all up, the resulting 2_7A00h address of the BPB is to most
closely mimic the original loader, which arrived there to partially raise
a limit caused by that loader's size constraints (from the original 29 KiB
limit to 157 KiB). That level of compatibility might be unnecessary (for
the kernel itself), but apart from reproducing the original's more recent
file size limit there is no harm in it.
Note that in GRUB's ntldr.c, the BPB is written at 0_7C00h and no
equivalent file size limit exists because the next stage (NTLDR) file is
loaded starting at a higher address, 2_0000h.
> Do you plan to contribute more? If so I'd prefer to make you sign a
> copyright assignment, otherwise this patch can go in, once it's clear
> with addresses.
I could gladly sign an assignment, however I don't yet plan to contribute
more.
Regards,
Chris Masloch
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Improve FreeDOS direct loading support compatibility.
2012-07-08 8:28 ` C. Masloch
2012-09-09 19:49 ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2013-01-27 15:08 ` Vladimir 'φ-coder/phcoder' Serbinenko
1 sibling, 0 replies; 6+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2013-01-27 15:08 UTC (permalink / raw)
To: grub-devel
[-- Attachment #1: Type: text/plain, Size: 7314 bytes --]
Committed with minor problems fixed. Thanks.
On 08.07.2012 10:28, C. Masloch wrote:
> 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,29 @@
> 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 others.
> + 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 other DOS-like kernels and loaders (including
> + the older FreeDOS kernel releases called DOS-C).
> + (Among those, FreeDOS's new 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 +110,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 +127,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)
> + {
> + 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;
> };
>
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-01-27 15:08 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-06-30 5:43 [PATCH] Improve FreeDOS direct loading support compatibility C. Masloch
2012-07-07 12:23 ` Vladimir 'φ-coder/phcoder' Serbinenko
2012-07-08 8:28 ` C. Masloch
2012-09-09 19:49 ` Vladimir 'φ-coder/phcoder' Serbinenko
2012-09-09 21:37 ` C. Masloch
2013-01-27 15:08 ` Vladimir 'φ-coder/phcoder' Serbinenko
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.