[Qemu-devel] [PATCH v4 0/3] fdc: fix media handling

[Qemu-devel] [PATCH v4 0/3] fdc: fix media handling

[Pavel Hrdina]

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 <phrdina@redhat.com>

Pavel Hrdina (3):
  fdc: floppy drive should be visible after start without media
  fdc: fix media detection
  fdc-test: introduced qtest no_media_on_start and cmos qtest for
    floppy

 hw/fdc.c         |   14 ++++++-----
 hw/pc.c          |    2 +-
 tests/fdc-test.c |   65 +++++++++++++++++++++++++++++++++++++++--------------
 3 files changed, 57 insertions(+), 24 deletions(-)

-- 
1.7.7.6

[Qemu-devel] [PATCH v4 1/3] fdc: floppy drive should be visible after start without media

[Pavel Hrdina]

If you start guest with floppy drive but without media inserted, guest
still should see floppy drive pressent.

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

diff --git a/hw/pc.c b/hw/pc.c
index 4d34a33..967c17a 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -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);
-- 
1.7.7.6

Re: [Qemu-devel] [PATCH v4 1/3] fdc: floppy drive should be visible after start without media

[Marcelo Tosatti]

On Thu, May 24, 2012 at 11:02:28AM +0200, Pavel Hrdina wrote:
> If you start guest with floppy drive but without media inserted, guest
> still should see floppy drive pressent.
> 
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
>  hw/pc.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/pc.c b/hw/pc.c
> index 4d34a33..967c17a 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -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);
> -- 

This commit breaks WinXP.32 installation. It crashes during early setup 
stage, without a floppy disk specified.

Specifying a floppy disk with -fda fixes the problem.

Re: [Qemu-devel] [PATCH v4 1/3] fdc: floppy drive should be visible after start without media

[Markus Armbruster]

Marcelo Tosatti <mtosatti@redhat.com> writes:

> On Thu, May 24, 2012 at 11:02:28AM +0200, Pavel Hrdina wrote:
>> If you start guest with floppy drive but without media inserted, guest
>> still should see floppy drive pressent.
>> 
>> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
>> ---
>>  hw/pc.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>> 
>> diff --git a/hw/pc.c b/hw/pc.c
>> index 4d34a33..967c17a 100644
>> --- a/hw/pc.c
>> +++ b/hw/pc.c
>> @@ -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);
>> -- 
>
> This commit breaks WinXP.32 installation. It crashes during early setup 
> stage, without a floppy disk specified.
>
> Specifying a floppy disk with -fda fixes the problem.

Backtrace?

Re: [Qemu-devel] [PATCH v4 1/3] fdc: floppy drive should be visible after start without media

[Marcelo Tosatti]

On Tue, Jun 05, 2012 at 08:23:53AM +0200, Markus Armbruster wrote:
> Marcelo Tosatti <mtosatti@redhat.com> writes:
> 
> > On Thu, May 24, 2012 at 11:02:28AM +0200, Pavel Hrdina wrote:
> >> If you start guest with floppy drive but without media inserted, guest
> >> still should see floppy drive pressent.
> >> 
> >> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> >> ---
> >>  hw/pc.c |    2 +-
> >>  1 files changed, 1 insertions(+), 1 deletions(-)
> >> 
> >> diff --git a/hw/pc.c b/hw/pc.c
> >> index 4d34a33..967c17a 100644
> >> --- a/hw/pc.c
> >> +++ b/hw/pc.c
> >> @@ -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);
> >> -- 
> >
> > This commit breaks WinXP.32 installation. It crashes during early setup 
> > stage, without a floppy disk specified.
> >
> > Specifying a floppy disk with -fda fixes the problem.
> 
> Backtrace?

None, WinXP is spinning (presumably the floppy driver).

Re: [Qemu-devel] [PATCH v4 1/3] fdc: floppy drive should be visible after start without media

[Markus Armbruster]

Marcelo Tosatti <mtosatti@redhat.com> writes:

> On Tue, Jun 05, 2012 at 08:23:53AM +0200, Markus Armbruster wrote:
>> Marcelo Tosatti <mtosatti@redhat.com> writes:
>> 
>> > On Thu, May 24, 2012 at 11:02:28AM +0200, Pavel Hrdina wrote:
>> >> If you start guest with floppy drive but without media inserted, guest
>> >> still should see floppy drive pressent.
>> >> 
>> >> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
>> >> ---
>> >>  hw/pc.c |    2 +-
>> >>  1 files changed, 1 insertions(+), 1 deletions(-)
>> >> 
>> >> diff --git a/hw/pc.c b/hw/pc.c
>> >> index 4d34a33..967c17a 100644
>> >> --- a/hw/pc.c
>> >> +++ b/hw/pc.c
>> >> @@ -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);
>> >> -- 
>> >
>> > This commit breaks WinXP.32 installation. It crashes during early setup 
>> > stage, without a floppy disk specified.
>> >
>> > Specifying a floppy disk with -fda fixes the problem.
>> 
>> Backtrace?
>
> None, WinXP is spinning (presumably the floppy driver).

Bummer.  Pavel, can you look into this?

Re: [Qemu-devel] [PATCH v4 1/3] fdc: floppy drive should be visible after start without media

[Pavel Hrdina]

On 06/05/2012 02:05 PM, Markus Armbruster wrote:
> Marcelo Tosatti<mtosatti@redhat.com>  writes:
>
>> On Tue, Jun 05, 2012 at 08:23:53AM +0200, Markus Armbruster wrote:
>>> Marcelo Tosatti<mtosatti@redhat.com>  writes:
>>>
>>>> On Thu, May 24, 2012 at 11:02:28AM +0200, Pavel Hrdina wrote:
>>>>> If you start guest with floppy drive but without media inserted, guest
>>>>> still should see floppy drive pressent.
>>>>>
>>>>> Signed-off-by: Pavel Hrdina<phrdina@redhat.com>
>>>>> ---
>>>>>   hw/pc.c |    2 +-
>>>>>   1 files changed, 1 insertions(+), 1 deletions(-)
>>>>>
>>>>> diff --git a/hw/pc.c b/hw/pc.c
>>>>> index 4d34a33..967c17a 100644
>>>>> --- a/hw/pc.c
>>>>> +++ b/hw/pc.c
>>>>> @@ -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);
>>>>> -- 
>>>> This commit breaks WinXP.32 installation. It crashes during early setup
>>>> stage, without a floppy disk specified.
>>>>
>>>> Specifying a floppy disk with -fda fixes the problem.
>>> Backtrace?
>> None, WinXP is spinning (presumably the floppy driver).
> Bummer.  Pavel, can you look into this?
Hi, I have just read this issue. Sure, I will look into.

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

[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'.

We still have to set default drive geometry in 'fd_revalidate' even
if there is no media in drive. When you try to open (windows) or mount (linux)
floppy the driver tries to seek on track 1. Linux guest stuck in loop then
kernel crashes and windows guest prints error message.

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

diff --git a/hw/fdc.c b/hw/fdc.c
index cb4cd25..30d34e3 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -179,12 +179,14 @@ static void fd_revalidate(FDrive *drv)
     FDriveRate rate;
 
     FLOPPY_DPRINTF("revalidate\n");
-    if (drv->bs != NULL && bdrv_is_inserted(drv->bs)) {
+    if (drv->bs != NULL) {
         ro = bdrv_is_read_only(drv->bs);
         bdrv_get_floppy_geometry_hint(drv->bs, &nb_heads, &max_track,
                                       &last_sect, drv->drive, &drive, &rate);
-        if (nb_heads != 0 && max_track != 0 && last_sect != 0) {
-            FLOPPY_DPRINTF("User defined disk (%d %d %d)",
+        if (!bdrv_is_inserted(drv->bs)) {
+            FLOPPY_DPRINTF("No disk in drive\n");
+        } else if (nb_heads != 0 && max_track != 0 && last_sect != 0) {
+            FLOPPY_DPRINTF("User defined disk (%d %d %d)\n",
                            nb_heads - 1, max_track, last_sect);
         } else {
             FLOPPY_DPRINTF("Floppy disk (%d h %d t %d s) %s\n", nb_heads,
@@ -201,7 +203,7 @@ static void fd_revalidate(FDrive *drv)
         drv->drive = drive;
         drv->media_rate = rate;
     } else {
-        FLOPPY_DPRINTF("No disk in drive\n");
+        FLOPPY_DPRINTF("No drive connected\n");
         drv->last_sect = 0;
         drv->max_track = 0;
         drv->flags &= ~FDISK_DBL_SIDES;
@@ -709,7 +711,7 @@ static void fdctrl_raise_irq(FDCtrl *fdctrl, uint8_t status0)
         FDrive *cur_drv;
         /* A seek clears the disk change line (if a disk is inserted) */
         cur_drv = get_cur_drv(fdctrl);
-        if (cur_drv->max_track) {
+        if (cur_drv->bs != NULL && bdrv_is_inserted(cur_drv->bs)) {
             cur_drv->media_changed = 0;
         }
     }
@@ -1878,7 +1880,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);
         }
-- 
1.7.7.6

[Qemu-devel] [PATCH v4 3/3] fdc-test: introduced qtest no_media_on_start and cmos qtest for floppy

[Pavel Hrdina]

As default a guest has always one floppy drive so 0x10 byte in CMOS
has to have 0x40 value. Higher 4 bits means that the first floppy drive
is 1.44 Mb 3"5 drive and lower 4 bits means the second drive is not present.

After the guest starts DSKCHG bit in DIR register should be set. If there
is no media in drive, this bit should be set all the time.

Because we start the guest without media in drive, we have to swap
'eject' and 'change' in 'test_media_change'.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
 tests/fdc-test.c |   65 +++++++++++++++++++++++++++++++++++++++--------------
 1 files changed, 48 insertions(+), 17 deletions(-)

diff --git a/tests/fdc-test.c b/tests/fdc-test.c
index 5b5dd74..22d24ac 100644
--- a/tests/fdc-test.c
+++ b/tests/fdc-test.c
@@ -63,6 +63,12 @@ char test_image[] = "/tmp/qtest.XXXXXX";
 #define assert_bit_set(data, mask) g_assert_cmphex((data) & (mask), ==, (mask))
 #define assert_bit_clear(data, mask) g_assert_cmphex((data) & (mask), ==, 0)
 
+static uint8_t base = 0x70;
+
+enum {
+    CMOS_FLOPPY     = 0x10,
+};
+
 static void floppy_send(uint8_t byte)
 {
     uint8_t msr;
@@ -108,34 +114,43 @@ static void send_step_pulse(void)
     cyl = (cyl + 1) % 4;
 }
 
-static void test_media_change(void)
+static uint8_t cmos_read(uint8_t reg)
 {
-    uint8_t dir;
+    outb(base + 0, reg);
+    return inb(base + 1);
+}
 
-    /* Media changed bit must be up-to-date after step pulse. Do two SEEKs
-     * because we may already happen to be on the right cylinder initially. */
-    send_step_pulse();
-    send_step_pulse();
-    dir = inb(FLOPPY_BASE + reg_dir);
-    assert_bit_clear(dir, DSKCHG);
+static void test_cmos(void)
+{
+    uint8_t cmos;
 
-    /* Eject the floppy and check that DSKCHG is set. Reading it out doesn't
-     * reset the bit. */
-    qmp("{'execute':'eject', 'arguments':{ 'device':'floppy0' }}");
-    qmp(""); /* ignore event */
+    cmos = cmos_read(CMOS_FLOPPY);
+    g_assert(cmos == 0x40);
+}
 
+static void test_no_media_on_start(void)
+{
+    uint8_t dir;
+
+    /* Media changed bit must be set all time after start if there is
+     * no media in drive. */
     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();
     send_step_pulse();
     dir = inb(FLOPPY_BASE + reg_dir);
     assert_bit_set(dir, DSKCHG);
     dir = inb(FLOPPY_BASE + reg_dir);
     assert_bit_set(dir, DSKCHG);
+}
+
+static void test_media_change(void)
+{
+    uint8_t dir;
 
-    /* And then insert it again. DSKCHK should not be reset until a step pulse
+    /* Insert media in drive. DSKCHK should not be reset until a step pulse
      * is sent. */
     qmp("{'execute':'change', 'arguments':{ 'device':'floppy0', "
         "'target': '%s' }}", test_image);
@@ -152,6 +167,22 @@ static void test_media_change(void)
     assert_bit_clear(dir, DSKCHG);
     dir = inb(FLOPPY_BASE + reg_dir);
     assert_bit_clear(dir, DSKCHG);
+
+    /* Eject the floppy and check that DSKCHG is set. Reading it out doesn't
+     * reset the bit. */
+    qmp("{'execute':'eject', 'arguments':{ 'device':'floppy0' }}");
+    qmp(""); /* ignore event */
+
+    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();
+    dir = inb(FLOPPY_BASE + reg_dir);
+    assert_bit_set(dir, DSKCHG);
+    dir = inb(FLOPPY_BASE + reg_dir);
+    assert_bit_set(dir, DSKCHG);
 }
 
 int main(int argc, char **argv)
@@ -177,12 +208,12 @@ int main(int argc, char **argv)
     /* Run the tests */
     g_test_init(&argc, &argv, NULL);
 
-    cmdline = g_strdup_printf("-vnc none "
-        "-drive file=%s,if=floppy,cache=writeback ",
-        test_image);
+    cmdline = g_strdup_printf("-vnc none ");
 
     qtest_start(cmdline);
     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/media_change", test_media_change);
 
     ret = g_test_run();
-- 
1.7.7.6

Re: [Qemu-devel] [PATCH v4 0/3] fdc: fix media handling

[Kevin Wolf]

Am 24.05.2012 11:02, schrieb Pavel Hrdina:
> 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 <phrdina@redhat.com>
> 
> Pavel Hrdina (3):
>   fdc: floppy drive should be visible after start without media
>   fdc: fix media detection
>   fdc-test: introduced qtest no_media_on_start and cmos qtest for
>     floppy
> 
>  hw/fdc.c         |   14 ++++++-----
>  hw/pc.c          |    2 +-
>  tests/fdc-test.c |   65 +++++++++++++++++++++++++++++++++++++++--------------
>  3 files changed, 57 insertions(+), 24 deletions(-)
> 

Thanks, applied all to the block branch for 1.1.

Kevin