From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-162.mimecast.com (us-smtp-delivery-162.mimecast.com [170.10.129.162]) by mx.groups.io with SMTP id smtpd.web11.9829.1679836732643227621 for ; Sun, 26 Mar 2023 06:18:54 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@hp.com header.s=mimecast20180716 header.b=Th/jOSNG; spf=pass (domain: hp.com, ip: 170.10.129.162, mailfrom: anbazhagan@hp.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=hp.com; s=mimecast20180716; t=1679836731; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=rhipz0n0Jfnq0Boaj6NsC01thyklxlJeVFEycWA4YlI=; b=Th/jOSNGhArBQaOwR5ph4uqkCR5g5/F7enI13Ajl94wzXrwZoS7/jNvEodQFgtjNf+e35w FKW9/OvbKnm3sCBojpzL4+uSwRcCxdy1F3hA6tfOADQaQNtv5VynREpZ7yJyR9C604bgBH N0sZbXv/5rgaatoiAus636mdPm5zPhM= Received: from NAM10-DM6-obe.outbound.protection.outlook.com (mail-dm6nam10lp2102.outbound.protection.outlook.com [104.47.58.102]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-389-GJEH0UdwM0qrd1WY0WaanA-2; Sun, 26 Mar 2023 09:18:48 -0400 X-MC-Unique: GJEH0UdwM0qrd1WY0WaanA-2 Received: from DM4PR84MB1520.NAMPRD84.PROD.OUTLOOK.COM (2603:10b6:8:4b::12) by PH7PR84MB1814.NAMPRD84.PROD.OUTLOOK.COM (2603:10b6:510:154::15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6222.28; Sun, 26 Mar 2023 13:18:45 +0000 Received: from DM4PR84MB1520.NAMPRD84.PROD.OUTLOOK.COM ([fe80::e19a:de6:ce3c:e331]) by DM4PR84MB1520.NAMPRD84.PROD.OUTLOOK.COM ([fe80::e19a:de6:ce3c:e331%4]) with mapi id 15.20.6222.028; Sun, 26 Mar 2023 13:18:45 +0000 From: "Anbazhagan, Baraneedharan" To: "devel@edk2.groups.io" , "hao.a.wu@intel.com" , "Albecki, Mateusz" CC: "Ni, Ray" , "Chang, Hunter" Subject: Re: [edk2-devel] [PATCH 0/1] MdeModulePkg/Ahci: Skip retry for non-transient errors Thread-Topic: [edk2-devel] [PATCH 0/1] MdeModulePkg/Ahci: Skip retry for non-transient errors Thread-Index: AQHZXDKnz+mHwfIPPk+e5x8VKAFxIK8GXx8AgAI3rACAAIyXAIAD7a9Q Date: Sun, 26 Mar 2023 13:18:45 +0000 Message-ID: References: <20230321202015.1877-1-mateusz.albecki@intel.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-publictraffictype: Email x-ms-traffictypediagnostic: DM4PR84MB1520:EE_|PH7PR84MB1814:EE_ x-ms-office365-filtering-correlation-id: 947e7e20-24a2-428a-b55a-08db2dfca0c9 x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0 x-microsoft-antispam-message-info: zNzPNiYfraCJxsoY6Gg9J2GU1BxhLcOpbxT9MN+B/wK6mJ3g5EERl2EpaDsqfyANhy3rMpUUc5qSssK4XrQkOxxx7t1KReuf82h9sh8QQ0/Hn6mpJfyAPh3CeJ7Slyt+QcE9BuqZCFQ/8CcNabZLbs4oO/740LKAQaRAtEfVXV0LTbM9gDzlNaEKHnkBV75aPR/9dY6wqBdRBqH5u8DFcIN0uLsitU2RQICnS6kDNAngOqvq8+Z0+2Lv/CfjBHyauwaFECWJldn6VXavSTl8qAi4pUP7jC5//Lsq0kMF1VfctdbXWjARUfirrZIiHdQFJR0u/6fSG4ijwX4ejjEChwD0RIhnVHO+zASMHw5IJg+J+YrOQeKtkGLGGdi2GFdHDeM7te1F169qqphE5N/cJ5TsALva1qSgQxHO0BqUcqA1NYZg+6AOWTdojEUQjiIBALhwm+2HfonVIW4Qduugo6aiM68nsO+wuMgWyv/MSfB+2I3unDs+wmHuAeJI7KS+m4q4JQv2qpYo2fcdIsrY9wZNJu2F5ILh/uUxEz0vGv5CZiPz0D6gSDzZXMvNBeoiGH7DDxAIafFfEXZFG4c9pHMJ3ia27mK7BmlSNkMGMqOM0AW9KUl1qzdEHL7y2Err9y/z516FMjIx1dPCyxWLHw== x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:DM4PR84MB1520.NAMPRD84.PROD.OUTLOOK.COM;PTR:;CAT:NONE;SFS:(13230028)(4636009)(136003)(376002)(346002)(39860400002)(366004)(396003)(451199021)(83380400001)(76116006)(4326008)(66446008)(66476007)(66946007)(66556008)(64756008)(8676002)(966005)(54906003)(7696005)(71200400001)(478600001)(53546011)(9686003)(6506007)(26005)(186003)(110136005)(316002)(55016003)(38070700005)(2906002)(166002)(33656002)(86362001)(41300700001)(52536014)(38100700002)(76236004)(5660300002)(8936002)(82960400001)(122000001)(66899021)(1406899024);DIR:OUT;SFP:1102 x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?2BmWxvFT18cycpFkVrf2EFa1Q6MMGe7BZ6yg5S6xfqH+yWf/T618gMoNtvdX?= =?us-ascii?Q?rAHT0mFJQOaigVFtkYz31fGnBBVqBB70Jvx+bH085NPRsk4QgnSDBiJAz0rp?= =?us-ascii?Q?INyaZyFjqkekRjrnbkN2hwzzBrG7et7SGptmNbo+JjomQHrvWtZZp08DKB6a?= =?us-ascii?Q?cfsNg9Kt5EUBAFuJQ7ZClCeGAFCpDfpFycHypC+lGs9oc8wz0Df0MLgA0Ddf?= =?us-ascii?Q?dTujNyTHzE+QCV251i6iEyyNpjT7fRxNsK/xW7KxPbMaWu/fjcIqS7cAU+Ee?= =?us-ascii?Q?I0dJHT+xHmvyzOtF6jWIun6jk5/MhFNRi6pJz2hlmz6nBjcE1FuC9FoBVgdr?= =?us-ascii?Q?PyVqSM5k/BloQWDWlmEVK0rWrpAbWPP3MkN2nJx5rSTcwMrVVQ+TtSq3oo4i?= =?us-ascii?Q?hZ3laicWj9Q8AMyOOzc9djgwmLD+VhM2DwCK8OwK0NH/UH6Xx0ptd/++PuzO?= =?us-ascii?Q?0Ip7V1aNIMG92/6hY54cNBa/L7EM2bFHt6cUjvk+hvoG6dYJ6/apQayHzsDO?= =?us-ascii?Q?eTs+9o6B2XWV9SGpZ2t71S7zxsAy2+BLcOXE3G5a66oVFmLU++uUnBNJIDHH?= =?us-ascii?Q?6FO34NqS4kdYaG115YpNEtrwAiYQ1bG/P6eejZ9ny75F0eTIyaBBAqSiicpJ?= =?us-ascii?Q?KRBzgrqfdAn6jNEpUkNJhaNugR8JK8KFQESu961qrCwvXuQX/dwVp2LrVnyE?= =?us-ascii?Q?JgZsva+CoCEawW9IO64v+7EfK2C3lsp1KMc7uJvSrPXU0bLWCZDcn+M1X6Vq?= =?us-ascii?Q?+TXVbcajLrswOMoAgVQWQyU2tfe/sTFvQd/2qnWS7qGMEkqQL2h8oqRQrul0?= =?us-ascii?Q?lzkXtiPX3IXaUKt89hE57P/Yd/89j4+qzhuYOg7oBeUovc/QA7XnFciqGgVV?= =?us-ascii?Q?7uQzv/O00yTC02TPqLqdSq/m9zRQdzwxzlNs3fpndC9mtBLxWGjrZBzLJZi9?= =?us-ascii?Q?IjPpnF161KzpbaQXlHGqjNo9VKsd7MGmja2CtqxMiFjvnVkf3hELJTIwBcsN?= =?us-ascii?Q?kQlfxMJdD8BNJYYOBt+J4qkcx7/KzJdULKDkIqps31VcC8RdCkEszciN2cny?= =?us-ascii?Q?iaNkBObye/6Pg2fIKhli7y0kHeAhBMqodcep+zKSVTOrKavR/49vUcyOll1D?= =?us-ascii?Q?mUwA2L46N9Qm4B6iSDEOCOVGNFxc0sI2IEdh8qHj7wHwxAuiN+SpIMOGkP21?= =?us-ascii?Q?sR95GY1sxv8bA1VrLTlHe5l2m6YOFD72Yk0AnlkWKW8WOo/Ms02GIWn4ftAT?= =?us-ascii?Q?ANk4d0MWyRma8fiXhSbcgX6ki6jUGHa+2C1RRE3MbVG3l/G9Gzuzopn5oWzK?= =?us-ascii?Q?YExLrcsGye14gz7fboi6ZfFpBXGCtZl2P7qUQKiN92QIeaAqzDG7jLGGsxOW?= =?us-ascii?Q?p4XGEynvoB010msPLNBkG8PhTRQGt7WpmIzXO8PJbGsNet1ZUOgniwt9R7wK?= =?us-ascii?Q?cqwbaeR77vFvNk89Px0Kk7paXMJSRwBO5/x8pCThE4sMlqZMIkMX4/Ym0On4?= =?us-ascii?Q?WWDH1pnvE3oo4wfXUrr24cvU0cGl0Mhs3QSa/enLWUbwNGVB0zFYfX0WPUnV?= =?us-ascii?Q?Tt9+UPzH2HZYWsq6KDI=3D?= MIME-Version: 1.0 X-OriginatorOrg: hp.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: DM4PR84MB1520.NAMPRD84.PROD.OUTLOOK.COM X-MS-Exchange-CrossTenant-Network-Message-Id: 947e7e20-24a2-428a-b55a-08db2dfca0c9 X-MS-Exchange-CrossTenant-originalarrivaltime: 26 Mar 2023 13:18:45.5344 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: ca7981a2-785a-463d-b82a-3db87dfc3ce6 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: m86+BMOsX5Jn2s6KbIwxsc0S0KyoOeviZT6maW/SEN4WQlcnTrYbOjLGTUw3dj/M1PPKhb8gXN7EVohXWgowhw== X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR84MB1814 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: hp.com Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_DM4PR84MB1520A2BF285540897ED48F7DBA8A9DM4PR84MB1520NAMP_" --_000_DM4PR84MB1520A2BF285540897ED48F7DBA8A9DM4PR84MB1520NAMP_ Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: quoted-printable It's OK to remove the PCD since password issue is fixed in different way. O= n other hand, PCD allows the platform to determine the retries independent = of specific command. Am OK with either approach on this one currently. Than= ks. From: devel@edk2.groups.io On Behalf Of Wu, Hao A vi= a groups.io Sent: Thursday, March 23, 2023 8:14 PM To: Anbazhagan, Baraneedharan ; devel@edk2.groups.io; Al= becki, Mateusz Cc: Ni, Ray ; Chang, Hunter Subject: Re: [edk2-devel] [PATCH 0/1] MdeModulePkg/Ahci: Skip retry for non= -transient errors CAUTION: External Email Thanks Baranee. We will proceed to merge the this patch after reviewing pro= cess. For the PCD (PcdAhciCommandRetryCount) previously introduced to address the= password retry issue, what is your opinion on it? Do you think we can remove it or keep it for other reason? Thanks. Best Regards, Hao Wu From: devel@edk2.groups.io > On Behalf Of Anbazhagan, Baraneedharan via= groups.io Sent: Friday, March 24, 2023 12:55 AM To: devel@edk2.groups.io; Wu, Hao A >; Albecki, Mateusz > Cc: Ni, Ray >; Chang, Hunter > Subject: Re: [edk2-devel] [PATCH 0/1] MdeModulePkg/Ahci: Skip retry for non= -transient errors Hi, This patch seems to resolve the issue reported in 4011 - MdeModulePkg: Need= configurable AHCI command retries (tianocore.org) and verified with 'AHCI_COMMAND_RETRIES' as = 5. Able to unlock the drive with correct password on 2nd attempt after prov= iding an incorrect password. Thanks, Baranee From: devel@edk2.groups.io > On Behalf Of Wu, Hao A via groups.io Sent: Wednesday, March 22, 2023 1:59 AM To: Albecki, Mateusz >; Anbazhagan, Baraneedharan >; devel@edk2.groups.io Cc: Ni, Ray >; Chang, Hunter > Subject: Re: [edk2-devel] [PATCH 0/1] MdeModulePkg/Ahci: Skip retry for non= -transient errors CAUTION: External Email Thanks Mateusz, the patch looks good to me. I noticed that there are some check failures in https://github.com/tianocor= e/edk2/pull/4157, could you he= lp to address them? Hello Baraneedharan Anbazhagan, Could you help to check if this patch can resolve the issue https://bugzill= a.tianocore.org/show_bug.cgi?id=3D4011 when switching back to: "#define AHCI_COMMAND_RETRIES 5"= ? This change can be accessed for integration at: https://patch-diff.githubus= ercontent.com/raw/tianocore/edk2/pull/4157.patch Thanks in advance. Best Regards, Hao Wu > -----Original Message----- > From: Albecki, Mateusz > > Sent: Wednesday, March 22, 2023 4:20 AM > To: devel@edk2.groups.io > Cc: Albecki, Mateusz >; Wu, Hao A > >; Ni, Ray >; Chang, Hunter > > > Subject: [PATCH 0/1] MdeModulePkg/Ahci: Skip retry for non-transient erro= rs > > Fix for the recovery logic which causes hdd unlock to fail if user suppli= es > incorrect password. Every failed packet used to be recovered which is cau= sing > the incorrect password to be tried multiple times. This patch series fixe= s the > logic to only retry commands that failed due to CRC error. > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3D4011 > > Github pull: https://github.com/tianocore/edk2/pull/4157 > > Tests: > - tested basic linux boot from AHCI on qemu > - tested basic linux boot from AHCI on custom qemu which will fail 50% of= the > DMA commands with CRC error. > Observed that all of the packets that failed were successfully retried. C= ustom > Qemu: https://github.com/matalbec/qemu/tree/sata_dma_50p_fail > - additionally Hunter Chang tested and confirmed that the password issue = is no > longer observed. > > Cc: Hao A Wu > > Cc: Ray Ni > > Cc: Hunter Chang > > > Mateusz Albecki (1): > MdeModulePkg/Ahci: Skip retry for non-transient errors > > .../Bus/Ata/AtaAtapiPassThru/AhciMode.c | 69 +++++++++++++++++-- > .../Bus/Ata/AtaAtapiPassThru/AhciMode.h | 3 +- > 2 files changed, 67 insertions(+), 5 deletions(-) > > -- > 2.39.1.windows.1 --_000_DM4PR84MB1520A2BF285540897ED48F7DBA8A9DM4PR84MB1520NAMP_ Content-Type: text/html; charset=WINDOWS-1252 Content-Transfer-Encoding: quoted-printable

It’s OK to remove the PCD since password issue= is fixed in different way. On other hand, PCD allows the platform to deter= mine the retries independent of specific command. Am OK with either approac= h on this one currently. Thanks.

 

From: devel@edk2.groups.io <devel@edk2.gro= ups.io> On Behalf Of Wu, Hao A via groups.io
Sent: Thursday, March 23, 2023 8:14 PM
To: Anbazhagan, Baraneedharan <anbazhagan@hp.com>; devel@edk2.= groups.io; Albecki, Mateusz <mateusz.albecki@intel.com>
Cc: Ni, Ray <ray.ni@intel.com>; Chang, Hunter <hunter.chang= @intel.com>
Subject: Re: [edk2-devel] [PATCH 0/1] MdeModulePkg/Ahci: Skip retry = for non-transient errors

 

CAUTION: External Email=

Thanks Baranee. We will proceed to merge the this pa= tch after reviewing process.

 

For the PCD (PcdAhciCommandRetryCount) previously in= troduced to address the password retry issue, what is your opinion on it?

Do you think we can remove it or keep it for other r= eason? Thanks.

 

Best Regards,

Hao Wu

 

From: devel@edk2.groups.io <devel= @edk2.groups.io> On Behalf Of Anbazhagan, Baraneedharan via groups.io
Sent: Friday, March 24, 2023 12:55 AM
To: devel@edk2.groups.io= ; Wu, Hao A <hao.a.wu@intel.com>; Albecki, Mateusz <ma= teusz.albecki@intel.com>
Cc: Ni, Ray <ray.ni@intel.com= >; Chang, Hunter <hunte= r.chang@intel.com>
Subject: Re: [edk2-devel] [PATCH 0/1] MdeModulePkg/Ahci: Skip retry = for non-transient errors

 

Hi,

This patch seems to resolve the issue reported in 4011 – MdeModulePkg: Need configurable AHCI command retries (tianocor= e.org) and verified with ‘AHCI_COMMAND_RETRIES’ as 5. Able = to unlock the drive with correct password on 2nd attempt after p= roviding an incorrect password.

 

Thanks,

Baranee

 

From: devel@edk2.groups.io <devel= @edk2.groups.io> On Behalf Of Wu, Hao A via groups.io
Sent: Wednesday, March 22, 2023 1:59 AM
To: Albecki, Mateusz <mateusz.albecki@intel.com>; Anbazhagan, Baraneedharan <anbazhagan@hp.com>; devel@edk2.groups.io
Cc: Ni, Ray <ray.ni@intel.com= >; Chang, Hunter <hunte= r.chang@intel.com>
Subject: Re: [edk2-devel] [PATCH 0/1] MdeModulePkg/Ahci: Skip retry = for non-transient errors

 

CAUTION: External Email=

Thanks Mateusz, the p= atch looks good to me.
I noticed that there are some check failures in https://github.com/tianocore/edk2/pull/4157, could you help to address = them?


Hello Baraneedharan Anbazhagan,
Could you help to check if this patch can resolve the issue https://bugzilla.tianocore.org/show_bug.cgi?id=3D4011 when switching ba= ck to: "#define AHCI_COMMAND_RETRIES 5"?
This change can be accessed for integration at: https://patch-diff.githubusercontent.com/raw/tianocore/edk2/pull/4157.patch=
Thanks in advance.

Best Regards,
Hao Wu

> -----Original Message-----
> From: Albecki, Mateusz <mateusz.albecki@intel.com>
> Sent: Wednesday, March 22, 2023 4:20 AM
> To: devel@edk2.groups.io > Cc: Albecki, Mateusz <= mateusz.albecki@intel.com>; Wu, Hao A
> <hao.a.wu@intel.com>; = Ni, Ray <ray.ni@intel.com>; C= hang, Hunter
> <hunter.chang@intel.com>
> Subject: [PATCH 0/1] MdeModulePkg/Ahci: Skip retry for non-transient e= rrors
>
> Fix for the recovery logic which causes hdd unlock to fail if user sup= plies
> incorrect password. Every failed packet used to be recovered which is = causing
> the incorrect password to be tried multiple times. This patch series f= ixes the
> logic to only retry commands that failed due to CRC error.
>
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3D4011
>
> Github pull: https://github.com/tianocore/edk2/pull/4157
>
> Tests:
> - tested basic linux boot from AHCI on qemu
> - tested basic linux boot from AHCI on custom qemu which will fail 50%= of the
> DMA commands with CRC error.
> Observed that all of the packets that failed were successfully retried= . Custom
> Qemu: https://github.com/matalbec/qemu/tree/sata_dma_50p_fail
> - additionally Hunter Chang tested and confirmed that the password iss= ue is no
> longer observed.
>
> Cc: Hao A Wu <hao.a.wu@intel.= com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Hunter Chang <hunter.= chang@intel.com>
>
> Mateusz Albecki (1):
> MdeModulePkg/Ahci: Skip retry for non-transient errors
>
> .../Bus/Ata/AtaAtapiPassThru/AhciMode.c | 69 +++++++++++++++++--
> .../Bus/Ata/AtaAtapiPassThru/AhciMode.h | 3 +-
> 2 files changed, 67 insertions(+), 5 deletions(-)
>
> --
> 2.39.1.windows.1

--_000_DM4PR84MB1520A2BF285540897ED48F7DBA8A9DM4PR84MB1520NAMP_--