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: 53709 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-la0-f72.google.com (mail-la0-f72.google.com [209.85.215.72]) by patches.linaro.org (Postfix) with ESMTPS id C3EFE22DE5 for ; Wed, 16 Sep 2015 02:18:20 +0000 (UTC) Received: by lamf6 with SMTP id f6sf32606828lam.1 for ; Tue, 15 Sep 2015 19:18:19 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:delivered-to:from:to:date :message-id:cc:subject:precedence:list-id:list-unsubscribe :list-archive:list-post:list-help:list-subscribe:errors-to:sender :x-original-sender:x-original-authentication-results:mailing-list; bh=1RpXESdQ/FIJNWMvtl+bQdT0fFTQOdUzl+n24lEoyHc=; b=CRKNQILUSa8UuvGFI4in2gbAnQVkc0XO/+RDuzIqFB3tLSj+HF7rENqzQRctXpUpvd qGKR1zelTdCIiK41Qcig/LNagj2JsNBbhSO4cEBIV9Csmw++9uBDAv3QUWoyvXaSGwSL ntWCoKapXjTGPZ1dEx2PqMXZijPjc5L994pMeBFXHUA0B/kiJGb/tM6xWIRoISyYMOYY eEtKvaOUlVWMsuh133zlu2kxHgFhEvzAduYy9W7khcS7GabOpBfqJKG4ft8OEKbcTMYp Xixn2Q6VSsPea0OxggPUJpCoSEAWr8gmUJOAHF4JT/3MM/TExz6s+IFi/7HuUSr5bPtD Yq2w== X-Gm-Message-State: ALoCoQn+e7t7L9f3wJYHII6N2Yt2tA/rZ5wuRk2IG1AFz80XEpa/b+JJGHY5qxJmbZ8vEQmY3h8k X-Received: by 10.112.48.41 with SMTP id i9mr4891562lbn.23.1442369899768; Tue, 15 Sep 2015 19:18:19 -0700 (PDT) MIME-Version: 1.0 X-BeenThere: patchwork-forward@linaro.org Received: by 10.152.20.229 with SMTP id q5ls56622lae.17.gmail; Tue, 15 Sep 2015 19:18:19 -0700 (PDT) X-Received: by 10.112.138.170 with SMTP id qr10mr26145247lbb.14.1442369899582; Tue, 15 Sep 2015 19:18:19 -0700 (PDT) Received: from mail-lb0-f178.google.com (mail-lb0-f178.google.com. [209.85.217.178]) by mx.google.com with ESMTPS id wq1si16548760lac.103.2015.09.15.19.18.19 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 15 Sep 2015 19:18:19 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.217.178 as permitted sender) client-ip=209.85.217.178; Received: by lbcao8 with SMTP id ao8so95468516lbc.3 for ; Tue, 15 Sep 2015 19:18:19 -0700 (PDT) X-Received: by 10.152.236.12 with SMTP id uq12mr20309884lac.35.1442369899467; Tue, 15 Sep 2015 19:18:19 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.112.59.35 with SMTP id w3csp2157159lbq; Tue, 15 Sep 2015 19:18:18 -0700 (PDT) X-Received: by 10.107.128.145 with SMTP id k17mr43349230ioi.24.1442369898487; Tue, 15 Sep 2015 19:18:18 -0700 (PDT) Received: from lists.gnu.org (lists.gnu.org. [2001:4830:134:3::11]) by mx.google.com with ESMTPS id c16si14664745igo.8.2015.09.15.19.18.18 for (version=TLSv1 cipher=RC4-SHA bits=128/128); Tue, 15 Sep 2015 19:18:18 -0700 (PDT) Received-SPF: pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 2001:4830:134:3::11 as permitted sender) client-ip=2001:4830:134:3::11; Received: from localhost ([::1]:46919 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zc2Ib-0005zO-Al for patch@linaro.org; Tue, 15 Sep 2015 22:18:17 -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+patch=linaro.org@nongnu.org Sender: qemu-devel-bounces+patch=linaro.org@nongnu.org X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: lersek@redhat.com X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.217.178 as permitted sender) smtp.mailfrom=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 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)) {