From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mout01.posteo.de (mout01.posteo.de [185.67.36.65]) by mx.groups.io with SMTP id smtpd.web10.102621.1680617438061627930 for ; Tue, 04 Apr 2023 07:10:38 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@posteo.de header.s=2017 header.b=eMvTibqf; spf=pass (domain: posteo.de, ip: 185.67.36.65, mailfrom: mhaeuser@posteo.de) Received: from submission (posteo.de [185.67.36.169]) by mout01.posteo.de (Postfix) with ESMTPS id 87F612405DE for ; Tue, 4 Apr 2023 16:10:35 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.de; s=2017; t=1680617435; bh=1Tn0a522m2M2A7O1KB8NheK4pbLNuymLEpdxE4Kd1SM=; h=From:Subject:Date:Cc:To:From; b=eMvTibqf/iniNQbGPVe17jspVkC+kO3zkLOaGUHcXfF3uVKGJK5AlctcymbMYrEuK OOrSZEYW/Xmr8jmAxv8rya/lJ775yq7se0pyilgZBi6ztJYys8I6yJhNMlVYL4qaeI bR7V/lCiTMkT7cgFHwSc9hiCd4Vj8/pt9MJ6qWomFZcCtNgWZ3bQSKpIovTPMaHPVC CPTtMUJrkTKIYIbfXsujPqhLp1IdhZeIVcRGqs/hsErwesOS6hgO+aaRQLW/7te40K N33NG44zIAfah7D5mhJXzBMaCC8SSSPx1tB9YBk32SJ7hCTPK7Pe1Ndzq6qfKtnCRI f2d5ANRXqT/uQ== Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4PrV613Nxgz9rxQ; Tue, 4 Apr 2023 16:10:33 +0200 (CEST) From: =?UTF-8?B?TWFydmluIEjDpHVzZXI=?= Message-Id: Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.500.231\)) Subject: Re: Linker scripts use of "-z common-page-size=0x20" etc. Date: Tue, 4 Apr 2023 14:10:25 +0000 In-Reply-To: Cc: Rebecca Cran , Pedro Falcato , Liming Gao , Ard Biesheuvel , "devel@edk2.groups.io" To: Ard Biesheuvel References: <9befc3e1-02b6-c0c4-7eba-cccd84dedd3d@bsdio.com> <5c7df62e-2802-6a70-49a4-3af5c1625a16@bsdio.com> Content-Type: multipart/alternative; boundary="Apple-Mail=_91C5B0EF-1C65-44C1-B80E-7086B4F6CAF3" --Apple-Mail=_91C5B0EF-1C65-44C1-B80E-7086B4F6CAF3 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=us-ascii > On 4. Apr 2023, at 16:03, Ard Biesheuvel wrote: >=20 > On Tue, 4 Apr 2023 at 13:10, Rebecca Cran > wrote: >>=20 >> On 4/4/23 1:22 AM, Ard Biesheuvel wrote: >>> On Mon, 3 Apr 2023 at 22:33, Rebecca Cran > 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.". >>>>=20 >>> Stupid question: if it breaks stuff, why do you use -n ? >>=20 >> As far as I can see, clang adds it automatically when it invokes = ld.lld. >>=20 >=20 > Ah right, fair enough. >=20 >>>> The following messages are printed: >>>>=20 >>>>=20 >>>> 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) >>>>=20 >>>> I tracked the problem down to GccBase.lds and ClangBase.lds, which = have: >>>>=20 >>>> /* * 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.*) } >>>>=20 >>>> I can work around the problem by removing "ALIGN(ALIGNOF(.text)", = but >>>> I'm wondering if our use of COMMONPAGESIZE/MAXPAGESIZE is correct. >>>>=20 >>> 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. >>=20 >> On Discord, I think Marvin was saying that we shouldn't be using >> COMMONPAGESIZE, but MAXPAGESIZE. >>=20 >=20 > I don't have a preference for one over the other, although I fail to > see how this fixes the -n problem above. >=20 >> Since .text is aligned to COMMONPAGESIZE, I think I can remove the >> ALIGN(ALIGNOF(.text)) and keep the ALIGN(CONSTANT(COMMONPAGESIZE))? >>=20 >=20 > 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. >=20 > In this case, the PE/COFF section alignment will be 2k, and so .data > should start at a 2k aligned address as well. >=20 >>=20 >>> '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. >>>=20 >>>> 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. >>>=20 >>>> 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? >>>>=20 >>> 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) >>>=20 >>> 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. >>=20 >> 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." >>=20 >=20 > Let me know if you manage to reproduce that - I'd like to understand > what exactly is happening there. >=20 >> 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. >>=20 >=20 > Yes, that would be preferable. It seems your mail server ate my mail as spam: = https://edk2.groups.io/g/devel/message/102464 --Apple-Mail=_91C5B0EF-1C65-44C1-B80E-7086B4F6CAF3 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=us-ascii

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> = wrote:

On 4/4/23 1:22 AM, Ard Biesheuvel = wrote:
On Mon, 3 Apr 2023 at 22:33, Rebecca = Cran <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.


= --Apple-Mail=_91C5B0EF-1C65-44C1-B80E-7086B4F6CAF3--