Patchwork [v3,7/9] fdc: check if media rate is correct before doing any transfer

login
register
mail settings
Submitter Hervé Poussineau
Date Jan. 23, 2012, 8:50 a.m.
Message ID <1327308641-14736-8-git-send-email-hpoussin@reactos.org>
Download mbox | patch
Permalink /patch/137299/
State New
Headers show

Comments

Hervé Poussineau - Jan. 23, 2012, 8:50 a.m.
The programmed rate has to be the same as the required rate for the
floppy format ; if that's not the case, the transfer should abort.

Revalidate floppy after migration, so media_rate field doesn't have
to be saved/restored.

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/fdc.c |   25 ++++++++++++++++++++++++-
 1 files changed, 24 insertions(+), 1 deletions(-)
Markus Armbruster - Jan. 27, 2012, 8:56 a.m.
Hervé Poussineau <hpoussin@reactos.org> writes:

> The programmed rate has to be the same as the required rate for the
> floppy format ; if that's not the case, the transfer should abort.
>
> Revalidate floppy after migration, so media_rate field doesn't have
> to be saved/restored.

I'm not at all sure that's a good idea.

The rate is guest state.  It's value depends on the media.  We guess the
media to use for a particular image.  Same thing as for the geometry
(heads, tracks, sectors per track).  We do migrate the geometry, see
vmstate_fdrive.

Why is it a good idea to treat rate differently?

Doesn't your fd_revalidate() in fdc_post_load() clobber the migrated
geometry?

Patch

diff --git a/hw/fdc.c b/hw/fdc.c
index 60acd60..8c200fb 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -83,6 +83,7 @@  typedef struct FDrive {
     uint16_t bps;             /* Bytes per sector       */
     uint8_t ro;               /* Is read-only           */
     uint8_t media_changed;    /* Is media changed       */
+    uint8_t media_rate;       /* Data rate of medium    */
 } FDrive;
 
 static void fd_init(FDrive *drv)
@@ -195,6 +196,7 @@  static void fd_revalidate(FDrive *drv)
         drv->last_sect = last_sect;
         drv->ro = ro;
         drv->drive = drive;
+        drv->media_rate = rate;
     } else {
         FLOPPY_DPRINTF("No disk in drive\n");
         drv->last_sect = 0;
@@ -303,6 +305,7 @@  enum {
 };
 
 enum {
+    FD_SR1_MA       = 0x01, /* Missing address mark */
     FD_SR1_NW       = 0x02, /* Not writable */
     FD_SR1_EC       = 0x80, /* End of cylinder */
 };
@@ -582,6 +585,7 @@  static int fdc_post_load(void *opaque, int version_id)
 
     SET_CUR_DRV(s, s->dor_vmstate & FD_DOR_SELMASK);
     s->dor = s->dor_vmstate & ~FD_DOR_SELMASK;
+    fd_revalidate(get_cur_drv(s));
     return 0;
 }
 
@@ -1077,6 +1081,18 @@  static void fdctrl_start_transfer(FDCtrl *fdctrl, int direction)
         break;
     }
 
+    /* Check the data rate. If the programmed data rate does not match
+     * the currently inserted medium, the operation has to fail. */
+    if ((fdctrl->dsr & FD_DSR_DRATEMASK) != cur_drv->media_rate) {
+        FLOPPY_DPRINTF("data rate mismatch (fdc=%d, media=%d)\n",
+                       fdctrl->dsr & FD_DSR_DRATEMASK, cur_drv->media_rate);
+        fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, FD_SR1_MA, 0x00);
+        fdctrl->fifo[3] = kt;
+        fdctrl->fifo[4] = kh;
+        fdctrl->fifo[5] = ks;
+        return;
+    }
+
     /* Set the FIFO state */
     fdctrl->data_dir = direction;
     fdctrl->data_pos = 0;
@@ -1799,7 +1815,14 @@  static void fdctrl_result_timer(void *opaque)
     if (cur_drv->last_sect != 0) {
         cur_drv->sect = (cur_drv->sect % cur_drv->last_sect) + 1;
     }
-    fdctrl_stop_transfer(fdctrl, 0x00, 0x00, 0x00);
+    /* READ_ID can't automatically succeed! */
+    if ((fdctrl->dsr & FD_DSR_DRATEMASK) != cur_drv->media_rate) {
+        FLOPPY_DPRINTF("read id rate mismatch (fdc=%d, media=%d)\n",
+                       fdctrl->dsr & FD_DSR_DRATEMASK, cur_drv->media_rate);
+        fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, FD_SR1_MA, 0x00);
+    } else {
+        fdctrl_stop_transfer(fdctrl, 0x00, 0x00, 0x00);
+    }
 }
 
 static void fdctrl_change_cb(void *opaque, bool load)