Re: [edk2-devel] [PATCH v8] MinPlatformPkg: Update HWSignature filed in FACS
Hi, Nate
The patch has been merged on May 31, 2023 with patch v10.
For details, please check below two links. Thanks.
https://bugzilla.tianocore.org/show_bug.cgi?id=4428
https://github.com/tianocore/edk2-platforms/commit/1a7bd150d39007bfb72c4727feda3184c23efe96
Best Regards,
Vincent
-Original Message-
From: Desimone, Nathaniel L
Sent: Wednesday, May 15, 2024 7:42 AM
To: Ke, VincentX ; devel@edk2.groups.io
Cc: Chiu, Chasel ; Gao, Liming
; Dong, Eric ; Sinha, Ankit
Subject: RE: [PATCH v8] MinPlatformPkg: Update HWSignature filed in FACS
Importance: High
Hi Vincent,
This patch has merge conflicts with the newest edk2-platforms. Is this change
still needed? If yes, please rebase it to latest and re-send the patch.
Thank You,
Nate
> -Original Message-
> From: Ke, VincentX
> Sent: Friday, May 19, 2023 3:29 AM
> To: devel@edk2.groups.io
> Cc: Ke, VincentX ; Chiu, Chasel
> ; Desimone, Nathaniel L
> ; Oram, Isaac W
> ; Gao, Liming ;
> Dong, Eric ; Sinha, Ankit
> Subject: [PATCH v8] MinPlatformPkg: Update HWSignature filed in FACS
>
> From: VincentX Ke
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4428
>
> Calculating CRC based on each ACPI table.
> Update HWSignature filed in FACS based on CRC while ACPI table changed.
>
> Change-Id: Ic0ca66ff10cda0fbcd0683020fab1bc9aea9b78c
> Signed-off-by: VincentX Ke
> Cc: Chasel Chiu
> Cc: Nate DeSimone
> Cc: Isaac Oram
> Cc: Liming Gao
> Cc: Eric Dong
> Cc: Ankit Sinha
> Signed-off-by: VincentX Ke
> ---
> Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 299
> +++-
> Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf | 1 +
> 2 files changed, 233 insertions(+), 67 deletions(-)
>
> diff --git
> a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> index 2f2c96f907..5005d1a524 100644
> --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> @@ -1191,98 +1191,263 @@ PlatformUpdateTables ( }
>
>
>
> /**
>
> - This function calculates RCR based on PCI Device ID and Vendor ID
> from the devices
>
> - available on the platform.
>
> - It also includes other instances of BIOS change to calculate CRC
> and provides as
>
> - HWSignature filed in FADT table.
>
> + This function calculates CRC based on each offset in the ACPI table.
>
> +
>
> + @param[in] Table The ACPI table required to calculate CRC.
>
> +
>
> + @retval CRC A pointer to allocate UINT32 that
>
> +contains the CRC32 data.
>
> +**/
>
> +UINT32
>
> +AcpiTableCrcCalculator (
>
> + IN EFI_ACPI_COMMON_HEADER *Table
>
> + )
>
> +{
>
> + EFI_STATUS Status;
>
> + UINT32 CRC;
>
> +
>
> + Status = EFI_SUCCESS;
>
> + CRC= 0;
>
> +
>
> + //
>
> + // Calculate CRC value.
>
> + //
>
> + if (Table->Signature ==
> EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
>
> +//
>
> +// Zero HardwareSignature field before Calculating FACS CRC
>
> +//
>
> +((EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE *)Table)-
> >HardwareSignature = 0;
>
> + }
>
> +
>
> + Status = gBS->CalculateCrc32 ((UINT8 *)Table, (UINTN)Table->Length,
> &CRC);
>
> + return CRC;
>
> +}
>
> +
>
> +/**
>
> + This function count ACPI tables in RSDT/XSDT and return the result.
>
> +
>
> + @param[in] SdtACPI XSDT/RSDT.
>
> + @param[in] TablePointerSize Size of table pointer:
>
> +4(RSDT) or 8(XSDT).
>
> +
>
> + @retval TableCountThe total number of ACPI tables in
>
> +RSDT or XSDT.
>
> +**/
>
> +UINTN
>
> +CountTableInSDT (
>
> + IN EFI_ACPI_DESCRIPTION_HEADER *Sdt,
>
> + IN UINTNTablePointerSize
>
> + )
>
> +{
>
> + UINTN Index;
>
> + UINTN TableCount;
>
> + UINTN EntryCount;
>
> + UINT64 EntryPtr;
>
> + UINTN BasePtr;
>
> + EFI_ACPI_COMMON_HEADER *Table;
>
> + EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE *FadtPtr;
>
> +
>
> + EntryCount = (Sdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) /
> Tab
[edk2-devel] [PATCH v1] MinPlatformPkg: Update HWSignature filed in FADT
From: VincentX Ke
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4428
Calculating CRC based on checksum from all ACPI tables.
Update HWSignature filed in FADT based on CRC while ACPI table changed.
Signed-off-by: VincentX Ke
---
Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 110
+++-
Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf | 1 +
2 files changed, 110 insertions(+), 1 deletion(-)
diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index e967031a3b..bfc3cd0b33 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -1285,6 +1285,108 @@ IsHardwareChange (
FreePool (HWChange);
}
+/**
+ This function calculates RCR based on Checksum from ACPI tables.
+ It also calculates CRC and provides as HWSignature filed in FADT table.
+**/
+VOID
+IsAcpiTableChange (
+ VOID
+ )
+{
+ EFI_STATUSStatus;
+ UINTN Index;
+ UINTN AcpiTableCount;
+ UINT32Table;
+ UINT32CRC;
+ UINT32*AcpiTable;
+ EFI_ACPI_6_5_ROOT_SYSTEM_DESCRIPTION_POINTER *Rsdp;
+ EFI_ACPI_DESCRIPTION_HEADER *Rsdt;
+ EFI_ACPI_DESCRIPTION_HEADER *Xsdt;
+ EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE *FacsPtr;
+ EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE *pFADT;
+
+ AcpiTableCount = 0;
+ AcpiTable = NULL;
+ Rsdp = NULL;
+ Rsdt = NULL;
+ Xsdt = NULL;
+ FacsPtr= NULL;
+ pFADT = NULL;
+
+ DEBUG ((DEBUG_INFO, "%a() - Start\n", __FUNCTION__));
+
+ Status = EfiGetSystemConfigurationTable (&gEfiAcpiTableGuid, (VOID **)&Rsdp);
+ if (EFI_ERROR (Status) || (Rsdp == NULL)) {
+return;
+ }
+
+ //
+ // Count ACPI table start with 2 since RSDT and XSDT already be found.
+ // Then add ACPI tables found by XSDT and FADT.
+ //
+ Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)Rsdp->RsdtAddress;
+ Xsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)Rsdp->XsdtAddress;
+ AcpiTableCount = AcpiTableCount + 2;
+ AcpiTableCount = AcpiTableCount + (Xsdt->Length - sizeof
(EFI_ACPI_DESCRIPTION_HEADER))/sizeof (UINT64);
+
+ for (Index = sizeof (EFI_ACPI_DESCRIPTION_HEADER); Index < (Xsdt->Length);
Index = Index + sizeof (UINT64)) {
+Table = *((UINT32 *)((UINT8 *)Xsdt + Index));
+if (((EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)Table)->Signature ==
EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
+ pFADT = (EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE *)(UINTN)Table;
+ if ((pFADT->XDsdt != 0) || (pFADT->Dsdt != 0)) {
+AcpiTableCount = AcpiTableCount + 1;
+ }
+}
+ }
+
+ //
+ // Allocate memory for founded ACPI tables and add additional entries
+ //
+ AcpiTable = AllocateZeroPool (sizeof (UINT32) * AcpiTableCount);
+ if (AcpiTable == NULL) {
+return;
+ }
+
+ AcpiTableCount = 0;
+ AcpiTable[AcpiTableCount++] = Rsdt->Checksum;
+ AcpiTable[AcpiTableCount++] = Xsdt->Checksum;
+
+ for (Index = sizeof (EFI_ACPI_DESCRIPTION_HEADER); Index < (Xsdt->Length);
Index = Index + sizeof (UINT64)) {
+Table = *((UINT32 *)((UINT8 *)Xsdt + Index));
+AcpiTable[AcpiTableCount++] = ((EFI_ACPI_DESCRIPTION_HEADER
*)(UINTN)Table)->Checksum;
+if (((EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)Table)->Signature ==
EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
+ pFADT = (EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE *)(UINTN)Table;
+ if (pFADT->XDsdt != 0) {
+AcpiTable[AcpiTableCount++] = ((EFI_ACPI_DESCRIPTION_HEADER
*)(UINTN)pFADT->XDsdt)->Checksum;
+ } else {
+AcpiTable[AcpiTableCount++] = ((EFI_ACPI_DESCRIPTION_HEADER
*)(UINTN)pFADT->Dsdt)->Checksum;
+ }
+}
+ }
+
+ //
+ // pFADT table not found
+ //
+ if (pFADT == NULL) {
+return;
+ }
+
+ //
+ // Calculate CRC value with HWChange data.
+ //
+ Status = gBS->CalculateCrc32 (AcpiTable, AcpiTableCount, &CRC);
+ DEBUG ((DEBUG_INFO, "CRC = %x and Status = %r\n", CRC, Status));
+
+ //
+ // Set HardwareSignature value based on CRC value.
+ //
+ FacsPtr= (EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE
*)(UINTN)pFADT->FirmwareCtrl;
+ FacsPtr->HardwareSignature = CRC;
+ FreePool (AcpiTable);
+ DEBUG ((DEBUG_INFO, "%a() - End\n", __FUNCTION__));
+}
+
VOID
UpdateLocalTable (
VOID
@@ -1329,7 +1431,13 @@ AcpiEndOfDxeEvent (
//
// Calculate Hardware Signature value based on current platform
configurations
//
- IsHardwareChange ();
+ if ((PcdGet8 (PcdFadtM
[edk2-devel] [PATCH v2] MinPlatformPkg: Update HWSignature filed in FADT
From: VincentX Ke
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4428
Calculating CRC based on checksum from all ACPI tables.
Update HWSignature filed in FADT based on CRC while ACPI table changed.
Signed-off-by: VincentX Ke
Cc: Chasel Chiu
Cc: Nate DeSimone
Cc: Isaac Oram
Cc: Liming Gao
Cc: Eric Dong
---
Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 110
+++-
Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf | 1 +
2 files changed, 110 insertions(+), 1 deletion(-)
diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index e967031a3b..bfc3cd0b33 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -1285,6 +1285,108 @@ IsHardwareChange (
FreePool (HWChange);
}
+/**
+ This function calculates RCR based on Checksum from ACPI tables.
+ It also calculates CRC and provides as HWSignature filed in FADT table.
+**/
+VOID
+IsAcpiTableChange (
+ VOID
+ )
+{
+ EFI_STATUSStatus;
+ UINTN Index;
+ UINTN AcpiTableCount;
+ UINT32Table;
+ UINT32CRC;
+ UINT32*AcpiTable;
+ EFI_ACPI_6_5_ROOT_SYSTEM_DESCRIPTION_POINTER *Rsdp;
+ EFI_ACPI_DESCRIPTION_HEADER *Rsdt;
+ EFI_ACPI_DESCRIPTION_HEADER *Xsdt;
+ EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE *FacsPtr;
+ EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE *pFADT;
+
+ AcpiTableCount = 0;
+ AcpiTable = NULL;
+ Rsdp = NULL;
+ Rsdt = NULL;
+ Xsdt = NULL;
+ FacsPtr= NULL;
+ pFADT = NULL;
+
+ DEBUG ((DEBUG_INFO, "%a() - Start\n", __FUNCTION__));
+
+ Status = EfiGetSystemConfigurationTable (&gEfiAcpiTableGuid, (VOID **)&Rsdp);
+ if (EFI_ERROR (Status) || (Rsdp == NULL)) {
+return;
+ }
+
+ //
+ // Count ACPI table start with 2 since RSDT and XSDT already be found.
+ // Then add ACPI tables found by XSDT and FADT.
+ //
+ Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)Rsdp->RsdtAddress;
+ Xsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)Rsdp->XsdtAddress;
+ AcpiTableCount = AcpiTableCount + 2;
+ AcpiTableCount = AcpiTableCount + (Xsdt->Length - sizeof
(EFI_ACPI_DESCRIPTION_HEADER))/sizeof (UINT64);
+
+ for (Index = sizeof (EFI_ACPI_DESCRIPTION_HEADER); Index < (Xsdt->Length);
Index = Index + sizeof (UINT64)) {
+Table = *((UINT32 *)((UINT8 *)Xsdt + Index));
+if (((EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)Table)->Signature ==
EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
+ pFADT = (EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE *)(UINTN)Table;
+ if ((pFADT->XDsdt != 0) || (pFADT->Dsdt != 0)) {
+AcpiTableCount = AcpiTableCount + 1;
+ }
+}
+ }
+
+ //
+ // Allocate memory for founded ACPI tables and add additional entries
+ //
+ AcpiTable = AllocateZeroPool (sizeof (UINT32) * AcpiTableCount);
+ if (AcpiTable == NULL) {
+return;
+ }
+
+ AcpiTableCount = 0;
+ AcpiTable[AcpiTableCount++] = Rsdt->Checksum;
+ AcpiTable[AcpiTableCount++] = Xsdt->Checksum;
+
+ for (Index = sizeof (EFI_ACPI_DESCRIPTION_HEADER); Index < (Xsdt->Length);
Index = Index + sizeof (UINT64)) {
+Table = *((UINT32 *)((UINT8 *)Xsdt + Index));
+AcpiTable[AcpiTableCount++] = ((EFI_ACPI_DESCRIPTION_HEADER
*)(UINTN)Table)->Checksum;
+if (((EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)Table)->Signature ==
EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
+ pFADT = (EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE *)(UINTN)Table;
+ if (pFADT->XDsdt != 0) {
+AcpiTable[AcpiTableCount++] = ((EFI_ACPI_DESCRIPTION_HEADER
*)(UINTN)pFADT->XDsdt)->Checksum;
+ } else {
+AcpiTable[AcpiTableCount++] = ((EFI_ACPI_DESCRIPTION_HEADER
*)(UINTN)pFADT->Dsdt)->Checksum;
+ }
+}
+ }
+
+ //
+ // pFADT table not found
+ //
+ if (pFADT == NULL) {
+return;
+ }
+
+ //
+ // Calculate CRC value with HWChange data.
+ //
+ Status = gBS->CalculateCrc32 (AcpiTable, AcpiTableCount, &CRC);
+ DEBUG ((DEBUG_INFO, "CRC = %x and Status = %r\n", CRC, Status));
+
+ //
+ // Set HardwareSignature value based on CRC value.
+ //
+ FacsPtr= (EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE
*)(UINTN)pFADT->FirmwareCtrl;
+ FacsPtr->HardwareSignature = CRC;
+ FreePool (AcpiTable);
+ DEBUG ((DEBUG_INFO, "%a() - End\n", __FUNCTION__));
+}
+
VOID
UpdateLocalTable (
VOID
@@ -1329,7 +1431,13 @@ AcpiEndOfDxeEvent (
//
// Calculate Hardware Signature value
[edk2-devel] [PATCH v3] MinPlatformPkg: Update HWSignature filed in FADT
From: VincentX Ke
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4428
Calculating CRC based on checksum from all ACPI tables.
Update HWSignature filed in FADT based on CRC while ACPI table changed.
Signed-off-by: VincentX Ke
Cc: Chasel Chiu
Cc: Nate DeSimone
Cc: Isaac Oram
Cc: Liming Gao
Cc: Eric Dong
---
Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 110
+++-
Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf | 1 +
2 files changed, 110 insertions(+), 1 deletion(-)
diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index e967031a3b..d84c1d4f6d 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -1285,6 +1285,108 @@ IsHardwareChange (
FreePool (HWChange);
}
+/**
+ This function calculates RCR based on Checksum from all ACPI tables.
+ It also calculates CRC and provides as HWSignature filed in FADT table.
+**/
+VOID
+IsAcpiTableChange (
+ VOID
+ )
+{
+ EFI_STATUSStatus;
+ UINTN Index;
+ UINTN AcpiTableCount;
+ UINT32Table;
+ UINT32CRC;
+ UINT32*AcpiTable;
+ EFI_ACPI_6_5_ROOT_SYSTEM_DESCRIPTION_POINTER *Rsdp;
+ EFI_ACPI_DESCRIPTION_HEADER *Rsdt;
+ EFI_ACPI_DESCRIPTION_HEADER *Xsdt;
+ EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE *FacsPtr;
+ EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE *pFADT;
+
+ AcpiTableCount = 0;
+ AcpiTable = NULL;
+ Rsdp = NULL;
+ Rsdt = NULL;
+ Xsdt = NULL;
+ FacsPtr= NULL;
+ pFADT = NULL;
+
+ DEBUG ((DEBUG_INFO, "%a() - Start\n", __FUNCTION__));
+
+ Status = EfiGetSystemConfigurationTable (&gEfiAcpiTableGuid, (VOID **)&Rsdp);
+ if (EFI_ERROR (Status) || (Rsdp == NULL)) {
+return;
+ }
+
+ //
+ // ACPI table count starts with 2 as RSDT and XSDT are already located.
+ // Then add ACPI tables found by XSDT and FADT.
+ //
+ Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)Rsdp->RsdtAddress;
+ Xsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)Rsdp->XsdtAddress;
+ AcpiTableCount = AcpiTableCount + 2;
+ AcpiTableCount = AcpiTableCount + (Xsdt->Length - sizeof
(EFI_ACPI_DESCRIPTION_HEADER))/sizeof (UINT64);
+
+ for (Index = sizeof (EFI_ACPI_DESCRIPTION_HEADER); Index < (Xsdt->Length);
Index = Index + sizeof (UINT64)) {
+Table = *((UINT32 *)((UINT8 *)Xsdt + Index));
+if (((EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)Table)->Signature ==
EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
+ pFADT = (EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE *)(UINTN)Table;
+ if ((pFADT->XDsdt != 0) || (pFADT->Dsdt != 0)) {
+AcpiTableCount = AcpiTableCount + 1;
+ }
+}
+ }
+
+ //
+ // Allocate memory for founded ACPI tables.
+ //
+ AcpiTable = AllocateZeroPool (sizeof (UINT32) * AcpiTableCount);
+ if (AcpiTable == NULL) {
+return;
+ }
+
+ AcpiTableCount = 0;
+ AcpiTable[AcpiTableCount++] = Rsdt->Checksum;
+ AcpiTable[AcpiTableCount++] = Xsdt->Checksum;
+
+ for (Index = sizeof (EFI_ACPI_DESCRIPTION_HEADER); Index < (Xsdt->Length);
Index = Index + sizeof (UINT64)) {
+Table = *((UINT32 *)((UINT8 *)Xsdt + Index));
+AcpiTable[AcpiTableCount++] = ((EFI_ACPI_DESCRIPTION_HEADER
*)(UINTN)Table)->Checksum;
+if (((EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)Table)->Signature ==
EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
+ pFADT = (EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE *)(UINTN)Table;
+ if (pFADT->XDsdt != 0) {
+AcpiTable[AcpiTableCount++] = ((EFI_ACPI_DESCRIPTION_HEADER
*)(UINTN)pFADT->XDsdt)->Checksum;
+ } else {
+AcpiTable[AcpiTableCount++] = ((EFI_ACPI_DESCRIPTION_HEADER
*)(UINTN)pFADT->Dsdt)->Checksum;
+ }
+}
+ }
+
+ //
+ // pFADT table not found.
+ //
+ if (pFADT == NULL) {
+return;
+ }
+
+ //
+ // Calculate CRC value.
+ //
+ Status = gBS->CalculateCrc32 (AcpiTable, AcpiTableCount, &CRC);
+ DEBUG ((DEBUG_INFO, "CRC = %x and Status = %r\n", CRC, Status));
+
+ //
+ // Set HardwareSignature value based on CRC value.
+ //
+ FacsPtr= (EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE
*)(UINTN)pFADT->FirmwareCtrl;
+ FacsPtr->HardwareSignature = CRC;
+ FreePool (AcpiTable);
+ DEBUG ((DEBUG_INFO, "%a() - End\n", __FUNCTION__));
+}
+
VOID
UpdateLocalTable (
VOID
@@ -1329,7 +1431,13 @@ AcpiEndOfDxeEvent (
//
// Calculate Hardware Signature value based on current platform
configurations
//
- IsHar
Re: [edk2-devel] [PATCH v3] MinPlatformPkg: Update HWSignature filed in FADT
Hi, Ray
Based on ACPI Spec 6.5, "Hardware Signature" filed changed the description.
Here is the new description.
"The only thing that determines the hardware signature is the ACPI tables.
If any content or structure of the ACPI tables has changed,
including adding or removing of tables, then the hardware signature must
change."
I keep version 6.3 only for Intel code base test. It should be updated to 6.4.
Thanks for the reminder.
BR,
Vincent
-Original Message-
From: Ni, Ray
Sent: Friday, April 28, 2023 11:08 AM
To: devel@edk2.groups.io; Ke, VincentX
Cc: Chiu, Chasel ; Desimone, Nathaniel L
; Oram, Isaac W ; Gao,
Liming ; Dong, Eric
Subject: RE: [edk2-devel] [PATCH v3] MinPlatformPkg: Update HWSignature filed
in FADT
Vincent,
It's an interesting patch.
The original logic is to use the HardwareSignature field to indicate any
changes in adding/removing PCI devices.
Your new logic is to expand this field to indicate any changes in any tables
when FADT version is > 6.3.
Why?
> -Original Message-
> From: devel@edk2.groups.io On Behalf Of
> VincentX Ke
> Sent: Friday, April 28, 2023 10:57 AM
> To: devel@edk2.groups.io
> Cc: Ke, VincentX ; Chiu, Chasel
> ; Desimone, Nathaniel L
> ; Oram, Isaac W
> ; Gao, Liming ;
> Dong, Eric
> Subject: [edk2-devel] [PATCH v3] MinPlatformPkg: Update HWSignature
> filed in FADT
>
> From: VincentX Ke
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4428
>
> Calculating CRC based on checksum from all ACPI tables.
> Update HWSignature filed in FADT based on CRC while ACPI table changed.
>
> Signed-off-by: VincentX Ke
> Cc: Chasel Chiu
> Cc: Nate DeSimone
> Cc: Isaac Oram
> Cc: Liming Gao
> Cc: Eric Dong
> ---
> Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 110
> +++-
> Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf | 1 +
> 2 files changed, 110 insertions(+), 1 deletion(-)
>
> diff --git
> a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> index e967031a3b..d84c1d4f6d 100644
> --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> @@ -1285,6 +1285,108 @@ IsHardwareChange (
>FreePool (HWChange);
>
> }
>
>
>
> +/**
>
> + This function calculates RCR based on Checksum from all ACPI tables.
>
> + It also calculates CRC and provides as HWSignature filed in FADT table.
>
> +**/
>
> +VOID
>
> +IsAcpiTableChange (
>
> + VOID
>
> + )
>
> +{
>
> + EFI_STATUSStatus;
>
> + UINTN Index;
>
> + UINTN AcpiTableCount;
>
> + UINT32Table;
>
> + UINT32CRC;
>
> + UINT32*AcpiTable;
>
> + EFI_ACPI_6_5_ROOT_SYSTEM_DESCRIPTION_POINTER *Rsdp;
>
> + EFI_ACPI_DESCRIPTION_HEADER *Rsdt;
>
> + EFI_ACPI_DESCRIPTION_HEADER *Xsdt;
>
> + EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE *FacsPtr;
>
> + EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE *pFADT;
>
> +
>
> + AcpiTableCount = 0;
>
> + AcpiTable = NULL;
>
> + Rsdp = NULL;
>
> + Rsdt = NULL;
>
> + Xsdt = NULL;
>
> + FacsPtr= NULL;
>
> + pFADT = NULL;
>
> +
>
> + DEBUG ((DEBUG_INFO, "%a() - Start\n", __FUNCTION__));
>
> +
>
> + Status = EfiGetSystemConfigurationTable (&gEfiAcpiTableGuid, (VOID
> **)&Rsdp);
>
> + if (EFI_ERROR (Status) || (Rsdp == NULL)) {
>
> +return;
>
> + }
>
> +
>
> + //
>
> + // ACPI table count starts with 2 as RSDT and XSDT are already located.
>
> + // Then add ACPI tables found by XSDT and FADT.
>
> + //
>
> + Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)Rsdp-
> >RsdtAddress;
>
> + Xsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)Rsdp-
> >XsdtAddress;
>
> + AcpiTableCount = AcpiTableCount + 2;
>
> + AcpiTableCount = AcpiTableCount + (Xsdt->Length - sizeof
> (EFI_ACPI_DESCRIPTION_HEADER))/sizeof (UINT64);
>
> +
>
> + for (Index = sizeof (EFI_ACPI_DESCRIPTION_HEADER); Index < (Xsdt-
> >Length); Index = Index + sizeof (UINT64)) {
>
> +Table = *((UINT32 *)((UINT8 *)Xsdt + Index));
>
> +if (((EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)Table)->Sig
[edk2-devel] [PATCH v4] MinPlatformPkg: Update HWSignature filed in FADT
From: VincentX Ke
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4428
Calculating CRC based on checksum from all ACPI tables.
Update HWSignature filed in FADT based on CRC while ACPI table changed.
Signed-off-by: VincentX Ke
Cc: Chasel Chiu
Cc: Nate DeSimone
Cc: Isaac Oram
Cc: Liming Gao
Cc: Eric Dong
---
Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 110
+++-
Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf | 1 +
2 files changed, 110 insertions(+), 1 deletion(-)
diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index e967031a3b..716b24d6e7 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -1285,6 +1285,108 @@ IsHardwareChange (
FreePool (HWChange);
}
+/**
+ This function calculates RCR based on Checksum from all ACPI tables.
+ It also calculates CRC and provides as HWSignature filed in FADT table.
+**/
+VOID
+IsAcpiTableChange (
+ VOID
+ )
+{
+ EFI_STATUSStatus;
+ UINTN Index;
+ UINTN AcpiTableCount;
+ UINT32Table;
+ UINT32CRC;
+ UINT32*AcpiTable;
+ EFI_ACPI_6_5_ROOT_SYSTEM_DESCRIPTION_POINTER *Rsdp;
+ EFI_ACPI_DESCRIPTION_HEADER *Rsdt;
+ EFI_ACPI_DESCRIPTION_HEADER *Xsdt;
+ EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE *FacsPtr;
+ EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE *pFADT;
+
+ AcpiTableCount = 0;
+ AcpiTable = NULL;
+ Rsdp = NULL;
+ Rsdt = NULL;
+ Xsdt = NULL;
+ FacsPtr= NULL;
+ pFADT = NULL;
+
+ DEBUG ((DEBUG_INFO, "%a() - Start\n", __FUNCTION__));
+
+ Status = EfiGetSystemConfigurationTable (&gEfiAcpiTableGuid, (VOID **)&Rsdp);
+ if (EFI_ERROR (Status) || (Rsdp == NULL)) {
+return;
+ }
+
+ //
+ // ACPI table count starts with 2 as RSDT and XSDT are already located.
+ // Then add ACPI tables found by XSDT and FADT.
+ //
+ Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)Rsdp->RsdtAddress;
+ Xsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)Rsdp->XsdtAddress;
+ AcpiTableCount = AcpiTableCount + 2;
+ AcpiTableCount = AcpiTableCount + (Xsdt->Length - sizeof
(EFI_ACPI_DESCRIPTION_HEADER))/sizeof (UINT64);
+
+ for (Index = sizeof (EFI_ACPI_DESCRIPTION_HEADER); Index < (Xsdt->Length);
Index = Index + sizeof (UINT64)) {
+Table = *((UINT32 *)((UINT8 *)Xsdt + Index));
+if (((EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)Table)->Signature ==
EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
+ pFADT = (EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE *)(UINTN)Table;
+ if ((pFADT->XDsdt != 0) || (pFADT->Dsdt != 0)) {
+AcpiTableCount = AcpiTableCount + 1;
+ }
+}
+ }
+
+ //
+ // Allocate memory for founded ACPI tables.
+ //
+ AcpiTable = AllocateZeroPool (sizeof (UINT32) * AcpiTableCount);
+ if (AcpiTable == NULL) {
+return;
+ }
+
+ AcpiTableCount = 0;
+ AcpiTable[AcpiTableCount++] = Rsdt->Checksum;
+ AcpiTable[AcpiTableCount++] = Xsdt->Checksum;
+
+ for (Index = sizeof (EFI_ACPI_DESCRIPTION_HEADER); Index < (Xsdt->Length);
Index = Index + sizeof (UINT64)) {
+Table = *((UINT32 *)((UINT8 *)Xsdt + Index));
+AcpiTable[AcpiTableCount++] = ((EFI_ACPI_DESCRIPTION_HEADER
*)(UINTN)Table)->Checksum;
+if (((EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)Table)->Signature ==
EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
+ pFADT = (EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE *)(UINTN)Table;
+ if (pFADT->XDsdt != 0) {
+AcpiTable[AcpiTableCount++] = ((EFI_ACPI_DESCRIPTION_HEADER
*)(UINTN)pFADT->XDsdt)->Checksum;
+ } else {
+AcpiTable[AcpiTableCount++] = ((EFI_ACPI_DESCRIPTION_HEADER
*)(UINTN)pFADT->Dsdt)->Checksum;
+ }
+}
+ }
+
+ //
+ // pFADT table not found.
+ //
+ if (pFADT == NULL) {
+return;
+ }
+
+ //
+ // Calculate CRC value.
+ //
+ Status = gBS->CalculateCrc32 (AcpiTable, AcpiTableCount, &CRC);
+ DEBUG ((DEBUG_INFO, "CRC = %x and Status = %r\n", CRC, Status));
+
+ //
+ // Set HardwareSignature value based on CRC value.
+ //
+ FacsPtr= (EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE
*)(UINTN)pFADT->FirmwareCtrl;
+ FacsPtr->HardwareSignature = CRC;
+ FreePool (AcpiTable);
+ DEBUG ((DEBUG_INFO, "%a() - End\n", __FUNCTION__));
+}
+
VOID
UpdateLocalTable (
VOID
@@ -1329,7 +1431,13 @@ AcpiEndOfDxeEvent (
//
// Calculate Hardware Signature value based on current platform
configurations
//
- IsHar
Re: [edk2-devel] [PATCH v3] MinPlatformPkg: Update HWSignature filed in FADT
Dear All,
Patch v4 updated. Please let me know if there is any suggestion. Thanks
[PATCH v4] https://edk2.groups.io/g/devel/message/103738
BR,
Vincent
-Original Message-
From: Ke, VincentX
Sent: Friday, April 28, 2023 11:30 AM
To: Ni, Ray ; devel@edk2.groups.io
Cc: Chiu, Chasel ; Desimone, Nathaniel L
; Oram, Isaac W ; Gao,
Liming ; Dong, Eric
Subject: RE: [edk2-devel] [PATCH v3] MinPlatformPkg: Update HWSignature filed
in FADT
Hi, Ray
Based on ACPI Spec 6.5, "Hardware Signature" filed changed the description.
Here is the new description.
"The only thing that determines the hardware signature is the ACPI tables.
If any content or structure of the ACPI tables has changed, including adding or
removing of tables, then the hardware signature must change."
I keep version 6.3 only for Intel code base test. It should be updated to 6.4.
Thanks for the reminder.
BR,
Vincent
-Original Message-
From: Ni, Ray
Sent: Friday, April 28, 2023 11:08 AM
To: devel@edk2.groups.io; Ke, VincentX
Cc: Chiu, Chasel ; Desimone, Nathaniel L
; Oram, Isaac W ; Gao,
Liming ; Dong, Eric
Subject: RE: [edk2-devel] [PATCH v3] MinPlatformPkg: Update HWSignature filed
in FADT
Vincent,
It's an interesting patch.
The original logic is to use the HardwareSignature field to indicate any
changes in adding/removing PCI devices.
Your new logic is to expand this field to indicate any changes in any tables
when FADT version is > 6.3.
Why?
> -Original Message-
> From: devel@edk2.groups.io On Behalf Of
> VincentX Ke
> Sent: Friday, April 28, 2023 10:57 AM
> To: devel@edk2.groups.io
> Cc: Ke, VincentX ; Chiu, Chasel
> ; Desimone, Nathaniel L
> ; Oram, Isaac W
> ; Gao, Liming ;
> Dong, Eric
> Subject: [edk2-devel] [PATCH v3] MinPlatformPkg: Update HWSignature
> filed in FADT
>
> From: VincentX Ke
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4428
>
> Calculating CRC based on checksum from all ACPI tables.
> Update HWSignature filed in FADT based on CRC while ACPI table changed.
>
> Signed-off-by: VincentX Ke
> Cc: Chasel Chiu
> Cc: Nate DeSimone
> Cc: Isaac Oram
> Cc: Liming Gao
> Cc: Eric Dong
> ---
> Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 110
> +++-
> Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf | 1 +
> 2 files changed, 110 insertions(+), 1 deletion(-)
>
> diff --git
> a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> index e967031a3b..d84c1d4f6d 100644
> --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> @@ -1285,6 +1285,108 @@ IsHardwareChange (
>FreePool (HWChange);
>
> }
>
>
>
> +/**
>
> + This function calculates RCR based on Checksum from all ACPI tables.
>
> + It also calculates CRC and provides as HWSignature filed in FADT table.
>
> +**/
>
> +VOID
>
> +IsAcpiTableChange (
>
> + VOID
>
> + )
>
> +{
>
> + EFI_STATUSStatus;
>
> + UINTN Index;
>
> + UINTN AcpiTableCount;
>
> + UINT32Table;
>
> + UINT32CRC;
>
> + UINT32*AcpiTable;
>
> + EFI_ACPI_6_5_ROOT_SYSTEM_DESCRIPTION_POINTER *Rsdp;
>
> + EFI_ACPI_DESCRIPTION_HEADER *Rsdt;
>
> + EFI_ACPI_DESCRIPTION_HEADER *Xsdt;
>
> + EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE *FacsPtr;
>
> + EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE *pFADT;
>
> +
>
> + AcpiTableCount = 0;
>
> + AcpiTable = NULL;
>
> + Rsdp = NULL;
>
> + Rsdt = NULL;
>
> + Xsdt = NULL;
>
> + FacsPtr= NULL;
>
> + pFADT = NULL;
>
> +
>
> + DEBUG ((DEBUG_INFO, "%a() - Start\n", __FUNCTION__));
>
> +
>
> + Status = EfiGetSystemConfigurationTable (&gEfiAcpiTableGuid, (VOID
> **)&Rsdp);
>
> + if (EFI_ERROR (Status) || (Rsdp == NULL)) {
>
> +return;
>
> + }
>
> +
>
> + //
>
> + // ACPI table count starts with 2 as RSDT and XSDT are already located.
>
> + // Then add ACPI tables found by XSDT and FADT.
>
> + //
>
> + Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)Rsdp-
> >RsdtAddress;
>
> + Xsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UIN
[edk2-devel] [PATCH v5] MinPlatformPkg: Update HWSignature filed in FACS
From: VincentX Ke
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4428
Calculating CRC based on each ACPI table.
Update HWSignature filed in FACS based on CRC while ACPI table changed.
Change-Id: Ic0ca66ff10cda0fbcd0683020fab1bc9aea9b78c
Signed-off-by: VincentX Ke
Cc: Chasel Chiu
Cc: Nate DeSimone
Cc: Isaac Oram
Cc: Liming Gao
Cc: Eric Dong
Signed-off-by: VincentX Ke
---
Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 182
+---
Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf | 1 +
2 files changed, 118 insertions(+), 65 deletions(-)
diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index e967031a3b..a940424ced 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -1191,98 +1191,150 @@ PlatformUpdateTables (
}
/**
- This function calculates RCR based on PCI Device ID and Vendor ID from the
devices
- available on the platform.
- It also includes other instances of BIOS change to calculate CRC and
provides as
- HWSignature filed in FADT table.
+ This function calculates CRC based on each offset in the ACPI table.
+
+ @param[in] Table The pointer to ACPI table that
+required to calculate CRC.
+
+ @retval CRC A pointer to allocate UINT32 that
+contains the CRC32 data.
+**/
+UINT32
+AcpiTableCrcCalculator (
+ IN VOID *Table
+ )
+{
+ UINT32 CRC;
+
+ CRC = 0;
+
+ //
+ // Calculate CRC value.
+ //
+ if (((EFI_ACPI_6_5_COMMON_HEADER *)(UINTN)Table)->Signature ==
EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
+//
+// Zero HardwareSignature field before Calculating FACS CRC
+//
+do {
+ ((EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE
*)(UINTN)Table)->HardwareSignature = 0;
+} while (((EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE
*)(UINTN)Table)->HardwareSignature);
+
+gBS->CalculateCrc32 ((UINT8 *)Table,
(UINTN)((EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE *)(UINTN)Table)->Length,
&CRC);
+ } else {
+gBS->CalculateCrc32 ((UINT8 *)Table, (UINTN)((EFI_ACPI_DESCRIPTION_HEADER
*)(UINTN)Table)->Length, &CRC);
+ }
+
+ return CRC;
+}
+
+/**
+ This function calculates CRC based on each ACPI table.
+ It also calculates CRC and provides as HWSignature filed in FACS.
**/
VOID
-IsHardwareChange (
+IsAcpiTableChange (
VOID
)
{
- EFI_STATUSStatus;
- UINTN Index;
- UINTN HandleCount;
- EFI_HANDLE*HandleBuffer;
- EFI_PCI_IO_PROTOCOL *PciIo;
- UINT32CRC;
- UINT32*HWChange;
- UINTN HWChangeSize;
- UINT32PciId;
- UINTN Handle;
- EFI_ACPI_6_3_FIRMWARE_ACPI_CONTROL_STRUCTURE *FacsPtr;
- EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE*pFADT;
-
- HandleCount = 0;
- HandleBuffer = NULL;
-
- Status = gBS->LocateHandleBuffer (
- ByProtocol,
- &gEfiPciIoProtocolGuid,
- NULL,
- &HandleCount,
- &HandleBuffer
- );
- if (EFI_ERROR (Status)) {
-return; // PciIO protocol not installed yet!
+ EFI_STATUSStatus;
+ UINTN Index;
+ UINTN AcpiTableCount;
+ UINT32Table;
+ UINT32HWSignature;
+ UINT32*AcpiTableCrc;
+ EFI_ACPI_6_5_ROOT_SYSTEM_DESCRIPTION_POINTER *Rsdp;
+ EFI_ACPI_DESCRIPTION_HEADER *Rsdt;
+ EFI_ACPI_DESCRIPTION_HEADER *Xsdt;
+ EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE *FacsPtr;
+ EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE *pFADT;
+
+ AcpiTableCount = 0;
+ AcpiTableCrc = NULL;
+ Rsdp = NULL;
+ Rsdt = NULL;
+ Xsdt = NULL;
+ FacsPtr= NULL;
+ pFADT = NULL;
+
+ DEBUG ((DEBUG_INFO, "%a() - Start\n", __FUNCTION__));
+
+ Status = EfiGetSystemConfigurationTable (&gEfiAcpiTableGuid, (VOID **)&Rsdp);
+ if (EFI_ERROR (Status) || (Rsdp == NULL)) {
+return;
}
//
- // Allocate memory for HWChange and add additional entrie for
- // pFADT->XDsdt
+ // ACPI table count starts with 2 as RSDT and XSDT are already located.
+ // Then add ACPI tables found by XSDT and FADT.
//
- HWChangeSize = HandleCount + 1;
- HWChange = AllocateZeroPool (sizeof(UINT32) * HWChangeSize);
- ASSERT(HWChange != NULL);
+ Rsdt= (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)Rsdp->RsdtAddress;
+ Xsdt
Re: [edk2-devel] [PATCH v3] MinPlatformPkg: Update HWSignature filed in FADT
Hi, Ray
Update patch and fix your concern in the patch v5.
Please let me know if there is any suggestion. Thanks.
[Patch v5] https://edk2.groups.io/g/devel/message/103877
BR,
Vincent
-Original Message-
From: Ni, Ray
Sent: Friday, April 28, 2023 11:55 AM
To: Ke, VincentX ; devel@edk2.groups.io
Cc: Chiu, Chasel ; Desimone, Nathaniel L
; Oram, Isaac W ; Gao,
Liming ; Dong, Eric
Subject: RE: [edk2-devel] [PATCH v3] MinPlatformPkg: Update HWSignature filed
in FADT
RSDP points to RSDT or XSDT.
RSDT/XSDT contains multiple tables, one of which is FADT, others are SSDTs.
FADT contains two pointers, one pointing to FACS, the other pointing to DSDT.
So:
1. Your code assume if Table->Checksum is not changed, the Table is not
changed. I don't think it's a valid assumption.
2. Your code "measures" the DSDT change which is good. (still only reading
Checksum is not good.) 3. FACS table is not "measured".
Thanks,
Ray
> -Original Message-
> From: Ke, VincentX
> Sent: Friday, April 28, 2023 11:30 AM
> To: Ni, Ray ; devel@edk2.groups.io
> Cc: Chiu, Chasel ; Desimone, Nathaniel L
> ; Oram, Isaac W
> ; Gao, Liming ;
> Dong, Eric
> Subject: RE: [edk2-devel] [PATCH v3] MinPlatformPkg: Update
> HWSignature filed in FADT
>
> Hi, Ray
>
> Based on ACPI Spec 6.5, "Hardware Signature" filed changed the description.
>
> Here is the new description.
> "The only thing that determines the hardware signature is the ACPI tables.
> If any content or structure of the ACPI tables has changed, including
> adding or removing of tables, then the hardware signature must
> change."
>
> I keep version 6.3 only for Intel code base test. It should be updated to 6.4.
> Thanks for the reminder.
>
> BR,
> Vincent
> -Original Message-
> From: Ni, Ray
> Sent: Friday, April 28, 2023 11:08 AM
> To: devel@edk2.groups.io; Ke, VincentX
> Cc: Chiu, Chasel ; Desimone, Nathaniel L
> ; Oram, Isaac W
> ; Gao, Liming ;
> Dong, Eric
> Subject: RE: [edk2-devel] [PATCH v3] MinPlatformPkg: Update
> HWSignature filed in FADT
>
> Vincent,
> It's an interesting patch.
>
> The original logic is to use the HardwareSignature field to indicate
> any changes in adding/removing PCI devices.
> Your new logic is to expand this field to indicate any changes in any
> tables when FADT version is > 6.3.
>
> Why?
>
>
> > -Original Message-
> > From: devel@edk2.groups.io On Behalf Of
> > VincentX Ke
> > Sent: Friday, April 28, 2023 10:57 AM
> > To: devel@edk2.groups.io
> > Cc: Ke, VincentX ; Chiu, Chasel
> > ; Desimone, Nathaniel L
> > ; Oram, Isaac W
> > ; Gao, Liming ;
> > Dong, Eric
> > Subject: [edk2-devel] [PATCH v3] MinPlatformPkg: Update HWSignature
> > filed in FADT
> >
> > From: VincentX Ke
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4428
> >
> > Calculating CRC based on checksum from all ACPI tables.
> > Update HWSignature filed in FADT based on CRC while ACPI table changed.
> >
> > Signed-off-by: VincentX Ke
> > Cc: Chasel Chiu
> > Cc: Nate DeSimone
> > Cc: Isaac Oram
> > Cc: Liming Gao
> > Cc: Eric Dong
> > ---
> > Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 110
> > +++-
> > Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf | 1 +
> > 2 files changed, 110 insertions(+), 1 deletion(-)
> >
> > diff --git
> > a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> > b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> > index e967031a3b..d84c1d4f6d 100644
> > --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> > +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> > @@ -1285,6 +1285,108 @@ IsHardwareChange (
> >FreePool (HWChange);
> >
> > }
> >
> >
> >
> > +/**
> >
> > + This function calculates RCR based on Checksum from all ACPI tables.
> >
> > + It also calculates CRC and provides as HWSignature filed in FADT table.
> >
> > +**/
> >
> > +VOID
> >
> > +IsAcpiTableChange (
> >
> > + VOID
> >
> > + )
> >
> > +{
> >
> > + EFI_STATUSStatus;
> >
> > + UINTN Index;
> >
> > + UINTN AcpiTableCount;
> >
> > + UINT32Table;
> >
> > + UINT32CRC;
[edk2-devel] [PATCH v6] MinPlatformPkg: Update HWSignature filed in FACS
From: VincentX Ke
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4428
Calculating CRC based on each ACPI table.
Update HWSignature filed in FACS based on CRC while ACPI table changed.
Change-Id: Ic0ca66ff10cda0fbcd0683020fab1bc9aea9b78c
Signed-off-by: VincentX Ke
Cc: Chasel Chiu
Cc: Nate DeSimone
Cc: Isaac Oram
Cc: Liming Gao
Cc: Eric Dong
Cc: Ankit Sinha
Signed-off-by: VincentX Ke
---
Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 271
+++-
Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf | 1 +
2 files changed, 209 insertions(+), 63 deletions(-)
diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index e967031a3b..bb0f4a1f04 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -1191,98 +1191,243 @@ PlatformUpdateTables (
}
/**
- This function calculates RCR based on PCI Device ID and Vendor ID from the
devices
- available on the platform.
- It also includes other instances of BIOS change to calculate CRC and
provides as
- HWSignature filed in FADT table.
+ This function calculates CRC based on each offset in the ACPI table.
+
+ @param[in] Table The ACPI table required to calculate CRC.
+
+ @retval CRC A pointer to allocate UINT32 that
+contains the CRC32 data.
+**/
+UINT32
+AcpiTableCrcCalculator (
+ IN EFI_ACPI_COMMON_HEADER *Table
+ )
+{
+ EFI_STATUS Status;
+ UINT32 CRC;
+
+ Status = EFI_SUCCESS;
+ CRC= 0;
+
+ //
+ // Calculate CRC value.
+ //
+ if (Table->Signature ==
EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
+//
+// Zero HardwareSignature field before Calculating FACS CRC
+//
+((EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE *)Table)->HardwareSignature
= 0;
+ }
+
+ Status = gBS->CalculateCrc32 ((UINT8 *)Table, (UINTN)Table->Length, &CRC);
+ return CRC;
+}
+
+/**
+ This function count ACPI tables in RSDT/XSDT and return the result.
+
+ @param[in] SdtACPI XSDT/RSDT.
+ @param[in] TablePointerSize Size of table pointer:
+4(RSDT) or 8(XSDT).
+
+ @retval TableCountThe total number of ACPI tables in
+RSDT or XSDT.
+**/
+UINTN
+CountTableInSDT (
+ IN EFI_ACPI_DESCRIPTION_HEADER *Sdt,
+ IN UINTNTablePointerSize
+ )
+{
+ UINTN Index;
+ UINTN TableCount;
+ UINTN EntryCount;
+ UINT64 EntryPtr;
+ UINTN BasePtr;
+ EFI_ACPI_COMMON_HEADER *Table;
+
+ EntryCount = (Sdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) /
TablePointerSize;
+ BasePtr= (UINTN)(Sdt + 1);
+
+ for (Index = 0, TableCount = 0; Index < EntryCount; Index++) {
+EntryPtr = 0;
+Table= NULL;
+CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * TablePointerSize),
TablePointerSize);
+Table = (EFI_ACPI_COMMON_HEADER *)((UINTN)(EntryPtr));
+if (Table) {
+ TableCount++;
+}
+
+if (Table->Signature ==
EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
+ CopyMem ((VOID *)&Fadt, (VOID *)Table, sizeof
(EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE));
+ if (Fadt.FirmwareCtrl || Fadt.XFirmwareCtrl) {
+TableCount++;
+ }
+
+ if (Fadt.Dsdt || Fadt.XDsdt) {
+TableCount++;
+ }
+}
+ }
+
+ return TableCount;
+}
+
+/**
+ This function calculates CRC based on each ACPI table.
+ It also calculates CRC and provides as HWSignature filed in FACS.
**/
VOID
-IsHardwareChange (
+IsAcpiTableChange (
VOID
)
{
- EFI_STATUSStatus;
- UINTN Index;
- UINTN HandleCount;
- EFI_HANDLE*HandleBuffer;
- EFI_PCI_IO_PROTOCOL *PciIo;
- UINT32CRC;
- UINT32*HWChange;
- UINTN HWChangeSize;
- UINT32PciId;
- UINTN Handle;
- EFI_ACPI_6_3_FIRMWARE_ACPI_CONTROL_STRUCTURE *FacsPtr;
- EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE*pFADT;
-
- HandleCount = 0;
- HandleBuffer = NULL;
-
- Status = gBS->LocateHandleBuffer (
- ByProtocol,
- &gEfiPciIoProtocolGuid,
- NULL,
- &HandleCount,
- &HandleBuffer
- );
- if (EFI_ERROR (Status)) {
-return; // PciIO protocol not installed yet!
+ EFI_STATUSStatus;
+ UINTN Index;
+ UINTN AcpiTableCount;
+ UINTN EntryCount;
+ UINTN
Re: [edk2-devel] [PATCH v5] MinPlatformPkg: Update HWSignature filed in FACS
Hi, Ray
To fix the concern you mentioned.
[Patch V6] updated with https://edk2.groups.io/g/devel/message/104537
Please kindly code review and let me know your suggestion.
Thanks a lot.
Vincent
-Original Message-
From: Ni, Ray
Sent: Monday, May 8, 2023 11:23 AM
To: devel@edk2.groups.io; Ke, VincentX
Cc: Chiu, Chasel ; Desimone, Nathaniel L
; Oram, Isaac W ; Gao,
Liming ; Dong, Eric ; Sinha,
Ankit
Subject: RE: [edk2-devel] [PATCH v5] MinPlatformPkg: Update HWSignature filed
in FACS
Vincent,
I don't quite understand your code logic.
For example:
1. Why you use XSDT only? What if there is only RSDT?
2. Why there is a do-while in AcpiTableCrcCalculator()?
3. Why (VOID *) is converted to (UINTN) then to (EFI_ACPI_DESCRIPTION_HEADER*)?
4. Why you "measure" the XSDT and RSDT table? The two tables only contain
pointers.
5. Can you please review " MdePkg/Library/UefiLib/Acpi.c" which contains logic
that iterates the acpi tables?
Thanks,
Ray
> -Original Message-
> From: devel@edk2.groups.io On Behalf Of
> VincentX Ke
> Sent: Wednesday, May 3, 2023 10:40 AM
> To: devel@edk2.groups.io
> Cc: Ke, VincentX ; Chiu, Chasel
> ; Desimone, Nathaniel L
> ; Oram, Isaac W
> ; Gao, Liming ;
> Dong, Eric
> Subject: [edk2-devel] [PATCH v5] MinPlatformPkg: Update HWSignature
> filed in FACS
>
> From: VincentX Ke
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4428
>
> Calculating CRC based on each ACPI table.
> Update HWSignature filed in FACS based on CRC while ACPI table changed.
>
> Change-Id: Ic0ca66ff10cda0fbcd0683020fab1bc9aea9b78c
> Signed-off-by: VincentX Ke
> Cc: Chasel Chiu
> Cc: Nate DeSimone
> Cc: Isaac Oram
> Cc: Liming Gao
> Cc: Eric Dong
> Signed-off-by: VincentX Ke
> ---
> Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 182
> +---
> Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf | 1 +
> 2 files changed, 118 insertions(+), 65 deletions(-)
>
> diff --git
> a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> index e967031a3b..a940424ced 100644
> --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> @@ -1191,98 +1191,150 @@ PlatformUpdateTables ( }
>
>
>
> /**
>
> - This function calculates RCR based on PCI Device ID and Vendor ID
> from the devices
>
> - available on the platform.
>
> - It also includes other instances of BIOS change to calculate CRC
> and provides as
>
> - HWSignature filed in FADT table.
>
> + This function calculates CRC based on each offset in the ACPI table.
>
> +
>
> + @param[in] Table The pointer to ACPI table that
>
> +required to calculate CRC.
>
> +
>
> + @retval CRC A pointer to allocate UINT32 that
>
> +contains the CRC32 data.
>
> +**/
>
> +UINT32
>
> +AcpiTableCrcCalculator (
>
> + IN VOID *Table
>
> + )
>
> +{
>
> + UINT32 CRC;
>
> +
>
> + CRC = 0;
>
> +
>
> + //
>
> + // Calculate CRC value.
>
> + //
>
> + if (((EFI_ACPI_6_5_COMMON_HEADER *)(UINTN)Table)->Signature ==
> EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
>
> +//
>
> +// Zero HardwareSignature field before Calculating FACS CRC
>
> +//
>
> +do {
>
> + ((EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE
> *)(UINTN)Table)->HardwareSignature = 0;
>
> +} while (((EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE
> *)(UINTN)Table)->HardwareSignature);
>
> +
>
> +gBS->CalculateCrc32 ((UINT8 *)Table,
> (UINTN)((EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE
> *)(UINTN)Table)->Length, &CRC);
>
> + } else {
>
> +gBS->CalculateCrc32 ((UINT8 *)Table,
> (UINTN)((EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)Table)->Length, &CRC);
>
> + }
>
> +
>
> + return CRC;
>
> +}
>
> +
>
> +/**
>
> + This function calculates CRC based on each ACPI table.
>
> + It also calculates CRC and provides as HWSignature filed in FACS.
>
> **/
>
> VOID
>
> -IsHardwareChange (
>
> +IsAcpiTableChange (
>
>VOID
>
>)
>
> {
>
> - EFI_STATUSStatus;
>
> - UINTN Index;
>
> - UINTN HandleCount;
>
> - EFI_HANDLE*HandleBuffer;
>
> -
[edk2-devel] [PATCH v7] MinPlatformPkg: Update HWSignature filed in FACS
From: VincentX Ke
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4428
Calculating CRC based on each ACPI table.
Update HWSignature filed in FACS based on CRC while ACPI table changed.
Change-Id: Ic0ca66ff10cda0fbcd0683020fab1bc9aea9b78c
Signed-off-by: VincentX Ke
Cc: Chasel Chiu
Cc: Nate DeSimone
Cc: Isaac Oram
Cc: Liming Gao
Cc: Eric Dong
Cc: Ankit Sinha
Signed-off-by: VincentX Ke
---
Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 287
+++-
Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf | 1 +
2 files changed, 223 insertions(+), 65 deletions(-)
diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index e967031a3b..3dca6f99f7 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -1191,98 +1191,255 @@ PlatformUpdateTables (
}
/**
- This function calculates RCR based on PCI Device ID and Vendor ID from the
devices
- available on the platform.
- It also includes other instances of BIOS change to calculate CRC and
provides as
- HWSignature filed in FADT table.
+ This function calculates CRC based on each offset in the ACPI table.
+
+ @param[in] Table The ACPI table required to calculate CRC.
+
+ @retval CRC A pointer to allocate UINT32 that
+contains the CRC32 data.
+**/
+UINT32
+AcpiTableCrcCalculator (
+ IN EFI_ACPI_COMMON_HEADER *Table
+ )
+{
+ EFI_STATUS Status;
+ UINT32 CRC;
+
+ Status = EFI_SUCCESS;
+ CRC= 0;
+
+ //
+ // Calculate CRC value.
+ //
+ if (Table->Signature ==
EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
+//
+// Zero HardwareSignature field before Calculating FACS CRC
+//
+((EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE *)Table)->HardwareSignature
= 0;
+ }
+
+ Status = gBS->CalculateCrc32 ((UINT8 *)Table, (UINTN)Table->Length, &CRC);
+ return CRC;
+}
+
+/**
+ This function count ACPI tables in RSDT/XSDT and return the result.
+
+ @param[in] SdtACPI XSDT/RSDT.
+ @param[in] TablePointerSize Size of table pointer:
+4(RSDT) or 8(XSDT).
+
+ @retval TableCountThe total number of ACPI tables in
+RSDT or XSDT.
+**/
+UINTN
+CountTableInSDT (
+ IN EFI_ACPI_DESCRIPTION_HEADER *Sdt,
+ IN UINTNTablePointerSize
+ )
+{
+ UINTN Index;
+ UINTN TableCount;
+ UINTN EntryCount;
+ UINT64 EntryPtr;
+ UINTN BasePtr;
+ EFI_ACPI_COMMON_HEADER *Table;
+
+ EntryCount = (Sdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) /
TablePointerSize;
+ BasePtr= (UINTN)(Sdt + 1);
+
+ for (Index = 0, TableCount = 0; Index < EntryCount; Index++) {
+EntryPtr = 0;
+Table= NULL;
+CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * TablePointerSize),
TablePointerSize);
+Table = (EFI_ACPI_COMMON_HEADER *)((UINTN)(EntryPtr));
+if (Table) {
+ TableCount++;
+}
+
+if (Table->Signature ==
EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
+ CopyMem ((VOID *)&Fadt, (VOID *)Table, sizeof
(EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE));
+ if (Fadt.FirmwareCtrl || Fadt.XFirmwareCtrl) {
+TableCount++;
+ }
+
+ if (Fadt.Dsdt || Fadt.XDsdt) {
+TableCount++;
+ }
+}
+ }
+
+ return TableCount;
+}
+
+/**
+ This function calculates CRC based on each ACPI table.
+ It also calculates CRC and provides as HWSignature filed in FACS.
**/
VOID
-IsHardwareChange (
+IsAcpiTableChange (
VOID
)
{
- EFI_STATUSStatus;
- UINTN Index;
- UINTN HandleCount;
- EFI_HANDLE*HandleBuffer;
- EFI_PCI_IO_PROTOCOL *PciIo;
- UINT32CRC;
- UINT32*HWChange;
- UINTN HWChangeSize;
- UINT32PciId;
- UINTN Handle;
- EFI_ACPI_6_3_FIRMWARE_ACPI_CONTROL_STRUCTURE *FacsPtr;
- EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE*pFADT;
-
- HandleCount = 0;
- HandleBuffer = NULL;
-
- Status = gBS->LocateHandleBuffer (
- ByProtocol,
- &gEfiPciIoProtocolGuid,
- NULL,
- &HandleCount,
- &HandleBuffer
- );
- if (EFI_ERROR (Status)) {
-return; // PciIO protocol not installed yet!
+ EFI_STATUSStatus;
+ UINTN Index;
+ UINTN AcpiTableCount;
+ UINTN EntryCount;
+ UINTN
Re: [edk2-devel] [PATCH v6] MinPlatformPkg: Update HWSignature filed in FACS
Hi, Ray
Thanks for the correction and those suggestion in previous patches.
As your suggestion, the best way is keep following the logic of ACPI spec
definition.
[Patch V7] updated with https://edk2.groups.io/g/devel/message/104685
I apologize for those flaws in previous patches.
Sincerely,
Vincent
-Original Message-
From: Ni, Ray
Sent: Thursday, May 11, 2023 11:42 AM
To: devel@edk2.groups.io; Ke, VincentX
Cc: Chiu, Chasel ; Desimone, Nathaniel L
; Oram, Isaac W ; Gao,
Liming ; Dong, Eric ; Sinha,
Ankit
Subject: RE: [edk2-devel] [PATCH v6] MinPlatformPkg: Update HWSignature filed
in FACS
Vincent,
ACPI spec contains following:
The XSDT provides identical functionality to the RSDT but accommodates
physical addresses of DESCRIPTION
HEADERs that are larger than 32 bits. Notice that both the XSDT and the RSDT
can be pointed to by the RSDP
structure. An ACPI-compatible OS must use the XSDT if present.
Which means, XSDT is used over RSDT when it exists.
But your code measures both XSDT and RSDT. It's not right.
This change is not a simple change. I suggest you review the ACPI spec in
detail and close the opens within your team before sending another version of
patches.
Thanks,
Ray
> -Original Message-
> From: devel@edk2.groups.io On Behalf Of
> VincentX Ke
> Sent: Wednesday, May 10, 2023 5:58 PM
> To: devel@edk2.groups.io
> Cc: Ke, VincentX ; Chiu, Chasel
> ; Desimone, Nathaniel L
> ; Oram, Isaac W
> ; Gao, Liming ;
> Dong, Eric ; Sinha, Ankit
> Subject: [edk2-devel] [PATCH v6] MinPlatformPkg: Update HWSignature
> filed in FACS
>
> From: VincentX Ke
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4428
>
> Calculating CRC based on each ACPI table.
> Update HWSignature filed in FACS based on CRC while ACPI table changed.
>
> Change-Id: Ic0ca66ff10cda0fbcd0683020fab1bc9aea9b78c
> Signed-off-by: VincentX Ke
> Cc: Chasel Chiu
> Cc: Nate DeSimone
> Cc: Isaac Oram
> Cc: Liming Gao
> Cc: Eric Dong
> Cc: Ankit Sinha
> Signed-off-by: VincentX Ke
> ---
> Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 271
> +++-
> Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf | 1 +
> 2 files changed, 209 insertions(+), 63 deletions(-)
>
> diff --git
> a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> index e967031a3b..bb0f4a1f04 100644
> --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> @@ -1191,98 +1191,243 @@ PlatformUpdateTables ( }
>
>
>
> /**
>
> - This function calculates RCR based on PCI Device ID and Vendor ID
> from the devices
>
> - available on the platform.
>
> - It also includes other instances of BIOS change to calculate CRC
> and provides as
>
> - HWSignature filed in FADT table.
>
> + This function calculates CRC based on each offset in the ACPI table.
>
> +
>
> + @param[in] Table The ACPI table required to calculate CRC.
>
> +
>
> + @retval CRC A pointer to allocate UINT32 that
>
> +contains the CRC32 data.
>
> +**/
>
> +UINT32
>
> +AcpiTableCrcCalculator (
>
> + IN EFI_ACPI_COMMON_HEADER *Table
>
> + )
>
> +{
>
> + EFI_STATUS Status;
>
> + UINT32 CRC;
>
> +
>
> + Status = EFI_SUCCESS;
>
> + CRC= 0;
>
> +
>
> + //
>
> + // Calculate CRC value.
>
> + //
>
> + if (Table->Signature ==
> EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
>
> +//
>
> +// Zero HardwareSignature field before Calculating FACS CRC
>
> +//
>
> +((EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE *)Table)-
> >HardwareSignature = 0;
>
> + }
>
> +
>
> + Status = gBS->CalculateCrc32 ((UINT8 *)Table, (UINTN)Table->Length,
> + &CRC);
>
> + return CRC;
>
> +}
>
> +
>
> +/**
>
> + This function count ACPI tables in RSDT/XSDT and return the result.
>
> +
>
> + @param[in] SdtACPI XSDT/RSDT.
>
> + @param[in] TablePointerSize Size of table pointer:
>
> +4(RSDT) or 8(XSDT).
>
> +
>
> + @retval TableCountThe total number of ACPI tables in
>
> +RSDT or XSDT.
>
> +**/
>
> +UINTN
>
> +CountTableInSDT (
>
> + IN EFI_ACPI_DESCRIPTION_HEADER *Sdt,
>
> + IN UINTNTablePointerSize
>
> + )
>
> +{
>
> + UINTN
Re: [edk2-devel] [PATCH v7] MinPlatformPkg: Update HWSignature filed in FACS
Hi, Ray
Please see below replies. And thanks for the code review.
#2. Different versions of FADT have different sizes. This CopyMem is dangerous.
And why do you CopyMem()?
Reply: Based on https://edk2.groups.io/g/devel/topic/98821097#104899, The
extern variable "Facs" will only keep the unique one in AcpiPlatform.c, "extern
EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE Facs" should be follow up if
version changed. And since the purpose is calculating CRC, we only need data of
FACS rather than physical pointer to FACS.
#5. You don't need to check the Signature. You can check if Xsdt is not 0, use
XSDT, otherwise use RSDT.
Reply: To avoid the case that XSDT is garbage in RSDT only platform. Only
checking with "Xsdt is not 0" cannot meet the requirement.
 #7. Why not return earlier but here?
Reply: The reason to return here is AllocateZeroPool() failed or XSDT/RSDT
doesn't find out any pointer entry. This return should be kept.
Sincerely,
Vincent
-Original Message-
From: Ni, Ray
Sent: Friday, May 12, 2023 5:33 PM
To: devel@edk2.groups.io; Ke, VincentX
Cc: Chiu, Chasel ; Desimone, Nathaniel L
; Oram, Isaac W ; Gao,
Liming ; Dong, Eric ; Sinha,
Ankit
Subject: RE: [edk2-devel] [PATCH v7] MinPlatformPkg: Update HWSignature filed
in FACS
> -Original Message-
> From: devel@edk2.groups.io On Behalf Of
> VincentX Ke
> Sent: Thursday, May 11, 2023 6:00 PM
> To: devel@edk2.groups.io
> Cc: Ke, VincentX ; Chiu, Chasel
> ; Desimone, Nathaniel L
> ; Oram, Isaac W
> ; Gao, Liming ;
> Dong, Eric ; Sinha, Ankit
> Subject: [edk2-devel] [PATCH v7] MinPlatformPkg: Update HWSignature
> filed in FACS
>
> From: VincentX Ke
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4428
>
> Calculating CRC based on each ACPI table.
> Update HWSignature filed in FACS based on CRC while ACPI table changed.
>
> Change-Id: Ic0ca66ff10cda0fbcd0683020fab1bc9aea9b78c
> Signed-off-by: VincentX Ke
> Cc: Chasel Chiu
> Cc: Nate DeSimone
> Cc: Isaac Oram
> Cc: Liming Gao
> Cc: Eric Dong
> Cc: Ankit Sinha
> Signed-off-by: VincentX Ke
> ---
> Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 287
> +++-
> Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf | 1 +
> 2 files changed, 223 insertions(+), 65 deletions(-)
>
> diff --git
> a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> index e967031a3b..3dca6f99f7 100644
> --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> @@ -1191,98 +1191,255 @@ PlatformUpdateTables ( }
>
>
>
> /**
>
> - This function calculates RCR based on PCI Device ID and Vendor ID
> from the devices
>
> - available on the platform.
>
> - It also includes other instances of BIOS change to calculate CRC
> and provides as
>
> - HWSignature filed in FADT table.
>
> + This function calculates CRC based on each offset in the ACPI table.
>
> +
>
> + @param[in] Table The ACPI table required to calculate CRC.
>
> +
>
> + @retval CRC A pointer to allocate UINT32 that
>
> +contains the CRC32 data.
>
> +**/
>
> +UINT32
>
> +AcpiTableCrcCalculator (
>
> + IN EFI_ACPI_COMMON_HEADER *Table
>
> + )
>
> +{
>
> + EFI_STATUS Status;
>
> + UINT32 CRC;
>
> +
>
> + Status = EFI_SUCCESS;
>
> + CRC= 0;
>
> +
>
> + //
>
> + // Calculate CRC value.
>
> + //
>
> + if (Table->Signature ==
> EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
>
> +//
>
> +// Zero HardwareSignature field before Calculating FACS CRC
>
> +//
>
> +((EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE *)Table)-
> >HardwareSignature = 0;
>
> + }
>
> +
>
> + Status = gBS->CalculateCrc32 ((UINT8 *)Table, (UINTN)Table->Length,
> + &CRC);
>
> + return CRC;
>
> +}
>
> +
>
> +/**
>
> + This function count ACPI tables in RSDT/XSDT and return the result.
>
> +
>
> + @param[in] SdtACPI XSDT/RSDT.
>
> + @param[in] TablePointerSize Size of table pointer:
>
> +4(RSDT) or 8(XSDT).
>
> +
>
> + @retval TableCountThe total number of ACPI tables in
>
> +RSDT or XSDT.
>
> +**/
>
> +UINTN
>
> +CountTableInSDT (
>
> + IN EFI_ACPI_DESCRIPTION_HEADER *Sdt,
>
> + IN UINTN
[edk2-devel] [PATCH v8] MinPlatformPkg: Update HWSignature filed in FACS
From: VincentX Ke
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4428
Calculating CRC based on each ACPI table.
Update HWSignature filed in FACS based on CRC while ACPI table changed.
Change-Id: Ic0ca66ff10cda0fbcd0683020fab1bc9aea9b78c
Signed-off-by: VincentX Ke
Cc: Chasel Chiu
Cc: Nate DeSimone
Cc: Isaac Oram
Cc: Liming Gao
Cc: Eric Dong
Cc: Ankit Sinha
Signed-off-by: VincentX Ke
---
Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 299
+++-
Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf | 1 +
2 files changed, 233 insertions(+), 67 deletions(-)
diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index 2f2c96f907..5005d1a524 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -1191,98 +1191,263 @@ PlatformUpdateTables (
}
/**
- This function calculates RCR based on PCI Device ID and Vendor ID from the
devices
- available on the platform.
- It also includes other instances of BIOS change to calculate CRC and
provides as
- HWSignature filed in FADT table.
+ This function calculates CRC based on each offset in the ACPI table.
+
+ @param[in] Table The ACPI table required to calculate CRC.
+
+ @retval CRC A pointer to allocate UINT32 that
+contains the CRC32 data.
+**/
+UINT32
+AcpiTableCrcCalculator (
+ IN EFI_ACPI_COMMON_HEADER *Table
+ )
+{
+ EFI_STATUS Status;
+ UINT32 CRC;
+
+ Status = EFI_SUCCESS;
+ CRC= 0;
+
+ //
+ // Calculate CRC value.
+ //
+ if (Table->Signature ==
EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
+//
+// Zero HardwareSignature field before Calculating FACS CRC
+//
+((EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE *)Table)->HardwareSignature
= 0;
+ }
+
+ Status = gBS->CalculateCrc32 ((UINT8 *)Table, (UINTN)Table->Length, &CRC);
+ return CRC;
+}
+
+/**
+ This function count ACPI tables in RSDT/XSDT and return the result.
+
+ @param[in] SdtACPI XSDT/RSDT.
+ @param[in] TablePointerSize Size of table pointer:
+4(RSDT) or 8(XSDT).
+
+ @retval TableCountThe total number of ACPI tables in
+RSDT or XSDT.
+**/
+UINTN
+CountTableInSDT (
+ IN EFI_ACPI_DESCRIPTION_HEADER *Sdt,
+ IN UINTNTablePointerSize
+ )
+{
+ UINTN Index;
+ UINTN TableCount;
+ UINTN EntryCount;
+ UINT64 EntryPtr;
+ UINTN BasePtr;
+ EFI_ACPI_COMMON_HEADER *Table;
+ EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE *FadtPtr;
+
+ EntryCount = (Sdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) /
TablePointerSize;
+ BasePtr= (UINTN)(Sdt + 1);
+ FadtPtr= NULL;
+
+ for (Index = 0, TableCount = 0; Index < EntryCount; Index++) {
+EntryPtr = 0;
+Table= NULL;
+CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * TablePointerSize),
TablePointerSize);
+Table = (EFI_ACPI_COMMON_HEADER *)((UINTN)(EntryPtr));
+if (Table != NULL) {
+ TableCount++;
+}
+
+if (Table->Signature ==
EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
+ FadtPtr = (EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE *)Table;
+ if (FadtPtr->FirmwareCtrl || FadtPtr->XFirmwareCtrl) {
+TableCount++;
+ }
+
+ if (FadtPtr->Dsdt || FadtPtr->XDsdt) {
+TableCount++;
+ }
+}
+ }
+
+ return TableCount;
+}
+
+/**
+ This function calculates CRC based on each ACPI table.
+ It also calculates CRC and provides as HWSignature filed in FACS.
**/
VOID
-IsHardwareChange (
+IsAcpiTableChange (
VOID
)
{
- EFI_STATUS Status;
- UINTNIndex;
- UINTNHandleCount;
- EFI_HANDLE *HandleBuffer;
- EFI_PCI_IO_PROTOCOL *PciIo;
- UINT32 CRC;
- UINT32 *HWChange;
- UINTNHWChangeSize;
- UINT32 PciId;
- UINTNHandle;
- EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE *FacsPtr;
- EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE*pFADT;
-
- HandleCount = 0;
- HandleBuffer = NULL;
-
- Status = gBS->LocateHandleBuffer (
- ByProtocol,
- &gEfiPciIoProtocolGuid,
- NULL,
- &HandleCount,
- &HandleBuffer
- );
- if (EFI_ERROR (Sta
[edk2-devel] [PATCH v9] MinPlatformPkg: Update HWSignature field in FACS
From: VincentX Ke
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4428
Calculating CRC based on each ACPI table.
Update HWSignature field in FACS based on CRC while ACPI table changed.
Change-Id: Ic0ca66ff10cda0fbcd0683020fab1bc9aea9b78c
Signed-off-by: VincentX Ke
Cc: Chasel Chiu
Cc: Nate DeSimone
Cc: Isaac Oram
Cc: Liming Gao
Cc: Eric Dong
Cc: Ankit Sinha
Signed-off-by: VincentX Ke
---
Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 282
+++-
Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf | 1 +
2 files changed, 215 insertions(+), 68 deletions(-)
diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index 2f2c96f907..ca1c73f6fe 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -1191,98 +1191,244 @@ PlatformUpdateTables (
}
/**
- This function calculates RCR based on PCI Device ID and Vendor ID from the
devices
- available on the platform.
- It also includes other instances of BIOS change to calculate CRC and
provides as
- HWSignature filed in FADT table.
+ Function prototype for GetAcpiTableCount/CalculateAcpiTableCrc.
+
+ @param[in] TableThe pointer to ACPI table.
+ @param[in] TableIndex The ACPI table index.
+ @param[in] Context The pointer to UINTN for GetAcpiTableCount.
+ The pointer to UINT32 array for
CalculateAcpiTableCrc.
**/
+typedef
VOID
-IsHardwareChange (
- VOID
+(EFIAPI *ACPI_TABLE_CALLBACK)(
+ IN EFI_ACPI_COMMON_HEADER *Table,
+ IN UINTN TableIndex,
+ IN VOID*Context
+ );
+
+/**
+ Enumerate all ACPI tables in RSDT/XSDT.
+
+ @param[in] SdtACPI XSDT/RSDT.
+ @param[in] TablePointerSize Size of table pointer:
+4(RSDT) or 8(XSDT).
+ @param[in] CallbackFunction The pointer to
GetAcpiTableCount/CalculateAcpiTableCrc.
+ @param[in] ContextThe pointer to UINTN for GetAcpiTableCount.
+The pointer to UINT32 array for
CalculateAcpiTableCrc.
+**/
+VOID
+EnumerateAllAcpiTables (
+ IN EFI_ACPI_DESCRIPTION_HEADER *Sdt,
+ IN UINTNTablePointerSize,
+ IN ACPI_TABLE_CALLBACK CallbackFunction,
+ IN VOID *Context
)
{
- EFI_STATUS Status;
- UINTNIndex;
- UINTNHandleCount;
- EFI_HANDLE *HandleBuffer;
- EFI_PCI_IO_PROTOCOL *PciIo;
- UINT32 CRC;
- UINT32 *HWChange;
- UINTNHWChangeSize;
- UINT32 PciId;
- UINTNHandle;
- EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE *FacsPtr;
- EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE*pFADT;
-
- HandleCount = 0;
- HandleBuffer = NULL;
-
- Status = gBS->LocateHandleBuffer (
- ByProtocol,
- &gEfiPciIoProtocolGuid,
- NULL,
- &HandleCount,
- &HandleBuffer
- );
- if (EFI_ERROR (Status)) {
-return; // PciIO protocol not installed yet!
+ UINTN Index;
+ UINTN TableIndex;
+ UINTN EntryCount;
+ UINT64 EntryPtr;
+ UINTN BasePtr;
+ EFI_ACPI_COMMON_HEADER *Table;
+ EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE *FadtPtr;
+
+ Index = 0;
+ TableIndex = 0;
+ EntryCount = (Sdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) /
TablePointerSize;
+ EntryPtr = 0;
+ BasePtr= (UINTN)(Sdt + 1);
+ Table = NULL;
+ FadtPtr= NULL;
+
+ if (Sdt == NULL) {
+ASSERT (Sdt != NULL);
+return;
}
- //
- // Allocate memory for HWChange and add additional entrie for
- // pFADT->XDsdt
- //
- HWChangeSize = HandleCount + 1;
- HWChange = AllocateZeroPool (sizeof(UINT32) * HWChangeSize);
- ASSERT(HWChange != NULL);
+ for (Index = 0; Index < EntryCount; Index++) {
+EntryPtr = 0;
+Table= NULL;
+CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * TablePointerSize),
TablePointerSize);
+Table = (EFI_ACPI_COMMON_HEADER *)((UINTN)(EntryPtr));
+if (Table != NULL) {
+ CallbackFunction (Table, TableIndex++, Context);
+}
+
+if (Table->Signature ==
EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
+ FadtPtr = (EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE *)Table;
+ if (FadtPtr->Header.Revision <
EFI_ACPI_2_0_
[edk2-devel] [PATCH v10] MinPlatformPkg: Update HWSignature field in FACS
From: VincentX Ke
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4428
Calculating CRC based on each ACPI table.
Update HWSignature field in FACS based on CRC while ACPI table changed.
Change-Id: Ic0ca66ff10cda0fbcd0683020fab1bc9aea9b78c
Signed-off-by: VincentX Ke
Cc: Chasel Chiu
Cc: Nate DeSimone
Cc: Isaac Oram
Cc: Liming Gao
Cc: Eric Dong
Cc: Ankit Sinha
Signed-off-by: VincentX Ke
---
Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 284
+++-
Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf | 1 +
2 files changed, 217 insertions(+), 68 deletions(-)
diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index 2f2c96f907..2a833ec99c 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -1191,98 +1191,246 @@ PlatformUpdateTables (
}
/**
- This function calculates RCR based on PCI Device ID and Vendor ID from the
devices
- available on the platform.
- It also includes other instances of BIOS change to calculate CRC and
provides as
- HWSignature filed in FADT table.
+ Function prototype for GetAcpiTableCount/CalculateAcpiTableCrc.
+
+ @param[in] TableThe pointer to ACPI table.
+ @param[in] TableIndex The ACPI table index.
+ @param[in] Context The pointer to UINTN for GetAcpiTableCount.
+ The pointer to UINT32 array for
CalculateAcpiTableCrc.
**/
+typedef
VOID
-IsHardwareChange (
- VOID
+(EFIAPI *ACPI_TABLE_CALLBACK)(
+ IN EFI_ACPI_COMMON_HEADER *Table,
+ IN UINTN TableIndex,
+ IN VOID*Context
+ );
+
+/**
+ Enumerate all ACPI tables in RSDT/XSDT.
+
+ @param[in] SdtACPI XSDT/RSDT.
+ @param[in] TablePointerSize Size of table pointer:
+4(RSDT) or 8(XSDT).
+ @param[in] CallbackFunction The pointer to
GetAcpiTableCount/CalculateAcpiTableCrc.
+ @param[in] ContextThe pointer to UINTN for GetAcpiTableCount.
+The pointer to UINT32 array for
CalculateAcpiTableCrc.
+**/
+VOID
+EnumerateAllAcpiTables (
+ IN EFI_ACPI_DESCRIPTION_HEADER *Sdt,
+ IN UINTNTablePointerSize,
+ IN ACPI_TABLE_CALLBACK CallbackFunction,
+ IN VOID *Context
)
{
- EFI_STATUS Status;
- UINTNIndex;
- UINTNHandleCount;
- EFI_HANDLE *HandleBuffer;
- EFI_PCI_IO_PROTOCOL *PciIo;
- UINT32 CRC;
- UINT32 *HWChange;
- UINTNHWChangeSize;
- UINT32 PciId;
- UINTNHandle;
- EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE *FacsPtr;
- EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE*pFADT;
-
- HandleCount = 0;
- HandleBuffer = NULL;
-
- Status = gBS->LocateHandleBuffer (
- ByProtocol,
- &gEfiPciIoProtocolGuid,
- NULL,
- &HandleCount,
- &HandleBuffer
- );
- if (EFI_ERROR (Status)) {
-return; // PciIO protocol not installed yet!
+ UINTN Index;
+ UINTN TableIndex;
+ UINTN EntryCount;
+ UINT64 EntryPtr;
+ UINTN BasePtr;
+ EFI_ACPI_COMMON_HEADER *Table;
+ EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE *FadtPtr;
+
+ Index = 0;
+ TableIndex = 0;
+ EntryCount = (Sdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) /
TablePointerSize;
+ EntryPtr = 0;
+ BasePtr= (UINTN)(Sdt + 1);
+ Table = NULL;
+ FadtPtr= NULL;
+
+ if (Sdt == NULL) {
+ASSERT (Sdt != NULL);
+return;
}
- //
- // Allocate memory for HWChange and add additional entrie for
- // pFADT->XDsdt
- //
- HWChangeSize = HandleCount + 1;
- HWChange = AllocateZeroPool (sizeof(UINT32) * HWChangeSize);
- ASSERT(HWChange != NULL);
+ for (Index = 0; Index < EntryCount; Index++) {
+EntryPtr = 0;
+Table= NULL;
+CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * TablePointerSize),
TablePointerSize);
+Table = (EFI_ACPI_COMMON_HEADER *)((UINTN)(EntryPtr));
+if (Table != NULL) {
+ CallbackFunction (Table, TableIndex++, Context);
+}
+
+if (Table->Signature ==
EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
+ FadtPtr = (EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE *)Table;
+ if (FadtPtr->Header.Revision <
EFI_ACPI_2_0_
Re: [edk2-devel] [PATCH v10] MinPlatformPkg: Update HWSignature field in FACS
Thanks for review and merge.
-Original Message-
From: Chiu, Chasel
Sent: Wednesday, May 31, 2023 1:38 PM
To: Ke, VincentX ; devel@edk2.groups.io
Cc: Desimone, Nathaniel L ; Oram, Isaac W
; Gao, Liming ; Dong, Eric
; Sinha, Ankit ; Ni, Ray
Subject: RE: [PATCH v10] MinPlatformPkg: Update HWSignature field in FACS
Patch merged:
https://github.com/tianocore/edk2-platforms/commit/1a7bd150d39007bfb72c4727feda3184c23efe96
Thanks,
Chasel
> -Original Message-
> From: Ke, VincentX
> Sent: Tuesday, May 30, 2023 7:48 PM
> To: devel@edk2.groups.io
> Cc: Ke, VincentX ; Chiu, Chasel
> ; Desimone, Nathaniel L
> ; Oram, Isaac W
> ; Gao, Liming ;
> Dong, Eric ; Sinha, Ankit
> Subject: [PATCH v10] MinPlatformPkg: Update HWSignature field in FACS
>
> From: VincentX Ke
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4428
>
> Calculating CRC based on each ACPI table.
> Update HWSignature field in FACS based on CRC while ACPI table changed.
>
> Change-Id: Ic0ca66ff10cda0fbcd0683020fab1bc9aea9b78c
> Signed-off-by: VincentX Ke
> Cc: Chasel Chiu
> Cc: Nate DeSimone
> Cc: Isaac Oram
> Cc: Liming Gao
> Cc: Eric Dong
> Cc: Ankit Sinha
> Signed-off-by: VincentX Ke
> ---
> Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 284
> +++-
> Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf | 1 +
> 2 files changed, 217 insertions(+), 68 deletions(-)
>
> diff --git
> a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> index 2f2c96f907..2a833ec99c 100644
> --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> @@ -1191,98 +1191,246 @@ PlatformUpdateTables ( }
>
>
>
> /**
>
> - This function calculates RCR based on PCI Device ID and Vendor ID
> from the devices
>
> - available on the platform.
>
> - It also includes other instances of BIOS change to calculate CRC
> and provides as
>
> - HWSignature filed in FADT table.
>
> + Function prototype for GetAcpiTableCount/CalculateAcpiTableCrc.
>
> +
>
> + @param[in] TableThe pointer to ACPI table.
>
> + @param[in] TableIndex The ACPI table index.
>
> + @param[in] Context The pointer to UINTN for GetAcpiTableCount.
>
> + The pointer to UINT32 array for
> CalculateAcpiTableCrc.
>
> **/
>
> +typedef
>
> VOID
>
> -IsHardwareChange (
>
> - VOID
>
> +(EFIAPI *ACPI_TABLE_CALLBACK)(
>
> + IN EFI_ACPI_COMMON_HEADER *Table,
>
> + IN UINTN TableIndex,
>
> + IN VOID*Context
>
> + );
>
> +
>
> +/**
>
> + Enumerate all ACPI tables in RSDT/XSDT.
>
> +
>
> + @param[in] SdtACPI XSDT/RSDT.
>
> + @param[in] TablePointerSize Size of table pointer:
>
> +4(RSDT) or 8(XSDT).
>
> + @param[in] CallbackFunction The pointer to
> GetAcpiTableCount/CalculateAcpiTableCrc.
>
> + @param[in] ContextThe pointer to UINTN for GetAcpiTableCount.
>
> +The pointer to UINT32 array for
> CalculateAcpiTableCrc.
>
> +**/
>
> +VOID
>
> +EnumerateAllAcpiTables (
>
> + IN EFI_ACPI_DESCRIPTION_HEADER *Sdt,
>
> + IN UINTNTablePointerSize,
>
> + IN ACPI_TABLE_CALLBACK CallbackFunction,
>
> + IN VOID *Context
>
>)
>
> {
>
> - EFI_STATUS Status;
>
> - UINTNIndex;
>
> - UINTNHandleCount;
>
> - EFI_HANDLE *HandleBuffer;
>
> - EFI_PCI_IO_PROTOCOL *PciIo;
>
> - UINT32 CRC;
>
> - UINT32 *HWChange;
>
> - UINTNHWChangeSize;
>
> - UINT32 PciId;
>
> - UINTNHandle;
>
> - EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE *FacsPtr;
>
> - EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE*pFADT;
>
> -
>
> - HandleCount = 0;
>
> - HandleBuffer = NULL;
>
> -
>
> - Status = gBS->LocateHandleBuffer (
>
> - ByProtocol,
>
> - &gEfiPciIoProtocolGuid,
>
> -
[edk2-devel] Bug 3714 - Changing UFS descriptor configuration and retry for support device can't response in time
true -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#84664): https://edk2.groups.io/g/devel/message/84664 Mute This Topic: https://groups.io/mt/87681736/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=- 0001-MdeModulePkg-Replace-with-UfsUnitDesc-to-fix-respons.patch Description: Binary data
[edk2-devel] [PATCH] MdeModulePkg: Replace with UfsUnitDesc to fix response timeout problem.
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3714
Replace with UFS_UNIT_DESC to fix response timeout problem.
Cc: Hao A Wu
Cc: Ray Ni
Cc: Ian Chiu
Cc: Maggie Chu
Signed-off-by: VincentX Ke
---
.../Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c | 24 +--
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
index b331c0f3e3..3a55348cac 100644
--- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
+++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
@@ -1037,9 +1037,10 @@ InitializeUfsBlockIoPeim (
UFS_PEIM_HC_PRIVATE_DATA *Private;
EDKII_UFS_HOST_CONTROLLER_PPI *UfsHcPpi;
UINT32 Index;
- UFS_CONFIG_DESCConfig;
UINTN MmioBase;
UINT8 Controller;
+ UFS_UNIT_DESC UnitDescriptor;
+ UINT32 UnitDescriptorSize;
//
// Shadow this PEIM to run from memory
@@ -1126,19 +1127,18 @@ InitializeUfsBlockIoPeim (
}
//
-// Get Ufs Device's Lun Info by reading Configuration Descriptor.
+// Check if 8 common luns are active and set corresponding bit mask.
//
-Status = UfsRwDeviceDesc (Private, TRUE, UfsConfigDesc, 0, 0, &Config,
sizeof (UFS_CONFIG_DESC));
-if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_ERROR, "Ufs Get Configuration Descriptor Error, Status =
%r\n", Status));
- Controller++;
- continue;
-}
-
-for (Index = 0; Index < UFS_PEIM_MAX_LUNS; Index++) {
- if (Config.UnitDescConfParams[Index].LunEn != 0) {
+UnitDescriptorSize = sizeof (UFS_UNIT_DESC);
+for (Index = 0; Index < 8; Index++) {
+ Status = UfsRwDeviceDesc (Private, TRUE, UfsUnitDesc, (UINT8) Index, 0,
&UnitDescriptor, UnitDescriptorSize);
+ if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "Failed to read unit descriptor, index = %X,
status = %r\n", Index, Status));
+continue;
+ }
+ if (UnitDescriptor.LunEn == 0x1) {
+DEBUG ((DEBUG_INFO, "UFS LUN %X is enabled\n", Index));
Private->Luns.BitMask |= (BIT0 << Index);
-DEBUG ((DEBUG_INFO, "Ufs %d Lun %d is enabled\n", Controller, Index));
}
}
--
2.18.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#84795): https://edk2.groups.io/g/devel/message/84795
Mute This Topic: https://groups.io/mt/87716515/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v2] MdeModulePkg: Replace with UFS_UNIT_DESC to fix response timeout problem.
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3714
Replace with UFS_UNIT_DESC to fix response timeout problem.
Cc: Hao A Wu
Cc: Ray Ni
Cc: Ian Chiu
Cc: Maggie Chu
Signed-off-by: VincentX Ke
---
.../Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c | 20 +--
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
index b331c0f3e3..96a1f38727 100644
--- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
+++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
@@ -1037,9 +1037,9 @@ InitializeUfsBlockIoPeim (
UFS_PEIM_HC_PRIVATE_DATA *Private;
EDKII_UFS_HOST_CONTROLLER_PPI *UfsHcPpi;
UINT32 Index;
- UFS_CONFIG_DESCConfig;
UINTN MmioBase;
UINT8 Controller;
+ UFS_UNIT_DESC UnitDescriptor;
//
// Shadow this PEIM to run from memory
@@ -1126,19 +1126,17 @@ InitializeUfsBlockIoPeim (
}
//
-// Get Ufs Device's Lun Info by reading Configuration Descriptor.
+// Check if 8 common luns are active and set corresponding bit mask.
//
-Status = UfsRwDeviceDesc (Private, TRUE, UfsConfigDesc, 0, 0, &Config,
sizeof (UFS_CONFIG_DESC));
-if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_ERROR, "Ufs Get Configuration Descriptor Error, Status =
%r\n", Status));
- Controller++;
- continue;
-}
-
for (Index = 0; Index < UFS_PEIM_MAX_LUNS; Index++) {
- if (Config.UnitDescConfParams[Index].LunEn != 0) {
+ Status = UfsRwDeviceDesc (Private, TRUE, UfsUnitDesc, (UINT8) Index, 0,
&UnitDescriptor, sizeof (UFS_UNIT_DESC));
+ if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "Fail to read UFS Unit Descriptor, Index = %X,
Status = %r\n", Index, Status));
+continue;
+ }
+ if (UnitDescriptor.LunEn == 0x1) {
+DEBUG ((DEBUG_INFO, "UFS LUN %X is enabled\n", Index));
Private->Luns.BitMask |= (BIT0 << Index);
-DEBUG ((DEBUG_INFO, "Ufs %d Lun %d is enabled\n", Controller, Index));
}
}
--
2.18.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#84845): https://edk2.groups.io/g/devel/message/84845
Mute This Topic: https://groups.io/mt/87736351/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v3] MdeModulePkg: Replace with UFS_UNIT_DESC to fix response timeout problem.
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3714
Replace with UFS_UNIT_DESC to fix response timeout problem.
Cc: Hao A Wu
Cc: Ray Ni
Cc: Ian Chiu
Cc: Maggie Chu
Signed-off-by: VincentX Ke
---
.../Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c | 20 +--
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
index b331c0f3e3..9ad1e19fe0 100644
--- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
+++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
@@ -1037,9 +1037,9 @@ InitializeUfsBlockIoPeim (
UFS_PEIM_HC_PRIVATE_DATA *Private;
EDKII_UFS_HOST_CONTROLLER_PPI *UfsHcPpi;
UINT32 Index;
- UFS_CONFIG_DESCConfig;
UINTN MmioBase;
UINT8 Controller;
+ UFS_UNIT_DESC UnitDescriptor;
//
// Shadow this PEIM to run from memory
@@ -1126,19 +1126,17 @@ InitializeUfsBlockIoPeim (
}
//
-// Get Ufs Device's Lun Info by reading Configuration Descriptor.
+// Check if 8 common luns are active and set corresponding bit mask.
//
-Status = UfsRwDeviceDesc (Private, TRUE, UfsConfigDesc, 0, 0, &Config,
sizeof (UFS_CONFIG_DESC));
-if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_ERROR, "Ufs Get Configuration Descriptor Error, Status =
%r\n", Status));
- Controller++;
- continue;
-}
-
for (Index = 0; Index < UFS_PEIM_MAX_LUNS; Index++) {
- if (Config.UnitDescConfParams[Index].LunEn != 0) {
-Private->Luns.BitMask |= (BIT0 << Index);
+ Status = UfsRwDeviceDesc (Private, TRUE, UfsUnitDesc, (UINT8) Index, 0,
&UnitDescriptor, sizeof (UFS_UNIT_DESC));
+ if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "Fail to read UFS Unit Descriptor, Index = %X,
Status = %r\n", Index, Status));
+continue;
+ }
+ if (UnitDescriptor.LunEn == 0x1) {
DEBUG ((DEBUG_INFO, "Ufs %d Lun %d is enabled\n", Controller, Index));
+Private->Luns.BitMask |= (BIT0 << Index);
}
}
--
2.18.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#84848): https://edk2.groups.io/g/devel/message/84848
Mute This Topic: https://groups.io/mt/87737651/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH] MdeModulePkg: Retry up to 5 times while sending UFS DME request to fix timing problem.
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3775
Refactoring UFS DME request sending function and retry up to 5 times.
Cc: Hao A Wu
Cc: Ray Ni
Cc: Ian Chiu
Cc: Maggie Chu
Signed-off-by: VincentX Ke
---
MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c | 306 +++-
1 file changed, 176 insertions(+), 130 deletions(-)
diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
index 2baa57593e..9fa5fcf46f 100644
--- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
+++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
@@ -1,6 +1,6 @@
/** @file
- Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved.
+ Copyright (c) 2014 - 2021, Intel Corporation. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -747,60 +747,91 @@ UfsStopExecCmd (
}
/**
- Read or write specified device descriptor of a UFS device.
+ Extracts return data from query response upiu.
- @param[in] Private The pointer to the UFS_PEIM_HC_PRIVATE_DATA
data structure.
- @param[in] Read The boolean variable to show r/w direction.
- @param[in] DescIdThe ID of device descriptor.
- @param[in] Index The Index of device descriptor.
- @param[in] Selector The Selector of device descriptor.
- @param[in, out] DescriptorThe buffer of device descriptor to be read or
written.
- @param[in] DescSize The size of device descriptor buffer.
+ @param[in] PacketPointer to the
UFS_DEVICE_MANAGEMENT_REQUEST_PACKET.
+ @param[in] QueryResp Pointer to the query response.
- @retval EFI_SUCCESS The device descriptor was read/written
successfully.
- @retval EFI_DEVICE_ERROR A device error occurred while attempting to
r/w the device descriptor.
- @retval EFI_TIMEOUT A timeout occurred while waiting for the
completion of r/w the device descriptor.
+ @retval EFI_INVALID_PARAMETER Packet or QueryResp are empty or opcode is
invalid.
+ @retval EFI_DEVICE_ERROR Data returned from device is invalid.
+ @retval EFI_SUCCESS Data extracted.
**/
EFI_STATUS
-UfsRwDeviceDesc (
- IN UFS_PEIM_HC_PRIVATE_DATA *Private,
- IN BOOLEAN Read,
- IN UINT8 DescId,
- IN UINT8 Index,
- IN UINT8 Selector,
- IN OUT VOID *Descriptor,
- IN UINT32DescSize
+UfsGetReturnDataFromQueryResponse (
+ IN UFS_DEVICE_MANAGEMENT_REQUEST_PACKET *Packet,
+ IN UTP_QUERY_RESP_UPIU *QueryResp
)
{
- EFI_STATUSStatus;
- UFS_DEVICE_MANAGEMENT_REQUEST_PACKET Packet;
- UINT8 Slot;
- UTP_TRD *Trd;
- UINTN Address;
- UTP_QUERY_RESP_UPIU *QueryResp;
- UINT8 *CmdDescBase;
- UINT32CmdDescSize;
- UINT16ReturnDataSize;
+ UINT16 ReturnDataSize;
- ZeroMem (&Packet, sizeof (UFS_DEVICE_MANAGEMENT_REQUEST_PACKET));
+ ReturnDataSize = 0;
- if (Read) {
-Packet.DataDirection= UfsDataIn;
-Packet.InDataBuffer = Descriptor;
-Packet.InTransferLength = DescSize;
-Packet.Opcode = UtpQueryFuncOpcodeRdDesc;
- } else {
-Packet.DataDirection = UfsDataOut;
-Packet.OutDataBuffer = Descriptor;
-Packet.OutTransferLength = DescSize;
-Packet.Opcode= UtpQueryFuncOpcodeWrDesc;
+ if (Packet == NULL || QueryResp == NULL) {
+return EFI_INVALID_PARAMETER;
}
- Packet.DescId = DescId;
- Packet.Index= Index;
- Packet.Selector = Selector;
- Packet.Timeout = UFS_TIMEOUT;
+ switch (Packet->Opcode) {
+case UtpQueryFuncOpcodeRdDesc:
+ ReturnDataSize = QueryResp->Tsf.Length;
+ SwapLittleEndianToBigEndian ((UINT8 *) &ReturnDataSize, sizeof (UINT16));
+ //
+ // Make sure the hardware device does not return more data than expected.
+ //
+ if (ReturnDataSize > Packet->InTransferLength) {
+return EFI_DEVICE_ERROR;
+ }
+
+ CopyMem (Packet->InDataBuffer, (QueryResp + 1), ReturnDataSize);
+ Packet->InTransferLength = ReturnDataSize;
+ break;
+case UtpQueryFuncOpcodeWrDesc:
+ ReturnDataSize = QueryResp->Tsf.Length;
+ SwapLittleEndianToBigEndian ((UINT8 *) &ReturnDataSize, sizeof (UINT16));
+ Packet->OutTransferLength = ReturnDataSize;
+ break;
+case UtpQueryFuncOpcodeRdFlag:
+case UtpQueryFuncOpcodeSetFlag:
+case UtpQueryFuncOpcodeClrFlag:
+case UtpQueryFuncOpcodeTogFlag:
+ //
+ // The 'FLAG VALUE' field is at byte offset 3 of QueryResp->Tsf.Value
+ //
+ *((UINT8 *) (Packet->OutDataBuffer)) = *((UINT8 *)
&(QueryResp->
[edk2-devel] [PATCH] MdeModulePkg: Moving UFS HCS.DP checking out of UfsExecUicCommands() to fix timing problem
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3776
Moving UFS HCS.DP (Device Attached) checking under UfsDeviceDetection() to fix
timing problem.
Cc: Hao A Wu
Cc: Ray Ni
Cc: Ian Chiu
Cc: Maggie Chu
Signed-off-by: VincentX Ke
---
MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c | 51 +
1 file changed, 21 insertions(+), 30 deletions(-)
diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
index 9fa5fcf46f..447d6cab7f 100644
--- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
+++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
@@ -1353,23 +1353,6 @@ UfsExecUicCommands (
}
}
- //
- // Check value of HCS.DP and make sure that there is a device attached to
the Link.
- //
- Address = UfsHcBase + UFS_HC_STATUS_OFFSET;
- Data= MmioRead32 (Address);
- if ((Data & UFS_HC_HCS_DP) == 0) {
-Address = UfsHcBase + UFS_HC_IS_OFFSET;
-Status = UfsWaitMemSet (Address, UFS_HC_IS_ULSS, UFS_HC_IS_ULSS,
UFS_TIMEOUT);
-if (EFI_ERROR (Status)) {
- return EFI_DEVICE_ERROR;
-}
-
-return EFI_NOT_FOUND;
- }
-
- DEBUG ((DEBUG_INFO, "UfsblockioPei: found a attached UFS device\n"));
-
return EFI_SUCCESS;
}
@@ -1442,8 +1425,10 @@ UfsDeviceDetection (
IN UFS_PEIM_HC_PRIVATE_DATA *Private
)
{
- UINTN Retry;
- EFI_STATUS Status;
+ UINTN Retry;
+ UINTN Address;
+ UINT32Data;
+ EFI_STATUSStatus;
//
// Start UFS device detection.
@@ -1451,22 +1436,28 @@ UfsDeviceDetection (
//
for (Retry = 0; Retry < 3; Retry++) {
Status = UfsExecUicCommands (Private, UfsUicDmeLinkStartup, 0, 0, 0);
-if (!EFI_ERROR (Status)) {
- break;
+if (EFI_ERROR (Status)) {
+ return EFI_DEVICE_ERROR;
}
-if (Status == EFI_NOT_FOUND) {
- continue;
+//
+// Check value of HCS.DP and make sure that there is a device attached to
the Link.
+//
+Address = Private->UfsHcBase + UFS_HC_STATUS_OFFSET;
+Data= MmioRead32 (Address);
+if ((Data & UFS_HC_HCS_DP) == 0) {
+ Address = Private->UfsHcBase + UFS_HC_IS_OFFSET;
+ Status = UfsWaitMemSet (Address, UFS_HC_IS_ULSS, UFS_HC_IS_ULSS,
UFS_TIMEOUT);
+ if (EFI_ERROR (Status)) {
+return EFI_DEVICE_ERROR;
+ }
+} else {
+ DEBUG ((DEBUG_INFO, "UfsblockioPei: found a attached UFS device\n"));
+ return EFI_SUCCESS;
}
-
-return EFI_DEVICE_ERROR;
}
- if (Retry == 3) {
-return EFI_NOT_FOUND;
- }
-
- return EFI_SUCCESS;
+ return EFI_NOT_FOUND;
}
/**
--
2.18.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#84964): https://edk2.groups.io/g/devel/message/84964
Mute This Topic: https://groups.io/mt/87762457/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v4] MdeModulePkg: Replace with UFS_UNIT_DESC to fix timeout problem
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3714
Replace with UFS_UNIT_DESC to fix response timeout problem.
Cc: Hao A Wu
Cc: Ray Ni
Cc: Ian Chiu
Cc: Maggie Chu
Signed-off-by: VincentX Ke
---
.../Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c | 20 +--
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
index b331c0f3e3..9ad1e19fe0 100644
--- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
+++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
@@ -1037,9 +1037,9 @@ InitializeUfsBlockIoPeim (
UFS_PEIM_HC_PRIVATE_DATA *Private;
EDKII_UFS_HOST_CONTROLLER_PPI *UfsHcPpi;
UINT32 Index;
- UFS_CONFIG_DESCConfig;
UINTN MmioBase;
UINT8 Controller;
+ UFS_UNIT_DESC UnitDescriptor;
//
// Shadow this PEIM to run from memory
@@ -1126,19 +1126,17 @@ InitializeUfsBlockIoPeim (
}
//
-// Get Ufs Device's Lun Info by reading Configuration Descriptor.
+// Check if 8 common luns are active and set corresponding bit mask.
//
-Status = UfsRwDeviceDesc (Private, TRUE, UfsConfigDesc, 0, 0, &Config,
sizeof (UFS_CONFIG_DESC));
-if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_ERROR, "Ufs Get Configuration Descriptor Error, Status =
%r\n", Status));
- Controller++;
- continue;
-}
-
for (Index = 0; Index < UFS_PEIM_MAX_LUNS; Index++) {
- if (Config.UnitDescConfParams[Index].LunEn != 0) {
-Private->Luns.BitMask |= (BIT0 << Index);
+ Status = UfsRwDeviceDesc (Private, TRUE, UfsUnitDesc, (UINT8) Index, 0,
&UnitDescriptor, sizeof (UFS_UNIT_DESC));
+ if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "Fail to read UFS Unit Descriptor, Index = %X,
Status = %r\n", Index, Status));
+continue;
+ }
+ if (UnitDescriptor.LunEn == 0x1) {
DEBUG ((DEBUG_INFO, "Ufs %d Lun %d is enabled\n", Controller, Index));
+Private->Luns.BitMask |= (BIT0 << Index);
}
}
--
2.18.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#85052): https://edk2.groups.io/g/devel/message/85052
Mute This Topic: https://groups.io/mt/87785543/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v2] MdeModulePkg: Refactoring UFS DME request and fix timing problem
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3775
Refactoring UFS DME request function and retry up to 5 times.
Cc: Hao A Wu
Cc: Ray Ni
Cc: Ian Chiu
Cc: Maggie Chu
Signed-off-by: VincentX Ke
---
MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c | 302 +++-
1 file changed, 174 insertions(+), 128 deletions(-)
diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
index 2baa57593e..77d7882700 100644
--- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
+++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
@@ -1,6 +1,6 @@
/** @file
- Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved.
+ Copyright (c) 2014 - 2021, Intel Corporation. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -747,60 +747,91 @@ UfsStopExecCmd (
}
/**
- Read or write specified device descriptor of a UFS device.
+ Extracts return data from query response upiu.
- @param[in] Private The pointer to the UFS_PEIM_HC_PRIVATE_DATA
data structure.
- @param[in] Read The boolean variable to show r/w direction.
- @param[in] DescIdThe ID of device descriptor.
- @param[in] Index The Index of device descriptor.
- @param[in] Selector The Selector of device descriptor.
- @param[in, out] DescriptorThe buffer of device descriptor to be read or
written.
- @param[in] DescSize The size of device descriptor buffer.
+ @param[in] Packet Pointer to the
UFS_DEVICE_MANAGEMENT_REQUEST_PACKET.
+ @param[in] QueryResp Pointer to the query response.
- @retval EFI_SUCCESS The device descriptor was read/written
successfully.
- @retval EFI_DEVICE_ERROR A device error occurred while attempting to
r/w the device descriptor.
- @retval EFI_TIMEOUT A timeout occurred while waiting for the
completion of r/w the device descriptor.
+ @retval EFI_INVALID_PARAMETER Packet or QueryResp are empty or opcode is
invalid.
+ @retval EFI_DEVICE_ERROR Data returned from device is invalid.
+ @retval EFI_SUCCESS Data extracted.
**/
EFI_STATUS
-UfsRwDeviceDesc (
- IN UFS_PEIM_HC_PRIVATE_DATA *Private,
- IN BOOLEAN Read,
- IN UINT8 DescId,
- IN UINT8 Index,
- IN UINT8 Selector,
- IN OUT VOID *Descriptor,
- IN UINT32DescSize
+UfsGetReturnDataFromQueryResponse (
+ IN UFS_DEVICE_MANAGEMENT_REQUEST_PACKET *Packet,
+ IN UTP_QUERY_RESP_UPIU *QueryResp
)
{
- EFI_STATUSStatus;
- UFS_DEVICE_MANAGEMENT_REQUEST_PACKET Packet;
- UINT8 Slot;
- UTP_TRD *Trd;
- UINTN Address;
- UTP_QUERY_RESP_UPIU *QueryResp;
- UINT8 *CmdDescBase;
- UINT32CmdDescSize;
- UINT16ReturnDataSize;
+ UINT16 ReturnDataSize;
- ZeroMem (&Packet, sizeof (UFS_DEVICE_MANAGEMENT_REQUEST_PACKET));
+ ReturnDataSize = 0;
- if (Read) {
-Packet.DataDirection= UfsDataIn;
-Packet.InDataBuffer = Descriptor;
-Packet.InTransferLength = DescSize;
-Packet.Opcode = UtpQueryFuncOpcodeRdDesc;
- } else {
-Packet.DataDirection = UfsDataOut;
-Packet.OutDataBuffer = Descriptor;
-Packet.OutTransferLength = DescSize;
-Packet.Opcode= UtpQueryFuncOpcodeWrDesc;
+ if ((Packet == NULL) || (QueryResp == NULL)) {
+return EFI_INVALID_PARAMETER;
}
- Packet.DescId = DescId;
- Packet.Index= Index;
- Packet.Selector = Selector;
- Packet.Timeout = UFS_TIMEOUT;
+ switch (Packet->Opcode) {
+case UtpQueryFuncOpcodeRdDesc:
+ ReturnDataSize = QueryResp->Tsf.Length;
+ SwapLittleEndianToBigEndian ((UINT8 *)&ReturnDataSize, sizeof (UINT16));
+ //
+ // Make sure the hardware device does not return more data than expected.
+ //
+ if (ReturnDataSize > Packet->InTransferLength) {
+return EFI_DEVICE_ERROR;
+ }
+
+ CopyMem (Packet->InDataBuffer, (QueryResp + 1), ReturnDataSize);
+ Packet->InTransferLength = ReturnDataSize;
+ break;
+case UtpQueryFuncOpcodeWrDesc:
+ ReturnDataSize = QueryResp->Tsf.Length;
+ SwapLittleEndianToBigEndian ((UINT8 *)&ReturnDataSize, sizeof (UINT16));
+ Packet->OutTransferLength = ReturnDataSize;
+ break;
+case UtpQueryFuncOpcodeRdFlag:
+case UtpQueryFuncOpcodeSetFlag:
+case UtpQueryFuncOpcodeClrFlag:
+case UtpQueryFuncOpcodeTogFlag:
+ //
+ // The 'FLAG VALUE' field is at byte offset 3 of QueryResp->Tsf.Value
+ //
+ *((UINT8 *)(Packet->OutDataBuffer)) = *((UINT8 *)&(QueryResp->
[edk2-devel] [PATCH v2] MdeModulePkg: Put off UFS HCS.DP checking to fix timing problem
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3776
Put off UFS HCS.DP (Device Attached) checking
until UfsDeviceDetection() to fix timing problem.
Cc: Hao A Wu
Cc: Ray Ni
Cc: Ian Chiu
Cc: Maggie Chu
Signed-off-by: VincentX Ke
---
MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c | 47 +
1 file changed, 19 insertions(+), 28 deletions(-)
diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
index 2baa57593e..3da4274ca2 100644
--- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
+++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
@@ -1307,23 +1307,6 @@ UfsExecUicCommands (
}
}
- //
- // Check value of HCS.DP and make sure that there is a device attached to
the Link.
- //
- Address = UfsHcBase + UFS_HC_STATUS_OFFSET;
- Data= MmioRead32 (Address);
- if ((Data & UFS_HC_HCS_DP) == 0) {
-Address = UfsHcBase + UFS_HC_IS_OFFSET;
-Status = UfsWaitMemSet (Address, UFS_HC_IS_ULSS, UFS_HC_IS_ULSS,
UFS_TIMEOUT);
-if (EFI_ERROR (Status)) {
- return EFI_DEVICE_ERROR;
-}
-
-return EFI_NOT_FOUND;
- }
-
- DEBUG ((DEBUG_INFO, "UfsblockioPei: found a attached UFS device\n"));
-
return EFI_SUCCESS;
}
@@ -1397,6 +1380,8 @@ UfsDeviceDetection (
)
{
UINTN Retry;
+ UINTN Address;
+ UINT32 Data;
EFI_STATUS Status;
//
@@ -1405,22 +1390,28 @@ UfsDeviceDetection (
//
for (Retry = 0; Retry < 3; Retry++) {
Status = UfsExecUicCommands (Private, UfsUicDmeLinkStartup, 0, 0, 0);
-if (!EFI_ERROR (Status)) {
- break;
+if (EFI_ERROR (Status)) {
+ return EFI_DEVICE_ERROR;
}
-if (Status == EFI_NOT_FOUND) {
- continue;
+//
+// Check value of HCS.DP and make sure that there is a device attached to
the Link
+//
+Address = Private->UfsHcBase + UFS_HC_STATUS_OFFSET;
+Data= MmioRead32 (Address);
+if ((Data & UFS_HC_HCS_DP) == 0) {
+ Address = Private->UfsHcBase + UFS_HC_IS_OFFSET;
+ Status = UfsWaitMemSet (Address, UFS_HC_IS_ULSS, UFS_HC_IS_ULSS,
UFS_TIMEOUT);
+ if (EFI_ERROR (Status)) {
+return EFI_DEVICE_ERROR;
+ }
+} else {
+ DEBUG ((DEBUG_INFO, "UfsblockioPei: found a attached UFS device\n"));
+ return EFI_SUCCESS;
}
-
-return EFI_DEVICE_ERROR;
}
- if (Retry == 3) {
-return EFI_NOT_FOUND;
- }
-
- return EFI_SUCCESS;
+ return EFI_NOT_FOUND;
}
/**
--
2.18.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#85054): https://edk2.groups.io/g/devel/message/85054
Mute This Topic: https://groups.io/mt/87786397/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v5 0/3] MdeModulePkg: Fix UfsBlockIoPei timing problem
Cc: Hao A Wu Cc: Ray Ni Cc: Ian Chiu Cc: Maggie Chu Signed-off-by: VincentX Ke VincentX Ke (3): MdeModulePkg: Replace with UFS_UNIT_DESC to fix timeout problem MdeModulePkg: Refactoring UFS DME request and fix timing problem MdeModulePkg: Put off UFS HCS.DP checking to fix timing problem .../Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c | 23 +- MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c | 346 ++ 2 files changed, 202 insertions(+), 167 deletions(-) -- 2.31.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#85156): https://edk2.groups.io/g/devel/message/85156 Mute This Topic: https://groups.io/mt/87893929/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v5 1/3] MdeModulePkg: Replace with UFS_UNIT_DESC to fix timeout problem
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3714
Replace with UFS_UNIT_DESC to fix response timeout problem.
Cc: Hao A Wu
Cc: Ray Ni
Cc: Ian Chiu
Cc: Maggie Chu
Signed-off-by: VincentX Ke
---
.../Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c | 23 +--
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
index b331c0f3e3..b8651ff998 100644
--- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
+++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
@@ -1,6 +1,6 @@
/** @file
- Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved.
+ Copyright (c) 2014 - 2021, Intel Corporation. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -1037,9 +1037,9 @@ InitializeUfsBlockIoPeim (
UFS_PEIM_HC_PRIVATE_DATA *Private;
EDKII_UFS_HOST_CONTROLLER_PPI *UfsHcPpi;
UINT32 Index;
- UFS_CONFIG_DESCConfig;
UINTN MmioBase;
UINT8 Controller;
+ UFS_UNIT_DESC UnitDescriptor;
//
// Shadow this PEIM to run from memory
@@ -1126,19 +1126,18 @@ InitializeUfsBlockIoPeim (
}
//
-// Get Ufs Device's Lun Info by reading Configuration Descriptor.
+// Check if 8 common luns are active and set corresponding bit mask.
//
-Status = UfsRwDeviceDesc (Private, TRUE, UfsConfigDesc, 0, 0, &Config,
sizeof (UFS_CONFIG_DESC));
-if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_ERROR, "Ufs Get Configuration Descriptor Error, Status =
%r\n", Status));
- Controller++;
- continue;
-}
-
for (Index = 0; Index < UFS_PEIM_MAX_LUNS; Index++) {
- if (Config.UnitDescConfParams[Index].LunEn != 0) {
-Private->Luns.BitMask |= (BIT0 << Index);
+ Status = UfsRwDeviceDesc (Private, TRUE, UfsUnitDesc, (UINT8)Index, 0,
&UnitDescriptor, sizeof (UFS_UNIT_DESC));
+ if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "Fail to read UFS Unit Descriptor, Index = %X,
Status = %r\n", Index, Status));
+continue;
+ }
+
+ if (UnitDescriptor.LunEn == 0x1) {
DEBUG ((DEBUG_INFO, "Ufs %d Lun %d is enabled\n", Controller, Index));
+Private->Luns.BitMask |= (BIT0 << Index);
}
}
--
2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#85157): https://edk2.groups.io/g/devel/message/85157
Mute This Topic: https://groups.io/mt/87893937/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v5 3/3] MdeModulePkg: Put off UFS HCS.DP checking to fix timing problem
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3776
Put off UFS HCS.DP (Device Attached) checking
until UfsDeviceDetection() to fix timing problem.
Cc: Hao A Wu
Cc: Ray Ni
Cc: Ian Chiu
Cc: Maggie Chu
Signed-off-by: VincentX Ke
---
MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c | 47 +
1 file changed, 19 insertions(+), 28 deletions(-)
diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
index e2099c22e8..ddbc32dac7 100644
--- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
+++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
@@ -1352,23 +1352,6 @@ UfsExecUicCommands (
}
}
- //
- // Check value of HCS.DP and make sure that there is a device attached to
the Link.
- //
- Address = UfsHcBase + UFS_HC_STATUS_OFFSET;
- Data= MmioRead32 (Address);
- if ((Data & UFS_HC_HCS_DP) == 0) {
-Address = UfsHcBase + UFS_HC_IS_OFFSET;
-Status = UfsWaitMemSet (Address, UFS_HC_IS_ULSS, UFS_HC_IS_ULSS,
UFS_TIMEOUT);
-if (EFI_ERROR (Status)) {
- return EFI_DEVICE_ERROR;
-}
-
-return EFI_NOT_FOUND;
- }
-
- DEBUG ((DEBUG_INFO, "UfsblockioPei: found a attached UFS device\n"));
-
return EFI_SUCCESS;
}
@@ -1442,6 +1425,8 @@ UfsDeviceDetection (
)
{
UINTN Retry;
+ UINTN Address;
+ UINT32 Data;
EFI_STATUS Status;
//
@@ -1450,22 +1435,28 @@ UfsDeviceDetection (
//
for (Retry = 0; Retry < 3; Retry++) {
Status = UfsExecUicCommands (Private, UfsUicDmeLinkStartup, 0, 0, 0);
-if (!EFI_ERROR (Status)) {
- break;
+if (EFI_ERROR (Status)) {
+ return EFI_DEVICE_ERROR;
}
-if (Status == EFI_NOT_FOUND) {
- continue;
+//
+// Check value of HCS.DP and make sure that there is a device attached to
the Link
+//
+Address = Private->UfsHcBase + UFS_HC_STATUS_OFFSET;
+Data= MmioRead32 (Address);
+if ((Data & UFS_HC_HCS_DP) == 0) {
+ Address = Private->UfsHcBase + UFS_HC_IS_OFFSET;
+ Status = UfsWaitMemSet (Address, UFS_HC_IS_ULSS, UFS_HC_IS_ULSS,
UFS_TIMEOUT);
+ if (EFI_ERROR (Status)) {
+return EFI_DEVICE_ERROR;
+ }
+} else {
+ DEBUG ((DEBUG_INFO, "UfsblockioPei: found a attached UFS device\n"));
+ return EFI_SUCCESS;
}
-
-return EFI_DEVICE_ERROR;
}
- if (Retry == 3) {
-return EFI_NOT_FOUND;
- }
-
- return EFI_SUCCESS;
+ return EFI_NOT_FOUND;
}
/**
--
2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#85159): https://edk2.groups.io/g/devel/message/85159
Mute This Topic: https://groups.io/mt/87893944/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v5 2/3] MdeModulePkg: Refactoring UFS DME request and fix timing problem
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3775
Refactoring UFS DME request function and retry up to 5 times.
Cc: Hao A Wu
Cc: Ray Ni
Cc: Ian Chiu
Cc: Maggie Chu
Signed-off-by: VincentX Ke
---
MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c | 299 +++-
1 file changed, 172 insertions(+), 127 deletions(-)
diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
index 2baa57593e..e2099c22e8 100644
--- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
+++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
@@ -1,6 +1,6 @@
/** @file
- Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved.
+ Copyright (c) 2014 - 2021, Intel Corporation. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -747,60 +747,91 @@ UfsStopExecCmd (
}
/**
- Read or write specified device descriptor of a UFS device.
+ Extracts return data from query response upiu.
- @param[in] Private The pointer to the UFS_PEIM_HC_PRIVATE_DATA
data structure.
- @param[in] Read The boolean variable to show r/w direction.
- @param[in] DescIdThe ID of device descriptor.
- @param[in] Index The Index of device descriptor.
- @param[in] Selector The Selector of device descriptor.
- @param[in, out] DescriptorThe buffer of device descriptor to be read or
written.
- @param[in] DescSize The size of device descriptor buffer.
+ @param[in] Packet Pointer to the
UFS_DEVICE_MANAGEMENT_REQUEST_PACKET.
+ @param[in] QueryResp Pointer to the query response.
- @retval EFI_SUCCESS The device descriptor was read/written
successfully.
- @retval EFI_DEVICE_ERROR A device error occurred while attempting to
r/w the device descriptor.
- @retval EFI_TIMEOUT A timeout occurred while waiting for the
completion of r/w the device descriptor.
+ @retval EFI_INVALID_PARAMETER Packet or QueryResp are empty or opcode is
invalid.
+ @retval EFI_DEVICE_ERROR Data returned from device is invalid.
+ @retval EFI_SUCCESS Data extracted.
**/
EFI_STATUS
-UfsRwDeviceDesc (
- IN UFS_PEIM_HC_PRIVATE_DATA *Private,
- IN BOOLEAN Read,
- IN UINT8 DescId,
- IN UINT8 Index,
- IN UINT8 Selector,
- IN OUT VOID *Descriptor,
- IN UINT32DescSize
+UfsGetReturnDataFromQueryResponse (
+ IN UFS_DEVICE_MANAGEMENT_REQUEST_PACKET *Packet,
+ IN UTP_QUERY_RESP_UPIU *QueryResp
)
{
- EFI_STATUSStatus;
- UFS_DEVICE_MANAGEMENT_REQUEST_PACKET Packet;
- UINT8 Slot;
- UTP_TRD *Trd;
- UINTN Address;
- UTP_QUERY_RESP_UPIU *QueryResp;
- UINT8 *CmdDescBase;
- UINT32CmdDescSize;
- UINT16ReturnDataSize;
+ UINT16 ReturnDataSize;
- ZeroMem (&Packet, sizeof (UFS_DEVICE_MANAGEMENT_REQUEST_PACKET));
+ ReturnDataSize = 0;
- if (Read) {
-Packet.DataDirection= UfsDataIn;
-Packet.InDataBuffer = Descriptor;
-Packet.InTransferLength = DescSize;
-Packet.Opcode = UtpQueryFuncOpcodeRdDesc;
- } else {
-Packet.DataDirection = UfsDataOut;
-Packet.OutDataBuffer = Descriptor;
-Packet.OutTransferLength = DescSize;
-Packet.Opcode= UtpQueryFuncOpcodeWrDesc;
+ if ((Packet == NULL) || (QueryResp == NULL)) {
+return EFI_INVALID_PARAMETER;
}
- Packet.DescId = DescId;
- Packet.Index= Index;
- Packet.Selector = Selector;
- Packet.Timeout = UFS_TIMEOUT;
+ switch (Packet->Opcode) {
+case UtpQueryFuncOpcodeRdDesc:
+ ReturnDataSize = QueryResp->Tsf.Length;
+ SwapLittleEndianToBigEndian ((UINT8 *)&ReturnDataSize, sizeof (UINT16));
+ //
+ // Make sure the hardware device does not return more data than expected.
+ //
+ if (ReturnDataSize > Packet->InTransferLength) {
+return EFI_DEVICE_ERROR;
+ }
+
+ CopyMem (Packet->InDataBuffer, (QueryResp + 1), ReturnDataSize);
+ Packet->InTransferLength = ReturnDataSize;
+ break;
+case UtpQueryFuncOpcodeWrDesc:
+ ReturnDataSize = QueryResp->Tsf.Length;
+ SwapLittleEndianToBigEndian ((UINT8 *)&ReturnDataSize, sizeof (UINT16));
+ Packet->OutTransferLength = ReturnDataSize;
+ break;
+case UtpQueryFuncOpcodeRdFlag:
+case UtpQueryFuncOpcodeSetFlag:
+case UtpQueryFuncOpcodeClrFlag:
+case UtpQueryFuncOpcodeTogFlag:
+ //
+ // The 'FLAG VALUE' field is at byte offset 3 of QueryResp->Tsf.Value
+ //
+ *((UINT8 *)(Packet->OutDataBuffer)) = *((UINT8 *)&(QueryResp->
[edk2-devel] [PATCH 0/3] MdeModulePkg: Fix UfsBlockIoPei timing problem
*** BLURB HERE *** VincentX Ke (3): MdeModulePkg: Replace with UFS_UNIT_DESC to fix timeout problem MdeModulePkg: Refactoring UFS DME request and fix timing problem MdeModulePkg: Put off UFS HCS.DP checking to fix timing problem .../Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c | 23 +- MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c | 357 ++ 2 files changed, 208 insertions(+), 172 deletions(-) -- 2.31.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#85167): https://edk2.groups.io/g/devel/message/85167 Mute This Topic: https://groups.io/mt/87895203/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 1/3] MdeModulePkg: Replace with UFS_UNIT_DESC to fix timeout problem
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3714
Replace with UFS_UNIT_DESC to fix response timeout problem.
Cc: Hao A Wu
Cc: Ray Ni
Cc: Ian Chiu
Cc: Maggie Chu
Signed-off-by: VincentX Ke
---
.../Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c | 23 +--
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
index b331c0f3e3..b8651ff998 100644
--- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
+++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
@@ -1,6 +1,6 @@
/** @file
- Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved.
+ Copyright (c) 2014 - 2021, Intel Corporation. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -1037,9 +1037,9 @@ InitializeUfsBlockIoPeim (
UFS_PEIM_HC_PRIVATE_DATA *Private;
EDKII_UFS_HOST_CONTROLLER_PPI *UfsHcPpi;
UINT32 Index;
- UFS_CONFIG_DESCConfig;
UINTN MmioBase;
UINT8 Controller;
+ UFS_UNIT_DESC UnitDescriptor;
//
// Shadow this PEIM to run from memory
@@ -1126,19 +1126,18 @@ InitializeUfsBlockIoPeim (
}
//
-// Get Ufs Device's Lun Info by reading Configuration Descriptor.
+// Check if 8 common luns are active and set corresponding bit mask.
//
-Status = UfsRwDeviceDesc (Private, TRUE, UfsConfigDesc, 0, 0, &Config,
sizeof (UFS_CONFIG_DESC));
-if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_ERROR, "Ufs Get Configuration Descriptor Error, Status =
%r\n", Status));
- Controller++;
- continue;
-}
-
for (Index = 0; Index < UFS_PEIM_MAX_LUNS; Index++) {
- if (Config.UnitDescConfParams[Index].LunEn != 0) {
-Private->Luns.BitMask |= (BIT0 << Index);
+ Status = UfsRwDeviceDesc (Private, TRUE, UfsUnitDesc, (UINT8)Index, 0,
&UnitDescriptor, sizeof (UFS_UNIT_DESC));
+ if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "Fail to read UFS Unit Descriptor, Index = %X,
Status = %r\n", Index, Status));
+continue;
+ }
+
+ if (UnitDescriptor.LunEn == 0x1) {
DEBUG ((DEBUG_INFO, "Ufs %d Lun %d is enabled\n", Controller, Index));
+Private->Luns.BitMask |= (BIT0 << Index);
}
}
--
2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#85168): https://edk2.groups.io/g/devel/message/85168
Mute This Topic: https://groups.io/mt/87895210/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 2/3] MdeModulePkg: Refactoring UFS DME request and fix timing problem
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3775
Refactoring UFS DME request function and retry up to 5 times.
Cc: Hao A Wu
Cc: Ray Ni
Cc: Ian Chiu
Cc: Maggie Chu
Signed-off-by: VincentX Ke
---
MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c | 310 +++-
1 file changed, 178 insertions(+), 132 deletions(-)
diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
index 2baa57593e..4305fab7dc 100644
--- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
+++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
@@ -1,6 +1,6 @@
/** @file
- Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved.
+ Copyright (c) 2014 - 2021, Intel Corporation. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -747,60 +747,91 @@ UfsStopExecCmd (
}
/**
- Read or write specified device descriptor of a UFS device.
+ Extracts return data from query response upiu.
- @param[in] Private The pointer to the UFS_PEIM_HC_PRIVATE_DATA
data structure.
- @param[in] Read The boolean variable to show r/w direction.
- @param[in] DescIdThe ID of device descriptor.
- @param[in] Index The Index of device descriptor.
- @param[in] Selector The Selector of device descriptor.
- @param[in, out] DescriptorThe buffer of device descriptor to be read or
written.
- @param[in] DescSize The size of device descriptor buffer.
+ @param[in, out] PacketPointer to the
UFS_DEVICE_MANAGEMENT_REQUEST_PACKET.
+ @param[in] QueryResp Pointer to the query response.
- @retval EFI_SUCCESS The device descriptor was read/written
successfully.
- @retval EFI_DEVICE_ERROR A device error occurred while attempting to
r/w the device descriptor.
- @retval EFI_TIMEOUT A timeout occurred while waiting for the
completion of r/w the device descriptor.
+ @retval EFI_INVALID_PARAMETER Packet or QueryResp are empty or opcode is
invalid.
+ @retval EFI_DEVICE_ERROR Data returned from device is invalid.
+ @retval EFI_SUCCESS Data extracted.
**/
EFI_STATUS
-UfsRwDeviceDesc (
- IN UFS_PEIM_HC_PRIVATE_DATA *Private,
- IN BOOLEAN Read,
- IN UINT8 DescId,
- IN UINT8 Index,
- IN UINT8 Selector,
- IN OUT VOID *Descriptor,
- IN UINT32DescSize
+UfsGetReturnDataFromQueryResponse (
+ IN OUT UFS_DEVICE_MANAGEMENT_REQUEST_PACKET *Packet,
+ IN UTP_QUERY_RESP_UPIU *QueryResp
)
{
- EFI_STATUSStatus;
- UFS_DEVICE_MANAGEMENT_REQUEST_PACKET Packet;
- UINT8 Slot;
- UTP_TRD *Trd;
- UINTN Address;
- UTP_QUERY_RESP_UPIU *QueryResp;
- UINT8 *CmdDescBase;
- UINT32CmdDescSize;
- UINT16ReturnDataSize;
+ UINT16 ReturnDataSize;
- ZeroMem (&Packet, sizeof (UFS_DEVICE_MANAGEMENT_REQUEST_PACKET));
+ ReturnDataSize = 0;
- if (Read) {
-Packet.DataDirection= UfsDataIn;
-Packet.InDataBuffer = Descriptor;
-Packet.InTransferLength = DescSize;
-Packet.Opcode = UtpQueryFuncOpcodeRdDesc;
- } else {
-Packet.DataDirection = UfsDataOut;
-Packet.OutDataBuffer = Descriptor;
-Packet.OutTransferLength = DescSize;
-Packet.Opcode= UtpQueryFuncOpcodeWrDesc;
+ if ((Packet == NULL) || (QueryResp == NULL)) {
+return EFI_INVALID_PARAMETER;
}
- Packet.DescId = DescId;
- Packet.Index= Index;
- Packet.Selector = Selector;
- Packet.Timeout = UFS_TIMEOUT;
+ switch (Packet->Opcode) {
+case UtpQueryFuncOpcodeRdDesc:
+ ReturnDataSize = QueryResp->Tsf.Length;
+ SwapLittleEndianToBigEndian ((UINT8 *)&ReturnDataSize, sizeof (UINT16));
+ //
+ // Make sure the hardware device does not return more data than expected.
+ //
+ if (ReturnDataSize > Packet->InTransferLength) {
+return EFI_DEVICE_ERROR;
+ }
+
+ CopyMem (Packet->InDataBuffer, (QueryResp + 1), ReturnDataSize);
+ Packet->InTransferLength = ReturnDataSize;
+ break;
+case UtpQueryFuncOpcodeWrDesc:
+ ReturnDataSize = QueryResp->Tsf.Length;
+ SwapLittleEndianToBigEndian ((UINT8 *)&ReturnDataSize, sizeof (UINT16));
+ Packet->OutTransferLength = ReturnDataSize;
+ break;
+case UtpQueryFuncOpcodeRdFlag:
+case UtpQueryFuncOpcodeSetFlag:
+case UtpQueryFuncOpcodeClrFlag:
+case UtpQueryFuncOpcodeTogFlag:
+ //
+ // The 'FLAG VALUE' field is at byte offset 3 of QueryResp->Tsf.Value
+ //
+ *((UINT8 *)(Packet->OutDataBuffer)) = *((UINT8 *)&(QueryResp->
[edk2-devel] [PATCH 3/3] MdeModulePkg: Put off UFS HCS.DP checking to fix timing problem
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3776
Put off UFS HCS.DP (Device Attached) checking
until UfsDeviceDetection() to fix timing problem.
Cc: Hao A Wu
Cc: Ray Ni
Cc: Ian Chiu
Cc: Maggie Chu
Signed-off-by: VincentX Ke
---
MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c | 47 +
1 file changed, 19 insertions(+), 28 deletions(-)
diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
index 4305fab7dc..d0f14d511c 100644
--- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
+++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
@@ -1353,23 +1353,6 @@ UfsExecUicCommands (
}
}
- //
- // Check value of HCS.DP and make sure that there is a device attached to
the Link.
- //
- Address = UfsHcBase + UFS_HC_STATUS_OFFSET;
- Data= MmioRead32 (Address);
- if ((Data & UFS_HC_HCS_DP) == 0) {
-Address = UfsHcBase + UFS_HC_IS_OFFSET;
-Status = UfsWaitMemSet (Address, UFS_HC_IS_ULSS, UFS_HC_IS_ULSS,
UFS_TIMEOUT);
-if (EFI_ERROR (Status)) {
- return EFI_DEVICE_ERROR;
-}
-
-return EFI_NOT_FOUND;
- }
-
- DEBUG ((DEBUG_INFO, "UfsblockioPei: found a attached UFS device\n"));
-
return EFI_SUCCESS;
}
@@ -1443,6 +1426,8 @@ UfsDeviceDetection (
)
{
UINTN Retry;
+ UINTN Address;
+ UINT32 Data;
EFI_STATUS Status;
//
@@ -1451,22 +1436,28 @@ UfsDeviceDetection (
//
for (Retry = 0; Retry < 3; Retry++) {
Status = UfsExecUicCommands (Private, UfsUicDmeLinkStartup, 0, 0, 0);
-if (!EFI_ERROR (Status)) {
- break;
+if (EFI_ERROR (Status)) {
+ return EFI_DEVICE_ERROR;
}
-if (Status == EFI_NOT_FOUND) {
- continue;
+//
+// Check value of HCS.DP and make sure that there is a device attached to
the Link
+//
+Address = Private->UfsHcBase + UFS_HC_STATUS_OFFSET;
+Data= MmioRead32 (Address);
+if ((Data & UFS_HC_HCS_DP) == 0) {
+ Address = Private->UfsHcBase + UFS_HC_IS_OFFSET;
+ Status = UfsWaitMemSet (Address, UFS_HC_IS_ULSS, UFS_HC_IS_ULSS,
UFS_TIMEOUT);
+ if (EFI_ERROR (Status)) {
+return EFI_DEVICE_ERROR;
+ }
+} else {
+ DEBUG ((DEBUG_INFO, "UfsblockioPei: found a attached UFS device\n"));
+ return EFI_SUCCESS;
}
-
-return EFI_DEVICE_ERROR;
}
- if (Retry == 3) {
-return EFI_NOT_FOUND;
- }
-
- return EFI_SUCCESS;
+ return EFI_NOT_FOUND;
}
/**
--
2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#85170): https://edk2.groups.io/g/devel/message/85170
Mute This Topic: https://groups.io/mt/87895212/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v6 0/3] MdeModulePkg: Fix UfsBlockIoPei timing problem
*** BLURB HERE *** VincentX Ke (3): MdeModulePkg: Replace with UFS_UNIT_DESC to fix timeout problem MdeModulePkg: Refactoring UFS DME request and fix timing problem MdeModulePkg: Put off UFS HCS.DP checking to fix timing problem .../Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c | 23 +- MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c | 359 ++ 2 files changed, 209 insertions(+), 173 deletions(-) -- 2.31.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#85185): https://edk2.groups.io/g/devel/message/85185 Mute This Topic: https://groups.io/mt/87913150/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v6 1/3] MdeModulePkg: Replace with UFS_UNIT_DESC to fix timeout problem
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3714
Replace with UFS_UNIT_DESC to fix response timeout problem.
Cc: Hao A Wu
Cc: Ray Ni
Cc: Ian Chiu
Cc: Maggie Chu
Signed-off-by: VincentX Ke
---
.../Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c | 23 +--
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
index b331c0f3e3..b8651ff998 100644
--- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
+++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
@@ -1,6 +1,6 @@
/** @file
- Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved.
+ Copyright (c) 2014 - 2021, Intel Corporation. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -1037,9 +1037,9 @@ InitializeUfsBlockIoPeim (
UFS_PEIM_HC_PRIVATE_DATA *Private;
EDKII_UFS_HOST_CONTROLLER_PPI *UfsHcPpi;
UINT32 Index;
- UFS_CONFIG_DESCConfig;
UINTN MmioBase;
UINT8 Controller;
+ UFS_UNIT_DESC UnitDescriptor;
//
// Shadow this PEIM to run from memory
@@ -1126,19 +1126,18 @@ InitializeUfsBlockIoPeim (
}
//
-// Get Ufs Device's Lun Info by reading Configuration Descriptor.
+// Check if 8 common luns are active and set corresponding bit mask.
//
-Status = UfsRwDeviceDesc (Private, TRUE, UfsConfigDesc, 0, 0, &Config,
sizeof (UFS_CONFIG_DESC));
-if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_ERROR, "Ufs Get Configuration Descriptor Error, Status =
%r\n", Status));
- Controller++;
- continue;
-}
-
for (Index = 0; Index < UFS_PEIM_MAX_LUNS; Index++) {
- if (Config.UnitDescConfParams[Index].LunEn != 0) {
-Private->Luns.BitMask |= (BIT0 << Index);
+ Status = UfsRwDeviceDesc (Private, TRUE, UfsUnitDesc, (UINT8)Index, 0,
&UnitDescriptor, sizeof (UFS_UNIT_DESC));
+ if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "Fail to read UFS Unit Descriptor, Index = %X,
Status = %r\n", Index, Status));
+continue;
+ }
+
+ if (UnitDescriptor.LunEn == 0x1) {
DEBUG ((DEBUG_INFO, "Ufs %d Lun %d is enabled\n", Controller, Index));
+Private->Luns.BitMask |= (BIT0 << Index);
}
}
--
2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#85186): https://edk2.groups.io/g/devel/message/85186
Mute This Topic: https://groups.io/mt/87913152/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v6 2/3] MdeModulePkg: Refactoring UFS DME request and fix timing problem
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3775
Refactoring UFS DME request function and retry up to 5 times.
Cc: Hao A Wu
Cc: Ray Ni
Cc: Ian Chiu
Cc: Maggie Chu
Signed-off-by: VincentX Ke
---
MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c | 312 +++-
1 file changed, 179 insertions(+), 133 deletions(-)
diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
index 2baa57593e..cffe8e02a7 100644
--- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
+++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
@@ -1,6 +1,6 @@
/** @file
- Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved.
+ Copyright (c) 2014 - 2021, Intel Corporation. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -557,7 +557,7 @@ UfsCreateDMCommandDesc (
return EFI_INVALID_PARAMETER;
}
- if ( ((Opcode == UtpQueryFuncOpcodeSetFlag) || (Opcode ==
UtpQueryFuncOpcodeClrFlag) || (Opcode == UtpQueryFuncOpcodeTogFlag))
+ if ( ((Opcode == UtpQueryFuncOpcodeClrFlag) || (Opcode ==
UtpQueryFuncOpcodeTogFlag))
&& ((DataSize != 0) || (Data != NULL)))
{
return EFI_INVALID_PARAMETER;
@@ -747,60 +747,91 @@ UfsStopExecCmd (
}
/**
- Read or write specified device descriptor of a UFS device.
+ Extracts return data from query response upiu.
- @param[in] Private The pointer to the UFS_PEIM_HC_PRIVATE_DATA
data structure.
- @param[in] Read The boolean variable to show r/w direction.
- @param[in] DescIdThe ID of device descriptor.
- @param[in] Index The Index of device descriptor.
- @param[in] Selector The Selector of device descriptor.
- @param[in, out] DescriptorThe buffer of device descriptor to be read or
written.
- @param[in] DescSize The size of device descriptor buffer.
+ @param[in, out] PacketPointer to the
UFS_DEVICE_MANAGEMENT_REQUEST_PACKET.
+ @param[in] QueryResp Pointer to the query response.
- @retval EFI_SUCCESS The device descriptor was read/written
successfully.
- @retval EFI_DEVICE_ERROR A device error occurred while attempting to
r/w the device descriptor.
- @retval EFI_TIMEOUT A timeout occurred while waiting for the
completion of r/w the device descriptor.
+ @retval EFI_INVALID_PARAMETER Packet or QueryResp are empty or opcode is
invalid.
+ @retval EFI_DEVICE_ERROR Data returned from device is invalid.
+ @retval EFI_SUCCESS Data extracted.
**/
EFI_STATUS
-UfsRwDeviceDesc (
- IN UFS_PEIM_HC_PRIVATE_DATA *Private,
- IN BOOLEAN Read,
- IN UINT8 DescId,
- IN UINT8 Index,
- IN UINT8 Selector,
- IN OUT VOID *Descriptor,
- IN UINT32DescSize
+UfsGetReturnDataFromQueryResponse (
+ IN OUT UFS_DEVICE_MANAGEMENT_REQUEST_PACKET *Packet,
+ IN UTP_QUERY_RESP_UPIU *QueryResp
)
{
- EFI_STATUSStatus;
- UFS_DEVICE_MANAGEMENT_REQUEST_PACKET Packet;
- UINT8 Slot;
- UTP_TRD *Trd;
- UINTN Address;
- UTP_QUERY_RESP_UPIU *QueryResp;
- UINT8 *CmdDescBase;
- UINT32CmdDescSize;
- UINT16ReturnDataSize;
+ UINT16 ReturnDataSize;
- ZeroMem (&Packet, sizeof (UFS_DEVICE_MANAGEMENT_REQUEST_PACKET));
+ ReturnDataSize = 0;
- if (Read) {
-Packet.DataDirection= UfsDataIn;
-Packet.InDataBuffer = Descriptor;
-Packet.InTransferLength = DescSize;
-Packet.Opcode = UtpQueryFuncOpcodeRdDesc;
- } else {
-Packet.DataDirection = UfsDataOut;
-Packet.OutDataBuffer = Descriptor;
-Packet.OutTransferLength = DescSize;
-Packet.Opcode= UtpQueryFuncOpcodeWrDesc;
+ if ((Packet == NULL) || (QueryResp == NULL)) {
+return EFI_INVALID_PARAMETER;
}
- Packet.DescId = DescId;
- Packet.Index= Index;
- Packet.Selector = Selector;
- Packet.Timeout = UFS_TIMEOUT;
+ switch (Packet->Opcode) {
+case UtpQueryFuncOpcodeRdDesc:
+ ReturnDataSize = QueryResp->Tsf.Length;
+ SwapLittleEndianToBigEndian ((UINT8 *)&ReturnDataSize, sizeof (UINT16));
+ //
+ // Make sure the hardware device does not return more data than expected.
+ //
+ if (ReturnDataSize > Packet->InTransferLength) {
+return EFI_DEVICE_ERROR;
+ }
+
+ CopyMem (Packet->InDataBuffer, (QueryResp + 1), ReturnDataSize);
+ Packet->InTransferLength = ReturnDataSize;
+ break;
+case UtpQueryFuncOpcodeWrDesc:
+ ReturnDataSize = QueryResp->Tsf.Length;
+ SwapLittleEndianToBigEndian ((UINT8 *)&ReturnDataSize, sizeof (U
[edk2-devel] [PATCH v6 3/3] MdeModulePkg: Put off UFS HCS.DP checking to fix timing problem
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3776
Put off UFS HCS.DP (Device Attached) checking
until UfsDeviceDetection() to fix timing problem.
Cc: Hao A Wu
Cc: Ray Ni
Cc: Ian Chiu
Cc: Maggie Chu
Signed-off-by: VincentX Ke
---
MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c | 47 +
1 file changed, 19 insertions(+), 28 deletions(-)
diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
index cffe8e02a7..864bf6928d 100644
--- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
+++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
@@ -1353,23 +1353,6 @@ UfsExecUicCommands (
}
}
- //
- // Check value of HCS.DP and make sure that there is a device attached to
the Link.
- //
- Address = UfsHcBase + UFS_HC_STATUS_OFFSET;
- Data= MmioRead32 (Address);
- if ((Data & UFS_HC_HCS_DP) == 0) {
-Address = UfsHcBase + UFS_HC_IS_OFFSET;
-Status = UfsWaitMemSet (Address, UFS_HC_IS_ULSS, UFS_HC_IS_ULSS,
UFS_TIMEOUT);
-if (EFI_ERROR (Status)) {
- return EFI_DEVICE_ERROR;
-}
-
-return EFI_NOT_FOUND;
- }
-
- DEBUG ((DEBUG_INFO, "UfsblockioPei: found a attached UFS device\n"));
-
return EFI_SUCCESS;
}
@@ -1443,6 +1426,8 @@ UfsDeviceDetection (
)
{
UINTN Retry;
+ UINTN Address;
+ UINT32 Data;
EFI_STATUS Status;
//
@@ -1451,22 +1436,28 @@ UfsDeviceDetection (
//
for (Retry = 0; Retry < 3; Retry++) {
Status = UfsExecUicCommands (Private, UfsUicDmeLinkStartup, 0, 0, 0);
-if (!EFI_ERROR (Status)) {
- break;
+if (EFI_ERROR (Status)) {
+ return EFI_DEVICE_ERROR;
}
-if (Status == EFI_NOT_FOUND) {
- continue;
+//
+// Check value of HCS.DP and make sure that there is a device attached to
the Link
+//
+Address = Private->UfsHcBase + UFS_HC_STATUS_OFFSET;
+Data= MmioRead32 (Address);
+if ((Data & UFS_HC_HCS_DP) == 0) {
+ Address = Private->UfsHcBase + UFS_HC_IS_OFFSET;
+ Status = UfsWaitMemSet (Address, UFS_HC_IS_ULSS, UFS_HC_IS_ULSS,
UFS_TIMEOUT);
+ if (EFI_ERROR (Status)) {
+return EFI_DEVICE_ERROR;
+ }
+} else {
+ DEBUG ((DEBUG_INFO, "UfsblockioPei: found a attached UFS device\n"));
+ return EFI_SUCCESS;
}
-
-return EFI_DEVICE_ERROR;
}
- if (Retry == 3) {
-return EFI_NOT_FOUND;
- }
-
- return EFI_SUCCESS;
+ return EFI_NOT_FOUND;
}
/**
--
2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#85188): https://edk2.groups.io/g/devel/message/85188
Mute This Topic: https://groups.io/mt/87913160/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v7 0/3] MdeModulePkg: Fix UfsBlockIoPei timing problem
*** BLURB HERE *** VincentX Ke (3): MdeModulePkg: Replace with UFS_UNIT_DESC to fix timeout problem MdeModulePkg: Refactoring UFS DME request and fix timing problem MdeModulePkg: Put off UFS HCS.DP checking to fix timing problem .../Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c | 23 +- MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c | 373 ++ 2 files changed, 217 insertions(+), 179 deletions(-) -- 2.31.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#85193): https://edk2.groups.io/g/devel/message/85193 Mute This Topic: https://groups.io/mt/87914345/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v7 1/3] MdeModulePkg: Replace with UFS_UNIT_DESC to fix timeout problem
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3714
Replace with UFS_UNIT_DESC to fix response timeout problem.
Cc: Hao A Wu
Cc: Ray Ni
Cc: Ian Chiu
Cc: Maggie Chu
Signed-off-by: VincentX Ke
---
.../Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c | 23 +--
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
index b331c0f3e3..b8651ff998 100644
--- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
+++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
@@ -1,6 +1,6 @@
/** @file
- Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved.
+ Copyright (c) 2014 - 2021, Intel Corporation. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -1037,9 +1037,9 @@ InitializeUfsBlockIoPeim (
UFS_PEIM_HC_PRIVATE_DATA *Private;
EDKII_UFS_HOST_CONTROLLER_PPI *UfsHcPpi;
UINT32 Index;
- UFS_CONFIG_DESCConfig;
UINTN MmioBase;
UINT8 Controller;
+ UFS_UNIT_DESC UnitDescriptor;
//
// Shadow this PEIM to run from memory
@@ -1126,19 +1126,18 @@ InitializeUfsBlockIoPeim (
}
//
-// Get Ufs Device's Lun Info by reading Configuration Descriptor.
+// Check if 8 common luns are active and set corresponding bit mask.
//
-Status = UfsRwDeviceDesc (Private, TRUE, UfsConfigDesc, 0, 0, &Config,
sizeof (UFS_CONFIG_DESC));
-if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_ERROR, "Ufs Get Configuration Descriptor Error, Status =
%r\n", Status));
- Controller++;
- continue;
-}
-
for (Index = 0; Index < UFS_PEIM_MAX_LUNS; Index++) {
- if (Config.UnitDescConfParams[Index].LunEn != 0) {
-Private->Luns.BitMask |= (BIT0 << Index);
+ Status = UfsRwDeviceDesc (Private, TRUE, UfsUnitDesc, (UINT8)Index, 0,
&UnitDescriptor, sizeof (UFS_UNIT_DESC));
+ if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "Fail to read UFS Unit Descriptor, Index = %X,
Status = %r\n", Index, Status));
+continue;
+ }
+
+ if (UnitDescriptor.LunEn == 0x1) {
DEBUG ((DEBUG_INFO, "Ufs %d Lun %d is enabled\n", Controller, Index));
+Private->Luns.BitMask |= (BIT0 << Index);
}
}
--
2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#85194): https://edk2.groups.io/g/devel/message/85194
Mute This Topic: https://groups.io/mt/87914346/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v7 3/3] MdeModulePkg: Put off UFS HCS.DP checking to fix timing problem
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3776
Put off UFS HCS.DP (Device Attached) checking
until UfsDeviceDetection() to fix timing problem.
Cc: Hao A Wu
Cc: Ray Ni
Cc: Ian Chiu
Cc: Maggie Chu
Signed-off-by: VincentX Ke
---
MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c | 47 +
1 file changed, 19 insertions(+), 28 deletions(-)
diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
index 409ea283f5..d19a7fed6e 100644
--- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
+++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
@@ -1355,23 +1355,6 @@ UfsExecUicCommands (
}
}
- //
- // Check value of HCS.DP and make sure that there is a device attached to
the Link.
- //
- Address = UfsHcBase + UFS_HC_STATUS_OFFSET;
- Data= MmioRead32 (Address);
- if ((Data & UFS_HC_HCS_DP) == 0) {
-Address = UfsHcBase + UFS_HC_IS_OFFSET;
-Status = UfsWaitMemSet (Address, UFS_HC_IS_ULSS, UFS_HC_IS_ULSS,
UFS_TIMEOUT);
-if (EFI_ERROR (Status)) {
- return EFI_DEVICE_ERROR;
-}
-
-return EFI_NOT_FOUND;
- }
-
- DEBUG ((DEBUG_INFO, "UfsblockioPei: found a attached UFS device\n"));
-
return EFI_SUCCESS;
}
@@ -1445,6 +1428,8 @@ UfsDeviceDetection (
)
{
UINTN Retry;
+ UINTN Address;
+ UINT32 Data;
EFI_STATUS Status;
//
@@ -1453,22 +1438,28 @@ UfsDeviceDetection (
//
for (Retry = 0; Retry < 3; Retry++) {
Status = UfsExecUicCommands (Private, UfsUicDmeLinkStartup, 0, 0, 0);
-if (!EFI_ERROR (Status)) {
- break;
+if (EFI_ERROR (Status)) {
+ return EFI_DEVICE_ERROR;
}
-if (Status == EFI_NOT_FOUND) {
- continue;
+//
+// Check value of HCS.DP and make sure that there is a device attached to
the Link
+//
+Address = Private->UfsHcBase + UFS_HC_STATUS_OFFSET;
+Data= MmioRead32 (Address);
+if ((Data & UFS_HC_HCS_DP) == 0) {
+ Address = Private->UfsHcBase + UFS_HC_IS_OFFSET;
+ Status = UfsWaitMemSet (Address, UFS_HC_IS_ULSS, UFS_HC_IS_ULSS,
UFS_TIMEOUT);
+ if (EFI_ERROR (Status)) {
+return EFI_DEVICE_ERROR;
+ }
+} else {
+ DEBUG ((DEBUG_INFO, "UfsblockioPei: found a attached UFS device\n"));
+ return EFI_SUCCESS;
}
-
-return EFI_DEVICE_ERROR;
}
- if (Retry == 3) {
-return EFI_NOT_FOUND;
- }
-
- return EFI_SUCCESS;
+ return EFI_NOT_FOUND;
}
/**
--
2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#85196): https://edk2.groups.io/g/devel/message/85196
Mute This Topic: https://groups.io/mt/87914350/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v7 2/3] MdeModulePkg: Refactoring UFS DME request and fix timing problem
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3775
Refactoring UFS DME request function and retry up to 5 times.
Cc: Hao A Wu
Cc: Ray Ni
Cc: Ian Chiu
Cc: Maggie Chu
Signed-off-by: VincentX Ke
---
MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c | 326 +++-
1 file changed, 187 insertions(+), 139 deletions(-)
diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
index 2baa57593e..409ea283f5 100644
--- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
+++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
@@ -1,6 +1,6 @@
/** @file
- Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved.
+ Copyright (c) 2014 - 2021, Intel Corporation. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -551,14 +551,9 @@ UfsCreateDMCommandDesc (
Data = NULL;
}
- if ( ((Opcode != UtpQueryFuncOpcodeSetFlag) && (Opcode !=
UtpQueryFuncOpcodeClrFlag) && (Opcode != UtpQueryFuncOpcodeTogFlag))
- && ((DataSize == 0) || (Data == NULL)))
- {
-return EFI_INVALID_PARAMETER;
- }
-
- if ( ((Opcode == UtpQueryFuncOpcodeSetFlag) || (Opcode ==
UtpQueryFuncOpcodeClrFlag) || (Opcode == UtpQueryFuncOpcodeTogFlag))
- && ((DataSize != 0) || (Data != NULL)))
+ if (((Opcode != UtpQueryFuncOpcodeRdFlag) && (Opcode !=
UtpQueryFuncOpcodeSetFlag) &&
+ (Opcode != UtpQueryFuncOpcodeClrFlag) && (Opcode !=
UtpQueryFuncOpcodeTogFlag)) &&
+ ((DataSize == 0) || (Data == NULL)))
{
return EFI_INVALID_PARAMETER;
}
@@ -747,60 +742,96 @@ UfsStopExecCmd (
}
/**
- Read or write specified device descriptor of a UFS device.
+ Extracts return data from query response upiu.
- @param[in] Private The pointer to the UFS_PEIM_HC_PRIVATE_DATA
data structure.
- @param[in] Read The boolean variable to show r/w direction.
- @param[in] DescIdThe ID of device descriptor.
- @param[in] Index The Index of device descriptor.
- @param[in] Selector The Selector of device descriptor.
- @param[in, out] DescriptorThe buffer of device descriptor to be read or
written.
- @param[in] DescSize The size of device descriptor buffer.
+ @param[in, out] PacketPointer to the
UFS_DEVICE_MANAGEMENT_REQUEST_PACKET.
+ @param[in] QueryResp Pointer to the query response.
- @retval EFI_SUCCESS The device descriptor was read/written
successfully.
- @retval EFI_DEVICE_ERROR A device error occurred while attempting to
r/w the device descriptor.
- @retval EFI_TIMEOUT A timeout occurred while waiting for the
completion of r/w the device descriptor.
+ @retval EFI_INVALID_PARAMETER Packet or QueryResp are empty or opcode is
invalid.
+ @retval EFI_DEVICE_ERROR Data returned from device is invalid.
+ @retval EFI_SUCCESS Data extracted.
**/
EFI_STATUS
-UfsRwDeviceDesc (
- IN UFS_PEIM_HC_PRIVATE_DATA *Private,
- IN BOOLEAN Read,
- IN UINT8 DescId,
- IN UINT8 Index,
- IN UINT8 Selector,
- IN OUT VOID *Descriptor,
- IN UINT32DescSize
+UfsGetReturnDataFromQueryResponse (
+ IN OUT UFS_DEVICE_MANAGEMENT_REQUEST_PACKET *Packet,
+ IN UTP_QUERY_RESP_UPIU *QueryResp
)
{
- EFI_STATUSStatus;
- UFS_DEVICE_MANAGEMENT_REQUEST_PACKET Packet;
- UINT8 Slot;
- UTP_TRD *Trd;
- UINTN Address;
- UTP_QUERY_RESP_UPIU *QueryResp;
- UINT8 *CmdDescBase;
- UINT32CmdDescSize;
- UINT16ReturnDataSize;
+ UINT16 ReturnDataSize;
- ZeroMem (&Packet, sizeof (UFS_DEVICE_MANAGEMENT_REQUEST_PACKET));
+ ReturnDataSize = 0;
- if (Read) {
-Packet.DataDirection= UfsDataIn;
-Packet.InDataBuffer = Descriptor;
-Packet.InTransferLength = DescSize;
-Packet.Opcode = UtpQueryFuncOpcodeRdDesc;
- } else {
-Packet.DataDirection = UfsDataOut;
-Packet.OutDataBuffer = Descriptor;
-Packet.OutTransferLength = DescSize;
-Packet.Opcode= UtpQueryFuncOpcodeWrDesc;
+ if ((Packet == NULL) || (QueryResp == NULL)) {
+return EFI_INVALID_PARAMETER;
}
- Packet.DescId = DescId;
- Packet.Index= Index;
- Packet.Selector = Selector;
- Packet.Timeout = UFS_TIMEOUT;
+ switch (Packet->Opcode) {
+case UtpQueryFuncOpcodeRdDesc:
+ ReturnDataSize = QueryResp->Tsf.Length;
+ SwapLittleEndianToBigEndian ((UINT8 *)&ReturnDataSize, sizeof (UINT16));
+ //
+ // Make sure the hardware device does not return more data than expected.
+ //
+ if
