fdc: fix/rewrite seek, media_changed and interrupt handling

PATCH In review Pavel Hrdina 3 participants 12 messages Jun 22, 2012 – Jul 4, 2012 1 message not in the archive

lore

Share & tools

lore

Pavel Hrdina

author

2012-06-22 10:34 UTC #0 lore raw

v6 cover fdc: fix/rewrite seek, media_changed and interrupt handling

The fd_seek will return 'track is invalid' if there is no media in drive.

Implied seek should have the same behavior as normal seek. We will use the fd_seek function instead of a direct modification of head, track and sector values. The result value is used only while read/write commands were issued.

Fix sense interrupt status command.

v6:
- update fdc_test test_media_change
- add fdc-test test_sense_interrupt

v5:
- correction of misspelled words

v4:
- fix coding style
- better bit clearing in 'fdctrl_handle_sense_interrupt_status'

v3:
- add fdc-test test_read_without_media
- rewrite seek and media_changed handling
- fix interrupt status handling

v2:
- better way to detect media missing

Pavel Hrdina (6):
fdc: fix implied seek while there is no media in drive
fdc-test: introduced qtest read_without_media
fdc: rewrite seek and DSKCHG bit handling
fdc: fix interrupt handling
fdc_test: update media_change test
fdc_test: introduce test_sense_interrupt

 hw/fdc.c         |  117 ++++++++++++++++++++++++++++++-----------------------
 tests/fdc-test.c |  118 ++++++++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 176 insertions(+), 59 deletions(-)

Signature · 1 line

1.7.7.6

Pavel Hrdina

author

2012-06-22 10:34 UTC #1 lore raw

PATCH v6 1/6 fdc: fix implied seek while there is no media in drive

The Windows uses 'READ' command at the start of an instalation without checking the 'dir' register. We have to abort the transfer with an abnormal termination if there is no media in the drive.

Signed-off-by Pavel Hrdina <[email protected]>

---
 hw/fdc.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

hw/fdc.c +4 -0

index 30d34e3..be35201 100644
@@ -159,6 +159,10 @@ static int fd_seek(FDrive *drv, uint8_t head, uint8_t track, uint8_t sect,
         drv->sect = sect;
     }
 
+    if (drv->bs == NULL || !bdrv_is_inserted(drv->bs)) {
+        ret = 2;
+    }
+
     return ret;
 }

Signature · 1 line

1.7.7.6

Pavel Hrdina

author

2012-06-22 10:34 UTC #2 lore raw

PATCH v6 2/6 fdc-test: introduced qtest read_without_media

In reply to Pavel Hrdina

If you try to read from a floppy drive without a media, you should get an abnormal termination error.

Signed-off-by Pavel Hrdina <[email protected]>

---
 tests/fdc-test.c |   66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 66 insertions(+), 0 deletions(-)

tests/fdc-test.c +66 -0

index 22d24ac..e730398 100644
@@ -49,6 +49,7 @@ enum {
 enum {
     CMD_SENSE_INT   = 0x08,
     CMD_SEEK        = 0x0f,
+    CMD_READ        = 0xe6,
 };
 
 enum {

@@ -99,6 +100,62 @@ static void ack_irq(void)
     g_assert(!get_irq(FLOPPY_IRQ));
 }
 
+static uint8_t send_read_command(void)
+{
+    uint8_t drive = 0;
+    uint8_t head = 0;
+    uint8_t cyl = 0;
+    uint8_t sect_addr = 1;
+    uint8_t sect_size = 2;
+    uint8_t eot = 1;
+    uint8_t gap = 0x1b;
+    uint8_t gpl = 0xff;
+
+    uint8_t msr = 0;
+    uint8_t st0;
+
+    uint8_t ret = 0;
+
+    floppy_send(CMD_READ);
+    floppy_send(head << 2 | drive);
+    g_assert(!get_irq(FLOPPY_IRQ));
+    floppy_send(cyl);
+    floppy_send(head);
+    floppy_send(sect_addr);
+    floppy_send(sect_size);
+    floppy_send(eot);
+    floppy_send(gap);
+    floppy_send(gpl);
+
+    uint8_t i = 0;
+    uint8_t n = 2;
+    for (; i < n; i++) {
+        msr = inb(FLOPPY_BASE + reg_msr);
+        if (msr == 0xd0) {
+            break;
+        }
+        sleep(1);
+    }
+
+    if (i >= n) {
+        return 1;
+    }
+
+    st0 = floppy_recv();
+    if (st0 != 0x40) {
+        ret = 1;
+    }
+
+    floppy_recv();
+    floppy_recv();
+    floppy_recv();
+    floppy_recv();
+    floppy_recv();
+    floppy_recv();
+
+    return ret;
+}
+
 static void send_step_pulse(void)
 {
     int drive = 0;

@@ -146,6 +203,14 @@ static void test_no_media_on_start(void)
     assert_bit_set(dir, DSKCHG);
 }
 
+static void test_read_without_media(void)
+{
+    uint8_t ret;
+
+    ret = send_read_command();
+    g_assert(ret == 0);
+}
+
 static void test_media_change(void)
 {
     uint8_t dir;

@@ -214,6 +279,7 @@ int main(int argc, char **argv)
     qtest_irq_intercept_in(global_qtest, "ioapic");
     qtest_add_func("/fdc/cmos", test_cmos);
     qtest_add_func("/fdc/no_media_on_start", test_no_media_on_start);
+    qtest_add_func("/fdc/read_without_media", test_read_without_media);
     qtest_add_func("/fdc/media_change", test_media_change);
 
     ret = g_test_run();

Signature · 1 line

1.7.7.6

Pavel Hrdina

author

2012-06-22 10:34 UTC #3 lore raw

PATCH v6 3/6 fdc: rewrite seek and DSKCHG bit handling

In reply to Pavel Hrdina

This bit is cleared on every successful seek to a different track (cylinder). The seek is also called on revalidate or on read/write/format commands which also clear the DSKCHG bit.

Signed-off-by Pavel Hrdina <[email protected]>

---
 hw/fdc.c |   79 ++++++++++++++++++++++++++++++++-----------------------------
 1 files changed, 41 insertions(+), 38 deletions(-)

hw/fdc.c +41 ~~-38~~

index 5b3224b..0270264 100644
@@ -153,8 +153,12 @@ static int fd_seek(FDrive *drv, uint8_t head, uint8_t track, uint8_t sect,
         }
 #endif
         drv->head = head;
-        if (drv->track != track)
+        if (drv->track != track) {
+            if (drv->bs != NULL && bdrv_is_inserted(drv->bs)) {
+                drv->media_changed = 0;
+            }
             ret = 1;
+        }
         drv->track = track;
         drv->sect = sect;
     }

@@ -170,9 +174,7 @@ static int fd_seek(FDrive *drv, uint8_t head, uint8_t track, uint8_t sect,
 static void fd_recalibrate(FDrive *drv)
 {
     FLOPPY_DPRINTF("recalibrate\n");
-    drv->head = 0;
-    drv->track = 0;
-    drv->sect = 1;
+    fd_seek(drv, 0, 0, 1, 1);
 }
 
 /* Revalidate a disk drive after a disk change */

@@ -711,14 +713,6 @@ static void fdctrl_raise_irq(FDCtrl *fdctrl, uint8_t status0)
         qemu_set_irq(fdctrl->irq, 1);
         fdctrl->sra |= FD_SRA_INTPEND;
     }
-    if (status0 & FD_SR0_SEEK) {
-        FDrive *cur_drv;
-        /* A seek clears the disk change line (if a disk is inserted) */
-        cur_drv = get_cur_drv(fdctrl);
-        if (cur_drv->bs != NULL && bdrv_is_inserted(cur_drv->bs)) {
-            cur_drv->media_changed = 0;
-        }
-    }
 
     fdctrl->reset_sensei = 0;
     fdctrl->status0 = status0;

@@ -997,7 +991,10 @@ static void fdctrl_unimplemented(FDCtrl *fdctrl, int direction)
     fdctrl_set_fifo(fdctrl, 1, 0);
 }
 
-/* Seek to next sector */
+/* Seek to next sector
+ * returns 0 when end of track reached (for DBL_SIDES on head 1)
+ * otherwise returns 1
+ */
 static int fdctrl_seek_to_next_sect(FDCtrl *fdctrl, FDrive *cur_drv)
 {
     FLOPPY_DPRINTF("seek to next sector (%d %02x %02x => %d)\n",

@@ -1005,30 +1002,39 @@ static int fdctrl_seek_to_next_sect(FDCtrl *fdctrl, FDrive *cur_drv)
                    fd_sector(cur_drv));
     /* XXX: cur_drv->sect >= cur_drv->last_sect should be an
        error in fact */
-    if (cur_drv->sect >= cur_drv->last_sect ||
-        cur_drv->sect == fdctrl->eot) {
-        cur_drv->sect = 1;
+    uint8_t new_head = cur_drv->head;
+    uint8_t new_track = cur_drv->track;
+    uint8_t new_sect = cur_drv->sect;
+
+    int ret = 1;
+
+    if (new_sect >= cur_drv->last_sect ||
+        new_sect == fdctrl->eot) {
+        new_sect = 1;
         if (FD_MULTI_TRACK(fdctrl->data_state)) {
-            if (cur_drv->head == 0 &&
+            if (new_head == 0 &&
                 (cur_drv->flags & FDISK_DBL_SIDES) != 0) {
-                cur_drv->head = 1;
+                new_head = 1;
             } else {
-                cur_drv->head = 0;
-                cur_drv->track++;
-                if ((cur_drv->flags & FDISK_DBL_SIDES) == 0)
-                    return 0;
+                new_head = 0;
+                new_track++;
+                if ((cur_drv->flags & FDISK_DBL_SIDES) == 0) {
+                    ret = 0;
+                }
             }
         } else {
-            cur_drv->track++;
-            return 0;
+            new_track++;
+            ret = 0;
+        }
+        if (ret == 1) {
+            FLOPPY_DPRINTF("seek to next track (%d %02x %02x => %d)\n",
+                    new_head, new_track, new_sect, fd_sector(cur_drv));
         }
-        FLOPPY_DPRINTF("seek to next track (%d %02x %02x => %d)\n",
-                       cur_drv->head, cur_drv->track,
-                       cur_drv->sect, fd_sector(cur_drv));
     } else {
-        cur_drv->sect++;
+        new_sect++;
     }
-    return 1;
+    fd_seek(cur_drv, new_head, new_track, new_sect, 1);
+    return ret;
 }
 
 /* Callback for transfer end (stop or abort) */

@@ -1626,11 +1632,7 @@ static void fdctrl_handle_seek(FDCtrl *fdctrl, int direction)
     /* The seek command just sends step pulses to the drive and doesn't care if
      * there is a medium inserted of if it's banging the head against the drive.
      */
-    if (fdctrl->fifo[2] > cur_drv->max_track) {
-        cur_drv->track = cur_drv->max_track;
-    } else {
-        cur_drv->track = fdctrl->fifo[2];
-    }
+    fd_seek(cur_drv, cur_drv->head, fdctrl->fifo[2], cur_drv->sect, 1);
     /* Raise Interrupt */
     fdctrl_raise_irq(fdctrl, FD_SR0_SEEK);
 }

@@ -1695,9 +1697,10 @@ static void fdctrl_handle_relative_seek_out(FDCtrl *fdctrl, int direction)
     SET_CUR_DRV(fdctrl, fdctrl->fifo[1] & FD_DOR_SELMASK);
     cur_drv = get_cur_drv(fdctrl);
     if (fdctrl->fifo[2] + cur_drv->track >= cur_drv->max_track) {
-        cur_drv->track = cur_drv->max_track - 1;
+        fd_seek(cur_drv, cur_drv->head, cur_drv->max_track - 1,
+                cur_drv->sect, 1);
     } else {
-        cur_drv->track += fdctrl->fifo[2];
+        fd_seek(cur_drv, cur_drv->head, fdctrl->fifo[2], cur_drv->sect, 1);
     }
     fdctrl_reset_fifo(fdctrl);
     /* Raise Interrupt */

@@ -1711,9 +1714,9 @@ static void fdctrl_handle_relative_seek_in(FDCtrl *fdctrl, int direction)
     SET_CUR_DRV(fdctrl, fdctrl->fifo[1] & FD_DOR_SELMASK);
     cur_drv = get_cur_drv(fdctrl);
     if (fdctrl->fifo[2] > cur_drv->track) {
-        cur_drv->track = 0;
+        fd_seek(cur_drv, cur_drv->head, 0, cur_drv->sect, 1);
     } else {
-        cur_drv->track -= fdctrl->fifo[2];
+        fd_seek(cur_drv, cur_drv->head, fdctrl->fifo[2], cur_drv->sect, 1);
     }
     fdctrl_reset_fifo(fdctrl);
     /* Raise Interrupt */

Signature · 1 line

1.7.7.6

Pavel Hrdina

author

2012-06-22 10:34 UTC #4 lore raw

PATCH v6 4/6 fdc: fix interrupt handling

In reply to Pavel Hrdina

If you call the SENSE INTERRUPT STATUS command while there is no interrupt waiting you get as result unknown command.

Fixed status0 register handling for read/write/format commands.

Signed-off-by Pavel Hrdina <[email protected]>

---
 hw/fdc.c |   34 +++++++++++++++++++++-------------
 1 files changed, 21 insertions(+), 13 deletions(-)

hw/fdc.c +21 ~~-13~~

index 0270264..e28841c 100644
@@ -307,6 +307,9 @@ enum {
 };
 
 enum {
+    FD_SR0_DS0      = 0x01,
+    FD_SR0_DS1      = 0x02,
+    FD_SR0_HEAD     = 0x04,
     FD_SR0_EQPMT    = 0x10,
     FD_SR0_SEEK     = 0x20,
     FD_SR0_ABNTERM  = 0x40,

@@ -972,14 +975,15 @@ static void fdctrl_reset_fifo(FDCtrl *fdctrl)
 }
 
 /* Set FIFO status for the host to read */
-static void fdctrl_set_fifo(FDCtrl *fdctrl, int fifo_len, int do_irq)
+static void fdctrl_set_fifo(FDCtrl *fdctrl, int fifo_len, uint8_t status0)
 {
     fdctrl->data_dir = FD_DIR_READ;
     fdctrl->data_len = fifo_len;
     fdctrl->data_pos = 0;
     fdctrl->msr |= FD_MSR_CMDBUSY | FD_MSR_RQM | FD_MSR_DIO;
-    if (do_irq)
-        fdctrl_raise_irq(fdctrl, 0x00);
+    if (status0) {
+        fdctrl_raise_irq(fdctrl, status0);
+    }
 }
 
 /* Set an error: unimplemented/unknown command */

@@ -1044,10 +1048,12 @@ static void fdctrl_stop_transfer(FDCtrl *fdctrl, uint8_t status0,
     FDrive *cur_drv;
 
     cur_drv = get_cur_drv(fdctrl);
+    fdctrl->status0 = status0 | FD_SR0_SEEK | (cur_drv->head << 2) |
+                      GET_CUR_DRV(fdctrl);
+
     FLOPPY_DPRINTF("transfer status: %02x %02x %02x (%02x)\n",
-                   status0, status1, status2,
-                   status0 | (cur_drv->head << 2) | GET_CUR_DRV(fdctrl));
-    fdctrl->fifo[0] = status0 | (cur_drv->head << 2) | GET_CUR_DRV(fdctrl);
+                   status0, status1, status2, fdctrl->status0);
+    fdctrl->fifo[0] = fdctrl->status0;
     fdctrl->fifo[1] = status1;
     fdctrl->fifo[2] = status2;
     fdctrl->fifo[3] = cur_drv->track;

@@ -1060,7 +1066,7 @@ static void fdctrl_stop_transfer(FDCtrl *fdctrl, uint8_t status0,
     }
     fdctrl->msr |= FD_MSR_RQM | FD_MSR_DIO;
     fdctrl->msr &= ~FD_MSR_NONDMA;
-    fdctrl_set_fifo(fdctrl, 7, 1);
+    fdctrl_set_fifo(fdctrl, 7, fdctrl->status0);
 }
 
 /* Prepare a data transfer (either DMA or FIFO) */

@@ -1175,7 +1181,7 @@ static void fdctrl_start_transfer(FDCtrl *fdctrl, int direction)
     if (direction != FD_DIR_WRITE)
         fdctrl->msr |= FD_MSR_DIO;
     /* IO based transfer: calculate len */
-    fdctrl_raise_irq(fdctrl, 0x00);
+    fdctrl_raise_irq(fdctrl, FD_SR0_SEEK);
 
     return;
 }

@@ -1604,16 +1610,18 @@ static void fdctrl_handle_sense_interrupt_status(FDCtrl *fdctrl, int direction)
 {
     FDrive *cur_drv = get_cur_drv(fdctrl);
 
-    if(fdctrl->reset_sensei > 0) {
+    if (fdctrl->reset_sensei > 0) {
         fdctrl->fifo[0] =
             FD_SR0_RDYCHG + FD_RESET_SENSEI_COUNT - fdctrl->reset_sensei;
         fdctrl->reset_sensei--;
+    } else if (!(fdctrl->sra & FD_SRA_INTPEND)) {
+        fdctrl->fifo[0] = FD_SR0_INVCMD;
+        fdctrl_set_fifo(fdctrl, 1, 0);
+        return;
     } else {
-        /* XXX: status0 handling is broken for read/write
-           commands, so we do this hack. It should be suppressed
-           ASAP */
         fdctrl->fifo[0] =
-            FD_SR0_SEEK | (cur_drv->head << 2) | GET_CUR_DRV(fdctrl);
+                (fdctrl->status0 & ~(FD_SR0_HEAD | FD_SR0_DS1 | FD_SR0_DS0))
+                | GET_CUR_DRV(fdctrl);
     }
 
     fdctrl->fifo[1] = cur_drv->track;

Signature · 1 line

1.7.7.6

Pavel Hrdina

author

2012-06-22 10:34 UTC #5 lore raw

PATCH v6 5/6 fdc_test: update media_change test

In reply to Pavel Hrdina

After rewrite DSKCHG bit handling the test has to be updated. Now is needed to seek to different track to clear DSKCHG bit.

Signed-off-by Pavel Hrdina <[email protected]>

---
 tests/fdc-test.c |   29 +++++++++++++++++++++--------
 1 files changed, 21 insertions(+), 8 deletions(-)

tests/fdc-test.c +21 -8

index 610e2f1..5280eff 100644
@@ -156,19 +156,20 @@ static uint8_t send_read_command(void)
     return ret;
 }
 
-static void send_step_pulse(void)
+static void send_step_pulse(bool chg_cyl)
 {
     int drive = 0;
     int head = 0;
-    static int cyl = 0;
+    int cyl = 0;
+
+    if (chg_cyl)
+        cyl = (cyl + 1) % 4;
 
     floppy_send(CMD_SEEK);
     floppy_send(head << 2 | drive);
     g_assert(!get_irq(FLOPPY_IRQ));
     floppy_send(cyl);
     ack_irq();
-
-    cyl = (cyl + 1) % 4;
 }
 
 static uint8_t cmos_read(uint8_t reg)

@@ -195,8 +196,7 @@ static void test_no_media_on_start(void)
     assert_bit_set(dir, DSKCHG);
     dir = inb(FLOPPY_BASE + reg_dir);
     assert_bit_set(dir, DSKCHG);
-    send_step_pulse();
-    send_step_pulse();
+    send_step_pulse(1);
     dir = inb(FLOPPY_BASE + reg_dir);
     assert_bit_set(dir, DSKCHG);
     dir = inb(FLOPPY_BASE + reg_dir);

@@ -227,7 +227,14 @@ static void test_media_change(void)
     dir = inb(FLOPPY_BASE + reg_dir);
     assert_bit_set(dir, DSKCHG);
 
-    send_step_pulse();
+    send_step_pulse(0);
+    dir = inb(FLOPPY_BASE + reg_dir);
+    assert_bit_set(dir, DSKCHG);
+    dir = inb(FLOPPY_BASE + reg_dir);
+    assert_bit_set(dir, DSKCHG);
+
+    /* Step to next track should clear DSKCHG bit. */
+    send_step_pulse(1);
     dir = inb(FLOPPY_BASE + reg_dir);
     assert_bit_clear(dir, DSKCHG);
     dir = inb(FLOPPY_BASE + reg_dir);

@@ -243,7 +250,13 @@ static void test_media_change(void)
     dir = inb(FLOPPY_BASE + reg_dir);
     assert_bit_set(dir, DSKCHG);
 
-    send_step_pulse();
+    send_step_pulse(0);
+    dir = inb(FLOPPY_BASE + reg_dir);
+    assert_bit_set(dir, DSKCHG);
+    dir = inb(FLOPPY_BASE + reg_dir);
+    assert_bit_set(dir, DSKCHG);
+
+    send_step_pulse(1);
     dir = inb(FLOPPY_BASE + reg_dir);
     assert_bit_set(dir, DSKCHG);
     dir = inb(FLOPPY_BASE + reg_dir);

Signature · 1 line

1.7.7.6

Blue Swirl

2012-06-24 06:17 UTC #6 lore raw

Re: [Qemu-devel] [PATCH v6 5/6] fdc_test: update media_change test

Quoted 16 lines · Pavel Hrdina

After rewrite DSKCHG bit handling the test has to be updated. Now is needed to seek to different track to clear DSKCHG bit.
Signed-off-by Pavel Hrdina <[email protected]>
---
tests/fdc-test.c | 29 +++++++++++++++++++++--------
 1 files changed, 21 insertions(+), 8 deletions(-)
tests/fdc-test.c +0 -0
index 610e2f1..5280eff 100644
@@ -156,19 +156,20 @@ static uint8_t send_read_command(void)
     return ret;
  }
 

tests/fdc-test.c static uint8_t send_read_command(void) · line 166

Quoted 10 lines

tests/fdc-test.c +5 -2

-static void send_step_pulse(void)
+static void send_step_pulse(bool chg_cyl)
  {
     int drive = 0;
     int head = 0;
-    static int cyl = 0;
+    int cyl = 0;
+
+    if (chg_cyl)
+        cyl = (cyl + 1) % 4;

Missing braces, please use checkpatch.pl to avoid these issues.

% 4 could be turned into & 3, maybe with a separate patch.

Quoted 56 lines

tests/fdc-test.c +16 -7

     floppy_send(CMD_SEEK);
     floppy_send(head << 2 | drive);
     g_assert(!get_irq(FLOPPY_IRQ));
     floppy_send(cyl);
     ack_irq();
-
-    cyl = (cyl + 1) % 4;
  }
 
  static uint8_t cmos_read(uint8_t reg)

@@ -195,8 +196,7 @@ static void test_no_media_on_start(void)
     assert_bit_set(dir, DSKCHG);
     dir = inb(FLOPPY_BASE + reg_dir);
     assert_bit_set(dir, DSKCHG);
-    send_step_pulse();
-    send_step_pulse();
+    send_step_pulse(1);
     dir = inb(FLOPPY_BASE + reg_dir);
     assert_bit_set(dir, DSKCHG);
     dir = inb(FLOPPY_BASE + reg_dir);

@@ -227,7 +227,14 @@ static void test_media_change(void)
     dir = inb(FLOPPY_BASE + reg_dir);
     assert_bit_set(dir, DSKCHG);
 
-    send_step_pulse();
+    send_step_pulse(0);
+    dir = inb(FLOPPY_BASE + reg_dir);
+    assert_bit_set(dir, DSKCHG);
+    dir = inb(FLOPPY_BASE + reg_dir);
+    assert_bit_set(dir, DSKCHG);
+
+    /* Step to next track should clear DSKCHG bit. */
+    send_step_pulse(1);
     dir = inb(FLOPPY_BASE + reg_dir);
     assert_bit_clear(dir, DSKCHG);
     dir = inb(FLOPPY_BASE + reg_dir);

@@ -243,7 +250,13 @@ static void test_media_change(void)
     dir = inb(FLOPPY_BASE + reg_dir);
     assert_bit_set(dir, DSKCHG);
 
-    send_step_pulse();
+    send_step_pulse(0);
+    dir = inb(FLOPPY_BASE + reg_dir);
+    assert_bit_set(dir, DSKCHG);
+    dir = inb(FLOPPY_BASE + reg_dir);
+    assert_bit_set(dir, DSKCHG);
+
+    send_step_pulse(1);
     dir = inb(FLOPPY_BASE + reg_dir);
     assert_bit_set(dir, DSKCHG);
     dir = inb(FLOPPY_BASE + reg_dir);
--

1.7.7.6

A message that is not in the archive (sent off-list or not yet received): [email protected]

Pavel Hrdina

author

2012-07-03 14:43 UTC #8 lore raw

Re: [Qemu-devel] [PATCH v6 5/6] fdc_test: update media_change test

Quoted 31 lines · Kevin Wolf

Am 22.06.2012 12:33, schrieb Pavel Hrdina:
Quoted 16 lines
After rewrite DSKCHG bit handling the test has to be updated. Now is needed to seek to different track to clear DSKCHG bit.
Signed-off-by Pavel Hrdina<[email protected]>
---
  tests/fdc-test.c |   29 +++++++++++++++++++++--------
  1 files changed, 21 insertions(+), 8 deletions(-)
tests/fdc-test.c +0 -0
index 610e2f1..5280eff 100644
@@ -156,19 +156,20 @@ static uint8_t send_read_command(void)
      return ret;
  }
 
tests/fdc-test.c static uint8_t send_read_command(void) · line 166
Quoted 10 lines
tests/fdc-test.c +5 -2
-static void send_step_pulse(void)
+static void send_step_pulse(bool chg_cyl)
  {
      int drive = 0;
      int head = 0;
-    static int cyl = 0;
+    int cyl = 0;
+
+    if (chg_cyl)
+        cyl = (cyl + 1) % 4;
Why do you remove the static? Previously, the function would cycle through cylinders 1-4. Now it's always 0 if !chg_cyl and 1 if chg_cyl. I think you need to fix this.

Quoted 2 lines

I also don't quite understand why you move the increment to here instead of leaving it as the bottom. It doesn't look wrong, but pointless.

Now the functionality is different. I need seek to cylinder 0 or 1. After every change of media an actual cylinder is set to 0 and I need to try seek to cylinder 0 to test, that after this seek the DSKCHG bit is still set. Then I need to seek to different cylinder to reset the DSKCHG and for that seek to 1 is enough.

I'll chagne the

cyl = (cyl + 1) % 4; to cyl = 1;

Quoted 20 lines

Quoted 12 lines

tests/fdc-test.c +0 -2

      floppy_send(CMD_SEEK);
      floppy_send(head<<  2 | drive);
      g_assert(!get_irq(FLOPPY_IRQ));
      floppy_send(cyl);
      ack_irq();
-
-    cyl = (cyl + 1) % 4;
  }
 
  static uint8_t cmos_read(uint8_t reg)

tests/fdc-test.c static void test_no_media_on_start(void) · line 199

Quoted 6 lines

tests/fdc-test.c +1 -2

@@ -195,8 +196,7 @@ static void test_no_media_on_start(void)
      assert_bit_set(dir, DSKCHG);
      dir = inb(FLOPPY_BASE + reg_dir);
      assert_bit_set(dir, DSKCHG);
-    send_step_pulse();
-    send_step_pulse();
+    send_step_pulse(1);

This is a bool parameter, so you should pass true/false instead of 0/1.

Quoted 6 lines

Other than that, the test case additions look good and I'll accept it as sufficient for the DSKCHG handling change. It would be great, though, to add some more cases that test the implicit seeks during commands like READ (ideally one for each command that can seek).
Kevin

I'll fix the bool parameter and then I'll create new patches for more test cases.

Pavel

Pavel Hrdina

author

2012-06-22 10:34 UTC #9 lore raw

PATCH v6 6/6 fdc_test: introduce test_sense_interrupt

In reply to Pavel Hrdina

Calling sense interrupt status while there is no interrupt should return invalid command (0x80).

Read command should always returns in st0 seek_end bit set to 1.

Signed-off-by Pavel Hrdina <[email protected]>

---
 tests/fdc-test.c |   25 ++++++++++++++++++++++++-
 1 files changed, 24 insertions(+), 1 deletions(-)

tests/fdc-test.c +24 -1

index 5280eff..be14d83 100644
@@ -142,7 +142,7 @@ static uint8_t send_read_command(void)
     }
 
     st0 = floppy_recv();
-    if (st0 != 0x40) {
+    if (st0 != 0x60) {
         ret = 1;
     }

@@ -263,6 +263,28 @@ static void test_media_change(void)
     assert_bit_set(dir, DSKCHG);
 }
 
+static void test_sense_interrupt(void)
+{
+    int drive = 0;
+    int head = 0;
+    int cyl = 0;
+    int ret = 0;
+
+    floppy_send(CMD_SENSE_INT);
+    ret = floppy_recv();
+    g_assert(ret == 0x80);
+
+    floppy_send(CMD_SEEK);
+    floppy_send(head << 2 | drive);
+    g_assert(!get_irq(FLOPPY_IRQ));
+    floppy_send(cyl);
+
+    floppy_send(CMD_SENSE_INT);
+    ret = floppy_recv();
+    g_assert(ret != 0x80);
+    floppy_recv();
+}
+
 /* success if no crash or abort */
 static void fuzz_registers(void)
 {

@@ -310,6 +332,7 @@ int main(int argc, char **argv)
     qtest_add_func("/fdc/no_media_on_start", test_no_media_on_start);
     qtest_add_func("/fdc/read_without_media", test_read_without_media);
     qtest_add_func("/fdc/media_change", test_media_change);
+    qtest_add_func("/fdc/sense_interrupt", test_sense_interrupt);
     qtest_add_func("/fdc/fuzz-registers", fuzz_registers);
 
     ret = g_test_run();

Signature · 1 line

1.7.7.6

Pavel Hrdina

author

2012-07-04 09:07 UTC #10 lore raw

PATCH v7 5/6 fdc_test: update media_change test

In reply to Pavel Hrdina

After rewrite DSKCHG bit handling the test has to be updated. Now is needed to seek to different track to clear DSKCHG bit.

Signed-off-by Pavel Hrdina <[email protected]>

---
 tests/fdc-test.c |   30 ++++++++++++++++++++++--------
 1 files changed, 22 insertions(+), 8 deletions(-)

tests/fdc-test.c +22 -8

index 610e2f1..f865576 100644
@@ -156,19 +156,21 @@ static uint8_t send_read_command(void)
     return ret;
 }
 
-static void send_step_pulse(void)
+static void send_step_pulse(bool chg_cyl)
 {
     int drive = 0;
     int head = 0;
-    static int cyl = 0;
+    int cyl = 0;
+
+    if (chg_cyl) {
+        cyl = 1;
+    }
 
     floppy_send(CMD_SEEK);
     floppy_send(head << 2 | drive);
     g_assert(!get_irq(FLOPPY_IRQ));
     floppy_send(cyl);
     ack_irq();
-
-    cyl = (cyl + 1) % 4;
 }
 
 static uint8_t cmos_read(uint8_t reg)

@@ -195,8 +197,7 @@ static void test_no_media_on_start(void)
     assert_bit_set(dir, DSKCHG);
     dir = inb(FLOPPY_BASE + reg_dir);
     assert_bit_set(dir, DSKCHG);
-    send_step_pulse();
-    send_step_pulse();
+    send_step_pulse(true);
     dir = inb(FLOPPY_BASE + reg_dir);
     assert_bit_set(dir, DSKCHG);
     dir = inb(FLOPPY_BASE + reg_dir);

@@ -227,7 +228,14 @@ static void test_media_change(void)
     dir = inb(FLOPPY_BASE + reg_dir);
     assert_bit_set(dir, DSKCHG);
 
-    send_step_pulse();
+    send_step_pulse(false);
+    dir = inb(FLOPPY_BASE + reg_dir);
+    assert_bit_set(dir, DSKCHG);
+    dir = inb(FLOPPY_BASE + reg_dir);
+    assert_bit_set(dir, DSKCHG);
+
+    /* Step to next track should clear DSKCHG bit. */
+    send_step_pulse(true);
     dir = inb(FLOPPY_BASE + reg_dir);
     assert_bit_clear(dir, DSKCHG);
     dir = inb(FLOPPY_BASE + reg_dir);

@@ -243,7 +251,13 @@ static void test_media_change(void)
     dir = inb(FLOPPY_BASE + reg_dir);
     assert_bit_set(dir, DSKCHG);
 
-    send_step_pulse();
+    send_step_pulse(false);
+    dir = inb(FLOPPY_BASE + reg_dir);
+    assert_bit_set(dir, DSKCHG);
+    dir = inb(FLOPPY_BASE + reg_dir);
+    assert_bit_set(dir, DSKCHG);
+
+    send_step_pulse(true);
     dir = inb(FLOPPY_BASE + reg_dir);
     assert_bit_set(dir, DSKCHG);
     dir = inb(FLOPPY_BASE + reg_dir);

Signature · 1 line

1.7.7.6

Kevin Wolf

Applied

2012-07-04 14:44 UTC #11 lore raw

Re: [Qemu-devel] [PATCH v8 5/6] fdc_test: update media_change test

Am 04.07.2012 16:26, schrieb Pavel Hrdina:

Quoted 7 lines

After rewrite DSKCHG bit handling the test has to be updated. Now is needed to seek to different track to clear DSKCHG bit.
Signed-off-by Pavel Hrdina <[email protected]>
---
 tests/fdc-test.c |   30 ++++++++++++++++++++++--------
 1 files changed, 22 insertions(+), 8 deletions(-)

Thanks, applied all.

Kevin

Pavel Hrdina

author

2012-07-04 09:07 UTC #12 lore raw

PATCH v7 6/6 fdc_test: introduce test_sense_interrupt

In reply to Pavel Hrdina

Calling sense interrupt status while there is no interrupt should return invalid command (0x80).

Read command should always returns in st0 seek_end bit set to 1.

Signed-off-by Pavel Hrdina <[email protected]>

---
 tests/fdc-test.c |   25 ++++++++++++++++++++++++-
 1 files changed, 24 insertions(+), 1 deletions(-)

tests/fdc-test.c +24 -1

index f865576..db83103 100644
@@ -142,7 +142,7 @@ static uint8_t send_read_command(void)
     }
 
     st0 = floppy_recv();
-    if (st0 != 0x40) {
+    if (st0 != 0x60) {
         ret = 1;
     }

@@ -264,6 +264,28 @@ static void test_media_change(void)
     assert_bit_set(dir, DSKCHG);
 }
 
+static void test_sense_interrupt(void)
+{
+    int drive = 0;
+    int head = 0;
+    int cyl = 0;
+    int ret = 0;
+
+    floppy_send(CMD_SENSE_INT);
+    ret = floppy_recv();
+    g_assert(ret == 0x80);
+
+    floppy_send(CMD_SEEK);
+    floppy_send(head << 2 | drive);
+    g_assert(!get_irq(FLOPPY_IRQ));
+    floppy_send(cyl);
+
+    floppy_send(CMD_SENSE_INT);
+    ret = floppy_recv();
+    g_assert(ret != 0x80);
+    floppy_recv();
+}
+
 /* success if no crash or abort */
 static void fuzz_registers(void)
 {

@@ -311,6 +333,7 @@ int main(int argc, char **argv)
     qtest_add_func("/fdc/no_media_on_start", test_no_media_on_start);
     qtest_add_func("/fdc/read_without_media", test_read_without_media);
     qtest_add_func("/fdc/media_change", test_media_change);
+    qtest_add_func("/fdc/sense_interrupt", test_sense_interrupt);
     qtest_add_func("/fdc/fuzz-registers", fuzz_registers);
 
     ret = g_test_run();

Signature · 1 line

1.7.7.6