Re: [edk2-devel] [edk2-staging/RISC-V-V2 PATCH v2 25/29] BaseTools/Scripts

["Leif Lindholm" <leif.lindholm@linaro.org>]

On Tue, Oct 15, 2019 at 06:31:06AM +0000, Chang, Abner (HPS SW/FW Technologist) wrote:
> > -----Original Message-----
> > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> > Leif Lindholm
> > Sent: Friday, September 27, 2019 4:51 AM
> > To: devel@edk2.groups.io; Chang, Abner (HPS SW/FW Technologist)
> > <abner.chang@hpe.com>
> > Subject: Re: [edk2-devel] [edk2-staging/RISC-V-V2 PATCH v2 25/29]
> > BaseTools/Scripts
> > 
> > On Mon, Sep 23, 2019 at 08:31:51AM +0800, Abner Chang wrote:
> > > Add RISC-V specific LD scripts. ."rela(INFO)" in the latest
> > > GccBase.lds causes PE32 relocation error.
> > > This is the temporaty solution untill we find the root casue.
> > 
> > I presume you mean that having a separate version for RISCV is temporary? I
> > am OK with this for now. However, the diff is not what I would expect. Can
> > you verify that the difference is only what you actually need?
> 
> Yes. Hope I can remove this change after I figuring out the toolchain issue.
> 
> > For example:
> > 
> > > Signed-off-by: Abner Chang <abner.chang@hpe.com>
> > > ---
> > >  BaseTools/Scripts/GccBaseRiscV.lds | 66
> > > ++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 66 insertions(+)
> > >  create mode 100644 BaseTools/Scripts/GccBaseRiscV.lds
> > >
> > > diff --git a/BaseTools/Scripts/GccBaseRiscV.lds
> > > b/BaseTools/Scripts/GccBaseRiscV.lds
> > > new file mode 100644
> > > index 0000000..7f9fae6
> > > --- /dev/null
> > > +++ b/BaseTools/Scripts/GccBaseRiscV.lds
> > > @@ -0,0 +1,66 @@
> > > +/** @file
> > > +
> > > +  Unified linker script for GCC based builds
> > > +
> > > +  Copyright (c) 2010 - 2015, Intel Corporation. All rights
> > > + reserved.<BR>  Copyright (c) 2015, Linaro Ltd. All rights
> > > + reserved.<BR>  Copyright (c) 2019, Hewlett Packard Enterprise
> > > + Development LP. All rights reserved.<BR>
> > > +
> > > +  SPDX-License-Identifier: BSD-2-Clause-Patent
> > > +
> > > +**/
> > > +
> > > +SECTIONS {
> > > +
> > > +  /*
> > > +   * The PE/COFF binary consists of DOS and PE/COFF headers, and a
> > sequence of
> > > +   * section headers adding up to PECOFF_HEADER_SIZE bytes (which
> > differs
> > > +   * between 32-bit and 64-bit builds). The actual start of the .text section
> > > +   * will be rounded up based on its actual alignment.
> > > +   */
> > > +  . = PECOFF_HEADER_SIZE;
> > > +
> > > +  .text : ALIGN(CONSTANT(COMMONPAGESIZE)) {
> > > +    *(.text .text.* .stub .gnu.linkonce.t.*)
> > > +    *(.rodata .rodata.* .gnu.linkonce.r.*)
> > > +    *(.got .got.*)
> > > +
> > > +    /*
> > > +     * The contents of AutoGen.c files are constant from the POV of the
> > program,
> > > +     * but most of its contents end up in .data or .bss by default since few of
> > > +     * the variable definitions that get emitted are declared as CONST.
> > > +     */
> > 
> > The above comment block looks completely different - in GccBase.lds:
> >      /*
> >       * The contents of AutoGen.c files are mostly constant from the POV of
> > the
> >       * program, but most of it ends up in .data or .bss by default since few of
> >       * the variable definitions that get emitted are declared as CONST.
> >       * Unfortunately, we cannot pull it into the .text section entirely, since
> >       * patchable PCDs are also emitted here, but we can at least move all of
> > the
> >       * emitted GUIDs here.
> >       */
> > 
> > > +    *:AutoGen.obj(.data.g*Guid)
> > > +  }
> > > +
> > > +  /*
> > > +   * 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.*)
> > > +  }
> > > +
> > > +  .eh_frame ALIGN(CONSTANT(COMMONPAGESIZE)) : {
> > > +    KEEP (*(.eh_frame))
> > > +  }
> > > +
> > > +  .rela ALIGN(CONSTANT(COMMONPAGESIZE)) : {
> > 
> > The above line is, I believe, the only necessary change for your workaround?
>
> Yes. only this line is necessary. I guess I copied entire lds file from my old edk2 tree.

Yeah, that makes sense.
If you can provide an updated version based on the current common
.lds, I'm OK for that to go into the -staging branch.

/
    Leif

> > 
> > > +    *(.rela .rela.*)
> > > +  }
> > > +
> > 
> > The original one as the following:
> >   .hii : ALIGN(CONSTANT(COMMONPAGESIZE)) {
> >     KEEP (*(.hii))
> >   }
> > 
> > I do not think including this could be the cause of any issues.
> > 
> > Also, GccBase.lds includes here:
> >   /*
> >    * Retain the GNU build id but in a non-allocatable section so GenFw
> >    * does not copy it into the PE/COFF image.
> >    */
> >   .build-id (INFO) : { *(.note.gnu.build-id) }
> > 
> > > +  /DISCARD/ : {
> > > +    *(.note.GNU-stack)
> > > +    *(.gnu_debuglink)
> > > +    *(.interp)
> > > +    *(.dynsym)
> > > +    *(.dynstr)
> > > +    *(.dynamic)
> > > +    *(.hash)
> > 
> > In GccBase.lds:
> >        *(.hash .gnu.hash)
> > 
> > /
> >     Leif
> > 
> > > +    *(.comment)
> > > +    *(COMMON)
> > > +  }
> > > +}
> > > --
> > > 2.7.4
> > >
> > >
> > >
> > >
> > 
> > 
>