From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52759) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XSkgD-0005DJ-Pq for qemu-devel@nongnu.org; Sat, 13 Sep 2014 06:35:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XSkg3-0000Rj-6b for qemu-devel@nongnu.org; Sat, 13 Sep 2014 06:35:45 -0400 Received: from mail-wg0-x232.google.com ([2a00:1450:400c:c00::232]:58307) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XSkg2-0000RW-PU for qemu-devel@nongnu.org; Sat, 13 Sep 2014 06:35:34 -0400 Received: by mail-wg0-f50.google.com with SMTP id x13so1773940wgg.33 for ; Sat, 13 Sep 2014 03:35:34 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <54141DF2.9000007@redhat.com> Date: Sat, 13 Sep 2014 12:35:30 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1410580272-19429-1-git-send-email-jsnow@redhat.com> <1410580272-19429-2-git-send-email-jsnow@redhat.com> In-Reply-To: <1410580272-19429-2-git-send-email-jsnow@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 1/1] ide/atapi: Mark non-data commands as complete List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: John Snow , qemu-devel@nongnu.org Cc: kwolf@redhat.com, stefanha@redhat.com Il 13/09/2014 05:51, John Snow ha scritto: > When the command completion code in IDE and AHCI > was unified to put all command completion inside > of a callback, "cmd_done," we neglected to > ensure that all AHCI/ATAPI command paths would > eventually register as finished. for the PCI > interface to IDE this is not a problem because > cmd_done is a nop, but the AHCI implementation > needs to send a D2H_REG_FIS and interrupt back > to the guest to inform of completion. > > This patch adds calls to ide_stop_transfer, > which calls ide_cmd_done, inside of > ide_atapi_cmd_ok and ide_atapi_cmd_error. > > This fixes regressions observed by trying to boot QEMU > with a Fedora 20 live CD under Q35/AHCI, which uses > ATAPI command 0x00, which is a status check that may > cause a hang because we never complete, and ATAPI > command 0x56, which is unsupported by our current > implementation and results in an error that we never > report back to the guest. > > Signed-off-by: John Snow > --- > hw/ide/atapi.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c > index 3d92b52..ee80c22 100644 > --- a/hw/ide/atapi.c > +++ b/hw/ide/atapi.c > @@ -134,6 +134,7 @@ void ide_atapi_cmd_ok(IDEState *s) > s->error = 0; > s->status = READY_STAT | SEEK_STAT; > s->nsector = (s->nsector & ~7) | ATAPI_INT_REASON_IO | ATAPI_INT_REASON_CD; > + ide_transfer_stop(s); > ide_set_irq(s->bus); > } > > @@ -147,6 +148,7 @@ void ide_atapi_cmd_error(IDEState *s, int sense_key, int asc) > s->nsector = (s->nsector & ~7) | ATAPI_INT_REASON_IO | ATAPI_INT_REASON_CD; > s->sense_key = sense_key; > s->asc = asc; > + ide_transfer_stop(s); > ide_set_irq(s->bus); > } > > @@ -174,9 +176,7 @@ void ide_atapi_cmd_reply_end(IDEState *s) > #endif > if (s->packet_transfer_size <= 0) { > /* end of transfer */ > - s->status = READY_STAT | SEEK_STAT; > - s->nsector = (s->nsector & ~7) | ATAPI_INT_REASON_IO | ATAPI_INT_REASON_CD; > - ide_transfer_stop(s); > + ide_atapi_cmd_ok(s); > ide_set_irq(s->bus); > #ifdef DEBUG_IDE_ATAPI > printf("status=0x%x\n", s->status); > @@ -186,7 +186,6 @@ void ide_atapi_cmd_reply_end(IDEState *s) > if (s->lba != -1 && s->io_buffer_index >= s->cd_sector_size) { > ret = cd_read_sector(s, s->lba, s->io_buffer, s->cd_sector_size); > if (ret < 0) { > - ide_transfer_stop(s); > ide_atapi_io_error(s, ret); > return; > } > Reviewed-by: Paolo Bonzini