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(-)
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.
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|0x4000000e
+ ## This PCD specifies the Maximum data length for a PRD Entry
+ gEfiMdeModulePkgTokenSpaceGuid.PcdPrdtMaxDataLength|0x3FFFFF|UINT32|0x4000000f
+
[PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
## UART clock frequency is for the baud rate configuration.
# @Prompt Serial Port Clock Rate.
--
1.9.1
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Do you have a full patch already?
Why the PcdPrdtMaxDataLength is defined to 0x3FFFFF, but not 0x400000?
Thanks,
Star
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Meenakshi Aggarwal
Sent: Monday, January 8, 2018 7:17 PM
To: ard.biesheuvel@linaro.org; leif.lindholm@linaro.org; edk2-devel@lists.01.org; Zeng, Star <star.zeng@intel.com>; Dong, Eric <eric.dong@intel.com>
Subject: [edk2] [RFC] SATA : Implemented NXP errata A008402
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.
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|0x4000000e
+ ## This PCD specifies the Maximum data length for a PRD Entry
+ gEfiMdeModulePkgTokenSpaceGuid.PcdPrdtMaxDataLength|0x3FFFFF|UINT32|0x4000000f
+
[PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
## UART clock frequency is for the baud rate configuration.
# @Prompt Serial Port Clock Rate.
--
1.9.1
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Hi,
I didn't prepare the full patch but will send in next few minutes,
i made the very basic changes required to test Errata implementation.
I will redefine it to 0x400000.
PcdPrdtMaxDataLength was defined to 0x3FFFFF just for testing purpose.
Thanks,
Meenakshi
> -----Original Message-----
> From: Zeng, Star [mailto:star.zeng@intel.com]
> Sent: Monday, January 08, 2018 11:19 AM
> To: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>;
> ard.biesheuvel@linaro.org; leif.lindholm@linaro.org; edk2-
> devel@lists.01.org; Dong, Eric <eric.dong@intel.com>
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
>
> Do you have a full patch already?
> Why the PcdPrdtMaxDataLength is defined to 0x3FFFFF, but not 0x400000?
>
>
> Thanks,
> Star
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Meenakshi Aggarwal
> Sent: Monday, January 8, 2018 7:17 PM
> To: ard.biesheuvel@linaro.org; leif.lindholm@linaro.org; edk2-
> devel@lists.01.org; Zeng, Star <star.zeng@intel.com>; Dong, Eric
> <eric.dong@intel.com>
> Subject: [edk2] [RFC] SATA : Implemented NXP errata A008402
>
> 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.
>
> 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.
> --
> 1.9.1
>
> _______________________________________________
> 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%7C9b1e2bde5b
> 4b47746d0e08d5565b8394%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C
> 0%7C636509873433437533&sdata=wg3Sg5SiU2MrbZSkZMlboldcXnMBKKg3jG
> yA45YZo1Q%3D&reserved=0
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Hi Star,
Apologies and some correction in my last reply.
As per the errata, PRDT Maximum value needs to be set to 0 only when creating a PRD entry for
a maximum data transfer size.
So there is no need to replace all occurrences of EFI_AHCI_MAX_DATA_PER_PRDT in file.
Just need to change it where we are setting the Data length.
I define it to 0x3FFFFF, as this is the actual value we are setting and this PCD need to be used only once.
I know, its NXP specific patch only, and i try to made changes which will not impact any other package.
Thanks,
Meenakshi
> -----Original Message-----
> From: Meenakshi Aggarwal
> Sent: Monday, January 08, 2018 11:25 AM
> To: 'Zeng, Star' <star.zeng@intel.com>; ard.biesheuvel@linaro.org;
> leif.lindholm@linaro.org; edk2-devel@lists.01.org; Dong, Eric
> <eric.dong@intel.com>
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>
> Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
>
> Hi,
>
> I didn't prepare the full patch but will send in next few minutes,
>
> i made the very basic changes required to test Errata implementation.
>
> I will redefine it to 0x400000.
> PcdPrdtMaxDataLength was defined to 0x3FFFFF just for testing purpose.
>
> Thanks,
> Meenakshi
>
> > -----Original Message-----
> > From: Zeng, Star [mailto:star.zeng@intel.com]
> > Sent: Monday, January 08, 2018 11:19 AM
> > To: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>;
> > ard.biesheuvel@linaro.org; leif.lindholm@linaro.org; edk2-
> > devel@lists.01.org; Dong, Eric <eric.dong@intel.com>
> > Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star <star.zeng@intel.com>
> > Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> >
> > Do you have a full patch already?
> > Why the PcdPrdtMaxDataLength is defined to 0x3FFFFF, but not 0x400000?
> >
> >
> > Thanks,
> > Star
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > Meenakshi Aggarwal
> > Sent: Monday, January 8, 2018 7:17 PM
> > To: ard.biesheuvel@linaro.org; leif.lindholm@linaro.org; edk2-
> > devel@lists.01.org; Zeng, Star <star.zeng@intel.com>; Dong, Eric
> > <eric.dong@intel.com>
> > Subject: [edk2] [RFC] SATA : Implemented NXP errata A008402
> >
> > 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.
> >
> > 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.
> > --
> > 1.9.1
> >
> > _______________________________________________
> > 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%7C9b1e2bde5b
> >
> 4b47746d0e08d5565b8394%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C
> >
> 0%7C636509873433437533&sdata=wg3Sg5SiU2MrbZSkZMlboldcXnMBKKg3jG
> > yA45YZo1Q%3D&reserved=0
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
So this PCD needs to be defined as 0x3FFFFF or may be 0x400000, then it needs to be configured to 0 for your case, right?
Could the PCD be configured to other values?
According to the statement you provided, is it possible to handle the case in the device firmware?
" Due to a logic error, 3F_FFFFh is misinterpreted by the device as zero length."
Thanks,
Star
-----Original Message-----
From: Meenakshi Aggarwal [mailto:meenakshi.aggarwal@nxp.com]
Sent: Monday, January 8, 2018 2:26 PM
To: Zeng, Star <star.zeng@intel.com>; ard.biesheuvel@linaro.org; leif.lindholm@linaro.org; edk2-devel@lists.01.org; Dong, Eric <eric.dong@intel.com>
Cc: Ni, Ruiyu <ruiyu.ni@intel.com>
Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
Hi Star,
Apologies and some correction in my last reply.
As per the errata, PRDT Maximum value needs to be set to 0 only when creating a PRD entry for a maximum data transfer size.
So there is no need to replace all occurrences of EFI_AHCI_MAX_DATA_PER_PRDT in file.
Just need to change it where we are setting the Data length.
I define it to 0x3FFFFF, as this is the actual value we are setting and this PCD need to be used only once.
I know, its NXP specific patch only, and i try to made changes which will not impact any other package.
Thanks,
Meenakshi
> -----Original Message-----
> From: Meenakshi Aggarwal
> Sent: Monday, January 08, 2018 11:25 AM
> To: 'Zeng, Star' <star.zeng@intel.com>; ard.biesheuvel@linaro.org;
> leif.lindholm@linaro.org; edk2-devel@lists.01.org; Dong, Eric
> <eric.dong@intel.com>
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>
> Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
>
> Hi,
>
> I didn't prepare the full patch but will send in next few minutes,
>
> i made the very basic changes required to test Errata implementation.
>
> I will redefine it to 0x400000.
> PcdPrdtMaxDataLength was defined to 0x3FFFFF just for testing purpose.
>
> Thanks,
> Meenakshi
>
> > -----Original Message-----
> > From: Zeng, Star [mailto:star.zeng@intel.com]
> > Sent: Monday, January 08, 2018 11:19 AM
> > To: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>;
> > ard.biesheuvel@linaro.org; leif.lindholm@linaro.org; edk2-
> > devel@lists.01.org; Dong, Eric <eric.dong@intel.com>
> > Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star <star.zeng@intel.com>
> > Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> >
> > Do you have a full patch already?
> > Why the PcdPrdtMaxDataLength is defined to 0x3FFFFF, but not 0x400000?
> >
> >
> > Thanks,
> > Star
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf
> > Of Meenakshi Aggarwal
> > Sent: Monday, January 8, 2018 7:17 PM
> > To: ard.biesheuvel@linaro.org; leif.lindholm@linaro.org; edk2-
> > devel@lists.01.org; Zeng, Star <star.zeng@intel.com>; Dong, Eric
> > <eric.dong@intel.com>
> > Subject: [edk2] [RFC] SATA : Implemented NXP errata A008402
> >
> > 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.
> >
> > 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.
> > --
> > 1.9.1
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> >
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis
> t
> > s.01.org%2Fmailman%2Flistinfo%2Fedk2-
> >
> devel&data=02%7C01%7Cmeenakshi.aggarwal%40nxp.com%7C9b1e2bde5b
> >
> 4b47746d0e08d5565b8394%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C
> >
> 0%7C636509873433437533&sdata=wg3Sg5SiU2MrbZSkZMlboldcXnMBKKg3jG
> > yA45YZo1Q%3D&reserved=0
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
> -----Original Message-----
> From: Zeng, Star [mailto:star.zeng@intel.com]
> Sent: Monday, January 08, 2018 3:18 PM
> To: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>;
> ard.biesheuvel@linaro.org; leif.lindholm@linaro.org; edk2-
> devel@lists.01.org; Dong, Eric <eric.dong@intel.com>
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
>
> So this PCD needs to be defined as 0x3FFFFF or may be 0x400000, then it
> needs to be configured to 0 for your case, right?
> Could the PCD be configured to other values?
No, in my case i need to set it to 0 only. No other value is needed.
>
> According to the statement you provided, is it possible to handle the case in
> the device firmware?
>
> " Due to a logic error, 3F_FFFFh is misinterpreted by the device as zero
> length."
>
No, it can't be handle in device firmware for existing SATA controller.
There are chances that in future releases of SATA controller it will get fixed in RTL,
but this change will still be needed for LS2088 board.
>
> Thanks,
> Star
> -----Original Message-----
> From: Meenakshi Aggarwal [mailto:meenakshi.aggarwal@nxp.com]
> Sent: Monday, January 8, 2018 2:26 PM
> To: Zeng, Star <star.zeng@intel.com>; ard.biesheuvel@linaro.org;
> leif.lindholm@linaro.org; edk2-devel@lists.01.org; Dong, Eric
> <eric.dong@intel.com>
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>
> Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
>
> Hi Star,
>
> Apologies and some correction in my last reply.
>
> As per the errata, PRDT Maximum value needs to be set to 0 only when
> creating a PRD entry for a maximum data transfer size.
>
> So there is no need to replace all occurrences of
> EFI_AHCI_MAX_DATA_PER_PRDT in file.
> Just need to change it where we are setting the Data length.
>
> I define it to 0x3FFFFF, as this is the actual value we are setting and this PCD
> need to be used only once.
>
> I know, its NXP specific patch only, and i try to made changes which will not
> impact any other package.
>
>
> Thanks,
> Meenakshi
>
> > -----Original Message-----
> > From: Meenakshi Aggarwal
> > Sent: Monday, January 08, 2018 11:25 AM
> > To: 'Zeng, Star' <star.zeng@intel.com>; ard.biesheuvel@linaro.org;
> > leif.lindholm@linaro.org; edk2-devel@lists.01.org; Dong, Eric
> > <eric.dong@intel.com>
> > Cc: Ni, Ruiyu <ruiyu.ni@intel.com>
> > Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> >
> > Hi,
> >
> > I didn't prepare the full patch but will send in next few minutes,
> >
> > i made the very basic changes required to test Errata implementation.
> >
> > I will redefine it to 0x400000.
> > PcdPrdtMaxDataLength was defined to 0x3FFFFF just for testing purpose.
> >
> > Thanks,
> > Meenakshi
> >
> > > -----Original Message-----
> > > From: Zeng, Star [mailto:star.zeng@intel.com]
> > > Sent: Monday, January 08, 2018 11:19 AM
> > > To: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>;
> > > ard.biesheuvel@linaro.org; leif.lindholm@linaro.org; edk2-
> > > devel@lists.01.org; Dong, Eric <eric.dong@intel.com>
> > > Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star <star.zeng@intel.com>
> > > Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> > >
> > > Do you have a full patch already?
> > > Why the PcdPrdtMaxDataLength is defined to 0x3FFFFF, but not
> 0x400000?
> > >
> > >
> > > Thanks,
> > > Star
> > > -----Original Message-----
> > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf
> > > Of Meenakshi Aggarwal
> > > Sent: Monday, January 8, 2018 7:17 PM
> > > To: ard.biesheuvel@linaro.org; leif.lindholm@linaro.org; edk2-
> > > devel@lists.01.org; Zeng, Star <star.zeng@intel.com>; Dong, Eric
> > > <eric.dong@intel.com>
> > > Subject: [edk2] [RFC] SATA : Implemented NXP errata A008402
> > >
> > > 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.
> > >
> > > 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.
> > > --
> > > 1.9.1
> > >
> > > _______________________________________________
> > > edk2-devel mailing list
> > > edk2-devel@lists.01.org
> > >
> >
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis
> > t
> > > s.01.org%2Fmailman%2Flistinfo%2Fedk2-
> > >
> >
> devel&data=02%7C01%7Cmeenakshi.aggarwal%40nxp.com%7C9b1e2bde5b
> > >
> >
> 4b47746d0e08d5565b8394%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C
> > >
> >
> 0%7C636509873433437533&sdata=wg3Sg5SiU2MrbZSkZMlboldcXnMBKKg3jG
> > > yA45YZo1Q%3D&reserved=0
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
How will the code work based on your patch if the this PCD is configured to other value, for example 0x200000/0x300000?
Thanks,
Star
-----Original Message-----
From: Meenakshi Aggarwal [mailto:meenakshi.aggarwal@nxp.com]
Sent: Monday, January 8, 2018 5:54 PM
To: Zeng, Star <star.zeng@intel.com>; ard.biesheuvel@linaro.org; leif.lindholm@linaro.org; edk2-devel@lists.01.org; Dong, Eric <eric.dong@intel.com>
Cc: Ni, Ruiyu <ruiyu.ni@intel.com>
Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> -----Original Message-----
> From: Zeng, Star [mailto:star.zeng@intel.com]
> Sent: Monday, January 08, 2018 3:18 PM
> To: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>;
> ard.biesheuvel@linaro.org; leif.lindholm@linaro.org; edk2-
> devel@lists.01.org; Dong, Eric <eric.dong@intel.com>
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
>
> So this PCD needs to be defined as 0x3FFFFF or may be 0x400000, then
> it needs to be configured to 0 for your case, right?
> Could the PCD be configured to other values?
No, in my case i need to set it to 0 only. No other value is needed.
>
> According to the statement you provided, is it possible to handle the
> case in the device firmware?
>
> " Due to a logic error, 3F_FFFFh is misinterpreted by the device as
> zero length."
>
No, it can't be handle in device firmware for existing SATA controller.
There are chances that in future releases of SATA controller it will get fixed in RTL, but this change will still be needed for LS2088 board.
>
> Thanks,
> Star
> -----Original Message-----
> From: Meenakshi Aggarwal [mailto:meenakshi.aggarwal@nxp.com]
> Sent: Monday, January 8, 2018 2:26 PM
> To: Zeng, Star <star.zeng@intel.com>; ard.biesheuvel@linaro.org;
> leif.lindholm@linaro.org; edk2-devel@lists.01.org; Dong, Eric
> <eric.dong@intel.com>
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>
> Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
>
> Hi Star,
>
> Apologies and some correction in my last reply.
>
> As per the errata, PRDT Maximum value needs to be set to 0 only when
> creating a PRD entry for a maximum data transfer size.
>
> So there is no need to replace all occurrences of
> EFI_AHCI_MAX_DATA_PER_PRDT in file.
> Just need to change it where we are setting the Data length.
>
> I define it to 0x3FFFFF, as this is the actual value we are setting
> and this PCD need to be used only once.
>
> I know, its NXP specific patch only, and i try to made changes which
> will not impact any other package.
>
>
> Thanks,
> Meenakshi
>
> > -----Original Message-----
> > From: Meenakshi Aggarwal
> > Sent: Monday, January 08, 2018 11:25 AM
> > To: 'Zeng, Star' <star.zeng@intel.com>; ard.biesheuvel@linaro.org;
> > leif.lindholm@linaro.org; edk2-devel@lists.01.org; Dong, Eric
> > <eric.dong@intel.com>
> > Cc: Ni, Ruiyu <ruiyu.ni@intel.com>
> > Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> >
> > Hi,
> >
> > I didn't prepare the full patch but will send in next few minutes,
> >
> > i made the very basic changes required to test Errata implementation.
> >
> > I will redefine it to 0x400000.
> > PcdPrdtMaxDataLength was defined to 0x3FFFFF just for testing purpose.
> >
> > Thanks,
> > Meenakshi
> >
> > > -----Original Message-----
> > > From: Zeng, Star [mailto:star.zeng@intel.com]
> > > Sent: Monday, January 08, 2018 11:19 AM
> > > To: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>;
> > > ard.biesheuvel@linaro.org; leif.lindholm@linaro.org; edk2-
> > > devel@lists.01.org; Dong, Eric <eric.dong@intel.com>
> > > Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star
> > > <star.zeng@intel.com>
> > > Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> > >
> > > Do you have a full patch already?
> > > Why the PcdPrdtMaxDataLength is defined to 0x3FFFFF, but not
> 0x400000?
> > >
> > >
> > > Thanks,
> > > Star
> > > -----Original Message-----
> > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On
> > > Behalf Of Meenakshi Aggarwal
> > > Sent: Monday, January 8, 2018 7:17 PM
> > > To: ard.biesheuvel@linaro.org; leif.lindholm@linaro.org; edk2-
> > > devel@lists.01.org; Zeng, Star <star.zeng@intel.com>; Dong, Eric
> > > <eric.dong@intel.com>
> > > Subject: [edk2] [RFC] SATA : Implemented NXP errata A008402
> > >
> > > 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.
> > >
> > > 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.
> > > --
> > > 1.9.1
> > >
> > > _______________________________________________
> > > edk2-devel mailing list
> > > edk2-devel@lists.01.org
> > >
> >
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis
> > t
> > > s.01.org%2Fmailman%2Flistinfo%2Fedk2-
> > >
> >
> devel&data=02%7C01%7Cmeenakshi.aggarwal%40nxp.com%7C9b1e2bde5b
> > >
> >
> 4b47746d0e08d5565b8394%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C
> > >
> >
> 0%7C636509873433437533&sdata=wg3Sg5SiU2MrbZSkZMlboldcXnMBKKg3jG
> > > yA45YZo1Q%3D&reserved=0
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Hi,
We will set PCD value to 0 to support our board and its default value is set to maximum data length.
If someone change its value to any other value in their package, then he/she must be aware of the consequences.
I cannot say how controller will react if value is other than 0x3f_ffff and 0 (in our case).
Thanks,
Meenakshi
> -----Original Message-----
> From: Zeng, Star [mailto:star.zeng@intel.com]
> Sent: Monday, January 08, 2018 3:54 PM
> To: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>;
> ard.biesheuvel@linaro.org; leif.lindholm@linaro.org; edk2-
> devel@lists.01.org; Dong, Eric <eric.dong@intel.com>
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
>
> How will the code work based on your patch if the this PCD is configured to
> other value, for example 0x200000/0x300000?
>
>
> Thanks,
> Star
> -----Original Message-----
> From: Meenakshi Aggarwal [mailto:meenakshi.aggarwal@nxp.com]
> Sent: Monday, January 8, 2018 5:54 PM
> To: Zeng, Star <star.zeng@intel.com>; ard.biesheuvel@linaro.org;
> leif.lindholm@linaro.org; edk2-devel@lists.01.org; Dong, Eric
> <eric.dong@intel.com>
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>
> Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
>
>
> > -----Original Message-----
> > From: Zeng, Star [mailto:star.zeng@intel.com]
> > Sent: Monday, January 08, 2018 3:18 PM
> > To: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>;
> > ard.biesheuvel@linaro.org; leif.lindholm@linaro.org; edk2-
> > devel@lists.01.org; Dong, Eric <eric.dong@intel.com>
> > Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star <star.zeng@intel.com>
> > Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> >
> > So this PCD needs to be defined as 0x3FFFFF or may be 0x400000, then
> > it needs to be configured to 0 for your case, right?
> > Could the PCD be configured to other values?
> No, in my case i need to set it to 0 only. No other value is needed.
> >
> > According to the statement you provided, is it possible to handle the
> > case in the device firmware?
> >
> > " Due to a logic error, 3F_FFFFh is misinterpreted by the device as
> > zero length."
> >
> No, it can't be handle in device firmware for existing SATA controller.
> There are chances that in future releases of SATA controller it will get fixed in
> RTL, but this change will still be needed for LS2088 board.
>
>
> >
> > Thanks,
> > Star
> > -----Original Message-----
> > From: Meenakshi Aggarwal [mailto:meenakshi.aggarwal@nxp.com]
> > Sent: Monday, January 8, 2018 2:26 PM
> > To: Zeng, Star <star.zeng@intel.com>; ard.biesheuvel@linaro.org;
> > leif.lindholm@linaro.org; edk2-devel@lists.01.org; Dong, Eric
> > <eric.dong@intel.com>
> > Cc: Ni, Ruiyu <ruiyu.ni@intel.com>
> > Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> >
> > Hi Star,
> >
> > Apologies and some correction in my last reply.
> >
> > As per the errata, PRDT Maximum value needs to be set to 0 only when
> > creating a PRD entry for a maximum data transfer size.
> >
> > So there is no need to replace all occurrences of
> > EFI_AHCI_MAX_DATA_PER_PRDT in file.
> > Just need to change it where we are setting the Data length.
> >
> > I define it to 0x3FFFFF, as this is the actual value we are setting
> > and this PCD need to be used only once.
> >
> > I know, its NXP specific patch only, and i try to made changes which
> > will not impact any other package.
> >
> >
> > Thanks,
> > Meenakshi
> >
> > > -----Original Message-----
> > > From: Meenakshi Aggarwal
> > > Sent: Monday, January 08, 2018 11:25 AM
> > > To: 'Zeng, Star' <star.zeng@intel.com>; ard.biesheuvel@linaro.org;
> > > leif.lindholm@linaro.org; edk2-devel@lists.01.org; Dong, Eric
> > > <eric.dong@intel.com>
> > > Cc: Ni, Ruiyu <ruiyu.ni@intel.com>
> > > Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> > >
> > > Hi,
> > >
> > > I didn't prepare the full patch but will send in next few minutes,
> > >
> > > i made the very basic changes required to test Errata implementation.
> > >
> > > I will redefine it to 0x400000.
> > > PcdPrdtMaxDataLength was defined to 0x3FFFFF just for testing purpose.
> > >
> > > Thanks,
> > > Meenakshi
> > >
> > > > -----Original Message-----
> > > > From: Zeng, Star [mailto:star.zeng@intel.com]
> > > > Sent: Monday, January 08, 2018 11:19 AM
> > > > To: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>;
> > > > ard.biesheuvel@linaro.org; leif.lindholm@linaro.org; edk2-
> > > > devel@lists.01.org; Dong, Eric <eric.dong@intel.com>
> > > > Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star
> > > > <star.zeng@intel.com>
> > > > Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> > > >
> > > > Do you have a full patch already?
> > > > Why the PcdPrdtMaxDataLength is defined to 0x3FFFFF, but not
> > 0x400000?
> > > >
> > > >
> > > > Thanks,
> > > > Star
> > > > -----Original Message-----
> > > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On
> > > > Behalf Of Meenakshi Aggarwal
> > > > Sent: Monday, January 8, 2018 7:17 PM
> > > > To: ard.biesheuvel@linaro.org; leif.lindholm@linaro.org; edk2-
> > > > devel@lists.01.org; Zeng, Star <star.zeng@intel.com>; Dong, Eric
> > > > <eric.dong@intel.com>
> > > > Subject: [edk2] [RFC] SATA : Implemented NXP errata A008402
> > > >
> > > > 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.
> > > >
> > > > 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.
> > > > --
> > > > 1.9.1
> > > >
> > > > _______________________________________________
> > > > edk2-devel mailing list
> > > > edk2-devel@lists.01.org
> > > >
> > >
> >
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis
> > > t
> > > > s.01.org%2Fmailman%2Flistinfo%2Fedk2-
> > > >
> > >
> >
> devel&data=02%7C01%7Cmeenakshi.aggarwal%40nxp.com%7C9b1e2bde5b
> > > >
> > >
> >
> 4b47746d0e08d5565b8394%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C
> > > >
> > >
> >
> 0%7C636509873433437533&sdata=wg3Sg5SiU2MrbZSkZMlboldcXnMBKKg3jG
> > > > yA45YZo1Q%3D&reserved=0
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
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.
>
> 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|0x4000000e
>
> + ## This PCD specifies the Maximum data length for a PRD Entry
> + gEfiMdeModulePkgTokenSpaceGuid.PcdPrdtMaxDataLength|0x3FFFFF|UINT32|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://lists.01.org/mailman/listinfo/edk2-devel
> -----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.
> >
> > 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
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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Hi Ruiyu,
> -----Original Message-----
> >
> > 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.
I don't see someone using this driver will set Pcd randomly, but I agree on this
point other value should be handled.
Error or Assert could be added, if Pcd value is not 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.
Duplicating is always a possibility :), but When we will push this duplicated driver
(just for one line change) for upstreaming.
will this be acceptable ??
Thanks
> >
> >>>
> >>> 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%2Fli
> >> st
> >> 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://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis
> > ts.01.org%2Fmailman%2Flistinfo%2Fedk2-
> devel&data=02%7C01%7Cudit.kumar%
> >
> 40nxp.com%7C0b697e2364f348745a7c08d5580cf5bf%7C686ea1d3bc2b4c6fa9
> 2cd99
> >
> c5c301635%7C0%7C0%7C636511735060291299&sdata=eWI7ueE5wzr2oSNv0t
> CP5hBE0
> > uEfB9S0PyABHWocBaA%3D&reserved=0
> >
>
>
> --
> Thanks,
> Ray
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.
> 01.org%2Fmailman%2Flistinfo%2Fedk2-
> devel&data=02%7C01%7Cudit.kumar%40nxp.com%7C0b697e2364f348745a7c
> 08d5580cf5bf%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636511
> 735060291299&sdata=eWI7ueE5wzr2oSNv0tCP5hBE0uEfB9S0PyABHWocBaA
> %3D&reserved=0
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On 10 January 2018 at 09:43, Udit Kumar <udit.kumar@nxp.com> wrote: > Hi Ruiyu, > >> -----Original Message----- >> > >> > 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. > > I don't see someone using this driver will set Pcd randomly, but I agree on this > point other value should be handled. > Error or Assert could be added, if Pcd value is not 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. > > Duplicating is always a possibility :), but When we will push this duplicated driver > (just for one line change) for upstreaming. > will this be acceptable ?? My main concern with this (and with using a PCD) is that the setting affects all SATA controllers in the system, including ones the you stick into a PCIe slot. So I think forking the driver is the only possible solution, but it will not be a one-line change: you need to ensure that you apply the workaround only to the SATA controllers in the SoC. -- Ard. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
> -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > Sent: Wednesday, January 10, 2018 3:23 PM > To: Udit Kumar <udit.kumar@nxp.com> > Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Meenakshi Aggarwal > <meenakshi.aggarwal@nxp.com>; edk2-devel@lists.01.org; Zeng, Star > <star.zeng@intel.com>; leif.lindholm@linaro.org > Subject: Re: [edk2] [RFC] SATA : Implemented NXP errata A008402 > > On 10 January 2018 at 09:43, Udit Kumar <udit.kumar@nxp.com> wrote: > > Hi Ruiyu, > > > >> -----Original Message----- > >> > > >> > 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. > > > > I don't see someone using this driver will set Pcd randomly, but I > > agree on this point other value should be handled. > > Error or Assert could be added, if Pcd value is not 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. > > > > Duplicating is always a possibility :), but When we will push this > > duplicated driver (just for one line change) for upstreaming. > > will this be acceptable ?? > > My main concern with this (and with using a PCD) is that the setting affects > all SATA controllers in the system, including ones the you stick into a PCIe > slot. Agreed, If two types of Sata controllers are used then this patch may break one controller. At present, we don’t have anyone asking SATA controller behind PCIe but we shouldn’t ignore this. This is valid use case. > So I think forking the driver is the only possible solution, but it will not be a > one-line change: you need to ensure that you apply the workaround only to > the SATA controllers in the SoC. Yes, Considering unrelated SATA controllers this will be more than copy > -- > Ard. Thanks Udit _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 1/10/2018 5:52 PM, Ard Biesheuvel wrote: > On 10 January 2018 at 09:43, Udit Kumar <udit.kumar@nxp.com> wrote: >> Hi Ruiyu, >> >>> -----Original Message----- >>>> >>>> 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. >> >> I don't see someone using this driver will set Pcd randomly, but I agree on this >> point other value should be handled. >> Error or Assert could be added, if Pcd value is not 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. >> >> Duplicating is always a possibility :), but When we will push this duplicated driver >> (just for one line change) for upstreaming. >> will this be acceptable ?? > > My main concern with this (and with using a PCD) is that the setting > affects all SATA controllers in the system, including ones the you > stick into a PCIe slot. > > So I think forking the driver is the only possible solution, but it > will not be a one-line change: you need to ensure that you apply the > workaround only to the SATA controllers in the SoC. > Depending on the new driver's location. The package owner decides whether forking is acceptable :) -- Thanks, Ray _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 11 January 2018 at 02:25, Ni, Ruiyu <ruiyu.ni@intel.com> wrote: > On 1/10/2018 5:52 PM, Ard Biesheuvel wrote: >> >> On 10 January 2018 at 09:43, Udit Kumar <udit.kumar@nxp.com> wrote: >>> >>> Hi Ruiyu, >>> >>>> -----Original Message----- >>>>> >>>>> >>>>> 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. >>> >>> >>> I don't see someone using this driver will set Pcd randomly, but I agree >>> on this >>> point other value should be handled. >>> Error or Assert could be added, if Pcd value is not 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. >>> >>> >>> Duplicating is always a possibility :), but When we will push this >>> duplicated driver >>> (just for one line change) for upstreaming. >>> will this be acceptable ?? >> >> >> My main concern with this (and with using a PCD) is that the setting >> affects all SATA controllers in the system, including ones the you >> stick into a PCIe slot. >> >> So I think forking the driver is the only possible solution, but it >> will not be a one-line change: you need to ensure that you apply the >> workaround only to the SATA controllers in the SoC. >> > > Depending on the new driver's location. The package owner decides > whether forking is acceptable :) > I think forking is reasonable in this case: the workaround does not belong in generic code, and it seems only early silicon is affected anyway. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
© 2016 - 2024 Red Hat, Inc.