diff mbox series

[2/3] tools/firmware: Build firmware as -ffreestanding

Message ID 20210225203010.11378-3-andrew.cooper3@citrix.com (mailing list archive)
State New
Headers show
Series Build firmware as freestanding | expand

Commit Message

Andrew Cooper Feb. 25, 2021, 8:30 p.m. UTC
firmware should always have been -ffreestanding, as it doesn't execute in the
host environment.

inttypes.h isn't a freestanding header, but the 32bitbios_support.c only wants
the stdint.h types so switch to the more appropriate include.

This removes the build time dependency on a 32bit libc just to compile the
hvmloader and friends.

Update README and the TravisCI configuration.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Ian Jackson <iwj@xenproject.org>

For 4.15.  Build tested in Travis (Ubuntu) and XenServer (CentOS) - no change
in the compiled HVMLoader binary.  I'm currently rebuilding the containers
locally to check Arch, Debian and OpenSUSE, but don't anticipate any problems.

This does not resolve the build issue on Alpine.  Exactly what to do there is
still TBC, but Roger has opened a bug with Apline concerning their GCC
packaging.
---
 .travis.yml                                  | 1 -
 README                                       | 3 ---
 tools/firmware/Rules.mk                      | 2 +-
 tools/firmware/hvmloader/32bitbios_support.c | 2 +-
 4 files changed, 2 insertions(+), 6 deletions(-)

Comments

Jan Beulich Feb. 26, 2021, 9:10 a.m. UTC | #1
On 25.02.2021 21:30, Andrew Cooper wrote:
> firmware should always have been -ffreestanding, as it doesn't execute in the
> host environment.
> 
> inttypes.h isn't a freestanding header, but the 32bitbios_support.c only wants
> the stdint.h types so switch to the more appropriate include.
> 
> This removes the build time dependency on a 32bit libc just to compile the
> hvmloader and friends.
> 
> Update README and the TravisCI configuration.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
preferably with one further adjustment:

> --- a/tools/firmware/Rules.mk
> +++ b/tools/firmware/Rules.mk
> @@ -16,4 +16,4 @@ CFLAGS += -Werror
>  $(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
>  
>  # Extra CFLAGS suitable for an embedded type of environment.
> -CFLAGS += -fno-builtin -msoft-float
> +CFLAGS += -fno-builtin -msoft-float -ffreestanding

As per gcc doc -ffreestanding implies -fno-builtin, so I think you
want to replace that one instead of adding the new option on top.

Jan
Andrew Cooper Feb. 26, 2021, 12:42 p.m. UTC | #2
On 26/02/2021 09:10, Jan Beulich wrote:
> On 25.02.2021 21:30, Andrew Cooper wrote:
>> firmware should always have been -ffreestanding, as it doesn't execute in the
>> host environment.
>>
>> inttypes.h isn't a freestanding header, but the 32bitbios_support.c only wants
>> the stdint.h types so switch to the more appropriate include.
>>
>> This removes the build time dependency on a 32bit libc just to compile the
>> hvmloader and friends.
>>
>> Update README and the TravisCI configuration.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> preferably with one further adjustment:
>
>> --- a/tools/firmware/Rules.mk
>> +++ b/tools/firmware/Rules.mk
>> @@ -16,4 +16,4 @@ CFLAGS += -Werror
>>  $(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
>>  
>>  # Extra CFLAGS suitable for an embedded type of environment.
>> -CFLAGS += -fno-builtin -msoft-float
>> +CFLAGS += -fno-builtin -msoft-float -ffreestanding
> As per gcc doc -ffreestanding implies -fno-builtin, so I think you
> want to replace that one instead of adding the new option on top.

Oops yes - fixed.

Thanks.

~Andrew
Ian Jackson March 1, 2021, 4:13 p.m. UTC | #3
Andrew Cooper writes ("[PATCH 2/3] tools/firmware: Build firmware as -ffreestanding"):
> firmware should always have been -ffreestanding, as it doesn't execute in the
> host environment.
> 
> inttypes.h isn't a freestanding header, but the 32bitbios_support.c only wants
> the stdint.h types so switch to the more appropriate include.
> 
> This removes the build time dependency on a 32bit libc just to compile the
> hvmloader and friends.
> 
> Update README and the TravisCI configuration.

> For 4.15.  Build tested in Travis (Ubuntu) and XenServer (CentOS) - no change
> in the compiled HVMLoader binary.  I'm currently rebuilding the containers
> locally to check Arch, Debian and OpenSUSE, but don't anticipate any problems.

Release-Acked-by: Ian Jackson <iwj@xenproject.org>
Reviewed-by: Ian Jackson <iwj@xenproject.org>
diff mbox series

Patch

diff --git a/.travis.yml b/.travis.yml
index 15ca9e9047..2362475f7a 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -58,7 +58,6 @@  addons:
             - acpica-tools
             - bin86
             - bcc
-            - libc6-dev-i386
             - libnl-3-dev
             - ocaml-nox
             - libfindlib-ocaml-dev
diff --git a/README b/README
index 33cdf6b826..5167bb1708 100644
--- a/README
+++ b/README
@@ -62,9 +62,6 @@  provided by your OS distributor:
     * GNU bison and GNU flex
     * GNU gettext
     * ACPI ASL compiler (iasl)
-    * Libc multiarch package (e.g. libc6-dev-i386 / glibc-devel.i686).
-      Required when building on a 64-bit platform to build
-      32-bit components which are enabled on a default build.
 
 In addition to the above there are a number of optional build
 prerequisites. Omitting these will cause the related features to be
diff --git a/tools/firmware/Rules.mk b/tools/firmware/Rules.mk
index 26bbddccd4..93abcabc67 100644
--- a/tools/firmware/Rules.mk
+++ b/tools/firmware/Rules.mk
@@ -16,4 +16,4 @@  CFLAGS += -Werror
 $(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
 
 # Extra CFLAGS suitable for an embedded type of environment.
-CFLAGS += -fno-builtin -msoft-float
+CFLAGS += -fno-builtin -msoft-float -ffreestanding
diff --git a/tools/firmware/hvmloader/32bitbios_support.c b/tools/firmware/hvmloader/32bitbios_support.c
index 6f28fb6bde..cee3804888 100644
--- a/tools/firmware/hvmloader/32bitbios_support.c
+++ b/tools/firmware/hvmloader/32bitbios_support.c
@@ -20,7 +20,7 @@ 
  * this program; If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include <inttypes.h>
+#include <stdint.h>
 #include <xen/libelf/elfstructs.h>
 
 #include "util.h"