From: "Marvin Häuser" <mhaeuser@posteo.de>
To: Ard Biesheuvel <ardb@kernel.org>
Cc: Rebecca Cran <rebecca@bsdio.com>,
Pedro Falcato <pedro.falcato@gmail.com>,
Liming Gao <gaoliming@byosoft.com.cn>,
Ard Biesheuvel <ardb+tianocore@kernel.org>,
"devel@edk2.groups.io" <devel@edk2.groups.io>
Subject: Re: Linker scripts use of "-z common-page-size=0x20" etc.
Date: Tue, 4 Apr 2023 14:10:25 +0000 [thread overview]
Message-ID: <AC75D653-2839-448F-B074-D692AE09D572@posteo.de> (raw)
In-Reply-To: <CAMj1kXHBD8n5cyQPgdq2wC_w5e80Cah7bGQ+db41oemy1O9Wog@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 5314 bytes --]
> On 4. Apr 2023, at 16:03, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Tue, 4 Apr 2023 at 13:10, Rebecca Cran <rebecca@bsdio.com <mailto:rebecca@bsdio.com>> wrote:
>>
>> On 4/4/23 1:22 AM, Ard Biesheuvel wrote:
>>> On Mon, 3 Apr 2023 at 22:33, Rebecca Cran <rebecca@bsdio.com <mailto:rebecca@bsdio.com>> wrote:
>>>> As part of my work on the toolchain definitions, I've come across a
>>>> situation where ld.lld fails to align sections correctly, due to it
>>>> being invoked via clang with the '-n' option, which causes GenFw to fail
>>>> with "Section address not aligned to its own alignment.".
>>>>
>>> Stupid question: if it breaks stuff, why do you use -n ?
>>
>> As far as I can see, clang adds it automatically when it invokes ld.lld.
>>
>
> Ah right, fair enough.
>
>>>> The following messages are printed:
>>>>
>>>>
>>>> ld.lld: warning: -z common-page-size set, but paging disabled by omagic
>>>> or nmagic ld.lld: warning: address (0x558) of section .data is not a
>>>> multiple of alignment (16)
>>>>
>>>> I tracked the problem down to GccBase.lds and ClangBase.lds, which have:
>>>>
>>>> /* * The alignment of the .data section should be less than or equal to
>>>> the * alignment of the .text section. This ensures that the relative
>>>> offset * between these sections is the same in the ELF and the PE/COFF
>>>> versions of * this binary. */ .data ALIGN(ALIGNOF(.text)) :
>>>> ALIGN(CONSTANT(COMMONPAGESIZE)) { *(.data .data.* .gnu.linkonce.d.*)
>>>> *(.bss .bss.*) }
>>>>
>>>> I can work around the problem by removing "ALIGN(ALIGNOF(.text)", but
>>>> I'm wondering if our use of COMMONPAGESIZE/MAXPAGESIZE is correct.
>>>>
>>> What do you mean by 'correct'? The intent is clearly to declare the
>>> mapping granule size, and for SEC/PEI binaries that execute in place
>>> from flash, the MMU page size is not the most useful quantum here.
>>
>> On Discord, I think Marvin was saying that we shouldn't be using
>> COMMONPAGESIZE, but MAXPAGESIZE.
>>
>
> I don't have a preference for one over the other, although I fail to
> see how this fixes the -n problem above.
>
>> Since .text is aligned to COMMONPAGESIZE, I think I can remove the
>> ALIGN(ALIGNOF(.text)) and keep the ALIGN(CONSTANT(COMMONPAGESIZE))?
>>
>
> No. The alignment of .text could exceed COMMONPAGESIZE; this happens
> e.g., with AArch64 vector tables which are 2k aligned, when they are
> emitted into a SEC or PEIM module which only has 32 byte alignment by
> default.
>
> In this case, the PE/COFF section alignment will be 2k, and so .data
> should start at a 2k aligned address as well.
>
>>
>>> 'Page size' is highly context specific, and toolchain people are
>>> (imho) usually quite quick to call something abuse if it does not
>>> match their narrow definition of how a compiler or linker should be
>>> used. For the same reason, it has been so difficult to get a compiler
>>> to understand that the desire for position independent code does not
>>> imply that we want GOTs, or care about ELF symbol preemption or
>>> copy-on-write footprint of relocatable segments. In general, the bare
>>> metal use case (which includes EFI) is quite poorly understood by many
>>> people working on toolchains.
>>>
>>>> I'm wondering what the correct approach is here: should we do something
>>>> similar to how we set PECOFF_HEADER_SIZE and define a SECTION_ALIGNMENT
>>>> symbol?
>>> We cannot, that is the reason for using the page size switches here:
>>> using a symbol to set the location pointer is fine, but using a symbol
>>> to set the alignment of a section is not.
>>>
>>>> Or, as discussed on Discord should we just use
>>>> CONSTANT(MAXPAGESIZE) and ignore how it's normally used to specify the
>>>> maximum allowable page size for an executable?
>>>>
>>> Note that (when I last checked), the only effect of setting -z
>>> xxx-page-size is that those macros assume the associated value in the
>>> linker script. Nothing else in the linker changes behavior (with the
>>> exception of the warning you are seeing)
>>>
>>> So claiming abuse because the provided value does not match the page
>>> size of an OS that might also run on the same system is strenuous, and
>>> I think our use of it is fine. AIUI, the reason for having
>>> ClangBase.lds in addition to GccBase.lds is the fact that LLD does not
>>> support common but only max page size, so I think it should be fine to
>>> merge the two, and use max-page-size everywhere.
>>
>> Thanks for the explanation. This is probably an error on my part, but
>> when I tried to use max-page-size everywhere, the AARCH64 build of
>> ArmVirtQemu failed, with GenFw saying "AARCH64 small code model requires
>> identical ELF and PE/COFF section offsets modulo 4 KB."
>>
>
> Let me know if you manage to reproduce that - I'd like to understand
> what exactly is happening there.
>
>> LLD added support for common-page-size in 2019
>> (https://reviews.llvm.org/D56205), so I think we can keep the current
>> usage of common and max page size and just combine GccBase.lds and
>> ClangBase.lds.
>>
>
> Yes, that would be preferable.
It seems your mail server ate my mail as spam: https://edk2.groups.io/g/devel/message/102464
[-- Attachment #2: Type: text/html, Size: 20191 bytes --]
prev parent reply other threads:[~2023-04-04 14:10 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-03 20:33 Linker scripts use of "-z common-page-size=0x20" etc Rebecca Cran
2023-04-03 20:58 ` Marvin Häuser
2023-04-03 22:56 ` Rebecca Cran
2023-04-04 7:22 ` Ard Biesheuvel
2023-04-04 7:43 ` Marvin Häuser
2023-04-04 11:10 ` Rebecca Cran
2023-04-04 14:03 ` Ard Biesheuvel
2023-04-04 14:10 ` Marvin Häuser [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=AC75D653-2839-448F-B074-D692AE09D572@posteo.de \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox