Re: [edk2-devel] [PATCHv2 0/4] MdeModulePkg/AtaAtapiPassThru: Add SATA error recovery

["Wu, Hao A" <hao.a.wu@intel.com>]

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wu, Hao
> A
> Sent: Monday, November 9, 2020 9:17 AM
> To: Albecki, Mateusz <mateusz.albecki@intel.com>; devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>
> Subject: Re: [edk2-devel] [PATCHv2 0/4] MdeModulePkg/AtaAtapiPassThru:
> Add SATA error recovery
> 
> > -----Original Message-----
> > From: Albecki, Mateusz <mateusz.albecki@intel.com>
> > Sent: Thursday, November 5, 2020 8:51 PM
> > To: devel@edk2.groups.io; Albecki, Mateusz <mateusz.albecki@intel.com>
> > Cc: Ni, Ray <ray.ni@intel.com>; Wu, Hao A <hao.a.wu@intel.com>
> > Subject: RE: [edk2-devel] [PATCHv2 0/4]
> MdeModulePkg/AtaAtapiPassThru:
> > Add SATA error recovery
> >
> > Forgot to put BZ tracker into v2 commit msg so I have resent the
> > series as v3 with proper bz info.
> 
> 
> (Seems no separate cover-letter sent for V3, replies here) I have given my 'R-
> b' tags for the V3 series:
> https://edk2.groups.io/g/devel/topic/78049844#67059
> https://edk2.groups.io/g/devel/topic/78049845#67060
> https://edk2.groups.io/g/devel/topic/78049846#67061
> https://edk2.groups.io/g/devel/topic/78049847#67063
> 
> Will wait a couple of days to see if there are comments from others.


V3 series has been pushed via:
PR:
https://github.com/tianocore/edk2/pull/1113

Commits:
https://github.com/tianocore/edk2/commit/cc28ab7a1d7353ad7ade55d19e58e85c50d8fa4d
https://github.com/tianocore/edk2/commit/b465a811006eb1663068380dded041d287ace907
https://github.com/tianocore/edk2/commit/64e25d4b062c907dab2dd30b686de9219d8e372c
https://github.com/tianocore/edk2/commit/91d95113d07aee4a11e9dac02c1a836c7360d422

Best Regards,
Hao Wu


> 
> Best Regards,
> Hao Wu
> 
> 
> >
> > > -----Original Message-----
> > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > > Albecki, Mateusz
> > > Sent: Thursday, November 5, 2020 1:41 PM
> > > To: devel@edk2.groups.io
> > > Cc: Albecki, Mateusz <mateusz.albecki@intel.com>; Ni, Ray
> > > <ray.ni@intel.com>; Wu, Hao A <hao.a.wu@intel.com>
> > > Subject: [edk2-devel] [PATCHv2 0/4] MdeModulePkg/AtaAtapiPassThru:
> > > Add SATA error recovery
> > >
> > > To increase boot stability when booting from SATA drives SATA driver
> > > should implement the AHCI spec defined port error recovery. This
> > > will allow the driver to handle random fails on SATA link.
> > >
> > > Performed tests on 2 setups. One with AHCI controller booting OS
> > > successfully without error recovery(control setup) and other which
> > > fails 1 in 5 times(fail setup).
> > >
> > > Tests performed:
> > > 1. Booted control setup to OS successfully.
> > > 2. Checked if during normal boot none of the packets is
> > > repeated(this came up after previous code version had a bug which
> > > repeated each DMA packet 5 times).
> > > 3. Booted control setup to OS with simulated errors appearing on
> > > first packet of every DMA transaction.
> > > 4. Performed extensive tests on fail setup. Fail rate decreased from
> > > 20% failure to ~1% failure. 1% failure is observed during OS
> > > execution, not BIOS so in a way boot is 100% stable).
> > >
> > > Change pushed to github:
> > > This series:
> > > https://github.com/malbecki/edk2/commits/sata_recovery2
> > > Simulated errors:
> > >
> >
> https://github.com/malbecki/edk2/commits/sata_recovery_simulated_erro
> > > r
> > >
> > > Some more information:
> > > For the bug mentioned above in point 2, the discussion can be
> > > referred
> > here:
> > >
> >
> https://github.com/malbecki/edk2/commit/9ea81cadf38725e194ec01e0b0c5
> > > 56fd133f3ced#r43226067
> > >
> > > For the other discussion we did before this patch, please refer to:
> > > https://github.com/malbecki/edk2/commits/sata_recovery
> > >
> > > For logging here is the example of verbose command log(taken from v1
> > > patch):
> > > Starting commmand:
> > > ATA COMMAND BLOCK:
> > > AtaCommand: 37
> > > AtaFeatures: 0
> > > AtaSectorNumber: 0
> > > AtaCylinderLow: 16
> > > AtaCylinderHigh: 16
> > > AtaDeviceHead: 224
> > > AtaSectorNumberExp: 57
> > > AtaCylinderLowExp: 0
> > > AtaCylinderHighExp: 0
> > > AtaFeaturesExp: 0
> > > AtaSectorCount: 64
> > > AtaSectorCountExp: 0
> > > DMA command failed at retry: 0
> > > DMA retry 1
> > > Starting commmand:
> > > ATA COMMAND BLOCK:
> > > AtaCommand: 37
> > > AtaFeatures: 0
> > > AtaSectorNumber: 0
> > > AtaCylinderLow: 16
> > > AtaCylinderHigh: 16
> > > AtaDeviceHead: 224
> > > AtaSectorNumberExp: 57
> > > AtaCylinderLowExp: 0
> > > AtaCylinderHighExp: 0
> > > AtaFeaturesExp: 0
> > > AtaSectorCount: 64
> > > AtaSectorCountExp: 0
> > > ATA STATUS BLOCK:
> > > AtaStatus: 80
> > > AtaError: 0
> > > DMA retry 0
> > >
> > > Changes in v2:
> > > - Changed logging per review suggestions
> > > - Fixed small issues with commit message
> > >
> > > Signed-off-by: Mateusz Albecki <mateusz.albecki@intel.com>
> > >
> > > Cc: Ray Ni <ray.ni@intel.com>
> > > Cc: Hao A Wu <hao.a.wu@intel.com>
> > >
> > >
> > > Mateusz Albecki (4):
> > >   MdeModulePkg/AtaAtapiPassThru: Check IS to check for command
> > >     completion
> > >   MdeModulePkg/AtaAtapiPassThru: Add SATA error recovery flow
> > >   MdeModulePkg/AtaAtapiPassThru: Restart failed packets
> > >   MdeModulePkg/AtaAtapiPassThru: Trace ATA packets
> > >
> > >  .../Bus/Ata/AtaAtapiPassThru/AhciMode.c       | 799 +++++++++++-------
> > >  .../Bus/Ata/AtaAtapiPassThru/AhciMode.h       |  18 +-
> > >  2 files changed, 532 insertions(+), 285 deletions(-)
> > >
> > > --
> > > 2.28.0.windows.1
> > >
> > > --------------------------------------------------------------------
> > > -
> > > Intel Technology Poland sp. z o.o.
> > > ul. Sowackiego 173 | 80-298 Gdask | Sd Rejonowy Gdask Pnoc | VII
> > > Wydzia Gospodarczy Krajowego Rejestru Sdowego - KRS 101882 | NIP
> > > 957-07-52-316
> > > | Kapita zakadowy 200.000 PLN.
> > > Ta wiadomo wraz z zacznikami jest przeznaczona dla okrelonego
> > > adresata i moe zawiera informacje poufne. W razie przypadkowego
> > > otrzymania tej wiadomoci, prosimy o powiadomienie nadawcy oraz trwae
> > > jej usunicie; jakiekolwiek przegldanie 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.
> > >
> > >
> > >
> > >
> > >
> > >
> 
> 
> 
> 
>