From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: "Philippe Mathieu-Daudé" <philmd@redhat.com>
Cc: Alex <coderain@sdf.org>,
Nikolay Nikolov <nickysn@users.sourceforge.net>,
seabios@seabios.org, John Snow <jsnow@redhat.com>,
QEMU Developers <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] [SeaBIOS] Re: Regression with floppy drive controller
Date: Tue, 20 Aug 2019 16:04:43 +0100 [thread overview]
Message-ID: <20190820150443.GM2867@work-vm> (raw)
In-Reply-To: <70fd81a1-08bb-5cc8-616c-68ec2a7472e2@redhat.com>
* Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
> On 8/20/19 4:36 PM, Dr. David Alan Gilbert wrote:
> > * Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
> >> On 8/20/19 3:38 PM, Philippe Mathieu-Daudé wrote:
> >>> On 8/20/19 3:12 PM, John Snow wrote:
> >>>> On 8/20/19 6:25 AM, Philippe Mathieu-Daudé wrote:
> >>>>> [cross posting QEMU & SeaBIOS]
> >>>>>
> >>>>> Hello,
> >>>>>
> >>>>> I'v been looking at a QEMU bug report [1] which bisection resulted in a
> >>>>> SeaBIOS commit:
> >>>>>
> >>>>> 4a6dbcea3e412fe12effa2f812f50dd7eae90955 is the first bad commit
> >>>>> commit 4a6dbcea3e412fe12effa2f812f50dd7eae90955
> >>>>> Author: Nikolay Nikolov <nickysn@users.sourceforge.net>
> >>>>> Date: Sun Feb 4 17:27:01 2018 +0200
> >>>>>
> >>>>> floppy: Use timer_check() in floppy_wait_irq()
> >>>>>
> >>>>> Use timer_check() instead of using floppy_motor_counter in BDA for the
> >>>>> timeout check in floppy_wait_irq().
> >>>>>
> >>>>> The problem with using floppy_motor_counter was that, after it reaches
> >>>>> 0, it immediately stops the floppy motors, which is not what is
> >>>>> supposed to happen on real hardware. Instead, after a timeout (like in
> >>>>> the end of every floppy operation, regardless of the result - success,
> >>>>> timeout or error), the floppy motors must be kept spinning for
> >>>>> additional 2 seconds (the FLOPPY_MOTOR_TICKS). So, now the
> >>>>> floppy_motor_counter is initialized to 255 (the max value) in the
> >>>>> beginning of the floppy operation. For IRQ timeouts, a different
> >>>>> timeout is used, specified by the new FLOPPY_IRQ_TIMEOUT constant
> >>>>> (currently set to 5 seconds - a fairly conservative value, but should
> >>>>> work reliably on most floppies).
> >>>>>
> >>>>> After the floppy operation, floppy_drive_pio() resets the
> >>>>> floppy_motor_counter to 2 seconds (FLOPPY_MOTOR_TICKS).
> >>>>>
> >>>>> This is also consistent with what other PC BIOSes do.
> >>>>>
> >>>>>
> >>>>> This commit improve behavior with real hardware, so maybe QEMU is not
> >>>>> modelling something or modelling it incorrectly?
> [...]
> >> Looking at the fdc timer I noticed it use a static '50 ns' magic value.
> >
> > That's not 50ns
> >
> >> Increasing this value allows the floppy image to boot again, using this
> >> snippet:
> >>
> >> -- >8 --
> >> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
> >> index 9b24cb9b85..5fc54073fd 100644
> >> --- a/hw/block/fdc.c
> >> +++ b/hw/block/fdc.c
> >> @@ -2134,7 +2134,7 @@ static void fdctrl_handle_readid(FDCtrl *fdctrl,
> >> int direction)
> >>
> >> cur_drv->head = (fdctrl->fifo[1] >> 2) & 1;
> >> timer_mod(fdctrl->result_timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
> >> - (NANOSECONDS_PER_SECOND / 50));
> >
> > That's 1/50th of a second in ns.
>
> Just noticed that too, so we have here 20ms.
>
> >> + (NANOSECONDS_PER_SECOND / 5000));
> >
> > I'm not too sure about readid; but assuming we're rotating at 360rpm,
> > that's 6 revolutions/second, and 18 sectors/track = 108 sectors/second
> > (half of that for a double density disk).
> >
> > So, the wait for a sector to spin around and read feels like it should
> > be in the region of 1/108 of a second + some latency - so 1/50th of a
> > second would seem to be in the ballpark or being right, where as 1/5000
> > of a second is way too fast for a poor old floppy.
>
> The first command sent is READ_ID.
>
> Reading the Intel 82077AA datasheet:
>
> The READ ID command is used to find the present
> position of the recording heads. The 82077AA
> stores the values from the first ID Field it is able to
> read into its registers. If the 82077AA does not find
> an ID Address Mark on the diskette after the second
> occurrence of a pulse on the IDX pin, it then sets the
> IC code in Status Register 0 to ‘‘01’’ (Abnormal ter-
> mination), sets the MA bit in Status Register 1 to
> ‘’1’’, and terminates the command.
>
> Then later the SPECIFICATIONS table:
>
> nRD/nWR Pulse Width: min 90ns
> INDEX Pulse Width: min 5 'Internal Clock Period'
Note that's the pulse width, not the gap between the idx pulses.
My understanding is that an index pulse is once per rotation; ie. every
1/60th of a second.
The failure after 2 IDX pin pulses makes sense, that's saying if you've
not found a sector after spinning the disk twice then you fail.
So, your time to deliver a good result to a readid shoukd be the
rotational time for 1 or 2 sectors, where as the time to fail should
be the rotational time for about 2 whole rotations (ie 1/30 of a
second).
Dave
> The nominal values for the 'internal clock period' for the various
> data rates are:
>
> 1 Mbps: 3 * osc period = 125ns
> 500 Kbps: 6 * osc period = 250ns
> 300 Kbps: 10 * osc period = 420ns
> 250 Kbps: 12 * osc period = 500ns
>
> IIUC the model we have DATARATE SELECT REGISTER (DSR) = 0
>
> So DRATESEL=0 => datarate = 500 Kbps
>
> So we should wait at least 250ns.
>
> Trying the following snippet it also works:
>
> -- >8 --
> @@ -2133,8 +2133,8 @@ static void fdctrl_handle_readid(FDCtrl *fdctrl,
> int direction)
> FDrive *cur_drv = get_cur_drv(fdctrl);
>
> cur_drv->head = (fdctrl->fifo[1] >> 2) & 1;
> - timer_mod(fdctrl->result_timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
> - (NANOSECONDS_PER_SECOND / 50));
> + timer_mod(fdctrl->result_timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)
> + + 250);
> }
> ---
>
> Note this is not the spining-up delay on reset:
>
> Before data can be transferred to or from the disk-
> ette, the disk drive motor must be brought up to
> speed. For most 3(/2 × disk drives, the spin-up time is
> 300 ms, while the 5(/4 × drive usually requires about
> 500 ms due to the increased moment of inertia asso-
> ciated with the larger diameter diskette.
>
> This looks more closer to the 20ms order. So maybe what we miss
> here is a RESET delay (of 500ms?) previous to the READ_ID?
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
next prev parent reply other threads:[~2019-08-20 15:06 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-20 10:25 [Qemu-devel] Regression with floppy drive controller Philippe Mathieu-Daudé
2019-08-20 13:12 ` John Snow
2019-08-20 13:38 ` Philippe Mathieu-Daudé
2019-08-20 14:00 ` Philippe Mathieu-Daudé
2019-08-20 14:36 ` [Qemu-devel] [SeaBIOS] " Dr. David Alan Gilbert
2019-08-20 14:54 ` Philippe Mathieu-Daudé
2019-08-20 15:02 ` Philippe Mathieu-Daudé
2019-08-20 15:04 ` Dr. David Alan Gilbert [this message]
2019-08-20 16:21 ` [Qemu-devel] " Philippe Mathieu-Daudé
2019-08-20 20:37 ` Eduardo Habkost
2019-08-21 6:42 ` Gerd Hoffmann
2019-08-21 7:45 ` Paolo Bonzini
2019-08-21 13:31 ` [Qemu-devel] [SeaBIOS] " Kevin O'Connor
2019-08-22 8:32 ` Gerd Hoffmann
2019-08-22 8:42 ` Gerd Hoffmann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190820150443.GM2867@work-vm \
--to=dgilbert@redhat.com \
--cc=coderain@sdf.org \
--cc=jsnow@redhat.com \
--cc=nickysn@users.sourceforge.net \
--cc=philmd@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=seabios@seabios.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).