Patchwork [v2] Replace bdrv_* to bdrv_aio_* functions in pio mode in fdc.c.

login
register
mail settings
Submitter Zhi Hui Li
Date March 31, 2012, 1:15 p.m.
Message ID <1333199710-9061-1-git-send-email-zhihuili@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/149830/
State New
Headers show

Comments

Zhi Hui Li - March 31, 2012, 1:15 p.m.
Replace bdrv_* to bdrv_aio_* functions in pio mode in fdc.c.

Signed-off-by: Li Zhi Hui <zhihuili@linux.vnet.ibm.com>
---
 hw/fdc.c |  123 +++++++++++++++++++++++++++++++++++++++++++++-----------------
 1 files changed, 89 insertions(+), 34 deletions(-)
Stefan Hajnoczi - April 2, 2012, 12:07 p.m.
On Sat, Mar 31, 2012 at 09:15:10PM +0800, Li Zhi Hui wrote:
> Replace bdrv_* to bdrv_aio_* functions in pio mode in fdc.c.
> 
> Signed-off-by: Li Zhi Hui <zhihuili@linux.vnet.ibm.com>
> ---
>  hw/fdc.c |  123 +++++++++++++++++++++++++++++++++++++++++++++-----------------
>  1 files changed, 89 insertions(+), 34 deletions(-)

How can I test this patch?

Stefan
Paolo Bonzini - April 2, 2012, 1:08 p.m.
Il 02/04/2012 14:07, Stefan Hajnoczi ha scritto:
> On Sat, Mar 31, 2012 at 09:15:10PM +0800, Li Zhi Hui wrote:
>> Replace bdrv_* to bdrv_aio_* functions in pio mode in fdc.c.
>>
>> Signed-off-by: Li Zhi Hui <zhihuili@linux.vnet.ibm.com>
>> ---
>>  hw/fdc.c |  123 +++++++++++++++++++++++++++++++++++++++++++++-----------------
>>  1 files changed, 89 insertions(+), 34 deletions(-)
> 
> How can I test this patch?

Looks like one of floppy=nodma or floppy.floppy=nodma should do it.

Paolo
Zhi Hui Li - April 3, 2012, 11:56 a.m.
On 2012年04月02日 20:07, Stefan Hajnoczi wrote:
> On Sat, Mar 31, 2012 at 09:15:10PM +0800, Li Zhi Hui wrote:
>> Replace bdrv_* to bdrv_aio_* functions in pio mode in fdc.c.
>>
>> Signed-off-by: Li Zhi Hui<zhihuili@linux.vnet.ibm.com>
>> ---
>>   hw/fdc.c |  123 +++++++++++++++++++++++++++++++++++++++++++++-----------------
>>   1 files changed, 89 insertions(+), 34 deletions(-)
>
> How can I test this patch?
>
> Stefan
>

sorry, I don't find a good way to test it directly.

but I have read the datasheet of floppy, I modify it according to the 
datasheet.
Kevin Wolf - April 3, 2012, 12:29 p.m.
Am 03.04.2012 13:56, schrieb Zhi Hui Li:
> On 2012年04月02日 20:07, Stefan Hajnoczi wrote:
>> On Sat, Mar 31, 2012 at 09:15:10PM +0800, Li Zhi Hui wrote:
>>> Replace bdrv_* to bdrv_aio_* functions in pio mode in fdc.c.
>>>
>>> Signed-off-by: Li Zhi Hui<zhihuili@linux.vnet.ibm.com>
>>> ---
>>>   hw/fdc.c |  123 +++++++++++++++++++++++++++++++++++++++++++++-----------------
>>>   1 files changed, 89 insertions(+), 34 deletions(-)
>>
>> How can I test this patch?
>>
>> Stefan
>>
> 
> sorry, I don't find a good way to test it directly.
> 
> but I have read the datasheet of floppy, I modify it according to the 
> datasheet.

How about adding some qtest-based test cases for floppy? It's one of the
devices that I usually don't test manually, so having an automated test
would be good anyway.

Kevin
Stefan Hajnoczi - April 3, 2012, 3:33 p.m.
On Mon, Apr 2, 2012 at 2:08 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 02/04/2012 14:07, Stefan Hajnoczi ha scritto:
>> On Sat, Mar 31, 2012 at 09:15:10PM +0800, Li Zhi Hui wrote:
>>> Replace bdrv_* to bdrv_aio_* functions in pio mode in fdc.c.
>>>
>>> Signed-off-by: Li Zhi Hui <zhihuili@linux.vnet.ibm.com>
>>> ---
>>>  hw/fdc.c |  123 +++++++++++++++++++++++++++++++++++++++++++++-----------------
>>>  1 files changed, 89 insertions(+), 34 deletions(-)
>>
>> How can I test this patch?
>
> Looks like one of floppy=nodma or floppy.floppy=nodma should do it.

I'm getting the following output:
$ cat /dev/fd0 >/dev/null
floppy0: long rw: 24 instead of 12
rs=1 s=1
rh=0 h=0
rt=1 t=0
heads=2 eoc=0
spt=18 st=0 ss=1
in_sector_offset=0

It looks like pio mode doesn't even work with the Linux floppy driver today!

Stefan
Stefan Hajnoczi - April 3, 2012, 3:34 p.m.
On Tue, Apr 3, 2012 at 4:33 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Mon, Apr 2, 2012 at 2:08 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Il 02/04/2012 14:07, Stefan Hajnoczi ha scritto:
>>> On Sat, Mar 31, 2012 at 09:15:10PM +0800, Li Zhi Hui wrote:
>>>> Replace bdrv_* to bdrv_aio_* functions in pio mode in fdc.c.
>>>>
>>>> Signed-off-by: Li Zhi Hui <zhihuili@linux.vnet.ibm.com>
>>>> ---
>>>>  hw/fdc.c |  123 +++++++++++++++++++++++++++++++++++++++++++++-----------------
>>>>  1 files changed, 89 insertions(+), 34 deletions(-)
>>>
>>> How can I test this patch?
>>
>> Looks like one of floppy=nodma or floppy.floppy=nodma should do it.
>
> I'm getting the following output:
> $ cat /dev/fd0 >/dev/null
> floppy0: long rw: 24 instead of 12
> rs=1 s=1
> rh=0 h=0
> rt=1 t=0
> heads=2 eoc=0
> spt=18 st=0 ss=1
> in_sector_offset=0
>
> It looks like pio mode doesn't even work with the Linux floppy driver today!

I just want to clarify that I tested unmodified qemu.git/master.  This
means hw/fdc.c doesn't even work with the Linux floppy driver today.

Stefan

Patch

diff --git a/hw/fdc.c b/hw/fdc.c
index a0236b7..0d178d2 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -230,6 +230,8 @@  static uint32_t fdctrl_read_data(FDCtrl *fdctrl);
 static void fdctrl_write_data(FDCtrl *fdctrl, uint32_t value);
 static uint32_t fdctrl_read_dir(FDCtrl *fdctrl);
 static void fdctrl_write_ccr(FDCtrl *fdctrl, uint32_t value);
+static void fdctrl_read_pio_cb(void *opaque, int ret);
+static void fdctrl_write_pio_cb(void *opaque, int ret);
 
 enum {
     FD_DIR_WRITE   = 0,
@@ -429,6 +431,8 @@  struct FDCtrl {
     /* Timers state */
     uint8_t timer0;
     uint8_t timer1;
+    QEMUIOVector qiov;
+    struct iovec iov;
 };
 
 typedef struct FDCtrlSysBus {
@@ -443,6 +447,9 @@  typedef struct FDCtrlISABus {
     int32_t bootindexB;
 } FDCtrlISABus;
 
+/* Associate command to an index in the 'handlers' array */
+static uint8_t command_to_handler[256];
+
 static uint32_t fdctrl_read (void *opaque, uint32_t reg)
 {
     FDCtrl *fdctrl = opaque;
@@ -1164,6 +1171,16 @@  static void fdctrl_start_transfer(FDCtrl *fdctrl, int direction)
     fdctrl->msr |= FD_MSR_NONDMA;
     if (direction != FD_DIR_WRITE)
         fdctrl->msr |= FD_MSR_DIO;
+
+    if (0 == command_to_handler[fdctrl->fifo[0] & 0xff]) {
+            fdctrl->msr &= ~FD_MSR_RQM;
+            fdctrl->iov.iov_base = fdctrl->fifo;
+            fdctrl->iov.iov_len  = FD_SECTOR_LEN;
+            qemu_iovec_init_external(&fdctrl->qiov, &fdctrl->iov, 1);
+            bdrv_aio_readv(cur_drv->bs, fd_sector(cur_drv),
+                &fdctrl->qiov, 1, fdctrl_read_pio_cb, fdctrl);
+        }
+
     /* IO based transfer: calculate len */
     fdctrl_raise_irq(fdctrl, 0x00);
 
@@ -1301,6 +1318,23 @@  static int fdctrl_transfer_handler (void *opaque, int nchan,
     return len;
 }
 
+static void fdctrl_read_pio_cb(void *opaque, int ret)
+{
+    FDCtrl *fdctrl = opaque;
+    FDrive *cur_drv;
+
+    if (ret < 0) {
+        cur_drv = get_cur_drv(fdctrl);
+        FLOPPY_DPRINTF("error getting sector %d\n",
+                       fd_sector(cur_drv));
+        /* Sure, image size is too small... */
+        memset(fdctrl->fifo, 0, FD_SECTOR_LEN);
+        return;
+    }
+
+    fdctrl->msr |= FD_MSR_RQM;
+    return;
+}
 /* Data register : 0x05 */
 static uint32_t fdctrl_read_data(FDCtrl *fdctrl)
 {
@@ -1315,23 +1349,7 @@  static uint32_t fdctrl_read_data(FDCtrl *fdctrl)
         return 0;
     }
     pos = fdctrl->data_pos;
-    if (fdctrl->msr & FD_MSR_NONDMA) {
-        pos %= FD_SECTOR_LEN;
-        if (pos == 0) {
-            if (fdctrl->data_pos != 0)
-                if (!fdctrl_seek_to_next_sect(fdctrl, cur_drv)) {
-                    FLOPPY_DPRINTF("error seeking to next sector %d\n",
-                                   fd_sector(cur_drv));
-                    return 0;
-                }
-            if (bdrv_read(cur_drv->bs, fd_sector(cur_drv), fdctrl->fifo, 1) < 0) {
-                FLOPPY_DPRINTF("error getting sector %d\n",
-                               fd_sector(cur_drv));
-                /* Sure, image size is too small... */
-                memset(fdctrl->fifo, 0, FD_SECTOR_LEN);
-            }
-        }
-    }
+    pos %= FD_SECTOR_LEN;
     retval = fdctrl->fifo[pos];
     if (++fdctrl->data_pos == fdctrl->data_len) {
         fdctrl->data_pos = 0;
@@ -1344,9 +1362,27 @@  static uint32_t fdctrl_read_data(FDCtrl *fdctrl)
             fdctrl_reset_fifo(fdctrl);
             fdctrl_reset_irq(fdctrl);
         }
+        goto end;
     }
-    FLOPPY_DPRINTF("data register: 0x%02x\n", retval);
 
+    if (fdctrl->msr & FD_MSR_NONDMA) {
+        if (pos + 1 == FD_SECTOR_LEN) {
+            if (!fdctrl_seek_to_next_sect(fdctrl, cur_drv)) {
+                FLOPPY_DPRINTF("error seeking to next sector %d\n",
+                               fd_sector(cur_drv));
+                return 0;
+            }
+            fdctrl->msr &= ~FD_MSR_RQM;
+            fdctrl->iov.iov_base = fdctrl->fifo;
+            fdctrl->iov.iov_len  = FD_SECTOR_LEN;
+            qemu_iovec_init_external(&fdctrl->qiov, &fdctrl->iov, 1);
+            bdrv_aio_readv(cur_drv->bs, fd_sector(cur_drv),
+                &fdctrl->qiov, 1, fdctrl_read_pio_cb, fdctrl);
+        }
+    }
+
+end:
+    FLOPPY_DPRINTF("data register: 0x%02x\n", retval);
     return retval;
 }
 
@@ -1756,8 +1792,34 @@  static const struct {
     { FD_CMD_WRITE, 0x1f, "WRITE (BeOS)", 8, fdctrl_start_transfer, FD_DIR_WRITE }, /* not in specification ; BeOS 4.5 bug */
     { 0, 0, "unknown", 0, fdctrl_unimplemented }, /* default handler */
 };
-/* Associate command to an index in the 'handlers' array */
-static uint8_t command_to_handler[256];
+
+static void fdctrl_write_pio_cb(void *opaque, int ret)
+{
+    FDCtrl *fdctrl = opaque;
+    FDrive *cur_drv;
+
+    cur_drv = get_cur_drv(fdctrl);
+    if (ret < 0) {
+        FLOPPY_ERROR("writing sector %d\n", fd_sector(cur_drv));
+        return;
+    }
+    if (!fdctrl_seek_to_next_sect(fdctrl, cur_drv)) {
+        FLOPPY_DPRINTF("error seeking to next sector %d\n",
+                       fd_sector(cur_drv));
+        return;
+    }
+
+    fdctrl->msr |= FD_MSR_RQM;
+
+    /* Switch from transfer mode to status mode
+     * then from status mode to command mode
+     */
+    if (fdctrl->data_pos == fdctrl->data_len) {
+        fdctrl_stop_transfer(fdctrl, FD_SR0_SEEK, 0x00, 0x00);
+    }
+
+    return;
+}
 
 static void fdctrl_write_data(FDCtrl *fdctrl, uint32_t value)
 {
@@ -1783,21 +1845,14 @@  static void fdctrl_write_data(FDCtrl *fdctrl, uint32_t value)
         if (pos == FD_SECTOR_LEN - 1 ||
             fdctrl->data_pos == fdctrl->data_len) {
             cur_drv = get_cur_drv(fdctrl);
-            if (bdrv_write(cur_drv->bs, fd_sector(cur_drv), fdctrl->fifo, 1) < 0) {
-                FLOPPY_ERROR("writing sector %d\n", fd_sector(cur_drv));
-                return;
-            }
-            if (!fdctrl_seek_to_next_sect(fdctrl, cur_drv)) {
-                FLOPPY_DPRINTF("error seeking to next sector %d\n",
-                               fd_sector(cur_drv));
-                return;
-            }
+
+            fdctrl->msr &= ~FD_MSR_RQM;
+            fdctrl->iov.iov_base = fdctrl->fifo;
+            fdctrl->iov.iov_len  = FD_SECTOR_LEN;
+            qemu_iovec_init_external(&fdctrl->qiov, &fdctrl->iov, 1);
+            bdrv_aio_writev(cur_drv->bs, fd_sector(cur_drv),
+                &fdctrl->qiov, 1, fdctrl_write_pio_cb, fdctrl);
         }
-        /* Switch from transfer mode to status mode
-         * then from status mode to command mode
-         */
-        if (fdctrl->data_pos == fdctrl->data_len)
-            fdctrl_stop_transfer(fdctrl, FD_SR0_SEEK, 0x00, 0x00);
         return;
     }
     if (fdctrl->data_pos == 0) {