Re: [Qemu-devel] [PATCH 2/3] ahci: fix PxCI register race

[John Snow <jsnow@redhat.com>]

>From comments on the cover letter, I am adding:

Fixes: https://bugs.launchpad.net/qemu/+bug/1769189

On 05/30/2018 08:43 PM, John Snow wrote:
> AHCI presently signals completion prior to the PxCI register being
> cleared to indicate completion. If a guest driver attempts to issue
> a new command in its IRQ handler, it might be surprised to learn there
> is still a command pending.
> 
> In the case of Windows 10's boot driver, it will actually poll the IRQ
> register hoping to find out when the command is done running -- which
> will never happen, as there isn't a command running.
> 
> Fix this: clear PxCI in ahci_cmd_done and not in the asynchronous BH.
> Because it now runs synchronously, we don't need to check if the command
> is actually done by spying on the ATA registers. We know it's done.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>

and:

CC: qemu-stable <qemu-stable@nongnu.org>
Reported-by: François Guerraz <kubrick@fgv6.net>
Tested-by: Bruce Rogers <brogers@suse.com>

(Stable note: only patch #2 here is strictly required.)

> ---
>  hw/ide/ahci.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index b7a6f68790..a9558e45e7 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -532,13 +532,6 @@ static void ahci_check_cmd_bh(void *opaque)
>      qemu_bh_delete(ad->check_bh);
>      ad->check_bh = NULL;
>  
> -    if ((ad->busy_slot != -1) &&
> -        !(ad->port.ifs[0].status & (BUSY_STAT|DRQ_STAT))) {
> -        /* no longer busy */
> -        ad->port_regs.cmd_issue &= ~(1 << ad->busy_slot);
> -        ad->busy_slot = -1;
> -    }
> -
>      check_cmd(ad->hba, ad->port_no);
>  }
>  
> @@ -1425,6 +1418,12 @@ static void ahci_cmd_done(IDEDMA *dma)
>  
>      trace_ahci_cmd_done(ad->hba, ad->port_no);
>  
> +    /* no longer busy */
> +    if (ad->busy_slot != -1) {
> +        ad->port_regs.cmd_issue &= ~(1 << ad->busy_slot);
> +        ad->busy_slot = -1;
> +    }
> +
>      /* update d2h status */
>      ahci_write_fis_d2h(ad);
>  
>