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

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

> -----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.

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.
> >
> >
> >
> >
> > 
> >