Skip to contentSkip to conversation

fdc: fix media handling

PATCH Superseded Pavel Hrdina 2 participants 7 messages May 22, 2012

v1 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Pavel Hrdina

author
#0 lore raw

cover fdc: fix media handling

This patch series fixes handling of FDC when you don't have media inserted. Guest should see floppy drive if you start guest without media and should detect that there is no media in drive.

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

Pavel Hrdina (2):
fdc: floppy drive should be visible after start without media
fdc: fix media detection

 hw/fdc.c |    8 ++------
 hw/pc.c  |    2 +-
 2 files changed, 3 insertions(+), 7 deletions(-)
Signature · 1 line
1.7.7.6
author
#1 lore raw

PATCH 1/2 fdc: floppy drive should be visible after start without media

If you start guest with floppy drive but without media inserted, the guest now can see floppy drive.

Signed-off-by Pavel Hrdina <[email protected]>
---
 hw/pc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
hw/pc.c +1 -1
index 4d34a33..967c17a 100644
@@ -382,7 +382,7 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size,
     if (floppy) {
         fdc_get_bs(fd, floppy);
         for (i = 0; i < 2; i++) {
-            if (fd[i] && bdrv_is_inserted(fd[i])) {
+            if (fd[i]) {
                 bdrv_get_floppy_geometry_hint(fd[i], &nb_heads, &max_track,
                                               &last_sect, FDRIVE_DRV_NONE,
                                               &fd_type[i], &rate);
Signature · 1 line
1.7.7.6
author
#2 lore raw

PATCH 2/2 fdc: fix media detection

In reply to Pavel Hrdina

We have to set up 'media_changed' after guest start so floppy driver could detect that there is no media in drive. For this purpose we call 'fdctrl_change_cb' instead of 'fd_revalidate' in 'fdctrl_connect_drives'. 'fd_revalidate' is called inside 'fdctrl_change_cb'.

In 'fdctrl_handle_seek' we always set current track because we don't care if there is media inserted or not.

Signed-off-by Pavel Hrdina <[email protected]>
---
 hw/fdc.c |    8 ++------
 1 files changed, 2 insertions(+), 6 deletions(-)
hw/fdc.c +2 -6
index cb4cd25..337b35a 100644
@@ -1617,11 +1617,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];
-    }
+    cur_drv->track = fdctrl->fifo[2];
     /* Raise Interrupt */
     fdctrl_raise_irq(fdctrl, FD_SR0_SEEK);
 }

@@ -1878,7 +1874,7 @@ static int fdctrl_connect_drives(FDCtrl *fdctrl)
         }
 
         fd_init(drive);
-        fd_revalidate(drive);
+        fdctrl_change_cb(drive, 0);
         if (drive->bs) {
             bdrv_set_dev_ops(drive->bs, &fdctrl_block_ops, drive);
         }
Signature · 1 line
1.7.7.6
#3 lore raw

Re: [Qemu-devel] [PATCH 2/2] fdc: fix media detection

Am 22.05.2012 12:59, schrieb Pavel Hrdina:

Quoted 9 lines

We have to set up 'media_changed' after guest start so floppy driver could detect that there is no media in drive. For this purpose we call 'fdctrl_change_cb' instead of 'fd_revalidate' in 'fdctrl_connect_drives'. 'fd_revalidate' is called inside 'fdctrl_change_cb'.

In 'fdctrl_handle_seek' we always set current track because we don't care if there is media inserted or not.

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

Can you please add a qtest case that shows the problems that you're fixing in this series?

Quoted 5 lines
hw/fdc.c +0 -0
index cb4cd25..337b35a 100644

hw/fdc.c static void fdctrl_handle_seek(FDCtrl *fdctrl, int direction) · line 1620

Quoted 9 lines
hw/fdc.c +1 -5
@@ -1617,11 +1617,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];
-    }
+    cur_drv->track = fdctrl->fifo[2];

Why is it okay to have cur_drv->track point outside the floppy? Won't it mess up future calculations? Not all other places check it again cur_drv->max_track.

Kevin

author
#4 lore raw

Re: [Qemu-devel] [PATCH 2/2] fdc: fix media detection

Quoted 6 lines · Kevin Wolf

Am 22.05.2012 12:59, schrieb Pavel Hrdina:

Quoted 5 lines

We have to set up 'media_changed' after guest start so floppy driver could detect that there is no media in drive. For this purpose we call 'fdctrl_change_cb' instead of 'fd_revalidate' in 'fdctrl_connect_drives'. 'fd_revalidate' is called inside 'fdctrl_change_cb'.

Quoted 6 lines
Quoted 4 lines

In 'fdctrl_handle_seek' we always set current track because we don't care if there is media inserted or not.

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

Can you please add a qtest case that shows the problems that you're fixing in this series?

I'm new to qemu. By "add a qtest case" you mean update tests/fdc-test.c ?

Quoted 18 lines
Quoted 5 lines
hw/fdc.c +0 -0
index cb4cd25..337b35a 100644

hw/fdc.c static void fdctrl_handle_seek(FDCtrl *fdctrl, int direction) · line 1620

Quoted 9 lines
hw/fdc.c +1 -5
@@ -1617,11 +1617,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];
-    }
+    cur_drv->track = fdctrl->fifo[2];

Why is it okay to have cur_drv->track point outside the floppy? Won't it mess up future calculations? Not all other places check it again cur_drv->max_track.

Quoted 1 line

Kevin

Well, you are right. Than we have to set 'max_track' even if there is no media. I tested this on bare-metal without media and where floppy driver ask to seek to specific track, it ends good and return specific track position as actual. I'll rewrite this behavior and send patch v2.

#5 lore raw

Re: [Qemu-devel] [PATCH 2/2] fdc: fix media detection

Am 22.05.2012 15:28, schrieb Pavel Hrdina:

Quoted 7 lines · Kevin Wolf
Quoted 6 lines

Am 22.05.2012 12:59, schrieb Pavel Hrdina:

Quoted 5 lines

We have to set up 'media_changed' after guest start so floppy driver could detect that there is no media in drive. For this purpose we call 'fdctrl_change_cb' instead of 'fd_revalidate' in 'fdctrl_connect_drives'. 'fd_revalidate' is called inside 'fdctrl_change_cb'.

Quoted 7 lines
Quoted 6 lines
Quoted 4 lines

In 'fdctrl_handle_seek' we always set current track because we don't care if there is media inserted or not.

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

Can you please add a qtest case that shows the problems that you're fixing in this series?

I'm new to qemu. By "add a qtest case" you mean update tests/fdc-test.c ?

Sorry, I should have been more specific. Yes, that's what I mean.

Quoted 18 lines
Quoted 18 lines
Quoted 5 lines
hw/fdc.c +0 -0
index cb4cd25..337b35a 100644

hw/fdc.c static void fdctrl_handle_seek(FDCtrl *fdctrl, int direction) · line 1620

Quoted 9 lines
hw/fdc.c +1 -5
@@ -1617,11 +1617,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];
-    }
+    cur_drv->track = fdctrl->fifo[2];

Why is it okay to have cur_drv->track point outside the floppy? Won't it mess up future calculations? Not all other places check it again cur_drv->max_track.

Quoted 6 lines
Quoted 1 line

Kevin

Well, you are right. Than we have to set 'max_track' even if there is no media. I tested this on bare-metal without media and where floppy driver ask to seek to specific track, it ends good and return specific track position as actual.
I'll rewrite this behavior and send patch v2.

You mean max_track = 0 isn't a good value to work with? How can a real drive position the head correctly when it doesn't have a media (and therefore doesn't know its geometry)?

But if you have a good default value for max_track that we should use when no medium is present, go ahead.

Kevin

author
#6 lore raw

Re: [Qemu-devel] [PATCH 2/2] fdc: fix media detection

Quoted 41 lines · Kevin Wolf

Am 22.05.2012 15:28, schrieb Pavel Hrdina:

Quoted 7 lines · Kevin Wolf
Quoted 6 lines

Am 22.05.2012 12:59, schrieb Pavel Hrdina:

Quoted 5 lines

We have to set up 'media_changed' after guest start so floppy driver could detect that there is no media in drive. For this purpose we call 'fdctrl_change_cb' instead of 'fd_revalidate' in 'fdctrl_connect_drives'. 'fd_revalidate' is called inside 'fdctrl_change_cb'.

Quoted 7 lines
Quoted 6 lines
Quoted 4 lines

In 'fdctrl_handle_seek' we always set current track because we don't care if there is media inserted or not.

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

Can you please add a qtest case that shows the problems that you're fixing in this series?

I'm new to qemu. By "add a qtest case" you mean update tests/fdc-test.c ?

Sorry, I should have been more specific. Yes, that's what I mean.

Quoted 18 lines
Quoted 18 lines
Quoted 5 lines
hw/fdc.c +0 -0
index cb4cd25..337b35a 100644

hw/fdc.c static void fdctrl_handle_seek(FDCtrl *fdctrl, int direction) · line 1620

Quoted 9 lines
hw/fdc.c +1 -5
@@ -1617,11 +1617,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];
-    }
+    cur_drv->track = fdctrl->fifo[2];

Why is it okay to have cur_drv->track point outside the floppy? Won't it mess up future calculations? Not all other places check it again cur_drv->max_track.

Quoted 6 lines
Quoted 1 line

Kevin

Well, you are right. Than we have to set 'max_track' even if there is no media. I tested this on bare-metal without media and where floppy driver ask to seek to specific track, it ends good and return specific track position as actual.
I'll rewrite this behavior and send patch v2.

Quoted 8 lines

You mean max_track = 0 isn't a good value to work with? How can a real drive position the head correctly when it doesn't have a media (and therefore doesn't know its geometry)?

But if you have a good default value for max_track that we should use when no medium is present, go ahead.

Kevin

When you try mount floppy in linux while there is no media then a floppy driver tries to seek on track 1. Virtual guest always get actual track position 0 so tries to seek again and stuck in loop which cause kernel panic. On bare-metal floppy driver gets actual track position 1 and stop seeking.

I thing, that a real drive ignore geometry and just seeks to specific position.

Pavel