In SctPkg/TestCase/UEFI/EFI/Protocol/StorageSecurityCommand/BlackBoxTest/StorageSecurityCommandBBTestConformance.c test case SCT_Log/MediaAccessTest/StorageSecurityCommandProtocolTest0/ ReceiveData_Conf It tries to pass a 10 bytes length buffer to StorageSecurityCommand protocol and expect it return an error because "for ATA8-ACS SecurityProtocol, 512 byte is a request". This works with edk2 AtaAtapiPassThru driver before the SATA recovery flow was introduced. However after the recovery is implemented, the ATA passthru will recover the port error and return an EFI_SUCCESS, then the SCT test will fail. I've no idea if it's the AtaAtapiPassThru driver's bug (if it should return success when command execute is actually failed then port recovered), or the SCT test case should update align with edk2 implementation.
Does the new recovery code follow the specification? Consult the implementer from Intel for the details of the recovery code. According to spec, does 10 bytes denote failure? To Fu Siyuan: Please give the complete output of the failing test? Which line number causes the error ?
I think it is a bug in SATA driver. The bug is that the Status variable is used to store ErrorRecovery data and overrides packet transfer status on last iteration of the retry loop: in https://github.com/tianocore/edk2/commit/64e25d4b062c907dab2dd30b686de9219d8e372c AhciMode.c:976 The correct flow should be to return packet transfer/operation status. Bug is present in both PIO and DMA transfer.
(In reply to Mateusz Albecki from comment #2) > I think it is a bug in SATA driver. The bug is that the Status variable is > used to store ErrorRecovery data and overrides packet transfer status on > last iteration of the retry loop: > > in > https://github.com/tianocore/edk2/commit/ > 64e25d4b062c907dab2dd30b686de9219d8e372c > > AhciMode.c:976 > > The correct flow should be to return packet transfer/operation status. Bug > is present in both PIO and DMA transfer. I hold the same opinion. The recovery result status override the failed transfer status. It failed in SctPkg/TestCase/UEFI/EFI/Protocol/StorageSecurityCommand/BlackBoxTest/StorageSecurityCommandBBTestConformance.c:235: Status: Success ExpectedStatus:DeviceError
Hi Fu Siyan, Since this an issue in the firmware, can we close the ticket in edk2-test without fix?
I am actually planning to fix this in EDK2. I had different priorities until now but I have some spare time so I will fix it by end of week.
Patch: https://edk2.groups.io/g/devel/topic/patch_0_1_mdemodulepkg_ata/94411381?p=,,,20,0,0,0::recentpostdate/sticky,,,20,2,0,94411381,previd%3D1666108503382017884,nextid%3D1666099844113401112&previd=1666108503382017884&nextid=1666099844113401112
Hi Fu Siyuan, Can we close this ticket on edk2-test since the solution raised in edk2 code base?
(In reply to Edhay from comment #7) > Hi Fu Siyuan, > > Can we close this ticket on edk2-test since the solution raised in edk2 > code base? Yes please close This， thanks
Solution for this issue is raised in edk2 code base: https://edk2.groups.io/g/devel/topic/patch_0_1_mdemodulepkg_ata/94411381?p=,,,20,0,0,0::recentpostdate/sticky,,,20,2,0,94411381,previd%3D1666108503382017884,nextid%3D1666099844113401112&previd=1666108503382017884&nextid=1666099844113401112
Note: edk2 solution is merged via: PR - https://github.com/tianocore/edk2/pull/3746 Commit - https://github.com/tianocore/edk2/commit/a6542894391bae58b7704b2624b541a2b8b9ed93
