Re: [RFC] SATA : Implemented NXP errata A008402

["Ni, Ruiyu" <ruiyu.ni@Intel.com>]

On 1/10/2018 4:56 PM, Meenakshi Aggarwal wrote:
> 
>> -----Original Message-----
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ni,
>> Ruiyu
>> Sent: Tuesday, January 09, 2018 9:11 AM
>> To: edk2-devel@lists.01.org
>> Subject: Re: [edk2] [RFC] SATA : Implemented NXP errata A008402
>>
>> On 1/8/2018 7:16 PM, Meenakshi Aggarwal wrote:
>>> Description:
>>> Commands with 4 MB PRD length entries fail if PRD[DBC] is
>>> set to the value according to AHCI standard spec.
>>> Due to a logic error, 3F_FFFFh is misinterpreted by the
>>> device as zero length.
>>
>> Is the logic error mentioned here is the error in HW?
>> Then I do not prefer to add such PCD for a HW workaround.
>>
> Yes, the error is in hardware.
> There is no other option visible to implement this errata if we want to use the existing AHCI framework.
> 
> I completely agree with you, but no other solution seems to exist in this case.
> 
> And this change will not impact any other hardware so no one is basically impacted by this change.

Your buggy HW only need the value zero. But the addition of PCD
exposes an interface that can use any size of PRD.
I am not sure the AtaAtapiPassThru can work if some platform
sets the PCD value to others than 0 or 3F_FFFFh.

Can you please
  just duplicate the AtaAtapiPassThru in your platform?
Because the driver is very stable today, not much code sync effort
will be needed if core version is changed.

> 
>>>
>>> Workaround:
>>> Set PRD length to 0 when creating a PRD entry for
>>> a maximum data transfer size of 4 MB to fix the erratum.
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
>>> ---
>>>    MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c           | 2 +-
>>>    MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf | 1 +
>>>    MdeModulePkg/MdeModulePkg.dec                              | 3 +++
>>>    3 files changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
>> b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
>>> index e6de5d6..fb6dc0b 100644
>>> --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
>>> +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
>>> @@ -591,7 +591,7 @@ AhciBuildCommand (
>>>        if (RemainedData < EFI_AHCI_MAX_DATA_PER_PRDT) {
>>>          AhciRegisters->AhciCommandTable-
>>> PrdtTable[PrdtIndex].AhciPrdtDbc = (UINT32)RemainedData - 1;
>>>        } else {
>>> -      AhciRegisters->AhciCommandTable-
>>> PrdtTable[PrdtIndex].AhciPrdtDbc = EFI_AHCI_MAX_DATA_PER_PRDT - 1;
>>> +      AhciRegisters->AhciCommandTable-
>>> PrdtTable[PrdtIndex].AhciPrdtDbc = PcdGet32 (PcdPrdtMaxDataLength);
>>>        }
>>>
>>>        Data64.Uint64 = (UINT64)MemAddr;
>>> diff --git
>> a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
>> b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
>>> index 82d5f7a..8921dd5 100644
>>> --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
>>> +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
>>> @@ -70,6 +70,7 @@
>>>
>>>    [Pcd]
>>>      gEfiMdeModulePkgTokenSpaceGuid.PcdAtaSmartEnable   ##
>> SOMETIMES_CONSUMES
>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdPrdtMaxDataLength
>>>
>>>    # [Event]
>>>    # EVENT_TYPE_PERIODIC_TIMER ## SOMETIMES_CONSUMES
>>> diff --git a/MdeModulePkg/MdeModulePkg.dec
>> b/MdeModulePkg/MdeModulePkg.dec
>>> index 8efad57..b2f9f2b 100644
>>> --- a/MdeModulePkg/MdeModulePkg.dec
>>> +++ b/MdeModulePkg/MdeModulePkg.dec
>>> @@ -1434,6 +1434,9 @@
>>>      # @Prompt Console Output Row of Text Setup
>>>
>> gEfiMdeModulePkgTokenSpaceGuid.PcdSetupConOutRow|25|UINT32|0x40
>> 00000e
>>>
>>> +  ## This PCD specifies the Maximum data length for a PRD Entry
>>> +
>> gEfiMdeModulePkgTokenSpaceGuid.PcdPrdtMaxDataLength|0x3FFFFF|UIN
>> T32|0x4000000f
>>> +
>>>    [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic,
>> PcdsDynamicEx]
>>>      ## UART clock frequency is for the baud rate configuration.
>>>      # @Prompt Serial Port Clock Rate.
>>>
>>
>>
>> --
>> Thanks,
>> Ray
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist
>> s.01.org%2Fmailman%2Flistinfo%2Fedk2-
>> devel&data=02%7C01%7Cmeenakshi.aggarwal%40nxp.com%7C60cadf13ac95
>> 48486c4908d55712d85c%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%
>> 7C636510660888390946&sdata=4oW2agf8z869IMz%2F7%2B2di9vC%2BB3js7K
>> hJx1LR2b4Tc4%3D&reserved=0
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 


-- 
Thanks,
Ray