[Qemu-devel] [PULL 00/13] IDE & FDC patches

[Qemu-devel] [PULL 00/13] IDE & FDC patches

[John Snow]

The following changes since commit 6ee06cc3dc7e8eb238e2f60cfd04f094d5c6b948:

  Merge remote-tracking branch 'remotes/lalrae/tags/mips-20160125' into staging (2016-01-25 10:42:52 +0000)

are available in the git repository at:

  https://github.com/jnsnow/qemu.git tags/ide-pull-request

for you to fetch changes up to 4812fa27fa75bce89738a82a191755853dd88408:

  fdc: change auto fallback drive for ISA FDC to 288 (2016-01-25 14:36:01 -0500)

----------------------------------------------------------------

----------------------------------------------------------------

John Snow (12):
  fdc: move pick_geometry
  fdc: reduce number of pick_geometry arguments
  fdc: add drive type qapi enum
  fdc: add disk field
  fdc: Throw an assertion on misconfigured fd_formats table
  fdc: add pick_drive
  fdc: Add fallback option
  fdc: add drive type option
  fdc: add physical disk sizes
  fdc: rework pick_geometry
  qtest/fdc: Support for 2.88MB drives
  fdc: change auto fallback drive for ISA FDC to 288

Shmulik Ladkani (1):
  ide: Correct the CHS 'cyls_max' limit to be 65535

 hw/block/fdc.c               | 326 ++++++++++++++++++++++++++++++-------------
 hw/core/qdev-properties.c    |  11 ++
 hw/i386/pc.c                 |  17 +--
 hw/ide/qdev.c                |   2 +-
 include/hw/block/fdc.h       |   9 +-
 include/hw/compat.h          |   4 +
 include/hw/qdev-properties.h |   1 +
 qapi/block.json              |  16 +++
 tests/fdc-test.c             |   2 +-
 9 files changed, 271 insertions(+), 117 deletions(-)

-- 
2.4.3

[Qemu-devel] [PULL 01/13] ide: Correct the CHS 'cyls_max' limit to be 65535

[John Snow]

From: Shmulik Ladkani <shmulik.ladkani@ravellosystems.com>

In b7eb0c9:
  hw/block-common: Factor out fall back to legacy -drive cyls=...
'blkconf_geometry()' was introduced, factoring out CHS limit validation
code that was repeated in ide, scsi, virtio-blk.

The original IDE CHS limit prior b7eb0c9 was 65535,16,255 (as per ATA
CHS addressing).
However the 'cyls_max' argument passed to 'blkconf_geometry' in the
ide_dev_initfn case was accidentally set to 65536 instead of 65535.

Fix, providing the correct 'cyls_max'.

Signed-off-by: Shmulik Ladkani <shmulik.ladkani@ravellosystems.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 1453112371-29760-1-git-send-email-shmulik.ladkani@ravellosystems.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
 hw/ide/qdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 1f83109..fea5425 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -172,7 +172,7 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
 
     blkconf_serial(&dev->conf, &dev->serial);
     if (kind != IDE_CD) {
-        blkconf_geometry(&dev->conf, &dev->chs_trans, 65536, 16, 255, &err);
+        blkconf_geometry(&dev->conf, &dev->chs_trans, 65535, 16, 255, &err);
         if (err) {
             error_report_err(err);
             return -1;
-- 
2.4.3

[Qemu-devel] [PULL 02/13] fdc: move pick_geometry

[John Snow]

Code motion: I want to refactor this function to work with FDrive
directly, so shuffle it below that definition.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 1453495865-9649-2-git-send-email-jsnow@redhat.com
---
 hw/block/fdc.c | 90 +++++++++++++++++++++++++++++-----------------------------
 1 file changed, 45 insertions(+), 45 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 6711c6a..71d931e 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -115,51 +115,6 @@ static const FDFormat fd_formats[] = {
     { FDRIVE_DRV_NONE, -1, -1, 0, 0, },
 };
 
-static void pick_geometry(BlockBackend *blk, int *nb_heads,
-                          int *max_track, int *last_sect,
-                          FDriveType drive_in, FDriveType *drive,
-                          FDriveRate *rate)
-{
-    const FDFormat *parse;
-    uint64_t nb_sectors, size;
-    int i, first_match, match;
-
-    blk_get_geometry(blk, &nb_sectors);
-    match = -1;
-    first_match = -1;
-    for (i = 0; ; i++) {
-        parse = &fd_formats[i];
-        if (parse->drive == FDRIVE_DRV_NONE) {
-            break;
-        }
-        if (drive_in == parse->drive ||
-            drive_in == FDRIVE_DRV_NONE) {
-            size = (parse->max_head + 1) * parse->max_track *
-                parse->last_sect;
-            if (nb_sectors == size) {
-                match = i;
-                break;
-            }
-            if (first_match == -1) {
-                first_match = i;
-            }
-        }
-    }
-    if (match == -1) {
-        if (first_match == -1) {
-            match = 1;
-        } else {
-            match = first_match;
-        }
-        parse = &fd_formats[match];
-    }
-    *nb_heads = parse->max_head + 1;
-    *max_track = parse->max_track;
-    *last_sect = parse->last_sect;
-    *drive = parse->drive;
-    *rate = parse->rate;
-}
-
 #define GET_CUR_DRV(fdctrl) ((fdctrl)->cur_drv)
 #define SET_CUR_DRV(fdctrl, drive) ((fdctrl)->cur_drv = (drive))
 
@@ -287,6 +242,51 @@ static void fd_recalibrate(FDrive *drv)
     fd_seek(drv, 0, 0, 1, 1);
 }
 
+static void pick_geometry(BlockBackend *blk, int *nb_heads,
+                          int *max_track, int *last_sect,
+                          FDriveType drive_in, FDriveType *drive,
+                          FDriveRate *rate)
+{
+    const FDFormat *parse;
+    uint64_t nb_sectors, size;
+    int i, first_match, match;
+
+    blk_get_geometry(blk, &nb_sectors);
+    match = -1;
+    first_match = -1;
+    for (i = 0; ; i++) {
+        parse = &fd_formats[i];
+        if (parse->drive == FDRIVE_DRV_NONE) {
+            break;
+        }
+        if (drive_in == parse->drive ||
+            drive_in == FDRIVE_DRV_NONE) {
+            size = (parse->max_head + 1) * parse->max_track *
+                parse->last_sect;
+            if (nb_sectors == size) {
+                match = i;
+                break;
+            }
+            if (first_match == -1) {
+                first_match = i;
+            }
+        }
+    }
+    if (match == -1) {
+        if (first_match == -1) {
+            match = 1;
+        } else {
+            match = first_match;
+        }
+        parse = &fd_formats[match];
+    }
+    *nb_heads = parse->max_head + 1;
+    *max_track = parse->max_track;
+    *last_sect = parse->last_sect;
+    *drive = parse->drive;
+    *rate = parse->rate;
+}
+
 /* Revalidate a disk drive after a disk change */
 static void fd_revalidate(FDrive *drv)
 {
-- 
2.4.3

[Qemu-devel] [PULL 03/13] fdc: reduce number of pick_geometry arguments

[John Snow]

Modify this function to operate directly on FDrive objects instead of
unpacking and passing all of those parameters manually. Reduces the
complexity in the caller and reduces the number of args to just one.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 1453495865-9649-3-git-send-email-jsnow@redhat.com
---
 hw/block/fdc.c | 50 ++++++++++++++++++++------------------------------
 1 file changed, 20 insertions(+), 30 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 71d931e..5505219 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -242,11 +242,9 @@ static void fd_recalibrate(FDrive *drv)
     fd_seek(drv, 0, 0, 1, 1);
 }
 
-static void pick_geometry(BlockBackend *blk, int *nb_heads,
-                          int *max_track, int *last_sect,
-                          FDriveType drive_in, FDriveType *drive,
-                          FDriveRate *rate)
+static void pick_geometry(FDrive *drv)
 {
+    BlockBackend *blk = drv->blk;
     const FDFormat *parse;
     uint64_t nb_sectors, size;
     int i, first_match, match;
@@ -259,8 +257,8 @@ static void pick_geometry(BlockBackend *blk, int *nb_heads,
         if (parse->drive == FDRIVE_DRV_NONE) {
             break;
         }
-        if (drive_in == parse->drive ||
-            drive_in == FDRIVE_DRV_NONE) {
+        if (drv->drive == parse->drive ||
+            drv->drive == FDRIVE_DRV_NONE) {
             size = (parse->max_head + 1) * parse->max_track *
                 parse->last_sect;
             if (nb_sectors == size) {
@@ -280,41 +278,33 @@ static void pick_geometry(BlockBackend *blk, int *nb_heads,
         }
         parse = &fd_formats[match];
     }
-    *nb_heads = parse->max_head + 1;
-    *max_track = parse->max_track;
-    *last_sect = parse->last_sect;
-    *drive = parse->drive;
-    *rate = parse->rate;
+
+    if (parse->max_head == 0) {
+        drv->flags &= ~FDISK_DBL_SIDES;
+    } else {
+        drv->flags |= FDISK_DBL_SIDES;
+    }
+    drv->max_track = parse->max_track;
+    drv->last_sect = parse->last_sect;
+    drv->drive = parse->drive;
+    drv->media_rate = parse->rate;
 }
 
 /* Revalidate a disk drive after a disk change */
 static void fd_revalidate(FDrive *drv)
 {
-    int nb_heads, max_track, last_sect, ro;
-    FDriveType drive;
-    FDriveRate rate;
-
     FLOPPY_DPRINTF("revalidate\n");
     if (drv->blk != NULL) {
-        ro = blk_is_read_only(drv->blk);
-        pick_geometry(drv->blk, &nb_heads, &max_track,
-                      &last_sect, drv->drive, &drive, &rate);
+        drv->ro = blk_is_read_only(drv->blk);
+        pick_geometry(drv);
         if (!drv->media_inserted) {
             FLOPPY_DPRINTF("No disk in drive\n");
         } else {
-            FLOPPY_DPRINTF("Floppy disk (%d h %d t %d s) %s\n", nb_heads,
-                           max_track, last_sect, ro ? "ro" : "rw");
+            FLOPPY_DPRINTF("Floppy disk (%d h %d t %d s) %s\n",
+                           (drv->flags & FDISK_DBL_SIDES) ? 2 : 1,
+                           drv->max_track, drv->last_sect,
+                           drv->ro ? "ro" : "rw");
         }
-        if (nb_heads == 1) {
-            drv->flags &= ~FDISK_DBL_SIDES;
-        } else {
-            drv->flags |= FDISK_DBL_SIDES;
-        }
-        drv->max_track = max_track;
-        drv->last_sect = last_sect;
-        drv->ro = ro;
-        drv->drive = drive;
-        drv->media_rate = rate;
     } else {
         FLOPPY_DPRINTF("No drive connected\n");
         drv->last_sect = 0;
-- 
2.4.3

[Qemu-devel] [PULL 04/13] fdc: add drive type qapi enum

[John Snow]

Change the floppy drive type to a QAPI enum type, to allow us to
specify the floppy drive type from the CLI in a forthcoming patch.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 1453495865-9649-4-git-send-email-jsnow@redhat.com
---
 hw/block/fdc.c         | 80 +++++++++++++++++++++++++-------------------------
 hw/i386/pc.c           | 17 ++++++-----
 include/hw/block/fdc.h |  9 +-----
 qapi/block.json        | 16 ++++++++++
 4 files changed, 66 insertions(+), 56 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 5505219..e37934d 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -61,7 +61,7 @@ typedef enum FDriveRate {
 } FDriveRate;
 
 typedef struct FDFormat {
-    FDriveType drive;
+    FloppyDriveType drive;
     uint8_t last_sect;
     uint8_t max_track;
     uint8_t max_head;
@@ -71,48 +71,48 @@ typedef struct FDFormat {
 static const FDFormat fd_formats[] = {
     /* First entry is default format */
     /* 1.44 MB 3"1/2 floppy disks */
-    { FDRIVE_DRV_144, 18, 80, 1, FDRIVE_RATE_500K, },
-    { FDRIVE_DRV_144, 20, 80, 1, FDRIVE_RATE_500K, },
-    { FDRIVE_DRV_144, 21, 80, 1, FDRIVE_RATE_500K, },
-    { FDRIVE_DRV_144, 21, 82, 1, FDRIVE_RATE_500K, },
-    { FDRIVE_DRV_144, 21, 83, 1, FDRIVE_RATE_500K, },
-    { FDRIVE_DRV_144, 22, 80, 1, FDRIVE_RATE_500K, },
-    { FDRIVE_DRV_144, 23, 80, 1, FDRIVE_RATE_500K, },
-    { FDRIVE_DRV_144, 24, 80, 1, FDRIVE_RATE_500K, },
+    { FLOPPY_DRIVE_TYPE_144, 18, 80, 1, FDRIVE_RATE_500K, },
+    { FLOPPY_DRIVE_TYPE_144, 20, 80, 1, FDRIVE_RATE_500K, },
+    { FLOPPY_DRIVE_TYPE_144, 21, 80, 1, FDRIVE_RATE_500K, },
+    { FLOPPY_DRIVE_TYPE_144, 21, 82, 1, FDRIVE_RATE_500K, },
+    { FLOPPY_DRIVE_TYPE_144, 21, 83, 1, FDRIVE_RATE_500K, },
+    { FLOPPY_DRIVE_TYPE_144, 22, 80, 1, FDRIVE_RATE_500K, },
+    { FLOPPY_DRIVE_TYPE_144, 23, 80, 1, FDRIVE_RATE_500K, },
+    { FLOPPY_DRIVE_TYPE_144, 24, 80, 1, FDRIVE_RATE_500K, },
     /* 2.88 MB 3"1/2 floppy disks */
-    { FDRIVE_DRV_288, 36, 80, 1, FDRIVE_RATE_1M, },
-    { FDRIVE_DRV_288, 39, 80, 1, FDRIVE_RATE_1M, },
-    { FDRIVE_DRV_288, 40, 80, 1, FDRIVE_RATE_1M, },
-    { FDRIVE_DRV_288, 44, 80, 1, FDRIVE_RATE_1M, },
-    { FDRIVE_DRV_288, 48, 80, 1, FDRIVE_RATE_1M, },
+    { FLOPPY_DRIVE_TYPE_288, 36, 80, 1, FDRIVE_RATE_1M, },
+    { FLOPPY_DRIVE_TYPE_288, 39, 80, 1, FDRIVE_RATE_1M, },
+    { FLOPPY_DRIVE_TYPE_288, 40, 80, 1, FDRIVE_RATE_1M, },
+    { FLOPPY_DRIVE_TYPE_288, 44, 80, 1, FDRIVE_RATE_1M, },
+    { FLOPPY_DRIVE_TYPE_288, 48, 80, 1, FDRIVE_RATE_1M, },
     /* 720 kB 3"1/2 floppy disks */
-    { FDRIVE_DRV_144,  9, 80, 1, FDRIVE_RATE_250K, },
-    { FDRIVE_DRV_144, 10, 80, 1, FDRIVE_RATE_250K, },
-    { FDRIVE_DRV_144, 10, 82, 1, FDRIVE_RATE_250K, },
-    { FDRIVE_DRV_144, 10, 83, 1, FDRIVE_RATE_250K, },
-    { FDRIVE_DRV_144, 13, 80, 1, FDRIVE_RATE_250K, },
-    { FDRIVE_DRV_144, 14, 80, 1, FDRIVE_RATE_250K, },
+    { FLOPPY_DRIVE_TYPE_144,  9, 80, 1, FDRIVE_RATE_250K, },
+    { FLOPPY_DRIVE_TYPE_144, 10, 80, 1, FDRIVE_RATE_250K, },
+    { FLOPPY_DRIVE_TYPE_144, 10, 82, 1, FDRIVE_RATE_250K, },
+    { FLOPPY_DRIVE_TYPE_144, 10, 83, 1, FDRIVE_RATE_250K, },
+    { FLOPPY_DRIVE_TYPE_144, 13, 80, 1, FDRIVE_RATE_250K, },
+    { FLOPPY_DRIVE_TYPE_144, 14, 80, 1, FDRIVE_RATE_250K, },
     /* 1.2 MB 5"1/4 floppy disks */
-    { FDRIVE_DRV_120, 15, 80, 1, FDRIVE_RATE_500K, },
-    { FDRIVE_DRV_120, 18, 80, 1, FDRIVE_RATE_500K, },
-    { FDRIVE_DRV_120, 18, 82, 1, FDRIVE_RATE_500K, },
-    { FDRIVE_DRV_120, 18, 83, 1, FDRIVE_RATE_500K, },
-    { FDRIVE_DRV_120, 20, 80, 1, FDRIVE_RATE_500K, },
+    { FLOPPY_DRIVE_TYPE_120, 15, 80, 1, FDRIVE_RATE_500K, },
+    { FLOPPY_DRIVE_TYPE_120, 18, 80, 1, FDRIVE_RATE_500K, },
+    { FLOPPY_DRIVE_TYPE_120, 18, 82, 1, FDRIVE_RATE_500K, },
+    { FLOPPY_DRIVE_TYPE_120, 18, 83, 1, FDRIVE_RATE_500K, },
+    { FLOPPY_DRIVE_TYPE_120, 20, 80, 1, FDRIVE_RATE_500K, },
     /* 720 kB 5"1/4 floppy disks */
-    { FDRIVE_DRV_120,  9, 80, 1, FDRIVE_RATE_250K, },
-    { FDRIVE_DRV_120, 11, 80, 1, FDRIVE_RATE_250K, },
+    { FLOPPY_DRIVE_TYPE_120,  9, 80, 1, FDRIVE_RATE_250K, },
+    { FLOPPY_DRIVE_TYPE_120, 11, 80, 1, FDRIVE_RATE_250K, },
     /* 360 kB 5"1/4 floppy disks */
-    { FDRIVE_DRV_120,  9, 40, 1, FDRIVE_RATE_300K, },
-    { FDRIVE_DRV_120,  9, 40, 0, FDRIVE_RATE_300K, },
-    { FDRIVE_DRV_120, 10, 41, 1, FDRIVE_RATE_300K, },
-    { FDRIVE_DRV_120, 10, 42, 1, FDRIVE_RATE_300K, },
+    { FLOPPY_DRIVE_TYPE_120,  9, 40, 1, FDRIVE_RATE_300K, },
+    { FLOPPY_DRIVE_TYPE_120,  9, 40, 0, FDRIVE_RATE_300K, },
+    { FLOPPY_DRIVE_TYPE_120, 10, 41, 1, FDRIVE_RATE_300K, },
+    { FLOPPY_DRIVE_TYPE_120, 10, 42, 1, FDRIVE_RATE_300K, },
     /* 320 kB 5"1/4 floppy disks */
-    { FDRIVE_DRV_120,  8, 40, 1, FDRIVE_RATE_250K, },
-    { FDRIVE_DRV_120,  8, 40, 0, FDRIVE_RATE_250K, },
+    { FLOPPY_DRIVE_TYPE_120,  8, 40, 1, FDRIVE_RATE_250K, },
+    { FLOPPY_DRIVE_TYPE_120,  8, 40, 0, FDRIVE_RATE_250K, },
     /* 360 kB must match 5"1/4 better than 3"1/2... */
-    { FDRIVE_DRV_144,  9, 80, 0, FDRIVE_RATE_250K, },
+    { FLOPPY_DRIVE_TYPE_144,  9, 80, 0, FDRIVE_RATE_250K, },
     /* end */
-    { FDRIVE_DRV_NONE, -1, -1, 0, 0, },
+    { FLOPPY_DRIVE_TYPE_NONE, -1, -1, 0, 0, },
 };
 
 #define GET_CUR_DRV(fdctrl) ((fdctrl)->cur_drv)
@@ -134,7 +134,7 @@ typedef struct FDrive {
     FDCtrl *fdctrl;
     BlockBackend *blk;
     /* Drive status */
-    FDriveType drive;
+    FloppyDriveType drive;    /* CMOS drive type        */
     uint8_t perpendicular;    /* 2.88 MB access mode    */
     /* Position */
     uint8_t head;
@@ -155,7 +155,7 @@ typedef struct FDrive {
 static void fd_init(FDrive *drv)
 {
     /* Drive */
-    drv->drive = FDRIVE_DRV_NONE;
+    drv->drive = FLOPPY_DRIVE_TYPE_NONE;
     drv->perpendicular = 0;
     /* Disk */
     drv->last_sect = 0;
@@ -254,11 +254,11 @@ static void pick_geometry(FDrive *drv)
     first_match = -1;
     for (i = 0; ; i++) {
         parse = &fd_formats[i];
-        if (parse->drive == FDRIVE_DRV_NONE) {
+        if (parse->drive == FLOPPY_DRIVE_TYPE_NONE) {
             break;
         }
         if (drv->drive == parse->drive ||
-            drv->drive == FDRIVE_DRV_NONE) {
+            drv->drive == FLOPPY_DRIVE_TYPE_NONE) {
             size = (parse->max_head + 1) * parse->max_track *
                 parse->last_sect;
             if (nb_sectors == size) {
@@ -2397,7 +2397,7 @@ static void sysbus_fdc_common_realize(DeviceState *dev, Error **errp)
     fdctrl_realize_common(fdctrl, errp);
 }
 
-FDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i)
+FloppyDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i)
 {
     FDCtrlISABus *isa = ISA_FDC(fdc);
 
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 838636c..78cf8fa 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -199,24 +199,24 @@ static void pic_irq_request(void *opaque, int irq, int level)
 
 #define REG_EQUIPMENT_BYTE          0x14
 
-static int cmos_get_fd_drive_type(FDriveType fd0)
+static int cmos_get_fd_drive_type(FloppyDriveType fd0)
 {
     int val;
 
     switch (fd0) {
-    case FDRIVE_DRV_144:
+    case FLOPPY_DRIVE_TYPE_144:
         /* 1.44 Mb 3"5 drive */
         val = 4;
         break;
-    case FDRIVE_DRV_288:
+    case FLOPPY_DRIVE_TYPE_288:
         /* 2.88 Mb 3"5 drive */
         val = 5;
         break;
-    case FDRIVE_DRV_120:
+    case FLOPPY_DRIVE_TYPE_120:
         /* 1.2 Mb 5"5 drive */
         val = 2;
         break;
-    case FDRIVE_DRV_NONE:
+    case FLOPPY_DRIVE_TYPE_NONE:
     default:
         val = 0;
         break;
@@ -287,7 +287,8 @@ static void pc_boot_set(void *opaque, const char *boot_device, Error **errp)
 static void pc_cmos_init_floppy(ISADevice *rtc_state, ISADevice *floppy)
 {
     int val, nb, i;
-    FDriveType fd_type[2] = { FDRIVE_DRV_NONE, FDRIVE_DRV_NONE };
+    FloppyDriveType fd_type[2] = { FLOPPY_DRIVE_TYPE_NONE,
+                                   FLOPPY_DRIVE_TYPE_NONE };
 
     /* floppy type */
     if (floppy) {
@@ -301,10 +302,10 @@ static void pc_cmos_init_floppy(ISADevice *rtc_state, ISADevice *floppy)
 
     val = rtc_get_memory(rtc_state, REG_EQUIPMENT_BYTE);
     nb = 0;
-    if (fd_type[0] < FDRIVE_DRV_NONE) {
+    if (fd_type[0] != FLOPPY_DRIVE_TYPE_NONE) {
         nb++;
     }
-    if (fd_type[1] < FDRIVE_DRV_NONE) {
+    if (fd_type[1] != FLOPPY_DRIVE_TYPE_NONE) {
         nb++;
     }
     switch (nb) {
diff --git a/include/hw/block/fdc.h b/include/hw/block/fdc.h
index d48b2f8..adce14f 100644
--- a/include/hw/block/fdc.h
+++ b/include/hw/block/fdc.h
@@ -6,13 +6,6 @@
 /* fdc.c */
 #define MAX_FD 2
 
-typedef enum FDriveType {
-    FDRIVE_DRV_144  = 0x00,   /* 1.44 MB 3"5 drive      */
-    FDRIVE_DRV_288  = 0x01,   /* 2.88 MB 3"5 drive      */
-    FDRIVE_DRV_120  = 0x02,   /* 1.2  MB 5"25 drive     */
-    FDRIVE_DRV_NONE = 0x03,   /* No drive connected     */
-} FDriveType;
-
 #define TYPE_ISA_FDC "isa-fdc"
 
 ISADevice *fdctrl_init_isa(ISABus *bus, DriveInfo **fds);
@@ -21,6 +14,6 @@ void fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
 void sun4m_fdctrl_init(qemu_irq irq, hwaddr io_base,
                        DriveInfo **fds, qemu_irq *fdc_tc);
 
-FDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i);
+FloppyDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i);
 
 #endif
diff --git a/qapi/block.json b/qapi/block.json
index 84022f1..ed61f82 100644
--- a/qapi/block.json
+++ b/qapi/block.json
@@ -40,6 +40,22 @@
   'data': ['auto', 'none', 'lba', 'large', 'rechs']}
 
 ##
+# @FloppyDriveType
+#
+# Type of Floppy drive to be emulated by the Floppy Disk Controller.
+#
+# @144:  1.44MB 3.5" drive
+# @288:  2.88MB 3.5" drive
+# @120:  1.2MB 5.25" drive
+# @none: No drive connected
+# @auto: Automatically determined by inserted media at boot
+#
+# Since: 2.6
+##
+{ 'enum': 'FloppyDriveType',
+  'data': ['144', '288', '120', 'none', 'auto']}
+
+##
 # @BlockdevSnapshotInternal
 #
 # @device: the name of the device to generate the snapshot from
-- 
2.4.3

[Qemu-devel] [PULL 05/13] fdc: add disk field

[John Snow]

Currently, 'drive' is used both to represent the current diskette
type as well as the current drive type.

This patch adds a 'disk' field that is updated explicitly to match
the type of the disk.

As of this patch, disk and drive are always the same, but forthcoming
patches to change the behavior of pick_geometry will invalidate this
assumption.

disk does not need to be migrated because it is not user-visible state
nor is it currently used for any calculations. It is purely informative,
and will be rebuilt automatically via fd_revalidate on the new host.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 1453495865-9649-5-git-send-email-jsnow@redhat.com
---
 hw/block/fdc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index e37934d..18e363b 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -141,6 +141,7 @@ typedef struct FDrive {
     uint8_t track;
     uint8_t sect;
     /* Media */
+    FloppyDriveType disk;     /* Current disk type      */
     FDiskFlags flags;
     uint8_t last_sect;        /* Nb sector per track    */
     uint8_t max_track;        /* Nb of tracks           */
@@ -158,6 +159,7 @@ static void fd_init(FDrive *drv)
     drv->drive = FLOPPY_DRIVE_TYPE_NONE;
     drv->perpendicular = 0;
     /* Disk */
+    drv->disk = FLOPPY_DRIVE_TYPE_NONE;
     drv->last_sect = 0;
     drv->max_track = 0;
 }
@@ -287,6 +289,7 @@ static void pick_geometry(FDrive *drv)
     drv->max_track = parse->max_track;
     drv->last_sect = parse->last_sect;
     drv->drive = parse->drive;
+    drv->disk = drv->media_inserted ? parse->drive : FLOPPY_DRIVE_TYPE_NONE;
     drv->media_rate = parse->rate;
 }
 
-- 
2.4.3

[Qemu-devel] [PULL 06/13] fdc: Throw an assertion on misconfigured fd_formats table

[John Snow]

pick_geometry is a convoluted function that makes it difficult to tell
at a glance what QEMU's current behavior for choosing a floppy drive
type is when it can't quite identify the diskette.

The code iterates over all entries in the candidate geometry table
("fd_formats") and if our specific drive type matches a row in the table,
then either "match" is set to that entry (an exact match) and the loop
exits, or "first_match" will be non-negative (the first such entry that
shares the same drive type), and the loop continues. If our specific
drive type is NONE, then all drive types in the candidate geometry table
are considered. After iteration, if "match" was not set, we fall back to
"first match".

This means that either "match" was set, or we exited the loop without an
exact match, in which case:

- If drive type is NONE, the default is truly fd_formats[0], a 1.44MB
  type, because "first_match" will always get set to the first item.

- If drive type is not NONE, pick_geometry's iteration was fussier and
  only looked at rows that matched our drive type. However, since all
  possible drive types are represented in the table, we still know that
  "first match" was set.

- If drive type is not NONE and the fd_formats table lists no options for
  our drive type, we choose fd_formats[1], an incomprehensibly bizarre
  choice that can never happen anyway.

Correct this: If first_match is -1, it can ONLY mean we didn't edit our
fd_formats table correctly. Throw an assertion instead.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 1453495865-9649-6-git-send-email-jsnow@redhat.com
---
 hw/block/fdc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 18e363b..a8f0cf2 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -274,7 +274,9 @@ static void pick_geometry(FDrive *drv)
     }
     if (match == -1) {
         if (first_match == -1) {
-            match = 1;
+            error_setg(&error_abort, "No candidate geometries present in table "
+                       " for floppy drive type '%s'",
+                       FloppyDriveType_lookup[drv->drive]);
         } else {
             match = first_match;
         }
-- 
2.4.3

[Qemu-devel] [PULL 07/13] fdc: add pick_drive

[John Snow]

Split apart pick_geometry by creating a pick_drive routine that will only
ever called during device bring-up instead of relying on pick_geometry to
be used in both cases.

With this change, the drive field is changed to be 'write once'. It is
not altered after the initialization routines exit.

media_validated does not need to be migrated. The target VM
will just revalidate the media on post_load anyway.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 1453495865-9649-7-git-send-email-jsnow@redhat.com
---
 hw/block/fdc.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 46 insertions(+), 10 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index a8f0cf2..f8e070e 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -151,6 +151,7 @@ typedef struct FDrive {
     uint8_t media_rate;       /* Data rate of medium    */
 
     bool media_inserted;      /* Is there a medium in the tray */
+    bool media_validated;     /* Have we validated the media? */
 } FDrive;
 
 static void fd_init(FDrive *drv)
@@ -162,6 +163,8 @@ static void fd_init(FDrive *drv)
     drv->disk = FLOPPY_DRIVE_TYPE_NONE;
     drv->last_sect = 0;
     drv->max_track = 0;
+    drv->ro = true;
+    drv->media_changed = 1;
 }
 
 #define NUM_SIDES(drv) ((drv)->flags & FDISK_DBL_SIDES ? 2 : 1)
@@ -244,13 +247,24 @@ static void fd_recalibrate(FDrive *drv)
     fd_seek(drv, 0, 0, 1, 1);
 }
 
-static void pick_geometry(FDrive *drv)
+/**
+ * Determine geometry based on inserted diskette.
+ * Will not operate on an empty drive.
+ *
+ * @return: 0 on success, -1 if the drive is empty.
+ */
+static int pick_geometry(FDrive *drv)
 {
     BlockBackend *blk = drv->blk;
     const FDFormat *parse;
     uint64_t nb_sectors, size;
     int i, first_match, match;
 
+    /* We can only pick a geometry if we have a diskette. */
+    if (!drv->media_inserted) {
+        return -1;
+    }
+
     blk_get_geometry(blk, &nb_sectors);
     match = -1;
     first_match = -1;
@@ -290,31 +304,51 @@ static void pick_geometry(FDrive *drv)
     }
     drv->max_track = parse->max_track;
     drv->last_sect = parse->last_sect;
-    drv->drive = parse->drive;
-    drv->disk = drv->media_inserted ? parse->drive : FLOPPY_DRIVE_TYPE_NONE;
+    drv->disk = parse->drive;
     drv->media_rate = parse->rate;
+    return 0;
+}
+
+static void pick_drive_type(FDrive *drv)
+{
+    if (pick_geometry(drv) == 0) {
+        drv->drive = drv->disk;
+    } else {
+        /* Legacy behavior: default to 1.44MB floppy */
+        drv->drive = FLOPPY_DRIVE_TYPE_144;
+    }
 }
 
 /* Revalidate a disk drive after a disk change */
 static void fd_revalidate(FDrive *drv)
 {
+    int rc;
+
     FLOPPY_DPRINTF("revalidate\n");
     if (drv->blk != NULL) {
         drv->ro = blk_is_read_only(drv->blk);
-        pick_geometry(drv);
         if (!drv->media_inserted) {
             FLOPPY_DPRINTF("No disk in drive\n");
-        } else {
-            FLOPPY_DPRINTF("Floppy disk (%d h %d t %d s) %s\n",
-                           (drv->flags & FDISK_DBL_SIDES) ? 2 : 1,
-                           drv->max_track, drv->last_sect,
-                           drv->ro ? "ro" : "rw");
+            drv->disk = FLOPPY_DRIVE_TYPE_NONE;
+        } else if (!drv->media_validated) {
+            rc = pick_geometry(drv);
+            if (rc) {
+                FLOPPY_DPRINTF("Could not validate floppy drive media");
+            } else {
+                drv->media_validated = true;
+                FLOPPY_DPRINTF("Floppy disk (%d h %d t %d s) %s\n",
+                               (drv->flags & FDISK_DBL_SIDES) ? 2 : 1,
+                               drv->max_track, drv->last_sect,
+                               drv->ro ? "ro" : "rw");
+            }
         }
     } else {
         FLOPPY_DPRINTF("No drive connected\n");
         drv->last_sect = 0;
         drv->max_track = 0;
         drv->flags &= ~FDISK_DBL_SIDES;
+        drv->drive = FLOPPY_DRIVE_TYPE_NONE;
+        drv->disk = FLOPPY_DRIVE_TYPE_NONE;
     }
 }
 
@@ -2185,6 +2219,7 @@ static void fdctrl_change_cb(void *opaque, bool load)
     drive->media_inserted = load && drive->blk && blk_is_inserted(drive->blk);
 
     drive->media_changed = 1;
+    drive->media_validated = false;
     fd_revalidate(drive);
 }
 
@@ -2221,11 +2256,12 @@ static void fdctrl_connect_drives(FDCtrl *fdctrl, Error **errp)
         }
 
         fd_init(drive);
-        fdctrl_change_cb(drive, 0);
         if (drive->blk) {
             blk_set_dev_ops(drive->blk, &fdctrl_block_ops, drive);
             drive->media_inserted = blk_is_inserted(drive->blk);
+            pick_drive_type(drive);
         }
+        fd_revalidate(drive);
     }
 }
 
-- 
2.4.3

Re: [Qemu-devel] [PULL 07/13] fdc: add pick_drive

[Paolo Bonzini]

On 25/01/2016 20:41, John Snow wrote:
> Split apart pick_geometry by creating a pick_drive routine that will only
> ever called during device bring-up instead of relying on pick_geometry to
> be used in both cases.
> 
> With this change, the drive field is changed to be 'write once'. It is
> not altered after the initialization routines exit.
> 
> media_validated does not need to be migrated. The target VM
> will just revalidate the media on post_load anyway.
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: John Snow <jsnow@redhat.com>
> Message-id: 1453495865-9649-7-git-send-email-jsnow@redhat.com

Sorry, this breaks the RHEL6.5 Linux installer CD.  It just hangs at
floppy detection and finally panics.

Paolo

> ---
>  hw/block/fdc.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 46 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
> index a8f0cf2..f8e070e 100644
> --- a/hw/block/fdc.c
> +++ b/hw/block/fdc.c
> @@ -151,6 +151,7 @@ typedef struct FDrive {
>      uint8_t media_rate;       /* Data rate of medium    */
>  
>      bool media_inserted;      /* Is there a medium in the tray */
> +    bool media_validated;     /* Have we validated the media? */
>  } FDrive;
>  
>  static void fd_init(FDrive *drv)
> @@ -162,6 +163,8 @@ static void fd_init(FDrive *drv)
>      drv->disk = FLOPPY_DRIVE_TYPE_NONE;
>      drv->last_sect = 0;
>      drv->max_track = 0;
> +    drv->ro = true;
> +    drv->media_changed = 1;
>  }
>  
>  #define NUM_SIDES(drv) ((drv)->flags & FDISK_DBL_SIDES ? 2 : 1)
> @@ -244,13 +247,24 @@ static void fd_recalibrate(FDrive *drv)
>      fd_seek(drv, 0, 0, 1, 1);
>  }
>  
> -static void pick_geometry(FDrive *drv)
> +/**
> + * Determine geometry based on inserted diskette.
> + * Will not operate on an empty drive.
> + *
> + * @return: 0 on success, -1 if the drive is empty.
> + */
> +static int pick_geometry(FDrive *drv)
>  {
>      BlockBackend *blk = drv->blk;
>      const FDFormat *parse;
>      uint64_t nb_sectors, size;
>      int i, first_match, match;
>  
> +    /* We can only pick a geometry if we have a diskette. */
> +    if (!drv->media_inserted) {
> +        return -1;
> +    }
> +
>      blk_get_geometry(blk, &nb_sectors);
>      match = -1;
>      first_match = -1;
> @@ -290,31 +304,51 @@ static void pick_geometry(FDrive *drv)
>      }
>      drv->max_track = parse->max_track;
>      drv->last_sect = parse->last_sect;
> -    drv->drive = parse->drive;
> -    drv->disk = drv->media_inserted ? parse->drive : FLOPPY_DRIVE_TYPE_NONE;
> +    drv->disk = parse->drive;
>      drv->media_rate = parse->rate;
> +    return 0;
> +}
> +
> +static void pick_drive_type(FDrive *drv)
> +{
> +    if (pick_geometry(drv) == 0) {
> +        drv->drive = drv->disk;
> +    } else {
> +        /* Legacy behavior: default to 1.44MB floppy */
> +        drv->drive = FLOPPY_DRIVE_TYPE_144;
> +    }
>  }
>  
>  /* Revalidate a disk drive after a disk change */
>  static void fd_revalidate(FDrive *drv)
>  {
> +    int rc;
> +
>      FLOPPY_DPRINTF("revalidate\n");
>      if (drv->blk != NULL) {
>          drv->ro = blk_is_read_only(drv->blk);
> -        pick_geometry(drv);
>          if (!drv->media_inserted) {
>              FLOPPY_DPRINTF("No disk in drive\n");
> -        } else {
> -            FLOPPY_DPRINTF("Floppy disk (%d h %d t %d s) %s\n",
> -                           (drv->flags & FDISK_DBL_SIDES) ? 2 : 1,
> -                           drv->max_track, drv->last_sect,
> -                           drv->ro ? "ro" : "rw");
> +            drv->disk = FLOPPY_DRIVE_TYPE_NONE;
> +        } else if (!drv->media_validated) {
> +            rc = pick_geometry(drv);
> +            if (rc) {
> +                FLOPPY_DPRINTF("Could not validate floppy drive media");
> +            } else {
> +                drv->media_validated = true;
> +                FLOPPY_DPRINTF("Floppy disk (%d h %d t %d s) %s\n",
> +                               (drv->flags & FDISK_DBL_SIDES) ? 2 : 1,
> +                               drv->max_track, drv->last_sect,
> +                               drv->ro ? "ro" : "rw");
> +            }
>          }
>      } else {
>          FLOPPY_DPRINTF("No drive connected\n");
>          drv->last_sect = 0;
>          drv->max_track = 0;
>          drv->flags &= ~FDISK_DBL_SIDES;
> +        drv->drive = FLOPPY_DRIVE_TYPE_NONE;
> +        drv->disk = FLOPPY_DRIVE_TYPE_NONE;
>      }
>  }
>  
> @@ -2185,6 +2219,7 @@ static void fdctrl_change_cb(void *opaque, bool load)
>      drive->media_inserted = load && drive->blk && blk_is_inserted(drive->blk);
>  
>      drive->media_changed = 1;
> +    drive->media_validated = false;
>      fd_revalidate(drive);
>  }
>  
> @@ -2221,11 +2256,12 @@ static void fdctrl_connect_drives(FDCtrl *fdctrl, Error **errp)
>          }
>  
>          fd_init(drive);
> -        fdctrl_change_cb(drive, 0);
>          if (drive->blk) {
>              blk_set_dev_ops(drive->blk, &fdctrl_block_ops, drive);
>              drive->media_inserted = blk_is_inserted(drive->blk);
> +            pick_drive_type(drive);
>          }
> +        fd_revalidate(drive);
>      }
>  }
>  
> 

Re: [Qemu-devel] [PULL 07/13] fdc: add pick_drive

[John Snow]

On 01/27/2016 07:38 AM, Paolo Bonzini wrote:
> 
> 
> On 25/01/2016 20:41, John Snow wrote:
>> Split apart pick_geometry by creating a pick_drive routine that will only
>> ever called during device bring-up instead of relying on pick_geometry to
>> be used in both cases.
>>
>> With this change, the drive field is changed to be 'write once'. It is
>> not altered after the initialization routines exit.
>>
>> media_validated does not need to be migrated. The target VM
>> will just revalidate the media on post_load anyway.
>>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> Message-id: 1453495865-9649-7-git-send-email-jsnow@redhat.com
> 
> Sorry, this breaks the RHEL6.5 Linux installer CD.  It just hangs at
> floppy detection and finally panics.
> 
> Paolo
> 

Crap, alright. I'll get right to it. That's what I get for poking a
hibernating bear.

>> ---
>>  hw/block/fdc.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++----------
>>  1 file changed, 46 insertions(+), 10 deletions(-)
>>
>> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
>> index a8f0cf2..f8e070e 100644
>> --- a/hw/block/fdc.c
>> +++ b/hw/block/fdc.c
>> @@ -151,6 +151,7 @@ typedef struct FDrive {
>>      uint8_t media_rate;       /* Data rate of medium    */
>>  
>>      bool media_inserted;      /* Is there a medium in the tray */
>> +    bool media_validated;     /* Have we validated the media? */
>>  } FDrive;
>>  
>>  static void fd_init(FDrive *drv)
>> @@ -162,6 +163,8 @@ static void fd_init(FDrive *drv)
>>      drv->disk = FLOPPY_DRIVE_TYPE_NONE;
>>      drv->last_sect = 0;
>>      drv->max_track = 0;
>> +    drv->ro = true;
>> +    drv->media_changed = 1;
>>  }
>>  
>>  #define NUM_SIDES(drv) ((drv)->flags & FDISK_DBL_SIDES ? 2 : 1)
>> @@ -244,13 +247,24 @@ static void fd_recalibrate(FDrive *drv)
>>      fd_seek(drv, 0, 0, 1, 1);
>>  }
>>  
>> -static void pick_geometry(FDrive *drv)
>> +/**
>> + * Determine geometry based on inserted diskette.
>> + * Will not operate on an empty drive.
>> + *
>> + * @return: 0 on success, -1 if the drive is empty.
>> + */
>> +static int pick_geometry(FDrive *drv)
>>  {
>>      BlockBackend *blk = drv->blk;
>>      const FDFormat *parse;
>>      uint64_t nb_sectors, size;
>>      int i, first_match, match;
>>  
>> +    /* We can only pick a geometry if we have a diskette. */
>> +    if (!drv->media_inserted) {
>> +        return -1;
>> +    }
>> +
>>      blk_get_geometry(blk, &nb_sectors);
>>      match = -1;
>>      first_match = -1;
>> @@ -290,31 +304,51 @@ static void pick_geometry(FDrive *drv)
>>      }
>>      drv->max_track = parse->max_track;
>>      drv->last_sect = parse->last_sect;
>> -    drv->drive = parse->drive;
>> -    drv->disk = drv->media_inserted ? parse->drive : FLOPPY_DRIVE_TYPE_NONE;
>> +    drv->disk = parse->drive;
>>      drv->media_rate = parse->rate;
>> +    return 0;
>> +}
>> +
>> +static void pick_drive_type(FDrive *drv)
>> +{
>> +    if (pick_geometry(drv) == 0) {
>> +        drv->drive = drv->disk;
>> +    } else {
>> +        /* Legacy behavior: default to 1.44MB floppy */
>> +        drv->drive = FLOPPY_DRIVE_TYPE_144;
>> +    }
>>  }
>>  
>>  /* Revalidate a disk drive after a disk change */
>>  static void fd_revalidate(FDrive *drv)
>>  {
>> +    int rc;
>> +
>>      FLOPPY_DPRINTF("revalidate\n");
>>      if (drv->blk != NULL) {
>>          drv->ro = blk_is_read_only(drv->blk);
>> -        pick_geometry(drv);
>>          if (!drv->media_inserted) {
>>              FLOPPY_DPRINTF("No disk in drive\n");
>> -        } else {
>> -            FLOPPY_DPRINTF("Floppy disk (%d h %d t %d s) %s\n",
>> -                           (drv->flags & FDISK_DBL_SIDES) ? 2 : 1,
>> -                           drv->max_track, drv->last_sect,
>> -                           drv->ro ? "ro" : "rw");
>> +            drv->disk = FLOPPY_DRIVE_TYPE_NONE;
>> +        } else if (!drv->media_validated) {
>> +            rc = pick_geometry(drv);
>> +            if (rc) {
>> +                FLOPPY_DPRINTF("Could not validate floppy drive media");
>> +            } else {
>> +                drv->media_validated = true;
>> +                FLOPPY_DPRINTF("Floppy disk (%d h %d t %d s) %s\n",
>> +                               (drv->flags & FDISK_DBL_SIDES) ? 2 : 1,
>> +                               drv->max_track, drv->last_sect,
>> +                               drv->ro ? "ro" : "rw");
>> +            }
>>          }
>>      } else {
>>          FLOPPY_DPRINTF("No drive connected\n");
>>          drv->last_sect = 0;
>>          drv->max_track = 0;
>>          drv->flags &= ~FDISK_DBL_SIDES;
>> +        drv->drive = FLOPPY_DRIVE_TYPE_NONE;
>> +        drv->disk = FLOPPY_DRIVE_TYPE_NONE;
>>      }
>>  }
>>  
>> @@ -2185,6 +2219,7 @@ static void fdctrl_change_cb(void *opaque, bool load)
>>      drive->media_inserted = load && drive->blk && blk_is_inserted(drive->blk);
>>  
>>      drive->media_changed = 1;
>> +    drive->media_validated = false;
>>      fd_revalidate(drive);
>>  }
>>  
>> @@ -2221,11 +2256,12 @@ static void fdctrl_connect_drives(FDCtrl *fdctrl, Error **errp)
>>          }
>>  
>>          fd_init(drive);
>> -        fdctrl_change_cb(drive, 0);
>>          if (drive->blk) {
>>              blk_set_dev_ops(drive->blk, &fdctrl_block_ops, drive);
>>              drive->media_inserted = blk_is_inserted(drive->blk);
>> +            pick_drive_type(drive);
>>          }
>> +        fd_revalidate(drive);
>>      }
>>  }
>>  
>>
> 

-- 
—js

Re: [Qemu-devel] [PULL 07/13] fdc: add pick_drive

[John Snow]

On 01/27/2016 07:38 AM, Paolo Bonzini wrote:
> 
> 
> On 25/01/2016 20:41, John Snow wrote:
>> Split apart pick_geometry by creating a pick_drive routine that will only
>> ever called during device bring-up instead of relying on pick_geometry to
>> be used in both cases.
>>
>> With this change, the drive field is changed to be 'write once'. It is
>> not altered after the initialization routines exit.
>>
>> media_validated does not need to be migrated. The target VM
>> will just revalidate the media on post_load anyway.
>>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> Message-id: 1453495865-9649-7-git-send-email-jsnow@redhat.com
> 
> Sorry, this breaks the RHEL6.5 Linux installer CD.  It just hangs at
> floppy detection and finally panics.
> 
> Paolo
> 

Looks like the problem is that after this patch, drives with no inserted
medium no longer populate the diskette geometry fields.

This leads to a problem where Linux attempts to seek to the first sector
on an empty drive. When it reads back the status interrupt information,
it finds that the command has succeeded but the current
head/track/sector values are unchanged, so it tries again. (I think. The
linux floppy code is... well, it certainly _is_.)

fd_seek itself guards against out-of-bounds seeks, even though Hervé
Poussineau patched fdctrl_handle_seek explicitly to allow such
out-of-bound seeks.

The end result is that QEMU "lies" about having done the seek, and Linux
appears to get very, very confused.

It looks as if QEMU tries to keep the current track within a sane
boundary for integrity reasons, but the Linux FDC driver expects to be
able to seek an empty drive. In reality, QEMU must consider current
sector/track to be "untrusted" values, but it currently tries to enforce
them being valid, but there doesn't appear to be any precedent for
refusing/erroring out on a SEEK command.

(Unrelatedly, in trying to fix this, I tried to see what would happen if
on an invalid seek through FD_SEEK alone I set ABNTERM, and it leads the
Linux kernel through a route where it tries to issue
DRIVE_SPECIFICATION, which will also break QEMU/Linux, because
DRIVE_SPECIFICATION can be anywhere between 3-7 bytes, but our version
accepts statically exactly 6 bytes.)

((Even fixing the above, Linux still doesn't appear to take a rejected
SEEK very well, just infinitely resetting, seeking, recalibrating.))

Given the above, it looks like the quick, dumb, and easy way to fix this
for now without risking jostling esoteric hardware will be to just set
dummy geometries on boot for empty drives, like we used to.

Hervé, any thoughts?

--js

>> ---
>>  hw/block/fdc.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++----------
>>  1 file changed, 46 insertions(+), 10 deletions(-)
>>
>> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
>> index a8f0cf2..f8e070e 100644
>> --- a/hw/block/fdc.c
>> +++ b/hw/block/fdc.c
>> @@ -151,6 +151,7 @@ typedef struct FDrive {
>>      uint8_t media_rate;       /* Data rate of medium    */
>>  
>>      bool media_inserted;      /* Is there a medium in the tray */
>> +    bool media_validated;     /* Have we validated the media? */
>>  } FDrive;
>>  
>>  static void fd_init(FDrive *drv)
>> @@ -162,6 +163,8 @@ static void fd_init(FDrive *drv)
>>      drv->disk = FLOPPY_DRIVE_TYPE_NONE;
>>      drv->last_sect = 0;
>>      drv->max_track = 0;
>> +    drv->ro = true;
>> +    drv->media_changed = 1;
>>  }
>>  
>>  #define NUM_SIDES(drv) ((drv)->flags & FDISK_DBL_SIDES ? 2 : 1)
>> @@ -244,13 +247,24 @@ static void fd_recalibrate(FDrive *drv)
>>      fd_seek(drv, 0, 0, 1, 1);
>>  }
>>  
>> -static void pick_geometry(FDrive *drv)
>> +/**
>> + * Determine geometry based on inserted diskette.
>> + * Will not operate on an empty drive.
>> + *
>> + * @return: 0 on success, -1 if the drive is empty.
>> + */
>> +static int pick_geometry(FDrive *drv)
>>  {
>>      BlockBackend *blk = drv->blk;
>>      const FDFormat *parse;
>>      uint64_t nb_sectors, size;
>>      int i, first_match, match;
>>  
>> +    /* We can only pick a geometry if we have a diskette. */
>> +    if (!drv->media_inserted) {
>> +        return -1;
>> +    }
>> +
>>      blk_get_geometry(blk, &nb_sectors);
>>      match = -1;
>>      first_match = -1;
>> @@ -290,31 +304,51 @@ static void pick_geometry(FDrive *drv)
>>      }
>>      drv->max_track = parse->max_track;
>>      drv->last_sect = parse->last_sect;
>> -    drv->drive = parse->drive;
>> -    drv->disk = drv->media_inserted ? parse->drive : FLOPPY_DRIVE_TYPE_NONE;
>> +    drv->disk = parse->drive;
>>      drv->media_rate = parse->rate;
>> +    return 0;
>> +}
>> +
>> +static void pick_drive_type(FDrive *drv)
>> +{
>> +    if (pick_geometry(drv) == 0) {
>> +        drv->drive = drv->disk;
>> +    } else {
>> +        /* Legacy behavior: default to 1.44MB floppy */
>> +        drv->drive = FLOPPY_DRIVE_TYPE_144;
>> +    }
>>  }
>>  
>>  /* Revalidate a disk drive after a disk change */
>>  static void fd_revalidate(FDrive *drv)
>>  {
>> +    int rc;
>> +
>>      FLOPPY_DPRINTF("revalidate\n");
>>      if (drv->blk != NULL) {
>>          drv->ro = blk_is_read_only(drv->blk);
>> -        pick_geometry(drv);
>>          if (!drv->media_inserted) {
>>              FLOPPY_DPRINTF("No disk in drive\n");
>> -        } else {
>> -            FLOPPY_DPRINTF("Floppy disk (%d h %d t %d s) %s\n",
>> -                           (drv->flags & FDISK_DBL_SIDES) ? 2 : 1,
>> -                           drv->max_track, drv->last_sect,
>> -                           drv->ro ? "ro" : "rw");
>> +            drv->disk = FLOPPY_DRIVE_TYPE_NONE;
>> +        } else if (!drv->media_validated) {
>> +            rc = pick_geometry(drv);
>> +            if (rc) {
>> +                FLOPPY_DPRINTF("Could not validate floppy drive media");
>> +            } else {
>> +                drv->media_validated = true;
>> +                FLOPPY_DPRINTF("Floppy disk (%d h %d t %d s) %s\n",
>> +                               (drv->flags & FDISK_DBL_SIDES) ? 2 : 1,
>> +                               drv->max_track, drv->last_sect,
>> +                               drv->ro ? "ro" : "rw");
>> +            }
>>          }
>>      } else {
>>          FLOPPY_DPRINTF("No drive connected\n");
>>          drv->last_sect = 0;
>>          drv->max_track = 0;
>>          drv->flags &= ~FDISK_DBL_SIDES;
>> +        drv->drive = FLOPPY_DRIVE_TYPE_NONE;
>> +        drv->disk = FLOPPY_DRIVE_TYPE_NONE;
>>      }
>>  }
>>  
>> @@ -2185,6 +2219,7 @@ static void fdctrl_change_cb(void *opaque, bool load)
>>      drive->media_inserted = load && drive->blk && blk_is_inserted(drive->blk);
>>  
>>      drive->media_changed = 1;
>> +    drive->media_validated = false;
>>      fd_revalidate(drive);
>>  }
>>  
>> @@ -2221,11 +2256,12 @@ static void fdctrl_connect_drives(FDCtrl *fdctrl, Error **errp)
>>          }
>>  
>>          fd_init(drive);
>> -        fdctrl_change_cb(drive, 0);
>>          if (drive->blk) {
>>              blk_set_dev_ops(drive->blk, &fdctrl_block_ops, drive);
>>              drive->media_inserted = blk_is_inserted(drive->blk);
>> +            pick_drive_type(drive);
>>          }
>> +        fd_revalidate(drive);
>>      }
>>  }
>>  
>>
> 

[Qemu-devel] [PULL 08/13] fdc: Add fallback option

[John Snow]

Currently, QEMU chooses a drive type automatically based on the inserted
media. If there is no disk inserted, it chooses a 1.44MB drive type.

Change this behavior to be configurable, but leave it defaulted to 1.44.

This is not earnestly intended to be used by a user or a management
library, but rather exists so that pre-2.6 board types can configure it
to be a legacy value.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 1453495865-9649-8-git-send-email-jsnow@redhat.com
---
 hw/block/fdc.c               | 25 +++++++++++++++++++++++--
 hw/core/qdev-properties.c    | 11 +++++++++++
 include/hw/qdev-properties.h |  1 +
 3 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index f8e070e..4caed9b 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -154,6 +154,9 @@ typedef struct FDrive {
     bool media_validated;     /* Have we validated the media? */
 } FDrive;
 
+
+static FloppyDriveType get_fallback_drive_type(FDrive *drv);
+
 static void fd_init(FDrive *drv)
 {
     /* Drive */
@@ -314,8 +317,7 @@ static void pick_drive_type(FDrive *drv)
     if (pick_geometry(drv) == 0) {
         drv->drive = drv->disk;
     } else {
-        /* Legacy behavior: default to 1.44MB floppy */
-        drv->drive = FLOPPY_DRIVE_TYPE_144;
+        drv->drive = get_fallback_drive_type(drv);
     }
 }
 
@@ -598,11 +600,17 @@ struct FDCtrl {
     FDrive drives[MAX_FD];
     int reset_sensei;
     uint32_t check_media_rate;
+    FloppyDriveType fallback; /* type=auto failure fallback */
     /* Timers state */
     uint8_t timer0;
     uint8_t timer1;
 };
 
+static FloppyDriveType get_fallback_drive_type(FDrive *drv)
+{
+    return drv->fdctrl->fallback;
+}
+
 #define TYPE_SYSBUS_FDC "base-sysbus-fdc"
 #define SYSBUS_FDC(obj) OBJECT_CHECK(FDCtrlSysBus, (obj), TYPE_SYSBUS_FDC)
 
@@ -2338,6 +2346,10 @@ static void fdctrl_realize_common(FDCtrl *fdctrl, Error **errp)
     int i, j;
     static int command_tables_inited = 0;
 
+    if (fdctrl->fallback == FLOPPY_DRIVE_TYPE_AUTO) {
+        error_setg(errp, "Cannot choose a fallback FDrive type of 'auto'");
+    }
+
     /* Fill 'command_to_handler' lookup table */
     if (!command_tables_inited) {
         command_tables_inited = 1;
@@ -2463,6 +2475,9 @@ static Property isa_fdc_properties[] = {
     DEFINE_PROP_DRIVE("driveB", FDCtrlISABus, state.drives[1].blk),
     DEFINE_PROP_BIT("check_media_rate", FDCtrlISABus, state.check_media_rate,
                     0, true),
+    DEFINE_PROP_DEFAULT("fallback", FDCtrlISABus, state.fallback,
+                        FLOPPY_DRIVE_TYPE_144, qdev_prop_fdc_drive_type,
+                        FloppyDriveType),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -2511,6 +2526,9 @@ static const VMStateDescription vmstate_sysbus_fdc ={
 static Property sysbus_fdc_properties[] = {
     DEFINE_PROP_DRIVE("driveA", FDCtrlSysBus, state.drives[0].blk),
     DEFINE_PROP_DRIVE("driveB", FDCtrlSysBus, state.drives[1].blk),
+    DEFINE_PROP_DEFAULT("fallback", FDCtrlISABus, state.fallback,
+                        FLOPPY_DRIVE_TYPE_144, qdev_prop_fdc_drive_type,
+                        FloppyDriveType),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -2531,6 +2549,9 @@ static const TypeInfo sysbus_fdc_info = {
 
 static Property sun4m_fdc_properties[] = {
     DEFINE_PROP_DRIVE("drive", FDCtrlSysBus, state.drives[0].blk),
+    DEFINE_PROP_DEFAULT("fallback", FDCtrlISABus, state.fallback,
+                        FLOPPY_DRIVE_TYPE_144, qdev_prop_fdc_drive_type,
+                        FloppyDriveType),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 3572810..aacad66 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -541,6 +541,17 @@ PropertyInfo qdev_prop_bios_chs_trans = {
     .set = set_enum,
 };
 
+/* --- FDC default drive types */
+
+PropertyInfo qdev_prop_fdc_drive_type = {
+    .name = "FdcDriveType",
+    .description = "FDC drive type, "
+                   "144/288/120/none/auto",
+    .enum_table = FloppyDriveType_lookup,
+    .get = get_enum,
+    .set = set_enum
+};
+
 /* --- pci address --- */
 
 /*
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 254afd8..03a1b91 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -20,6 +20,7 @@ extern PropertyInfo qdev_prop_ptr;
 extern PropertyInfo qdev_prop_macaddr;
 extern PropertyInfo qdev_prop_losttickpolicy;
 extern PropertyInfo qdev_prop_bios_chs_trans;
+extern PropertyInfo qdev_prop_fdc_drive_type;
 extern PropertyInfo qdev_prop_drive;
 extern PropertyInfo qdev_prop_netdev;
 extern PropertyInfo qdev_prop_vlan;
-- 
2.4.3

[Qemu-devel] [PULL 09/13] fdc: add drive type option

[John Snow]

This patch adds a new explicit Floppy Drive Type option. The existing
behavior in QEMU is to automatically guess a drive type based on the
media inserted, or if a diskette is not present, arbitrarily assign one.

This behavior can be described as "auto." This patch adds the option
to pick an explicit behavior: 120, 144, 288 or none. The new "auto"
option is intended to mimic current behavior, while the other types
pick one explicitly.

Set the type given by the CLI during fd_init. If the type remains the
default (auto), we'll attempt to scan an inserted diskette if present
to determine a type. If auto is selected but no diskette is present,
we fall back to a predetermined default (currently 1.44MB to match
legacy QEMU behavior.)

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 1453495865-9649-9-git-send-email-jsnow@redhat.com
---
 hw/block/fdc.c | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 4caed9b..e89ce2d 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -160,7 +160,6 @@ static FloppyDriveType get_fallback_drive_type(FDrive *drv);
 static void fd_init(FDrive *drv)
 {
     /* Drive */
-    drv->drive = FLOPPY_DRIVE_TYPE_NONE;
     drv->perpendicular = 0;
     /* Disk */
     drv->disk = FLOPPY_DRIVE_TYPE_NONE;
@@ -264,7 +263,7 @@ static int pick_geometry(FDrive *drv)
     int i, first_match, match;
 
     /* We can only pick a geometry if we have a diskette. */
-    if (!drv->media_inserted) {
+    if (!drv->media_inserted || drv->drive == FLOPPY_DRIVE_TYPE_NONE) {
         return -1;
     }
 
@@ -277,7 +276,7 @@ static int pick_geometry(FDrive *drv)
             break;
         }
         if (drv->drive == parse->drive ||
-            drv->drive == FLOPPY_DRIVE_TYPE_NONE) {
+            drv->drive == FLOPPY_DRIVE_TYPE_AUTO) {
             size = (parse->max_head + 1) * parse->max_track *
                 parse->last_sect;
             if (nb_sectors == size) {
@@ -314,11 +313,17 @@ static int pick_geometry(FDrive *drv)
 
 static void pick_drive_type(FDrive *drv)
 {
+    if (drv->drive != FLOPPY_DRIVE_TYPE_AUTO) {
+        return;
+    }
+
     if (pick_geometry(drv) == 0) {
         drv->drive = drv->disk;
     } else {
         drv->drive = get_fallback_drive_type(drv);
     }
+
+    g_assert(drv->drive != FLOPPY_DRIVE_TYPE_AUTO);
 }
 
 /* Revalidate a disk drive after a disk change */
@@ -2475,6 +2480,12 @@ static Property isa_fdc_properties[] = {
     DEFINE_PROP_DRIVE("driveB", FDCtrlISABus, state.drives[1].blk),
     DEFINE_PROP_BIT("check_media_rate", FDCtrlISABus, state.check_media_rate,
                     0, true),
+    DEFINE_PROP_DEFAULT("fdtypeA", FDCtrlISABus, state.drives[0].drive,
+                        FLOPPY_DRIVE_TYPE_AUTO, qdev_prop_fdc_drive_type,
+                        FloppyDriveType),
+    DEFINE_PROP_DEFAULT("fdtypeB", FDCtrlISABus, state.drives[1].drive,
+                        FLOPPY_DRIVE_TYPE_AUTO, qdev_prop_fdc_drive_type,
+                        FloppyDriveType),
     DEFINE_PROP_DEFAULT("fallback", FDCtrlISABus, state.fallback,
                         FLOPPY_DRIVE_TYPE_144, qdev_prop_fdc_drive_type,
                         FloppyDriveType),
@@ -2526,6 +2537,12 @@ static const VMStateDescription vmstate_sysbus_fdc ={
 static Property sysbus_fdc_properties[] = {
     DEFINE_PROP_DRIVE("driveA", FDCtrlSysBus, state.drives[0].blk),
     DEFINE_PROP_DRIVE("driveB", FDCtrlSysBus, state.drives[1].blk),
+    DEFINE_PROP_DEFAULT("fdtypeA", FDCtrlSysBus, state.drives[0].drive,
+                        FLOPPY_DRIVE_TYPE_AUTO, qdev_prop_fdc_drive_type,
+                        FloppyDriveType),
+    DEFINE_PROP_DEFAULT("fdtypeB", FDCtrlSysBus, state.drives[1].drive,
+                        FLOPPY_DRIVE_TYPE_AUTO, qdev_prop_fdc_drive_type,
+                        FloppyDriveType),
     DEFINE_PROP_DEFAULT("fallback", FDCtrlISABus, state.fallback,
                         FLOPPY_DRIVE_TYPE_144, qdev_prop_fdc_drive_type,
                         FloppyDriveType),
@@ -2549,6 +2566,9 @@ static const TypeInfo sysbus_fdc_info = {
 
 static Property sun4m_fdc_properties[] = {
     DEFINE_PROP_DRIVE("drive", FDCtrlSysBus, state.drives[0].blk),
+    DEFINE_PROP_DEFAULT("fdtype", FDCtrlSysBus, state.drives[0].drive,
+                        FLOPPY_DRIVE_TYPE_AUTO, qdev_prop_fdc_drive_type,
+                        FloppyDriveType),
     DEFINE_PROP_DEFAULT("fallback", FDCtrlISABus, state.fallback,
                         FLOPPY_DRIVE_TYPE_144, qdev_prop_fdc_drive_type,
                         FloppyDriveType),
-- 
2.4.3

[Qemu-devel] [PULL 10/13] fdc: add physical disk sizes

[John Snow]

2.88MB capable drives can accept 1.44MB floppies,
for instance. To rework the pick_geometry function,
we need to know if our current drive can even accept
the type of disks we're considering.

NB: This allows us to distinguish between all of the
"total sectors" collisions between 1.20MB and 1.44MB
diskette types, by using the physical drive size as a
differentiator.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 1453495865-9649-10-git-send-email-jsnow@redhat.com
---
 hw/block/fdc.c | 40 ++++++++++++++++++++++++++++++++--------
 1 file changed, 32 insertions(+), 8 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index e89ce2d..e51154b 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -60,6 +60,12 @@ typedef enum FDriveRate {
     FDRIVE_RATE_1M   = 0x03,  /*   1 Mbps */
 } FDriveRate;
 
+typedef enum FDriveSize {
+    FDRIVE_SIZE_UNKNOWN,
+    FDRIVE_SIZE_350,
+    FDRIVE_SIZE_525,
+} FDriveSize;
+
 typedef struct FDFormat {
     FloppyDriveType drive;
     uint8_t last_sect;
@@ -68,11 +74,15 @@ typedef struct FDFormat {
     FDriveRate rate;
 } FDFormat;
 
+/* In many cases, the total sector size of a format is enough to uniquely
+ * identify it. However, there are some total sector collisions between
+ * formats of different physical size, and these are noted below by
+ * highlighting the total sector size for entries with collisions. */
 static const FDFormat fd_formats[] = {
     /* First entry is default format */
     /* 1.44 MB 3"1/2 floppy disks */
-    { FLOPPY_DRIVE_TYPE_144, 18, 80, 1, FDRIVE_RATE_500K, },
-    { FLOPPY_DRIVE_TYPE_144, 20, 80, 1, FDRIVE_RATE_500K, },
+    { FLOPPY_DRIVE_TYPE_144, 18, 80, 1, FDRIVE_RATE_500K, }, /* 3.5" 2880 */
+    { FLOPPY_DRIVE_TYPE_144, 20, 80, 1, FDRIVE_RATE_500K, }, /* 3.5" 3200 */
     { FLOPPY_DRIVE_TYPE_144, 21, 80, 1, FDRIVE_RATE_500K, },
     { FLOPPY_DRIVE_TYPE_144, 21, 82, 1, FDRIVE_RATE_500K, },
     { FLOPPY_DRIVE_TYPE_144, 21, 83, 1, FDRIVE_RATE_500K, },
@@ -86,7 +96,7 @@ static const FDFormat fd_formats[] = {
     { FLOPPY_DRIVE_TYPE_288, 44, 80, 1, FDRIVE_RATE_1M, },
     { FLOPPY_DRIVE_TYPE_288, 48, 80, 1, FDRIVE_RATE_1M, },
     /* 720 kB 3"1/2 floppy disks */
-    { FLOPPY_DRIVE_TYPE_144,  9, 80, 1, FDRIVE_RATE_250K, },
+    { FLOPPY_DRIVE_TYPE_144,  9, 80, 1, FDRIVE_RATE_250K, }, /* 3.5" 1440 */
     { FLOPPY_DRIVE_TYPE_144, 10, 80, 1, FDRIVE_RATE_250K, },
     { FLOPPY_DRIVE_TYPE_144, 10, 82, 1, FDRIVE_RATE_250K, },
     { FLOPPY_DRIVE_TYPE_144, 10, 83, 1, FDRIVE_RATE_250K, },
@@ -94,15 +104,15 @@ static const FDFormat fd_formats[] = {
     { FLOPPY_DRIVE_TYPE_144, 14, 80, 1, FDRIVE_RATE_250K, },
     /* 1.2 MB 5"1/4 floppy disks */
     { FLOPPY_DRIVE_TYPE_120, 15, 80, 1, FDRIVE_RATE_500K, },
-    { FLOPPY_DRIVE_TYPE_120, 18, 80, 1, FDRIVE_RATE_500K, },
+    { FLOPPY_DRIVE_TYPE_120, 18, 80, 1, FDRIVE_RATE_500K, }, /* 5.25" 2880 */
     { FLOPPY_DRIVE_TYPE_120, 18, 82, 1, FDRIVE_RATE_500K, },
     { FLOPPY_DRIVE_TYPE_120, 18, 83, 1, FDRIVE_RATE_500K, },
-    { FLOPPY_DRIVE_TYPE_120, 20, 80, 1, FDRIVE_RATE_500K, },
+    { FLOPPY_DRIVE_TYPE_120, 20, 80, 1, FDRIVE_RATE_500K, }, /* 5.25" 3200 */
     /* 720 kB 5"1/4 floppy disks */
-    { FLOPPY_DRIVE_TYPE_120,  9, 80, 1, FDRIVE_RATE_250K, },
+    { FLOPPY_DRIVE_TYPE_120,  9, 80, 1, FDRIVE_RATE_250K, }, /* 5.25" 1440 */
     { FLOPPY_DRIVE_TYPE_120, 11, 80, 1, FDRIVE_RATE_250K, },
     /* 360 kB 5"1/4 floppy disks */
-    { FLOPPY_DRIVE_TYPE_120,  9, 40, 1, FDRIVE_RATE_300K, },
+    { FLOPPY_DRIVE_TYPE_120,  9, 40, 1, FDRIVE_RATE_300K, }, /* 5.25" 720 */
     { FLOPPY_DRIVE_TYPE_120,  9, 40, 0, FDRIVE_RATE_300K, },
     { FLOPPY_DRIVE_TYPE_120, 10, 41, 1, FDRIVE_RATE_300K, },
     { FLOPPY_DRIVE_TYPE_120, 10, 42, 1, FDRIVE_RATE_300K, },
@@ -110,11 +120,25 @@ static const FDFormat fd_formats[] = {
     { FLOPPY_DRIVE_TYPE_120,  8, 40, 1, FDRIVE_RATE_250K, },
     { FLOPPY_DRIVE_TYPE_120,  8, 40, 0, FDRIVE_RATE_250K, },
     /* 360 kB must match 5"1/4 better than 3"1/2... */
-    { FLOPPY_DRIVE_TYPE_144,  9, 80, 0, FDRIVE_RATE_250K, },
+    { FLOPPY_DRIVE_TYPE_144,  9, 80, 0, FDRIVE_RATE_250K, }, /* 3.5" 720 */
     /* end */
     { FLOPPY_DRIVE_TYPE_NONE, -1, -1, 0, 0, },
 };
 
+__attribute__((__unused__))
+static FDriveSize drive_size(FloppyDriveType drive)
+{
+    switch (drive) {
+    case FLOPPY_DRIVE_TYPE_120:
+        return FDRIVE_SIZE_525;
+    case FLOPPY_DRIVE_TYPE_144:
+    case FLOPPY_DRIVE_TYPE_288:
+        return FDRIVE_SIZE_350;
+    default:
+        return FDRIVE_SIZE_UNKNOWN;
+    }
+}
+
 #define GET_CUR_DRV(fdctrl) ((fdctrl)->cur_drv)
 #define SET_CUR_DRV(fdctrl, drive) ((fdctrl)->cur_drv = (drive))
 
-- 
2.4.3

[Qemu-devel] [PULL 11/13] fdc: rework pick_geometry

[John Snow]

This one is the crazy one.

fd_revalidate currently uses pick_geometry to tell if the diskette
geometry has changed upon an eject/insert event, but it won't allow us
to insert a 1.44MB diskette into a 2.88MB drive. This is inflexible.

The new algorithm applies a new heuristic to guessing disk geometries
that allows us to switch diskette types as long as the physical size
matches before falling back to the old heuristic.

The old one is roughly:
 - If the size (sectors) and type matches, choose it.
 - Fall back to the first geometry that matched our type.

The new one is:
 - If the size (sectors) and type matches, choose it.
 - If the size (sectors) and physical size match, choose it.
 - Fall back to the first geometry that matched our type.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 1453495865-9649-11-git-send-email-jsnow@redhat.com
---
 hw/block/fdc.c | 72 ++++++++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 52 insertions(+), 20 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index e51154b..6e0c5fc 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -125,7 +125,6 @@ static const FDFormat fd_formats[] = {
     { FLOPPY_DRIVE_TYPE_NONE, -1, -1, 0, 0, },
 };
 
-__attribute__((__unused__))
 static FDriveSize drive_size(FloppyDriveType drive)
 {
     switch (drive) {
@@ -284,45 +283,78 @@ static int pick_geometry(FDrive *drv)
     BlockBackend *blk = drv->blk;
     const FDFormat *parse;
     uint64_t nb_sectors, size;
-    int i, first_match, match;
+    int i;
+    int match, size_match, type_match;
+    bool magic = drv->drive == FLOPPY_DRIVE_TYPE_AUTO;
 
     /* We can only pick a geometry if we have a diskette. */
     if (!drv->media_inserted || drv->drive == FLOPPY_DRIVE_TYPE_NONE) {
         return -1;
     }
 
+    /* We need to determine the likely geometry of the inserted medium.
+     * In order of preference, we look for:
+     * (1) The same drive type and number of sectors,
+     * (2) The same diskette size and number of sectors,
+     * (3) The same drive type.
+     *
+     * In all cases, matches that occur higher in the drive table will take
+     * precedence over matches that occur later in the table.
+     */
     blk_get_geometry(blk, &nb_sectors);
-    match = -1;
-    first_match = -1;
+    match = size_match = type_match = -1;
     for (i = 0; ; i++) {
         parse = &fd_formats[i];
         if (parse->drive == FLOPPY_DRIVE_TYPE_NONE) {
             break;
         }
-        if (drv->drive == parse->drive ||
-            drv->drive == FLOPPY_DRIVE_TYPE_AUTO) {
-            size = (parse->max_head + 1) * parse->max_track *
-                parse->last_sect;
-            if (nb_sectors == size) {
-                match = i;
-                break;
+        size = (parse->max_head + 1) * parse->max_track * parse->last_sect;
+        if (nb_sectors == size) {
+            if (magic || parse->drive == drv->drive) {
+                /* (1) perfect match -- nb_sectors and drive type */
+                goto out;
+            } else if (drive_size(parse->drive) == drive_size(drv->drive)) {
+                /* (2) size match -- nb_sectors and physical medium size */
+                match = (match == -1) ? i : match;
+            } else {
+                /* This is suspicious -- Did the user misconfigure? */
+                size_match = (size_match == -1) ? i : size_match;
             }
-            if (first_match == -1) {
-                first_match = i;
+        } else if (type_match == -1) {
+            if ((parse->drive == drv->drive) ||
+                (magic && (parse->drive == get_fallback_drive_type(drv)))) {
+                /* (3) type match -- nb_sectors mismatch, but matches the type
+                 *     specified explicitly by the user, or matches the fallback
+                 *     default type when using the drive autodetect mechanism */
+                type_match = i;
             }
         }
     }
+
+    /* No exact match found */
     if (match == -1) {
-        if (first_match == -1) {
-            error_setg(&error_abort, "No candidate geometries present in table "
-                       " for floppy drive type '%s'",
-                       FloppyDriveType_lookup[drv->drive]);
-        } else {
-            match = first_match;
+        if (size_match != -1) {
+            parse = &fd_formats[size_match];
+            FLOPPY_DPRINTF("User requested floppy drive type '%s', "
+                           "but inserted medium appears to be a "
+                           "%d sector '%s' type\n",
+                           FloppyDriveType_lookup[drv->drive],
+                           nb_sectors,
+                           FloppyDriveType_lookup[parse->drive]);
         }
-        parse = &fd_formats[match];
+        match = type_match;
     }
 
+    /* No match of any kind found -- fd_format is misconfigured, abort. */
+    if (match == -1) {
+        error_setg(&error_abort, "No candidate geometries present in table "
+                   " for floppy drive type '%s'",
+                   FloppyDriveType_lookup[drv->drive]);
+    }
+
+    parse = &(fd_formats[match]);
+
+ out:
     if (parse->max_head == 0) {
         drv->flags &= ~FDISK_DBL_SIDES;
     } else {
-- 
2.4.3

[Qemu-devel] [PULL 12/13] qtest/fdc: Support for 2.88MB drives

[John Snow]

The old test assumes a 1.44MB drive.
Assert that the QEMU default drive is now either 1.44 or 2.88.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 1453495865-9649-12-git-send-email-jsnow@redhat.com
---
 tests/fdc-test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/fdc-test.c b/tests/fdc-test.c
index b5a4696..526d459 100644
--- a/tests/fdc-test.c
+++ b/tests/fdc-test.c
@@ -267,7 +267,7 @@ static void test_cmos(void)
     uint8_t cmos;
 
     cmos = cmos_read(CMOS_FLOPPY);
-    g_assert(cmos == 0x40);
+    g_assert(cmos == 0x40 || cmos == 0x50);
 }
 
 static void test_no_media_on_start(void)
-- 
2.4.3

[Qemu-devel] [PULL 13/13] fdc: change auto fallback drive for ISA FDC to 288

[John Snow]

The 2.88 drive is more suitable as a default because
it can still read 1.44 images correctly, but the reverse
is not true.

Since there exist virtio-win drivers that are shipped on
2.88 floppy images, this patch will allow VMs booted without
a floppy disk inserted to later insert a 2.88MB floppy and
have that work.

This patch has been tested with msdos, freedos, fedora,
windows 8 and windows 10 without issue: if problems do
arise for certain guests being unable to cope with 2.88MB
drives as the default, they are in the minority and can use
type=144 as needed (or insert a proper boot medium and omit
type=144/288 or use type=auto) to obtain different drive types.

As icing, the default will remain auto/144 for any pre-2.6
machine types, hopefully minimizing the impact of this change
in legacy hw to basically zero.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 1453495865-9649-13-git-send-email-jsnow@redhat.com
---
 hw/block/fdc.c      | 2 +-
 include/hw/compat.h | 4 ++++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 6e0c5fc..e3b0e1e 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -2543,7 +2543,7 @@ static Property isa_fdc_properties[] = {
                         FLOPPY_DRIVE_TYPE_AUTO, qdev_prop_fdc_drive_type,
                         FloppyDriveType),
     DEFINE_PROP_DEFAULT("fallback", FDCtrlISABus, state.fallback,
-                        FLOPPY_DRIVE_TYPE_144, qdev_prop_fdc_drive_type,
+                        FLOPPY_DRIVE_TYPE_288, qdev_prop_fdc_drive_type,
                         FloppyDriveType),
     DEFINE_PROP_END_OF_LIST(),
 };
diff --git a/include/hw/compat.h b/include/hw/compat.h
index 491884b..2ebe739 100644
--- a/include/hw/compat.h
+++ b/include/hw/compat.h
@@ -3,6 +3,10 @@
 
 #define HW_COMPAT_2_5 \
     {\
+        .driver   = "isa-fdc",\
+        .property = "fallback",\
+        .value    = "144",\
+    },{\
         .driver   = "pvscsi",\
         .property = "x-old-pci-configuration",\
         .value    = "on",\
-- 
2.4.3

Re: [Qemu-devel] [PULL 00/13] IDE & FDC patches

[Peter Maydell]

On 25 January 2016 at 19:41, John Snow <jsnow@redhat.com> wrote:
> The following changes since commit 6ee06cc3dc7e8eb238e2f60cfd04f094d5c6b948:
>
>   Merge remote-tracking branch 'remotes/lalrae/tags/mips-20160125' into staging (2016-01-25 10:42:52 +0000)
>
> are available in the git repository at:
>
>   https://github.com/jnsnow/qemu.git tags/ide-pull-request
>
> for you to fetch changes up to 4812fa27fa75bce89738a82a191755853dd88408:
>
>   fdc: change auto fallback drive for ISA FDC to 288 (2016-01-25 14:36:01 -0500)
>
> ----------------------------------------------------------------

Applied, thanks.

-- PMM