diff mbox series

[edk2] BaseTools/tools_def AARCH64: move GCC49/GGC5 to 4 KB section alignment

Message ID 20181210141339.17108-1-ard.biesheuvel@linaro.org
State Accepted
Commit de3c440e8a54c201c527b85da7b89d58486ece4d
Headers show
Series [edk2] BaseTools/tools_def AARCH64: move GCC49/GGC5 to 4 KB section alignment | expand

Commit Message

Ard Biesheuvel Dec. 10, 2018, 2:13 p.m. UTC
Since 4 KB section alignment is required when mapping PE/COFF images
with strict permissions, update the default section alignment when
using GCC49 and GCC5 in RELEASE mode. Note that XIP modules such as
SEC, PEIMs or PEI core are not affected by this change, since the
override to 32 byte aligment remains in effect.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---
 BaseTools/Conf/tools_def.template | 24 +++++++++++---------
 1 file changed, 13 insertions(+), 11 deletions(-)

-- 
2.19.2

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Laszlo Ersek Dec. 10, 2018, 6:08 p.m. UTC | #1
On 12/10/18 15:13, Ard Biesheuvel wrote:
> Since 4 KB section alignment is required when mapping PE/COFF images

> with strict permissions, update the default section alignment when

> using GCC49 and GCC5 in RELEASE mode. Note that XIP modules such as

> SEC, PEIMs or PEI core are not affected by this change, since the

> override to 32 byte aligment remains in effect.

> 

> Contributed-under: TianoCore Contribution Agreement 1.1

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---

>  BaseTools/Conf/tools_def.template | 24 +++++++++++---------

>  1 file changed, 13 insertions(+), 11 deletions(-)

> 

> diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template

> index e0e68fd7fb49..5d34333dc54f 100755

> --- a/BaseTools/Conf/tools_def.template

> +++ b/BaseTools/Conf/tools_def.template

> @@ -4263,11 +4263,11 @@ DEFINE GCC49_ARM_ASM_FLAGS           = DEF(GCC48_ARM_ASM_FLAGS)

>  DEFINE GCC49_AARCH64_ASM_FLAGS       = DEF(GCC48_AARCH64_ASM_FLAGS)

>  DEFINE GCC49_ARM_CC_FLAGS            = DEF(GCC48_ARM_CC_FLAGS)

>  DEFINE GCC49_ARM_CC_XIPFLAGS         = DEF(GCC48_ARM_CC_XIPFLAGS)

> -DEFINE GCC49_AARCH64_CC_FLAGS        = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC44_ALL_CC_FLAGS) DEF(GCC_AARCH64_CC_FLAGS)

> +DEFINE GCC49_AARCH64_CC_FLAGS        = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC44_ALL_CC_FLAGS) DEF(GCC_AARCH64_CC_FLAGS) -mcmodel=small

>  DEFINE GCC49_AARCH64_CC_XIPFLAGS     = DEF(GCC48_AARCH64_CC_XIPFLAGS)

>  DEFINE GCC49_ARM_DLINK_FLAGS         = DEF(GCC48_ARM_DLINK_FLAGS)

>  DEFINE GCC49_ARM_DLINK2_FLAGS        = DEF(GCC48_ARM_DLINK2_FLAGS)

> -DEFINE GCC49_AARCH64_DLINK_FLAGS     = DEF(GCC48_AARCH64_DLINK_FLAGS)

> +DEFINE GCC49_AARCH64_DLINK_FLAGS     = DEF(GCC48_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000

>  DEFINE GCC49_AARCH64_DLINK2_FLAGS    = DEF(GCC48_AARCH64_DLINK2_FLAGS)

>  DEFINE GCC49_ARM_ASLDLINK_FLAGS      = DEF(GCC48_ARM_ASLDLINK_FLAGS)

>  DEFINE GCC49_AARCH64_ASLDLINK_FLAGS  = DEF(GCC48_AARCH64_ASLDLINK_FLAGS)

> @@ -5034,15 +5034,16 @@ RELEASE_GCC49_ARM_CC_FLAGS       = DEF(GCC49_ARM_CC_FLAGS) -Wno-unused-but-set-v

>  *_GCC49_AARCH64_VFRPP_FLAGS      = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC_VFRPP_FLAGS)

>  *_GCC49_AARCH64_CC_XIPFLAGS      = DEF(GCC49_AARCH64_CC_XIPFLAGS)

>  

> -  DEBUG_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -O0 -mcmodel=small

> -  DEBUG_GCC49_AARCH64_DLINK_FLAGS  = DEF(GCC49_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000

> +  DEBUG_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -O0

> +  DEBUG_GCC49_AARCH64_DLINK_FLAGS  = DEF(GCC49_AARCH64_DLINK_FLAGS)

>    DEBUG_GCC49_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20

>  

> -RELEASE_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -Wno-unused-but-set-variable -Wno-unused-const-variable -mcmodel=tiny

> +RELEASE_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -Wno-unused-but-set-variable -Wno-unused-const-variable

>  RELEASE_GCC49_AARCH64_DLINK_FLAGS  = DEF(GCC49_AARCH64_DLINK_FLAGS)

> +RELEASE_GCC49_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20

>  

> -  NOOPT_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -O0 -mcmodel=small

> -  NOOPT_GCC49_AARCH64_DLINK_FLAGS  = DEF(GCC49_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000 -O0

> +  NOOPT_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -O0

> +  NOOPT_GCC49_AARCH64_DLINK_FLAGS  = DEF(GCC49_AARCH64_DLINK_FLAGS) -O0

>    NOOPT_GCC49_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20 -O0

>  

>  ####################################################################################

> @@ -5189,14 +5190,15 @@ RELEASE_GCC5_ARM_DLINK_FLAGS     = DEF(GCC5_ARM_DLINK_FLAGS) -flto -Os -L$(WORKS

>  *_GCC5_AARCH64_VFRPP_FLAGS       = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC_VFRPP_FLAGS)

>  *_GCC5_AARCH64_CC_XIPFLAGS       = DEF(GCC5_AARCH64_CC_XIPFLAGS)

>  

> -  DEBUG_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -flto -Wno-unused-but-set-variable -Wno-unused-const-variable -mcmodel=small

> -  DEBUG_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000 -flto -Os -L$(WORKSPACE)/ArmPkg/Library/GccLto -llto-aarch64 -Wl,-plugin-opt=-pass-through=-llto-aarch64 -Wno-lto-type-mismatch -mcmodel=small

> +  DEBUG_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -flto -Wno-unused-but-set-variable -Wno-unused-const-variable

> +  DEBUG_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -flto -Os -L$(WORKSPACE)/ArmPkg/Library/GccLto -llto-aarch64 -Wl,-plugin-opt=-pass-through=-llto-aarch64 -Wno-lto-type-mismatch

>    DEBUG_GCC5_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20

>  

> -RELEASE_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -flto -Wno-unused-but-set-variable -Wno-unused-const-variable -mcmodel=tiny

> +RELEASE_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -flto -Wno-unused-but-set-variable -Wno-unused-const-variable

>  RELEASE_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -flto -Os -L$(WORKSPACE)/ArmPkg/Library/GccLto -llto-aarch64 -Wl,-plugin-opt=-pass-through=-llto-aarch64 -Wno-lto-type-mismatch

> +RELEASE_GCC5_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20

>  

> -  NOOPT_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -O0 -mcmodel=small

> +  NOOPT_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -O0

>    NOOPT_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000 -O0

>    NOOPT_GCC5_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20 -O0

>  

> 


Looking at this patch with "--word-diff" helps a little (it's too bad we
can't break these mile long option sequences to multiple lines). So:

* For GCC49:

- The compiler option "-mcmodel=small" is upstreamed from both
NOOPT_GCC49_AARCH64_CC_FLAGS and DEBUG_GCC49_AARCH64_CC_FLAGS to
GCC49_AARCH64_CC_FLAGS. This is a no-op for DEBUG and NOOPT, however the
option will now also apply to RELEASE.

- As a consequence, "-mcmodel=tiny" is not usable for RELEASE, and it is
removed.

- The same upstreaming occurs for "-z common-page-size=0x1000", in
GCC49_AARCH64_DLINK_FLAGS. RELEASE needs no fixup, it simply benefits
from the change. However, RELEASE_GCC49_AARCH64_DLINK_XIPFLAGS needs an
override, to stick with the original 0x20 alignment.

* For GCC5:

- GCC5_AARCH64_CC_FLAGS is defined as GCC49_AARCH64_CC_FLAGS, therefore
the "-mcmodel=small" inheritance from the above is automatic. Only the
DEBUG/NOOPT removals, and the RELEASE fixup are needed.

- GCC5_AARCH64_DLINK_FLAGS is defined as GCC49_AARCH64_DLINK_FLAGS,
therefore the same inheritance pattern applies to "-z
common-page-size=0x1000". I'm noticing an omission here however: "-z
common-page-size=0x1000" should have been removed from
NOOPT_GCC5_AARCH64_DLINK_FLAGS as well.

- The RELEASE_GCC5_AARCH64_DLINK_XIPFLAGS override is spelled out the same.


So... do you agree that NOOPT_GCC5_AARCH64_DLINK_FLAGS should have been
trimmed as well?

Furthermore, I suggest squeezing the word RELEASE into the subject,
given that the change is only observable in RELEASE builds.

Thanks!
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel Dec. 10, 2018, 6:12 p.m. UTC | #2
On Mon, 10 Dec 2018 at 19:08, Laszlo Ersek <lersek@redhat.com> wrote:
>

> On 12/10/18 15:13, Ard Biesheuvel wrote:

> > Since 4 KB section alignment is required when mapping PE/COFF images

> > with strict permissions, update the default section alignment when

> > using GCC49 and GCC5 in RELEASE mode. Note that XIP modules such as

> > SEC, PEIMs or PEI core are not affected by this change, since the

> > override to 32 byte aligment remains in effect.

> >

> > Contributed-under: TianoCore Contribution Agreement 1.1

> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> > ---

> >  BaseTools/Conf/tools_def.template | 24 +++++++++++---------

> >  1 file changed, 13 insertions(+), 11 deletions(-)

> >

> > diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template

> > index e0e68fd7fb49..5d34333dc54f 100755

> > --- a/BaseTools/Conf/tools_def.template

> > +++ b/BaseTools/Conf/tools_def.template

> > @@ -4263,11 +4263,11 @@ DEFINE GCC49_ARM_ASM_FLAGS           = DEF(GCC48_ARM_ASM_FLAGS)

> >  DEFINE GCC49_AARCH64_ASM_FLAGS       = DEF(GCC48_AARCH64_ASM_FLAGS)

> >  DEFINE GCC49_ARM_CC_FLAGS            = DEF(GCC48_ARM_CC_FLAGS)

> >  DEFINE GCC49_ARM_CC_XIPFLAGS         = DEF(GCC48_ARM_CC_XIPFLAGS)

> > -DEFINE GCC49_AARCH64_CC_FLAGS        = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC44_ALL_CC_FLAGS) DEF(GCC_AARCH64_CC_FLAGS)

> > +DEFINE GCC49_AARCH64_CC_FLAGS        = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC44_ALL_CC_FLAGS) DEF(GCC_AARCH64_CC_FLAGS) -mcmodel=small

> >  DEFINE GCC49_AARCH64_CC_XIPFLAGS     = DEF(GCC48_AARCH64_CC_XIPFLAGS)

> >  DEFINE GCC49_ARM_DLINK_FLAGS         = DEF(GCC48_ARM_DLINK_FLAGS)

> >  DEFINE GCC49_ARM_DLINK2_FLAGS        = DEF(GCC48_ARM_DLINK2_FLAGS)

> > -DEFINE GCC49_AARCH64_DLINK_FLAGS     = DEF(GCC48_AARCH64_DLINK_FLAGS)

> > +DEFINE GCC49_AARCH64_DLINK_FLAGS     = DEF(GCC48_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000

> >  DEFINE GCC49_AARCH64_DLINK2_FLAGS    = DEF(GCC48_AARCH64_DLINK2_FLAGS)

> >  DEFINE GCC49_ARM_ASLDLINK_FLAGS      = DEF(GCC48_ARM_ASLDLINK_FLAGS)

> >  DEFINE GCC49_AARCH64_ASLDLINK_FLAGS  = DEF(GCC48_AARCH64_ASLDLINK_FLAGS)

> > @@ -5034,15 +5034,16 @@ RELEASE_GCC49_ARM_CC_FLAGS       = DEF(GCC49_ARM_CC_FLAGS) -Wno-unused-but-set-v

> >  *_GCC49_AARCH64_VFRPP_FLAGS      = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC_VFRPP_FLAGS)

> >  *_GCC49_AARCH64_CC_XIPFLAGS      = DEF(GCC49_AARCH64_CC_XIPFLAGS)

> >

> > -  DEBUG_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -O0 -mcmodel=small

> > -  DEBUG_GCC49_AARCH64_DLINK_FLAGS  = DEF(GCC49_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000

> > +  DEBUG_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -O0

> > +  DEBUG_GCC49_AARCH64_DLINK_FLAGS  = DEF(GCC49_AARCH64_DLINK_FLAGS)

> >    DEBUG_GCC49_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20

> >

> > -RELEASE_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -Wno-unused-but-set-variable -Wno-unused-const-variable -mcmodel=tiny

> > +RELEASE_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -Wno-unused-but-set-variable -Wno-unused-const-variable

> >  RELEASE_GCC49_AARCH64_DLINK_FLAGS  = DEF(GCC49_AARCH64_DLINK_FLAGS)

> > +RELEASE_GCC49_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20

> >

> > -  NOOPT_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -O0 -mcmodel=small

> > -  NOOPT_GCC49_AARCH64_DLINK_FLAGS  = DEF(GCC49_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000 -O0

> > +  NOOPT_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -O0

> > +  NOOPT_GCC49_AARCH64_DLINK_FLAGS  = DEF(GCC49_AARCH64_DLINK_FLAGS) -O0

> >    NOOPT_GCC49_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20 -O0

> >

> >  ####################################################################################

> > @@ -5189,14 +5190,15 @@ RELEASE_GCC5_ARM_DLINK_FLAGS     = DEF(GCC5_ARM_DLINK_FLAGS) -flto -Os -L$(WORKS

> >  *_GCC5_AARCH64_VFRPP_FLAGS       = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC_VFRPP_FLAGS)

> >  *_GCC5_AARCH64_CC_XIPFLAGS       = DEF(GCC5_AARCH64_CC_XIPFLAGS)

> >

> > -  DEBUG_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -flto -Wno-unused-but-set-variable -Wno-unused-const-variable -mcmodel=small

> > -  DEBUG_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000 -flto -Os -L$(WORKSPACE)/ArmPkg/Library/GccLto -llto-aarch64 -Wl,-plugin-opt=-pass-through=-llto-aarch64 -Wno-lto-type-mismatch -mcmodel=small

> > +  DEBUG_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -flto -Wno-unused-but-set-variable -Wno-unused-const-variable

> > +  DEBUG_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -flto -Os -L$(WORKSPACE)/ArmPkg/Library/GccLto -llto-aarch64 -Wl,-plugin-opt=-pass-through=-llto-aarch64 -Wno-lto-type-mismatch

> >    DEBUG_GCC5_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20

> >

> > -RELEASE_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -flto -Wno-unused-but-set-variable -Wno-unused-const-variable -mcmodel=tiny

> > +RELEASE_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -flto -Wno-unused-but-set-variable -Wno-unused-const-variable

> >  RELEASE_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -flto -Os -L$(WORKSPACE)/ArmPkg/Library/GccLto -llto-aarch64 -Wl,-plugin-opt=-pass-through=-llto-aarch64 -Wno-lto-type-mismatch

> > +RELEASE_GCC5_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20

> >

> > -  NOOPT_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -O0 -mcmodel=small

> > +  NOOPT_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -O0

> >    NOOPT_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000 -O0

> >    NOOPT_GCC5_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20 -O0

> >

> >

>

> Looking at this patch with "--word-diff" helps a little (it's too bad we

> can't break these mile long option sequences to multiple lines). So:

>

> * For GCC49:

>

> - The compiler option "-mcmodel=small" is upstreamed from both

> NOOPT_GCC49_AARCH64_CC_FLAGS and DEBUG_GCC49_AARCH64_CC_FLAGS to

> GCC49_AARCH64_CC_FLAGS. This is a no-op for DEBUG and NOOPT, however the

> option will now also apply to RELEASE.

>

> - As a consequence, "-mcmodel=tiny" is not usable for RELEASE, and it is

> removed.

>

> - The same upstreaming occurs for "-z common-page-size=0x1000", in

> GCC49_AARCH64_DLINK_FLAGS. RELEASE needs no fixup, it simply benefits

> from the change. However, RELEASE_GCC49_AARCH64_DLINK_XIPFLAGS needs an

> override, to stick with the original 0x20 alignment.

>

> * For GCC5:

>

> - GCC5_AARCH64_CC_FLAGS is defined as GCC49_AARCH64_CC_FLAGS, therefore

> the "-mcmodel=small" inheritance from the above is automatic. Only the

> DEBUG/NOOPT removals, and the RELEASE fixup are needed.

>

> - GCC5_AARCH64_DLINK_FLAGS is defined as GCC49_AARCH64_DLINK_FLAGS,

> therefore the same inheritance pattern applies to "-z

> common-page-size=0x1000". I'm noticing an omission here however: "-z

> common-page-size=0x1000" should have been removed from

> NOOPT_GCC5_AARCH64_DLINK_FLAGS as well.

>

> - The RELEASE_GCC5_AARCH64_DLINK_XIPFLAGS override is spelled out the same.

>

>

> So... do you agree that NOOPT_GCC5_AARCH64_DLINK_FLAGS should have been

> trimmed as well?

>


Yes, well spotted.

> Furthermore, I suggest squeezing the word RELEASE into the subject,

> given that the change is only observable in RELEASE builds.

>


Sure, I'll add that as well.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Laszlo Ersek Dec. 10, 2018, 6:19 p.m. UTC | #3
On 12/10/18 19:12, Ard Biesheuvel wrote:
> On Mon, 10 Dec 2018 at 19:08, Laszlo Ersek <lersek@redhat.com> wrote:

>>

>> On 12/10/18 15:13, Ard Biesheuvel wrote:

>>> Since 4 KB section alignment is required when mapping PE/COFF images

>>> with strict permissions, update the default section alignment when

>>> using GCC49 and GCC5 in RELEASE mode. Note that XIP modules such as

>>> SEC, PEIMs or PEI core are not affected by this change, since the

>>> override to 32 byte aligment remains in effect.

>>>

>>> Contributed-under: TianoCore Contribution Agreement 1.1

>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

>>> ---

>>>  BaseTools/Conf/tools_def.template | 24 +++++++++++---------

>>>  1 file changed, 13 insertions(+), 11 deletions(-)

>>>

>>> diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template

>>> index e0e68fd7fb49..5d34333dc54f 100755

>>> --- a/BaseTools/Conf/tools_def.template

>>> +++ b/BaseTools/Conf/tools_def.template

>>> @@ -4263,11 +4263,11 @@ DEFINE GCC49_ARM_ASM_FLAGS           = DEF(GCC48_ARM_ASM_FLAGS)

>>>  DEFINE GCC49_AARCH64_ASM_FLAGS       = DEF(GCC48_AARCH64_ASM_FLAGS)

>>>  DEFINE GCC49_ARM_CC_FLAGS            = DEF(GCC48_ARM_CC_FLAGS)

>>>  DEFINE GCC49_ARM_CC_XIPFLAGS         = DEF(GCC48_ARM_CC_XIPFLAGS)

>>> -DEFINE GCC49_AARCH64_CC_FLAGS        = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC44_ALL_CC_FLAGS) DEF(GCC_AARCH64_CC_FLAGS)

>>> +DEFINE GCC49_AARCH64_CC_FLAGS        = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC44_ALL_CC_FLAGS) DEF(GCC_AARCH64_CC_FLAGS) -mcmodel=small

>>>  DEFINE GCC49_AARCH64_CC_XIPFLAGS     = DEF(GCC48_AARCH64_CC_XIPFLAGS)

>>>  DEFINE GCC49_ARM_DLINK_FLAGS         = DEF(GCC48_ARM_DLINK_FLAGS)

>>>  DEFINE GCC49_ARM_DLINK2_FLAGS        = DEF(GCC48_ARM_DLINK2_FLAGS)

>>> -DEFINE GCC49_AARCH64_DLINK_FLAGS     = DEF(GCC48_AARCH64_DLINK_FLAGS)

>>> +DEFINE GCC49_AARCH64_DLINK_FLAGS     = DEF(GCC48_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000

>>>  DEFINE GCC49_AARCH64_DLINK2_FLAGS    = DEF(GCC48_AARCH64_DLINK2_FLAGS)

>>>  DEFINE GCC49_ARM_ASLDLINK_FLAGS      = DEF(GCC48_ARM_ASLDLINK_FLAGS)

>>>  DEFINE GCC49_AARCH64_ASLDLINK_FLAGS  = DEF(GCC48_AARCH64_ASLDLINK_FLAGS)

>>> @@ -5034,15 +5034,16 @@ RELEASE_GCC49_ARM_CC_FLAGS       = DEF(GCC49_ARM_CC_FLAGS) -Wno-unused-but-set-v

>>>  *_GCC49_AARCH64_VFRPP_FLAGS      = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC_VFRPP_FLAGS)

>>>  *_GCC49_AARCH64_CC_XIPFLAGS      = DEF(GCC49_AARCH64_CC_XIPFLAGS)

>>>

>>> -  DEBUG_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -O0 -mcmodel=small

>>> -  DEBUG_GCC49_AARCH64_DLINK_FLAGS  = DEF(GCC49_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000

>>> +  DEBUG_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -O0

>>> +  DEBUG_GCC49_AARCH64_DLINK_FLAGS  = DEF(GCC49_AARCH64_DLINK_FLAGS)

>>>    DEBUG_GCC49_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20

>>>

>>> -RELEASE_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -Wno-unused-but-set-variable -Wno-unused-const-variable -mcmodel=tiny

>>> +RELEASE_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -Wno-unused-but-set-variable -Wno-unused-const-variable

>>>  RELEASE_GCC49_AARCH64_DLINK_FLAGS  = DEF(GCC49_AARCH64_DLINK_FLAGS)

>>> +RELEASE_GCC49_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20

>>>

>>> -  NOOPT_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -O0 -mcmodel=small

>>> -  NOOPT_GCC49_AARCH64_DLINK_FLAGS  = DEF(GCC49_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000 -O0

>>> +  NOOPT_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -O0

>>> +  NOOPT_GCC49_AARCH64_DLINK_FLAGS  = DEF(GCC49_AARCH64_DLINK_FLAGS) -O0

>>>    NOOPT_GCC49_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20 -O0

>>>

>>>  ####################################################################################

>>> @@ -5189,14 +5190,15 @@ RELEASE_GCC5_ARM_DLINK_FLAGS     = DEF(GCC5_ARM_DLINK_FLAGS) -flto -Os -L$(WORKS

>>>  *_GCC5_AARCH64_VFRPP_FLAGS       = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC_VFRPP_FLAGS)

>>>  *_GCC5_AARCH64_CC_XIPFLAGS       = DEF(GCC5_AARCH64_CC_XIPFLAGS)

>>>

>>> -  DEBUG_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -flto -Wno-unused-but-set-variable -Wno-unused-const-variable -mcmodel=small

>>> -  DEBUG_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000 -flto -Os -L$(WORKSPACE)/ArmPkg/Library/GccLto -llto-aarch64 -Wl,-plugin-opt=-pass-through=-llto-aarch64 -Wno-lto-type-mismatch -mcmodel=small

>>> +  DEBUG_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -flto -Wno-unused-but-set-variable -Wno-unused-const-variable

>>> +  DEBUG_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -flto -Os -L$(WORKSPACE)/ArmPkg/Library/GccLto -llto-aarch64 -Wl,-plugin-opt=-pass-through=-llto-aarch64 -Wno-lto-type-mismatch

>>>    DEBUG_GCC5_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20

>>>

>>> -RELEASE_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -flto -Wno-unused-but-set-variable -Wno-unused-const-variable -mcmodel=tiny

>>> +RELEASE_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -flto -Wno-unused-but-set-variable -Wno-unused-const-variable

>>>  RELEASE_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -flto -Os -L$(WORKSPACE)/ArmPkg/Library/GccLto -llto-aarch64 -Wl,-plugin-opt=-pass-through=-llto-aarch64 -Wno-lto-type-mismatch

>>> +RELEASE_GCC5_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20

>>>

>>> -  NOOPT_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -O0 -mcmodel=small

>>> +  NOOPT_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -O0

>>>    NOOPT_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000 -O0

>>>    NOOPT_GCC5_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20 -O0

>>>

>>>

>>

>> Looking at this patch with "--word-diff" helps a little (it's too bad we

>> can't break these mile long option sequences to multiple lines). So:

>>

>> * For GCC49:

>>

>> - The compiler option "-mcmodel=small" is upstreamed from both

>> NOOPT_GCC49_AARCH64_CC_FLAGS and DEBUG_GCC49_AARCH64_CC_FLAGS to

>> GCC49_AARCH64_CC_FLAGS. This is a no-op for DEBUG and NOOPT, however the

>> option will now also apply to RELEASE.

>>

>> - As a consequence, "-mcmodel=tiny" is not usable for RELEASE, and it is

>> removed.

>>

>> - The same upstreaming occurs for "-z common-page-size=0x1000", in

>> GCC49_AARCH64_DLINK_FLAGS. RELEASE needs no fixup, it simply benefits

>> from the change. However, RELEASE_GCC49_AARCH64_DLINK_XIPFLAGS needs an

>> override, to stick with the original 0x20 alignment.

>>

>> * For GCC5:

>>

>> - GCC5_AARCH64_CC_FLAGS is defined as GCC49_AARCH64_CC_FLAGS, therefore

>> the "-mcmodel=small" inheritance from the above is automatic. Only the

>> DEBUG/NOOPT removals, and the RELEASE fixup are needed.

>>

>> - GCC5_AARCH64_DLINK_FLAGS is defined as GCC49_AARCH64_DLINK_FLAGS,

>> therefore the same inheritance pattern applies to "-z

>> common-page-size=0x1000". I'm noticing an omission here however: "-z

>> common-page-size=0x1000" should have been removed from

>> NOOPT_GCC5_AARCH64_DLINK_FLAGS as well.

>>

>> - The RELEASE_GCC5_AARCH64_DLINK_XIPFLAGS override is spelled out the same.

>>

>>

>> So... do you agree that NOOPT_GCC5_AARCH64_DLINK_FLAGS should have been

>> trimmed as well?

>>

> 

> Yes, well spotted.

> 

>> Furthermore, I suggest squeezing the word RELEASE into the subject,

>> given that the change is only observable in RELEASE builds.

>>

> 

> Sure, I'll add that as well.

> 


Thanks! Personally I don't need to see a repost to believe you. :)

With those updates:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>


Thanks!
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Leif Lindholm Dec. 10, 2018, 7:06 p.m. UTC | #4
On Mon, Dec 10, 2018 at 03:13:39PM +0100, Ard Biesheuvel wrote:
> Since 4 KB section alignment is required when mapping PE/COFF images

> with strict permissions, update the default section alignment when

> using GCC49 and GCC5 in RELEASE mode. Note that XIP modules such as

> SEC, PEIMs or PEI core are not affected by this change, since the

> override to 32 byte aligment remains in effect.

> 

> Contributed-under: TianoCore Contribution Agreement 1.1

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>


We have at least one platform in the wild that refuses to load
applications that don't have a 4KB aligned codeoffset.

From experimentation, Visual Studio builds already enforce this on
AArch64.

I guess my only question/comment would be if GCC48 is left out
intentionally?

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

Tested-by: Leif Lindholm <leif.lindholm@linaro.org>


> ---

>  BaseTools/Conf/tools_def.template | 24 +++++++++++---------

>  1 file changed, 13 insertions(+), 11 deletions(-)

> 

> diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template

> index e0e68fd7fb49..5d34333dc54f 100755

> --- a/BaseTools/Conf/tools_def.template

> +++ b/BaseTools/Conf/tools_def.template

> @@ -4263,11 +4263,11 @@ DEFINE GCC49_ARM_ASM_FLAGS           = DEF(GCC48_ARM_ASM_FLAGS)

>  DEFINE GCC49_AARCH64_ASM_FLAGS       = DEF(GCC48_AARCH64_ASM_FLAGS)

>  DEFINE GCC49_ARM_CC_FLAGS            = DEF(GCC48_ARM_CC_FLAGS)

>  DEFINE GCC49_ARM_CC_XIPFLAGS         = DEF(GCC48_ARM_CC_XIPFLAGS)

> -DEFINE GCC49_AARCH64_CC_FLAGS        = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC44_ALL_CC_FLAGS) DEF(GCC_AARCH64_CC_FLAGS)

> +DEFINE GCC49_AARCH64_CC_FLAGS        = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC44_ALL_CC_FLAGS) DEF(GCC_AARCH64_CC_FLAGS) -mcmodel=small

>  DEFINE GCC49_AARCH64_CC_XIPFLAGS     = DEF(GCC48_AARCH64_CC_XIPFLAGS)

>  DEFINE GCC49_ARM_DLINK_FLAGS         = DEF(GCC48_ARM_DLINK_FLAGS)

>  DEFINE GCC49_ARM_DLINK2_FLAGS        = DEF(GCC48_ARM_DLINK2_FLAGS)

> -DEFINE GCC49_AARCH64_DLINK_FLAGS     = DEF(GCC48_AARCH64_DLINK_FLAGS)

> +DEFINE GCC49_AARCH64_DLINK_FLAGS     = DEF(GCC48_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000

>  DEFINE GCC49_AARCH64_DLINK2_FLAGS    = DEF(GCC48_AARCH64_DLINK2_FLAGS)

>  DEFINE GCC49_ARM_ASLDLINK_FLAGS      = DEF(GCC48_ARM_ASLDLINK_FLAGS)

>  DEFINE GCC49_AARCH64_ASLDLINK_FLAGS  = DEF(GCC48_AARCH64_ASLDLINK_FLAGS)

> @@ -5034,15 +5034,16 @@ RELEASE_GCC49_ARM_CC_FLAGS       = DEF(GCC49_ARM_CC_FLAGS) -Wno-unused-but-set-v

>  *_GCC49_AARCH64_VFRPP_FLAGS      = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC_VFRPP_FLAGS)

>  *_GCC49_AARCH64_CC_XIPFLAGS      = DEF(GCC49_AARCH64_CC_XIPFLAGS)

>  

> -  DEBUG_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -O0 -mcmodel=small

> -  DEBUG_GCC49_AARCH64_DLINK_FLAGS  = DEF(GCC49_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000

> +  DEBUG_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -O0

> +  DEBUG_GCC49_AARCH64_DLINK_FLAGS  = DEF(GCC49_AARCH64_DLINK_FLAGS)

>    DEBUG_GCC49_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20

>  

> -RELEASE_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -Wno-unused-but-set-variable -Wno-unused-const-variable -mcmodel=tiny

> +RELEASE_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -Wno-unused-but-set-variable -Wno-unused-const-variable

>  RELEASE_GCC49_AARCH64_DLINK_FLAGS  = DEF(GCC49_AARCH64_DLINK_FLAGS)

> +RELEASE_GCC49_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20

>  

> -  NOOPT_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -O0 -mcmodel=small

> -  NOOPT_GCC49_AARCH64_DLINK_FLAGS  = DEF(GCC49_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000 -O0

> +  NOOPT_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -O0

> +  NOOPT_GCC49_AARCH64_DLINK_FLAGS  = DEF(GCC49_AARCH64_DLINK_FLAGS) -O0

>    NOOPT_GCC49_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20 -O0

>  

>  ####################################################################################

> @@ -5189,14 +5190,15 @@ RELEASE_GCC5_ARM_DLINK_FLAGS     = DEF(GCC5_ARM_DLINK_FLAGS) -flto -Os -L$(WORKS

>  *_GCC5_AARCH64_VFRPP_FLAGS       = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC_VFRPP_FLAGS)

>  *_GCC5_AARCH64_CC_XIPFLAGS       = DEF(GCC5_AARCH64_CC_XIPFLAGS)

>  

> -  DEBUG_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -flto -Wno-unused-but-set-variable -Wno-unused-const-variable -mcmodel=small

> -  DEBUG_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000 -flto -Os -L$(WORKSPACE)/ArmPkg/Library/GccLto -llto-aarch64 -Wl,-plugin-opt=-pass-through=-llto-aarch64 -Wno-lto-type-mismatch -mcmodel=small

> +  DEBUG_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -flto -Wno-unused-but-set-variable -Wno-unused-const-variable

> +  DEBUG_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -flto -Os -L$(WORKSPACE)/ArmPkg/Library/GccLto -llto-aarch64 -Wl,-plugin-opt=-pass-through=-llto-aarch64 -Wno-lto-type-mismatch

>    DEBUG_GCC5_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20

>  

> -RELEASE_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -flto -Wno-unused-but-set-variable -Wno-unused-const-variable -mcmodel=tiny

> +RELEASE_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -flto -Wno-unused-but-set-variable -Wno-unused-const-variable

>  RELEASE_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -flto -Os -L$(WORKSPACE)/ArmPkg/Library/GccLto -llto-aarch64 -Wl,-plugin-opt=-pass-through=-llto-aarch64 -Wno-lto-type-mismatch

> +RELEASE_GCC5_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20

>  

> -  NOOPT_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -O0 -mcmodel=small

> +  NOOPT_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -O0

>    NOOPT_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000 -O0

>    NOOPT_GCC5_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20 -O0

>  

> -- 

> 2.19.2

> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Gao, Liming Dec. 11, 2018, 1:34 p.m. UTC | #5
I have no comments for this change. Reviewed-by: Liming Gao <liming.gao@intel.com>

> -----Original Message-----

> From: Laszlo Ersek [mailto:lersek@redhat.com]

> Sent: Tuesday, December 11, 2018 2:19 AM

> To: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> Cc: edk2-devel@lists.01.org; Gao, Liming <liming.gao@intel.com>

> Subject: Re: [edk2] [PATCH] BaseTools/tools_def AARCH64: move GCC49/GGC5 to 4 KB section alignment

> 

> On 12/10/18 19:12, Ard Biesheuvel wrote:

> > On Mon, 10 Dec 2018 at 19:08, Laszlo Ersek <lersek@redhat.com> wrote:

> >>

> >> On 12/10/18 15:13, Ard Biesheuvel wrote:

> >>> Since 4 KB section alignment is required when mapping PE/COFF images

> >>> with strict permissions, update the default section alignment when

> >>> using GCC49 and GCC5 in RELEASE mode. Note that XIP modules such as

> >>> SEC, PEIMs or PEI core are not affected by this change, since the

> >>> override to 32 byte aligment remains in effect.

> >>>

> >>> Contributed-under: TianoCore Contribution Agreement 1.1

> >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> >>> ---

> >>>  BaseTools/Conf/tools_def.template | 24 +++++++++++---------

> >>>  1 file changed, 13 insertions(+), 11 deletions(-)

> >>>

> >>> diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template

> >>> index e0e68fd7fb49..5d34333dc54f 100755

> >>> --- a/BaseTools/Conf/tools_def.template

> >>> +++ b/BaseTools/Conf/tools_def.template

> >>> @@ -4263,11 +4263,11 @@ DEFINE GCC49_ARM_ASM_FLAGS           = DEF(GCC48_ARM_ASM_FLAGS)

> >>>  DEFINE GCC49_AARCH64_ASM_FLAGS       = DEF(GCC48_AARCH64_ASM_FLAGS)

> >>>  DEFINE GCC49_ARM_CC_FLAGS            = DEF(GCC48_ARM_CC_FLAGS)

> >>>  DEFINE GCC49_ARM_CC_XIPFLAGS         = DEF(GCC48_ARM_CC_XIPFLAGS)

> >>> -DEFINE GCC49_AARCH64_CC_FLAGS        = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC44_ALL_CC_FLAGS)

> DEF(GCC_AARCH64_CC_FLAGS)

> >>> +DEFINE GCC49_AARCH64_CC_FLAGS        = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC44_ALL_CC_FLAGS)

> DEF(GCC_AARCH64_CC_FLAGS) -mcmodel=small

> >>>  DEFINE GCC49_AARCH64_CC_XIPFLAGS     = DEF(GCC48_AARCH64_CC_XIPFLAGS)

> >>>  DEFINE GCC49_ARM_DLINK_FLAGS         = DEF(GCC48_ARM_DLINK_FLAGS)

> >>>  DEFINE GCC49_ARM_DLINK2_FLAGS        = DEF(GCC48_ARM_DLINK2_FLAGS)

> >>> -DEFINE GCC49_AARCH64_DLINK_FLAGS     = DEF(GCC48_AARCH64_DLINK_FLAGS)

> >>> +DEFINE GCC49_AARCH64_DLINK_FLAGS     = DEF(GCC48_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000

> >>>  DEFINE GCC49_AARCH64_DLINK2_FLAGS    = DEF(GCC48_AARCH64_DLINK2_FLAGS)

> >>>  DEFINE GCC49_ARM_ASLDLINK_FLAGS      = DEF(GCC48_ARM_ASLDLINK_FLAGS)

> >>>  DEFINE GCC49_AARCH64_ASLDLINK_FLAGS  = DEF(GCC48_AARCH64_ASLDLINK_FLAGS)

> >>> @@ -5034,15 +5034,16 @@ RELEASE_GCC49_ARM_CC_FLAGS       = DEF(GCC49_ARM_CC_FLAGS) -Wno-unused-but-set-v

> >>>  *_GCC49_AARCH64_VFRPP_FLAGS      = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC_VFRPP_FLAGS)

> >>>  *_GCC49_AARCH64_CC_XIPFLAGS      = DEF(GCC49_AARCH64_CC_XIPFLAGS)

> >>>

> >>> -  DEBUG_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -O0 -mcmodel=small

> >>> -  DEBUG_GCC49_AARCH64_DLINK_FLAGS  = DEF(GCC49_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000

> >>> +  DEBUG_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -O0

> >>> +  DEBUG_GCC49_AARCH64_DLINK_FLAGS  = DEF(GCC49_AARCH64_DLINK_FLAGS)

> >>>    DEBUG_GCC49_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20

> >>>

> >>> -RELEASE_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -Wno-unused-but-set-variable

> -Wno-unused-const-variable -mcmodel=tiny

> >>> +RELEASE_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -Wno-unused-but-set-variable

> -Wno-unused-const-variable

> >>>  RELEASE_GCC49_AARCH64_DLINK_FLAGS  = DEF(GCC49_AARCH64_DLINK_FLAGS)

> >>> +RELEASE_GCC49_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20

> >>>

> >>> -  NOOPT_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -O0 -mcmodel=small

> >>> -  NOOPT_GCC49_AARCH64_DLINK_FLAGS  = DEF(GCC49_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000 -O0

> >>> +  NOOPT_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -O0

> >>> +  NOOPT_GCC49_AARCH64_DLINK_FLAGS  = DEF(GCC49_AARCH64_DLINK_FLAGS) -O0

> >>>    NOOPT_GCC49_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20 -O0

> >>>

> >>>  ####################################################################################

> >>> @@ -5189,14 +5190,15 @@ RELEASE_GCC5_ARM_DLINK_FLAGS     = DEF(GCC5_ARM_DLINK_FLAGS) -flto -Os -L$(WORKS

> >>>  *_GCC5_AARCH64_VFRPP_FLAGS       = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC_VFRPP_FLAGS)

> >>>  *_GCC5_AARCH64_CC_XIPFLAGS       = DEF(GCC5_AARCH64_CC_XIPFLAGS)

> >>>

> >>> -  DEBUG_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -flto -Wno-unused-but-set-variable

> -Wno-unused-const-variable -mcmodel=small

> >>> -  DEBUG_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000 -flto -Os

> -L$(WORKSPACE)/ArmPkg/Library/GccLto -llto-aarch64 -Wl,-plugin-opt=-pass-through=-llto-aarch64 -Wno-lto-type-mismatch

> -mcmodel=small

> >>> +  DEBUG_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -flto -Wno-unused-but-set-variable

> -Wno-unused-const-variable

> >>> +  DEBUG_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -flto -Os

> -L$(WORKSPACE)/ArmPkg/Library/GccLto -llto-aarch64 -Wl,-plugin-opt=-pass-through=-llto-aarch64 -Wno-lto-type-mismatch

> >>>    DEBUG_GCC5_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20

> >>>

> >>> -RELEASE_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -flto -Wno-unused-but-set-variable

> -Wno-unused-const-variable -mcmodel=tiny

> >>> +RELEASE_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -flto -Wno-unused-but-set-variable

> -Wno-unused-const-variable

> >>>  RELEASE_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -flto -Os

> -L$(WORKSPACE)/ArmPkg/Library/GccLto -llto-aarch64 -Wl,-plugin-opt=-pass-through=-llto-aarch64 -Wno-lto-type-mismatch

> >>> +RELEASE_GCC5_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20

> >>>

> >>> -  NOOPT_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -O0 -mcmodel=small

> >>> +  NOOPT_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -O0

> >>>    NOOPT_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000 -O0

> >>>    NOOPT_GCC5_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20 -O0

> >>>

> >>>

> >>

> >> Looking at this patch with "--word-diff" helps a little (it's too bad we

> >> can't break these mile long option sequences to multiple lines). So:

> >>

> >> * For GCC49:

> >>

> >> - The compiler option "-mcmodel=small" is upstreamed from both

> >> NOOPT_GCC49_AARCH64_CC_FLAGS and DEBUG_GCC49_AARCH64_CC_FLAGS to

> >> GCC49_AARCH64_CC_FLAGS. This is a no-op for DEBUG and NOOPT, however the

> >> option will now also apply to RELEASE.

> >>

> >> - As a consequence, "-mcmodel=tiny" is not usable for RELEASE, and it is

> >> removed.

> >>

> >> - The same upstreaming occurs for "-z common-page-size=0x1000", in

> >> GCC49_AARCH64_DLINK_FLAGS. RELEASE needs no fixup, it simply benefits

> >> from the change. However, RELEASE_GCC49_AARCH64_DLINK_XIPFLAGS needs an

> >> override, to stick with the original 0x20 alignment.

> >>

> >> * For GCC5:

> >>

> >> - GCC5_AARCH64_CC_FLAGS is defined as GCC49_AARCH64_CC_FLAGS, therefore

> >> the "-mcmodel=small" inheritance from the above is automatic. Only the

> >> DEBUG/NOOPT removals, and the RELEASE fixup are needed.

> >>

> >> - GCC5_AARCH64_DLINK_FLAGS is defined as GCC49_AARCH64_DLINK_FLAGS,

> >> therefore the same inheritance pattern applies to "-z

> >> common-page-size=0x1000". I'm noticing an omission here however: "-z

> >> common-page-size=0x1000" should have been removed from

> >> NOOPT_GCC5_AARCH64_DLINK_FLAGS as well.

> >>

> >> - The RELEASE_GCC5_AARCH64_DLINK_XIPFLAGS override is spelled out the same.

> >>

> >>

> >> So... do you agree that NOOPT_GCC5_AARCH64_DLINK_FLAGS should have been

> >> trimmed as well?

> >>

> >

> > Yes, well spotted.

> >

> >> Furthermore, I suggest squeezing the word RELEASE into the subject,

> >> given that the change is only observable in RELEASE builds.

> >>

> >

> > Sure, I'll add that as well.

> >

> 

> Thanks! Personally I don't need to see a repost to believe you. :)

> 

> With those updates:

> 

> Reviewed-by: Laszlo Ersek <lersek@redhat.com>

> 

> Thanks!

> Laszlo

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel Dec. 11, 2018, 1:53 p.m. UTC | #6
On Tue, 11 Dec 2018 at 14:34, Gao, Liming <liming.gao@intel.com> wrote:
>

> I have no comments for this change. Reviewed-by: Liming Gao <liming.gao@intel.com>

>


Thanks all

Pushed as 765fb87c2b70..de3c440e8a54

> > -----Original Message-----

> > From: Laszlo Ersek [mailto:lersek@redhat.com]

> > Sent: Tuesday, December 11, 2018 2:19 AM

> > To: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> > Cc: edk2-devel@lists.01.org; Gao, Liming <liming.gao@intel.com>

> > Subject: Re: [edk2] [PATCH] BaseTools/tools_def AARCH64: move GCC49/GGC5 to 4 KB section alignment

> >

> > On 12/10/18 19:12, Ard Biesheuvel wrote:

> > > On Mon, 10 Dec 2018 at 19:08, Laszlo Ersek <lersek@redhat.com> wrote:

> > >>

> > >> On 12/10/18 15:13, Ard Biesheuvel wrote:

> > >>> Since 4 KB section alignment is required when mapping PE/COFF images

> > >>> with strict permissions, update the default section alignment when

> > >>> using GCC49 and GCC5 in RELEASE mode. Note that XIP modules such as

> > >>> SEC, PEIMs or PEI core are not affected by this change, since the

> > >>> override to 32 byte aligment remains in effect.

> > >>>

> > >>> Contributed-under: TianoCore Contribution Agreement 1.1

> > >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> > >>> ---

> > >>>  BaseTools/Conf/tools_def.template | 24 +++++++++++---------

> > >>>  1 file changed, 13 insertions(+), 11 deletions(-)

> > >>>

> > >>> diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template

> > >>> index e0e68fd7fb49..5d34333dc54f 100755

> > >>> --- a/BaseTools/Conf/tools_def.template

> > >>> +++ b/BaseTools/Conf/tools_def.template

> > >>> @@ -4263,11 +4263,11 @@ DEFINE GCC49_ARM_ASM_FLAGS           = DEF(GCC48_ARM_ASM_FLAGS)

> > >>>  DEFINE GCC49_AARCH64_ASM_FLAGS       = DEF(GCC48_AARCH64_ASM_FLAGS)

> > >>>  DEFINE GCC49_ARM_CC_FLAGS            = DEF(GCC48_ARM_CC_FLAGS)

> > >>>  DEFINE GCC49_ARM_CC_XIPFLAGS         = DEF(GCC48_ARM_CC_XIPFLAGS)

> > >>> -DEFINE GCC49_AARCH64_CC_FLAGS        = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC44_ALL_CC_FLAGS)

> > DEF(GCC_AARCH64_CC_FLAGS)

> > >>> +DEFINE GCC49_AARCH64_CC_FLAGS        = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC44_ALL_CC_FLAGS)

> > DEF(GCC_AARCH64_CC_FLAGS) -mcmodel=small

> > >>>  DEFINE GCC49_AARCH64_CC_XIPFLAGS     = DEF(GCC48_AARCH64_CC_XIPFLAGS)

> > >>>  DEFINE GCC49_ARM_DLINK_FLAGS         = DEF(GCC48_ARM_DLINK_FLAGS)

> > >>>  DEFINE GCC49_ARM_DLINK2_FLAGS        = DEF(GCC48_ARM_DLINK2_FLAGS)

> > >>> -DEFINE GCC49_AARCH64_DLINK_FLAGS     = DEF(GCC48_AARCH64_DLINK_FLAGS)

> > >>> +DEFINE GCC49_AARCH64_DLINK_FLAGS     = DEF(GCC48_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000

> > >>>  DEFINE GCC49_AARCH64_DLINK2_FLAGS    = DEF(GCC48_AARCH64_DLINK2_FLAGS)

> > >>>  DEFINE GCC49_ARM_ASLDLINK_FLAGS      = DEF(GCC48_ARM_ASLDLINK_FLAGS)

> > >>>  DEFINE GCC49_AARCH64_ASLDLINK_FLAGS  = DEF(GCC48_AARCH64_ASLDLINK_FLAGS)

> > >>> @@ -5034,15 +5034,16 @@ RELEASE_GCC49_ARM_CC_FLAGS       = DEF(GCC49_ARM_CC_FLAGS) -Wno-unused-but-set-v

> > >>>  *_GCC49_AARCH64_VFRPP_FLAGS      = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC_VFRPP_FLAGS)

> > >>>  *_GCC49_AARCH64_CC_XIPFLAGS      = DEF(GCC49_AARCH64_CC_XIPFLAGS)

> > >>>

> > >>> -  DEBUG_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -O0 -mcmodel=small

> > >>> -  DEBUG_GCC49_AARCH64_DLINK_FLAGS  = DEF(GCC49_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000

> > >>> +  DEBUG_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -O0

> > >>> +  DEBUG_GCC49_AARCH64_DLINK_FLAGS  = DEF(GCC49_AARCH64_DLINK_FLAGS)

> > >>>    DEBUG_GCC49_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20

> > >>>

> > >>> -RELEASE_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -Wno-unused-but-set-variable

> > -Wno-unused-const-variable -mcmodel=tiny

> > >>> +RELEASE_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -Wno-unused-but-set-variable

> > -Wno-unused-const-variable

> > >>>  RELEASE_GCC49_AARCH64_DLINK_FLAGS  = DEF(GCC49_AARCH64_DLINK_FLAGS)

> > >>> +RELEASE_GCC49_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20

> > >>>

> > >>> -  NOOPT_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -O0 -mcmodel=small

> > >>> -  NOOPT_GCC49_AARCH64_DLINK_FLAGS  = DEF(GCC49_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000 -O0

> > >>> +  NOOPT_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -O0

> > >>> +  NOOPT_GCC49_AARCH64_DLINK_FLAGS  = DEF(GCC49_AARCH64_DLINK_FLAGS) -O0

> > >>>    NOOPT_GCC49_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20 -O0

> > >>>

> > >>>  ####################################################################################

> > >>> @@ -5189,14 +5190,15 @@ RELEASE_GCC5_ARM_DLINK_FLAGS     = DEF(GCC5_ARM_DLINK_FLAGS) -flto -Os -L$(WORKS

> > >>>  *_GCC5_AARCH64_VFRPP_FLAGS       = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC_VFRPP_FLAGS)

> > >>>  *_GCC5_AARCH64_CC_XIPFLAGS       = DEF(GCC5_AARCH64_CC_XIPFLAGS)

> > >>>

> > >>> -  DEBUG_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -flto -Wno-unused-but-set-variable

> > -Wno-unused-const-variable -mcmodel=small

> > >>> -  DEBUG_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000 -flto -Os

> > -L$(WORKSPACE)/ArmPkg/Library/GccLto -llto-aarch64 -Wl,-plugin-opt=-pass-through=-llto-aarch64 -Wno-lto-type-mismatch

> > -mcmodel=small

> > >>> +  DEBUG_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -flto -Wno-unused-but-set-variable

> > -Wno-unused-const-variable

> > >>> +  DEBUG_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -flto -Os

> > -L$(WORKSPACE)/ArmPkg/Library/GccLto -llto-aarch64 -Wl,-plugin-opt=-pass-through=-llto-aarch64 -Wno-lto-type-mismatch

> > >>>    DEBUG_GCC5_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20

> > >>>

> > >>> -RELEASE_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -flto -Wno-unused-but-set-variable

> > -Wno-unused-const-variable -mcmodel=tiny

> > >>> +RELEASE_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -flto -Wno-unused-but-set-variable

> > -Wno-unused-const-variable

> > >>>  RELEASE_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -flto -Os

> > -L$(WORKSPACE)/ArmPkg/Library/GccLto -llto-aarch64 -Wl,-plugin-opt=-pass-through=-llto-aarch64 -Wno-lto-type-mismatch

> > >>> +RELEASE_GCC5_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20

> > >>>

> > >>> -  NOOPT_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -O0 -mcmodel=small

> > >>> +  NOOPT_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -O0

> > >>>    NOOPT_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000 -O0

> > >>>    NOOPT_GCC5_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20 -O0

> > >>>

> > >>>

> > >>

> > >> Looking at this patch with "--word-diff" helps a little (it's too bad we

> > >> can't break these mile long option sequences to multiple lines). So:

> > >>

> > >> * For GCC49:

> > >>

> > >> - The compiler option "-mcmodel=small" is upstreamed from both

> > >> NOOPT_GCC49_AARCH64_CC_FLAGS and DEBUG_GCC49_AARCH64_CC_FLAGS to

> > >> GCC49_AARCH64_CC_FLAGS. This is a no-op for DEBUG and NOOPT, however the

> > >> option will now also apply to RELEASE.

> > >>

> > >> - As a consequence, "-mcmodel=tiny" is not usable for RELEASE, and it is

> > >> removed.

> > >>

> > >> - The same upstreaming occurs for "-z common-page-size=0x1000", in

> > >> GCC49_AARCH64_DLINK_FLAGS. RELEASE needs no fixup, it simply benefits

> > >> from the change. However, RELEASE_GCC49_AARCH64_DLINK_XIPFLAGS needs an

> > >> override, to stick with the original 0x20 alignment.

> > >>

> > >> * For GCC5:

> > >>

> > >> - GCC5_AARCH64_CC_FLAGS is defined as GCC49_AARCH64_CC_FLAGS, therefore

> > >> the "-mcmodel=small" inheritance from the above is automatic. Only the

> > >> DEBUG/NOOPT removals, and the RELEASE fixup are needed.

> > >>

> > >> - GCC5_AARCH64_DLINK_FLAGS is defined as GCC49_AARCH64_DLINK_FLAGS,

> > >> therefore the same inheritance pattern applies to "-z

> > >> common-page-size=0x1000". I'm noticing an omission here however: "-z

> > >> common-page-size=0x1000" should have been removed from

> > >> NOOPT_GCC5_AARCH64_DLINK_FLAGS as well.

> > >>

> > >> - The RELEASE_GCC5_AARCH64_DLINK_XIPFLAGS override is spelled out the same.

> > >>

> > >>

> > >> So... do you agree that NOOPT_GCC5_AARCH64_DLINK_FLAGS should have been

> > >> trimmed as well?

> > >>

> > >

> > > Yes, well spotted.

> > >

> > >> Furthermore, I suggest squeezing the word RELEASE into the subject,

> > >> given that the change is only observable in RELEASE builds.

> > >>

> > >

> > > Sure, I'll add that as well.

> > >

> >

> > Thanks! Personally I don't need to see a repost to believe you. :)

> >

> > With those updates:

> >

> > Reviewed-by: Laszlo Ersek <lersek@redhat.com>

> >

> > Thanks!

> > Laszlo

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
diff mbox series

Patch

diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template
index e0e68fd7fb49..5d34333dc54f 100755
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -4263,11 +4263,11 @@  DEFINE GCC49_ARM_ASM_FLAGS           = DEF(GCC48_ARM_ASM_FLAGS)
 DEFINE GCC49_AARCH64_ASM_FLAGS       = DEF(GCC48_AARCH64_ASM_FLAGS)
 DEFINE GCC49_ARM_CC_FLAGS            = DEF(GCC48_ARM_CC_FLAGS)
 DEFINE GCC49_ARM_CC_XIPFLAGS         = DEF(GCC48_ARM_CC_XIPFLAGS)
-DEFINE GCC49_AARCH64_CC_FLAGS        = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC44_ALL_CC_FLAGS) DEF(GCC_AARCH64_CC_FLAGS)
+DEFINE GCC49_AARCH64_CC_FLAGS        = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC44_ALL_CC_FLAGS) DEF(GCC_AARCH64_CC_FLAGS) -mcmodel=small
 DEFINE GCC49_AARCH64_CC_XIPFLAGS     = DEF(GCC48_AARCH64_CC_XIPFLAGS)
 DEFINE GCC49_ARM_DLINK_FLAGS         = DEF(GCC48_ARM_DLINK_FLAGS)
 DEFINE GCC49_ARM_DLINK2_FLAGS        = DEF(GCC48_ARM_DLINK2_FLAGS)
-DEFINE GCC49_AARCH64_DLINK_FLAGS     = DEF(GCC48_AARCH64_DLINK_FLAGS)
+DEFINE GCC49_AARCH64_DLINK_FLAGS     = DEF(GCC48_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000
 DEFINE GCC49_AARCH64_DLINK2_FLAGS    = DEF(GCC48_AARCH64_DLINK2_FLAGS)
 DEFINE GCC49_ARM_ASLDLINK_FLAGS      = DEF(GCC48_ARM_ASLDLINK_FLAGS)
 DEFINE GCC49_AARCH64_ASLDLINK_FLAGS  = DEF(GCC48_AARCH64_ASLDLINK_FLAGS)
@@ -5034,15 +5034,16 @@  RELEASE_GCC49_ARM_CC_FLAGS       = DEF(GCC49_ARM_CC_FLAGS) -Wno-unused-but-set-v
 *_GCC49_AARCH64_VFRPP_FLAGS      = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC_VFRPP_FLAGS)
 *_GCC49_AARCH64_CC_XIPFLAGS      = DEF(GCC49_AARCH64_CC_XIPFLAGS)
 
-  DEBUG_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -O0 -mcmodel=small
-  DEBUG_GCC49_AARCH64_DLINK_FLAGS  = DEF(GCC49_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000
+  DEBUG_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -O0
+  DEBUG_GCC49_AARCH64_DLINK_FLAGS  = DEF(GCC49_AARCH64_DLINK_FLAGS)
   DEBUG_GCC49_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20
 
-RELEASE_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -Wno-unused-but-set-variable -Wno-unused-const-variable -mcmodel=tiny
+RELEASE_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -Wno-unused-but-set-variable -Wno-unused-const-variable
 RELEASE_GCC49_AARCH64_DLINK_FLAGS  = DEF(GCC49_AARCH64_DLINK_FLAGS)
+RELEASE_GCC49_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20
 
-  NOOPT_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -O0 -mcmodel=small
-  NOOPT_GCC49_AARCH64_DLINK_FLAGS  = DEF(GCC49_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000 -O0
+  NOOPT_GCC49_AARCH64_CC_FLAGS     = DEF(GCC49_AARCH64_CC_FLAGS) -O0
+  NOOPT_GCC49_AARCH64_DLINK_FLAGS  = DEF(GCC49_AARCH64_DLINK_FLAGS) -O0
   NOOPT_GCC49_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20 -O0
 
 ####################################################################################
@@ -5189,14 +5190,15 @@  RELEASE_GCC5_ARM_DLINK_FLAGS     = DEF(GCC5_ARM_DLINK_FLAGS) -flto -Os -L$(WORKS
 *_GCC5_AARCH64_VFRPP_FLAGS       = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC_VFRPP_FLAGS)
 *_GCC5_AARCH64_CC_XIPFLAGS       = DEF(GCC5_AARCH64_CC_XIPFLAGS)
 
-  DEBUG_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -flto -Wno-unused-but-set-variable -Wno-unused-const-variable -mcmodel=small
-  DEBUG_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000 -flto -Os -L$(WORKSPACE)/ArmPkg/Library/GccLto -llto-aarch64 -Wl,-plugin-opt=-pass-through=-llto-aarch64 -Wno-lto-type-mismatch -mcmodel=small
+  DEBUG_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -flto -Wno-unused-but-set-variable -Wno-unused-const-variable
+  DEBUG_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -flto -Os -L$(WORKSPACE)/ArmPkg/Library/GccLto -llto-aarch64 -Wl,-plugin-opt=-pass-through=-llto-aarch64 -Wno-lto-type-mismatch
   DEBUG_GCC5_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20
 
-RELEASE_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -flto -Wno-unused-but-set-variable -Wno-unused-const-variable -mcmodel=tiny
+RELEASE_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -flto -Wno-unused-but-set-variable -Wno-unused-const-variable
 RELEASE_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -flto -Os -L$(WORKSPACE)/ArmPkg/Library/GccLto -llto-aarch64 -Wl,-plugin-opt=-pass-through=-llto-aarch64 -Wno-lto-type-mismatch
+RELEASE_GCC5_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20
 
-  NOOPT_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -O0 -mcmodel=small
+  NOOPT_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -O0
   NOOPT_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000 -O0
   NOOPT_GCC5_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20 -O0