From nobody Fri Apr 19 18:24:37 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) client-ip=66.175.222.108; envelope-from=bounce+27952+101501+1787277+3901457@groups.io; helo=mail02.groups.io; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce+27952+101501+1787277+3901457@groups.io; dmarc=fail(p=none dis=none) header.from=intel.com ARC-Seal: i=1; a=rsa-sha256; t=1679430040; cv=none; d=zohomail.com; s=zohoarc; b=J6Q4IwfJRkSRNo85iC5KwbB8nX61jGUeDU9Vlgr38WPZ6DSAeQr6Xn40oSTGnHsEg6HweyTjzqFgJmgHZzmXTT94lDce14IBqA5gJvDRSYnLiZYdkhiQdwkay8kguj6wlDfwaucsoTEopb7AVVMqhEQzmJt9M6ooWRkdBlaNHM8= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1679430040; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Subscribe:List-Id:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Reply-To:References:Sender:Subject:To; bh=rfvuQcR4iuY0f9UX+ltIw1nIxrRq9/NXUOIW7bNK/yM=; b=aFxTdyrIHluMNUyvtORY+50F++Ql2+L4wQK29iNvEfB+RrzfUhg7rlQG2FtE5Rh6WH6PbbRW2ZAq7Jy85jGqFmOm4twdCRN5K0BAVj2txy81oVNilRiJBiTrn/bIFKrie1JsY6t6fDR9b/lAVORzvJMABI+9QNvZUN0x3uss9JA= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce+27952+101501+1787277+3901457@groups.io; dmarc=fail header.from= (p=none dis=none) Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by mx.zohomail.com with SMTPS id 1679430040149288.2295691287686; Tue, 21 Mar 2023 13:20:40 -0700 (PDT) Return-Path: X-Received: by 127.0.0.2 with SMTP id tqljYY1788612xpTk7yAL9Fs; Tue, 21 Mar 2023 13:20:39 -0700 X-Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by mx.groups.io with SMTP id smtpd.web10.25400.1679430038527149542 for ; Tue, 21 Mar 2023 13:20:39 -0700 X-IronPort-AV: E=McAfee;i="6600,9927,10656"; a="401623767" X-IronPort-AV: E=Sophos;i="5.98,279,1673942400"; d="scan'208";a="401623767" X-Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 Mar 2023 13:20:38 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10656"; a="1011101000" X-IronPort-AV: E=Sophos;i="5.98,279,1673942400"; d="scan'208";a="1011101000" X-Received: from malbecki-mobl1.ger.corp.intel.com ([10.213.7.55]) by fmsmga005-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 Mar 2023 13:20:36 -0700 From: "Albecki, Mateusz" To: devel@edk2.groups.io Cc: Mateusz Albecki , Hao A Wu , Ray Ni , Hunter Chang Subject: [edk2-devel] [PATCH 1/1] MdeModulePkg/Ahci: Skip retry for non-transient errors Date: Tue, 21 Mar 2023 21:20:15 +0100 Message-Id: <20230321202015.1877-2-mateusz.albecki@intel.com> In-Reply-To: <20230321202015.1877-1-mateusz.albecki@intel.com> References: <20230321202015.1877-1-mateusz.albecki@intel.com> MIME-Version: 1.0 Precedence: Bulk List-Unsubscribe: List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,mateusz.albecki@intel.com X-Gm-Message-State: YSePqRkbeLLo2vw7lM42vgcQx1787277AA= Content-Transfer-Encoding: quoted-printable DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=groups.io; q=dns/txt; s=20140610; t=1679430039; bh=2qDQUZpjW5f776fjh7QORS9oSxj9ZFKS6LF1sp2RjXY=; h=Cc:Content-Type:Date:From:Reply-To:Subject:To; b=novDNB3XT5ku1Zv6yGp3nx44xjYIbxETSVGjOowh+bW2nrDIMhZ63yWyW5+JUIBNcSP f38gk4NFmwaQwkR9vFBdUebjEhw2j3ZlJwt+7Szo1bjTN15MQvLR+HpF/dfZKugVH8C4x LLaxh2iaTvJ6651kfsjZ+21Eqh7ZCVmAz6U= X-ZohoMail-DKIM: pass (identity @groups.io) X-ZM-MESSAGEID: 1679430040957100002 Content-Type: text/plain; charset="utf-8" bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=3D4011 Currently AHCI driver will try to retry all failed packets regardless of the failure cause. This is a problem in password unlock flow where number of password retries is tracked by the device. If user passes a wrong password Ahci driver will try to send the wrong password multiple times which will exhaust number of password retries and force the user to restart the machine. This commit introduces a logic to check for the cause of packet failure and only retry packets which failed due to transient conditions on the link. With this patch only packets for which CRC error is flagged are retried. Cc: Hao A Wu Cc: Ray Ni Cc: Hunter Chang Signed-off-by: Mateusz Albecki --- .../Bus/Ata/AtaAtapiPassThru/AhciMode.c | 69 +++++++++++++++++-- .../Bus/Ata/AtaAtapiPassThru/AhciMode.h | 3 +- 2 files changed, 67 insertions(+), 5 deletions(-) diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c b/MdeModulePk= g/Bus/Ata/AtaAtapiPassThru/AhciMode.c index 06c4a3e052..90c9b4e69d 100644 --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c @@ -737,12 +737,66 @@ AhciRecoverPortError ( Status =3D AhciResetPort (PciIo, Port); if (EFI_ERROR (Status)) { DEBUG ((DEBUG_ERROR, "Failed to reset the port %d\n", Port)); + return EFI_DEVICE_ERROR; } } =20 return EFI_SUCCESS; } =20 +/** + This function will check if the failed command should be retired. Only e= rror + conditions which are a result of transient conditions on a link(either t= o system or to device). + + @param[in] PciIo Pointer to AHCI controller PciIo. + @param[in] Port SATA port index on which to check. + + @retval TRUE Command failure was caused by transient condition and sho= uld be retried + @retval FALSE Command should not be retried +**/ +BOOLEAN +AhciShouldCmdBeRetried ( + IN EFI_PCI_IO_PROTOCOL *PciIo, + IN UINT8 Port + ) +{ + UINT32 Offset; + UINT32 PortInterrupt; + UINT32 Serr; + UINT32 Tfd; + + Offset =3D EFI_AHCI_PORT_START + Port * EFI_AHCI_PORT_REG_WIDTH += EFI_AHCI_PORT_IS; + PortInterrupt =3D AhciReadReg (PciIo, Offset); + Offset =3D EFI_AHCI_PORT_START + Port * EFI_AHCI_PORT_REG_WIDTH + EFI_AH= CI_PORT_SERR; + Serr =3D AhciReadReg (PciIo, Offset); + Offset =3D EFI_AHCI_PORT_START + Port * EFI_AHCI_PORT_REG_WIDTH + EFI_AH= CI_PORT_TFD; + Tfd =3D AhciReadReg (PciIo, Offset); + + // + // This can occur if there was a CRC error on a path from system memory = to + // host controller. + // + if (PortInterrupt & EFI_AHCI_PORT_IS_HBDS) { + return TRUE; + // + // This can occur if there was a CRC error detected by host during commu= nication + // with the device + // + } else if ((PortInterrupt & (EFI_AHCI_PORT_IS_IFS | EFI_AHCI_PORT_IS_INF= S)) && + (Serr & EFI_AHCI_PORT_SERR_CRCE)) { + return TRUE; + // + // This can occur if there was a CRC error detected by device during com= munication + // with the host. Device returns error status to host with D2H FIS. + // + } else if ((PortInterrupt & EFI_AHCI_PORT_IS_TFES) && + (Tfd & EFI_AHCI_PORT_TFD_ERR_INT_CRC)) { + return TRUE; + } + + return FALSE; +} + /** Checks if specified FIS has been received. =20 @@ -950,6 +1004,7 @@ AhciPioTransfer ( UINT32 PrdCount; UINT32 Retry; EFI_STATUS RecoveryStatus; + BOOLEAN DoRetry; =20 if (Read) { Flag =3D EfiPciIoOperationBusMasterWrite; @@ -1027,8 +1082,9 @@ AhciPioTransfer ( =20 if (Status =3D=3D EFI_DEVICE_ERROR) { DEBUG ((DEBUG_ERROR, "PIO command failed at retry %d\n", Retry)); + DoRetry =3D AhciShouldCmdBeRetried (PciIo, Port); // needs to be cal= led before error recovery RecoveryStatus =3D AhciRecoverPortError (PciIo, Port); - if (EFI_ERROR (RecoveryStatus)) { + if (!DoRetry || EFI_ERROR (RecoveryStatus)) { break; } } else { @@ -1124,6 +1180,7 @@ AhciDmaTransfer ( EFI_TPL OldTpl; UINT32 Retry; EFI_STATUS RecoveryStatus; + BOOLEAN DoRetry; =20 Map =3D NULL; PciIo =3D Instance->PciIo; @@ -1222,8 +1279,9 @@ AhciDmaTransfer ( Status =3D AhciWaitUntilFisReceived (PciIo, Port, Timeout, SataFisD2= H); if (Status =3D=3D EFI_DEVICE_ERROR) { DEBUG ((DEBUG_ERROR, "DMA command failed at retry: %d\n", Retry)); + DoRetry =3D AhciShouldCmdBeRetried (PciIo, Port); // needs to be c= alled before error recovery RecoveryStatus =3D AhciRecoverPortError (PciIo, Port); - if (EFI_ERROR (RecoveryStatus)) { + if (!DoRetry || EFI_ERROR (RecoveryStatus)) { break; } } else { @@ -1263,6 +1321,7 @@ AhciDmaTransfer ( Status =3D AhciCheckFisReceived (PciIo, Port, SataFisD2H); if (Status =3D=3D EFI_DEVICE_ERROR) { DEBUG ((DEBUG_ERROR, "DMA command failed at retry: %d\n", Task->Re= tryTimes)); + DoRetry =3D AhciShouldCmdBeRetried (PciIo, Port); // call this bef= ore error recovery RecoveryStatus =3D AhciRecoverPortError (PciIo, Port); // // If recovery passed mark the Task as not started and change the = status @@ -1270,7 +1329,7 @@ AhciDmaTransfer ( // and on next call the command will be re-issued due to IsStart b= eing FALSE. // This also makes the next condition decrement the RetryTimes. // - if (RecoveryStatus =3D=3D EFI_SUCCESS) { + if (DoRetry && RecoveryStatus =3D=3D EFI_SUCCESS) { Task->IsStart =3D FALSE; Status =3D EFI_NOT_READY; } @@ -1378,6 +1437,7 @@ AhciNonDataTransfer ( EFI_AHCI_COMMAND_LIST CmdList; UINT32 Retry; EFI_STATUS RecoveryStatus; + BOOLEAN DoRetry; =20 // // Package read needed @@ -1418,8 +1478,9 @@ AhciNonDataTransfer ( Status =3D AhciWaitUntilFisReceived (PciIo, Port, Timeout, SataFisD2H); if (Status =3D=3D EFI_DEVICE_ERROR) { DEBUG ((DEBUG_ERROR, "Non data transfer failed at retry %d\n", Retry= )); + DoRetry =3D AhciShouldCmdBeRetried (PciIo, Port); // call this befor= e error recovery RecoveryStatus =3D AhciRecoverPortError (PciIo, Port); - if (EFI_ERROR (RecoveryStatus)) { + if (!DoRetry || EFI_ERROR (RecoveryStatus)) { break; } } else { diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h b/MdeModulePk= g/Bus/Ata/AtaAtapiPassThru/AhciMode.h index d7434b408c..5bb31057ec 100644 --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h @@ -146,7 +146,8 @@ typedef union { #define EFI_AHCI_PORT_TFD_BSY BIT7 #define EFI_AHCI_PORT_TFD_DRQ BIT3 #define EFI_AHCI_PORT_TFD_ERR BIT0 -#define EFI_AHCI_PORT_TFD_ERR_MASK 0x00FF00 +#define EFI_AHCI_PORT_TFD_ERR_MASK 0x00FF00 // ERROR field is speci= fied by ATA/ATAPI Command Set specification +#define EFI_AHCI_PORT_TFD_ERR_INT_CRC BIT15 #define EFI_AHCI_PORT_SIG 0x0024 #define EFI_AHCI_PORT_SSTS 0x0028 #define EFI_AHCI_PORT_SSTS_DET_MASK 0x000F --=20 2.39.1.windows.1 --------------------------------------------------------------------- Intel Technology Poland sp. z o.o. ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydz= ial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-31= 6 | Kapital zakladowy 200.000 PLN. Spolka oswiadcza, ze posiada status duzego przedsiebiorcy w rozumieniu usta= wy z dnia 8 marca 2013 r. o przeciwdzialaniu nadmiernym opoznieniom w trans= akcjach handlowych. Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata= i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wi= adomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiek= olwiek przegladanie lub rozpowszechnianie jest zabronione. This e-mail and any attachments may contain confidential material for the s= ole use of the intended recipient(s). If you are not the intended recipient= , please contact the sender and delete all copies; any review or distributi= on by others is strictly prohibited. -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#101501): https://edk2.groups.io/g/devel/message/101501 Mute This Topic: https://groups.io/mt/97764096/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-