On 27 July 2015 at 13:14, Gao, Liming <liming....@intel.com> wrote:
> Ard:
>   I understand your change. I also verify it for this case. It does work. 
> Thanks for your detail explanation.
>

Thanks for taking the time.

>   Besides, you say you will also clear the alignment bits in the FFS header, 
> and set the FFS_ATTRIB_FIXED attribute. If FFS has no FFS_ATTRIB_FIXED 
> attribute, it is allowed to be loaded into other memory address based on its 
> alignment. I don't except to see this attribute is auto set by tools. So, I 
> suggest this change should be done only for FFS with FFS_ATTRIB_FIXED 
> attribute. GenFv and GenFfs tool will first check FFS header FFS_ATTRIB_FIXED 
> attribute. If it is set, then apply the updated logic. If is not set, then 
> use the current logic.
>

May I suggest that we set the FFS_ATTRIB_FIXED bit only after
adjusting the padding section in GenFv? Before the padding adjustment,
the FFS is an ordinary FFS file except for the different section type
for the padding, and it can still be loaded anywhere in memory
provided that the minimum alignment is met.

Only after the padding section has been adjusted, the FFS is no longer
movable, since the metadata in the header cannot tell you how much
padding was removed, and it only contains the global alignment, but
does not know to which FFS sections it applies.

Regards,
Ard.


> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ard 
> Biesheuvel
> Sent: Monday, July 27, 2015 5:33 PM
> To: Gao, Liming
> Cc: Leif Lindholm; Liu, Yingke D; edk2-devel@lists.01.org; eugene.co...@hp.com
> Subject: Re: [edk2] [PATCH 0/4] FFS/FV aligment optimization (was: [RFC] 
> small C model and LLVM/clang support for AARCH64)
>
> On 27 July 2015 at 11:21, Gao, Liming <liming....@intel.com> wrote:
>> Ard:
>>   You update GenFv tool to adjust SUB GUID padding section to make the 
>> section data at the alignment. GenFv reads this alignment from FFS header. 
>> So, this alignment is the max alignment. For this case, its value is 4K.
>>
>
> No, I update the GenFv tool to compensate the misalignment of the start of 
> the entire FFS payload, not any individual section. If the start of the 
> entire FFS payload is aligned correctly, all sections will automatically be 
> aligned correctly as well. (The only reason GenFv looks at the next section 
> after the padding is to copy the FFS data closer to the beginning of the 
> file.)
>
> In an FFS with 4 KB alignment, all sections with alignment requirements are 
> individually aligned to their minimum alignment relative to the start of the 
> FFS data, and the maximum alignment of all those sections is 4 KB. This does 
> not mean that all sections are aligned to 4 KB, only that there exists at 
> least one section that needs 4 KB alignment, and others may require less, or 
> no alignment at all.
>
> This assumes that the placement inside the FV will ensure that the first byte 
> of the FFS data will be aligned to the FFS global alignment of 4 KB. If that 
> is the case, each FFS section will be at its minimum alignment.
>
> What my patch does is not align an individual section. It calculates the 
> misalignment of the start of the FFS data, since that is the basis for the 
> alignment of all FFS sections. If we can compensate this misalignment 
> relative to the global FFS alignment, all sections will shift into place.
>
>
>> -----Original Message-----
>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>> Sent: Monday, July 27, 2015 5:09 PM
>> To: Gao, Liming
>> Cc: Liu, Yingke D; Leif Lindholm; edk2-devel@lists.01.org;
>> eugene.co...@hp.com
>> Subject: Re: [edk2] [PATCH 0/4] FFS/FV aligment optimization (was:
>> [RFC] small C model and LLVM/clang support for AARCH64)
>>
>> On 27 July 2015 at 10:56, Gao, Liming <liming....@intel.com> wrote:
>>> Ard:
>>>   I don't think this is correct. On 4K alignment, its address is 0x1C00 
>>> that doesn't match.
>>>
>>> +-----------------+      0x0 ->    0x0
>>> |    FV header    |
>>> +-----------------+     0x78 ->   0x78
>>>
>>> ....
>>> |   FFS header    |
>>> +-----------------+     0xc90 ->   0xc90
>>> |    SUB GUID     |
>>> | padding section |
>>> +-----------------+    0x1090 ->  0x1000 (- 0x90)   --> here should be 4K 
>>> align, not 1K, because FFS header is 4K align.
>>
>> No, I don't think it works like that.
>>
>> The PI spec mentions the following regarding the alignment field in
>> the FFS header
>>
>> """
>> FFS_ATTRIB_DATA_ALIGNMENT
>> Indicates that the beginning of the file data (not the file header) must be 
>> aligned on a particular boundary relative to the firmware volume base """
>>
>> so the internal alignment for each section may be different, and is 
>> calculated from the first byte of the FFS file data. Then, if the FFS 
>> placement inside the FV adheres to the maximum alignment of all sections, 
>> they will all be aligned correctly. But this does not mean that all sections 
>> are aligned to the maximum alignment.
>>
>> --
>> Ard.
>>
>>
>>
>>> |  1 KB aligned   |
>>> |    section      |
>>> +-----------------+    0x1828 ->  0x1798 (- 0x90)
>>> |     normal      |
>>> | padding section |
>>> +-----------------+   0x1C90 -> 0x1C00 (- 0x90)
>>> |  4 KB aligned   |
>>> |    section      |
>>> +-----------------+   0x2748 -> 0x26b8 (- 0x90)
>>> |     normal      |
>>> | padding section |
>>> +-----------------+   0x2890 -> 0x2800 (- 0x90)
>>> |  1 KB aligned   |
>>> |    section      |
>>> +-----------------+
>>>
>>> Thanks
>>> Liming
>>> -----Original Message-----
>>> From: Liu, Yingke D
>>> Sent: Monday, July 27, 2015 4:52 PM
>>> To: Ard Biesheuvel
>>> Cc: Leif Lindholm; edk2-devel@lists.01.org; eugene.co...@hp.com; Gao,
>>> Liming
>>> Subject: RE: [edk2] [PATCH 0/4] FFS/FV aligment optimization (was:
>>> [RFC] small C model and LLVM/clang support for AARCH64)
>>>
>>> Hi Ard,
>>>
>>> Thanks for the details. I think it does work.
>>>
>>> A minor comment:
>>> +                             ((VOID *) 
>>> PaddingAlignmentSection.CommonHeader +
>>> +                                       PaddingSize);
>>>
>>> The void pointer arithmetic may be only recognized by GNU compiler, MS 
>>> compiler complains it, can you update it?
>>>
>>> Thanks,
>>> Dennis
>>>
>>> -----Original Message-----
>>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf
>>> Of Ard Biesheuvel
>>> Sent: Monday, July 27, 2015 16:34
>>> To: Liu, Yingke D
>>> Cc: Leif Lindholm; edk2-devel@lists.01.org; eugene.co...@hp.com; Gao,
>>> Liming
>>> Subject: Re: [edk2] [PATCH 0/4] FFS/FV aligment optimization (was:
>>> [RFC] small C model and LLVM/clang support for AARCH64)
>>>
>>> On 27 July 2015 at 10:08, Liu, Yingke D <yingke.d....@intel.com> wrote:
>>>> Hi Ard,
>>>>
>>>> From your patch, if the alignment of the section after 
>>>> SUBTYPE_GUID_SECTION is exactly the max alignment of the FFS alignment, it 
>>>> should work.
>>>> But if there are cases as Liming's example, the alignment of first section 
>>>> is 1K (2K data), the alignment of second section is 4K, it will not work.
>>>>
>>>> If we misunderstood your patch, can you please explain more?
>>>>
>>>
>>> Hello Dennis,
>>>
>>> According to the FFS spec, the global FFS alignment is the alignment of the 
>>> first byte after the FFS header. So it is up to GenFv to make sure that the 
>>> first byte after the header is aligned to the maximum alignment as 
>>> specified in the FFS header, and then, it will be guaranteed that all 
>>> sections inside the FFS, regardless of their alignment, will be aligned 
>>> correctly, since their alignment is specified relative to the first byte of 
>>> the FFS payload, which itself is aligned to the maximum of all section 
>>> alignments.
>>>
>>> With my patch, the only thing that changes is that, if the first byte after 
>>> the FFS header is misaligned relative to the maximum alignment, the same 
>>> number of bytes will be subtracted from a padding section that precedes all 
>>> FFS sections that need a certain alignment.
>>>
>>> So for instance, if the first FFS in an FV is to be loaded at 0x78, the 
>>> misalignment will be (0x78 + sizeof(FFS_HEADER)) == 0x90 bytes, and if we 
>>> would place an FFS at this position, all sections with alignment 
>>> requirements would be at <alignment> + 0x90 bytes instead of at 
>>> <alignment>, regardless of the exact value of <alignment>. So if we remove 
>>> 0x90 bytes from a single padding section that precedes all those aligned 
>>> sections, the alignment will be correct for all of them.
>>>
>>> +-----------------+      0x0 ->    0x0
>>> |    FV header    |
>>> +-----------------+     0x78 ->   0x78
>>> |   FFS header    |
>>> +-----------------+     0x90 ->   0x90
>>> |    SUB GUID     |
>>> | padding section |
>>> +-----------------+    0x490 ->  0x400 (- 0x90)
>>> |  1 KB aligned   |
>>> |    section      |
>>> +-----------------+    0xc28 ->  0xb98 (- 0x90)
>>> |     normal      |
>>> | padding section |
>>> +-----------------+   0x1090 -> 0x1000 (- 0x90)
>>> |  4 KB aligned   |
>>> |    section      |
>>> +-----------------+   0x1b48 -> 0x1ab8 (- 0x90)
>>> |     normal      |
>>> | padding section |
>>> +-----------------+   0x1c90 -> 0x1c00 (- 0x90)
>>> |  1 KB aligned   |
>>> |    section      |
>>> +-----------------+
>>>
>>> Note that the misalignment is based on the maximum alignment of all 
>>> sections, and the padding section is based on the alignment of the first 
>>> aligned section. So if the misalignment is too big (for instance, > 0x400 
>>> in the example), the SUB GUID section will be insufficient, and the only 
>>> way to get the correct alignment is to add a FV padding file.
>>>
>>> --
>>> Ard.
>>>
>>>
>>>> -----Original Message-----
>>>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf
>>>> Of Ard Biesheuvel
>>>> Sent: Monday, July 27, 2015 15:41
>>>> To: Gao, Liming
>>>> Cc: Liu, Yingke D; eugene.co...@hp.com; edk2-devel@lists.01.org;
>>>> Leif Lindholm
>>>> Subject: Re: [edk2] [PATCH 0/4] FFS/FV aligment optimization (was:
>>>> [RFC] small C model and LLVM/clang support for AARCH64)
>>>>
>>>> On 27 July 2015 at 06:28, Gao, Liming <liming....@intel.com> wrote:
>>>>> Ard:
>>>>>   I did more discussion with Dennis. We don't think this update can 
>>>>> resolve my case. We suggest to add SUB GUID section for FFS file that has 
>>>>> only one SECTION with the specific alignment. If so, after GenFv adjusts 
>>>>> SUB GUID section, it will have no impact on other section data. And, 
>>>>> after this change, GenFv tool also requires to update FFS HEAD to remove 
>>>>> alignment value, because the updated FFS doesn't meet with FFS alignment 
>>>>> requirement.
>>>>>
>>>>
>>>> Hello Liming,
>>>>
>>>> I am pretty confident that the current version of the implementation is 
>>>> correct, so I would love to see your counterexample.
>>>>
>>>> Note that the misalignment calculation and correction is based on the 
>>>> global alignment (max of all section alignments), and so it will correct 
>>>> the misalignment of all sections, even if one is 1 KB aligned and the 
>>>> other 4 KB. Do note that, if the 1 KB section comes first, the padding 
>>>> section may be too small to correct the FFS misalignment if it is much 
>>>> greater than 1 KB. In this case, we will just add a FV padding section as 
>>>> before.
>>>>
>>>> However, it does not make a difference for my particular use case, so I am 
>>>> fine implementing it like this.
>>>>
>>>> Thanks,
>>>> Ard.
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>>>>> Sent: Monday, July 27, 2015 3:04 AM
>>>>> To: Gao, Liming; Liu, Yingke D; edk2-devel@lists.01.org
>>>>> Cc: Leif Lindholm; eugene.co...@hp.com; Ard Biesheuvel
>>>>> Subject: Re: [PATCH 0/4] FFS/FV aligment optimization (was: [RFC]
>>>>> small C model and LLVM/clang support for AARCH64)
>>>>>
>>>>> On 22 July 2015 at 19:25, Ard Biesheuvel <ard.biesheu...@linaro.org> 
>>>>> wrote:
>>>>>> On 22 July 2015 at 19:21, Ard Biesheuvel <ard.biesheu...@linaro.org> 
>>>>>> wrote:
>>>>>>> These patches have been split off from the series 'small C model
>>>>>>> and LLVM/clang support for AARCH64', primarily because
>>>>>>> - I found out there is a tiny code model for GCC which is even better 
>>>>>>> than the
>>>>>>>   small model
>>>>>>> - there are better ways to implement the other size optimizations that 
>>>>>>> apply to
>>>>>>>   the ARM platform and virt packages.
>>>>>>>
>>>>>>> So this subset only deals with the basetools optimizations to get
>>>>>>> rid of excessive XIP alignment padding when using 2 KB or 4 KB
>>>>>>> alignment, which is common on AARCH64, since the exception vector
>>>>>>> table requires
>>>>>>> 2 KB alignment, and the small code model (which is the only one
>>>>>>> supported by the commercial LLVM based compiler supplied by ARM)
>>>>>>> requires 4 KB alignment. As it turns out, LLVM does support a
>>>>>>> large model, but it is even less suitable for PE/COFF conversion,
>>>>>>> because all absolute address are packed into series of moz/movk 
>>>>>>> instructions, and are not relocatable using COFF fixups.
>>>>>>>
>>>>>>> Patches #1 and #2 (formerly #2 and #3) are unchanged.
>>>>>>>
>>>>>>> Patch #3 (formerly #4) has been updated to ensure that a special
>>>>>>> reducible padding section is only emitted right before the first
>>>>>>> section in a FFS that has alignment requirements. Reducing the
>>>>>>> size of such a section will shift all subsequent sections into
>>>>>>> alignment, provided that the size of the padding is sufficient.
>>>>>>> In some cases, for instance, when the padding section is based on
>>>>>>> a section that has a minimum alignment of 32 bytes, and is
>>>>>>> followed by a section which requires 4 KB alignment, the size of
>>>>>>> the padding section may be too small and the adjustment will not be 
>>>>>>> possible.
>>>>>>> In this case, we simply proceed as if the padding section is an 
>>>>>>> ordinary padding section, and everything will just work as before.
>>>>>>>
>>>>>>
>>>>>> Forgot to add: I dropped the GenSec changes, and only changed
>>>>>> GenFfs this time, since I don't need it for my use case, and
>>>>>> section inside other section may be a bit complicated to reason about 
>>>>>> right now.
>>>>>>
>>>>>>> Patch #4 is unchanged, except for a clarification in the
>>>>>>> comments, to explain that the misalignment is calculated based on
>>>>>>> the first byte of the FFS payload, and not of the aligned
>>>>>>> section. Since all FFS sections are padded out relative to the
>>>>>>> first byte of the FFS payload, compensating its misalignment will shift 
>>>>>>> all sections into place.
>>>>>>>
>>>>>>> Ard Biesheuvel (4):
>>>>>>>   BaseTools/GenFw: move .debug contents to .data to save space
>>>>>>>   BaseTools/GenFw: move PE/COFF header closer to payload
>>>>>>>   BaseTools: use GUID identifiable section for FFS alignment padding
>>>>>>>   BaseTools/GenFv: optimize away redundant padding
>>>>>>>
>>>>>>
>>>>>> Branch is here:
>>>>>> https://git.linaro.org/people/ard.biesheuvel/uefi-next.git/shortlo
>>>>>> g/ r e fs/heads/ffs-fv-alignment-optimization
>>>>>>
>>>>>
>>>>> @Dennis: may I kindly ask you to share your opinion on these patches as 
>>>>> well? I think #1 and #2 are mostly uncontroversial (and Liming thinks 
>>>>> they are fine), but #3 and #4 may require more discussion and careful 
>>>>> review.
>>>>>
>>>>> Note that his series results in considerable space saving for PE/COFF 
>>>>> binaries with 2 KB or 4 KB section alignment, so they are important for 
>>>>> AARCH64.
>>>>>
>>>>> Kind regards,
>>>>> Ard.
>>>> _______________________________________________
>>>> edk2-devel mailing list
>>>> edk2-devel@lists.01.org
>>>> https://lists.01.org/mailman/listinfo/edk2-devel
>>> _______________________________________________
>>> edk2-devel mailing list
>>> edk2-devel@lists.01.org
>>> https://lists.01.org/mailman/listinfo/edk2-devel
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to