[Qemu-devel] [RFC 08/10] fdc: refactor 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 heurstic.

This patch attempts to leave all previously working use cases working
identically to how they did before. However, because we can now change
diskette types at runtime, there is now an early return pathway if there
is no diskette to prevent revalidation. We opt instead to leave all
previously known values the same as they were.

This will cause changes in behavior for previous cases where a user with
a 1.44MB floppy drive removed a 20 sector diskette. The old behavior will
revert the CHS to 80/18/2 instead of the previous 80/20/2. This should
hopefully not perturb any guests.

This was changed for this usage case: A 2.88MB drive has a 1.44MB diskette
inserted. The diskette is ejected. Revalidate occurs and changes the CHS
values *and the rate* back to the 2.88MB default. Commands like READ_ID
will now fail unless the user re-programs the rate.

So this patch skips re-alidation for drives with no diskettes to preserve
expected behavior in e.g. fdc-test.

Signed-off-by: John Snow <address@hidden>
---
 hw/block/fdc.c | 77 ++++++++++++++++++++++++++++++++++------------------------
 1 file changed, 45 insertions(+), 32 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 92c81eb..7d4206d 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -121,7 +121,6 @@ static const FDFormat fd_formats[] = {
     { FDRIVE_DRV_NONE, -1, -1, 0, 0, },
 };
 
-__attribute__((__unused__))
 static FDriveSize drive_size(FDriveType drive)
 {
     switch (drive) {
@@ -269,58 +268,70 @@ static void pick_geometry(FDrive *drv, int *nb_heads)
     BlockBackend *blk = drv->blk;
     const FDFormat *parse;
     uint64_t nb_sectors, size;
-    int i, first_match, match;
+    int i;
+    int match;
+    int size_match;
+    int type_match;
 
     /* Pick a default drive type if there's no media inserted AND we have
-     * not yet announced our drive type to the CMOS. */
+     * not yet announced our drive type to the CMOS. Otherwise, we will
+     * follow the legacy QEMU behavior of choosing a drive type based on
+     * the inserted media. */
     if (!blk_is_inserted(blk) && drv->drive == FDRIVE_DRV_NONE) {
-        for (i = 0; ; i++) {
-            parse = &fd_formats[i];
-            if ((parse->drive == FDRIVE_DRV_NONE) ||
-                (parse->drive == get_default_drive_type(drv))) {
-                break;
-            }
-        }
-        goto out;
+        /* Cement our drive type... */
+        drv->drive = get_default_drive_type(drv);
+        /* Let the loop below pick some suitable defaults */
+    } else if (!blk_is_inserted(blk)) {
+        /* Don't wipe out the previous values. */
+        return;
     }
 
     blk_get_geometry(blk, &nb_sectors);
-    match = -1;
-    first_match = -1;
+    match = size_match = type_match = -1;
+
+    /* Bear with me:
+     * 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 number of sectors,
+     * (4) 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.
+     */
     for (i = 0; ; i++) {
         parse = &fd_formats[i];
         if (parse->drive == FDRIVE_DRV_NONE) {
             break;
         }
-        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) {
-                match = i;
-                break;
+        size = (parse->max_head + 1) * parse->max_track * parse->last_sect;
+        if (nb_sectors == size) {
+            if (parse->drive == drv->drive) {
+                /* Perfect! */
+                goto out;
+            } else if (drive_size(parse->drive) == drive_size(drv->drive)) {
+                /* Probably good! */
+                match = (match == -1) ? i : match;
+            } else {
+                /* Starting to not look so great. */
+                size_match = (size_match == -1) ? i : size_match;
             }
-            if (first_match == -1) {
-                first_match = i;
+        } else if ((parse->drive == drv->drive) && type_match == -1) {
+            /* Not great, but matches legacy QEMU behavior. */
+            type_match = i;
             }
         }
-    }
+
     if (match == -1) {
-        if (first_match == -1) {
-            match = 1;
-        } else {
-            match = first_match;
+        match = (size_match != -1) ? size_match : type_match;
         }
-        parse = &fd_formats[match];
-    }
+    parse = &(fd_formats[match]);
 
  out:
     *nb_heads = parse->max_head + 1;
     drv->max_track = parse->max_track;
     drv->last_sect = parse->last_sect;
-    if (drv->drive == FDRIVE_DRV_NONE) {
-        drv->drive = parse->drive;
-    }
     drv->disk = parse->drive;
     drv->media_rate = parse->rate;
 }
@@ -336,11 +347,13 @@ static void fd_revalidate(FDrive *drv)
         pick_geometry(drv, &nb_heads);
         if (!blk_is_inserted(drv->blk)) {
             FLOPPY_DPRINTF("No disk in drive\n");
+            return;
         } else {
             FLOPPY_DPRINTF("Floppy disk (%d h %d t %d s) %s\n", nb_heads,
                            drv->max_track, drv->last_sect,
                            drv->ro ? "ro" : "rw");
         }
+        g_assert_cmpint(nb_heads, !=, -1);
         if (nb_heads == 1) {
             drv->flags &= ~FDISK_DBL_SIDES;
         } else {
-- 
2.1.0