Re: [Qemu-devel] [PATCH 08/16] ahci: clear error register before NCQ cmd

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1183 bytes --]

On Fri, Jun 19, 2015 at 09:50:39PM -0400, John Snow wrote:
> The legacy ide command execution layer will clear any errors outstanding
> before execution, but the NCQ layer doesn't.
> Even on success, this register will remain clogged.
> 
> Clear it out before each NCQ command so the guest can tell if the
> error code produced after completion is meaningful or not.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  hw/ide/ahci.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index 6bded67..e63ba9b 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -1048,6 +1048,8 @@ static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis,
>              ncq_tfs->lba, ncq_tfs->lba + ncq_tfs->sector_count - 1,
>              ide_state->nb_sectors - 1);
>  
> +    ide_state->error = 0;

I'm not sure it makes sense use ide_state at all in NCQ.

ide_state is per-port and NCQ can issue multiple asynchronous commands
per port.  If process_ncq_command() modifies ide_state, it may do that
while other commands are still pending or about to be processed.

This will clobber ide_state->error.

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]