qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH-for-5.0] roms/edk2-funcs.sh: Use available GCC for ARM/Aarch64 targets
@ 2019-12-04 22:12 Philippe Mathieu-Daudé
  2019-12-05 16:13 ` Laszlo Ersek
  0 siblings, 1 reply; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-12-04 22:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: Philippe Mathieu-Daudé, Laszlo Ersek

Centos 7.7 only provides cross GCC 4.8.5, but the script forces
us to use GCC5. Since the same machinery is valid to check the
GCC version, remove the $emulation_target check.

  $ cat /etc/redhat-release
  CentOS Linux release 7.7.1908 (Core)

  $ aarch64-linux-gnu-gcc -v 2>&1 | tail -1
  gcc version 4.8.5 20150623 (Red Hat 4.8.5-16) (GCC)

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
Patch to review with --ignore-all-space
---
 roms/edk2-funcs.sh | 48 +++++++++++++++++++---------------------------
 1 file changed, 20 insertions(+), 28 deletions(-)

diff --git a/roms/edk2-funcs.sh b/roms/edk2-funcs.sh
index 3f4485b201..a455611c0d 100644
--- a/roms/edk2-funcs.sh
+++ b/roms/edk2-funcs.sh
@@ -135,35 +135,27 @@ qemu_edk2_get_toolchain()
     return 1
   fi
 
-  case "$emulation_target" in
-    (arm|aarch64)
-      printf 'GCC5\n'
+  if ! cross_prefix=$(qemu_edk2_get_cross_prefix "$emulation_target"); then
+    return 1
+  fi
+
+  gcc_version=$("${cross_prefix}gcc" -v 2>&1 | tail -1 | awk '{print $3}')
+  # Run "git-blame" on "OvmfPkg/build.sh" in edk2 for more information on
+  # the mapping below.
+  case "$gcc_version" in
+    ([1-3].*|4.[0-7].*)
+      printf '%s: unsupported gcc version "%s"\n' \
+        "$program_name" "$gcc_version" >&2
+      return 1
       ;;
-
-    (i386|x86_64)
-      if ! cross_prefix=$(qemu_edk2_get_cross_prefix "$emulation_target"); then
-        return 1
-      fi
-
-      gcc_version=$("${cross_prefix}gcc" -v 2>&1 | tail -1 | awk '{print $3}')
-      # Run "git-blame" on "OvmfPkg/build.sh" in edk2 for more information on
-      # the mapping below.
-      case "$gcc_version" in
-        ([1-3].*|4.[0-7].*)
-          printf '%s: unsupported gcc version "%s"\n' \
-            "$program_name" "$gcc_version" >&2
-          return 1
-          ;;
-        (4.8.*)
-          printf 'GCC48\n'
-          ;;
-        (4.9.*|6.[0-2].*)
-          printf 'GCC49\n'
-          ;;
-        (*)
-          printf 'GCC5\n'
-          ;;
-      esac
+    (4.8.*)
+      printf 'GCC48\n'
+      ;;
+    (4.9.*|6.[0-2].*)
+      printf 'GCC49\n'
+      ;;
+    (*)
+      printf 'GCC5\n'
       ;;
   esac
 }
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH-for-5.0] roms/edk2-funcs.sh: Use available GCC for ARM/Aarch64 targets
  2019-12-04 22:12 [PATCH-for-5.0] roms/edk2-funcs.sh: Use available GCC for ARM/Aarch64 targets Philippe Mathieu-Daudé
@ 2019-12-05 16:13 ` Laszlo Ersek
  2019-12-05 16:27   ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 9+ messages in thread
From: Laszlo Ersek @ 2019-12-05 16:13 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé; +Cc: qemu-devel, Ard Biesheuvel

Hi Phil,

(+Ard)

On 12/04/19 23:12, Philippe Mathieu-Daudé wrote:
> Centos 7.7 only provides cross GCC 4.8.5, but the script forces
> us to use GCC5. Since the same machinery is valid to check the
> GCC version, remove the $emulation_target check.
>
>   $ cat /etc/redhat-release
>   CentOS Linux release 7.7.1908 (Core)
>
>   $ aarch64-linux-gnu-gcc -v 2>&1 | tail -1
>   gcc version 4.8.5 20150623 (Red Hat 4.8.5-16) (GCC)

this patch is not correct, in my opinion. ARM / AARCH64 support in edk2
requires GCC5 as a minimum. It was never tested with an earlier
toolchain, to my understanding. Not on my part, anyway.

To be more precise: when I tested cross-gcc toolchains earlier than
that, the ArmVirtQemu builds always failed. Minimally, those toolchains
didn't recognize some of the AARCH64 system registers.

If CentOS 7.7 does not provide a suitable (>=GCC5) toolchain, then we
can't build ArmVirtQemu binaries on CentOS 7.7, in my opinion.

Personally, on my RHEL7 laptop, over time I've used the following
toolchains, to satisfy the GCC5 requirement of ArmVirtQemu (which
requirement I took as experimental evidence):

- Initially (last quarter of 2014), I used binary distributions --
  tarballs -- of cross-binutils and cross-gcc, from Linaro.

- Later (last quarter of 2016), I rebuilt some SRPMs that were at the
  time Fedora-only for RHEL7. Namely:

  - cross-binutils-2.27-3.fc24
    https://koji.fedoraproject.org/koji/buildinfo?buildID=801348

  - gcc-6.1.1-2.fc24
    https://koji.fedoraproject.org/koji/buildinfo?buildID=761767

- Most recently, I've been using cross-binutils updated from EPEL7:

  - cross-binutils-2.27-9.el7.1
    https://koji.fedoraproject.org/koji/buildinfo?buildID=918474

To my knowledge, there is still no suitable cross-compiler available on
RHEL7, from any trustworthy RPM repository. So, to this day, I use
gcc-6.1.1-2 for cross-building ArmVirtQemu, on my RHEL7 laptop.

Again: I believe it does not matter if the gcc-4.8.5-based
cross-compiler in CentOS 7 "happens" to work. That's a compiler that I
have never tested with, or vetted for, upstream ArmVirtQemu.

Now, I realize that in edk2, we have stuff like

  GCC48_AARCH64_CC_FLAGS

in "BaseTools/Conf/tools_def.template" -- coming from commit
7a9dbf2c94d1 ("BaseTools/Conf/tools_def.template: drop ARM/AARCH support
from GCC46/GCC47", 2019-01-08). That doesn't change the fact that I've
never built or tested ArmVirtQemu with such a compiler. And so this
patch makes me quite uncomfortable.

If that rules out CentOS 7 as a QEMU project build / CI platform for the
bundled ArmVirtQemu binaries, then we need a more recent platform
(perhaps CentOS 8, not sure).

I think it's also educational to check the origin of the code that your
patch proposes to remove. Most recently it was moved around from a
different place, in QEMU commit 65a109ab4b1a ('roms: lift
"edk2-funcs.sh" from "tests/uefi-test-tools/build.sh"', 2019-04-17).

In that commit, for some reason I didn't keep the original code comments
(perhaps it would have been too difficult or messy to preserve the
comments sanely with the restructured / factored-out code). But, they
went like this (originally from commit 77db55fc8155,
"tests/uefi-test-tools: add build scripts", 2019-02-21):

# Expose cross_prefix (which is possibly empty) to the edk2 tools. While at it,
# determine the suitable edk2 toolchain as well.
# - For ARM and AARCH64, edk2 only offers the GCC5 toolchain tag, which covers
#   the gcc-5+ releases.
# - For IA32 and X64, edk2 offers the GCC44 through GCC49 toolchain tags, in
#   addition to GCC5. Unfortunately, the mapping between the toolchain tags and
#   the actual gcc releases isn't entirely trivial. Run "git-blame" on
#   "OvmfPkg/build.sh" in edk2 for more information.
# And, because the above is too simple, we have to assign cross_prefix to an
# edk2 build variable that is specific to both the toolchain tag and the target
# architecture.

So... unless Ard feels it is really totally safe to retro-actively rely
on the gcc-4.8.5-based compiler in CentOS 7, I'd rather we picked a more
recent build platform (OS) instead. For example, we build ArmVirtQemu on
RHEL8 regularly, so that's a reality-based "plus" for CentOS 8.


Independently of all of the above, the OVMF toolchain selection logic
that this patch proposes to reuse with ArmVirtQemu, is *really*
x86-specific. Please run "git blame" on "OvmfPkg/build.sh" in upstream
edk2, to see where the various branches come from (as the comments in
this shell script suggest as well). There had been mess like commit
656ac0c7d8ea ('Revert "OvmfPkg/build.sh: select the GCC49 toolchain
settings for gcc-7.*"', 2017-08-25).

Thanks,
Laszlo

>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> Patch to review with --ignore-all-space
> ---
>  roms/edk2-funcs.sh | 48 +++++++++++++++++++---------------------------
>  1 file changed, 20 insertions(+), 28 deletions(-)
>
> diff --git a/roms/edk2-funcs.sh b/roms/edk2-funcs.sh
> index 3f4485b201..a455611c0d 100644
> --- a/roms/edk2-funcs.sh
> +++ b/roms/edk2-funcs.sh
> @@ -135,35 +135,27 @@ qemu_edk2_get_toolchain()
>      return 1
>    fi
>
> -  case "$emulation_target" in
> -    (arm|aarch64)
> -      printf 'GCC5\n'
> +  if ! cross_prefix=$(qemu_edk2_get_cross_prefix "$emulation_target"); then
> +    return 1
> +  fi
> +
> +  gcc_version=$("${cross_prefix}gcc" -v 2>&1 | tail -1 | awk '{print $3}')
> +  # Run "git-blame" on "OvmfPkg/build.sh" in edk2 for more information on
> +  # the mapping below.
> +  case "$gcc_version" in
> +    ([1-3].*|4.[0-7].*)
> +      printf '%s: unsupported gcc version "%s"\n' \
> +        "$program_name" "$gcc_version" >&2
> +      return 1
>        ;;
> -
> -    (i386|x86_64)
> -      if ! cross_prefix=$(qemu_edk2_get_cross_prefix "$emulation_target"); then
> -        return 1
> -      fi
> -
> -      gcc_version=$("${cross_prefix}gcc" -v 2>&1 | tail -1 | awk '{print $3}')
> -      # Run "git-blame" on "OvmfPkg/build.sh" in edk2 for more information on
> -      # the mapping below.
> -      case "$gcc_version" in
> -        ([1-3].*|4.[0-7].*)
> -          printf '%s: unsupported gcc version "%s"\n' \
> -            "$program_name" "$gcc_version" >&2
> -          return 1
> -          ;;
> -        (4.8.*)
> -          printf 'GCC48\n'
> -          ;;
> -        (4.9.*|6.[0-2].*)
> -          printf 'GCC49\n'
> -          ;;
> -        (*)
> -          printf 'GCC5\n'
> -          ;;
> -      esac
> +    (4.8.*)
> +      printf 'GCC48\n'
> +      ;;
> +    (4.9.*|6.[0-2].*)
> +      printf 'GCC49\n'
> +      ;;
> +    (*)
> +      printf 'GCC5\n'
>        ;;
>    esac
>  }
>



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH-for-5.0] roms/edk2-funcs.sh: Use available GCC for ARM/Aarch64 targets
  2019-12-05 16:13 ` Laszlo Ersek
@ 2019-12-05 16:27   ` Philippe Mathieu-Daudé
  2019-12-05 16:47     ` Andrea Bolognani
  2019-12-05 16:50     ` Ard Biesheuvel
  0 siblings, 2 replies; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-12-05 16:27 UTC (permalink / raw)
  To: Laszlo Ersek; +Cc: qemu-devel, Ard Biesheuvel

On 12/5/19 5:13 PM, Laszlo Ersek wrote:
> Hi Phil,
> 
> (+Ard)
> 
> On 12/04/19 23:12, Philippe Mathieu-Daudé wrote:
>> Centos 7.7 only provides cross GCC 4.8.5, but the script forces
>> us to use GCC5. Since the same machinery is valid to check the
>> GCC version, remove the $emulation_target check.
>>
>>    $ cat /etc/redhat-release
>>    CentOS Linux release 7.7.1908 (Core)
>>
>>    $ aarch64-linux-gnu-gcc -v 2>&1 | tail -1
>>    gcc version 4.8.5 20150623 (Red Hat 4.8.5-16) (GCC)
> 
> this patch is not correct, in my opinion. ARM / AARCH64 support in edk2
> requires GCC5 as a minimum. It was never tested with an earlier
> toolchain, to my understanding. Not on my part, anyway.
> 
> To be more precise: when I tested cross-gcc toolchains earlier than
> that, the ArmVirtQemu builds always failed. Minimally, those toolchains
> didn't recognize some of the AARCH64 system registers.
> 
> If CentOS 7.7 does not provide a suitable (>=GCC5) toolchain, then we
> can't build ArmVirtQemu binaries on CentOS 7.7, in my opinion.
> 
> Personally, on my RHEL7 laptop, over time I've used the following
> toolchains, to satisfy the GCC5 requirement of ArmVirtQemu (which
> requirement I took as experimental evidence):
> 
> - Initially (last quarter of 2014), I used binary distributions --
>    tarballs -- of cross-binutils and cross-gcc, from Linaro.
> 
> - Later (last quarter of 2016), I rebuilt some SRPMs that were at the
>    time Fedora-only for RHEL7. Namely:
> 
>    - cross-binutils-2.27-3.fc24
>      https://koji.fedoraproject.org/koji/buildinfo?buildID=801348
> 
>    - gcc-6.1.1-2.fc24
>      https://koji.fedoraproject.org/koji/buildinfo?buildID=761767
> 
> - Most recently, I've been using cross-binutils updated from EPEL7:
> 
>    - cross-binutils-2.27-9.el7.1
>      https://koji.fedoraproject.org/koji/buildinfo?buildID=918474
> 
> To my knowledge, there is still no suitable cross-compiler available on
> RHEL7, from any trustworthy RPM repository. So, to this day, I use
> gcc-6.1.1-2 for cross-building ArmVirtQemu, on my RHEL7 laptop.
> 
> Again: I believe it does not matter if the gcc-4.8.5-based
> cross-compiler in CentOS 7 "happens" to work. That's a compiler that I
> have never tested with, or vetted for, upstream ArmVirtQemu.
> 
> Now, I realize that in edk2, we have stuff like
> 
>    GCC48_AARCH64_CC_FLAGS
> 
> in "BaseTools/Conf/tools_def.template" -- coming from commit
> 7a9dbf2c94d1 ("BaseTools/Conf/tools_def.template: drop ARM/AARCH support
> from GCC46/GCC47", 2019-01-08). That doesn't change the fact that I've
> never built or tested ArmVirtQemu with such a compiler. And so this
> patch makes me quite uncomfortable.
> 
> If that rules out CentOS 7 as a QEMU project build / CI platform for the
> bundled ArmVirtQemu binaries, then we need a more recent platform
> (perhaps CentOS 8, not sure).

Unfortunately CentOS 8 is not available as a Docker image, which is a 
convenient way to build EDK2 in a CI.

> I think it's also educational to check the origin of the code that your
> patch proposes to remove. Most recently it was moved around from a
> different place, in QEMU commit 65a109ab4b1a ('roms: lift
> "edk2-funcs.sh" from "tests/uefi-test-tools/build.sh"', 2019-04-17).
> 
> In that commit, for some reason I didn't keep the original code comments
> (perhaps it would have been too difficult or messy to preserve the
> comments sanely with the restructured / factored-out code). But, they
> went like this (originally from commit 77db55fc8155,
> "tests/uefi-test-tools: add build scripts", 2019-02-21):
> 
> # Expose cross_prefix (which is possibly empty) to the edk2 tools. While at it,
> # determine the suitable edk2 toolchain as well.
> # - For ARM and AARCH64, edk2 only offers the GCC5 toolchain tag, which covers
> #   the gcc-5+ releases.
> # - For IA32 and X64, edk2 offers the GCC44 through GCC49 toolchain tags, in
> #   addition to GCC5. Unfortunately, the mapping between the toolchain tags and
> #   the actual gcc releases isn't entirely trivial. Run "git-blame" on
> #   "OvmfPkg/build.sh" in edk2 for more information.
> # And, because the above is too simple, we have to assign cross_prefix to an
> # edk2 build variable that is specific to both the toolchain tag and the target
> # architecture.
> 
> So... unless Ard feels it is really totally safe to retro-actively rely
> on the gcc-4.8.5-based compiler in CentOS 7, I'd rather we picked a more
> recent build platform (OS) instead. For example, we build ArmVirtQemu on
> RHEL8 regularly, so that's a reality-based "plus" for CentOS 8.
> 
> 
> Independently of all of the above, the OVMF toolchain selection logic
> that this patch proposes to reuse with ArmVirtQemu, is *really*
> x86-specific. Please run "git blame" on "OvmfPkg/build.sh" in upstream
> edk2, to see where the various branches come from (as the comments in
> this shell script suggest as well). There had been mess like commit
> 656ac0c7d8ea ('Revert "OvmfPkg/build.sh: select the GCC49 toolchain
> settings for gcc-7.*"', 2017-08-25).

Thanks for all the pointers, very educative indeed :)

I'll see other setups I can use with GCC5+ available.

I still have to figure if there are free tier CI with less limitations 
than Travis/Shippable/GitLab, so we can keep the full EDK2 build output log.

> Thanks,
> Laszlo
> 
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>> Patch to review with --ignore-all-space
>> ---
>>   roms/edk2-funcs.sh | 48 +++++++++++++++++++---------------------------
>>   1 file changed, 20 insertions(+), 28 deletions(-)
>>
>> diff --git a/roms/edk2-funcs.sh b/roms/edk2-funcs.sh
>> index 3f4485b201..a455611c0d 100644
>> --- a/roms/edk2-funcs.sh
>> +++ b/roms/edk2-funcs.sh
>> @@ -135,35 +135,27 @@ qemu_edk2_get_toolchain()
>>       return 1
>>     fi
>>
>> -  case "$emulation_target" in
>> -    (arm|aarch64)
>> -      printf 'GCC5\n'
>> +  if ! cross_prefix=$(qemu_edk2_get_cross_prefix "$emulation_target"); then
>> +    return 1
>> +  fi
>> +
>> +  gcc_version=$("${cross_prefix}gcc" -v 2>&1 | tail -1 | awk '{print $3}')
>> +  # Run "git-blame" on "OvmfPkg/build.sh" in edk2 for more information on
>> +  # the mapping below.
>> +  case "$gcc_version" in
>> +    ([1-3].*|4.[0-7].*)
>> +      printf '%s: unsupported gcc version "%s"\n' \
>> +        "$program_name" "$gcc_version" >&2
>> +      return 1
>>         ;;
>> -
>> -    (i386|x86_64)
>> -      if ! cross_prefix=$(qemu_edk2_get_cross_prefix "$emulation_target"); then
>> -        return 1
>> -      fi
>> -
>> -      gcc_version=$("${cross_prefix}gcc" -v 2>&1 | tail -1 | awk '{print $3}')
>> -      # Run "git-blame" on "OvmfPkg/build.sh" in edk2 for more information on
>> -      # the mapping below.
>> -      case "$gcc_version" in
>> -        ([1-3].*|4.[0-7].*)
>> -          printf '%s: unsupported gcc version "%s"\n' \
>> -            "$program_name" "$gcc_version" >&2
>> -          return 1
>> -          ;;
>> -        (4.8.*)
>> -          printf 'GCC48\n'
>> -          ;;
>> -        (4.9.*|6.[0-2].*)
>> -          printf 'GCC49\n'
>> -          ;;
>> -        (*)
>> -          printf 'GCC5\n'
>> -          ;;
>> -      esac
>> +    (4.8.*)
>> +      printf 'GCC48\n'
>> +      ;;
>> +    (4.9.*|6.[0-2].*)
>> +      printf 'GCC49\n'
>> +      ;;
>> +    (*)
>> +      printf 'GCC5\n'
>>         ;;
>>     esac
>>   }
>>
> 
> 



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH-for-5.0] roms/edk2-funcs.sh: Use available GCC for ARM/Aarch64 targets
  2019-12-05 16:27   ` Philippe Mathieu-Daudé
@ 2019-12-05 16:47     ` Andrea Bolognani
  2019-12-05 17:04       ` Philippe Mathieu-Daudé
  2019-12-05 16:50     ` Ard Biesheuvel
  1 sibling, 1 reply; 9+ messages in thread
From: Andrea Bolognani @ 2019-12-05 16:47 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Laszlo Ersek; +Cc: qemu-devel, Ard Biesheuvel

On Thu, 2019-12-05 at 17:27 +0100, Philippe Mathieu-Daudé wrote:
> On 12/5/19 5:13 PM, Laszlo Ersek wrote:
> > If that rules out CentOS 7 as a QEMU project build / CI platform for the
> > bundled ArmVirtQemu binaries, then we need a more recent platform
> > (perhaps CentOS 8, not sure).
> 
> Unfortunately CentOS 8 is not available as a Docker image, which is a 
> convenient way to build EDK2 in a CI.

Uh?

  https://hub.docker.com/_/centos

seems to disagree with you. Is the 'centos:8' image not suitable
for some non-obvious reason?

-- 
Andrea Bolognani / Red Hat / Virtualization



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH-for-5.0] roms/edk2-funcs.sh: Use available GCC for ARM/Aarch64 targets
  2019-12-05 16:27   ` Philippe Mathieu-Daudé
  2019-12-05 16:47     ` Andrea Bolognani
@ 2019-12-05 16:50     ` Ard Biesheuvel
  2019-12-05 19:35       ` Laszlo Ersek
  1 sibling, 1 reply; 9+ messages in thread
From: Ard Biesheuvel @ 2019-12-05 16:50 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé; +Cc: Laszlo Ersek, QEMU Developers

On Thu, 5 Dec 2019 at 16:27, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> On 12/5/19 5:13 PM, Laszlo Ersek wrote:
> > Hi Phil,
> >
> > (+Ard)
> >
> > On 12/04/19 23:12, Philippe Mathieu-Daudé wrote:
> >> Centos 7.7 only provides cross GCC 4.8.5, but the script forces
> >> us to use GCC5. Since the same machinery is valid to check the
> >> GCC version, remove the $emulation_target check.
> >>
> >>    $ cat /etc/redhat-release
> >>    CentOS Linux release 7.7.1908 (Core)
> >>
> >>    $ aarch64-linux-gnu-gcc -v 2>&1 | tail -1
> >>    gcc version 4.8.5 20150623 (Red Hat 4.8.5-16) (GCC)
> >
> > this patch is not correct, in my opinion. ARM / AARCH64 support in edk2
> > requires GCC5 as a minimum. It was never tested with an earlier
> > toolchain, to my understanding. Not on my part, anyway.
> >
> > To be more precise: when I tested cross-gcc toolchains earlier than
> > that, the ArmVirtQemu builds always failed. Minimally, those toolchains
> > didn't recognize some of the AARCH64 system registers.
> >
> > If CentOS 7.7 does not provide a suitable (>=GCC5) toolchain, then we
> > can't build ArmVirtQemu binaries on CentOS 7.7, in my opinion.
> >
> > Personally, on my RHEL7 laptop, over time I've used the following
> > toolchains, to satisfy the GCC5 requirement of ArmVirtQemu (which
> > requirement I took as experimental evidence):
> >
> > - Initially (last quarter of 2014), I used binary distributions --
> >    tarballs -- of cross-binutils and cross-gcc, from Linaro.
> >
> > - Later (last quarter of 2016), I rebuilt some SRPMs that were at the
> >    time Fedora-only for RHEL7. Namely:
> >
> >    - cross-binutils-2.27-3.fc24
> >      https://koji.fedoraproject.org/koji/buildinfo?buildID=801348
> >
> >    - gcc-6.1.1-2.fc24
> >      https://koji.fedoraproject.org/koji/buildinfo?buildID=761767
> >
> > - Most recently, I've been using cross-binutils updated from EPEL7:
> >
> >    - cross-binutils-2.27-9.el7.1
> >      https://koji.fedoraproject.org/koji/buildinfo?buildID=918474
> >
> > To my knowledge, there is still no suitable cross-compiler available on
> > RHEL7, from any trustworthy RPM repository. So, to this day, I use
> > gcc-6.1.1-2 for cross-building ArmVirtQemu, on my RHEL7 laptop.
> >
> > Again: I believe it does not matter if the gcc-4.8.5-based
> > cross-compiler in CentOS 7 "happens" to work. That's a compiler that I
> > have never tested with, or vetted for, upstream ArmVirtQemu.
> >
> > Now, I realize that in edk2, we have stuff like
> >
> >    GCC48_AARCH64_CC_FLAGS
> >
> > in "BaseTools/Conf/tools_def.template" -- coming from commit
> > 7a9dbf2c94d1 ("BaseTools/Conf/tools_def.template: drop ARM/AARCH support
> > from GCC46/GCC47", 2019-01-08). That doesn't change the fact that I've
> > never built or tested ArmVirtQemu with such a compiler. And so this
> > patch makes me quite uncomfortable.
> >
> > If that rules out CentOS 7 as a QEMU project build / CI platform for the
> > bundled ArmVirtQemu binaries, then we need a more recent platform
> > (perhaps CentOS 8, not sure).
>
> Unfortunately CentOS 8 is not available as a Docker image, which is a
> convenient way to build EDK2 in a CI.
>
> > I think it's also educational to check the origin of the code that your
> > patch proposes to remove. Most recently it was moved around from a
> > different place, in QEMU commit 65a109ab4b1a ('roms: lift
> > "edk2-funcs.sh" from "tests/uefi-test-tools/build.sh"', 2019-04-17).
> >
> > In that commit, for some reason I didn't keep the original code comments
> > (perhaps it would have been too difficult or messy to preserve the
> > comments sanely with the restructured / factored-out code). But, they
> > went like this (originally from commit 77db55fc8155,
> > "tests/uefi-test-tools: add build scripts", 2019-02-21):
> >
> > # Expose cross_prefix (which is possibly empty) to the edk2 tools. While at it,
> > # determine the suitable edk2 toolchain as well.
> > # - For ARM and AARCH64, edk2 only offers the GCC5 toolchain tag, which covers
> > #   the gcc-5+ releases.
> > # - For IA32 and X64, edk2 offers the GCC44 through GCC49 toolchain tags, in
> > #   addition to GCC5. Unfortunately, the mapping between the toolchain tags and
> > #   the actual gcc releases isn't entirely trivial. Run "git-blame" on
> > #   "OvmfPkg/build.sh" in edk2 for more information.
> > # And, because the above is too simple, we have to assign cross_prefix to an
> > # edk2 build variable that is specific to both the toolchain tag and the target
> > # architecture.
> >
> > So... unless Ard feels it is really totally safe to retro-actively rely
> > on the gcc-4.8.5-based compiler in CentOS 7, I'd rather we picked a more
> > recent build platform (OS) instead. For example, we build ArmVirtQemu on
> > RHEL8 regularly, so that's a reality-based "plus" for CentOS 8.
> >
> >
> > Independently of all of the above, the OVMF toolchain selection logic
> > that this patch proposes to reuse with ArmVirtQemu, is *really*
> > x86-specific. Please run "git blame" on "OvmfPkg/build.sh" in upstream
> > edk2, to see where the various branches come from (as the comments in
> > this shell script suggest as well). There had been mess like commit
> > 656ac0c7d8ea ('Revert "OvmfPkg/build.sh: select the GCC49 toolchain
> > settings for gcc-7.*"', 2017-08-25).
>
> Thanks for all the pointers, very educative indeed :)
>
> I'll see other setups I can use with GCC5+ available.
>
> I still have to figure if there are free tier CI with less limitations
> than Travis/Shippable/GitLab, so we can keep the full EDK2 build output log.
>

My CI job for ArmVirtQemu/EDK2 build tested GCC48 and GCC49 until very
recently, and I never experienced any issues when running those
images, although it's been much longer that I actually tried that. So
I wouldn't recommend against it, and if we do identify any issues, we
should either deprecate GCC48 (for ArmVirtQemu or for AArch64
altogether) or fix them.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH-for-5.0] roms/edk2-funcs.sh: Use available GCC for ARM/Aarch64 targets
  2019-12-05 16:47     ` Andrea Bolognani
@ 2019-12-05 17:04       ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-12-05 17:04 UTC (permalink / raw)
  To: Andrea Bolognani, Laszlo Ersek; +Cc: qemu-devel, Ard Biesheuvel

On 12/5/19 5:47 PM, Andrea Bolognani wrote:
> On Thu, 2019-12-05 at 17:27 +0100, Philippe Mathieu-Daudé wrote:
>> On 12/5/19 5:13 PM, Laszlo Ersek wrote:
>>> If that rules out CentOS 7 as a QEMU project build / CI platform for the
>>> bundled ArmVirtQemu binaries, then we need a more recent platform
>>> (perhaps CentOS 8, not sure).
>>
>> Unfortunately CentOS 8 is not available as a Docker image, which is a
>> convenient way to build EDK2 in a CI.
> 
> Uh?
> 
>    https://hub.docker.com/_/centos
> 
> seems to disagree with you. Is the 'centos:8' image not suitable
> for some non-obvious reason?

Eh, last time I checked	I had an issue with docker, it was working with 
podman but CI don't provide podman.

I tested again, and now it seems to work, I'll give it another try.
gcc-aarch64-linux-gnu is still older than GCC5 although.

Thanks for the update :)

Phil.



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH-for-5.0] roms/edk2-funcs.sh: Use available GCC for ARM/Aarch64 targets
  2019-12-05 16:50     ` Ard Biesheuvel
@ 2019-12-05 19:35       ` Laszlo Ersek
  2019-12-06  5:07         ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 9+ messages in thread
From: Laszlo Ersek @ 2019-12-05 19:35 UTC (permalink / raw)
  To: Ard Biesheuvel, Philippe Mathieu-Daudé; +Cc: QEMU Developers

On 12/05/19 17:50, Ard Biesheuvel wrote:
> On Thu, 5 Dec 2019 at 16:27, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>
>> On 12/5/19 5:13 PM, Laszlo Ersek wrote:
>>> Hi Phil,
>>>
>>> (+Ard)
>>>
>>> On 12/04/19 23:12, Philippe Mathieu-Daudé wrote:
>>>> Centos 7.7 only provides cross GCC 4.8.5, but the script forces
>>>> us to use GCC5. Since the same machinery is valid to check the
>>>> GCC version, remove the $emulation_target check.
>>>>
>>>>    $ cat /etc/redhat-release
>>>>    CentOS Linux release 7.7.1908 (Core)
>>>>
>>>>    $ aarch64-linux-gnu-gcc -v 2>&1 | tail -1
>>>>    gcc version 4.8.5 20150623 (Red Hat 4.8.5-16) (GCC)
>>>
>>> this patch is not correct, in my opinion. ARM / AARCH64 support in edk2
>>> requires GCC5 as a minimum. It was never tested with an earlier
>>> toolchain, to my understanding. Not on my part, anyway.
>>>
>>> To be more precise: when I tested cross-gcc toolchains earlier than
>>> that, the ArmVirtQemu builds always failed. Minimally, those toolchains
>>> didn't recognize some of the AARCH64 system registers.
>>>
>>> If CentOS 7.7 does not provide a suitable (>=GCC5) toolchain, then we
>>> can't build ArmVirtQemu binaries on CentOS 7.7, in my opinion.
>>>
>>> Personally, on my RHEL7 laptop, over time I've used the following
>>> toolchains, to satisfy the GCC5 requirement of ArmVirtQemu (which
>>> requirement I took as experimental evidence):
>>>
>>> - Initially (last quarter of 2014), I used binary distributions --
>>>    tarballs -- of cross-binutils and cross-gcc, from Linaro.
>>>
>>> - Later (last quarter of 2016), I rebuilt some SRPMs that were at the
>>>    time Fedora-only for RHEL7. Namely:
>>>
>>>    - cross-binutils-2.27-3.fc24
>>>      https://koji.fedoraproject.org/koji/buildinfo?buildID=801348
>>>
>>>    - gcc-6.1.1-2.fc24
>>>      https://koji.fedoraproject.org/koji/buildinfo?buildID=761767
>>>
>>> - Most recently, I've been using cross-binutils updated from EPEL7:
>>>
>>>    - cross-binutils-2.27-9.el7.1
>>>      https://koji.fedoraproject.org/koji/buildinfo?buildID=918474
>>>
>>> To my knowledge, there is still no suitable cross-compiler available on
>>> RHEL7, from any trustworthy RPM repository. So, to this day, I use
>>> gcc-6.1.1-2 for cross-building ArmVirtQemu, on my RHEL7 laptop.
>>>
>>> Again: I believe it does not matter if the gcc-4.8.5-based
>>> cross-compiler in CentOS 7 "happens" to work. That's a compiler that I
>>> have never tested with, or vetted for, upstream ArmVirtQemu.
>>>
>>> Now, I realize that in edk2, we have stuff like
>>>
>>>    GCC48_AARCH64_CC_FLAGS
>>>
>>> in "BaseTools/Conf/tools_def.template" -- coming from commit
>>> 7a9dbf2c94d1 ("BaseTools/Conf/tools_def.template: drop ARM/AARCH support
>>> from GCC46/GCC47", 2019-01-08). That doesn't change the fact that I've
>>> never built or tested ArmVirtQemu with such a compiler. And so this
>>> patch makes me quite uncomfortable.
>>>
>>> If that rules out CentOS 7 as a QEMU project build / CI platform for the
>>> bundled ArmVirtQemu binaries, then we need a more recent platform
>>> (perhaps CentOS 8, not sure).
>>
>> Unfortunately CentOS 8 is not available as a Docker image, which is a
>> convenient way to build EDK2 in a CI.
>>
>>> I think it's also educational to check the origin of the code that your
>>> patch proposes to remove. Most recently it was moved around from a
>>> different place, in QEMU commit 65a109ab4b1a ('roms: lift
>>> "edk2-funcs.sh" from "tests/uefi-test-tools/build.sh"', 2019-04-17).
>>>
>>> In that commit, for some reason I didn't keep the original code comments
>>> (perhaps it would have been too difficult or messy to preserve the
>>> comments sanely with the restructured / factored-out code). But, they
>>> went like this (originally from commit 77db55fc8155,
>>> "tests/uefi-test-tools: add build scripts", 2019-02-21):
>>>
>>> # Expose cross_prefix (which is possibly empty) to the edk2 tools. While at it,
>>> # determine the suitable edk2 toolchain as well.
>>> # - For ARM and AARCH64, edk2 only offers the GCC5 toolchain tag, which covers
>>> #   the gcc-5+ releases.
>>> # - For IA32 and X64, edk2 offers the GCC44 through GCC49 toolchain tags, in
>>> #   addition to GCC5. Unfortunately, the mapping between the toolchain tags and
>>> #   the actual gcc releases isn't entirely trivial. Run "git-blame" on
>>> #   "OvmfPkg/build.sh" in edk2 for more information.
>>> # And, because the above is too simple, we have to assign cross_prefix to an
>>> # edk2 build variable that is specific to both the toolchain tag and the target
>>> # architecture.
>>>
>>> So... unless Ard feels it is really totally safe to retro-actively rely
>>> on the gcc-4.8.5-based compiler in CentOS 7, I'd rather we picked a more
>>> recent build platform (OS) instead. For example, we build ArmVirtQemu on
>>> RHEL8 regularly, so that's a reality-based "plus" for CentOS 8.
>>>
>>>
>>> Independently of all of the above, the OVMF toolchain selection logic
>>> that this patch proposes to reuse with ArmVirtQemu, is *really*
>>> x86-specific. Please run "git blame" on "OvmfPkg/build.sh" in upstream
>>> edk2, to see where the various branches come from (as the comments in
>>> this shell script suggest as well). There had been mess like commit
>>> 656ac0c7d8ea ('Revert "OvmfPkg/build.sh: select the GCC49 toolchain
>>> settings for gcc-7.*"', 2017-08-25).
>>
>> Thanks for all the pointers, very educative indeed :)
>>
>> I'll see other setups I can use with GCC5+ available.
>>
>> I still have to figure if there are free tier CI with less limitations
>> than Travis/Shippable/GitLab, so we can keep the full EDK2 build output log.
>>
> 
> My CI job for ArmVirtQemu/EDK2 build tested GCC48 and GCC49 until very
> recently, and I never experienced any issues when running those
> images, although it's been much longer that I actually tried that. So
> I wouldn't recommend against it, and if we do identify any issues, we
> should either deprecate GCC48 (for ArmVirtQemu or for AArch64
> altogether) or fix them.
> 

OK, thank you, I'm fully satisfied with this addition. :)

Phil, in this case I think we can indeed replace the hard-coded "GCC5"
with a bit of dynamic detection. Two remarks:

- Please CC Ard on v2, so he can ACK. I'd like that. :)

- Again, we shouldn't blindly reapply the x86 (OVMF) quirk(s). I mean
mainly the special casing of "6.[0-2].*" to GCC49, which comes from
upstream edk2 commit 432f1d83f77a ("OvmfPkg/build.sh: Use GCC49
toolchains with GCC 6.[0-2]", 2016-12-06).

... or is that GCC bug target-independent in fact? I can't really tell;
the upstream GCC bug
<https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70955> is ISA-specific
(x86-64), and so are function calling conventions.

I'd suggest *not* applying the quirk for ArmVirtQemu, initially.

Thanks
Laszlo



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH-for-5.0] roms/edk2-funcs.sh: Use available GCC for ARM/Aarch64 targets
  2019-12-05 19:35       ` Laszlo Ersek
@ 2019-12-06  5:07         ` Philippe Mathieu-Daudé
  2019-12-08 17:44           ` dann frazier
  0 siblings, 1 reply; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-12-06  5:07 UTC (permalink / raw)
  To: Laszlo Ersek, Ard Biesheuvel
  Cc: Debian QEMU Team, Serge Hallyn, dann frazier, QEMU Developers,
	Steve Langasek

On 12/5/19 8:35 PM, Laszlo Ersek wrote:
> On 12/05/19 17:50, Ard Biesheuvel wrote:
>> On Thu, 5 Dec 2019 at 16:27, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>>
>>> On 12/5/19 5:13 PM, Laszlo Ersek wrote:
>>>> Hi Phil,
>>>>
>>>> (+Ard)
>>>>
>>>> On 12/04/19 23:12, Philippe Mathieu-Daudé wrote:
>>>>> Centos 7.7 only provides cross GCC 4.8.5, but the script forces
>>>>> us to use GCC5. Since the same machinery is valid to check the
>>>>> GCC version, remove the $emulation_target check.
>>>>>
>>>>>     $ cat /etc/redhat-release
>>>>>     CentOS Linux release 7.7.1908 (Core)
>>>>>
>>>>>     $ aarch64-linux-gnu-gcc -v 2>&1 | tail -1
>>>>>     gcc version 4.8.5 20150623 (Red Hat 4.8.5-16) (GCC)
>>>>
>>>> this patch is not correct, in my opinion. ARM / AARCH64 support in edk2
>>>> requires GCC5 as a minimum. It was never tested with an earlier
>>>> toolchain, to my understanding. Not on my part, anyway.
>>>>
>>>> To be more precise: when I tested cross-gcc toolchains earlier than
>>>> that, the ArmVirtQemu builds always failed. Minimally, those toolchains
>>>> didn't recognize some of the AARCH64 system registers.
>>>>
>>>> If CentOS 7.7 does not provide a suitable (>=GCC5) toolchain, then we
>>>> can't build ArmVirtQemu binaries on CentOS 7.7, in my opinion.
>>>>
>>>> Personally, on my RHEL7 laptop, over time I've used the following
>>>> toolchains, to satisfy the GCC5 requirement of ArmVirtQemu (which
>>>> requirement I took as experimental evidence):
>>>>
>>>> - Initially (last quarter of 2014), I used binary distributions --
>>>>     tarballs -- of cross-binutils and cross-gcc, from Linaro.
>>>>
>>>> - Later (last quarter of 2016), I rebuilt some SRPMs that were at the
>>>>     time Fedora-only for RHEL7. Namely:
>>>>
>>>>     - cross-binutils-2.27-3.fc24
>>>>       https://koji.fedoraproject.org/koji/buildinfo?buildID=801348
>>>>
>>>>     - gcc-6.1.1-2.fc24
>>>>       https://koji.fedoraproject.org/koji/buildinfo?buildID=761767
>>>>
>>>> - Most recently, I've been using cross-binutils updated from EPEL7:
>>>>
>>>>     - cross-binutils-2.27-9.el7.1
>>>>       https://koji.fedoraproject.org/koji/buildinfo?buildID=918474
>>>>
>>>> To my knowledge, there is still no suitable cross-compiler available on
>>>> RHEL7, from any trustworthy RPM repository. So, to this day, I use
>>>> gcc-6.1.1-2 for cross-building ArmVirtQemu, on my RHEL7 laptop.
>>>>
>>>> Again: I believe it does not matter if the gcc-4.8.5-based
>>>> cross-compiler in CentOS 7 "happens" to work. That's a compiler that I
>>>> have never tested with, or vetted for, upstream ArmVirtQemu.
>>>>
>>>> Now, I realize that in edk2, we have stuff like
>>>>
>>>>     GCC48_AARCH64_CC_FLAGS
>>>>
>>>> in "BaseTools/Conf/tools_def.template" -- coming from commit
>>>> 7a9dbf2c94d1 ("BaseTools/Conf/tools_def.template: drop ARM/AARCH support
>>>> from GCC46/GCC47", 2019-01-08). That doesn't change the fact that I've
>>>> never built or tested ArmVirtQemu with such a compiler. And so this
>>>> patch makes me quite uncomfortable.
>>>>
>>>> If that rules out CentOS 7 as a QEMU project build / CI platform for the
>>>> bundled ArmVirtQemu binaries, then we need a more recent platform
>>>> (perhaps CentOS 8, not sure).
>>>
>>> Unfortunately CentOS 8 is not available as a Docker image, which is a
>>> convenient way to build EDK2 in a CI.
>>>
>>>> I think it's also educational to check the origin of the code that your
>>>> patch proposes to remove. Most recently it was moved around from a
>>>> different place, in QEMU commit 65a109ab4b1a ('roms: lift
>>>> "edk2-funcs.sh" from "tests/uefi-test-tools/build.sh"', 2019-04-17).
>>>>
>>>> In that commit, for some reason I didn't keep the original code comments
>>>> (perhaps it would have been too difficult or messy to preserve the
>>>> comments sanely with the restructured / factored-out code). But, they
>>>> went like this (originally from commit 77db55fc8155,
>>>> "tests/uefi-test-tools: add build scripts", 2019-02-21):
>>>>
>>>> # Expose cross_prefix (which is possibly empty) to the edk2 tools. While at it,
>>>> # determine the suitable edk2 toolchain as well.
>>>> # - For ARM and AARCH64, edk2 only offers the GCC5 toolchain tag, which covers
>>>> #   the gcc-5+ releases.
>>>> # - For IA32 and X64, edk2 offers the GCC44 through GCC49 toolchain tags, in
>>>> #   addition to GCC5. Unfortunately, the mapping between the toolchain tags and
>>>> #   the actual gcc releases isn't entirely trivial. Run "git-blame" on
>>>> #   "OvmfPkg/build.sh" in edk2 for more information.
>>>> # And, because the above is too simple, we have to assign cross_prefix to an
>>>> # edk2 build variable that is specific to both the toolchain tag and the target
>>>> # architecture.
>>>>
>>>> So... unless Ard feels it is really totally safe to retro-actively rely
>>>> on the gcc-4.8.5-based compiler in CentOS 7, I'd rather we picked a more
>>>> recent build platform (OS) instead. For example, we build ArmVirtQemu on
>>>> RHEL8 regularly, so that's a reality-based "plus" for CentOS 8.
>>>>
>>>>
>>>> Independently of all of the above, the OVMF toolchain selection logic
>>>> that this patch proposes to reuse with ArmVirtQemu, is *really*
>>>> x86-specific. Please run "git blame" on "OvmfPkg/build.sh" in upstream
>>>> edk2, to see where the various branches come from (as the comments in
>>>> this shell script suggest as well). There had been mess like commit
>>>> 656ac0c7d8ea ('Revert "OvmfPkg/build.sh: select the GCC49 toolchain
>>>> settings for gcc-7.*"', 2017-08-25).
>>>
>>> Thanks for all the pointers, very educative indeed :)
>>>
>>> I'll see other setups I can use with GCC5+ available.
>>>
>>> I still have to figure if there are free tier CI with less limitations
>>> than Travis/Shippable/GitLab, so we can keep the full EDK2 build output log.
>>>
>>
>> My CI job for ArmVirtQemu/EDK2 build tested GCC48 and GCC49 until very
>> recently, and I never experienced any issues when running those
>> images, although it's been much longer that I actually tried that. So
>> I wouldn't recommend against it, and if we do identify any issues, we
>> should either deprecate GCC48 (for ArmVirtQemu or for AArch64
>> altogether) or fix them.

FYI Debian/Ubuntu apparently force to GCC49:
https://salsa.debian.org/qemu-team/edk2/blob/debian/debian/rules#L9

> 
> OK, thank you, I'm fully satisfied with this addition. :)
> 
> Phil, in this case I think we can indeed replace the hard-coded "GCC5"
> with a bit of dynamic detection. Two remarks:
> 
> - Please CC Ard on v2, so he can ACK. I'd like that. :)
> 
> - Again, we shouldn't blindly reapply the x86 (OVMF) quirk(s). I mean
> mainly the special casing of "6.[0-2].*" to GCC49, which comes from
> upstream edk2 commit 432f1d83f77a ("OvmfPkg/build.sh: Use GCC49
> toolchains with GCC 6.[0-2]", 2016-12-06).
> 
> ... or is that GCC bug target-independent in fact? I can't really tell;
> the upstream GCC bug
> <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70955> is ISA-specific
> (x86-64), and so are function calling conventions.
> 
> I'd suggest *not* applying the quirk for ArmVirtQemu, initially.
> 
> Thanks
> Laszlo
> 



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH-for-5.0] roms/edk2-funcs.sh: Use available GCC for ARM/Aarch64 targets
  2019-12-06  5:07         ` Philippe Mathieu-Daudé
@ 2019-12-08 17:44           ` dann frazier
  0 siblings, 0 replies; 9+ messages in thread
From: dann frazier @ 2019-12-08 17:44 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Ard Biesheuvel, Serge Hallyn, QEMU Developers, Steve Langasek,
	Debian QEMU Team, Laszlo Ersek

On Fri, Dec 06, 2019 at 06:07:58AM +0100, Philippe Mathieu-Daudé wrote:
> On 12/5/19 8:35 PM, Laszlo Ersek wrote:
> > On 12/05/19 17:50, Ard Biesheuvel wrote:
> > > On Thu, 5 Dec 2019 at 16:27, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> > > > 
> > > > On 12/5/19 5:13 PM, Laszlo Ersek wrote:
> > > > > Hi Phil,
> > > > > 
> > > > > (+Ard)
> > > > > 
> > > > > On 12/04/19 23:12, Philippe Mathieu-Daudé wrote:
> > > > > > Centos 7.7 only provides cross GCC 4.8.5, but the script forces
> > > > > > us to use GCC5. Since the same machinery is valid to check the
> > > > > > GCC version, remove the $emulation_target check.
> > > > > > 
> > > > > >     $ cat /etc/redhat-release
> > > > > >     CentOS Linux release 7.7.1908 (Core)
> > > > > > 
> > > > > >     $ aarch64-linux-gnu-gcc -v 2>&1 | tail -1
> > > > > >     gcc version 4.8.5 20150623 (Red Hat 4.8.5-16) (GCC)
> > > > > 
> > > > > this patch is not correct, in my opinion. ARM / AARCH64 support in edk2
> > > > > requires GCC5 as a minimum. It was never tested with an earlier
> > > > > toolchain, to my understanding. Not on my part, anyway.
> > > > > 
> > > > > To be more precise: when I tested cross-gcc toolchains earlier than
> > > > > that, the ArmVirtQemu builds always failed. Minimally, those toolchains
> > > > > didn't recognize some of the AARCH64 system registers.
> > > > > 
> > > > > If CentOS 7.7 does not provide a suitable (>=GCC5) toolchain, then we
> > > > > can't build ArmVirtQemu binaries on CentOS 7.7, in my opinion.
> > > > > 
> > > > > Personally, on my RHEL7 laptop, over time I've used the following
> > > > > toolchains, to satisfy the GCC5 requirement of ArmVirtQemu (which
> > > > > requirement I took as experimental evidence):
> > > > > 
> > > > > - Initially (last quarter of 2014), I used binary distributions --
> > > > >     tarballs -- of cross-binutils and cross-gcc, from Linaro.
> > > > > 
> > > > > - Later (last quarter of 2016), I rebuilt some SRPMs that were at the
> > > > >     time Fedora-only for RHEL7. Namely:
> > > > > 
> > > > >     - cross-binutils-2.27-3.fc24
> > > > >       https://koji.fedoraproject.org/koji/buildinfo?buildID=801348
> > > > > 
> > > > >     - gcc-6.1.1-2.fc24
> > > > >       https://koji.fedoraproject.org/koji/buildinfo?buildID=761767
> > > > > 
> > > > > - Most recently, I've been using cross-binutils updated from EPEL7:
> > > > > 
> > > > >     - cross-binutils-2.27-9.el7.1
> > > > >       https://koji.fedoraproject.org/koji/buildinfo?buildID=918474
> > > > > 
> > > > > To my knowledge, there is still no suitable cross-compiler available on
> > > > > RHEL7, from any trustworthy RPM repository. So, to this day, I use
> > > > > gcc-6.1.1-2 for cross-building ArmVirtQemu, on my RHEL7 laptop.
> > > > > 
> > > > > Again: I believe it does not matter if the gcc-4.8.5-based
> > > > > cross-compiler in CentOS 7 "happens" to work. That's a compiler that I
> > > > > have never tested with, or vetted for, upstream ArmVirtQemu.
> > > > > 
> > > > > Now, I realize that in edk2, we have stuff like
> > > > > 
> > > > >     GCC48_AARCH64_CC_FLAGS
> > > > > 
> > > > > in "BaseTools/Conf/tools_def.template" -- coming from commit
> > > > > 7a9dbf2c94d1 ("BaseTools/Conf/tools_def.template: drop ARM/AARCH support
> > > > > from GCC46/GCC47", 2019-01-08). That doesn't change the fact that I've
> > > > > never built or tested ArmVirtQemu with such a compiler. And so this
> > > > > patch makes me quite uncomfortable.
> > > > > 
> > > > > If that rules out CentOS 7 as a QEMU project build / CI platform for the
> > > > > bundled ArmVirtQemu binaries, then we need a more recent platform
> > > > > (perhaps CentOS 8, not sure).
> > > > 
> > > > Unfortunately CentOS 8 is not available as a Docker image, which is a
> > > > convenient way to build EDK2 in a CI.
> > > > 
> > > > > I think it's also educational to check the origin of the code that your
> > > > > patch proposes to remove. Most recently it was moved around from a
> > > > > different place, in QEMU commit 65a109ab4b1a ('roms: lift
> > > > > "edk2-funcs.sh" from "tests/uefi-test-tools/build.sh"', 2019-04-17).
> > > > > 
> > > > > In that commit, for some reason I didn't keep the original code comments
> > > > > (perhaps it would have been too difficult or messy to preserve the
> > > > > comments sanely with the restructured / factored-out code). But, they
> > > > > went like this (originally from commit 77db55fc8155,
> > > > > "tests/uefi-test-tools: add build scripts", 2019-02-21):
> > > > > 
> > > > > # Expose cross_prefix (which is possibly empty) to the edk2 tools. While at it,
> > > > > # determine the suitable edk2 toolchain as well.
> > > > > # - For ARM and AARCH64, edk2 only offers the GCC5 toolchain tag, which covers
> > > > > #   the gcc-5+ releases.
> > > > > # - For IA32 and X64, edk2 offers the GCC44 through GCC49 toolchain tags, in
> > > > > #   addition to GCC5. Unfortunately, the mapping between the toolchain tags and
> > > > > #   the actual gcc releases isn't entirely trivial. Run "git-blame" on
> > > > > #   "OvmfPkg/build.sh" in edk2 for more information.
> > > > > # And, because the above is too simple, we have to assign cross_prefix to an
> > > > > # edk2 build variable that is specific to both the toolchain tag and the target
> > > > > # architecture.
> > > > > 
> > > > > So... unless Ard feels it is really totally safe to retro-actively rely
> > > > > on the gcc-4.8.5-based compiler in CentOS 7, I'd rather we picked a more
> > > > > recent build platform (OS) instead. For example, we build ArmVirtQemu on
> > > > > RHEL8 regularly, so that's a reality-based "plus" for CentOS 8.
> > > > > 
> > > > > 
> > > > > Independently of all of the above, the OVMF toolchain selection logic
> > > > > that this patch proposes to reuse with ArmVirtQemu, is *really*
> > > > > x86-specific. Please run "git blame" on "OvmfPkg/build.sh" in upstream
> > > > > edk2, to see where the various branches come from (as the comments in
> > > > > this shell script suggest as well). There had been mess like commit
> > > > > 656ac0c7d8ea ('Revert "OvmfPkg/build.sh: select the GCC49 toolchain
> > > > > settings for gcc-7.*"', 2017-08-25).
> > > > 
> > > > Thanks for all the pointers, very educative indeed :)
> > > > 
> > > > I'll see other setups I can use with GCC5+ available.
> > > > 
> > > > I still have to figure if there are free tier CI with less limitations
> > > > than Travis/Shippable/GitLab, so we can keep the full EDK2 build output log.
> > > > 
> > > 
> > > My CI job for ArmVirtQemu/EDK2 build tested GCC48 and GCC49 until very
> > > recently, and I never experienced any issues when running those
> > > images, although it's been much longer that I actually tried that. So
> > > I wouldn't recommend against it, and if we do identify any issues, we
> > > should either deprecate GCC48 (for ArmVirtQemu or for AArch64
> > > altogether) or fix them.
> 
> FYI Debian/Ubuntu apparently force to GCC49:
> https://salsa.debian.org/qemu-team/edk2/blob/debian/debian/rules#L9

fyi, that was to avoid shipping pre-compiled binaries:
https://salsa.debian.org/qemu-team/edk2/commit/d46a08968a1609725215f32230ab1ddd35d8b7fb

But I just uploaded a new version w/ this change that gets around it:
https://salsa.debian.org/qemu-team/edk2/commit/bfe4fc20408f35ffaa196817b57df11981eef620

  -dann

> > 
> > OK, thank you, I'm fully satisfied with this addition. :)
> > 
> > Phil, in this case I think we can indeed replace the hard-coded "GCC5"
> > with a bit of dynamic detection. Two remarks:
> > 
> > - Please CC Ard on v2, so he can ACK. I'd like that. :)
> > 
> > - Again, we shouldn't blindly reapply the x86 (OVMF) quirk(s). I mean
> > mainly the special casing of "6.[0-2].*" to GCC49, which comes from
> > upstream edk2 commit 432f1d83f77a ("OvmfPkg/build.sh: Use GCC49
> > toolchains with GCC 6.[0-2]", 2016-12-06).
> > 
> > ... or is that GCC bug target-independent in fact? I can't really tell;
> > the upstream GCC bug
> > <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70955> is ISA-specific
> > (x86-64), and so are function calling conventions.
> > 
> > I'd suggest *not* applying the quirk for ArmVirtQemu, initially.
> > 
> > Thanks
> > Laszlo
> > 
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-12-08 22:10 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-04 22:12 [PATCH-for-5.0] roms/edk2-funcs.sh: Use available GCC for ARM/Aarch64 targets Philippe Mathieu-Daudé
2019-12-05 16:13 ` Laszlo Ersek
2019-12-05 16:27   ` Philippe Mathieu-Daudé
2019-12-05 16:47     ` Andrea Bolognani
2019-12-05 17:04       ` Philippe Mathieu-Daudé
2019-12-05 16:50     ` Ard Biesheuvel
2019-12-05 19:35       ` Laszlo Ersek
2019-12-06  5:07         ` Philippe Mathieu-Daudé
2019-12-08 17:44           ` dann frazier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).