FYI I am adding the below patch to my GCC consolidation series.
-------------8<--------------- In GCC 4.7, a feature was added to the ARM backend that allows unaligned loads and stores to be emitted. Since it is enabled by default on ARMv6 and later CPUs, and since such code is not suitable in our case (i.e., bare metal code), we must disable it by passing the -mno-unaligned-access option if we are using GCC 4.7 or later.
However, this particular feature and its enabling by default have been backported to version 4.6 by Linaro. Since the Linaro toolchains are widely used for ARM development, and also shipped by distros such as Ubuntu, we should disable the feature on version 4.6 as well. Unfortunately, since the upstream version does not support the feature, it also does not understand the -mno-unaligned-access option.
Considering the above, and the fact that the oldest supported toolchain for AARCH64 is 4.7 as well, let's just drop support for 4.6 altogether.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel ard.biesheuvel@linaro.org --- BaseTools/Conf/tools_def.template | 30 -------------------- 1 file changed, 30 deletions(-)
diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template index fabfe7436167..5707aafec8cd 100644 --- a/BaseTools/Conf/tools_def.template +++ b/BaseTools/Conf/tools_def.template @@ -4160,36 +4160,6 @@ DEFINE GCC_AARCH64_RC_FLAGS = -I binary -O elf64-littleaarch64 -B aarch64 *_GCC46_X64_OBJCOPY_FLAGS = *_GCC46_X64_NASM_FLAGS = -f elf64
-################## -# GCC46 ARM definitions -################## -*_GCC46_ARM_OBJCOPY_PATH = echo -*_GCC46_ARM_CC_PATH = ENV(GCC46_ARM_PREFIX)gcc -*_GCC46_ARM_SLINK_PATH = ENV(GCC46_ARM_PREFIX)ar -*_GCC46_ARM_DLINK_PATH = ENV(GCC46_ARM_PREFIX)ld -*_GCC46_ARM_ASLDLINK_PATH = ENV(GCC46_ARM_PREFIX)ld -*_GCC46_ARM_ASM_PATH = ENV(GCC46_ARM_PREFIX)gcc -*_GCC46_ARM_PP_PATH = ENV(GCC46_ARM_PREFIX)gcc -*_GCC46_ARM_VFRPP_PATH = ENV(GCC46_ARM_PREFIX)gcc -*_GCC46_ARM_ASLCC_PATH = ENV(GCC46_ARM_PREFIX)gcc -*_GCC46_ARM_ASLPP_PATH = ENV(GCC46_ARM_PREFIX)gcc -*_GCC46_ARM_RC_PATH = ENV(GCC46_ARM_PREFIX)objcopy - -*_GCC46_ARM_ARCHCC_FLAGS = -*_GCC46_ARM_PLATFORM_FLAGS = -march=armv7-a - -*_GCC46_ARM_ASLCC_FLAGS = DEF(GCC_ASLCC_FLAGS) -*_GCC46_ARM_ASLDLINK_FLAGS = DEF(GCC_ARM_ASLDLINK_FLAGS) -*_GCC46_ARM_ASM_FLAGS = DEF(GCC_ARM_ASM_FLAGS) -*_GCC46_ARM_DLINK_FLAGS = DEF(GCC_ARM_DLINK_FLAGS) -*_GCC46_ARM_PLATFORM_FLAGS = -march=armv7-a -*_GCC46_ARM_PP_FLAGS = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC_PP_FLAGS) -*_GCC46_ARM_RC_FLAGS = DEF(GCC_ARM_RC_FLAGS) -*_GCC46_ARM_VFRPP_FLAGS = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC_VFRPP_FLAGS) - - DEBUG_GCC46_ARM_CC_FLAGS = DEF(GCC_ARM_CC_FLAGS) -O0 -RELEASE_GCC46_ARM_CC_FLAGS = DEF(GCC_ARM_CC_FLAGS) - #################################################################################### # # GCC 4.7 - This configuration is used to compile under Linux to produce
Hi Ard,
On Mon, Aug 17, 2015 at 10:53:59AM +0200, Ard Biesheuvel wrote:
FYI I am adding the below patch to my GCC consolidation series.
Whilst I agree with the spirit of this patch, I have some concerns (basically I think it's too soon).
-------------8<--------------- In GCC 4.7, a feature was added to the ARM backend that allows unaligned loads and stores to be emitted. Since it is enabled by default on ARMv6 and later CPUs, and since such code is not suitable in our case (i.e., bare metal code), we must disable it by passing the -mno-unaligned-access option if we are using GCC 4.7 or later.
However, this particular feature and its enabling by default have been backported to version 4.6 by Linaro. Since the Linaro toolchains are widely used for ARM development, and also shipped by distros such as Ubuntu, we should disable the feature on version 4.6 as well.
I don't think it's that simple. I know some of the Ubuntu ones did _not_ support this flag (I'm thinking it was 12.04 I was using for GRUB development, and hitting this one.) And that's an LTS which will be supported until 2017.
And I've also seen people using codesourcery for edk2 development.
Unfortunately, since the upstream version does not support the feature, it also does not understand the -mno-unaligned-access option.
Considering the above, and the fact that the oldest supported toolchain for AARCH64 is 4.7 as well, let's just drop support for 4.6 altogether.
My request would be to hold back on this one for a while. Certainly give it a few months between dropping pre-v7 support and dropping gcc4.6.
/ Leif
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel ard.biesheuvel@linaro.org
BaseTools/Conf/tools_def.template | 30 -------------------- 1 file changed, 30 deletions(-)
diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template index fabfe7436167..5707aafec8cd 100644 --- a/BaseTools/Conf/tools_def.template +++ b/BaseTools/Conf/tools_def.template @@ -4160,36 +4160,6 @@ DEFINE GCC_AARCH64_RC_FLAGS = -I binary -O elf64-littleaarch64 -B aarch64 *_GCC46_X64_OBJCOPY_FLAGS = *_GCC46_X64_NASM_FLAGS = -f elf64 -################## -# GCC46 ARM definitions -################## -*_GCC46_ARM_OBJCOPY_PATH = echo -*_GCC46_ARM_CC_PATH = ENV(GCC46_ARM_PREFIX)gcc -*_GCC46_ARM_SLINK_PATH = ENV(GCC46_ARM_PREFIX)ar -*_GCC46_ARM_DLINK_PATH = ENV(GCC46_ARM_PREFIX)ld -*_GCC46_ARM_ASLDLINK_PATH = ENV(GCC46_ARM_PREFIX)ld -*_GCC46_ARM_ASM_PATH = ENV(GCC46_ARM_PREFIX)gcc -*_GCC46_ARM_PP_PATH = ENV(GCC46_ARM_PREFIX)gcc -*_GCC46_ARM_VFRPP_PATH = ENV(GCC46_ARM_PREFIX)gcc -*_GCC46_ARM_ASLCC_PATH = ENV(GCC46_ARM_PREFIX)gcc -*_GCC46_ARM_ASLPP_PATH = ENV(GCC46_ARM_PREFIX)gcc -*_GCC46_ARM_RC_PATH = ENV(GCC46_ARM_PREFIX)objcopy
-*_GCC46_ARM_ARCHCC_FLAGS = -*_GCC46_ARM_PLATFORM_FLAGS = -march=armv7-a
-*_GCC46_ARM_ASLCC_FLAGS = DEF(GCC_ASLCC_FLAGS) -*_GCC46_ARM_ASLDLINK_FLAGS = DEF(GCC_ARM_ASLDLINK_FLAGS) -*_GCC46_ARM_ASM_FLAGS = DEF(GCC_ARM_ASM_FLAGS) -*_GCC46_ARM_DLINK_FLAGS = DEF(GCC_ARM_DLINK_FLAGS) -*_GCC46_ARM_PLATFORM_FLAGS = -march=armv7-a -*_GCC46_ARM_PP_FLAGS = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC_PP_FLAGS) -*_GCC46_ARM_RC_FLAGS = DEF(GCC_ARM_RC_FLAGS) -*_GCC46_ARM_VFRPP_FLAGS = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) DEF(GCC_VFRPP_FLAGS)
- DEBUG_GCC46_ARM_CC_FLAGS = DEF(GCC_ARM_CC_FLAGS) -O0
-RELEASE_GCC46_ARM_CC_FLAGS = DEF(GCC_ARM_CC_FLAGS)
#################################################################################### #
# GCC 4.7 - This configuration is used to compile under Linux to produce
1.9.1
On 17 August 2015 at 11:12, Leif Lindholm leif.lindholm@linaro.org wrote:
Hi Ard,
On Mon, Aug 17, 2015 at 10:53:59AM +0200, Ard Biesheuvel wrote:
FYI I am adding the below patch to my GCC consolidation series.
Whilst I agree with the spirit of this patch, I have some concerns (basically I think it's too soon).
-------------8<--------------- In GCC 4.7, a feature was added to the ARM backend that allows unaligned loads and stores to be emitted. Since it is enabled by default on ARMv6 and later CPUs, and since such code is not suitable in our case (i.e., bare metal code), we must disable it by passing the -mno-unaligned-access option if we are using GCC 4.7 or later.
However, this particular feature and its enabling by default have been backported to version 4.6 by Linaro. Since the Linaro toolchains are widely used for ARM development, and also shipped by distros such as Ubuntu, we should disable the feature on version 4.6 as well.
I don't think it's that simple. I know some of the Ubuntu ones did _not_ support this flag (I'm thinking it was 12.04 I was using for GRUB development, and hitting this one.) And that's an LTS which will be supported until 2017.
And I've also seen people using codesourcery for edk2 development.
The problem is that when you /do/ use the Linaro 4.6 toolchain, we won't pass the -mno-unaligned-access flag (since upstream does not support it) and it will go and emit unaligned memory accesses which cause the resulting firmware to blow up.
As a workaround, we could build the C code for armv5, I suppose, because that will implicitly disable the feature if it is supported, and only generate suboptimal code otherwise. Or enable unaligned accesses in the MMU?
Unfortunately, since the upstream version does not support the feature, it also does not understand the -mno-unaligned-access option.
Considering the above, and the fact that the oldest supported toolchain for AARCH64 is 4.7 as well, let's just drop support for 4.6 altogether.
My request would be to hold back on this one for a while. Certainly give it a few months between dropping pre-v7 support and dropping gcc4.6.
IMO we have to do something to address the current situation. Got any other ideas besides -march=armv5t ?
On Mon, Aug 17, 2015 at 11:19:59AM +0200, Ard Biesheuvel wrote:
On 17 August 2015 at 11:12, Leif Lindholm leif.lindholm@linaro.org wrote:
Hi Ard,
On Mon, Aug 17, 2015 at 10:53:59AM +0200, Ard Biesheuvel wrote:
FYI I am adding the below patch to my GCC consolidation series.
Whilst I agree with the spirit of this patch, I have some concerns (basically I think it's too soon).
-------------8<--------------- In GCC 4.7, a feature was added to the ARM backend that allows unaligned loads and stores to be emitted. Since it is enabled by default on ARMv6 and later CPUs, and since such code is not suitable in our case (i.e., bare metal code), we must disable it by passing the -mno-unaligned-access option if we are using GCC 4.7 or later.
However, this particular feature and its enabling by default have been backported to version 4.6 by Linaro. Since the Linaro toolchains are widely used for ARM development, and also shipped by distros such as Ubuntu, we should disable the feature on version 4.6 as well.
I don't think it's that simple. I know some of the Ubuntu ones did _not_ support this flag (I'm thinking it was 12.04 I was using for GRUB development, and hitting this one.) And that's an LTS which will be supported until 2017.
And I've also seen people using codesourcery for edk2 development.
The problem is that when you /do/ use the Linaro 4.6 toolchain, we won't pass the -mno-unaligned-access flag (since upstream does not support it) and it will go and emit unaligned memory accesses which cause the resulting firmware to blow up.
Sure, but that's the status quo. We won't break anyone's working infrastructure by keeping it that way.
As a workaround, we could build the C code for armv5, I suppose, because that will implicitly disable the feature if it is supported, and only generate suboptimal code otherwise. Or enable unaligned accesses in the MMU?
Don't think we can guarantee only aligned accesses before MMU is enabled with any level of certainty. And even if we could it would be very fragile, which is a bad combination with the lower level of testing 4.6 is likely to see.
Unfortunately, since the upstream version does not support the feature, it also does not understand the -mno-unaligned-access option.
Considering the above, and the fact that the oldest supported toolchain for AARCH64 is 4.7 as well, let's just drop support for 4.6 altogether.
My request would be to hold back on this one for a while. Certainly give it a few months between dropping pre-v7 support and dropping gcc4.6.
IMO we have to do something to address the current situation. Got any other ideas besides -march=armv5t ?
Do we? Surely, if someone is having problems deploying gcc4.6 because the Linaro version enables unaligned accesse where it shouldn't, they should switch to a pure upstream compiler or step to a later toolchain?
(Yes, I realise this includes our very own matrix build.)
/ Leif
On 17 August 2015 at 11:47, Leif Lindholm leif.lindholm@linaro.org wrote:
On Mon, Aug 17, 2015 at 11:19:59AM +0200, Ard Biesheuvel wrote:
On 17 August 2015 at 11:12, Leif Lindholm leif.lindholm@linaro.org wrote:
Hi Ard,
On Mon, Aug 17, 2015 at 10:53:59AM +0200, Ard Biesheuvel wrote:
FYI I am adding the below patch to my GCC consolidation series.
Whilst I agree with the spirit of this patch, I have some concerns (basically I think it's too soon).
-------------8<--------------- In GCC 4.7, a feature was added to the ARM backend that allows unaligned loads and stores to be emitted. Since it is enabled by default on ARMv6 and later CPUs, and since such code is not suitable in our case (i.e., bare metal code), we must disable it by passing the -mno-unaligned-access option if we are using GCC 4.7 or later.
However, this particular feature and its enabling by default have been backported to version 4.6 by Linaro. Since the Linaro toolchains are widely used for ARM development, and also shipped by distros such as Ubuntu, we should disable the feature on version 4.6 as well.
I don't think it's that simple. I know some of the Ubuntu ones did _not_ support this flag (I'm thinking it was 12.04 I was using for GRUB development, and hitting this one.) And that's an LTS which will be supported until 2017.
And I've also seen people using codesourcery for edk2 development.
The problem is that when you /do/ use the Linaro 4.6 toolchain, we won't pass the -mno-unaligned-access flag (since upstream does not support it) and it will go and emit unaligned memory accesses which cause the resulting firmware to blow up.
Sure, but that's the status quo. We won't break anyone's working infrastructure by keeping it that way.
I think that's a pretty unambitious definition of 'working' if it is producing builds that may be broken in unexpected ways.
As a workaround, we could build the C code for armv5, I suppose, because that will implicitly disable the feature if it is supported, and only generate suboptimal code otherwise. Or enable unaligned accesses in the MMU?
Don't think we can guarantee only aligned accesses before MMU is enabled with any level of certainty. And even if we could it would be very fragile, which is a bad combination with the lower level of testing 4.6 is likely to see.
Unfortunately, since the upstream version does not support the feature, it also does not understand the -mno-unaligned-access option.
Considering the above, and the fact that the oldest supported toolchain for AARCH64 is 4.7 as well, let's just drop support for 4.6 altogether.
My request would be to hold back on this one for a while. Certainly give it a few months between dropping pre-v7 support and dropping gcc4.6.
IMO we have to do something to address the current situation. Got any other ideas besides -march=armv5t ?
Do we?
As Linaro UEFI maintainers, I think we should do everything we can to prevent people from using *our* toolchain to produce firmware images that we know may be broken.
Surely, if someone is having problems deploying gcc4.6 because the Linaro version enables unaligned accesse where it shouldn't, they should switch to a pure upstream compiler or step to a later toolchain?
(Yes, I realise this includes our very own matrix build.)
That presumes awareness of the fact. The issue here is that the brokenness of the resulting firmware images has not manifested itself yet.
There is a __ARM_FEATURE_UNALIGNED builtin define that we can test for in some header that all platforms include, and #error if it is set. Other than that, I think dropping 4.6 is the only reasonable course of action. (Setting arch=armv5t conflicts badly with the mthumb and cpu=cortex-a15 options that are passed in various places)
On Mon, Aug 17, 2015 at 12:18:13PM +0200, Ard Biesheuvel wrote:
The problem is that when you /do/ use the Linaro 4.6 toolchain, we won't pass the -mno-unaligned-access flag (since upstream does not support it) and it will go and emit unaligned memory accesses which cause the resulting firmware to blow up.
Sure, but that's the status quo. We won't break anyone's working infrastructure by keeping it that way.
I think that's a pretty unambitious definition of 'working' if it is producing builds that may be broken in unexpected ways.
Dropping GCC46 and telling people to use GCC47 instead will break setups that are genuinely working today. Gcc4.6 without the Linaro patches does not generate unaligned accesses. A switch to supporting only GCC47 will explicitly break the setups that are the only ones known to be generating correct code today.
Unfortunately, since the upstream version does not support the feature, it also does not understand the -mno-unaligned-access option.
Considering the above, and the fact that the oldest supported toolchain for AARCH64 is 4.7 as well, let's just drop support for 4.6 altogether.
My request would be to hold back on this one for a while. Certainly give it a few months between dropping pre-v7 support and dropping gcc4.6.
IMO we have to do something to address the current situation. Got any other ideas besides -march=armv5t ?
Do we?
As Linaro UEFI maintainers, I think we should do everything we can to prevent people from using *our* toolchain to produce firmware images that we know may be broken.
As ARM architecture UEFI maintainers, I think it's important that we do not get into playing favourites. The best solution I see is actively discouraging people from using Linaro toolchains prior to gcc4.7 for edk2 development (see below).
Surely, if someone is having problems deploying gcc4.6 because the Linaro version enables unaligned accesse where it shouldn't, they should switch to a pure upstream compiler or step to a later toolchain?
(Yes, I realise this includes our very own matrix build.)
That presumes awareness of the fact. The issue here is that the brokenness of the resulting firmware images has not manifested itself yet.
Indeed.
There is a __ARM_FEATURE_UNALIGNED builtin define that we can test for in some header that all platforms include, and #error if it is set. Other than that, I think dropping 4.6 is the only reasonable course of action. (Setting arch=armv5t conflicts badly with the mthumb and cpu=cortex-a15 options that are passed in various places)
I would be entirely happy with a test in a critical header for __ARM_FEATURE_UNALIGNED with GCC46 and bailing on it. Unlike dropping GCC46, this will also raise a flag to current users of unsafe build setups that something is wrong.
Then if they come asking why their build breaks, we tell them that they need to use the GCC47 profile.
/ Leif
-
Ard Biesheuvel -
Leif Lindholm