On 8 July 2015 at 20:01, Andrew Fish <af...@apple.com> wrote: > > On Jul 8, 2015, at 10:49 AM, Ard Biesheuvel <ard.biesheu...@linaro.org> > wrote: > > On 8 July 2015 at 16:57, Olivier Martin <olivier.mar...@arm.com> wrote: > > When EDK2 is built for the small memory model with AArch64 LLVM, > some page-relative relocations (R_AARCH64_ADR_PREL_PG_HI21 and its > R_AARCH64_LDST(16|32|64|128)_ABS_LO12_NC companions) that were not > supported before by EDK2 BaseTools are now needed. > > This change adds support for these relocations in BaseTools. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Harry Liebel <harry.lie...@arm.com> > Reviewed-by: Olivier Martin <olivier.mar...@arm.com> > > > I think all the relocation arithmetic looks fine. > > I do have a concern about the consequences of using the small C model > in this way: nothing in the metadata of the resulting PE/COFF images > reflects that the 4 KB alignment needs to be preserved in order to > successfully execute them, so you are essentially producing corrupt > images. > > > Ard, > > I’m missing something here. The PE/COFF header contains SectionAlignment and > FileAlignment fields in the optional (for COFF, required for PE/COFF) > headers? > SectionAlignment is the alignment of sections when copied to memory. > FileAlignment is the alignment in the PE/COFF file. >
Indeed. But unlike ELF, PE/COFF does not allow the header to overlap with the .text section, so using 4 KB alignment for a XIP modules .text section always wastes 3.5 KB in the FV. Olivier's patch allows code to be generated that breaks if loaded at an incorrect alignment, since adding the alignment required to prevent such breakage results in the wasted space for XIP modules. Then it fixes GenFv by making it aware that AARCH64 can only be adjusted (relocated) by multiples of 4 KB even if the PE/COFF metadata indicates nothing of the sort. I would rather change GenFv so it can place the PE/COFF image in such a way that the .text section meets its minimum alignment without necessarily aligning the start of the image to the same value, either by omitting the PE/COFF header entirely in the FV (if that is allowed by the spec) or by moving the header closer to the .text section and loading the image SIZEOF_HEADERS bytes before the alignment boundary. That way, the section alignment correctly reflects the 4 KB value required to support the small C model for AArch64. -- Ard. > Every PE/COFF loader needs to be modified (including the ones > in GRUB and shim, for instance) to prevent such images from being > loaded in an incorrect way. (I know that most PE/COFF loaders use > AllocatePages() which aligns these binaries at 4 KB implicitly, but > this is not mandatory) > > So the correct way to do this would be to only use the small C model > when this alignment can be guaranteed, i.e., .text and .data are both > aligned at 4 KB. With the latest GenFw changes in place (which sets > PE/COFF section alignment to the largest ELF input section alignment > it encounters), the small C model works correctly since the PE/COFF > loaders adhere to the PE/COFF section alignment. As a bonus, the ELF > and PE/COFF images will be laid out identically in memory, so all the > recalculation arithmetic becomes unnecessary (although it probably > makes sense to perform some checks here) > > The downside, of course, is that 4 KB alignment for both .text and > .data wastes a lot of space: 3.5 KB only for the padding between the > PE/COFF header and the start of .text, and 2 KB on average between > .text and .data. Due to the compression of non-XIP binaries, this is > not such a penalty, but for XIP it is prohibitive (even if these > typically don't have a .data section) > > I am going to have a look at GenFv to figure out if there is some way > to work around this, i.e., drop the PE/COFF headers or move them in > some way. > > In the mean time, I think this patch is good except the first hunk. > The small model relocation arithmetic can be merged separately, I > think, and the logic and policies around how and when to relocate > PE/COFF images can be addressed in a followup patch. > > -- > Ard. > > > > ------------------------------------------------------------------------------ > Don't Limit Your Business. Reach for the Cloud. > GigeNET's Cloud Solutions provide you with the tools and support that > you need to offload your IT needs and focus on growing your business. > Configured For All Businesses. Start Your Cloud Today. > https://www.gigenetcloud.com/ > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/edk2-devel > ------------------------------------------------------------------------------ Don't Limit Your Business. Reach for the Cloud. GigeNET's Cloud Solutions provide you with the tools and support that you need to offload your IT needs and focus on growing your business. Configured For All Businesses. Start Your Cloud Today. https://www.gigenetcloud.com/ _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
