[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] Regression with floppy drive controller
From: |
Eduardo Habkost |
Subject: |
Re: [Qemu-devel] Regression with floppy drive controller |
Date: |
Tue, 20 Aug 2019 17:37:57 -0300 |
On Tue, Aug 20, 2019 at 06:21:28PM +0200, Philippe Mathieu-Daudé wrote:
> Cc'ing Eduardo, Paolo.
>
> 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 <address@hidden>
> >>> 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?
> > [...]
> >>
> >> Well, that's unfortunate.
> >>
> >> What version of QEMU shipped the SeaBIOS that caused the regression?
> >
> > See https://bugs.launchpad.net/qemu/+bug/1840719/comments/3
> >
> > QEMU commit 0b8f74488e, slighly before QEMU v3.1.0
> > (previous tag is v3.0.0).
> >
> > But you can use v4.1.0 too, simply change the SeaBIOS bios.bin, i.e.:
> >
> > qemu$ git checkout v4.1.0
> >
> > qemu$ (cd roms/seabios && git checkout 4a6dbcea3e4~) && \
> > make -C roms bios
> >
> > Now pc-bios/bios.bin is built using the last commit working,
> >
> > qemu$ (cd roms/seabios && git checkout 4a6dbcea3e4) && \
> > make -C roms bios
> >
> > And you can reproduce the error.
>
> Back from here.
>
> So the SeaBIOS patch is:
>
> diff --git a/src/hw/floppy.c b/src/hw/floppy.c
> index 77dbade..3012b3a 100644
> --- a/src/hw/floppy.c
> +++ b/src/hw/floppy.c
> @@ -34,6 +34,7 @@
> #define FLOPPY_GAPLEN 0x1B
> #define FLOPPY_FORMAT_GAPLEN 0x6c
> #define FLOPPY_PIO_TIMEOUT 1000
> +#define FLOPPY_IRQ_TIMEOUT 5000
>
> #define FLOPPY_DOR_MOTOR_D 0x80 // Set to turn drive 3's motor ON
> #define FLOPPY_DOR_MOTOR_C 0x40 // Set to turn drive 2's motor ON
> @@ -221,8 +222,9 @@ floppy_wait_irq(void)
> {
> u8 frs = GET_BDA(floppy_recalibration_status);
> SET_BDA(floppy_recalibration_status, frs & ~FRS_IRQ);
> + u32 end = timer_calc(FLOPPY_IRQ_TIMEOUT);
> for (;;) {
> - if (!GET_BDA(floppy_motor_counter)) {
> + if (timer_check(end)) {
> warn_timeout();
> floppy_disable_controller();
> return DISK_RET_ETIMEOUT;
>
> timer_calc() unit is milliseconds, so this patch should wait upto
> 5seconds before failing, and it seems the timeout is not used at all.
>
> SeaBIOS timer.c:
>
> // Return the TSC value that is 'msecs' time in the future.
> u32
> timer_calc(u32 msecs)
> {
> return timer_read() + (GET_GLOBAL(TimerKHz) * msecs);
> }
>
> static u32
> timer_read(void)
> {
> u16 port = GET_GLOBAL(TimerPort);
> if (CONFIG_TSC_TIMER && !port)
> // Read from CPU TSC
> return rdtscll() >> GET_GLOBAL(ShiftTSC);
> if (CONFIG_PMTIMER && port != PORT_PIT_COUNTER0)
> // Read from PMTIMER
> return timer_adjust_bits(inl(port), 0xffffff);
> // Read from PIT.
> outb(PM_SEL_READBACK | PM_READ_VALUE | PM_READ_COUNTER0, PORT_PIT_MODE);
> u16 v = inb(PORT_PIT_COUNTER0) | (inb(PORT_PIT_COUNTER0) << 8);
> return timer_adjust_bits(v, 0xffff);
> }
>
> Using the default QEMU config, we build SeaBIOS to use the TSC timer:
>
> builds/seabios-128k/.config:CONFIG_TSC_TIMER=y
> builds/seabios-256k/.config:CONFIG_TSC_TIMER=y
>
> $ qemu-system-i386 -M isapc -cpu 486 \
> -fda Windows\ 98\ Second\ Edition\ Boot.img \
> -chardev stdio,id=seabios \
> -device isa-debugcon,iobase=0x402,chardev=seabios
> Booting from Floppy...
> Floppy_drive_recal 0
> Floppy_enable_controller
> WARNING - Timeout at floppy_wait_irq:228!
> Floppy_disable_controller
> Floppy_enable_controller
> WARNING - Timeout at floppy_wait_irq:228!
> Floppy_disable_controller
> Boot failed: could not read the boot disk
>
> Now enabling the TSC feature:
>
> $ qemu-system-i386 -M isapc -cpu 486,tsc \
> -fda Windows\ 98\ Second\ Edition\ Boot.img \
> -chardev stdio,id=seabios \
> -device isa-debugcon,iobase=0x402,chardev=seabios
> Booting from Floppy...
> Floppy_drive_recal 0
> Floppy_enable_controller
> Floppy_media_sense on drive 0 found rate 0
> Booting from 0000:7c00
> Floppy_disable_controller
> Floppy_enable_controller
> Floppy_drive_recal 0
> Floppy_media_sense on drive 0 found rate 0
>
> Do we need a cpu with TSC support to run SeaBIOS?
>
> So we should use '-cpu Conroe' or '-cpu core2duo' minimum?
It's probably about time we update qemu64 (the default CPU model)
to provide a more modern set of features. Once libvirt adapts to
the CPU model alias/version interface we added in 4.1, this will
become easier to do.
--
Eduardo
- [Qemu-devel] Regression with floppy drive controller, Philippe Mathieu-Daudé, 2019/08/20
- Re: [Qemu-devel] Regression with floppy drive controller, John Snow, 2019/08/20
- Re: [Qemu-devel] Regression with floppy drive controller, Philippe Mathieu-Daudé, 2019/08/20
- Re: [Qemu-devel] Regression with floppy drive controller, Philippe Mathieu-Daudé, 2019/08/20
- Re: [Qemu-devel] [SeaBIOS] Re: Regression with floppy drive controller, Dr. David Alan Gilbert, 2019/08/20
- Re: [Qemu-devel] [SeaBIOS] Re: Regression with floppy drive controller, Philippe Mathieu-Daudé, 2019/08/20
- Re: [Qemu-devel] [SeaBIOS] Re: Regression with floppy drive controller, Philippe Mathieu-Daudé, 2019/08/20
- Re: [Qemu-devel] [SeaBIOS] Re: Regression with floppy drive controller, Dr. David Alan Gilbert, 2019/08/20
- Re: [Qemu-devel] Regression with floppy drive controller, Philippe Mathieu-Daudé, 2019/08/20
- Re: [Qemu-devel] Regression with floppy drive controller,
Eduardo Habkost <=
- Re: [Qemu-devel] Regression with floppy drive controller, Gerd Hoffmann, 2019/08/21
- Re: [Qemu-devel] Regression with floppy drive controller, Paolo Bonzini, 2019/08/21
- Re: [Qemu-devel] [SeaBIOS] Re: Regression with floppy drive controller, Kevin O'Connor, 2019/08/21
- Re: [Qemu-devel] [SeaBIOS] Re: Regression with floppy drive controller, Gerd Hoffmann, 2019/08/22
- Re: [Qemu-devel] [SeaBIOS] Re: Regression with floppy drive controller, Gerd Hoffmann, 2019/08/22