[Qemu-devel] [PATCH v2 1/2] fdc: fix relative seek

[Qemu-devel] [PATCH v2 1/2] fdc: fix relative seek

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
 hw/fdc.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/fdc.c b/hw/fdc.c
index edf0706..decb1f7 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -1705,7 +1705,8 @@ static void fdctrl_handle_relative_seek_out(FDCtrl *fdctrl, int direction)
         fd_seek(cur_drv, cur_drv->head, cur_drv->max_track - 1,
                 cur_drv->sect, 1);
     } else {
-        fd_seek(cur_drv, cur_drv->head, fdctrl->fifo[2], cur_drv->sect, 1);
+        fd_seek(cur_drv, cur_drv->head,
+                cur_drv->track + fdctrl->fifo[2], cur_drv->sect, 1);
     }
     fdctrl_reset_fifo(fdctrl);
     /* Raise Interrupt */
@@ -1721,7 +1722,8 @@ static void fdctrl_handle_relative_seek_in(FDCtrl *fdctrl, int direction)
     if (fdctrl->fifo[2] > cur_drv->track) {
         fd_seek(cur_drv, cur_drv->head, 0, cur_drv->sect, 1);
     } else {
-        fd_seek(cur_drv, cur_drv->head, fdctrl->fifo[2], cur_drv->sect, 1);
+        fd_seek(cur_drv, cur_drv->head,
+                cur_drv->track - fdctrl->fifo[2], cur_drv->sect, 1);
     }
     fdctrl_reset_fifo(fdctrl);
     /* Raise Interrupt */
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH v2 1/2] fdc: fix relative seek

[Kevin Wolf]

Am 16.07.2012 14:25, schrieb Pavel Hrdina:
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
>  hw/fdc.c |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

I applied both to the block branch for now. This restores the behaviour
as it was before 6be01b1e. However, I believe it is still wrong: The
direction should be interpreted the other way round, i.e. seek_out
should decrease the cylinder number and seek_in should increase it.

Do you have a guest that actually uses this command?

Kevin

Re: [Qemu-devel] [PATCH v2 1/2] fdc: fix relative seek

[Pavel Hrdina]

On 07/16/2012 03:24 PM, Kevin Wolf wrote:
> Am 16.07.2012 14:25, schrieb Pavel Hrdina:
>> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
>> ---
>>   hw/fdc.c |    6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
> I applied both to the block branch for now. This restores the behaviour
> as it was before 6be01b1e. However, I believe it is still wrong: The
> direction should be interpreted the other way round, i.e. seek_out
> should decrease the cylinder number and seek_in should increase it.
>
> Do you have a guest that actually uses this command?
>
> Kevin
I have host with real floppy device and I could send command directly, 
so I'll check this behavior.

Pavel

Re: [Qemu-devel] [PATCH v2 1/2] fdc: fix relative seek

[Pavel Hrdina]

On 07/16/2012 03:26 PM, Pavel Hrdina wrote:
> On 07/16/2012 03:24 PM, Kevin Wolf wrote:
>> Am 16.07.2012 14:25, schrieb Pavel Hrdina:
>>> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
>>> ---
>>>   hw/fdc.c |    6 ++++--
>>>   1 file changed, 4 insertions(+), 2 deletions(-)
>> I applied both to the block branch for now. This restores the behaviour
>> as it was before 6be01b1e. However, I believe it is still wrong: The
>> direction should be interpreted the other way round, i.e. seek_out
>> should decrease the cylinder number and seek_in should increase it.
>>
>> Do you have a guest that actually uses this command?
>>
>> Kevin
> I have host with real floppy device and I could send command directly, 
> so I'll check this behavior.
>
> Pavel
Well, you're right... seek in increase track and seek out decrease 
track. I suspect that it was correct.
I'll send v3 with this fix.

Pavel

[Qemu-devel] [PATCH v2 2/2] fdc-test: introduce test_relative_seek

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
 tests/fdc-test.c |   46 ++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 41 insertions(+), 5 deletions(-)

diff --git a/tests/fdc-test.c b/tests/fdc-test.c
index 585fb0e..4bc3515 100644
--- a/tests/fdc-test.c
+++ b/tests/fdc-test.c
@@ -47,9 +47,11 @@ enum {
 };
 
 enum {
-    CMD_SENSE_INT   = 0x08,
-    CMD_SEEK        = 0x0f,
-    CMD_READ        = 0xe6,
+    CMD_SENSE_INT           = 0x08,
+    CMD_SEEK                = 0x0f,
+    CMD_READ                = 0xe6,
+    CMD_RELATIVE_SEEK_OUT   = 0x8f,
+    CMD_RELATIVE_SEEK_IN    = 0xcf,
 };
 
 enum {
@@ -91,13 +93,17 @@ static uint8_t floppy_recv(void)
     return inb(FLOPPY_BASE + reg_fifo);
 }
 
-static void ack_irq(void)
+static uint8_t ack_irq(void)
 {
+    uint8_t ret;
+
     g_assert(get_irq(FLOPPY_IRQ));
     floppy_send(CMD_SENSE_INT);
     floppy_recv();
-    floppy_recv();
+    ret = floppy_recv();
     g_assert(!get_irq(FLOPPY_IRQ));
+
+    return ret;
 }
 
 static uint8_t send_read_command(void)
@@ -281,6 +287,35 @@ static void test_sense_interrupt(void)
     floppy_recv();
 }
 
+static void test_relative_seek(void)
+{
+    uint8_t drive = 0;
+    uint8_t head = 0;
+    uint8_t cyl = 1;
+    uint8_t ret;
+
+    /* Send seek to track 0 */
+    send_step_pulse(0);
+
+    /* Send relative seek to increase track by 1 */
+    floppy_send(CMD_RELATIVE_SEEK_OUT);
+    floppy_send(head << 2 | drive);
+    g_assert(!get_irq(FLOPPY_IRQ));
+    floppy_send(cyl);
+
+    ret = ack_irq();
+    g_assert(ret == 1);
+
+    /* Send relative seek to decrease track by 1 */
+    floppy_send(CMD_RELATIVE_SEEK_IN);
+    floppy_send(head << 2 | drive);
+    g_assert(!get_irq(FLOPPY_IRQ));
+    floppy_send(cyl);
+
+    ret = ack_irq();
+    g_assert(ret == 0);
+}
+
 /* success if no crash or abort */
 static void fuzz_registers(void)
 {
@@ -329,6 +371,7 @@ int main(int argc, char **argv)
     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/relative_seek", test_relative_seek);
     qtest_add_func("/fdc/fuzz-registers", fuzz_registers);
 
     ret = g_test_run();
-- 
1.7.10.4