From nobody Tue Feb 10 19:14:34 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1528312499907192.49792721656002; Wed, 6 Jun 2018 12:14:59 -0700 (PDT) Received: from localhost ([::1]:54161 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fQdtb-0004oz-4E for importer@patchew.org; Wed, 06 Jun 2018 15:14:59 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38398) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fQdop-0000im-DF for qemu-devel@nongnu.org; Wed, 06 Jun 2018 15:10:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fQdoo-0000ks-0w for qemu-devel@nongnu.org; Wed, 06 Jun 2018 15:10:03 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:54580 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fQdoj-0000eF-3i; Wed, 06 Jun 2018 15:09:57 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 436F188844; Wed, 6 Jun 2018 19:09:56 +0000 (UTC) Received: from probe.bos.redhat.com (dhcp-17-177.bos.redhat.com [10.18.17.177]) by smtp.corp.redhat.com (Postfix) with ESMTP id 095EE2026E0E; Wed, 6 Jun 2018 19:09:56 +0000 (UTC) From: John Snow To: qemu-block@nongnu.org, qemu-devel@nongnu.org Date: Wed, 6 Jun 2018 15:09:50 -0400 Message-Id: <20180606190955.20845-3-jsnow@redhat.com> In-Reply-To: <20180606190955.20845-1-jsnow@redhat.com> References: <20180606190955.20845-1-jsnow@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.2]); Wed, 06 Jun 2018 19:09:56 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.2]); Wed, 06 Jun 2018 19:09:56 +0000 (UTC) for IP:'10.11.54.4' DOMAIN:'int-mx04.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'jsnow@redhat.com' RCPT:'' Content-Transfer-Encoding: quoted-printable X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PATCH 2/7] ahci: move PIO Setup FIS before transfer, fix it for ATAPI commands X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Paolo Bonzini , John Snow Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" The PIO Setup FIS is written in the PIO:Entry state, which comes before the ATA and ATAPI data transfer states. As a result, the PIO Setup FIS interrupt is now raised before DMA ends for ATAPI commands, and tests have to be adjusted. This is also hinted by the description of the command header in the AHCI specification, where the "A" bit is described as When =E2=80=981=E2=80=99, indicates that a PIO setup FIS shall be sent = by the device indicating a transfer for the ATAPI command. and also by the description of the ACMD (ATAPI command region): The ATAPI command must be either 12 or 16 bytes in length. The length transmitted by the HBA is determined by the PIO setup FIS that is sent by the device requesting the ATAPI command. QEMU, which conflates the "generator" and the "receiver" of the FIS into one device, always uses ATAPI_PACKET_SIZE, aka 12, for the length. Signed-off-by: Paolo Bonzini Signed-off-by: John Snow --- hw/ide/ahci.c | 18 ++++++------------ tests/libqos/ahci.c | 35 +++++++++++++++++++++-------------- tests/libqos/ahci.h | 3 +-- 3 files changed, 28 insertions(+), 28 deletions(-) diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index 24dbad5125..5871333686 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -1198,7 +1198,6 @@ static void handle_reg_h2d_fis(AHCIState *s, int port, g_free(pretty_fis); } s->dev[port].done_atapi_packet =3D false; - /* XXX send PIO setup FIS */ } =20 ide_state->error =3D 0; @@ -1292,10 +1291,12 @@ static void ahci_start_transfer(IDEDMA *dma) int is_atapi =3D opts & AHCI_CMD_ATAPI; int has_sglist =3D 0; =20 + /* PIO FIS gets written prior to transfer */ + ahci_write_fis_pio(ad, size); + if (is_atapi && !ad->done_atapi_packet) { /* already prepopulated iobuffer */ ad->done_atapi_packet =3D true; - size =3D 0; goto out; } =20 @@ -1315,19 +1316,12 @@ static void ahci_start_transfer(IDEDMA *dma) } } =20 -out: - /* declare that we processed everything */ - s->data_ptr =3D s->data_end; - /* Update number of transferred bytes, destroy sglist */ dma_buf_commit(s, size); - +out: + /* declare that we processed everything */ + s->data_ptr =3D s->data_end; s->end_transfer_func(s); - - if (!(s->status & DRQ_STAT)) { - /* done with PIO send/receive */ - ahci_write_fis_pio(ad, le32_to_cpu(ad->cur_cmd->status)); - } } =20 static void ahci_start_dma(IDEDMA *dma, IDEState *s, diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c index 63e1f9b92d..7264e085d0 100644 --- a/tests/libqos/ahci.c +++ b/tests/libqos/ahci.c @@ -478,10 +478,10 @@ void ahci_port_check_d2h_sanity(AHCIQState *ahci, uin= t8_t port, uint8_t slot) g_free(d2h); } =20 -void ahci_port_check_pio_sanity(AHCIQState *ahci, uint8_t port, - uint8_t slot, size_t buffsize) +void ahci_port_check_pio_sanity(AHCIQState *ahci, AHCICommand *cmd) { PIOSetupFIS *pio =3D g_malloc0(0x20); + uint8_t port =3D cmd->port; =20 /* We cannot check the Status or E_Status registers, because * the status may have again changed between the PIO Setup FIS @@ -489,15 +489,22 @@ void ahci_port_check_pio_sanity(AHCIQState *ahci, uin= t8_t port, qtest_memread(ahci->parent->qts, ahci->port[port].fb + 0x20, pio, 0x20= ); g_assert_cmphex(pio->fis_type, =3D=3D, 0x5f); =20 - /* BUG: PIO Setup FIS as utilized by QEMU tries to fit the entire - * transfer size in a uint16_t field. The maximum transfer size can - * eclipse this; the field is meant to convey the size of data per - * each Data FIS, not the entire operation as a whole. For now, - * we will sanity check the broken case where applicable. */ - if (buffsize <=3D UINT16_MAX) { - g_assert_cmphex(le16_to_cpu(pio->tx_count), =3D=3D, buffsize); + /* Data transferred by PIO will either be: + * (1) 12 or 16 bytes for an ATAPI command packet (QEMU always uses 12= ), or + * (2) Actual data from the drive. + * If we do both, (2) winds up erasing any evidence of (1). + */ + if (cmd->props->atapi && (cmd->xbytes =3D=3D 0 || cmd->props->dma)) { + g_assert(le16_to_cpu(pio->tx_count) =3D=3D 12 || + le16_to_cpu(pio->tx_count) =3D=3D 16); + } else { + /* The AHCI test suite here does not test any PIO command that spe= cifies + * a DRQ block larger than one sector (like 0xC4), so this should = always + * be one sector or less. */ + size_t pio_len =3D ((cmd->xbytes % cmd->sector_size) ? + (cmd->xbytes % cmd->sector_size) : cmd->sector_s= ize); + g_assert_cmphex(le16_to_cpu(pio->tx_count), =3D=3D, pio_len); } - g_free(pio); } =20 @@ -832,9 +839,9 @@ void ahci_command_enable_atapi_dma(AHCICommand *cmd) RegH2DFIS *fis =3D &(cmd->fis); g_assert(cmd->props->atapi); fis->feature_low |=3D 0x01; - cmd->interrupts &=3D ~AHCI_PX_IS_PSS; + /* PIO is still used to transfer the ATAPI command */ + g_assert(cmd->props->pio); cmd->props->dma =3D true; - cmd->props->pio =3D false; /* BUG: We expect the DMA Setup interrupt for DMA commands */ /* cmd->interrupts |=3D AHCI_PX_IS_DSS; */ } @@ -846,7 +853,7 @@ AHCICommand *ahci_command_create(uint8_t command_name) =20 g_assert(props); cmd =3D g_new0(AHCICommand, 1); - g_assert(!(props->dma && props->pio)); + g_assert(!(props->dma && props->pio) || props->atapi); g_assert(!(props->lba28 && props->lba48)); g_assert(!(props->read && props->write)); g_assert(!props->size || props->data); @@ -1218,7 +1225,7 @@ void ahci_command_verify(AHCIQState *ahci, AHCIComman= d *cmd) ahci_port_check_d2h_sanity(ahci, port, slot); } if (cmd->props->pio) { - ahci_port_check_pio_sanity(ahci, port, slot, cmd->xbytes); + ahci_port_check_pio_sanity(ahci, cmd); } } =20 diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h index 715ca1e226..13f6d87b75 100644 --- a/tests/libqos/ahci.h +++ b/tests/libqos/ahci.h @@ -596,8 +596,7 @@ void ahci_port_check_interrupts(AHCIQState *ahci, uint8= _t port, uint32_t intr_mask); void ahci_port_check_nonbusy(AHCIQState *ahci, uint8_t port, uint8_t slot); void ahci_port_check_d2h_sanity(AHCIQState *ahci, uint8_t port, uint8_t sl= ot); -void ahci_port_check_pio_sanity(AHCIQState *ahci, uint8_t port, - uint8_t slot, size_t buffsize); +void ahci_port_check_pio_sanity(AHCIQState *ahci, AHCICommand *cmd); void ahci_port_check_cmd_sanity(AHCIQState *ahci, AHCICommand *cmd); =20 /* Misc */ --=20 2.14.3