Re: [PATCH 3/9] efi/x86: Move efi stub globals from .bss to .data

[Ard Biesheuvel]

On Fri, 10 Apr 2020 at 20:01, Arvind Sankar <nivedita@xxxxxxxxxxxx> wrote:
>
> On Fri, Apr 10, 2020 at 06:03:38PM +0200, Ard Biesheuvel wrote:
> > On Fri, 10 Apr 2020 at 17:16, Arvind Sankar <nivedita@xxxxxxxxxxxx> wrote:
> > >
> > > On Fri, Apr 10, 2020 at 10:20:42AM +0200, Ard Biesheuvel wrote:
> > > > On Thu, 9 Apr 2020 at 23:08, Arvind Sankar <nivedita@xxxxxxxxxxxx> wrote:
> > > > >
> > > > > On Thu, Apr 09, 2020 at 04:53:07PM -0400, Brian Gerst wrote:
> > > > > > > Can we use the -fno-zero-initialized-in-bss compiler flag instead of
> > > > > > > explicitly marking global variables?
> > > > > >
> > > > > > Scratch that.  Apparently it only works when a variable is explicitly
> > > > > > initialized to zero.
> > > > > >
> > > > > > --
> > > > > > Brian Gerst
> > > > >
> > > > > Right, there doesn't seem to be a compiler option to turn off the use of
> > > > > .bss altogether.
> > > >
> > > > Yeah. I'll try to come up with a way to consolidate this a bit across
> > > > architectures (which is a bit easier now that all of the EFI stub C
> > > > code lives in the same place). It is probably easiest to use a section
> > > > renaming trick similar to the one I added for ARM (as Arvind suggested
> > > > as well, IIRC), and get rid of the per-symbol annotations altogether.
> > >
> > > Does that work for 32-bit ARM, or does it need to be .data to tell the
> > > compiler to avoid generating GOT references? If that's fine, we don't
> > > actually need to rename sections -- linker script magic is enough. For
> > > eg, the below pulls the EFI stub bss into .data for x86 without the need
> > > for the annotations.
> > >
> > > diff --git a/arch/x86/boot/compressed/vmlinux.lds.S b/arch/x86/boot/compressed/vmlinux.lds.S
> > > index 508cfa6828c5..e324819c95bc 100644
> > > --- a/arch/x86/boot/compressed/vmlinux.lds.S
> > > +++ b/arch/x86/boot/compressed/vmlinux.lds.S
> > > @@ -52,6 +52,7 @@ SECTIONS
> > >                 _data = . ;
> > >                 *(.data)
> > >                 *(.data.*)
> > > +               drivers/firmware/efi/libstub/lib.a:(.bss .bss.*)
> > >                 _edata = . ;
> > >         }
> > >         . = ALIGN(L1_CACHE_BYTES);
> >
> > No, we can add this to ARM as well, and get rid of the
> > __efistub_global annotations entirely.
>
> Cool.
>
> >
> > We'll still need .data.efistub for the .data pieces, but that is a
> > separate issue.
>
> You can avoid that by using an archive specification like above. i.e.
> adding
>         drivers/firmware/efi/libstub/lib.a:(.data .data.*)
> to the .init.data output section will pull in just the .data input
> sections from the EFI stub into the .init.data section.

Sure. But the ARM decompressor linker script currently discards .data
before this point in the linker script, and relies on this as a safety
net to ensure that no new .data items get added to the decompressor
binary (which runs after the stub)