On 31 July 2015 at 01:17, Laszlo Ersek <ler...@redhat.com> wrote: > On 07/27/15 15:52, Ard Biesheuvel wrote: >> On 27 July 2015 at 15:34, Liu, Yingke D <yingke.d....@intel.com> wrote: >>> Reviewed-by: Yingke Liu <yingke.d....@intel.com> >>> >> >> Thank you >> >> Committed as SVN r18077 ... r18080 >> >> I do have another question related to the use of FIXED in a [Rule] section: >> since the increased alignment on AARCH64 will only be necessary for >> some toolchains, is it possible to have separate [Rule] sections? >> I did notice the 'DEBUG_MYTOOLS_IA32' in [Rule] section (for instance, >> EmbeddedPkg/EmbeddedPkg.fdf:102) but I don't quite understand how it >> works. > > Looking at the __GetFileOpts() method in > "BaseTools/Source/Python/GenFds/FdfParser.py", I think that pattern of > tool chain keywords serves exactly the purpose you have in mind. I > checked a couple occurrences of DEBUG_MYTOOLS_IA32 in various FDF files, > and it always seems to restrict rules for the file type > [Rule.Common.PEIM.TIANOCOMPRESSED]. So, in practice, it doesn matter: > for ARM you usually (universally?) don't have LZMA compressed PEIMs, > because PEIMs run from flash. The DEBUG_MYTOOLS_IA32 restriction doesn't > matter because you don't have that module type anyway. >
OK, so the restriction means that this particular rule will only be applied if you are building DEBUG for IA32 with the MYTOOLS toolchains? > Anyway, I think if you are going to add (or try) this kind of > restriction, then I should postpone reviewing > > [edk2] [PATCH 2/4] ArmVirtPkg: use 'auto' alignment and FIXED > placement for XIP modules > http://thread.gmane.org/gmane.comp.bios.edk2.devel/547/focus=545 > > because you might want to post a new version of it. Isn't that so? > No, not really. The reason I was asking is for the CLANG patches I posted as well. As explained before, CLANG requires 4 kB alignment so that its relative symbol references resolve correctly. > In any case, my most important question follows: the FDF spec says that > FIXED means the file cannot be moved, it is not relocateable. But, how > does that "enable new optimizations in GenFfs", as you write in the > linked patch? I found this in SVN r18079: > > + // > + // Only add a special reducible padding section if > + // - this FFS has the FFS_ATTRIB_FIXED attribute, > + // - none of the preceding sections have alignment requirements, > + // - the size of the padding is sufficient for the > + // EFI_SECTION_FREEFORM_SUBTYPE_GUID header. > + // > + if ((FfsAttrib & FFS_ATTRIB_FIXED) != 0 && > + MaxEncounteredAlignment <= 1 && > + Offset >= sizeof (EFI_FREEFORM_SUBTYPE_GUID_SECTION)) { > + SectHeader->CommonHeader.Type = EFI_SECTION_FREEFORM_SUBTYPE_GUID; > + SectHeader->SubTypeGuid = mEfiFfsSectionAlignmentPaddingGuid; > + } else { > + SectHeader->CommonHeader.Type = EFI_SECTION_RAW; > + } > > I'm quite sure you've explained why FIXED is important for this, but I > can't recall the reason. Care to refresh my memory? Is it inherently > related to mcmodel=small? > No, it has nothing to do with that. The optimization I implemented results in the FFS to be unmovable, i.e., you can no longer infer from the metadata at which alignment you would need to load it to get the various sections to line up correctly. So initially, I suggested that GenFfs would set the FFS_ATTRIB_FIXED flag when applying the optimization. However, as Liming pointed out, this may affect use cases where some FFS is loaded into memory programmatically: such images would lose their ability to be loaded anywhere all of a sudden due to this optimization being applied. So instead, we set the attribute upfront for files that don't need to be moved around, and only apply the optimization if it has the flag set already. Note that this does not affect the ability to shadow the PEIMs, those are loaded from the TE images, not from the FFS containers. -- Ard. >>> -----Original Message----- >>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] >>> Sent: Monday, July 27, 2015 8:32 PM >>> To: Gao, Liming; Liu, Yingke D; edk2-devel@lists.01.org >>> Cc: leif.lindh...@linaro.org; eugene.co...@hp.com; Ard Biesheuvel >>> Subject: [PATCH v2 0/4] FFS/FV aligment optimization (was: [RFC] small C >>> model and LLVM/clang support for AARCH64) >>> >>> This series 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. >>> >>> >>> -------------------------------------------------------------------------------- >>> v2 changelog >>> - patches #1 and #2 are unchanged >>> - patch #3 and #4 have been updated to only emit or adjust the special >>> padding >>> section if the FDF [Rule] defines the FFS_ATTRIB_FIXED attributed for the >>> file >>> - the adjustment logic in patch #4 has been reordered and the comments >>> updated >>> to reflect more clearly that the misalignment and adjustment are not >>> specific >>> to a single FFS section, but to the alignment of the FFS file as a whole >>> - patch #4 now clears the alignment bits in the FFS header since they have >>> no >>> meaning in FFS files that have been modified in place >>> - replaced pointer arithmetic using VOID* pointers >>> >>> -------------------------------------------------------------------------------- >>> v1 changelog compared to 'RFC: small C model and LLVM/clang support for >>> AARCH64' >>> >>> 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. >>> >>> 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 >>> >>> BaseTools/Source/C/GenFfs/GenFfs.c | 99 >>> +++++++----- >>> BaseTools/Source/C/GenFv/GenFvInternalLib.c | 169 >>> +++++++++++++++++++- >>> BaseTools/Source/C/GenFw/Elf32Convert.c | 66 ++++---- >>> BaseTools/Source/C/GenFw/Elf64Convert.c | 64 ++++---- >>> BaseTools/Source/C/Include/Guid/FfsSectionAlignmentPadding.h | 22 +++ >>> 5 files changed, 318 insertions(+), 102 deletions(-) create mode 100644 >>> BaseTools/Source/C/Include/Guid/FfsSectionAlignmentPadding.h >>> >>> -- >>> 1.9.1 >>> >> _______________________________________________ >> 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
