Message ID | 1371800618-14193-1-git-send-email-ryan.harkin@linaro.org |
---|---|
State | New |
Headers | show |
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
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
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;
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(-)