Re: [Qemu-devel] [RFC 07/10] ide/ahci: Reorder error cases in handle_cmd

[Paolo Bonzini]

Il 13/09/2014 06:34, John Snow ha scritto:
> Error checking in ahci's handle_cmd is re-ordered so that we
> initialize as few things as possible before we've done our
> sanity checking. This simplifies returning from this call
> in case of an error.
> 
> A check to make sure the DMA memory map succeeds with the
> correct size is also added, and the debug print of the
> command fis is cleaned up with its size corrected.
> 
> Signed-off-by: John Snow <address@hidden>
> ---
>  hw/ide/ahci.c | 28 +++++++++++++---------------
>  1 file changed, 13 insertions(+), 15 deletions(-)
> 
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index c2fa733..1153ce9 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -951,38 +951,36 @@ static int handle_cmd(AHCIState *s, int port, int slot)
>          return -1;
>      }
>  
> -    cmd = &((AHCICmdHdr *)s->dev[port].lst)[slot];
> -
>      if (!s->dev[port].lst) {
>          DPRINTF(port, "error: lst not given but cmd handled");
>          return -1;
>      }
> -
> +    cmd = &((AHCICmdHdr *)s->dev[port].lst)[slot];
>      /* remember current slot handle for later */
>      s->dev[port].cur_cmd = cmd;
>  
> +    /* The device we are working for */
> +    ide_state = &s->dev[port].port.ifs[0];
> +    if (!ide_state->bs) {
> +        DPRINTF(port, "error: guest accessed unused port");
> +        return -1;
> +    }
> +
>      opts = le32_to_cpu(cmd->opts);
>      tbl_addr = le64_to_cpu(cmd->tbl_addr);
> -
>      cmd_len = 0x80;
>      cmd_fis = dma_memory_map(s->as, tbl_addr, &cmd_len,
>                               DMA_DIRECTION_FROM_DEVICE);
> -
>      if (!cmd_fis) {
>          DPRINTF(port, "error: guest passed us an invalid cmd fis\n");
>          return -1;
> -    }
> -
> -    /* The device we are working for */
> -    ide_state = &s->dev[port].port.ifs[0];
> -
> -    if (!ide_state->bs) {
> -        DPRINTF(port, "error: guest accessed unused port");
> +    } else if (cmd_len != 0x80) {
> +        ahci_trigger_irq(s, &s->dev[port], PORT_IRQ_HBUS_ERR);
> +        DPRINTF(port, "error: dma_memory_map failed (len (%02x) < 0x80)\n",
> +                cmd_len);
>          goto out;
>      }
> -
> -    debug_print_fis(cmd_fis, 0x90);
> -    //debug_print_fis(cmd_fis, (opts & AHCI_CMD_HDR_CMD_FIS_LEN) * 4);
> +    debug_print_fis(cmd_fis, 0x80);
>  
>      switch (cmd_fis[0]) {
>          case SATA_FIS_TYPE_REGISTER_H2D:
> 

Reviewed-by: Paolo Bonzini <address@hidden>