Re: [edk2-devel] [PATCH 0/1] MdeModulePkg/Ahci: Skip retry for non-transient errors
It's OK to remove the PCD since password issue is fixed in different way. On other hand, PCD allows the platform to determine the retries independent of specific command. Am OK with either approach on this one currently. Thanks. From: devel@edk2.groups.io On Behalf Of Wu, Hao A via groups.io Sent: Thursday, March 23, 2023 8:14 PM To: Anbazhagan, Baraneedharan ; devel@edk2.groups.io; Albecki, 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 process. 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<mailto:devel@edk2.groups.io> mailto: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<mailto:devel@edk2.groups.io>; Wu, Hao A mailto:hao.a...@intel.com>>; Albecki, Mateusz mailto:mateusz.albe...@intel.com>> Cc: Ni, Ray mailto:ray...@intel.com>>; Chang, Hunter mailto:hunter.ch...@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 (tianocore.org)<https://bugzilla.tianocore.org/show_bug.cgi?id=4011> and verified with 'AHCI_COMMAND_RETRIES' as 5. Able to unlock the drive with correct password on 2nd attempt after providing an incorrect password. Thanks, Baranee From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> mailto:devel@edk2.groups.io>> On Behalf Of Wu, Hao A via groups.io Sent: Wednesday, March 22, 2023 1:59 AM To: Albecki, Mateusz mailto:mateusz.albe...@intel.com>>; Anbazhagan, Baraneedharan mailto:anbazha...@hp.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> Cc: Ni, Ray mailto:ray...@intel.com>>; Chang, Hunter mailto:hunter.ch...@intel.com>> 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/tianocore/edk2/pull/4157<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=4011<https://bugzilla.tianocore.org/show_bug.cgi?id=4011> when switching back 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<https://patch-diff.githubusercontent.com/raw/tianocore/edk2/pull/4157.patch> Thanks in advance. Best Regards, Hao Wu > -Original Message- > From: Albecki, Mateusz > mailto:mateusz.albe...@intel.com>> > Sent: Wednesday, March 22, 2023 4:20 AM > To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> > Cc: Albecki, Mateusz > mailto:mateusz.albe...@intel.com>>; Wu, Hao A > mailto:hao.a...@intel.com>>; Ni, Ray > mailto:ray...@intel.com>>; Chang, Hunter > mailto:hunter.ch...@intel.com>> > Subject: [PATCH 0/1] MdeModulePkg/Ahci: Skip retry for non-transient errors > > Fix for the recovery logic which causes hdd unlock to fail if user supplies > incorrect password. Every failed packet used to be recovered which is causing > the incorrect password to be tried multiple times. This patch series fixes the > logic to only retry commands that failed due to CRC error. > > BZ: > https://bugzilla.tianocore.org/show_bug.cgi?id=4011<https://bugzilla.tianocore.org/show_bug.cgi?id=4011> > > Github pull: > https://github.com/tianocore/edk2/pull/4157<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<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 mailto:hao.a...@intel.com>> > Cc: Ray Ni mailto:ray...@intel.com>> > Cc: Hunter Chang mailto:hunter.ch...@intel.com>> > > Mateusz Albecki (1): > MdeModulePkg/Ahci: Skip retry for non-transient errors > > .../Bus/Ata/AtaAtapiPassThru/AhciMod
Re: [edk2-devel] [PATCH 0/1] MdeModulePkg/Ahci: Skip retry for non-transient errors
Thanks Baranee. We will proceed to merge the this patch after reviewing process. 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)<https://bugzilla.tianocore.org/show_bug.cgi?id=4011> and verified with 'AHCI_COMMAND_RETRIES' as 5. Able to unlock the drive with correct password on 2nd attempt after providing an incorrect password. Thanks, Baranee From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> mailto:devel@edk2.groups.io>> On Behalf Of Wu, Hao A via groups.io Sent: Wednesday, March 22, 2023 1:59 AM To: Albecki, Mateusz mailto:mateusz.albe...@intel.com>>; Anbazhagan, Baraneedharan mailto:anbazha...@hp.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> Cc: Ni, Ray mailto:ray...@intel.com>>; Chang, Hunter mailto:hunter.ch...@intel.com>> 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/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=4011 when switching back 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 > mailto:mateusz.albe...@intel.com>> > Sent: Wednesday, March 22, 2023 4:20 AM > To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> > Cc: Albecki, Mateusz > mailto:mateusz.albe...@intel.com>>; Wu, Hao A > mailto:hao.a...@intel.com>>; Ni, Ray > mailto:ray...@intel.com>>; Chang, Hunter > mailto:hunter.ch...@intel.com>> > Subject: [PATCH 0/1] MdeModulePkg/Ahci: Skip retry for non-transient errors > > Fix for the recovery logic which causes hdd unlock to fail if user supplies > incorrect password. Every failed packet used to be recovered which is causing > the incorrect password to be tried multiple times. This patch series fixes the > logic to only retry commands that failed due to CRC error. > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4011 > > 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 issue is no > longer observed. > > Cc: Hao A Wu mailto:hao.a...@intel.com>> > Cc: Ray Ni mailto:ray...@intel.com>> > Cc: Hunter Chang mailto:hunter.ch...@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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#101713): https://edk2.groups.io/g/devel/message/101713 Mute This Topic: https://groups.io/mt/97764094/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
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)<https://bugzilla.tianocore.org/show_bug.cgi?id=4011> and verified with 'AHCI_COMMAND_RETRIES' as 5. Able to unlock the drive with correct password on 2nd attempt after providing 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/tianocore/edk2/pull/4157<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=4011<https://bugzilla.tianocore.org/show_bug.cgi?id=4011> when switching back 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<https://patch-diff.githubusercontent.com/raw/tianocore/edk2/pull/4157.patch> Thanks in advance. Best Regards, Hao Wu > -Original Message- > From: Albecki, Mateusz > mailto:mateusz.albe...@intel.com>> > Sent: Wednesday, March 22, 2023 4:20 AM > To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> > Cc: Albecki, Mateusz > mailto:mateusz.albe...@intel.com>>; Wu, Hao A > mailto:hao.a...@intel.com>>; Ni, Ray > mailto:ray...@intel.com>>; Chang, Hunter > mailto:hunter.ch...@intel.com>> > Subject: [PATCH 0/1] MdeModulePkg/Ahci: Skip retry for non-transient errors > > Fix for the recovery logic which causes hdd unlock to fail if user supplies > incorrect password. Every failed packet used to be recovered which is causing > the incorrect password to be tried multiple times. This patch series fixes the > logic to only retry commands that failed due to CRC error. > > BZ: > https://bugzilla.tianocore.org/show_bug.cgi?id=4011<https://bugzilla.tianocore.org/show_bug.cgi?id=4011> > > Github pull: > https://github.com/tianocore/edk2/pull/4157<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<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 mailto:hao.a...@intel.com>> > Cc: Ray Ni mailto:ray...@intel.com>> > Cc: Hunter Chang mailto:hunter.ch...@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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#101699): https://edk2.groups.io/g/devel/message/101699 Mute This Topic: https://groups.io/mt/97764094/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 0/1] MdeModulePkg/Ahci: Skip retry for non-transient errors
Seems like a code formatting issue. I will address it tomorrow. Regards, Mateusz -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#101698): https://edk2.groups.io/g/devel/message/101698 Mute This Topic: https://groups.io/mt/97764094/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 0/1] MdeModulePkg/Ahci: Skip retry for non-transient errors
Thanks Mateusz, the patch 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=4011 when switching back 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 > 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 errors > > Fix for the recovery logic which causes hdd unlock to fail if user supplies > incorrect password. Every failed packet used to be recovered which is causing > the incorrect password to be tried multiple times. This patch series fixes the > logic to only retry commands that failed due to CRC error. > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4011 > > 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 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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#101558): https://edk2.groups.io/g/devel/message/101558 Mute This Topic: https://groups.io/mt/97764094/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 0/1] MdeModulePkg/Ahci: Skip retry for non-transient errors
Fix for the recovery logic which causes hdd unlock to fail if user supplies incorrect password. Every failed packet used to be recovered which is causing the incorrect password to be tried multiple times. This patch series fixes the logic to only retry commands that failed due to CRC error. BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4011 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 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 - Intel Technology Poland sp. z o.o. ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN. Spolka oswiadcza, ze posiada status duzego przedsiebiorcy w rozumieniu ustawy z dnia 8 marca 2013 r. o przeciwdzialaniu nadmiernym opoznieniom w transakcjach handlowych. Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek przegladanie lub rozpowszechnianie jest zabronione. This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#101500): https://edk2.groups.io/g/devel/message/101500 Mute This Topic: https://groups.io/mt/97764094/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
