qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Gerd Hoffmann <kraxel@redhat.com>, qemu-devel@nongnu.org
Cc: jsnow@redhat.com, f4bug@amsat.org
Subject: Re: [Qemu-devel] regression: sata cdrom boot broken
Date: Wed, 20 Jun 2018 14:18:31 +0200	[thread overview]
Message-ID: <776e542e-b6e1-ccbb-fb9a-ea7d521cd955@redhat.com> (raw)
In-Reply-To: <20180620110741.gdywesuqbyhjs4hd@sirius.home.kraxel.org>

On 20/06/2018 13:07, Gerd Hoffmann wrote:
>   Hi,
> 
> $subject says all.  Noticed while testing the upcoming seabios update.
> Reproducer:
> 
>   qemu-system-x86_64 -M q35 -m 4G -cdrom Fedora-Workstation-Live-x86_64-28-1.1.iso
> 
> bisected to:
> 
>   commit 956556e131e35f387ac482ad7b41151576fef057
>   Author: John Snow <jsnow@redhat.com>
>   Date:   Wed Jun 6 15:09:50 2018 -0400
> 
>     ahci: move PIO Setup FIS before transfer, fix it for ATAPI commands
> 
> cheers,
>   Gerd
> 

If you know where to look at, the spec is actually pretty clear with
respect to when the interrupt is generated: "A PIO Setup FIS has been
received with the ‘I’ bit set, it has been copied into system memory,
and the data related to that FIS has been transferred".

However, after reading the SATA specification I believe that the PIO
Setup FIS should never generate an interrupt in SeaBIOS, because:

  If this is the first DRQ data block for this command, the Interrupt
  bit shall be cleared to zero. If this is not the first DRQ data block
  for this command, the Interrupt bit shall be set to one

Putting things together, there are two bugs in QEMU;

- the PIO Setup interrupt must be generated at the end of data transfer

- the PIO Setup interrupt must not be generated for the ATAPI command
transfer

*But* because SeaBIOS always uses DMA for ATAPI commands, there should
never be more than one DRQ data block for each command, and it should be
possible to remove the fishy PIO Setup FIS handling in SeaBIOS.

Paolo

  reply	other threads:[~2018-06-20 12:18 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-20 11:07 [Qemu-devel] regression: sata cdrom boot broken Gerd Hoffmann
2018-06-20 12:18 ` Paolo Bonzini [this message]
2018-06-20 20:28   ` John Snow
2018-06-21 10:50     ` Paolo Bonzini
2018-06-20 18:15 ` Thomas Huth
2018-06-21 11:22   ` Paolo Bonzini

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=776e542e-b6e1-ccbb-fb9a-ea7d521cd955@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=f4bug@amsat.org \
    --cc=jsnow@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=qemu-devel@nongnu.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).