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<phrd...@redhat.com> >>>>> Signed-off-by: Michal Novotny<minov...@redhat.com> >>>> 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 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 floppy drives set it on the cable. >> >> Does anyone have some pointer to a spec for this? > This is proper behaviour, because driver will read DIR register to > detect if there was media change. If it was, driver will run whatever he > was doing with saved state, that there was media changed and check DIR > again. If there is still bit7 setted to true, driver will decide that > there is really no media in drive and end with this "error". So I was pointed to a spec that actually says something about DSKCHG: "This signal is active at power-on and latched inactive when a diskette is present, the drive is selected and a step pulse occurs. This signal goes active when the diskette is removed from the drive. The presence of a diskette is determined by a sensor." I guess we should change the logic of our emulation to match this. Kevin
