[1/1] BaseTools: Apply ARM GenFv patch

Message ID 1371800618-14193-1-git-send-email-ryan.harkin@linaro.org
State New
Headers show

Commit Message

Ryan Harkin June 21, 2013, 7:43 a.m.
This patch is needed to fixup some builds, such as Versatile Express A5,
otherwise they hang on boot due to the first instruction being zero.

Signed-off-by: Ryan Harkin <ryan.harkin@linaro.org>
---
 BaseTools/Source/C/GenFv/GenFv.c            |    7 +---
 BaseTools/Source/C/GenFv/GenFvInternalLib.c |   48 +++++++++++----------------
 2 files changed, 21 insertions(+), 34 deletions(-)

Comments

Olivier Martin June 21, 2013, 9:13 a.m. | #1
I also offered a patch to the BaseTools project to fix the firmware when
linked at 0x0 in the past.
But my patch has been rejected and a new keyword has been introduced to fix
this BaseTools limitation.

In your Versatile Express A5 FDF file you have to replace:

[FV.FVMAIN_SEC]
FvAlignment        = 8

By:
[FV.FVMAIN_SEC]
FvBaseAddress      = 0x0
FvForceRebase      = TRUE
FvAlignment        = 8


> -----Original Message-----
> From: boot-architecture-bounces@lists.linaro.org [mailto:boot-
> architecture-bounces@lists.linaro.org] On Behalf Of Ryan Harkin
> Sent: 21 June 2013 08:44
> To: ryan.harkin@linaro.org; edk2-devel@lists.sourceforge.net; edk2-
> buildtools-devel@lists.sourceforge.net; patches@linaro.org; boot-
> architecture@lists.linaro.org
> Subject: [PATCH 1/1] BaseTools: Apply ARM GenFv patch
> 
> This patch is needed to fixup some builds, such as Versatile Express
> A5,
> otherwise they hang on boot due to the first instruction being zero.
> 
> Signed-off-by: Ryan Harkin <ryan.harkin@linaro.org>
> ---
>  BaseTools/Source/C/GenFv/GenFv.c            |    7 +---
>  BaseTools/Source/C/GenFv/GenFvInternalLib.c |   48 +++++++++++--------
> --------
>  2 files changed, 21 insertions(+), 34 deletions(-)
> 
> diff --git a/BaseTools/Source/C/GenFv/GenFv.c
> b/BaseTools/Source/C/GenFv/GenFv.c
> index fa86d00..a68c7b8 100644
> --- a/BaseTools/Source/C/GenFv/GenFv.c
> +++ b/BaseTools/Source/C/GenFv/GenFv.c
> @@ -623,12 +623,7 @@ Returns:
>                );
>    } else {
>      VerboseMsg ("Create Fv image and its map file");
> -    //
> -    // Will take rebase action at below situation:
> -    // 1. ForceRebase Flag specified to TRUE;
> -    // 2. ForceRebase Flag not specified, BaseAddress greater than
> zero.
> -    //
> -    if (((mFvDataInfo.BaseAddress > 0) && (mFvDataInfo.ForceRebase ==
> -1)) || (mFvDataInfo.ForceRebase == 1)) {
> +    if (mFvDataInfo.BaseAddressSet) {
>        VerboseMsg ("FvImage Rebase Address is 0x%llX", (unsigned long
> long) mFvDataInfo.BaseAddress);
>      }
>      //
> diff --git a/BaseTools/Source/C/GenFv/GenFvInternalLib.c
> b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
> index c01e504..d143040 100644
> --- a/BaseTools/Source/C/GenFv/GenFvInternalLib.c
> +++ b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
> @@ -506,6 +506,7 @@ Returns:
> 
>  EFI_STATUS
>  AddPadFile (
> +  IN FV_INFO          *FvInfo,
>    IN OUT MEMORY_FILE  *FvImage,
>    IN UINT32           DataAlignment,
>    IN VOID             *FvEnd,
> @@ -537,6 +538,8 @@ Returns:
>  {
>    EFI_FFS_FILE_HEADER *PadFile;
>    UINTN               PadFileSize;
> +  UINTN               PadFileOffset;
> +  UINTN               ExtHeaderSize;
> 
>    //
>    // Verify input parameters.
> @@ -559,32 +562,29 @@ Returns:
>    // This is the earliest possible valid offset (current plus pad file
> header
>    // plus the next file header)
>    //
> -  PadFileSize = (UINTN) FvImage->CurrentFilePointer - (UINTN) FvImage-
> >FileImage + (sizeof (EFI_FFS_FILE_HEADER) * 2);
> +  // The padding is added into its own FFS file (which requires a
> header) added before the aligned file:
> +  // | ... FV data before AlignedFile ... | Pad File FFS Header |
> Padding | AlignedFile FFS Header (+ ExtHeader) | AlignedData
> 
>    //
> -  // Add whatever it takes to get to the next aligned address
> +  // Calculate the Offset of the Pad File from the beginning of the FV
> file
>    //
> -  while ((PadFileSize % DataAlignment) != 0) {
> -    PadFileSize++;
> -  }
> -  //
> -  // Subtract the next file header size
> -  //
> -  PadFileSize -= sizeof (EFI_FFS_FILE_HEADER);
> -
> -  //
> -  // Subtract the starting offset to get size
> -  //
> -  PadFileSize -= (UINTN) FvImage->CurrentFilePointer - (UINTN)
> FvImage->FileImage;
> +  PadFileOffset = (UINTN) FvImage->CurrentFilePointer - (UINTN)
> FvImage->FileImage;
> 
>    //
> -  // Append extension header size
> +  // Get the size of the extension header if exists
>    //
>    if (ExtHeader != NULL) {
> -    PadFileSize = PadFileSize + ExtHeader->ExtHeaderSize;
> +    ExtHeaderSize = ExtHeader->ExtHeaderSize;
> +  } else {
> +    ExtHeaderSize = 0;
>    }
> 
>    //
> +  // Calculate the Size of the Padding to ensure the alignment of the
> data of the Next file
> +  //
> +  PadFileSize = DataAlignment - ((FvInfo->BaseAddress + PadFileOffset
> + sizeof (EFI_FFS_FILE_HEADER) + ExtHeaderSize) & (DataAlignment - 1));
> +
> +  //
>    // Verify that we have enough space for the file header
>    //
>    if (((UINTN) FvImage->CurrentFilePointer + PadFileSize) > (UINTN)
> FvEnd) {
> @@ -1115,7 +1115,7 @@ Returns:
>    //
>    // Add pad file if necessary
>    //
> -  Status = AddPadFile (FvImage, 1 << CurrentFileAlignment,
> *VtfFileImage, NULL);
> +  Status = AddPadFile (FvInfo, FvImage, 1 << CurrentFileAlignment,
> *VtfFileImage, NULL);
>    if (EFI_ERROR (Status)) {
>      Error (NULL, 0, 4002, "Resource", "FV space is full, could not add
> pad file for data alignment property.");
>      free (FileBuffer);
> @@ -2304,7 +2304,7 @@ Returns:
>      //
>      // Add FV Extended Header contents to the FV as a PAD file
>      //
> -    AddPadFile (&FvImageMemoryFile, 4, VtfFileImage, FvExtHeader);
> +    AddPadFile (&mFvDataInfo, &FvImageMemoryFile, 4, VtfFileImage,
> FvExtHeader);
> 
>      //
>      // Fv Extension header change update Fv Header Check sum
> @@ -2825,19 +2825,11 @@ Returns:
>    PeFileBuffer       = NULL;
> 
>    //
> -  // Don't need to relocate image when BaseAddress is zero and no
> ForceRebase Flag specified.
> +  // Don't need to relocate image when BaseAddress is not set.
>    //
> -  if ((FvInfo->BaseAddress == 0) && (FvInfo->ForceRebase == -1)) {
> +  if (FvInfo->BaseAddressSet == FALSE) {
>      return EFI_SUCCESS;
>    }
> -
> -  //
> -  // If ForceRebase Flag specified to FALSE, will always not take
> rebase action.
> -  //
> -  if (FvInfo->ForceRebase == 0) {
> -    return EFI_SUCCESS;
> -  }
> -
> 
>    XipBase = FvInfo->BaseAddress + XipOffset;
> 
> --
> 1.7.9.5
> 
> 
> _______________________________________________
> boot-architecture mailing list
> boot-architecture@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/boot-architecture
Ryan Harkin June 21, 2013, 9:33 a.m. | #2
On 21 June 2013 10:13, Olivier Martin <olivier.martin@arm.com> wrote:
> I also offered a patch to the BaseTools project to fix the firmware when
> linked at 0x0 in the past.
> But my patch has been rejected and a new keyword has been introduced to fix
> this BaseTools limitation.
>
> In your Versatile Express A5 FDF file you have to replace:
>
> [FV.FVMAIN_SEC]
> FvAlignment        = 8
>
> By:
> [FV.FVMAIN_SEC]
> FvBaseAddress      = 0x0
> FvForceRebase      = TRUE
> FvAlignment        = 8
>
>

Thanks Olivier, I'll try it and rework my branches!  I wasn't aware of
this and I've been carrying this patch, thinking I still needed it.



>> -----Original Message-----
>> From: boot-architecture-bounces@lists.linaro.org [mailto:boot-
>> architecture-bounces@lists.linaro.org] On Behalf Of Ryan Harkin
>> Sent: 21 June 2013 08:44
>> To: ryan.harkin@linaro.org; edk2-devel@lists.sourceforge.net; edk2-
>> buildtools-devel@lists.sourceforge.net; patches@linaro.org; boot-
>> architecture@lists.linaro.org
>> Subject: [PATCH 1/1] BaseTools: Apply ARM GenFv patch
>>
>> This patch is needed to fixup some builds, such as Versatile Express
>> A5,
>> otherwise they hang on boot due to the first instruction being zero.
>>
>> Signed-off-by: Ryan Harkin <ryan.harkin@linaro.org>
>> ---
>>  BaseTools/Source/C/GenFv/GenFv.c            |    7 +---
>>  BaseTools/Source/C/GenFv/GenFvInternalLib.c |   48 +++++++++++--------
>> --------
>>  2 files changed, 21 insertions(+), 34 deletions(-)
>>
>> diff --git a/BaseTools/Source/C/GenFv/GenFv.c
>> b/BaseTools/Source/C/GenFv/GenFv.c
>> index fa86d00..a68c7b8 100644
>> --- a/BaseTools/Source/C/GenFv/GenFv.c
>> +++ b/BaseTools/Source/C/GenFv/GenFv.c
>> @@ -623,12 +623,7 @@ Returns:
>>                );
>>    } else {
>>      VerboseMsg ("Create Fv image and its map file");
>> -    //
>> -    // Will take rebase action at below situation:
>> -    // 1. ForceRebase Flag specified to TRUE;
>> -    // 2. ForceRebase Flag not specified, BaseAddress greater than
>> zero.
>> -    //
>> -    if (((mFvDataInfo.BaseAddress > 0) && (mFvDataInfo.ForceRebase ==
>> -1)) || (mFvDataInfo.ForceRebase == 1)) {
>> +    if (mFvDataInfo.BaseAddressSet) {
>>        VerboseMsg ("FvImage Rebase Address is 0x%llX", (unsigned long
>> long) mFvDataInfo.BaseAddress);
>>      }
>>      //
>> diff --git a/BaseTools/Source/C/GenFv/GenFvInternalLib.c
>> b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
>> index c01e504..d143040 100644
>> --- a/BaseTools/Source/C/GenFv/GenFvInternalLib.c
>> +++ b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
>> @@ -506,6 +506,7 @@ Returns:
>>
>>  EFI_STATUS
>>  AddPadFile (
>> +  IN FV_INFO          *FvInfo,
>>    IN OUT MEMORY_FILE  *FvImage,
>>    IN UINT32           DataAlignment,
>>    IN VOID             *FvEnd,
>> @@ -537,6 +538,8 @@ Returns:
>>  {
>>    EFI_FFS_FILE_HEADER *PadFile;
>>    UINTN               PadFileSize;
>> +  UINTN               PadFileOffset;
>> +  UINTN               ExtHeaderSize;
>>
>>    //
>>    // Verify input parameters.
>> @@ -559,32 +562,29 @@ Returns:
>>    // This is the earliest possible valid offset (current plus pad file
>> header
>>    // plus the next file header)
>>    //
>> -  PadFileSize = (UINTN) FvImage->CurrentFilePointer - (UINTN) FvImage-
>> >FileImage + (sizeof (EFI_FFS_FILE_HEADER) * 2);
>> +  // The padding is added into its own FFS file (which requires a
>> header) added before the aligned file:
>> +  // | ... FV data before AlignedFile ... | Pad File FFS Header |
>> Padding | AlignedFile FFS Header (+ ExtHeader) | AlignedData
>>
>>    //
>> -  // Add whatever it takes to get to the next aligned address
>> +  // Calculate the Offset of the Pad File from the beginning of the FV
>> file
>>    //
>> -  while ((PadFileSize % DataAlignment) != 0) {
>> -    PadFileSize++;
>> -  }
>> -  //
>> -  // Subtract the next file header size
>> -  //
>> -  PadFileSize -= sizeof (EFI_FFS_FILE_HEADER);
>> -
>> -  //
>> -  // Subtract the starting offset to get size
>> -  //
>> -  PadFileSize -= (UINTN) FvImage->CurrentFilePointer - (UINTN)
>> FvImage->FileImage;
>> +  PadFileOffset = (UINTN) FvImage->CurrentFilePointer - (UINTN)
>> FvImage->FileImage;
>>
>>    //
>> -  // Append extension header size
>> +  // Get the size of the extension header if exists
>>    //
>>    if (ExtHeader != NULL) {
>> -    PadFileSize = PadFileSize + ExtHeader->ExtHeaderSize;
>> +    ExtHeaderSize = ExtHeader->ExtHeaderSize;
>> +  } else {
>> +    ExtHeaderSize = 0;
>>    }
>>
>>    //
>> +  // Calculate the Size of the Padding to ensure the alignment of the
>> data of the Next file
>> +  //
>> +  PadFileSize = DataAlignment - ((FvInfo->BaseAddress + PadFileOffset
>> + sizeof (EFI_FFS_FILE_HEADER) + ExtHeaderSize) & (DataAlignment - 1));
>> +
>> +  //
>>    // Verify that we have enough space for the file header
>>    //
>>    if (((UINTN) FvImage->CurrentFilePointer + PadFileSize) > (UINTN)
>> FvEnd) {
>> @@ -1115,7 +1115,7 @@ Returns:
>>    //
>>    // Add pad file if necessary
>>    //
>> -  Status = AddPadFile (FvImage, 1 << CurrentFileAlignment,
>> *VtfFileImage, NULL);
>> +  Status = AddPadFile (FvInfo, FvImage, 1 << CurrentFileAlignment,
>> *VtfFileImage, NULL);
>>    if (EFI_ERROR (Status)) {
>>      Error (NULL, 0, 4002, "Resource", "FV space is full, could not add
>> pad file for data alignment property.");
>>      free (FileBuffer);
>> @@ -2304,7 +2304,7 @@ Returns:
>>      //
>>      // Add FV Extended Header contents to the FV as a PAD file
>>      //
>> -    AddPadFile (&FvImageMemoryFile, 4, VtfFileImage, FvExtHeader);
>> +    AddPadFile (&mFvDataInfo, &FvImageMemoryFile, 4, VtfFileImage,
>> FvExtHeader);
>>
>>      //
>>      // Fv Extension header change update Fv Header Check sum
>> @@ -2825,19 +2825,11 @@ Returns:
>>    PeFileBuffer       = NULL;
>>
>>    //
>> -  // Don't need to relocate image when BaseAddress is zero and no
>> ForceRebase Flag specified.
>> +  // Don't need to relocate image when BaseAddress is not set.
>>    //
>> -  if ((FvInfo->BaseAddress == 0) && (FvInfo->ForceRebase == -1)) {
>> +  if (FvInfo->BaseAddressSet == FALSE) {
>>      return EFI_SUCCESS;
>>    }
>> -
>> -  //
>> -  // If ForceRebase Flag specified to FALSE, will always not take
>> rebase action.
>> -  //
>> -  if (FvInfo->ForceRebase == 0) {
>> -    return EFI_SUCCESS;
>> -  }
>> -
>>
>>    XipBase = FvInfo->BaseAddress + XipOffset;
>>
>> --
>> 1.7.9.5
>>
>>
>> _______________________________________________
>> boot-architecture mailing list
>> boot-architecture@lists.linaro.org
>> http://lists.linaro.org/mailman/listinfo/boot-architecture
>
>
>
>
>
> _______________________________________________
> boot-architecture mailing list
> boot-architecture@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/boot-architecture

Patch

diff --git a/BaseTools/Source/C/GenFv/GenFv.c b/BaseTools/Source/C/GenFv/GenFv.c
index fa86d00..a68c7b8 100644
--- a/BaseTools/Source/C/GenFv/GenFv.c
+++ b/BaseTools/Source/C/GenFv/GenFv.c
@@ -623,12 +623,7 @@  Returns:
               );
   } else {
     VerboseMsg ("Create Fv image and its map file");
-    //
-    // Will take rebase action at below situation:
-    // 1. ForceRebase Flag specified to TRUE;
-    // 2. ForceRebase Flag not specified, BaseAddress greater than zero.
-    //
-    if (((mFvDataInfo.BaseAddress > 0) && (mFvDataInfo.ForceRebase == -1)) || (mFvDataInfo.ForceRebase == 1)) {
+    if (mFvDataInfo.BaseAddressSet) {
       VerboseMsg ("FvImage Rebase Address is 0x%llX", (unsigned long long) mFvDataInfo.BaseAddress);
     }
     //
diff --git a/BaseTools/Source/C/GenFv/GenFvInternalLib.c b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
index c01e504..d143040 100644
--- a/BaseTools/Source/C/GenFv/GenFvInternalLib.c
+++ b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
@@ -506,6 +506,7 @@  Returns:
 
 EFI_STATUS
 AddPadFile (
+  IN FV_INFO          *FvInfo,
   IN OUT MEMORY_FILE  *FvImage,
   IN UINT32           DataAlignment,
   IN VOID             *FvEnd,
@@ -537,6 +538,8 @@  Returns:
 {
   EFI_FFS_FILE_HEADER *PadFile;
   UINTN               PadFileSize;
+  UINTN               PadFileOffset;
+  UINTN               ExtHeaderSize;
 
   //
   // Verify input parameters.
@@ -559,32 +562,29 @@  Returns:
   // This is the earliest possible valid offset (current plus pad file header
   // plus the next file header)
   //
-  PadFileSize = (UINTN) FvImage->CurrentFilePointer - (UINTN) FvImage->FileImage + (sizeof (EFI_FFS_FILE_HEADER) * 2);
+  // The padding is added into its own FFS file (which requires a header) added before the aligned file:
+  // | ... FV data before AlignedFile ... | Pad File FFS Header | Padding | AlignedFile FFS Header (+ ExtHeader) | AlignedData
 
   //
-  // Add whatever it takes to get to the next aligned address
+  // Calculate the Offset of the Pad File from the beginning of the FV file
   //
-  while ((PadFileSize % DataAlignment) != 0) {
-    PadFileSize++;
-  }
-  //
-  // Subtract the next file header size
-  //
-  PadFileSize -= sizeof (EFI_FFS_FILE_HEADER);
-
-  //
-  // Subtract the starting offset to get size
-  //
-  PadFileSize -= (UINTN) FvImage->CurrentFilePointer - (UINTN) FvImage->FileImage;
+  PadFileOffset = (UINTN) FvImage->CurrentFilePointer - (UINTN) FvImage->FileImage;
   
   //
-  // Append extension header size
+  // Get the size of the extension header if exists
   //
   if (ExtHeader != NULL) {
-    PadFileSize = PadFileSize + ExtHeader->ExtHeaderSize;
+    ExtHeaderSize = ExtHeader->ExtHeaderSize;
+  } else {
+    ExtHeaderSize = 0;
   }
 
   //
+  // Calculate the Size of the Padding to ensure the alignment of the data of the Next file
+  //
+  PadFileSize = DataAlignment - ((FvInfo->BaseAddress + PadFileOffset + sizeof (EFI_FFS_FILE_HEADER) + ExtHeaderSize) & (DataAlignment - 1));
+
+  //
   // Verify that we have enough space for the file header
   //
   if (((UINTN) FvImage->CurrentFilePointer + PadFileSize) > (UINTN) FvEnd) {
@@ -1115,7 +1115,7 @@  Returns:
   //
   // Add pad file if necessary
   //
-  Status = AddPadFile (FvImage, 1 << CurrentFileAlignment, *VtfFileImage, NULL);
+  Status = AddPadFile (FvInfo, FvImage, 1 << CurrentFileAlignment, *VtfFileImage, NULL);
   if (EFI_ERROR (Status)) {
     Error (NULL, 0, 4002, "Resource", "FV space is full, could not add pad file for data alignment property.");
     free (FileBuffer);
@@ -2304,7 +2304,7 @@  Returns:
     //
     // Add FV Extended Header contents to the FV as a PAD file
     //
-    AddPadFile (&FvImageMemoryFile, 4, VtfFileImage, FvExtHeader);
+    AddPadFile (&mFvDataInfo, &FvImageMemoryFile, 4, VtfFileImage, FvExtHeader);
 
     //
     // Fv Extension header change update Fv Header Check sum
@@ -2825,19 +2825,11 @@  Returns:
   PeFileBuffer       = NULL;
 
   //
-  // Don't need to relocate image when BaseAddress is zero and no ForceRebase Flag specified.
+  // Don't need to relocate image when BaseAddress is not set.
   //
-  if ((FvInfo->BaseAddress == 0) && (FvInfo->ForceRebase == -1)) {
+  if (FvInfo->BaseAddressSet == FALSE) {
     return EFI_SUCCESS;
   }
-  
-  //
-  // If ForceRebase Flag specified to FALSE, will always not take rebase action.
-  //
-  if (FvInfo->ForceRebase == 0) {
-    return EFI_SUCCESS;
-  }
-
 
   XipBase = FvInfo->BaseAddress + XipOffset;