[edk2-devel] [PATCH 1/4] BaseTools: Remove logic to create AP waking vector in GenFv

Zhiguang Liu posted 4 patches 2 years, 7 months ago
[edk2-devel] [PATCH 1/4] BaseTools: Remove logic to create AP waking vector in GenFv
Posted by Zhiguang Liu 2 years, 7 months ago
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4494

Today for SEC core(not VTF-0), GenFv finds free 4K aligned space in
FV for AP waking vector and JMP to 4G-30h in the waking vector.
There is no usage of this today. Remove the logic to avoid confusing
and save spaces in reset vector.

Cc: Rebecca Cran <rebecca@bsdio.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Yuwei Chen <yuwei.chen@intel.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
 BaseTools/Source/C/GenFv/GenFvInternalLib.c | 199 --------------------
 1 file changed, 199 deletions(-)

diff --git a/BaseTools/Source/C/GenFv/GenFvInternalLib.c b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
index f466324d61..29c3363a50 100644
--- a/BaseTools/Source/C/GenFv/GenFvInternalLib.c
+++ b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
@@ -116,63 +116,6 @@ CHAR8      *mFvbAlignmentName[] = {
   EFI_FVB2_ALIGNMENT_2G_STRING
 };
 
-//
-// This data array will be located at the base of the Firmware Volume Header (FVH)
-// in the boot block.  It must not exceed 14 bytes of code.  The last 2 bytes
-// will be used to keep the FVH checksum consistent.
-// This code will be run in response to a startup IPI for HT-enabled systems.
-//
-#define SIZEOF_STARTUP_DATA_ARRAY 0x10
-
-UINT8                                   m128kRecoveryStartupApDataArray[SIZEOF_STARTUP_DATA_ARRAY] = {
-  //
-  // EA D0 FF 00 F0               ; far jmp F000:FFD0
-  // 0, 0, 0, 0, 0, 0, 0, 0, 0,   ; Reserved bytes
-  // 0, 0                         ; Checksum Padding
-  //
-  0xEA,
-  0xD0,
-  0xFF,
-  0x0,
-  0xF0,
-  0x00,
-  0x00,
-  0x00,
-  0x00,
-  0x00,
-  0x00,
-  0x00,
-  0x00,
-  0x00,
-  0x00,
-  0x00
-};
-
-UINT8                                   m64kRecoveryStartupApDataArray[SIZEOF_STARTUP_DATA_ARRAY] = {
-  //
-  // EB CE                               ; jmp short ($-0x30)
-  // ; (from offset 0x0 to offset 0xFFD0)
-  // 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, ; Reserved bytes
-  // 0, 0                                ; Checksum Padding
-  //
-  0xEB,
-  0xCE,
-  0x00,
-  0x00,
-  0x00,
-  0x00,
-  0x00,
-  0x00,
-  0x00,
-  0x00,
-  0x00,
-  0x00,
-  0x00,
-  0x00,
-  0x00,
-  0x00
-};
-
 FV_INFO                     mFvDataInfo;
 CAP_INFO                    mCapDataInfo;
 BOOLEAN                     mIsLargeFfs = FALSE;
@@ -1568,12 +1511,6 @@ Returns:
   EFI_PHYSICAL_ADDRESS      SecCorePhysicalAddress;
   INT32                     Ia32SecEntryOffset;
   UINT32                    *Ia32ResetAddressPtr;
-  UINT8                     *BytePointer;
-  UINT8                     *BytePointer2;
-  UINT16                    *WordPointer;
-  UINT16                    CheckSum;
-  UINT32                    IpiVector;
-  UINTN                     Index;
   EFI_FFS_FILE_STATE        SavedState;
   BOOLEAN                   Vtf0Detected;
   UINT32                    FfsHeaderSize;
@@ -1745,65 +1682,6 @@ if (MachineType == IMAGE_FILE_MACHINE_I386 || MachineType == IMAGE_FILE_MACHINE_
     Ia32ResetAddressPtr   = (UINT32 *) ((UINTN) FvImage->Eof - 4);
     *Ia32ResetAddressPtr  = (UINT32) (FvInfo->BaseAddress);
     DebugMsg (NULL, 0, 9, "update BFV base address in the top FV image", "BFV base address = 0x%llX.", (unsigned long long) FvInfo->BaseAddress);
-
-    //
-    // Update the Startup AP in the FVH header block ZeroVector region.
-    //
-    BytePointer   = (UINT8 *) ((UINTN) FvImage->FileImage);
-    if (FvInfo->Size <= 0x10000) {
-      BytePointer2 = m64kRecoveryStartupApDataArray;
-    } else if (FvInfo->Size <= 0x20000) {
-      BytePointer2 = m128kRecoveryStartupApDataArray;
-    } else {
-      BytePointer2 = m128kRecoveryStartupApDataArray;
-      //
-      // Find the position to place Ap reset vector, the offset
-      // between the position and the end of Fvrecovery.fv file
-      // should not exceed 128kB to prevent Ap reset vector from
-      // outside legacy E and F segment
-      //
-      Status = FindApResetVectorPosition (FvImage, &BytePointer);
-      if (EFI_ERROR (Status)) {
-        Error (NULL, 0, 3000, "Invalid", "FV image does not have enough space to place AP reset vector. The FV image needs to reserve at least 4KB of unused space.");
-        return EFI_ABORTED;
-      }
-    }
-
-    for (Index = 0; Index < SIZEOF_STARTUP_DATA_ARRAY; Index++) {
-      BytePointer[Index] = BytePointer2[Index];
-    }
-    //
-    // Calculate the checksum
-    //
-    CheckSum              = 0x0000;
-    WordPointer = (UINT16 *) (BytePointer);
-    for (Index = 0; Index < SIZEOF_STARTUP_DATA_ARRAY / 2; Index++) {
-      CheckSum = (UINT16) (CheckSum + ((UINT16) *WordPointer));
-      WordPointer++;
-    }
-    //
-    // Update the checksum field
-    //
-    WordPointer   = (UINT16 *) (BytePointer + SIZEOF_STARTUP_DATA_ARRAY - 2);
-    *WordPointer  = (UINT16) (0x10000 - (UINT32) CheckSum);
-
-    //
-    // IpiVector at the 4k aligned address in the top 2 blocks in the PEI FV.
-    //
-    IpiVector  = (UINT32) (FV_IMAGES_TOP_ADDRESS - ((UINTN) FvImage->Eof - (UINTN) BytePointer));
-    DebugMsg (NULL, 0, 9, "Startup AP Vector address", "IpiVector at 0x%X", (unsigned) IpiVector);
-    if ((IpiVector & 0xFFF) != 0) {
-      Error (NULL, 0, 3000, "Invalid", "Startup AP Vector address are not 4K aligned, because the FV size is not 4K aligned");
-      return EFI_ABORTED;
-    }
-    IpiVector  = IpiVector >> 12;
-    IpiVector  = IpiVector & 0xFF;
-
-    //
-    // Write IPI Vector at Offset FvrecoveryFileSize - 8
-    //
-    Ia32ResetAddressPtr   = (UINT32 *) ((UINTN) FvImage->Eof - 8);
-    *Ia32ResetAddressPtr  = IpiVector;
   } else if (MachineType == IMAGE_FILE_MACHINE_ARMTHUMB_MIXED) {
     //
     // Since the ARM reset vector is in the FV Header you really don't need a
@@ -4190,83 +4068,6 @@ Returns:
   return EFI_SUCCESS;
 }
 
-EFI_STATUS
-FindApResetVectorPosition (
-  IN  MEMORY_FILE  *FvImage,
-  OUT UINT8        **Pointer
-  )
-/*++
-
-Routine Description:
-
-  Find the position in this FvImage to place Ap reset vector.
-
-Arguments:
-
-  FvImage       Memory file for the FV memory image.
-  Pointer       Pointer to pointer to position.
-
-Returns:
-
-  EFI_NOT_FOUND   - No satisfied position is found.
-  EFI_SUCCESS     - The suitable position is return.
-
---*/
-{
-  EFI_FFS_FILE_HEADER   *PadFile;
-  UINT32                Index;
-  EFI_STATUS            Status;
-  UINT8                 *FixPoint;
-  UINT32                FileLength;
-
-  for (Index = 1; ;Index ++) {
-    //
-    // Find Pad File to add ApResetVector info
-    //
-    Status = GetFileByType (EFI_FV_FILETYPE_FFS_PAD, Index, &PadFile);
-    if (EFI_ERROR (Status) || (PadFile == NULL)) {
-      //
-      // No Pad file to be found.
-      //
-      break;
-    }
-    //
-    // Get Pad file size.
-    //
-    FileLength = GetFfsFileLength(PadFile);
-    FileLength = (FileLength + EFI_FFS_FILE_HEADER_ALIGNMENT - 1) & ~(EFI_FFS_FILE_HEADER_ALIGNMENT - 1);
-    //
-    // FixPoint must be align on 0x1000 relative to FvImage Header
-    //
-    FixPoint = (UINT8*) PadFile + GetFfsHeaderLength(PadFile);
-    FixPoint = FixPoint + 0x1000 - (((UINTN) FixPoint - (UINTN) FvImage->FileImage) & 0xFFF);
-    //
-    // FixPoint be larger at the last place of one fv image.
-    //
-    while (((UINTN) FixPoint + SIZEOF_STARTUP_DATA_ARRAY - (UINTN) PadFile) <= FileLength) {
-      FixPoint += 0x1000;
-    }
-    FixPoint -= 0x1000;
-
-    if ((UINTN) FixPoint < ((UINTN) PadFile + GetFfsHeaderLength(PadFile))) {
-      //
-      // No alignment FixPoint in this Pad File.
-      //
-      continue;
-    }
-
-    if ((UINTN) FvImage->Eof - (UINTN)FixPoint <= 0x20000) {
-      //
-      // Find the position to place ApResetVector
-      //
-      *Pointer = FixPoint;
-      return EFI_SUCCESS;
-    }
-  }
-
-  return EFI_NOT_FOUND;
-}
-
 EFI_STATUS
 ParseCapInf (
   IN  MEMORY_FILE  *InfFile,
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106758): https://edk2.groups.io/g/devel/message/106758
Mute This Topic: https://groups.io/mt/100051788/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
回复: [edk2-devel] [PATCH 1/4] BaseTools: Remove logic to create AP waking vector in GenFv
Posted by gaoliming via groups.io 2 years, 7 months ago
Zhiguang:
  If there is no such usage any more, I agree to remove this logic.
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

Thanks
Liming
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Zhiguang Liu
> 发送时间: 2023年7月10日 11:17
> 收件人: devel@edk2.groups.io
> 抄送: Zhiguang Liu <zhiguang.liu@intel.com>; Rebecca Cran
> <rebecca@bsdio.com>; Liming Gao <gaoliming@byosoft.com.cn>; Bob Feng
> <bob.c.feng@intel.com>; Yuwei Chen <yuwei.chen@intel.com>
> 主题: [edk2-devel] [PATCH 1/4] BaseTools: Remove logic to create AP waking
> vector in GenFv
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4494
> 
> Today for SEC core(not VTF-0), GenFv finds free 4K aligned space in
> FV for AP waking vector and JMP to 4G-30h in the waking vector.
> There is no usage of this today. Remove the logic to avoid confusing
> and save spaces in reset vector.
> 
> Cc: Rebecca Cran <rebecca@bsdio.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Bob Feng <bob.c.feng@intel.com>
> Cc: Yuwei Chen <yuwei.chen@intel.com>
> Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
> ---
>  BaseTools/Source/C/GenFv/GenFvInternalLib.c | 199 --------------------
>  1 file changed, 199 deletions(-)
> 
> diff --git a/BaseTools/Source/C/GenFv/GenFvInternalLib.c
> b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
> index f466324d61..29c3363a50 100644
> --- a/BaseTools/Source/C/GenFv/GenFvInternalLib.c
> +++ b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
> @@ -116,63 +116,6 @@ CHAR8      *mFvbAlignmentName[] = {
>    EFI_FVB2_ALIGNMENT_2G_STRING
>  };
> 
> -//
> -// This data array will be located at the base of the Firmware Volume
Header
> (FVH)
> -// in the boot block.  It must not exceed 14 bytes of code.  The last 2
bytes
> -// will be used to keep the FVH checksum consistent.
> -// This code will be run in response to a startup IPI for HT-enabled
systems.
> -//
> -#define SIZEOF_STARTUP_DATA_ARRAY 0x10
> -
> -UINT8
> m128kRecoveryStartupApDataArray[SIZEOF_STARTUP_DATA_ARRAY] = {
> -  //
> -  // EA D0 FF 00 F0               ; far jmp F000:FFD0
> -  // 0, 0, 0, 0, 0, 0, 0, 0, 0,   ; Reserved bytes
> -  // 0, 0                         ; Checksum Padding
> -  //
> -  0xEA,
> -  0xD0,
> -  0xFF,
> -  0x0,
> -  0xF0,
> -  0x00,
> -  0x00,
> -  0x00,
> -  0x00,
> -  0x00,
> -  0x00,
> -  0x00,
> -  0x00,
> -  0x00,
> -  0x00,
> -  0x00
> -};
> -
> -UINT8
> m64kRecoveryStartupApDataArray[SIZEOF_STARTUP_DATA_ARRAY] = {
> -  //
> -  // EB CE                               ; jmp short ($-0x30)
> -  // ; (from offset 0x0 to offset 0xFFD0)
> -  // 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, ; Reserved bytes
> -  // 0, 0                                ; Checksum Padding
> -  //
> -  0xEB,
> -  0xCE,
> -  0x00,
> -  0x00,
> -  0x00,
> -  0x00,
> -  0x00,
> -  0x00,
> -  0x00,
> -  0x00,
> -  0x00,
> -  0x00,
> -  0x00,
> -  0x00,
> -  0x00,
> -  0x00
> -};
> -
>  FV_INFO                     mFvDataInfo;
>  CAP_INFO                    mCapDataInfo;
>  BOOLEAN                     mIsLargeFfs = FALSE;
> @@ -1568,12 +1511,6 @@ Returns:
>    EFI_PHYSICAL_ADDRESS      SecCorePhysicalAddress;
>    INT32                     Ia32SecEntryOffset;
>    UINT32                    *Ia32ResetAddressPtr;
> -  UINT8                     *BytePointer;
> -  UINT8                     *BytePointer2;
> -  UINT16                    *WordPointer;
> -  UINT16                    CheckSum;
> -  UINT32                    IpiVector;
> -  UINTN                     Index;
>    EFI_FFS_FILE_STATE        SavedState;
>    BOOLEAN                   Vtf0Detected;
>    UINT32                    FfsHeaderSize;
> @@ -1745,65 +1682,6 @@ if (MachineType == IMAGE_FILE_MACHINE_I386
> || MachineType == IMAGE_FILE_MACHINE_
>      Ia32ResetAddressPtr   = (UINT32 *) ((UINTN) FvImage->Eof - 4);
>      *Ia32ResetAddressPtr  = (UINT32) (FvInfo->BaseAddress);
>      DebugMsg (NULL, 0, 9, "update BFV base address in the top FV image",
> "BFV base address = 0x%llX.", (unsigned long long) FvInfo->BaseAddress);
> -
> -    //
> -    // Update the Startup AP in the FVH header block ZeroVector region.
> -    //
> -    BytePointer   = (UINT8 *) ((UINTN) FvImage->FileImage);
> -    if (FvInfo->Size <= 0x10000) {
> -      BytePointer2 = m64kRecoveryStartupApDataArray;
> -    } else if (FvInfo->Size <= 0x20000) {
> -      BytePointer2 = m128kRecoveryStartupApDataArray;
> -    } else {
> -      BytePointer2 = m128kRecoveryStartupApDataArray;
> -      //
> -      // Find the position to place Ap reset vector, the offset
> -      // between the position and the end of Fvrecovery.fv file
> -      // should not exceed 128kB to prevent Ap reset vector from
> -      // outside legacy E and F segment
> -      //
> -      Status = FindApResetVectorPosition (FvImage, &BytePointer);
> -      if (EFI_ERROR (Status)) {
> -        Error (NULL, 0, 3000, "Invalid", "FV image does not have enough
> space to place AP reset vector. The FV image needs to reserve at least 4KB
of
> unused space.");
> -        return EFI_ABORTED;
> -      }
> -    }
> -
> -    for (Index = 0; Index < SIZEOF_STARTUP_DATA_ARRAY; Index++) {
> -      BytePointer[Index] = BytePointer2[Index];
> -    }
> -    //
> -    // Calculate the checksum
> -    //
> -    CheckSum              = 0x0000;
> -    WordPointer = (UINT16 *) (BytePointer);
> -    for (Index = 0; Index < SIZEOF_STARTUP_DATA_ARRAY / 2; Index++) {
> -      CheckSum = (UINT16) (CheckSum + ((UINT16) *WordPointer));
> -      WordPointer++;
> -    }
> -    //
> -    // Update the checksum field
> -    //
> -    WordPointer   = (UINT16 *) (BytePointer +
> SIZEOF_STARTUP_DATA_ARRAY - 2);
> -    *WordPointer  = (UINT16) (0x10000 - (UINT32) CheckSum);
> -
> -    //
> -    // IpiVector at the 4k aligned address in the top 2 blocks in the PEI
FV.
> -    //
> -    IpiVector  = (UINT32) (FV_IMAGES_TOP_ADDRESS - ((UINTN)
> FvImage->Eof - (UINTN) BytePointer));
> -    DebugMsg (NULL, 0, 9, "Startup AP Vector address", "IpiVector at
0x%X",
> (unsigned) IpiVector);
> -    if ((IpiVector & 0xFFF) != 0) {
> -      Error (NULL, 0, 3000, "Invalid", "Startup AP Vector address are not
4K
> aligned, because the FV size is not 4K aligned");
> -      return EFI_ABORTED;
> -    }
> -    IpiVector  = IpiVector >> 12;
> -    IpiVector  = IpiVector & 0xFF;
> -
> -    //
> -    // Write IPI Vector at Offset FvrecoveryFileSize - 8
> -    //
> -    Ia32ResetAddressPtr   = (UINT32 *) ((UINTN) FvImage->Eof - 8);
> -    *Ia32ResetAddressPtr  = IpiVector;
>    } else if (MachineType == IMAGE_FILE_MACHINE_ARMTHUMB_MIXED) {
>      //
>      // Since the ARM reset vector is in the FV Header you really don't
need
> a
> @@ -4190,83 +4068,6 @@ Returns:
>    return EFI_SUCCESS;
>  }
> 
> -EFI_STATUS
> -FindApResetVectorPosition (
> -  IN  MEMORY_FILE  *FvImage,
> -  OUT UINT8        **Pointer
> -  )
> -/*++
> -
> -Routine Description:
> -
> -  Find the position in this FvImage to place Ap reset vector.
> -
> -Arguments:
> -
> -  FvImage       Memory file for the FV memory image.
> -  Pointer       Pointer to pointer to position.
> -
> -Returns:
> -
> -  EFI_NOT_FOUND   - No satisfied position is found.
> -  EFI_SUCCESS     - The suitable position is return.
> -
> ---*/
> -{
> -  EFI_FFS_FILE_HEADER   *PadFile;
> -  UINT32                Index;
> -  EFI_STATUS            Status;
> -  UINT8                 *FixPoint;
> -  UINT32                FileLength;
> -
> -  for (Index = 1; ;Index ++) {
> -    //
> -    // Find Pad File to add ApResetVector info
> -    //
> -    Status = GetFileByType (EFI_FV_FILETYPE_FFS_PAD, Index, &PadFile);
> -    if (EFI_ERROR (Status) || (PadFile == NULL)) {
> -      //
> -      // No Pad file to be found.
> -      //
> -      break;
> -    }
> -    //
> -    // Get Pad file size.
> -    //
> -    FileLength = GetFfsFileLength(PadFile);
> -    FileLength = (FileLength + EFI_FFS_FILE_HEADER_ALIGNMENT - 1) &
> ~(EFI_FFS_FILE_HEADER_ALIGNMENT - 1);
> -    //
> -    // FixPoint must be align on 0x1000 relative to FvImage Header
> -    //
> -    FixPoint = (UINT8*) PadFile + GetFfsHeaderLength(PadFile);
> -    FixPoint = FixPoint + 0x1000 - (((UINTN) FixPoint - (UINTN)
> FvImage->FileImage) & 0xFFF);
> -    //
> -    // FixPoint be larger at the last place of one fv image.
> -    //
> -    while (((UINTN) FixPoint + SIZEOF_STARTUP_DATA_ARRAY - (UINTN)
> PadFile) <= FileLength) {
> -      FixPoint += 0x1000;
> -    }
> -    FixPoint -= 0x1000;
> -
> -    if ((UINTN) FixPoint < ((UINTN) PadFile +
GetFfsHeaderLength(PadFile)))
> {
> -      //
> -      // No alignment FixPoint in this Pad File.
> -      //
> -      continue;
> -    }
> -
> -    if ((UINTN) FvImage->Eof - (UINTN)FixPoint <= 0x20000) {
> -      //
> -      // Find the position to place ApResetVector
> -      //
> -      *Pointer = FixPoint;
> -      return EFI_SUCCESS;
> -    }
> -  }
> -
> -  return EFI_NOT_FOUND;
> -}
> -
>  EFI_STATUS
>  ParseCapInf (
>    IN  MEMORY_FILE  *InfFile,
> --
> 2.31.1.windows.1
> 
> 
> 
> 
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106766): https://edk2.groups.io/g/devel/message/106766
Mute This Topic: https://groups.io/mt/100055736/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-