From patchwork Wed Sep 16 02:17:26 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laszlo Ersek X-Patchwork-Id: 518244 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 03B6A140187 for ; Wed, 16 Sep 2015 12:18:06 +1000 (AEST) Received: from localhost ([::1]:46919 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zc2IN-0005zO-PI for incoming@patchwork.ozlabs.org; Tue, 15 Sep 2015 22:18:03 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60414) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zc2Hz-0005OE-ER for qemu-devel@nongnu.org; Tue, 15 Sep 2015 22:17:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zc2Hy-0002WX-CX for qemu-devel@nongnu.org; Tue, 15 Sep 2015 22:17:39 -0400 Received: from mx1.redhat.com ([209.132.183.28]:48045) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zc2Hs-0002UL-V6; Tue, 15 Sep 2015 22:17:33 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (Postfix) with ESMTPS id D819E8AE6C; Wed, 16 Sep 2015 02:17:31 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-31.rdu2.redhat.com [10.10.116.31]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t8G2HTxf018720; Tue, 15 Sep 2015 22:17:30 -0400 From: Laszlo Ersek To: qemu-devel@nongnu.org Date: Wed, 16 Sep 2015 04:17:26 +0200 Message-Id: <1442369846-31890-1-git-send-email-lersek@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: John Snow , qemu-block@nongnu.org Subject: [Qemu-devel] [PATCH] hw/ide/ahci: advance IO buffer offset in multi-sector PIO transfer X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org The "MdeModulePkg/Bus/Ata/AtaAtapiPassThru" driver in edk2 submits a three sector long PIO read, when booting off various Fedora installer ISOs in UEFI mode. With DEBUG_IDE, DEBUG_IDE_ATAPI, DEBUG_AIO and DEBUG_AHCI enabled, plus a DPRINTF(ad->port_no, "offset=%d\n", offset); at the beginning of ahci_populate_sglist(), we get the following debug output: > fis: > 00:27 80 a0 00 00 fe ff e0 00 00 00 00 00 00 00 00 > 10:00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 20:00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 30:00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 40:28 00 00 00 00 38 00 00 03 00 00 00 00 00 00 00 > 50:00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 60:00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 70:00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > fis: > 00:28 00 00 00 00 38 00 00 03 00 00 00 00 00 00 00 > ide: CMD=a0 > ATAPI limit=0xfffe packet: 28 00 00 00 00 38 00 00 03 00 00 00 > read pio: LBA=56 nb_sectors=3 > reply: tx_size=6144 elem_tx_size=0 index=2048 > byte_count_limit=65534 > ahci: ahci_populate_sglist: [0] offset=0 > ahci: ahci_dma_prepare_buf: [0] len=0x800 > ahci: ahci_start_transfer: [0] reading 2048 bytes on atapi w/ sglist > reply: tx_size=4096 elem_tx_size=4096 index=2048 > ahci: ahci_populate_sglist: [0] offset=0 > ahci: ahci_dma_prepare_buf: [0] len=0x800 > ahci: ahci_start_transfer: [0] reading 2048 bytes on atapi w/ sglist > reply: tx_size=2048 elem_tx_size=2048 index=2048 > ahci: ahci_populate_sglist: [0] offset=0 > ahci: ahci_dma_prepare_buf: [0] len=0x800 > ahci: ahci_start_transfer: [0] reading 2048 bytes on atapi w/ sglist > reply: tx_size=0 elem_tx_size=0 index=2048 > ahci: ahci_cmd_done: [0] cmd done > [...] The following functions play recursive ping-pong, because ide_atapi_cmd_reply_end() segments the request into individual 2KB sectors: ide_transfer_start() <-----------------------+ ahci_start_transfer() via funcptr | | ahci_dma_prepare_buf() | ahci_populate_sglist() | | dma_buf_read() | | ahci_commit_buf() | | ide_atapi_cmd_reply_end() via funcptr | ide_transfer_start() ------------------+ The ahci_populate_sglist() correctly sets up the scatter-gather list for dma_buf_read(), based on the Physical Region Descriptors passed in by the guest. However, the offset into that scatter-gather list remains constant zero as ide_atapi_cmd_reply_end() wades through every sector of the three sector long PIO transfer. The consequence is that the first 2KB of the guest buffer(s), speaking "linearizedly", is repeatedly overwritten with the next CD-ROM sector. At the end of the transfer, the sector last read is visible in the first 2KB of the guest buffer(s), and the rest of the guest buffer(s) remains unwritten. Looking at the DMA request path; especially comparing the context of ahci_commit_buf() between its two callers ahci_dma_rw_buf() and ahci_start_transfer(), it seems like the latter forgets to advance "s->io_buffer_offset". Adding that increment enables the guest to receive valid data. Cc: John Snow Cc: qemu-block@nongnu.org Signed-off-by: Laszlo Ersek Tested-by: Laszlo Ersek --- Notes: I spent the better half of the night on this bug, so please be gentle. :) hw/ide/ahci.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index 44f6e27..b975c9f 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -1291,6 +1291,8 @@ out: /* Update number of transferred bytes, destroy sglist */ ahci_commit_buf(dma, size); + s->io_buffer_offset += size; + s->end_transfer_func(s); if (!(s->status & DRQ_STAT)) {