Re: [Qemu-devel] [PATCH] ahci: Force ICC bits in PxCMD to zero

[John Snow <jsnow@redhat.com>]

On 07/16/2015 03:54 PM, Stefan Fritsch wrote:
> The AHCI spec requires that the HBA sets the ICC bits to zero after the
> ICC change is done. Since we don't do any ICC change, force the bits to
> zero all the time.
> 
> This fixes delays with some OSs (e.g. OpenBSD) waiting for the ICC bits
> to change to 0.
> 
> Signed-off-by: Stefan Fritsch <sf@sfritsch.de>
> ---
>  hw/ide/ahci.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index bb6a92f..48749c1 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -279,8 +279,13 @@ static void  ahci_port_write(AHCIState *s, int port, int offset, uint32_t val)
>              break;
>          case PORT_CMD:
>              /* Block any Read-only fields from being set;
> -             * including LIST_ON and FIS_ON. */
> -            pr->cmd = (pr->cmd & PORT_CMD_RO_MASK) | (val & ~PORT_CMD_RO_MASK);
> +             * including LIST_ON and FIS_ON.
> +             * The spec requires to set ICC bits to zero after the ICC change
> +             * is done. We don't support ICC state changes, therefore always
> +             * force the ICC bits to zero.
> +             */
> +            pr->cmd = (pr->cmd & PORT_CMD_RO_MASK) |
> +                      (val & ~(PORT_CMD_RO_MASK|PORT_CMD_ICC_MASK));
>  
>              /* Check FIS RX and CLB engines, allow transition to false: */
>              ahci_cond_start_engines(&s->dev[port], true);
> 

I guess I'll need to start testing with OpenBSD...

Looks good to me -- (If there's a next time, please also CC qemu-devel
so patch management tools pick this up.)

Reviewed-by: John Snow <jsnow@redhat.com>

and:

Thanks, applied to my IDE tree:

https://github.com/jnsnow/qemu/commits/ide
https://github.com/jnsnow/qemu.git

--js