From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45153) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XSnMJ-00020L-PZ for qemu-devel@nongnu.org; Sat, 13 Sep 2014 09:27:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XSnMA-00014q-Ca for qemu-devel@nongnu.org; Sat, 13 Sep 2014 09:27:23 -0400 Received: from mail-wi0-x234.google.com ([2a00:1450:400c:c05::234]:47332) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XSnMA-00014i-6J for qemu-devel@nongnu.org; Sat, 13 Sep 2014 09:27:14 -0400 Received: by mail-wi0-f180.google.com with SMTP id ex7so2012303wid.1 for ; Sat, 13 Sep 2014 06:27:13 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <5414462E.40406@redhat.com> Date: Sat, 13 Sep 2014 15:27:10 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1410582855-21870-1-git-send-email-jsnow@redhat.com> <1410582855-21870-8-git-send-email-jsnow@redhat.com> In-Reply-To: <1410582855-21870-8-git-send-email-jsnow@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC 07/10] ide/ahci: Reorder error cases in handle_cmd List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: John Snow , qemu-devel@nongnu.org Cc: stefanha@redhat.com, mst@redhat.com 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 > --- > 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