Re: [Qemu-devel] [PATCH] Fix IDE FDC emulation for no media
On 04/26/2012 05:13 PM, Markus Armbruster wrote:
Pavel Hrdina writes:
Hi,
this is the patch to fix incorrect handling of IDE floppy drive controller
emulation
when no media is present. If the guest is booted without a media then the drive
was not being emulated at all but this patch enables the emulation with no
media present.
There was a bug in FDC emulation without media. Driver was not able to
recognize that
there is no media in drive.
This has been tested on both Fedora-16 x86_64 VM and Windows XP VM and the
behaviour
is as expected, i.e. as follows:
Linux guest (Fedora 16 x86_64) tries "mount /dev/fd0" and exit with error
"mount: /dev/fd0 is not a valid block device" which is the same behavior like
bare metal with real floppy device (you have to load floppy driver at first
using e.g. "modprobe floppy" command).
For Windows XP guest the Windows floppy driver is trying to seek the virtual
drive
when you want to open it but driver successfully detect that there is no media
in drive
and then it's asking user to insert floppy media in the drive.
I also tested behavior of this patch if you start guest with "-nodefaults" and
both
Windows and Linux guests detect only FDC but no drive.
Pavel
As Andreas already pointed out, your commit message needs work. Please
limit line length to 70 characters.
This patch has been written with help of specifications from:
http://www.ousob.com/ng/hardware/ngd127.php
http://www.isdaman.com/alsos/hardware/fdc/floppy.htm
http://wiki.osdev.org/Floppy_Disk_Controller
Signed-off-by: Pavel Hrdina
Signed-off-by: Michal Novotny
---
hw/fdc.c | 14 ++
hw/pc.c |3 ++-
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/hw/fdc.c b/hw/fdc.c
index a0236b7..6791eff 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 media 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,11 +203,12 @@ static void fd_revalidate(FDrive *drv)
drv->drive = drive;
drv->media_rate = rate;
} else {
-FLOPPY_DPRINTF("No disk in drive\n");
+FLOPPY_DPRINTF("Drive disabled\n");
drv->last_sect = 0;
drv->max_track = 0;
drv->flags&= ~FDISK_DBL_SIDES;
}
+
}
//
@@ -937,6 +940,9 @@ static int fdctrl_media_changed(FDrive *drv)
if (!drv->bs)
return 0;
+/* This is needed for driver to detect there is no media in drive */
+if (!bdrv_is_inserted(drv->bs))
+return 1;
if (drv->media_changed) {
drv->media_changed = 0;
ret = 1;
diff --git a/hw/pc.c b/hw/pc.c
index 1f5aacb..29a604b 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -382,7 +382,8 @@ 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 there is no media in a drive, we still have the drive
present */
This comment explains why you remove bdrv_is_inserted(). It makes no
sense once its gone.
+if (fd[i]) {
bdrv_get_floppy_geometry_hint(fd[i],&nb_heads,&max_track,
&last_sect, FDRIVE_DRV_NONE,
&fd_type[i],&rate);
This fixes the CMOS floppy initialization bug I described elsewhere in
the thread, because bdrv_get_floppy_geometry_hint() picks FDRIVE_DRV_144
when its bs argument is empty.
Are you sure the patches to fdc.c are necessary to fix the bug you're
seeing? To be honest, I doubt it.
Hi Markus,
yes, you are right that this is only needed to fix that bug and fixes to
fdc.c in this patch isn't necessary to fix this bug.
But I also need to fix media detection behaviour in fdc.c otherwise
windows guest will print error message while opening floppy drive and
kernel guest ends with kernel panic while mounting floppy drive.
I have already started rewriting this patch.
Pavel
Re: [Qemu-devel] [PATCH] Fix IDE FDC emulation for no media
Pavel Hrdina writes:
> Hi,
> this is the patch to fix incorrect handling of IDE floppy drive controller
> emulation
> when no media is present. If the guest is booted without a media then the
> drive
> was not being emulated at all but this patch enables the emulation with no
> media present.
>
> There was a bug in FDC emulation without media. Driver was not able to
> recognize that
> there is no media in drive.
>
> This has been tested on both Fedora-16 x86_64 VM and Windows XP VM and the
> behaviour
> is as expected, i.e. as follows:
>
> Linux guest (Fedora 16 x86_64) tries "mount /dev/fd0" and exit with error
> "mount: /dev/fd0 is not a valid block device" which is the same behavior like
> bare metal with real floppy device (you have to load floppy driver at first
> using e.g. "modprobe floppy" command).
>
> For Windows XP guest the Windows floppy driver is trying to seek the virtual
> drive
> when you want to open it but driver successfully detect that there is no
> media in drive
> and then it's asking user to insert floppy media in the drive.
>
> I also tested behavior of this patch if you start guest with "-nodefaults"
> and both
> Windows and Linux guests detect only FDC but no drive.
>
> Pavel
As Andreas already pointed out, your commit message needs work. Please
limit line length to 70 characters.
> This patch has been written with help of specifications from:
> http://www.ousob.com/ng/hardware/ngd127.php
> http://www.isdaman.com/alsos/hardware/fdc/floppy.htm
> http://wiki.osdev.org/Floppy_Disk_Controller
>
> Signed-off-by: Pavel Hrdina
> Signed-off-by: Michal Novotny
> ---
> hw/fdc.c | 14 ++
> hw/pc.c |3 ++-
> 2 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/hw/fdc.c b/hw/fdc.c
> index a0236b7..6791eff 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 media 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,11 +203,12 @@ static void fd_revalidate(FDrive *drv)
> drv->drive = drive;
> drv->media_rate = rate;
> } else {
> -FLOPPY_DPRINTF("No disk in drive\n");
> +FLOPPY_DPRINTF("Drive disabled\n");
> drv->last_sect = 0;
> drv->max_track = 0;
> drv->flags &= ~FDISK_DBL_SIDES;
> }
> +
> }
>
> //
> @@ -937,6 +940,9 @@ static int fdctrl_media_changed(FDrive *drv)
>
> if (!drv->bs)
> return 0;
> +/* This is needed for driver to detect there is no media in drive */
> +if (!bdrv_is_inserted(drv->bs))
> +return 1;
> if (drv->media_changed) {
> drv->media_changed = 0;
> ret = 1;
> diff --git a/hw/pc.c b/hw/pc.c
> index 1f5aacb..29a604b 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -382,7 +382,8 @@ 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 there is no media in a drive, we still have the drive
> present */
This comment explains why you remove bdrv_is_inserted(). It makes no
sense once its gone.
> +if (fd[i]) {
> bdrv_get_floppy_geometry_hint(fd[i], &nb_heads, &max_track,
>&last_sect, FDRIVE_DRV_NONE,
>&fd_type[i], &rate);
This fixes the CMOS floppy initialization bug I described elsewhere in
the thread, because bdrv_get_floppy_geometry_hint() picks FDRIVE_DRV_144
when its bs argument is empty.
Are you sure the patches to fdc.c are necessary to fix the bug you're
seeing? To be honest, I doubt it.
Re: [Qemu-devel] [PATCH] Fix IDE FDC emulation for no media
Kevin Wolf writes:
> Am 24.04.2012 11:55, schrieb Pavel Hrdina:
>> On 04/24/2012 11:32 AM, Kevin Wolf wrote:
>>> Am 23.04.2012 18:06, schrieb Pavel Hrdina:
Hi,
this is the patch to fix incorrect handling of IDE floppy drive controller
emulation
when no media is present. If the guest is booted without a media then the
drive
was not being emulated at all but this patch enables the emulation with no
media present.
There was a bug in FDC emulation without media. Driver was not able to
recognize that
there is no media in drive.
This has been tested on both Fedora-16 x86_64 VM and Windows XP VM and the
behaviour
is as expected, i.e. as follows:
Linux guest (Fedora 16 x86_64) tries "mount /dev/fd0" and exit with error
"mount: /dev/fd0 is not a valid block device" which is the same behavior
like
bare metal with real floppy device (you have to load floppy driver at first
using e.g. "modprobe floppy" command).
For Windows XP guest the Windows floppy driver is trying to seek the
virtual drive
when you want to open it but driver successfully detect that there is no
media in drive
and then it's asking user to insert floppy media in the drive.
I also tested behavior of this patch if you start guest with "-nodefaults"
and both
Windows and Linux guests detect only FDC but no drive.
Pavel
This patch has been written with help of specifications from:
http://www.ousob.com/ng/hardware/ngd127.php
http://www.isdaman.com/alsos/hardware/fdc/floppy.htm
http://wiki.osdev.org/Floppy_Disk_Controller
Signed-off-by: Pavel Hrdina
Signed-off-by: Michal Novotny
>>> It would be cool to have a qtest case for this. But I think we don't
>>> really have a nice way to talk to the qemu monitor yet, so I'm not
>>> requesting this before the patch can go in.
>>>
---
hw/fdc.c | 14 ++
hw/pc.c |3 ++-
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/hw/fdc.c b/hw/fdc.c
index a0236b7..6791eff 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);
>>> I'm not sure how your patch works, but I believe the behaviour of
>>> bdrv_get_floppy_geometry_hint might be one of the keys. If I understand
>>> correctly, it will just return the default geometry, which is one for
>>> 3.5" 1.44 MB floppies, or more precisely:
>>>
>>> { FDRIVE_DRV_144, 20, 80, 1, FDRIVE_RATE_500K, },
>>>
>>> Why it makes sense to have a medium geometry when there is no medium I
>>> haven't understood yet, but last_sect/max_track = 0 didn't seem to be
>>> enough for you. Do you know what exactly it is that makes your case work?
>>>
>>> ro has undefined value for a BlockDriverState with no medium, but I
>>> guess it doesn't hurt.
>> This modification is needed for floppy driver in guest to detect floppy
>> drive.
>
> My question was more about how the floppy drivers in the guest detect
> drives. They can't be relying on the geometry of a medium because no
> medium is inserted. So setting the geometry must have some side effect
> that I'm not aware of.
One way to detect floppies is the CMOS floppy byte, and that way is
broken right now. Have a look at pc_cmos_init():
FDriveType fd_type[2] = { FDRIVE_DRV_NONE, FDRIVE_DRV_NONE };
[...]
/* floppy type */
if (floppy) {
fdc_get_bs(fd, floppy);
for (i = 0; i < 2; i++) {
if (fd[i] && bdrv_is_inserted(fd[i])) {
bdrv_get_floppy_geometry_hint(fd[i], &nb_heads, &max_track,
&last_sect, FDRIVE_DRV_NONE,
&fd_type[i], &rate);
}
}
}
val = (cmos_get_fd_drive_type(fd_type[0]) << 4) |
cmos_get_fd_drive_type(fd_type[1]);
rtc_set_memory(s, 0x10, val);
The drive type remains FDRIVE_DRV_NONE, which means "no drive
connected", unless the drive exists *and* has media.
Actually, guessing the drive type from media is problematic. It should
be a qdev property.
See also https://bugzilla.redhat.com/show_bug.cgi?id=729244
[...]
Re: [Qemu-devel] [PATCH] Fix IDE FDC emulation for no media
Hi, Pavel Hrdina a écrit : On 04/24/2012 09:48 AM, Andreas Färber wrote: Am 23.04.2012 18:06, schrieb Pavel Hrdina: Hi, this is the patch to fix incorrect handling of IDE floppy drive controller emulation when no media is present. If the guest is booted without a media then the drive was not being emulated at all but this patch enables the emulation with no media present. There was a bug in FDC emulation without media. Driver was not able to recognize that there is no media in drive. >>> [...] I just sent a patch to simplify media change handling [1]. Can you try to see if it fixes your problem? Regards Hervé [1] http://article.gmane.org/gmane.comp.emulators.qemu/148187
Re: [Qemu-devel] [PATCH] Fix IDE FDC emulation for no media
Am 24.04.2012 11:55, schrieb Pavel Hrdina:
> On 04/24/2012 11:32 AM, Kevin Wolf wrote:
>> Am 23.04.2012 18:06, schrieb Pavel Hrdina:
>>> Hi,
>>> this is the patch to fix incorrect handling of IDE floppy drive controller
>>> emulation
>>> when no media is present. If the guest is booted without a media then the
>>> drive
>>> was not being emulated at all but this patch enables the emulation with no
>>> media present.
>>>
>>> There was a bug in FDC emulation without media. Driver was not able to
>>> recognize that
>>> there is no media in drive.
>>>
>>> This has been tested on both Fedora-16 x86_64 VM and Windows XP VM and the
>>> behaviour
>>> is as expected, i.e. as follows:
>>>
>>> Linux guest (Fedora 16 x86_64) tries "mount /dev/fd0" and exit with error
>>> "mount: /dev/fd0 is not a valid block device" which is the same behavior
>>> like
>>> bare metal with real floppy device (you have to load floppy driver at first
>>> using e.g. "modprobe floppy" command).
>>>
>>> For Windows XP guest the Windows floppy driver is trying to seek the
>>> virtual drive
>>> when you want to open it but driver successfully detect that there is no
>>> media in drive
>>> and then it's asking user to insert floppy media in the drive.
>>>
>>> I also tested behavior of this patch if you start guest with "-nodefaults"
>>> and both
>>> Windows and Linux guests detect only FDC but no drive.
>>>
>>> Pavel
>>>
>>> This patch has been written with help of specifications from:
>>> http://www.ousob.com/ng/hardware/ngd127.php
>>> http://www.isdaman.com/alsos/hardware/fdc/floppy.htm
>>> http://wiki.osdev.org/Floppy_Disk_Controller
>>>
>>> Signed-off-by: Pavel Hrdina
>>> Signed-off-by: Michal Novotny
>> It would be cool to have a qtest case for this. But I think we don't
>> really have a nice way to talk to the qemu monitor yet, so I'm not
>> requesting this before the patch can go in.
>>
>>> ---
>>> hw/fdc.c | 14 ++
>>> hw/pc.c |3 ++-
>>> 2 files changed, 12 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/hw/fdc.c b/hw/fdc.c
>>> index a0236b7..6791eff 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);
>> I'm not sure how your patch works, but I believe the behaviour of
>> bdrv_get_floppy_geometry_hint might be one of the keys. If I understand
>> correctly, it will just return the default geometry, which is one for
>> 3.5" 1.44 MB floppies, or more precisely:
>>
>> { FDRIVE_DRV_144, 20, 80, 1, FDRIVE_RATE_500K, },
>>
>> Why it makes sense to have a medium geometry when there is no medium I
>> haven't understood yet, but last_sect/max_track = 0 didn't seem to be
>> enough for you. Do you know what exactly it is that makes your case work?
>>
>> ro has undefined value for a BlockDriverState with no medium, but I
>> guess it doesn't hurt.
> This modification is needed for floppy driver in guest to detect floppy drive.
My question was more about how the floppy drivers in the guest detect
drives. They can't be relying on the geometry of a medium because no
medium is inserted. So setting the geometry must have some side effect
that I'm not aware of.
>>>
>>> //
>>> @@ -937,6 +940,9 @@ static int fdctrl_media_changed(FDrive *drv)
>>>
>>> if (!drv->bs)
>>> return 0;
>>> +/* This is needed for driver to detect there is no media in drive */
>>> +if (!bdrv_is_inserted(drv->bs))
>>> +return 1;
>> In which case is this required to detect that there is no media? After
>> eject? If so, why isn't the code in fdctrl_change_cb() enough?
>>
>> Or do you in fact need it for the initial state?
> As i wrote to Stefan,
> You can look here, http://www.isdaman.com/alsos/hardware/fdc/floppy.htm
> , for specification of DIR register. Bit7 is there as CHAN and in this
> bit is saved information whether media is changed or not. This bit is
> set to true while there is no media. And floppy driver is checking this
> bit to detect media change or media missing.
>
> And this is needed for all cases if there is no media in drive. Code in
> fdctrl_change_cb() is needed only for detect that there is no media when
> you try to mount it (linux guest) or open it (windows guest).
Hm. There seems to be more wrong that just this. I think the main
problem is that we clear the bit after reading it out once, whereas it
seems to stay set in reality. It would only be cleared once some command
(not sure which are possible) succeeds with a newly inserted disk.
Unfortunately I couldn't really find any appropriate documentation for
Re: [Qemu-devel] [PATCH] Fix IDE FDC emulation for no media
On 04/24/2012 01:38 PM, Stefan Hajnoczi wrote:
On Tue, Apr 24, 2012 at 10:46 AM, Pavel Hrdina wrote:
On 04/24/2012 11:06 AM, Stefan Hajnoczi wrote:
On Mon, Apr 23, 2012 at 5:06 PM, Pavel Hrdina wrote:
Hi,
this is the patch to fix incorrect handling of IDE floppy drive controller
emulation
s/IDE//
It's unrelated to IDE.
@@ -937,6 +940,9 @@ static int fdctrl_media_changed(FDrive *drv)
if (!drv->bs)
return 0;
+/* This is needed for driver to detect there is no media in drive */
+if (!bdrv_is_inserted(drv->bs))
+return 1;
if (drv->media_changed) {
drv->media_changed = 0;
ret = 1;
Why isn't the BlockDevOps.change_media_cb() mechanism enough to report
disk changes correctly?
Stefan
You can look here, http://www.isdaman.com/alsos/hardware/fdc/floppy.htm ,
for specification of DIR register. Bit7 is there as CHAN and in this bit is
saved information whether media is changed or not. This bit is set to true
while there is no media. And floppy driver is checking this bit to detect
media change or media missing.
I can't find anything from your link that says the bit is set while
there is no media.
"Disk Change: 1 = disk changed since last command, 0 = disk not changed"
The "82078 44 PIN CHMOS SINGLE-CHIP FLOPPY DISK CONTROLLER" datasheet
(http://wiki.qemu.org/images/f/f0/29047403.pdf) doesn't give details
but suggests what you are saying. It says "DSKCHG monitors the pin of
the same name and
reflects the opposite value seen on the disk cable". So perhaps this
is really a level-triggered "is the drive empty?" bit.
This is still pretty unclear to me. Also, if you need to implement
the level-triggered behavior, then can we remove/simplify/clean up the
existing media change code in hw/fdc.c?
Stefan
I didn't find anything about that too, but I checked this behaviour on
real floppy drive and it returns always this bit setted up without
media. It could be probably good to rewrite media change code.
Re: [Qemu-devel] [PATCH] Fix IDE FDC emulation for no media
On 04/24/2012 12:23 PM, Kevin Wolf wrote:
Am 24.04.2012 11:55, schrieb Pavel Hrdina:
On 04/24/2012 11:32 AM, Kevin Wolf wrote:
Am 23.04.2012 18:06, schrieb Pavel Hrdina:
Hi,
this is the patch to fix incorrect handling of IDE floppy drive controller
emulation
when no media is present. If the guest is booted without a media then the drive
was not being emulated at all but this patch enables the emulation with no
media present.
There was a bug in FDC emulation without media. Driver was not able to
recognize that
there is no media in drive.
This has been tested on both Fedora-16 x86_64 VM and Windows XP VM and the
behaviour
is as expected, i.e. as follows:
Linux guest (Fedora 16 x86_64) tries "mount /dev/fd0" and exit with error
"mount: /dev/fd0 is not a valid block device" which is the same behavior like
bare metal with real floppy device (you have to load floppy driver at first
using e.g. "modprobe floppy" command).
For Windows XP guest the Windows floppy driver is trying to seek the virtual
drive
when you want to open it but driver successfully detect that there is no media
in drive
and then it's asking user to insert floppy media in the drive.
I also tested behavior of this patch if you start guest with "-nodefaults" and
both
Windows and Linux guests detect only FDC but no drive.
Pavel
This patch has been written with help of specifications from:
http://www.ousob.com/ng/hardware/ngd127.php
http://www.isdaman.com/alsos/hardware/fdc/floppy.htm
http://wiki.osdev.org/Floppy_Disk_Controller
Signed-off-by: Pavel Hrdina
Signed-off-by: Michal Novotny
It would be cool to have a qtest case for this. But I think we don't
really have a nice way to talk to the qemu monitor yet, so I'm not
requesting this before the patch can go in.
---
hw/fdc.c | 14 ++
hw/pc.c |3 ++-
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/hw/fdc.c b/hw/fdc.c
index a0236b7..6791eff 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);
I'm not sure how your patch works, but I believe the behaviour of
bdrv_get_floppy_geometry_hint might be one of the keys. If I understand
correctly, it will just return the default geometry, which is one for
3.5" 1.44 MB floppies, or more precisely:
{ FDRIVE_DRV_144, 20, 80, 1, FDRIVE_RATE_500K, },
Why it makes sense to have a medium geometry when there is no medium I
haven't understood yet, but last_sect/max_track = 0 didn't seem to be
enough for you. Do you know what exactly it is that makes your case work?
ro has undefined value for a BlockDriverState with no medium, but I
guess it doesn't hurt.
This modification is needed for floppy driver in guest to detect floppy drive.
My question was more about how the floppy drivers in the guest detect
drives. They can't be relying on the geometry of a medium because no
medium is inserted. So setting the geometry must have some side effect
that I'm not aware of.
Well, this is good question, I'll investigate little bit more about this
and probably send new version of this patch.
//
@@ -937,6 +940,9 @@ static int fdctrl_media_changed(FDrive *drv)
if (!drv->bs)
return 0;
+/* This is needed for driver to detect there is no media in drive */
+if (!bdrv_is_inserted(drv->bs))
+return 1;
In which case is this required to detect that there is no media? After
eject? If so, why isn't the code in fdctrl_change_cb() enough?
Or do you in fact need it for the initial state?
As i wrote to Stefan,
You can look here, http://www.isdaman.com/alsos/hardware/fdc/floppy.htm
, for specification of DIR register. Bit7 is there as CHAN and in this
bit is saved information whether media is changed or not. This bit is
set to true while there is no media. And floppy driver is checking this
bit to detect media change or media missing.
And this is needed for all cases if there is no media in drive. Code in
fdctrl_change_cb() is needed only for detect that there is no media when
you try to mount it (linux guest) or open it (windows guest).
Hm. There seems to be more wrong that just this. I think the main
problem is that we clear the bit after reading it out once, whereas it
seems to stay set in reality. It would only be cleared once some command
(not sure which are possible) succeeds with a newly inserted disk.
Unfortunately I couldn't really find any appropriate documentation for
the bit. The FDC spec says "DSKCHG monitors the pin of the same name and
reflects the opposite value seen on the disk cable", but I couldn't find
any description on how flo
Re: [Qemu-devel] [PATCH] Fix IDE FDC emulation for no media
Am 24.04.2012 12:28, schrieb Pavel Hrdina:
> On 04/24/2012 12:23 PM, Kevin Wolf wrote:
>> Am 24.04.2012 11:55, schrieb Pavel Hrdina:
>>> On 04/24/2012 11:32 AM, Kevin Wolf wrote:
Am 23.04.2012 18:06, schrieb Pavel Hrdina:
> Hi,
> this is the patch to fix incorrect handling of IDE floppy drive
> controller emulation
> when no media is present. If the guest is booted without a media then the
> drive
> was not being emulated at all but this patch enables the emulation with
> no media present.
>
> There was a bug in FDC emulation without media. Driver was not able to
> recognize that
> there is no media in drive.
>
> This has been tested on both Fedora-16 x86_64 VM and Windows XP VM and
> the behaviour
> is as expected, i.e. as follows:
>
> Linux guest (Fedora 16 x86_64) tries "mount /dev/fd0" and exit with error
> "mount: /dev/fd0 is not a valid block device" which is the same behavior
> like
> bare metal with real floppy device (you have to load floppy driver at
> first
> using e.g. "modprobe floppy" command).
>
> For Windows XP guest the Windows floppy driver is trying to seek the
> virtual drive
> when you want to open it but driver successfully detect that there is no
> media in drive
> and then it's asking user to insert floppy media in the drive.
>
> I also tested behavior of this patch if you start guest with
> "-nodefaults" and both
> Windows and Linux guests detect only FDC but no drive.
>
> Pavel
>
> This patch has been written with help of specifications from:
> http://www.ousob.com/ng/hardware/ngd127.php
> http://www.isdaman.com/alsos/hardware/fdc/floppy.htm
> http://wiki.osdev.org/Floppy_Disk_Controller
>
> Signed-off-by: Pavel Hrdina
> Signed-off-by: Michal Novotny
It would be cool to have a qtest case for this. But I think we don't
really have a nice way to talk to the qemu monitor yet, so I'm not
requesting this before the patch can go in.
> ---
>hw/fdc.c | 14 ++
>hw/pc.c |3 ++-
>2 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/hw/fdc.c b/hw/fdc.c
> index a0236b7..6791eff 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);
I'm not sure how your patch works, but I believe the behaviour of
bdrv_get_floppy_geometry_hint might be one of the keys. If I understand
correctly, it will just return the default geometry, which is one for
3.5" 1.44 MB floppies, or more precisely:
{ FDRIVE_DRV_144, 20, 80, 1, FDRIVE_RATE_500K, },
Why it makes sense to have a medium geometry when there is no medium I
haven't understood yet, but last_sect/max_track = 0 didn't seem to be
enough for you. Do you know what exactly it is that makes your case work?
ro has undefined value for a BlockDriverState with no medium, but I
guess it doesn't hurt.
>>> This modification is needed for floppy driver in guest to detect floppy
>>> drive.
>> My question was more about how the floppy drivers in the guest detect
>> drives. They can't be relying on the geometry of a medium because no
>> medium is inserted. So setting the geometry must have some side effect
>> that I'm not aware of.
> Well, this is good question, I'll investigate little bit more about this
> and probably send new version of this patch.
Ok, thanks. I think it's important to understand _why_ a patch works,
not just _that_ it works (for a specific case).
>//
> @@ -937,6 +940,9 @@ static int fdctrl_media_changed(FDrive *drv)
>
>if (!drv->bs)
>return 0;
> +/* This is needed for driver to detect there is no media in drive */
> +if (!bdrv_is_inserted(drv->bs))
> +return 1;
In which case is this required to detect that there is no media? After
eject? If so, why isn't the code in fdctrl_change_cb() enough?
Or do you in fact need it for the initial state?
>>> As i wrote to Stefan,
>>> You can look here, http://www.isdaman.com/alsos/hardware/fdc/floppy.htm
>>> , for specification of DIR register. Bit7 is there as CHAN and in this
>>> bit is saved information whether media is changed or not. This bit is
>>> set to true while there is no media. And floppy driver is checking this
>>> bit to detect media change or media missing.
>>>
>>> And
Re: [Qemu-devel] [PATCH] Fix IDE FDC emulation for no media
On Tue, Apr 24, 2012 at 10:46 AM, Pavel Hrdina wrote:
>
> On 04/24/2012 11:06 AM, Stefan Hajnoczi wrote:
>
> On Mon, Apr 23, 2012 at 5:06 PM, Pavel Hrdina wrote:
>
> Hi,
> this is the patch to fix incorrect handling of IDE floppy drive controller
> emulation
>
> s/IDE//
>
> It's unrelated to IDE.
>
> @@ -937,6 +940,9 @@ static int fdctrl_media_changed(FDrive *drv)
>
> if (!drv->bs)
> return 0;
> + /* This is needed for driver to detect there is no media in drive */
> + if (!bdrv_is_inserted(drv->bs))
> + return 1;
> if (drv->media_changed) {
> drv->media_changed = 0;
> ret = 1;
>
> Why isn't the BlockDevOps.change_media_cb() mechanism enough to report
> disk changes correctly?
>
> Stefan
>
> You can look here, http://www.isdaman.com/alsos/hardware/fdc/floppy.htm ,
> for specification of DIR register. Bit7 is there as CHAN and in this bit is
> saved information whether media is changed or not. This bit is set to true
> while there is no media. And floppy driver is checking this bit to detect
> media change or media missing.
I can't find anything from your link that says the bit is set while
there is no media.
"Disk Change: 1 = disk changed since last command, 0 = disk not changed"
The "82078 44 PIN CHMOS SINGLE-CHIP FLOPPY DISK CONTROLLER" datasheet
(http://wiki.qemu.org/images/f/f0/29047403.pdf) doesn't give details
but suggests what you are saying. It says "DSKCHG monitors the pin of
the same name and
reflects the opposite value seen on the disk cable". So perhaps this
is really a level-triggered "is the drive empty?" bit.
This is still pretty unclear to me. Also, if you need to implement
the level-triggered behavior, then can we remove/simplify/clean up the
existing media change code in hw/fdc.c?
Stefan
Re: [Qemu-devel] [PATCH] Fix IDE FDC emulation for no media
On 04/24/2012 09:48 AM, Andreas Färber wrote:
Am 23.04.2012 18:06, schrieb Pavel Hrdina:
Hi,
this is the patch to fix incorrect handling of IDE floppy drive controller
emulation
when no media is present. If the guest is booted without a media then the drive
was not being emulated at all but this patch enables the emulation with no
media present.
There was a bug in FDC emulation without media. Driver was not able to
recognize that
there is no media in drive.
This has been tested on both Fedora-16 x86_64 VM and Windows XP VM and the
behaviour
is as expected, i.e. as follows:
Linux guest (Fedora 16 x86_64) tries "mount /dev/fd0" and exit with error
"mount: /dev/fd0 is not a valid block device" which is the same behavior like
bare metal with real floppy device (you have to load floppy driver at first
using e.g. "modprobe floppy" command).
For Windows XP guest the Windows floppy driver is trying to seek the virtual
drive
when you want to open it but driver successfully detect that there is no media
in drive
and then it's asking user to insert floppy media in the drive.
I also tested behavior of this patch if you start guest with "-nodefaults" and
both
Windows and Linux guests detect only FDC but no drive.
Pavel
This patch has been written with help of specifications from:
http://www.ousob.com/ng/hardware/ngd127.php
http://www.isdaman.com/alsos/hardware/fdc/floppy.htm
http://wiki.osdev.org/Floppy_Disk_Controller
Signed-off-by: Pavel Hrdina
Signed-off-by: Michal Novotny
---
Please see http://wiki.qemu.org/Contribute/SubmitAPatch for hints on
writing a commit message for a patch: Subject should have a prefix, such
as "fdc: Fix emulation for no media" and the body should not contain
personal letter-style contents such as "Hi, this is the patch". Any such
personal comments can go under the --- line where they are not committed
to git history.
Cc'ing floppy author and block maintainer.
Andreas
Thanks for correction, I'll send v2 patch with right commit message.
Pavel
hw/fdc.c | 14 ++
hw/pc.c |3 ++-
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/hw/fdc.c b/hw/fdc.c
index a0236b7..6791eff 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 media 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,11 +203,12 @@ static void fd_revalidate(FDrive *drv)
drv->drive = drive;
drv->media_rate = rate;
} else {
-FLOPPY_DPRINTF("No disk in drive\n");
+FLOPPY_DPRINTF("Drive disabled\n");
drv->last_sect = 0;
drv->max_track = 0;
drv->flags&= ~FDISK_DBL_SIDES;
}
+
}
//
@@ -937,6 +940,9 @@ static int fdctrl_media_changed(FDrive *drv)
if (!drv->bs)
return 0;
+/* This is needed for driver to detect there is no media in drive */
+if (!bdrv_is_inserted(drv->bs))
+return 1;
if (drv->media_changed) {
drv->media_changed = 0;
ret = 1;
diff --git a/hw/pc.c b/hw/pc.c
index 1f5aacb..29a604b 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -382,7 +382,8 @@ 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 there is no media in a drive, we still have the drive
present */
+if (fd[i]) {
bdrv_get_floppy_geometry_hint(fd[i],&nb_heads,&max_track,
&last_sect, FDRIVE_DRV_NONE,
&fd_type[i],&rate);
Re: [Qemu-devel] [PATCH] Fix IDE FDC emulation for no media
On 04/24/2012 11:32 AM, Kevin Wolf wrote:
Am 23.04.2012 18:06, schrieb Pavel Hrdina:
Hi,
this is the patch to fix incorrect handling of IDE floppy drive controller
emulation
when no media is present. If the guest is booted without a media then the drive
was not being emulated at all but this patch enables the emulation with no
media present.
There was a bug in FDC emulation without media. Driver was not able to
recognize that
there is no media in drive.
This has been tested on both Fedora-16 x86_64 VM and Windows XP VM and the
behaviour
is as expected, i.e. as follows:
Linux guest (Fedora 16 x86_64) tries "mount /dev/fd0" and exit with error
"mount: /dev/fd0 is not a valid block device" which is the same behavior like
bare metal with real floppy device (you have to load floppy driver at first
using e.g. "modprobe floppy" command).
For Windows XP guest the Windows floppy driver is trying to seek the virtual
drive
when you want to open it but driver successfully detect that there is no media
in drive
and then it's asking user to insert floppy media in the drive.
I also tested behavior of this patch if you start guest with "-nodefaults" and
both
Windows and Linux guests detect only FDC but no drive.
Pavel
This patch has been written with help of specifications from:
http://www.ousob.com/ng/hardware/ngd127.php
http://www.isdaman.com/alsos/hardware/fdc/floppy.htm
http://wiki.osdev.org/Floppy_Disk_Controller
Signed-off-by: Pavel Hrdina
Signed-off-by: Michal Novotny
It would be cool to have a qtest case for this. But I think we don't
really have a nice way to talk to the qemu monitor yet, so I'm not
requesting this before the patch can go in.
---
hw/fdc.c | 14 ++
hw/pc.c |3 ++-
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/hw/fdc.c b/hw/fdc.c
index a0236b7..6791eff 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);
I'm not sure how your patch works, but I believe the behaviour of
bdrv_get_floppy_geometry_hint might be one of the keys. If I understand
correctly, it will just return the default geometry, which is one for
3.5" 1.44 MB floppies, or more precisely:
{ FDRIVE_DRV_144, 20, 80, 1, FDRIVE_RATE_500K, },
Why it makes sense to have a medium geometry when there is no medium I
haven't understood yet, but last_sect/max_track = 0 didn't seem to be
enough for you. Do you know what exactly it is that makes your case work?
ro has undefined value for a BlockDriverState with no medium, but I
guess it doesn't hurt.
This modification is needed for floppy driver in guest to detect floppy drive.
-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 media 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,11 +203,12 @@ static void fd_revalidate(FDrive *drv)
drv->drive = drive;
drv->media_rate = rate;
} else {
-FLOPPY_DPRINTF("No disk in drive\n");
+FLOPPY_DPRINTF("Drive disabled\n");
drv->last_sect = 0;
drv->max_track = 0;
drv->flags&= ~FDISK_DBL_SIDES;
}
+
}
This code is needed to set up default drive geometry so guest can detect
floppy drive controller.
//
@@ -937,6 +940,9 @@ static int fdctrl_media_changed(FDrive *drv)
if (!drv->bs)
return 0;
+/* This is needed for driver to detect there is no media in drive */
+if (!bdrv_is_inserted(drv->bs))
+return 1;
In which case is this required to detect that there is no media? After
eject? If so, why isn't the code in fdctrl_change_cb() enough?
Or do you in fact need it for the initial state?
As i wrote to Stefan,
You can look here, http://www.isdaman.com/alsos/hardware/fdc/floppy.htm
, for specification of DIR register. Bit7 is there as CHAN and in this
bit is saved information whether media is changed or not. This bit is
set to true while there is no media. And floppy driver is checking this
bit to detect media change or media missing.
And this is needed for all cases if there is no media in drive. Code in
fdctrl_change_cb() is needed only for detect that there is no media when
you try to mount it (linux guest) or open it (windows guest).
Re: [Qemu-devel] [PATCH] Fix IDE FDC emulation for no media
Am 23.04.2012 18:06, schrieb Pavel Hrdina:
> Hi,
> this is the patch to fix incorrect handling of IDE floppy drive controller
> emulation
> when no media is present. If the guest is booted without a media then the
> drive
> was not being emulated at all but this patch enables the emulation with no
> media present.
>
> There was a bug in FDC emulation without media. Driver was not able to
> recognize that
> there is no media in drive.
>
> This has been tested on both Fedora-16 x86_64 VM and Windows XP VM and the
> behaviour
> is as expected, i.e. as follows:
>
> Linux guest (Fedora 16 x86_64) tries "mount /dev/fd0" and exit with error
> "mount: /dev/fd0 is not a valid block device" which is the same behavior like
> bare metal with real floppy device (you have to load floppy driver at first
> using e.g. "modprobe floppy" command).
>
> For Windows XP guest the Windows floppy driver is trying to seek the virtual
> drive
> when you want to open it but driver successfully detect that there is no
> media in drive
> and then it's asking user to insert floppy media in the drive.
>
> I also tested behavior of this patch if you start guest with "-nodefaults"
> and both
> Windows and Linux guests detect only FDC but no drive.
>
> Pavel
>
> This patch has been written with help of specifications from:
> http://www.ousob.com/ng/hardware/ngd127.php
> http://www.isdaman.com/alsos/hardware/fdc/floppy.htm
> http://wiki.osdev.org/Floppy_Disk_Controller
>
> Signed-off-by: Pavel Hrdina
> Signed-off-by: Michal Novotny
It would be cool to have a qtest case for this. But I think we don't
really have a nice way to talk to the qemu monitor yet, so I'm not
requesting this before the patch can go in.
> ---
> hw/fdc.c | 14 ++
> hw/pc.c |3 ++-
> 2 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/hw/fdc.c b/hw/fdc.c
> index a0236b7..6791eff 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);
I'm not sure how your patch works, but I believe the behaviour of
bdrv_get_floppy_geometry_hint might be one of the keys. If I understand
correctly, it will just return the default geometry, which is one for
3.5" 1.44 MB floppies, or more precisely:
{ FDRIVE_DRV_144, 20, 80, 1, FDRIVE_RATE_500K, },
Why it makes sense to have a medium geometry when there is no medium I
haven't understood yet, but last_sect/max_track = 0 didn't seem to be
enough for you. Do you know what exactly it is that makes your case work?
ro has undefined value for a BlockDriverState with no medium, but I
guess it doesn't hurt.
> -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 media 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,11 +203,12 @@ static void fd_revalidate(FDrive *drv)
> drv->drive = drive;
> drv->media_rate = rate;
> } else {
> -FLOPPY_DPRINTF("No disk in drive\n");
> +FLOPPY_DPRINTF("Drive disabled\n");
> drv->last_sect = 0;
> drv->max_track = 0;
> drv->flags &= ~FDISK_DBL_SIDES;
> }
> +
> }
>
> //
> @@ -937,6 +940,9 @@ static int fdctrl_media_changed(FDrive *drv)
>
> if (!drv->bs)
> return 0;
> +/* This is needed for driver to detect there is no media in drive */
> +if (!bdrv_is_inserted(drv->bs))
> +return 1;
In which case is this required to detect that there is no media? After
eject? If so, why isn't the code in fdctrl_change_cb() enough?
Or do you in fact need it for the initial state?
Kevin
Re: [Qemu-devel] [PATCH] Fix IDE FDC emulation for no media
On 04/24/2012 11:06 AM, Stefan Hajnoczi wrote:
On Mon, Apr 23, 2012 at 5:06 PM, Pavel Hrdina wrote:
Hi,
this is the patch to fix incorrect handling of IDE floppy drive controller
emulation
s/IDE//
It's unrelated to IDE.
@@ -937,6 +940,9 @@ static int fdctrl_media_changed(FDrive *drv)
if (!drv->bs)
return 0;
+/* This is needed for driver to detect there is no media in drive */
+if (!bdrv_is_inserted(drv->bs))
+return 1;
if (drv->media_changed) {
drv->media_changed = 0;
ret = 1;
Why isn't the BlockDevOps.change_media_cb() mechanism enough to report
disk changes correctly?
Stefan
You can look here, http://www.isdaman.com/alsos/hardware/fdc/floppy.htm
, for specification of DIR register. Bit7 is there as CHAN and in this
bit is saved information whether media is changed or not. This bit is
set to true while there is no media. And floppy driver is checking this
bit to detect media change or media missing.
Re: [Qemu-devel] [PATCH] Fix IDE FDC emulation for no media
On Mon, Apr 23, 2012 at 5:06 PM, Pavel Hrdina wrote:
> Hi,
> this is the patch to fix incorrect handling of IDE floppy drive controller
> emulation
s/IDE//
It's unrelated to IDE.
> @@ -937,6 +940,9 @@ static int fdctrl_media_changed(FDrive *drv)
>
> if (!drv->bs)
> return 0;
> + /* This is needed for driver to detect there is no media in drive */
> + if (!bdrv_is_inserted(drv->bs))
> + return 1;
> if (drv->media_changed) {
> drv->media_changed = 0;
> ret = 1;
Why isn't the BlockDevOps.change_media_cb() mechanism enough to report
disk changes correctly?
Stefan
Re: [Qemu-devel] [PATCH] Fix IDE FDC emulation for no media
Am 23.04.2012 18:06, schrieb Pavel Hrdina:
> Hi,
> this is the patch to fix incorrect handling of IDE floppy drive controller
> emulation
> when no media is present. If the guest is booted without a media then the
> drive
> was not being emulated at all but this patch enables the emulation with no
> media present.
>
> There was a bug in FDC emulation without media. Driver was not able to
> recognize that
> there is no media in drive.
>
> This has been tested on both Fedora-16 x86_64 VM and Windows XP VM and the
> behaviour
> is as expected, i.e. as follows:
>
> Linux guest (Fedora 16 x86_64) tries "mount /dev/fd0" and exit with error
> "mount: /dev/fd0 is not a valid block device" which is the same behavior like
> bare metal with real floppy device (you have to load floppy driver at first
> using e.g. "modprobe floppy" command).
>
> For Windows XP guest the Windows floppy driver is trying to seek the virtual
> drive
> when you want to open it but driver successfully detect that there is no
> media in drive
> and then it's asking user to insert floppy media in the drive.
>
> I also tested behavior of this patch if you start guest with "-nodefaults"
> and both
> Windows and Linux guests detect only FDC but no drive.
>
> Pavel
>
> This patch has been written with help of specifications from:
> http://www.ousob.com/ng/hardware/ngd127.php
> http://www.isdaman.com/alsos/hardware/fdc/floppy.htm
> http://wiki.osdev.org/Floppy_Disk_Controller
>
> Signed-off-by: Pavel Hrdina
> Signed-off-by: Michal Novotny
> ---
Please see http://wiki.qemu.org/Contribute/SubmitAPatch for hints on
writing a commit message for a patch: Subject should have a prefix, such
as "fdc: Fix emulation for no media" and the body should not contain
personal letter-style contents such as "Hi, this is the patch". Any such
personal comments can go under the --- line where they are not committed
to git history.
Cc'ing floppy author and block maintainer.
Andreas
> hw/fdc.c | 14 ++
> hw/pc.c |3 ++-
> 2 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/hw/fdc.c b/hw/fdc.c
> index a0236b7..6791eff 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 media 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,11 +203,12 @@ static void fd_revalidate(FDrive *drv)
> drv->drive = drive;
> drv->media_rate = rate;
> } else {
> -FLOPPY_DPRINTF("No disk in drive\n");
> +FLOPPY_DPRINTF("Drive disabled\n");
> drv->last_sect = 0;
> drv->max_track = 0;
> drv->flags &= ~FDISK_DBL_SIDES;
> }
> +
> }
>
> //
> @@ -937,6 +940,9 @@ static int fdctrl_media_changed(FDrive *drv)
>
> if (!drv->bs)
> return 0;
> +/* This is needed for driver to detect there is no media in drive */
> +if (!bdrv_is_inserted(drv->bs))
> +return 1;
> if (drv->media_changed) {
> drv->media_changed = 0;
> ret = 1;
> diff --git a/hw/pc.c b/hw/pc.c
> index 1f5aacb..29a604b 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -382,7 +382,8 @@ 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 there is no media in a drive, we still have the drive
> present */
> +if (fd[i]) {
> bdrv_get_floppy_geometry_hint(fd[i], &nb_heads, &max_track,
>&last_sect, FDRIVE_DRV_NONE,
>&fd_type[i], &rate);
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH] Fix IDE FDC emulation for no media
On 04/24/2012 08:55 AM, Paolo Bonzini wrote:
Il 23/04/2012 18:06, Pavel Hrdina ha scritto:
Hi,
this is the patch to fix incorrect handling of IDE floppy drive controller
emulation
when no media is present. If the guest is booted without a media then the drive
was not being emulated at all but this patch enables the emulation with no
media present.
There was a bug in FDC emulation without media. Driver was not able to
recognize that
there is no media in drive.
This has been tested on both Fedora-16 x86_64 VM and Windows XP VM and the
behaviour
is as expected, i.e. as follows:
Linux guest (Fedora 16 x86_64) tries "mount /dev/fd0" and exit with error
"mount: /dev/fd0 is not a valid block device" which is the same behavior like
bare metal with real floppy device (you have to load floppy driver at first
using e.g. "modprobe floppy" command).
For Windows XP guest the Windows floppy driver is trying to seek the virtual
drive
when you want to open it but driver successfully detect that there is no media
in drive
and then it's asking user to insert floppy media in the drive.
I also tested behavior of this patch if you start guest with "-nodefaults" and
both
Windows and Linux guests detect only FDC but no drive.
Pavel
This patch has been written with help of specifications from:
http://www.ousob.com/ng/hardware/ngd127.php
http://www.isdaman.com/alsos/hardware/fdc/floppy.htm
http://wiki.osdev.org/Floppy_Disk_Controller
Signed-off-by: Pavel Hrdina
Signed-off-by: Michal Novotny
---
hw/fdc.c | 14 ++
hw/pc.c |3 ++-
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/hw/fdc.c b/hw/fdc.c
index a0236b7..6791eff 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 media 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,11 +203,12 @@ static void fd_revalidate(FDrive *drv)
drv->drive = drive;
drv->media_rate = rate;
} else {
-FLOPPY_DPRINTF("No disk in drive\n");
+FLOPPY_DPRINTF("Drive disabled\n");
drv->last_sect = 0;
drv->max_track = 0;
drv->flags&= ~FDISK_DBL_SIDES;
}
+
}
//
@@ -937,6 +940,9 @@ static int fdctrl_media_changed(FDrive *drv)
if (!drv->bs)
return 0;
+/* This is needed for driver to detect there is no media in drive */
+if (!bdrv_is_inserted(drv->bs))
+return 1;
if (drv->media_changed) {
drv->media_changed = 0;
ret = 1;
diff --git a/hw/pc.c b/hw/pc.c
index 1f5aacb..29a604b 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -382,7 +382,8 @@ 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 there is no media in a drive, we still have the drive
present */
+if (fd[i]) {
bdrv_get_floppy_geometry_hint(fd[i],&nb_heads,&max_track,
&last_sect, FDRIVE_DRV_NONE,
&fd_type[i],&rate);
Strictly speaking this final hunk should not be necessary: the fd_type
is by default FDRIVE_DRV_NONE and it is correct if there is no medium in
the drive. However, it does not hurt.
The rest of the patch looks good. It's strictly a bugfix and doesn't
change the hardware exposed to the guest (only the media), so
I think this does not require a compatibility property.
Reviewed-by: Paolo Bonzini
Paolo
Hi,
there is bug that you cannot see any floppy drive in guest if you use
defaults or set up a floppy drive without media. On bare-metal you can
see floppy drive even without media.
For qemu you can check that there is floppy drive present by using qemu
monitor and "info block". After you inserted media to the floppy drive
on running guest, you still cannot see any floppy drive in guest.
This patch will set floppy drive to default values if you start guest
with floppy drive without media and after you insert proper media to
floppy drive you can access it from
Re: [Qemu-devel] [PATCH] Fix IDE FDC emulation for no media
Il 23/04/2012 18:06, Pavel Hrdina ha scritto:
> Hi,
> this is the patch to fix incorrect handling of IDE floppy drive controller
> emulation
> when no media is present. If the guest is booted without a media then the
> drive
> was not being emulated at all but this patch enables the emulation with no
> media present.
>
> There was a bug in FDC emulation without media. Driver was not able to
> recognize that
> there is no media in drive.
>
> This has been tested on both Fedora-16 x86_64 VM and Windows XP VM and the
> behaviour
> is as expected, i.e. as follows:
>
> Linux guest (Fedora 16 x86_64) tries "mount /dev/fd0" and exit with error
> "mount: /dev/fd0 is not a valid block device" which is the same behavior like
> bare metal with real floppy device (you have to load floppy driver at first
> using e.g. "modprobe floppy" command).
>
> For Windows XP guest the Windows floppy driver is trying to seek the virtual
> drive
> when you want to open it but driver successfully detect that there is no
> media in drive
> and then it's asking user to insert floppy media in the drive.
>
> I also tested behavior of this patch if you start guest with "-nodefaults"
> and both
> Windows and Linux guests detect only FDC but no drive.
>
> Pavel
>
> This patch has been written with help of specifications from:
> http://www.ousob.com/ng/hardware/ngd127.php
> http://www.isdaman.com/alsos/hardware/fdc/floppy.htm
> http://wiki.osdev.org/Floppy_Disk_Controller
>
> Signed-off-by: Pavel Hrdina
> Signed-off-by: Michal Novotny
> ---
> hw/fdc.c | 14 ++
> hw/pc.c |3 ++-
> 2 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/hw/fdc.c b/hw/fdc.c
> index a0236b7..6791eff 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 media 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,11 +203,12 @@ static void fd_revalidate(FDrive *drv)
> drv->drive = drive;
> drv->media_rate = rate;
> } else {
> -FLOPPY_DPRINTF("No disk in drive\n");
> +FLOPPY_DPRINTF("Drive disabled\n");
> drv->last_sect = 0;
> drv->max_track = 0;
> drv->flags &= ~FDISK_DBL_SIDES;
> }
> +
> }
>
> //
> @@ -937,6 +940,9 @@ static int fdctrl_media_changed(FDrive *drv)
>
> if (!drv->bs)
> return 0;
> +/* This is needed for driver to detect there is no media in drive */
> +if (!bdrv_is_inserted(drv->bs))
> +return 1;
> if (drv->media_changed) {
> drv->media_changed = 0;
> ret = 1;
> diff --git a/hw/pc.c b/hw/pc.c
> index 1f5aacb..29a604b 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -382,7 +382,8 @@ 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 there is no media in a drive, we still have the drive
> present */
> +if (fd[i]) {
> bdrv_get_floppy_geometry_hint(fd[i], &nb_heads, &max_track,
>&last_sect, FDRIVE_DRV_NONE,
>&fd_type[i], &rate);
Strictly speaking this final hunk should not be necessary: the fd_type
is by default FDRIVE_DRV_NONE and it is correct if there is no medium in
the drive. However, it does not hurt.
The rest of the patch looks good. It's strictly a bugfix and doesn't
change the hardware exposed to the guest (only the media), so
I think this does not require a compatibility property.
Reviewed-by: Paolo Bonzini
Paolo
[Qemu-devel] [PATCH] Fix IDE FDC emulation for no media
Hi,
this is the patch to fix incorrect handling of IDE floppy drive controller
emulation
when no media is present. If the guest is booted without a media then the drive
was not being emulated at all but this patch enables the emulation with no
media present.
There was a bug in FDC emulation without media. Driver was not able to
recognize that
there is no media in drive.
This has been tested on both Fedora-16 x86_64 VM and Windows XP VM and the
behaviour
is as expected, i.e. as follows:
Linux guest (Fedora 16 x86_64) tries "mount /dev/fd0" and exit with error
"mount: /dev/fd0 is not a valid block device" which is the same behavior like
bare metal with real floppy device (you have to load floppy driver at first
using e.g. "modprobe floppy" command).
For Windows XP guest the Windows floppy driver is trying to seek the virtual
drive
when you want to open it but driver successfully detect that there is no media
in drive
and then it's asking user to insert floppy media in the drive.
I also tested behavior of this patch if you start guest with "-nodefaults" and
both
Windows and Linux guests detect only FDC but no drive.
Pavel
This patch has been written with help of specifications from:
http://www.ousob.com/ng/hardware/ngd127.php
http://www.isdaman.com/alsos/hardware/fdc/floppy.htm
http://wiki.osdev.org/Floppy_Disk_Controller
Signed-off-by: Pavel Hrdina
Signed-off-by: Michal Novotny
---
hw/fdc.c | 14 ++
hw/pc.c |3 ++-
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/hw/fdc.c b/hw/fdc.c
index a0236b7..6791eff 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 media 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,11 +203,12 @@ static void fd_revalidate(FDrive *drv)
drv->drive = drive;
drv->media_rate = rate;
} else {
-FLOPPY_DPRINTF("No disk in drive\n");
+FLOPPY_DPRINTF("Drive disabled\n");
drv->last_sect = 0;
drv->max_track = 0;
drv->flags &= ~FDISK_DBL_SIDES;
}
+
}
//
@@ -937,6 +940,9 @@ static int fdctrl_media_changed(FDrive *drv)
if (!drv->bs)
return 0;
+/* This is needed for driver to detect there is no media in drive */
+if (!bdrv_is_inserted(drv->bs))
+return 1;
if (drv->media_changed) {
drv->media_changed = 0;
ret = 1;
diff --git a/hw/pc.c b/hw/pc.c
index 1f5aacb..29a604b 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -382,7 +382,8 @@ 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 there is no media in a drive, we still have the drive
present */
+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
