public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* Linker scripts use of "-z common-page-size=0x20" etc.
@ 2023-04-03 20:33 Rebecca Cran
  2023-04-03 20:58 ` Marvin Häuser
  2023-04-04  7:22 ` Ard Biesheuvel
  0 siblings, 2 replies; 8+ messages in thread
From: Rebecca Cran @ 2023-04-03 20:33 UTC (permalink / raw)
  To: Pedro Falcato, Marvin Häuser, Liming Gao, Ard Biesheuvel
  Cc: devel@edk2.groups.io

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.".

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.


We pass in a value of 0x20, 0x40 or 0x1000 to "-z common-page-size" with 
0x20 being the most common value.

Given the page size of the target will never be 32 bytes, the following 
comment on https://reviews.llvm.org/D61688 makes sense:


"There is at least one linkerscript in Tianocore edk2 that (ab)uses -n 
-z common-page-size=0x20 to use CONSTANT(COMMONPAGESIZE) as if it were a 
preprocessor macro set with -D in the compiler. The usual approach to 
this is to pre-process the linkerscript."


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? 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?


-- 
Rebecca Cran


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Linker scripts use of "-z common-page-size=0x20" etc.
  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
  1 sibling, 1 reply; 8+ messages in thread
From: Marvin Häuser @ 2023-04-03 20:58 UTC (permalink / raw)
  To: Rebecca Cran; +Cc: Pedro Falcato, Liming Gao, Ard Biesheuvel, devel


> On 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.".
> 
> 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.
> 
> 
> We pass in a value of 0x20, 0x40 or 0x1000 to "-z common-page-size" with 0x20 being the most common value.
> 
> Given the page size of the target will never be 32 bytes, the following comment on https://reviews.llvm.org/D61688 makes sense:
> 
> 
> "There is at least one linkerscript in Tianocore edk2 that (ab)uses -n -z common-page-size=0x20 to use CONSTANT(COMMONPAGESIZE) as if it were a preprocessor macro set with -D in the compiler. The usual approach to this is to pre-process the linkerscript."
> 
> 
> 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? 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?

That last part is actually not ignoring the use-case, that *is* our use-case. The terminology again is very OS-oriented, it’s important to know that generally OSes will fail to load binaries that are aligned less than the platform page size, as they cannot apply permissions (and probably also some implementation details of mmap / VM / whatever). That’s why the maximum page size you’re realistic to encounter is exactly the image segment alignment (by hardware convention, this is a power of two, thus a strict alignment satisfies all less strict alignment constraints).

The common page size on the other hand appears to be an optimization, for which you specify the most common page size (e.g., you may target AARCH64 which may require 16 KB alignment, but most of your targets will have only 4 KB pages), which the compiler will use to optimize the binary for the common targets. I have no idea why this is even used. There also were discussions on LLVM platforms that it should be avoided.

The naive approach would be to just use max-page-size, drop all references to common-page-size, and align all ELF sections that will be converted to PE sections by max-page-size. But I’m sure there’s some ancient workaround / compiler bug / edge use case / portability or whatever reason why common-page-size was used. :)
(CC Leif for related experience.)

edk2 generally sets this to a low value to save SPI (and possibly RAM) space, as nothing in the stack enforces memory protection ( :( ). I’m not sure why there’s both 32 and 64 Bytes, but I could imagine it’s some GNU ABI thing where some type of some arch actually requires this alignment, or maybe there’s different rules for global variables. A text segment must at least satisfy the maximum instruction alignment constraint (this may be a thing with, e.g., normalised instruction lengths), while data segments must satisfy at least the maximum data alignment constraint (which might usually be some large float? Not sure). 4 KB is used when memory protection is needed (usually RT drivers, as they’re mapped into the OS environment), but AARCH64 may actually require 16 KB (e.g. Apple A chips didn’t even support less for a while).

Best regards,
Marvin

> 
> 
> -- 
> Rebecca Cran
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Linker scripts use of "-z common-page-size=0x20" etc.
  2023-04-03 20:58 ` Marvin Häuser
@ 2023-04-03 22:56   ` Rebecca Cran
  0 siblings, 0 replies; 8+ messages in thread
From: Rebecca Cran @ 2023-04-03 22:56 UTC (permalink / raw)
  To: Marvin Häuser; +Cc: Pedro Falcato, Liming Gao, Ard Biesheuvel, devel

On 4/3/23 2:58 PM, Marvin Häuser wrote:

> That last part is actually not ignoring the use-case, that *is* our use-case. The terminology again is very OS-oriented, it’s important to know that generally OSes will fail to load binaries that are aligned less than the platform page size, as they cannot apply permissions (and probably also some implementation details of mmap / VM / whatever). That’s why the maximum page size you’re realistic to encounter is exactly the image segment alignment (by hardware convention, this is a power of two, thus a strict alignment satisfies all less strict alignment constraints).
>
> The common page size on the other hand appears to be an optimization, for which you specify the most common page size (e.g., you may target AARCH64 which may require 16 KB alignment, but most of your targets will have only 4 KB pages), which the compiler will use to optimize the binary for the common targets. I have no idea why this is even used. There also were discussions on LLVM platforms that it should be avoided.
>
> The naive approach would be to just use max-page-size, drop all references to common-page-size, and align all ELF sections that will be converted to PE sections by max-page-size. But I’m sure there’s some ancient workaround / compiler bug / edge use case / portability or whatever reason why common-page-size was used. :)
> (CC Leif for related experience.)
>
> edk2 generally sets this to a low value to save SPI (and possibly RAM) space, as nothing in the stack enforces memory protection ( :( ). I’m not sure why there’s both 32 and 64 Bytes, but I could imagine it’s some GNU ABI thing where some type of some arch actually requires this alignment, or maybe there’s different rules for global variables. A text segment must at least satisfy the maximum instruction alignment constraint (this may be a thing with, e.g., normalised instruction lengths), while data segments must satisfy at least the maximum data alignment constraint (which might usually be some large float? Not sure). 4 KB is used when memory protection is needed (usually RT drivers, as they’re mapped into the OS environment), but AARCH64 may actually require 16 KB (e.g. Apple A chips didn’t even support less for a while).


Thanks.

One problem with using max page size appears to be that there are some 
modules that set the common page size to 4KB in order to enable memory 
protection. e.g.:


commit ddd89cd50dd3a989e58a75ed38011168e3ec0954
Author: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Date:   Wed Sep 30 08:53:00 2015 +0000

     OvmfPkg: set 4 KB section alignment for DXE_RUNTIME_DRIVER modules

     Increase the section alignment to 4 KB for DXE_RUNTIME_DRIVER modules.
     This allows the OS to map them with tightened permissions (i.e., 
R-X for
     .text and RW- for .data). This is a prerequisite for enabling the
EFI_PROPERTIES_RUNTIME_MEMORY_PROTECTION_NON_EXECUTABLE_PE_DATA (sic)
     feature that was introduced in UEFIv2.5.

     Contributed-under: TianoCore Contribution Agreement 1.0
     Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
     Reviewed-by: Laszlo Ersek <lersek@redhat.com>
     Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
     Tested-by: Laszlo Ersek <lersek@redhat.com>

     git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@18564 
6f19259b-4bc3-4df7-8a09-765794883524


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Linker scripts use of "-z common-page-size=0x20" etc.
  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-04  7:22 ` Ard Biesheuvel
  2023-04-04  7:43   ` Marvin Häuser
  2023-04-04 11:10   ` Rebecca Cran
  1 sibling, 2 replies; 8+ messages in thread
From: Ard Biesheuvel @ 2023-04-04  7:22 UTC (permalink / raw)
  To: Rebecca Cran
  Cc: Pedro Falcato, Marvin Häuser, Liming Gao, Ard Biesheuvel,
	devel@edk2.groups.io

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 ?

> 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.

>
> We pass in a value of 0x20, 0x40 or 0x1000 to "-z common-page-size" with
> 0x20 being the most common value.
>
> Given the page size of the target will never be 32 bytes, the following
> comment on https://reviews.llvm.org/D61688 makes sense:
>
>
> "There is at least one linkerscript in Tianocore edk2 that (ab)uses -n
> -z common-page-size=0x20 to use CONSTANT(COMMONPAGESIZE) as if it were a
> preprocessor macro set with -D in the compiler. The usual approach to
> this is to pre-process the linkerscript."
>

'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.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Linker scripts use of "-z common-page-size=0x20" etc.
  2023-04-04  7:22 ` Ard Biesheuvel
@ 2023-04-04  7:43   ` Marvin Häuser
  2023-04-04 11:10   ` Rebecca Cran
  1 sibling, 0 replies; 8+ messages in thread
From: Marvin Häuser @ 2023-04-04  7:43 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Rebecca Cran, Pedro Falcato, Liming Gao, Ard Biesheuvel, devel


[-- Attachment #1.1: Type: text/html, Size: 10513 bytes --]

[-- Attachment #2: dummyfile.0.part --]
[-- Type: image/png, Size: 6182 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Linker scripts use of "-z common-page-size=0x20" etc.
  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
  1 sibling, 1 reply; 8+ messages in thread
From: Rebecca Cran @ 2023-04-04 11:10 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Pedro Falcato, Marvin Häuser, Liming Gao, Ard Biesheuvel,
	devel@edk2.groups.io

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.

>> 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.

Since .text is aligned to COMMONPAGESIZE, I think I can remove the 
ALIGN(ALIGNOF(.text)) and keep the ALIGN(CONSTANT(COMMONPAGESIZE))?


> '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."

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.


-- 
Rebecca Cran


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Linker scripts use of "-z common-page-size=0x20" etc.
  2023-04-04 11:10   ` Rebecca Cran
@ 2023-04-04 14:03     ` Ard Biesheuvel
  2023-04-04 14:10       ` Marvin Häuser
  0 siblings, 1 reply; 8+ messages in thread
From: Ard Biesheuvel @ 2023-04-04 14:03 UTC (permalink / raw)
  To: Rebecca Cran
  Cc: Pedro Falcato, Marvin Häuser, Liming Gao, Ard Biesheuvel,
	devel@edk2.groups.io

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.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Linker scripts use of "-z common-page-size=0x20" etc.
  2023-04-04 14:03     ` Ard Biesheuvel
@ 2023-04-04 14:10       ` Marvin Häuser
  0 siblings, 0 replies; 8+ messages in thread
From: Marvin Häuser @ 2023-04-04 14:10 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Rebecca Cran, Pedro Falcato, Liming Gao, Ard Biesheuvel,
	devel@edk2.groups.io

[-- 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 --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-04-04 14:10 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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 is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox