public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Jake Garver via groups.io" <jake=nvidia.com@groups.io>
To: Ard Biesheuvel <ardb@kernel.org>,
	Pedro Falcato <pedro.falcato@gmail.com>
Cc: "devel@edk2.groups.io" <devel@edk2.groups.io>,
	"rebecca@bsdio.com" <rebecca@bsdio.com>,
	"gaoliming@byosoft.com.cn" <gaoliming@byosoft.com.cn>,
	"bob.c.feng@intel.com" <bob.c.feng@intel.com>,
	"yuwei.chen@intel.com" <yuwei.chen@intel.com>
Subject: Re: [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when converting ADR to ADRP
Date: Thu, 2 Nov 2023 11:47:42 +0000	[thread overview]
Message-ID: <PH0PR12MB7888E5A1D9416C56CE811598ADA6A@PH0PR12MB7888.namprd12.prod.outlook.com> (raw)
In-Reply-To: <PH0PR12MB78888BF471FF31CDC602F968ADDCA@PH0PR12MB7888.namprd12.prod.outlook.com>

[-- Attachment #1: Type: text/plain, Size: 5247 bytes --]

Ard, Pedro,

How would you like to proceed here?

  1.
Do nothing.  Treat it as a toolchain bug and not attempt a work-around.  Practically speaking, this means the Ubuntu20 container at https://github.com/tianocore/containers cannot be updated to the latest GCC 10.x (10.5).  We can pin it to 10.3 or EOL it, migrating to Ubuntu22.
  2.  Rework the patch as a work-around.
  3.  ??? Something I missed.

Note in option 1, I'm assuming that other versions of GCC, e.g. 12.x used in Ubuntu22, don't eventually inherit this regression. But we can always revisit the work-around if we determine that to be the case.  I'm starting Ubuntu22 testing now, so I may have an answer soon.

Thanks,
Jake
________________________________
From: Jake Garver <jake@nvidia.com>
Sent: Friday, October 27, 2023 11:52 AM
To: Ard Biesheuvel <ardb@kernel.org>; Pedro Falcato <pedro.falcato@gmail.com>
Cc: devel@edk2.groups.io <devel@edk2.groups.io>; rebecca@bsdio.com <rebecca@bsdio.com>; gaoliming@byosoft.com.cn <gaoliming@byosoft.com.cn>; bob.c.feng@intel.com <bob.c.feng@intel.com>; yuwei.chen@intel.com <yuwei.chen@intel.com>
Subject: Re: [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when converting ADR to ADRP

Ard, Pedro,

> Could you try adding -Wl,--no-relax to the DLINK_FLAGS for your build and see if it makes a difference?

With "-Wl,--no-relax", I still see an ADR instruction.  Clean build.  Verified the "-Wl,--no-relax" is part of the gcc all to build this dll.

0000000000002fec <fdt_path_offset>:
    2fec:   a9b97bfd    stp   x29, x30, [sp, #-112]!
    2ff0:   910003fd    mov   x29, sp
    2ff4:   a90363f7    stp   x23, x24, [sp, #48]
    2ff8:   aa0003f8    mov   x24, x0
    2ffc:   10020020    adr   x0, 7000 <_cont+0xe98>
                  2ffc: R_AARCH64_ADR_GOT_PAGE  __stack_chk_guard
    3000:   a90153f3    stp   x19, x20, [sp, #16]
    3004:   aa0103f3    mov   x19, x1
    3008:   f946cc00    ldr   x0, [x0, #3480]
                  3008: R_AARCH64_LD64_GOT_LO12_NC    __stack_chk_guard

How do you think I should proceed?

Thanks for all your help!
Jake
________________________________
From: Ard Biesheuvel <ardb@kernel.org>
Sent: Friday, October 27, 2023 10:43 AM
To: Pedro Falcato <pedro.falcato@gmail.com>
Cc: devel@edk2.groups.io <devel@edk2.groups.io>; Jake Garver <jake@nvidia.com>; rebecca@bsdio.com <rebecca@bsdio.com>; gaoliming@byosoft.com.cn <gaoliming@byosoft.com.cn>; bob.c.feng@intel.com <bob.c.feng@intel.com>; yuwei.chen@intel.com <yuwei.chen@intel.com>
Subject: Re: [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when converting ADR to ADRP

External email: Use caution opening links or attachments


On Fri, 27 Oct 2023 at 16:26, Pedro Falcato <pedro.falcato@gmail.com> wrote:
>
> On Fri, Oct 27, 2023 at 3:13 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Fri, 27 Oct 2023 at 16:09, Jake Garver via groups.io
> > <jake=nvidia.com@groups.io> wrote:
> > >
> > > Hi Ard:
> > >
> > > > Can you double check the object file? I suspect this is a linker relaxation not a compiler issue.
> > >
> > > With LTO in play, is there a way to check the object file?  It's not in aarch64 assembly.
> > >
> >
> > Perhaps not.
> >
> > Could you try adding -Wl,--no-relax to the DLINK_FLAGS for your build
> > and see if it makes a difference?
> >
> > We should be adding that in any case, but it would be good to know if
> > it helps here too.
>
> I've never checked on aarch64, but don't you get solid space savings
> with linker relaxation? Or is it mostly meaningless?
>

The most important use case for linker relaxation is turning GOT
lookups into relative references, given that PIC object files are
typically constructed without prior knowledge of whether a given
external reference will be satisfied by the same dynamic object or a
different one. So each relaxation removes a load from the program and
may reduce the size of the GOT (if no other non-relaxable references
to it exist) and this might matter on a hot path.

None of this makes sense for UEFI, though, given that all executables
are fully linked and performance doesn't usually matter to the same
extent.

On AArch64, there is a specific advantage to being able to relax ADRP
to ADR (and this is why GenFw implements this relaxation itself too):
ADRP instructions need to appear at the same offset modulo 4k as they
were linked at, which means 4k alignment for the entire code section.
EDK2 builds are made up of lots of tiny SEC and PEI drivers and
rounding up each of those to 4k would result in a substantial increase
in footprint of the firmware image.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110524): https://edk2.groups.io/g/devel/message/110524
Mute This Topic: https://groups.io/mt/102202314/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 12628 bytes --]

  reply	other threads:[~2023-11-02 11:47 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-26 15:31 [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when converting ADR to ADRP Jake Garver via groups.io
2023-10-26 18:46 ` Pedro Falcato
2023-10-27 12:44   ` Jake Garver via groups.io
2023-10-27 13:46     ` Ard Biesheuvel
2023-10-27 14:09       ` Jake Garver via groups.io
2023-10-27 14:12         ` Pedro Falcato
2023-10-27 14:13         ` Ard Biesheuvel
2023-10-27 14:26           ` Pedro Falcato
2023-10-27 14:43             ` Ard Biesheuvel
2023-10-27 15:52               ` Jake Garver via groups.io
2023-11-02 11:47                 ` Jake Garver via groups.io [this message]
2023-11-02 12:47                   ` Pedro Falcato
2023-12-06 16:51                     ` Jake Garver via groups.io
2023-12-12  9:22                       ` Ard Biesheuvel
2023-12-13 14:57                         ` Jake Garver via groups.io
2023-12-13 17:31                           ` Ard Biesheuvel
2023-12-13 18:01                             ` Pedro Falcato
2023-12-13 19:47                               ` Jake Garver via groups.io
2023-12-19 23:29                                 ` Jake Garver via groups.io
2023-12-20  7:34                                   ` Ard Biesheuvel
2023-10-27 14:10       ` Pedro Falcato

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=PH0PR12MB7888E5A1D9416C56CE811598ADA6A@PH0PR12MB7888.namprd12.prod.outlook.com \
    --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