On Mon, 2007-12-17 at 03:28 +0000, Thiemo Seufer wrote:
> Benjamin David Lunt wrote:
> > Hi everyone,
> >
> > I only recently have started to use QEmu due to a request
> > on the alt.os.development usenet group.  My OS was not working
> > on QEmu due to it would not recognize the (emulated) floppy.
> >
> > After a lot of testing, QEmu does not return the correct
> > values for a Sense Interrupt command and the Status 0 byte.
> >
> > After looking over fdc.cc, someone has placed a hack write
> > where it should return these values.
> >
> > I am just wondering if this hack is temporary or if it will
> > be committed as code.
> 
> That's the current state of the code in CVS.

This hack is terribly bugged and is likely to break all commands
status...

> > For a more detailed description, QEmu returns the value
> > 0x20 while Bochs, VMware, and real hardware return the
> > values 0xC0, 0xC1, 0xC2, and 0xC3, for each drive 1 - 4
> > respectively.

The polling mode is not implemented (and is even emulated in real
hardware) and only exists for 8" drives compatibility, according to
Intel specification, then has no meaning when using other drives
formats. Any reasonable OS would then disable this feature with a
CONFIGURE command as it's pointless on any modern machine (all PCs, for
example).
But it can be easily added, implementing the missing internal "emulated
drive status change" register, as described in the 82078 datasheet.

> > I am asking for more information on this subject.
> 
> The interrupt status handling in QEMU's FDC emulation looks bogus
> to me, patches to fix it are welcome. :-)

After taking a look to the specs and the code, it seems to me that the
greatest bug here is the hack added in the SENSE_INTERRUPT_STATUS
command answer. The 'SE' bit in status register 0 is set properly in
case of 'implied seek' without the hack. And, as far as I can see, not
all hardware do update this bit after read and write commands (Intel
82078 does, NS and SMC superIOs do not, according to their datasheet),
so it should not hurt any OS not having it set after any READ or WRITE
command.
So, the hack is really more buggy than the previous code:
- the SE bit (0x20) should be set only when there have been a implied
seek for READ & WRITE command. This case is hopefully not the common
case, as OSes always try to do sequential reads/writes for performances
reasons, and is properly handled, at least in DMA transfer case. There's
a case to be checked in case of PIO transfers: the bit seems always set,
even if no implied seek was done, which is buggy.
- the SE bit is never set by some hardware on any READ & WRITE commands,
then any code that would rely on this bit to be set on those commands
won't run on real hardware and is broken.
- it breaks all other commands status cases

One case that seems obviously not correct is that the
SENSE_INTERRUPT_STATUS should be treated as an invalid command when
there is no interrupt condition set.

The following patch implements the polling mode and the invalid
SENSE_INTERRUPT_STATUS case.

-- 
J. Mayer <[EMAIL PROTECTED]>
Never organized
Index: hw/fdc.c
===================================================================
RCS file: /sources/qemu/qemu/hw/fdc.c,v
retrieving revision 1.33
diff -u -d -d -p -r1.33 fdc.c
--- hw/fdc.c	17 Nov 2007 17:14:41 -0000	1.33
+++ hw/fdc.c	17 Dec 2007 14:17:10 -0000
@@ -399,6 +399,8 @@ struct fdctrl_t {
     uint8_t lock;
     /* Power down config (also with status regB access mode */
     uint8_t pwrd;
+    /* Drive status change emulation */
+    uint8_t drstch;
     /* Sun4m quirks? */
     int sun4m;
     /* Floppy drives */
@@ -674,7 +676,7 @@ static void fdctrl_raise_irq (fdctrl_t *
         fdctrl->int_status = status;
         return;
     }
-    if (~(fdctrl->state & FD_CTRL_INTR)) {
+    if (!(fdctrl->state & FD_CTRL_INTR)) {
         qemu_set_irq(fdctrl->irq, 1);
         fdctrl->state |= FD_CTRL_INTR;
     }
@@ -698,6 +700,8 @@ static void fdctrl_reset (fdctrl_t *fdct
     fdctrl->data_dir = FD_DIR_WRITE;
     for (i = 0; i < MAX_FD; i++)
         fd_reset(&fdctrl->drives[i]);
+    /* Initialize the emulated drive status change register */
+    fdctrl->drstch = 0xF;
     fdctrl_reset_fifo(fdctrl);
     if (do_irq)
         fdctrl_raise_irq(fdctrl, 0xc0);
@@ -1410,17 +1414,36 @@ static void fdctrl_write_data (fdctrl_t 
             /* SENSE_INTERRUPT_STATUS */
             FLOPPY_DPRINTF("SENSE_INTERRUPT_STATUS command (%02x)\n",
                            fdctrl->int_status);
-            /* No parameters cmd: returns status if no interrupt */
+            /* No parameters cmd: returns interrupt status */
+            if (!(fdctrl->config & 0x10) && fdctrl->drstch != 0x00) {
+                /* If emulated polling mode is active... */
+                int i;
+                for (i = 0; i < 4; i++) {
+                    if (fdctrl->drstch & (1 << i)) {
+                        /* Emulate 8" drive 'i' status change */
+                        fdctrl->fifo[0] = 0xC0 | i;
+                        fdctrl->drstch &= ~(1 << i);
+                        break;
+                    }
+                }
+            } else if (fdctrl->state & FD_CTRL_INTR) {
 #if 0
-            fdctrl->fifo[0] =
-                fdctrl->int_status | (cur_drv->head << 2) | fdctrl->cur_drv;
+                fdctrl->fifo[0] =
+                    fdctrl->int_status | (cur_drv->head << 2) | fdctrl->cur_drv;
 #else
-            /* XXX: int_status handling is broken for read/write
-               commands, so we do this hack. It should be suppressed
-               ASAP */
-            fdctrl->fifo[0] =
-                0x20 | (cur_drv->head << 2) | fdctrl->cur_drv;
+                /* XXX: int_status handling is broken for read/write
+                   commands, so we do this hack. It should be suppressed
+                   ASAP */
+                fdctrl->fifo[0] =
+                    0x20 | (cur_drv->head << 2) | fdctrl->cur_drv;
 #endif
+            } else {
+                /* When no interrupt is pending, the command is invalid */
+                fdctrl->fifo[0] = 0x80;
+                fdctrl_set_fifo(fdctrl, 1, 0);
+                fdctrl->int_status = 0xC0;
+                return;
+            }
             fdctrl->fifo[1] = cur_drv->track;
             fdctrl_set_fifo(fdctrl, 2, 0);
             fdctrl_reset_irq(fdctrl);

Reply via email to