Re: [Qemu-devel] Boot failure with MS-Dos 6.22 (due to bad BIOS build?)

[Kevin O'Connor <kevin@koconnor.net>]

On Mon, Mar 19, 2012 at 09:03:09PM +0100, Jan Kiszka wrote:
> On 2012-03-19 01:29, Kevin O'Connor wrote:
> > On Mon, Feb 27, 2012 at 04:25:09PM +0100, Jan Kiszka wrote:
> >>> EAX=ffffffff EBX=ffffffff ECX=0000c934 EDX=00000068
> >>> ESI=00006801 EDI=00000000 EBP=0000002b ESP=0000fff5
> > 
> > I traced this down, and it appears to be a stack size issue.  It looks
> > like MSDOS calls "int 0x13" with 229 bytes of stack space during its
> > boot.  On my build gcc generates the handle_13() function with a
> > maximum of 140 bytes of stack space utilized (according to
> > tools/checkstack.py).  On your build, gcc created it with a maximum of
> > 216 bytes.  The entry functions use 42 bytes of stack space.  Add it
> > up and you can see that the additional stack space that gcc used
> > caused %esp to wrap and the stack was corrupted.
> > 
> > I'm not sure how to best work around this.  One way is to sprinkle
> > "noinline" keywords through disk.c.  (It seems like gcc got in trouble
> > on your build by inlining many functions into disk_13().)  Another way
> > would be to jump into the extra stack (the disk code already uses its
> > own stack) earlier in the handle_13 code.
> > 
> > Also, can you see what happens if you change "--param
> > large-stack-frame=4" to "--param large-stack-frame=0" in the build?
> 
> This makes no difference here, still 216 bytes.

I've noticed that if one takes a pointer to a variable on the stack,
gcc seems to do a poor job managing stack space.  I'm guessing the
patch below would fix the issue.  It's a bit ugly though.

-Kevin


diff --git a/src/disk.c b/src/disk.c
index 7a58af4..706b9f4 100644
--- a/src/disk.c
+++ b/src/disk.c
@@ -76,7 +76,7 @@ fillLCHS(struct drive_s *drive_g, u16 *nlc, u16 *nlh, u16 *nlspt)
 }
 
 // Perform read/write/verify using old-style chs accesses
-static void
+static void noinline
 basic_access(struct bregs *regs, struct drive_s *drive_g, u16 command)
 {
     struct disk_op_s dop;
@@ -119,7 +119,7 @@ basic_access(struct bregs *regs, struct drive_s *drive_g, u16 command)
 }
 
 // Perform read/write/verify using new-style "int13ext" accesses.
-static void
+static void noinline
 extended_access(struct bregs *regs, struct drive_s *drive_g, u16 command)
 {
     struct disk_op_s dop;
@@ -201,7 +201,7 @@ disk_1304(struct bregs *regs, struct drive_s *drive_g)
 }
 
 // format disk track
-static void
+static void noinline
 disk_1305(struct bregs *regs, struct drive_s *drive_g)
 {
     debug_stub(regs);
@@ -228,7 +228,7 @@ disk_1305(struct bregs *regs, struct drive_s *drive_g)
 }
 
 // read disk drive parameters
-static void
+static void noinline
 disk_1308(struct bregs *regs, struct drive_s *drive_g)
 {
     u16 ebda_seg = get_ebda_seg();
@@ -329,7 +329,7 @@ disk_1314(struct bregs *regs, struct drive_s *drive_g)
 }
 
 // read disk drive size
-static void
+static void noinline
 disk_1315(struct bregs *regs, struct drive_s *drive_g)
 {
     disk_ret(regs, DISK_RET_SUCCESS);
@@ -463,7 +463,7 @@ disk_1345(struct bregs *regs, struct drive_s *drive_g)
 }
 
 // IBM/MS eject media
-static void
+static void noinline
 disk_1346(struct bregs *regs, struct drive_s *drive_g)
 {
     if (regs->dl < EXTSTART_CD) {