diff mbox

ahci fix: windows boots fine. need to review with kwolf

Message ID 20131028190015.GA1923@redhat.com
State New
Headers show

Commit Message

Michael S. Tsirkin Oct. 28, 2013, 7 p.m. UTC
From: Alexander Graf <agraf@suse.de>

When AHCI executes an asynchronous IDE command, it checked DRDY without
checking either DRQ or BSY.  This sometimes caused interrupt to be sent
before command is actually completed.

This resulted in a race condition: if guest then managed to access the
device before command has completed, it would hang waiting for an
interrupt.
This was observed with windows 7 guests.

To fix, check for DRQ or BSY in additiona to DRDY, if set,
the command is asynchronous so delay the interrupt until
asynchronous done callback is invoked.

Reported-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Tested-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

---
 hw/ide/ahci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Michael S. Tsirkin Oct. 28, 2013, 7:02 p.m. UTC | #1
On Mon, Oct 28, 2013 at 09:00:15PM +0200, Michael S. Tsirkin wrote:
> From: Alexander Graf <agraf@suse.de>
> 
> When AHCI executes an asynchronous IDE command, it checked DRDY without
> checking either DRQ or BSY.  This sometimes caused interrupt to be sent
> before command is actually completed.
> 
> This resulted in a race condition: if guest then managed to access the
> device before command has completed, it would hang waiting for an
> interrupt.
> This was observed with windows 7 guests.
> 
> To fix, check for DRQ or BSY in additiona to DRDY, if set,
> the command is asynchronous so delay the interrupt until
> asynchronous done callback is invoked.
> 
> Reported-by: Michael S. Tsirkin <mst@redhat.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Tested-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Sorry about the subject, forgot to update it.
I reposted with a fixed subject.

> ---
>  hw/ide/ahci.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index a8be62c..fbea9e8 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -961,7 +961,8 @@ static int handle_cmd(AHCIState *s, int port, int slot)
>          /* We're ready to process the command in FIS byte 2. */
>          ide_exec_cmd(&s->dev[port].port, cmd_fis[2]);
>  
> -        if (s->dev[port].port.ifs[0].status & READY_STAT) {
> +        if ((s->dev[port].port.ifs[0].status & (READY_STAT|DRQ_STAT|BUSY_STAT)) ==
> +            READY_STAT) {
>              ahci_write_fis_d2h(&s->dev[port], cmd_fis);
>          }
>      }
> -- 
> MST
diff mbox

Patch

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index a8be62c..fbea9e8 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -961,7 +961,8 @@  static int handle_cmd(AHCIState *s, int port, int slot)
         /* We're ready to process the command in FIS byte 2. */
         ide_exec_cmd(&s->dev[port].port, cmd_fis[2]);
 
-        if (s->dev[port].port.ifs[0].status & READY_STAT) {
+        if ((s->dev[port].port.ifs[0].status & (READY_STAT|DRQ_STAT|BUSY_STAT)) ==
+            READY_STAT) {
             ahci_write_fis_d2h(&s->dev[port], cmd_fis);
         }
     }