Patchwork fdc: fix MAX_FD probelm

login
register
mail settings
Submitter 武田 =?ISO-2022-JP?B?IBskQj1TTGkbKEI=?=
Date Sept. 12, 2009, 7:52 p.m.
Message ID <200909121952.AA00105@YOUR-BD18D6DD63.m1.interq.or.jp>
Download mbox | patch
Permalink /patch/33538/
State Superseded
Headers show

Comments

武田 =?ISO-2022-JP?B?IBskQj1TTGkbKEI=?= - Sept. 12, 2009, 7:52 p.m.
Dear members,

In hw/fdc.c, it seems that MAX_FD is used for both the number of drives that fdc can control,
and the number of physical drives that may be connected for the system.
Now MAX_FD is defined 2 in hw/fdc.h, so FD_DOR_SELMASK is defined 1.
It will causes the problem that any attempt to select drive 2 results in drive 0 being selected,
as Stuart pointed out.

This patch is to fix this problem.
Stuart Brady - Sept. 13, 2009, 12:10 a.m.
On Sun, Sep 13, 2009 at 04:52:51AM +0900, 武田 俊也 wrote:
> +    if (fdctrl->num_floppies == 4) {
> +        fdctrl->fifo[2] = drv2(fdctrl)->track;
> +        fdctrl->fifo[3] = drv3(fdctrl)->track;
> +    } else {
> +        fdctrl->fifo[2] = 0;
> +        fdctrl->fifo[3] = 0;
> +    }

With real hardware, only a single drive might be connected, three drives
might be connected, or possibly none at all.  I don't even see why you
couldn't connect only drives 0 and 2 -- there could well be hardware
worth emulating that does something like that, but I don't know...

Perhaps something similar to the following should be used? :-

    static inline int drive_attached(fdctrl_t *fdctrl, int drv) {
        /* Assume that drives are attached contiguously,
           starting with drive 0. */
        return drv < fdctrl->num_floppies;
    }

And then:

    fdctrl->fifo[0] = drive_attached(0) ? drv0(fdctrl)->track : 0;
    fdctrl->fifo[1] = drive_attached(1) ? drv1(fdctrl)->track : 0;
    fdctrl->fifo[2] = drive_attached(2) ? drv2(fdctrl)->track : 0;
    fdctrl->fifo[3] = drive_attached(3) ? drv3(fdctrl)->track : 0;

You would need to modify that to take account the remapping of drives
performed by drv0(), drv1(), etc., though.

I'd suggest something similar in other places that test num_floppies,
although fdctrl_connect_drives() would of course be an exception.

Arguably, there should be something in fdrive_t (or fdctrl_t) to
indicate whether a particular drive is connected, but unfortunately the
'drive' member of fdrive_t holds the value FDRIVE_DRV_NONE even if the
drive exists, but no disk is inserted...

Cheers,

Patch

diff --git a/qemu/hw/fdc.c b/qemu/hw/fdc.c
index 389d9e6..56f775b 100644
--- a/qemu/hw/fdc.c
+++ b/qemu/hw/fdc.c
@@ -52,6 +52,15 @@ 
 /********************************************************/
 /* Floppy drive emulation                               */
 
+/* MAX_LOGICAL_FD determines the max drive number that Intel 82078 can control,
+   and it should be 4. (If MAX_LOGICAL_FD == 2, any attempt to select drive 2
+   results in drive 0 being selected,)
+   fdctrl->num_floppies determines the number of drives that may be connected
+   for the system and is usually initialized to MAX_PHYSICAL_FD, but it may be
+   initialized to other value if any system requires. */
+
+#define MAX_LOGICAL_FD 4
+
 #define GET_CUR_DRV(fdctrl) ((fdctrl)->cur_drv)
 #define SET_CUR_DRV(fdctrl, drive) ((fdctrl)->cur_drv = (drive))
 
@@ -387,6 +396,7 @@  enum {
 };
 
 enum {
+    FD_SR0_NOTRDY   = 0x08,
     FD_SR0_EQPMT    = 0x10,
     FD_SR0_SEEK     = 0x20,
     FD_SR0_ABNTERM  = 0x40,
@@ -424,11 +434,7 @@  enum {
 };
 
 enum {
-#if MAX_FD == 4
     FD_DOR_SELMASK  = 0x03,
-#else
-    FD_DOR_SELMASK  = 0x01,
-#endif
     FD_DOR_nRESET   = 0x04,
     FD_DOR_DMAEN    = 0x08,
     FD_DOR_MOTEN0   = 0x10,
@@ -438,11 +444,7 @@  enum {
 };
 
 enum {
-#if MAX_FD == 4
     FD_TDR_BOOTSEL  = 0x0c,
-#else
-    FD_TDR_BOOTSEL  = 0x04,
-#endif
 };
 
 enum {
@@ -508,10 +510,10 @@  struct fdctrl_t {
     /* Power down config (also with status regB access mode */
     uint8_t pwrd;
     /* Sun4m quirks? */
-    int sun4m;
+    uint8_t sun4m;
     /* Floppy drives */
     uint8_t num_floppies;
-    fdrive_t drives[MAX_FD];
+    fdrive_t drives[MAX_LOGICAL_FD];
     int reset_sensei;
 };
 
@@ -661,9 +663,9 @@  static int fdc_post_load(void *opaque)
 
 static const VMStateDescription vmstate_fdc = {
     .name = "fdc",
-    .version_id = 2,
-    .minimum_version_id = 2,
-    .minimum_version_id_old = 2,
+    .version_id = 3,
+    .minimum_version_id = 3,
+    .minimum_version_id_old = 3,
     .pre_save = fdc_pre_save,
     .post_load = fdc_post_load,
     .fields      = (VMStateField []) {
@@ -692,7 +694,7 @@  static const VMStateDescription vmstate_fdc = {
         VMSTATE_UINT8(lock, fdctrl_t),
         VMSTATE_UINT8(pwrd, fdctrl_t),
         VMSTATE_UINT8_EQUAL(num_floppies, fdctrl_t),
-        VMSTATE_STRUCT_ARRAY(drives, fdctrl_t, MAX_FD, 1,
+        VMSTATE_STRUCT_ARRAY(drives, fdctrl_t, MAX_LOGICAL_FD, 1,
                              vmstate_fdrive, fdrive_t),
         VMSTATE_END_OF_LIST()
     }
@@ -771,7 +773,7 @@  static void fdctrl_reset (fdctrl_t *fdctrl, int do_irq)
     fdctrl->data_len = 0;
     fdctrl->data_state = 0;
     fdctrl->data_dir = FD_DIR_WRITE;
-    for (i = 0; i < MAX_FD; i++)
+    for (i = 0; i < MAX_LOGICAL_FD; i++)
         fd_recalibrate(&fdctrl->drives[i]);
     fdctrl_reset_fifo(fdctrl);
     if (do_irq) {
@@ -793,7 +795,6 @@  static inline fdrive_t *drv1 (fdctrl_t *fdctrl)
         return &fdctrl->drives[0];
 }
 
-#if MAX_FD == 4
 static inline fdrive_t *drv2 (fdctrl_t *fdctrl)
 {
     if ((fdctrl->tdr & FD_TDR_BOOTSEL) < (2 << 2))
@@ -809,17 +810,14 @@  static inline fdrive_t *drv3 (fdctrl_t *fdctrl)
     else
         return &fdctrl->drives[2];
 }
-#endif
 
 static fdrive_t *get_cur_drv (fdctrl_t *fdctrl)
 {
     switch (fdctrl->cur_drv) {
         case 0: return drv0(fdctrl);
         case 1: return drv1(fdctrl);
-#if MAX_FD == 4
         case 2: return drv2(fdctrl);
         case 3: return drv3(fdctrl);
-#endif
         default: return NULL;
     }
 }
@@ -971,10 +969,8 @@  static uint32_t fdctrl_read_dir (fdctrl_t *fdctrl)
 
     if (fdctrl_media_changed(drv0(fdctrl))
      || fdctrl_media_changed(drv1(fdctrl))
-#if MAX_FD == 4
      || fdctrl_media_changed(drv2(fdctrl))
      || fdctrl_media_changed(drv3(fdctrl))
-#endif
         )
         retval |= FD_DIR_DSKCHG;
     if (retval != 0)
@@ -1418,13 +1414,13 @@  static void fdctrl_handle_dumpreg (fdctrl_t *fdctrl, int direction)
     /* Drives position */
     fdctrl->fifo[0] = drv0(fdctrl)->track;
     fdctrl->fifo[1] = drv1(fdctrl)->track;
-#if MAX_FD == 4
-    fdctrl->fifo[2] = drv2(fdctrl)->track;
-    fdctrl->fifo[3] = drv3(fdctrl)->track;
-#else
-    fdctrl->fifo[2] = 0;
-    fdctrl->fifo[3] = 0;
-#endif
+    if (fdctrl->num_floppies == 4) {
+        fdctrl->fifo[2] = drv2(fdctrl)->track;
+        fdctrl->fifo[3] = drv3(fdctrl)->track;
+    } else {
+        fdctrl->fifo[2] = 0;
+        fdctrl->fifo[3] = 0;
+    }
     /* timers */
     fdctrl->fifo[4] = fdctrl->timer0;
     fdctrl->fifo[5] = (fdctrl->timer1 << 1) | (fdctrl->dor & FD_DOR_DMAEN ? 1 : 0);
@@ -1456,10 +1452,10 @@  static void fdctrl_handle_restore (fdctrl_t *fdctrl, int direction)
     /* Drives position */
     drv0(fdctrl)->track = fdctrl->fifo[3];
     drv1(fdctrl)->track = fdctrl->fifo[4];
-#if MAX_FD == 4
-    drv2(fdctrl)->track = fdctrl->fifo[5];
-    drv3(fdctrl)->track = fdctrl->fifo[6];
-#endif
+    if (fdctrl->num_floppies == 4) {
+        drv2(fdctrl)->track = fdctrl->fifo[5];
+        drv3(fdctrl)->track = fdctrl->fifo[6];
+    }
     /* timers */
     fdctrl->timer0 = fdctrl->fifo[7];
     fdctrl->timer1 = fdctrl->fifo[8];
@@ -1481,13 +1477,13 @@  static void fdctrl_handle_save (fdctrl_t *fdctrl, int direction)
     /* Drives position */
     fdctrl->fifo[2] = drv0(fdctrl)->track;
     fdctrl->fifo[3] = drv1(fdctrl)->track;
-#if MAX_FD == 4
-    fdctrl->fifo[4] = drv2(fdctrl)->track;
-    fdctrl->fifo[5] = drv3(fdctrl)->track;
-#else
-    fdctrl->fifo[4] = 0;
-    fdctrl->fifo[5] = 0;
-#endif
+    if (fdctrl->num_floppies == 4) {
+        fdctrl->fifo[4] = drv2(fdctrl)->track;
+        fdctrl->fifo[5] = drv3(fdctrl)->track;
+    } else {
+        fdctrl->fifo[4] = 0;
+        fdctrl->fifo[5] = 0;
+    }
     /* timers */
     fdctrl->fifo[6] = fdctrl->timer0;
     fdctrl->fifo[7] = fdctrl->timer1;
@@ -1833,8 +1829,12 @@  static void fdctrl_connect_drives(fdctrl_t *fdctrl, BlockDriverState **fds)
 {
     unsigned int i;
 
-    for (i = 0; i < MAX_FD; i++) {
-        fd_init(&fdctrl->drives[i], fds[i]);
+    for (i = 0; i < MAX_LOGICAL_FD; i++) {
+        if (i < fdctrl->num_floppies) {
+            fd_init(&fdctrl->drives[i], fds[i]);
+        } else {
+            fd_init(&fdctrl->drives[i], NULL);
+        }
         fd_revalidate(&fdctrl->drives[i]);
     }
 }
@@ -1851,6 +1851,7 @@  fdctrl_t *fdctrl_init_isa(BlockDriverState **fds)
     fdctrl->dma_chann = dma_chann;
     DMA_register_channel(dma_chann, &fdctrl_transfer_handler, fdctrl);
 
+    fdctrl->num_floppies = MAX_PHYSICAL_FD;
     fdctrl_connect_drives(fdctrl, fds);
 
     return fdctrl;
@@ -1873,6 +1874,8 @@  fdctrl_t *fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
 
     fdctrl->dma_chann = dma_chann;
     DMA_register_channel(dma_chann, &fdctrl_transfer_handler, fdctrl);
+
+    fdctrl->num_floppies = MAX_PHYSICAL_FD;
     fdctrl_connect_drives(fdctrl, fds);
 
     return fdctrl;
@@ -1895,6 +1898,7 @@  fdctrl_t *sun4m_fdctrl_init (qemu_irq irq, target_phys_addr_t io_base,
 
     fdctrl->dma_chann = -1;
 
+    fdctrl->num_floppies = MAX_PHYSICAL_FD;
     fdctrl_connect_drives(fdctrl, fds);
 
     return fdctrl;
@@ -1925,7 +1929,6 @@  static int fdctrl_init_common(fdctrl_t *fdctrl)
 
     fdctrl->version = 0x90; /* Intel 82078 controller */
     fdctrl->config = FD_CONFIG_EIS | FD_CONFIG_EFIFO; /* Implicit seek, polling & FIFO enabled */
-    fdctrl->num_floppies = MAX_FD;
 
     fdctrl_external_reset(fdctrl);
     vmstate_register(-1, &vmstate_fdc, fdctrl);
diff --git a/qemu/hw/fdc.h b/qemu/hw/fdc.h
index 1b81ec1..9614e20 100644
--- a/qemu/hw/fdc.h
+++ b/qemu/hw/fdc.h
@@ -1,5 +1,5 @@ 
 /* fdc.c */
-#define MAX_FD 2
+#define MAX_PHYSICAL_FD 2
 
 typedef struct fdctrl_t fdctrl_t;
 
diff --git a/qemu/hw/mips_jazz.c b/qemu/hw/mips_jazz.c
index d62a584..bdb7865 100644
--- a/qemu/hw/mips_jazz.c
+++ b/qemu/hw/mips_jazz.c
@@ -127,7 +127,7 @@  void mips_jazz_init (ram_addr_t ram_size,
     int s_rtc, s_dma_dummy;
     NICInfo *nd;
     PITState *pit;
-    BlockDriverState *fds[MAX_FD];
+    BlockDriverState *fds[MAX_PHYSICAL_FD];
     qemu_irq esp_reset;
     ram_addr_t ram_offset;
     ram_addr_t bios_offset;
@@ -230,11 +230,11 @@  void mips_jazz_init (ram_addr_t ram_size,
              rc4030[5], &esp_reset);
 
     /* Floppy */
-    if (drive_get_max_bus(IF_FLOPPY) >= MAX_FD) {
+    if (drive_get_max_bus(IF_FLOPPY) >= MAX_PHYSICAL_FD) {
         fprintf(stderr, "qemu: too many floppy drives\n");
         exit(1);
     }
-    for (n = 0; n < MAX_FD; n++) {
+    for (n = 0; n < MAX_PHYSICAL_FD; n++) {
         DriveInfo *dinfo = drive_get(IF_FLOPPY, 0, n);
         fds[n] = dinfo ? dinfo->bdrv : NULL;
     }
diff --git a/qemu/hw/mips_malta.c b/qemu/hw/mips_malta.c
index 25e32bf..579db1c 100644
--- a/qemu/hw/mips_malta.c
+++ b/qemu/hw/mips_malta.c
@@ -777,7 +777,7 @@  void mips_malta_init (ram_addr_t ram_size,
     int i;
     DriveInfo *dinfo;
     DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
-    BlockDriverState *fd[MAX_FD];
+    BlockDriverState *fd[MAX_PHYSICAL_FD];
     int fl_idx = 0;
     int fl_sectors = 0;
 
@@ -928,7 +928,7 @@  void mips_malta_init (ram_addr_t ram_size,
     serial_init(0x2f8, isa_reserve_irq(3), 115200, serial_hds[1]);
     if (parallel_hds[0])
         parallel_init(0x378, isa_reserve_irq(7), parallel_hds[0]);
-    for(i = 0; i < MAX_FD; i++) {
+    for(i = 0; i < MAX_PHYSICAL_FD; i++) {
         dinfo = drive_get(IF_FLOPPY, 0, i);
         fd[i] = dinfo ? dinfo->bdrv : NULL;
     }
diff --git a/qemu/hw/pc.c b/qemu/hw/pc.c
index d96d756..a4d05e6 100644
--- a/qemu/hw/pc.c
+++ b/qemu/hw/pc.c
@@ -1133,7 +1133,7 @@  static void pc_init1(ram_addr_t ram_size,
     IsaIrqState *isa_irq_state;
     DriveInfo *dinfo;
     DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
-    BlockDriverState *fd[MAX_FD];
+    BlockDriverState *fd[MAX_PHYSICAL_FD];
     int using_vga = cirrus_vga_enabled || std_vga_enabled || vmsvga_enabled;
     void *fw_cfg;
 
@@ -1379,7 +1379,7 @@  static void pc_init1(ram_addr_t ram_size,
     audio_init(pci_enabled ? pci_bus : NULL, isa_irq);
 #endif
 
-    for(i = 0; i < MAX_FD; i++) {
+    for(i = 0; i < MAX_PHYSICAL_FD; i++) {
         dinfo = drive_get(IF_FLOPPY, 0, i);
         fd[i] = dinfo ? dinfo->bdrv : NULL;
     }
diff --git a/qemu/hw/ppc_prep.c b/qemu/hw/ppc_prep.c
index 1930146..afea2e9 100644
--- a/qemu/hw/ppc_prep.c
+++ b/qemu/hw/ppc_prep.c
@@ -564,7 +564,7 @@  static void ppc_prep_init (ram_addr_t ram_size,
     int ppc_boot_device;
     DriveInfo *dinfo;
     DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
-    BlockDriverState *fd[MAX_FD];
+    BlockDriverState *fd[MAX_PHYSICAL_FD];
 
     sysctrl = qemu_mallocz(sizeof(sysctrl_t));
 
@@ -715,7 +715,7 @@  static void ppc_prep_init (ram_addr_t ram_size,
     DMA_init(1);
     //    SB16_init();
 
-    for(i = 0; i < MAX_FD; i++) {
+    for(i = 0; i < MAX_PHYSICAL_FD; i++) {
         dinfo = drive_get(IF_FLOPPY, 0, i);
         fd[i] = dinfo ? dinfo->bdrv : NULL;
     }
diff --git a/qemu/hw/sun4m.c b/qemu/hw/sun4m.c
index d970723..fc28036 100644
--- a/qemu/hw/sun4m.c
+++ b/qemu/hw/sun4m.c
@@ -747,7 +747,7 @@  static void sun4m_hw_init(const struct sun4m_hwdef *hwdef, ram_addr_t RAM_size,
     qemu_irq fdc_tc;
     qemu_irq *cpu_halt;
     unsigned long kernel_size;
-    BlockDriverState *fd[MAX_FD];
+    BlockDriverState *fd[MAX_PHYSICAL_FD];
     void *fw_cfg;
     DriveInfo *dinfo;
 
@@ -1551,7 +1551,7 @@  static void sun4c_hw_init(const struct sun4c_hwdef *hwdef, ram_addr_t RAM_size,
     qemu_irq esp_reset;
     qemu_irq fdc_tc;
     unsigned long kernel_size;
-    BlockDriverState *fd[MAX_FD];
+    BlockDriverState *fd[MAX_PHYSICAL_FD];
     void *fw_cfg;
     DeviceState *dev;
     unsigned int i;
diff --git a/qemu/hw/sun4u.c b/qemu/hw/sun4u.c
index ffda4cd..ac91d14 100644
--- a/qemu/hw/sun4u.c
+++ b/qemu/hw/sun4u.c
@@ -561,7 +561,7 @@  static void sun4uv_init(ram_addr_t RAM_size,
     PCIBus *pci_bus, *pci_bus2, *pci_bus3;
     qemu_irq *irq;
     DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
-    BlockDriverState *fd[MAX_FD];
+    BlockDriverState *fd[MAX_PHYSICAL_FD];
     void *fw_cfg;
     DriveInfo *dinfo;
 
@@ -618,7 +618,7 @@  static void sun4uv_init(ram_addr_t RAM_size,
     pci_cmd646_ide_init(pci_bus, hd, 1);
 
     isa_create_simple("i8042");
-    for(i = 0; i < MAX_FD; i++) {
+    for(i = 0; i < MAX_PHYSICAL_FD; i++) {
         dinfo = drive_get(IF_FLOPPY, 0, i);
         fd[i] = dinfo ? dinfo->bdrv : NULL;
     }