From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by mx.groups.io with SMTP id smtpd.web11.102525.1680617039845624181 for ; Tue, 04 Apr 2023 07:04:00 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=R52hfrsm; spf=pass (domain: kernel.org, ip: 139.178.84.217, mailfrom: ardb@kernel.org) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 5554F62DE7 for ; Tue, 4 Apr 2023 14:03:59 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id BD432C4339E for ; Tue, 4 Apr 2023 14:03:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1680617038; bh=YZhrjvs/DGwFYqTTH4AuFgwQx2xKmzx5usj7MKR3IU4=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=R52hfrsm+N4ogKedhy/RDlXFyo1bmcp/K6dVTyksQLLTVsqFJCAcX2nDwoxbdXFjl Q2d8mJA1i9OjKobU/kfe0nXz8vWymlJo1Mywk71YG8QeMhsn+KJeqOD3/TsE3ZSetQ PI6/Tu0o/qtV4s0ifiXZAlWIXuOqxpC7uBY7lTmfrJ4ShE2Dr1aUxrupPHQXiIKeOB BHsQkTbL8i4/MOhzp6BoGCaZuwp0K2fdN41UvObzxS/6lgT5psMA7gJLtG/P6gCWew rrFtT9WKvDGYyOw4ZTJj1/AALYAy/YbpAjXOmMdDqzFhWVHp61877uSqXpue3NUR3F mA98Rb7bXNddw== Received: by mail-lj1-f175.google.com with SMTP id s20so13654013ljp.7 for ; Tue, 04 Apr 2023 07:03:58 -0700 (PDT) X-Gm-Message-State: AAQBX9eUV6EBBdbnyTgkPoWStkhvf8mhHwfKUXrPLkSTEkR5tmSggtXn XaEcGoVMcKSjEMgydpvriZZoQvCOvhpv/HcX/7k= X-Google-Smtp-Source: AKy350YnsNx0dvTn1keruImoRZaCiGTxjwIia2Qbphn9wcbUQg188D9LLUaAuHXZKJ4CR44bXZfN8CzcvZOOTkUj50M= X-Received: by 2002:a2e:99c2:0:b0:2a6:32bf:6745 with SMTP id l2-20020a2e99c2000000b002a632bf6745mr1027572ljj.2.1680617036684; Tue, 04 Apr 2023 07:03:56 -0700 (PDT) MIME-Version: 1.0 References: <9befc3e1-02b6-c0c4-7eba-cccd84dedd3d@bsdio.com> <5c7df62e-2802-6a70-49a4-3af5c1625a16@bsdio.com> In-Reply-To: <5c7df62e-2802-6a70-49a4-3af5c1625a16@bsdio.com> From: "Ard Biesheuvel" Date: Tue, 4 Apr 2023 16:03:45 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: Linker scripts use of "-z common-page-size=0x20" etc. To: Rebecca Cran Cc: Pedro Falcato , =?UTF-8?Q?Marvin_H=C3=A4user?= , Liming Gao , Ard Biesheuvel , "devel@edk2.groups.io" Content-Type: text/plain; charset="UTF-8" On Tue, 4 Apr 2023 at 13:10, Rebecca Cran wrote: > > 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.". > >> > > 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.