On 25 June 2015 at 05:33, Liu, Yingke D <yingke.d....@intel.com> wrote:
> Ard,
>
> Following is the full GenFw patch, please confirm it:
>
> BaseTools: Update GenFw to support 4K alignment.
>
> Get maximum section alignment from ELF sections
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
> ---
> BaseTools/Source/C/GenFw/Elf32Convert.c | 16 +++++++++++++++-
> BaseTools/Source/C/GenFw/Elf64Convert.c | 16 +++++++++++++++-
> BaseTools/Source/C/GenFw/ElfConvert.c | 4 ++--
> BaseTools/Source/C/GenFw/ElfConvert.h | 1 +
> 4 files changed, 33 insertions(+), 4 deletions(-)
>
Hello Dennis,
The patch looks fine to me.
Thanks,
Ard.
> diff --git a/BaseTools/Source/C/GenFw/Elf32Convert.c
> b/BaseTools/Source/C/GenFw/Elf32Convert.c
> index 5c7b689..10d9892 100644
> --- a/BaseTools/Source/C/GenFw/Elf32Convert.c
> +++ b/BaseTools/Source/C/GenFw/Elf32Convert.c
> @@ -96,7 +96,7 @@ STATIC Elf_Phdr *mPhdrBase;
> //
> // Coff information
> //
> -STATIC const UINT32 mCoffAlignment = 0x20;
> +STATIC UINT32 mCoffAlignment = 0x20;
>
> //
> // PE section alignment.
> @@ -292,6 +292,20 @@ ScanSections32 (
> mCoffOffset += mCoffNbrSections * sizeof(EFI_IMAGE_SECTION_HEADER);
>
> //
> + // Set mCoffAlignment to the maximum alignment of the input sections
> + // we care about
> + //
> + for (i = 0; i < mEhdr->e_shnum; i++) {
> + Elf_Shdr *shdr = GetShdrByIndex(i);
> + if (shdr->sh_addralign <= mCoffAlignment) {
> + continue;
> + }
> + if (IsTextShdr(shdr) || IsDataShdr(shdr) || IsHiiRsrcShdr(shdr)) {
> + mCoffAlignment = (UINT32)shdr->sh_addralign;
> + }
> + }
> +
> + //
> // First text sections.
> //
> mCoffOffset = CoffAlign(mCoffOffset);
> diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c
> b/BaseTools/Source/C/GenFw/Elf64Convert.c
> index 25b90e2..d2becf1 100644
> --- a/BaseTools/Source/C/GenFw/Elf64Convert.c
> +++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
> @@ -97,7 +97,7 @@ STATIC Elf_Phdr *mPhdrBase;
> //
> // Coff information
> //
> -STATIC const UINT32 mCoffAlignment = 0x20;
> +STATIC UINT32 mCoffAlignment = 0x20;
>
> //
> // PE section alignment.
> @@ -286,6 +286,20 @@ ScanSections64 (
> mCoffOffset += mCoffNbrSections * sizeof(EFI_IMAGE_SECTION_HEADER);
>
> //
> + // Set mCoffAlignment to the maximum alignment of the input sections
> + // we care about
> + //
> + for (i = 0; i < mEhdr->e_shnum; i++) {
> + Elf_Shdr *shdr = GetShdrByIndex(i);
> + if (shdr->sh_addralign <= mCoffAlignment) {
> + continue;
> + }
> + if (IsTextShdr(shdr) || IsDataShdr(shdr) || IsHiiRsrcShdr(shdr)) {
> + mCoffAlignment = (UINT32)shdr->sh_addralign;
> + }
> + }
> +
> + //
> // First text sections.
> //
> mCoffOffset = CoffAlign(mCoffOffset);
> diff --git a/BaseTools/Source/C/GenFw/ElfConvert.c
> b/BaseTools/Source/C/GenFw/ElfConvert.c
> index 1a84d3c..6211389 100644
> --- a/BaseTools/Source/C/GenFw/ElfConvert.c
> +++ b/BaseTools/Source/C/GenFw/ElfConvert.c
> @@ -96,11 +96,11 @@ CoffAddFixup(
>
> mCoffFile = realloc (
> mCoffFile,
> - mCoffOffset + sizeof(EFI_IMAGE_BASE_RELOCATION) + 2*0x1000
> + mCoffOffset + sizeof(EFI_IMAGE_BASE_RELOCATION) + 2 *
> MAX_COFF_ALIGNMENT
> );
> memset (
> mCoffFile + mCoffOffset, 0,
> - sizeof(EFI_IMAGE_BASE_RELOCATION) + 2*0x1000
> + sizeof(EFI_IMAGE_BASE_RELOCATION) + 2 * MAX_COFF_ALIGNMENT
> );
>
> mCoffBaseRel = (EFI_IMAGE_BASE_RELOCATION*)(mCoffFile + mCoffOffset);
> diff --git a/BaseTools/Source/C/GenFw/ElfConvert.h
> b/BaseTools/Source/C/GenFw/ElfConvert.h
> index b27a2f9..56f165e 100644
> --- a/BaseTools/Source/C/GenFw/ElfConvert.h
> +++ b/BaseTools/Source/C/GenFw/ElfConvert.h
> @@ -34,6 +34,7 @@ extern UINT32 mOutImageType;
> // Common EFI specific data.
> //
> #define ELF_HII_SECTION_NAME ".hii"
> +#define MAX_COFF_ALIGNMENT 0x10000
>
> //
> // Filter Types
> --
>
> Dennis
>
> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Wednesday, June 24, 2015 18:13
> To: edk2-devel@lists.sourceforge.net
> Subject: Re: [edk2] [Patch 2/3] BaseTools: Update GenFw to support 4K
> alignment.
>
> On 24 June 2015 at 10:26, Gao, Liming <liming....@intel.com> wrote:
>> Got your point. I agree with this change. We will include it in the patch
>> and add you in Signed-off-by list.
>>
>
> OK, thanks.
>
> There is one additional issue though: when using alignment of > 4 KB, the
> GenFw tool may crash, since it tries to pad out the .reloc section up to
> section alignment, which may cover unmapped host memory. So something like
> below is needed, and MAX_COFF_ALIGNMENT needs to be set to some sensible but
> sufficiently high value. Since PE/COFF specifies
> 64 KB as the maximum file alignment, and GenFw uses mCoffAlignment for both
> SectionAlignment and FileAlignment, I suppose 64 KB would be appropriate here.
>
> """
> diff --git a/BaseTools/Source/C/GenFw/ElfConvert.c
> b/BaseTools/Source/C/GenFw/ElfConvert.c
> index 1a84d3c28794..e315d913378d 100644
> --- a/BaseTools/Source/C/GenFw/ElfConvert.c
> +++ b/BaseTools/Source/C/GenFw/ElfConvert.c
> @@ -96,11 +101,11 @@ CoffAddFixup(
>
> mCoffFile = realloc (
> mCoffFile,
> - mCoffOffset + sizeof(EFI_IMAGE_BASE_RELOCATION) + 2*0x1000
> + mCoffOffset + sizeof(EFI_IMAGE_BASE_RELOCATION) + 2 *
> + MAX_COFF_ALIGNMENT
> );
> memset (
> mCoffFile + mCoffOffset, 0,
> - sizeof(EFI_IMAGE_BASE_RELOCATION) + 2*0x1000
> + sizeof(EFI_IMAGE_BASE_RELOCATION) + 2 * MAX_COFF_ALIGNMENT
> );
>
> mCoffBaseRel = (EFI_IMAGE_BASE_RELOCATION*)(mCoffFile + mCoffOffset); """
>
> --
> Ard.
>
>
>> -----Original Message-----
>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>> Sent: Wednesday, June 24, 2015 4:19 PM
>> To: edk2-devel@lists.sourceforge.net
>> Subject: Re: [edk2] [Patch 2/3] BaseTools: Update GenFw to support 4K
>> alignment.
>>
>> On 24 June 2015 at 10:15, Gao, Liming <liming....@intel.com> wrote:
>>> Ard:
>>> Good suggestion. How about go through every Shdr and choose the max
>>> Shdr->sh_addralign? The alignment should be power of 2.
>>>
>>
>> Indeed. Something like this seems to work fine:
>>
>> """
>> diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c
>> b/BaseTools/Source/C/GenFw/Elf64Convert.c
>> index 2266e487cec7..4025191e868e 100644
>> --- a/BaseTools/Source/C/GenFw/Elf64Convert.c
>> +++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
>> @@ -97,7 +97,7 @@ STATIC Elf_Phdr *mPhdrBase; // // Coff information //
>> -STATIC const UINT32 mCoffAlignment = 0x20;
>> +STATIC UINT32 mCoffAlignment = 0x20;
>>
>> //
>> // PE section alignment.
>> @@ -286,6 +286,20 @@ ScanSections64 (
>> mCoffOffset += mCoffNbrSections * sizeof(EFI_IMAGE_SECTION_HEADER);
>>
>> //
>> + // Set mCoffAlignment to the maximum alignment of the input sections
>> + // we care about // for (i = 0; i < mEhdr->e_shnum; i++) {
>> + Elf_Shdr *shdr = GetShdrByIndex(i);
>> + if (shdr->sh_addralign <= mCoffAlignment) {
>> + continue;
>> + }
>> + if (IsTextShdr(shdr) || IsDataShdr(shdr) || IsHiiRsrcShdr(shdr)) {
>> + mCoffAlignment = shdr->sh_addralign;
>> + }
>> + }
>> +
>> + //
>> // First text sections.
>> //
>> mCoffOffset = CoffAlign(mCoffOffset); """
>>
>> Note that we may want to use 64 KB instead of 4 KB on AArch64, since the OS
>> may use 64 KB pages. So we should avoid hardcoding 4 KB values for section
>> alignment.
>>
>> --
>> Ard.
>>
>>
>>> Thanks
>>> Liming
>>> -----Original Message-----
>>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>>> Sent: Tuesday, June 23, 2015 5:14 PM
>>> To: edk2-devel@lists.sourceforge.net
>>> Subject: Re: [edk2] [Patch 2/3] BaseTools: Update GenFw to support 4K
>>> alignment.
>>>
>>> On 23 June 2015 at 10:19, Yingke Liu <yingke.d....@intel.com> wrote:
>>>> If current ELF file is 4K aligned, the converted PE should also be 4K
>>>> aligned.
>>>>
>>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>> Signed-off-by: Yingke Liu <yingke.d....@intel.com>
>>>> ---
>>>> BaseTools/Source/C/GenFw/Elf32Convert.c | 8 ++++++--
>>>> BaseTools/Source/C/GenFw/Elf64Convert.c | 6 +++++-
>>>> 2 files changed, 11 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/BaseTools/Source/C/GenFw/Elf32Convert.c
>>>> b/BaseTools/Source/C/GenFw/Elf32Convert.c
>>>> index 5c7b689..9245851 100644
>>>> --- a/BaseTools/Source/C/GenFw/Elf32Convert.c
>>>> +++ b/BaseTools/Source/C/GenFw/Elf32Convert.c
>>>> @@ -96,7 +96,7 @@ STATIC Elf_Phdr *mPhdrBase; // // Coff information
>>>> // -STATIC const UINT32 mCoffAlignment = 0x20;
>>>> +STATIC UINT32 mCoffAlignment = 0x20;
>>>>
>>>> //
>>>> // PE section alignment.
>>>> @@ -154,7 +154,11 @@ InitializeElf32 (
>>>> Error (NULL, 0, 3000, "Unsupported", "ELF e_version (%u) not
>>>> EV_CURRENT (%d)", (unsigned) mEhdr->e_version, EV_CURRENT);
>>>> return FALSE;
>>>> }
>>>> -
>>>> +
>>>> + if ((mEhdr->e_entry & 0xFFF) == 0) {
>>>> + mCoffAlignment = 0x1000;
>>>> + }
>>>> +
>>>
>>> Can you explain why you are using the alignment of the entry point here?
>>> Wouldn't it be much better if mCoffAlignment is simply the max() of the
>>> alignments of all the sections?
>>>
>>>> //
>>>> // Update section header pointers
>>>> //
>>>> diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c
>>>> b/BaseTools/Source/C/GenFw/Elf64Convert.c
>>>> index 25b90e2..8737e30 100644
>>>> --- a/BaseTools/Source/C/GenFw/Elf64Convert.c
>>>> +++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
>>>> @@ -97,7 +97,7 @@ STATIC Elf_Phdr *mPhdrBase; // // Coff information
>>>> // -STATIC const UINT32 mCoffAlignment = 0x20;
>>>> +STATIC UINT32 mCoffAlignment = 0x20;
>>>>
>>>> //
>>>> // PE section alignment.
>>>> @@ -158,6 +158,10 @@ InitializeElf64 (
>>>> return FALSE;
>>>> }
>>>>
>>>> + if ((mEhdr->e_entry & 0xFFF) == 0) {
>>>> + mCoffAlignment = 0x1000;
>>>> + }
>>>> +
>>>> //
>>>> // Update section header pointers
>>>> //
>>>> --
>>>> 1.9.5.msysgit.0
>>>>
>>>>
>>>> ----------------------------------------------------------------------
>>>> -------- Monitor 25 network devices or servers for free with
>>>> OpManager!
>>>> OpManager is web-based network management software that monitors
>>>> network devices and physical & virtual servers, alerts via email & sms
>>>> for fault. Monitor 25 devices for free with no restriction. Download
>>>> now http://ad.doubleclick.net/ddm/clk/292181274;119417398;o
>>>> _______________________________________________
>>>> edk2-devel mailing list
>>>> edk2-devel@lists.sourceforge.net
>>>> https://lists.sourceforge.net/lists/listinfo/edk2-devel
>>>
>>> ------------------------------------------------------------------------------
>>> Monitor 25 network devices or servers for free with OpManager!
>>> OpManager is web-based network management software that monitors network
>>> devices and physical & virtual servers, alerts via email & sms for fault.
>>> Monitor 25 devices for free with no restriction. Download now
>>> http://ad.doubleclick.net/ddm/clk/292181274;119417398;o
>>> _______________________________________________
>>> edk2-devel mailing list
>>> edk2-devel@lists.sourceforge.net
>>> https://lists.sourceforge.net/lists/listinfo/edk2-devel
>>>
>>> ------------------------------------------------------------------------------
>>> Monitor 25 network devices or servers for free with OpManager!
>>> OpManager is web-based network management software that monitors
>>> network devices and physical & virtual servers, alerts via email & sms
>>> for fault. Monitor 25 devices for free with no restriction. Download now
>>> http://ad.doubleclick.net/ddm/clk/292181274;119417398;o
>>> _______________________________________________
>>> edk2-devel mailing list
>>> edk2-devel@lists.sourceforge.net
>>> https://lists.sourceforge.net/lists/listinfo/edk2-devel
>>
>> ------------------------------------------------------------------------------
>> Monitor 25 network devices or servers for free with OpManager!
>> OpManager is web-based network management software that monitors
>> network devices and physical & virtual servers, alerts via email & sms
>> for fault. Monitor 25 devices for free with no restriction. Download now
>> http://ad.doubleclick.net/ddm/clk/292181274;119417398;o
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/edk2-devel
>>
>> ------------------------------------------------------------------------------
>> Monitor 25 network devices or servers for free with OpManager!
>> OpManager is web-based network management software that monitors
>> network devices and physical & virtual servers, alerts via email & sms
>> for fault. Monitor 25 devices for free with no restriction. Download now
>> http://ad.doubleclick.net/ddm/clk/292181274;119417398;o
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/edk2-devel
>
> ------------------------------------------------------------------------------
> Monitor 25 network devices or servers for free with OpManager!
> OpManager is web-based network management software that monitors
> network devices and physical & virtual servers, alerts via email & sms
> for fault. Monitor 25 devices for free with no restriction. Download now
> http://ad.doubleclick.net/ddm/clk/292181274;119417398;o
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/edk2-devel
------------------------------------------------------------------------------
Monitor 25 network devices or servers for free with OpManager!
OpManager is web-based network management software that monitors
network devices and physical & virtual servers, alerts via email & sms
for fault. Monitor 25 devices for free with no restriction. Download now
http://ad.doubleclick.net/ddm/clk/292181274;119417398;o
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
