[Qemu-devel] [PATCH v6 0/6] fdc: fix/rewrite seek, media_changed and interrupt handling

[Qemu-devel] [PATCH v6 0/6] fdc: fix/rewrite seek, media_changed and interrupt handling

[Pavel Hrdina]

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(-)

-- 
1.7.7.6

[Qemu-devel] [PATCH v6 1/6] fdc: fix implied seek while there is no media in drive

[Pavel Hrdina]

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 <phrdina@redhat.com>
---
 hw/fdc.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/hw/fdc.c b/hw/fdc.c
index 30d34e3..be35201 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -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;
 }
 
-- 
1.7.7.6

[Qemu-devel] [PATCH v6 2/6] fdc-test: introduced qtest read_without_media

[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 <phrdina@redhat.com>
---
 tests/fdc-test.c |   66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 66 insertions(+), 0 deletions(-)

diff --git a/tests/fdc-test.c b/tests/fdc-test.c
index 22d24ac..e730398 100644
--- a/tests/fdc-test.c
+++ b/tests/fdc-test.c
@@ -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();
-- 
1.7.7.6

[Qemu-devel] [PATCH v6 3/6] fdc: rewrite seek and DSKCHG bit handling

[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 <phrdina@redhat.com>
---
 hw/fdc.c |   79 ++++++++++++++++++++++++++++++++-----------------------------
 1 files changed, 41 insertions(+), 38 deletions(-)

diff --git a/hw/fdc.c b/hw/fdc.c
index 5b3224b..0270264 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -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 */
-- 
1.7.7.6

[Qemu-devel] [PATCH v6 4/6] fdc: fix interrupt handling

[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 <phrdina@redhat.com>
---
 hw/fdc.c |   34 +++++++++++++++++++++-------------
 1 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/hw/fdc.c b/hw/fdc.c
index 0270264..e28841c 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -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;
-- 
1.7.7.6

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

[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 <phrdina@redhat.com>
---
 tests/fdc-test.c |   29 +++++++++++++++++++++--------
 1 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/tests/fdc-test.c b/tests/fdc-test.c
index 610e2f1..5280eff 100644
--- a/tests/fdc-test.c
+++ b/tests/fdc-test.c
@@ -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);
-- 
1.7.7.6

[Qemu-devel] [PATCH v6 6/6] fdc_test: introduce test_sense_interrupt

[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 <phrdina@redhat.com>
---
 tests/fdc-test.c |   25 ++++++++++++++++++++++++-
 1 files changed, 24 insertions(+), 1 deletions(-)

diff --git a/tests/fdc-test.c b/tests/fdc-test.c
index 5280eff..be14d83 100644
--- a/tests/fdc-test.c
+++ b/tests/fdc-test.c
@@ -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();
-- 
1.7.7.6

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

[Blue Swirl]

On Fri, Jun 22, 2012 at 10:33 AM, Pavel Hrdina <phrdina@redhat.com> wrote:
> 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 <phrdina@redhat.com>
> ---
>  tests/fdc-test.c |   29 +++++++++++++++++++++--------
>  1 files changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/tests/fdc-test.c b/tests/fdc-test.c
> index 610e2f1..5280eff 100644
> --- a/tests/fdc-test.c
> +++ b/tests/fdc-test.c
> @@ -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;

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

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

>
>     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
>
>

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

[Pavel Hrdina]

On 06/25/2012 11:50 AM, Kevin Wolf wrote:
> Am 22.06.2012 12:33, schrieb 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<phrdina@redhat.com>
>> ---
>>   tests/fdc-test.c |   29 +++++++++++++++++++++--------
>>   1 files changed, 21 insertions(+), 8 deletions(-)
>>
>> diff --git a/tests/fdc-test.c b/tests/fdc-test.c
>> index 610e2f1..5280eff 100644
>> --- a/tests/fdc-test.c
>> +++ b/tests/fdc-test.c
>> @@ -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;
> 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.
>
> 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;

>>
>>       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);
> This is a bool parameter, so you should pass true/false instead of 0/1.
>
> 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

[Qemu-devel] [PATCH v7 0/6] fdc: fix/rewrite seek, media_changed and interrupt handling

[Pavel Hrdina]

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.

v7:
    - small fixes in test_media_change and test_sense_interrupt

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(-)

-- 
1.7.7.6

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

[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 <phrdina@redhat.com>
---
 tests/fdc-test.c |   30 ++++++++++++++++++++++--------
 1 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/tests/fdc-test.c b/tests/fdc-test.c
index 610e2f1..f865576 100644
--- a/tests/fdc-test.c
+++ b/tests/fdc-test.c
@@ -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);
-- 
1.7.7.6

[Qemu-devel] [PATCH v7 6/6] fdc_test: introduce test_sense_interrupt

[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 <phrdina@redhat.com>
---
 tests/fdc-test.c |   25 ++++++++++++++++++++++++-
 1 files changed, 24 insertions(+), 1 deletions(-)

diff --git a/tests/fdc-test.c b/tests/fdc-test.c
index f865576..db83103 100644
--- a/tests/fdc-test.c
+++ b/tests/fdc-test.c
@@ -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();
-- 
1.7.7.6

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

[Kevin Wolf]

Am 04.07.2012 16:26, schrieb 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 <phrdina@redhat.com>
> ---
>  tests/fdc-test.c |   30 ++++++++++++++++++++++--------
>  1 files changed, 22 insertions(+), 8 deletions(-)

Thanks, applied all.

Kevin