[PATCH for-4.15 0/3] Build firmware as freestanding

[PATCH for-4.15 0/3] Build firmware as freestanding

[Andrew Cooper]

This fixes a bug we've had for ages, which even ended up being documented as
an inappropriate build dependency.

For 4.15.  I'm tempted to suggest that it wants backporting to the stable
branches as well.

Andrew Cooper (3):
  tools/hvmloader: Drop machelf include as well
  tools/firmware: Build firmware as -ffreestanding
  automation: Annotate that a 32bit libc is no longer a dependency

 .travis.yml                                          | 1 -
 README                                               | 3 ---
 automation/build/archlinux/current.dockerfile        | 1 +
 automation/build/centos/7.2.dockerfile               | 1 +
 automation/build/centos/7.dockerfile                 | 1 +
 automation/build/debian/jessie.dockerfile            | 1 +
 automation/build/debian/stretch.dockerfile           | 1 +
 automation/build/debian/unstable.dockerfile          | 1 +
 automation/build/fedora/29.dockerfile                | 1 +
 automation/build/suse/opensuse-leap.dockerfile       | 1 +
 automation/build/suse/opensuse-tumbleweed.dockerfile | 1 +
 automation/build/ubuntu/bionic.dockerfile            | 1 +
 automation/build/ubuntu/focal.dockerfile             | 1 +
 automation/build/ubuntu/trusty.dockerfile            | 1 +
 automation/build/ubuntu/xenial.dockerfile            | 1 +
 tools/firmware/Rules.mk                              | 2 +-
 tools/firmware/hvmloader/32bitbios_support.c         | 5 +----
 17 files changed, 15 insertions(+), 9 deletions(-)

-- 
2.11.0

[PATCH 1/3] tools/hvmloader: Drop machelf include as well

[Andrew Cooper]

The logic behind switching to elfstructs applies to sun builds as well.

Fixes: 81b2b328a2 ("hvmloader: use Xen private header for elf structs")
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>
---
 tools/firmware/hvmloader/32bitbios_support.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/tools/firmware/hvmloader/32bitbios_support.c b/tools/firmware/hvmloader/32bitbios_support.c
index e726946a7b..6f28fb6bde 100644
--- a/tools/firmware/hvmloader/32bitbios_support.c
+++ b/tools/firmware/hvmloader/32bitbios_support.c
@@ -22,9 +22,6 @@
 
 #include <inttypes.h>
 #include <xen/libelf/elfstructs.h>
-#ifdef __sun__
-#include <sys/machelf.h>
-#endif
 
 #include "util.h"
 #include "config.h"
-- 
2.11.0

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

[Andrew Cooper]

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(-)

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"
-- 
2.11.0

[PATCH 3/3] automation: Annotate that a 32bit libc is no longer a dependency

[Andrew Cooper]

We can't drop the 32bit libc from the existing containers, because they are
used on older Xen branches as well.

However, we can avoid the dependency being propagated into newer conainers
derived from our dockerfiles.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Doug Goldstein <cardoe@cardoe.com>
CC: Wei Liu <wl@xen.org>
CC: Ian Jackson <iwj@xenproject.org>

For 4.15.  Documentation changes only
---
 automation/build/archlinux/current.dockerfile        | 1 +
 automation/build/centos/7.2.dockerfile               | 1 +
 automation/build/centos/7.dockerfile                 | 1 +
 automation/build/debian/jessie.dockerfile            | 1 +
 automation/build/debian/stretch.dockerfile           | 1 +
 automation/build/debian/unstable.dockerfile          | 1 +
 automation/build/fedora/29.dockerfile                | 1 +
 automation/build/suse/opensuse-leap.dockerfile       | 1 +
 automation/build/suse/opensuse-tumbleweed.dockerfile | 1 +
 automation/build/ubuntu/bionic.dockerfile            | 1 +
 automation/build/ubuntu/focal.dockerfile             | 1 +
 automation/build/ubuntu/trusty.dockerfile            | 1 +
 automation/build/ubuntu/xenial.dockerfile            | 1 +
 13 files changed, 13 insertions(+)

diff --git a/automation/build/archlinux/current.dockerfile b/automation/build/archlinux/current.dockerfile
index d8fbebaf79..d46fc9d9ca 100644
--- a/automation/build/archlinux/current.dockerfile
+++ b/automation/build/archlinux/current.dockerfile
@@ -20,6 +20,7 @@ RUN pacman -S --refresh --sysupgrade --noconfirm --noprogressbar --needed \
         iasl \
         inetutils \
         iproute \
+        # lib32-glibc for Xen < 4.15
         lib32-glibc \
         libaio \
         libcacard \
diff --git a/automation/build/centos/7.2.dockerfile b/automation/build/centos/7.2.dockerfile
index c2f46b694c..af672a0be1 100644
--- a/automation/build/centos/7.2.dockerfile
+++ b/automation/build/centos/7.2.dockerfile
@@ -34,6 +34,7 @@ RUN rpm --rebuilddb && \
         yajl-devel \
         pixman-devel \
         glibc-devel \
+        # glibc-devel.i686 for Xen < 4.15
         glibc-devel.i686 \
         make \
         binutils \
diff --git a/automation/build/centos/7.dockerfile b/automation/build/centos/7.dockerfile
index e37d9d743a..5f83c97d0c 100644
--- a/automation/build/centos/7.dockerfile
+++ b/automation/build/centos/7.dockerfile
@@ -32,6 +32,7 @@ RUN yum -y install \
         yajl-devel \
         pixman-devel \
         glibc-devel \
+        # glibc-devel.i686 for Xen < 4.15
         glibc-devel.i686 \
         make \
         binutils \
diff --git a/automation/build/debian/jessie.dockerfile b/automation/build/debian/jessie.dockerfile
index 1232b9e204..808d6272e4 100644
--- a/automation/build/debian/jessie.dockerfile
+++ b/automation/build/debian/jessie.dockerfile
@@ -31,6 +31,7 @@ RUN apt-get update && \
         bin86 \
         bcc \
         liblzma-dev \
+        # libc6-dev-i386 for Xen < 4.15
         libc6-dev-i386 \
         libnl-3-dev \
         ocaml-nox \
diff --git a/automation/build/debian/stretch.dockerfile b/automation/build/debian/stretch.dockerfile
index 32742f7f39..e3bace1f87 100644
--- a/automation/build/debian/stretch.dockerfile
+++ b/automation/build/debian/stretch.dockerfile
@@ -32,6 +32,7 @@ RUN apt-get update && \
         bin86 \
         bcc \
         liblzma-dev \
+        # libc6-dev-i386 for Xen < 4.15
         libc6-dev-i386 \
         libnl-3-dev \
         ocaml-nox \
diff --git a/automation/build/debian/unstable.dockerfile b/automation/build/debian/unstable.dockerfile
index aeb4f3448b..9a10ee08d6 100644
--- a/automation/build/debian/unstable.dockerfile
+++ b/automation/build/debian/unstable.dockerfile
@@ -32,6 +32,7 @@ RUN apt-get update && \
         bin86 \
         bcc \
         liblzma-dev \
+        # libc6-dev-i386 for Xen < 4.15
         libc6-dev-i386 \
         libnl-3-dev \
         ocaml-nox \
diff --git a/automation/build/fedora/29.dockerfile b/automation/build/fedora/29.dockerfile
index 6a4e5b0413..5482952523 100644
--- a/automation/build/fedora/29.dockerfile
+++ b/automation/build/fedora/29.dockerfile
@@ -25,6 +25,7 @@ RUN dnf -y install \
         yajl-devel \
         pixman-devel \
         glibc-devel \
+        # glibc-devel.i686 for Xen < 4.15
         glibc-devel.i686 \
         make \
         binutils \
diff --git a/automation/build/suse/opensuse-leap.dockerfile b/automation/build/suse/opensuse-leap.dockerfile
index c60c13c943..685dd5d7fd 100644
--- a/automation/build/suse/opensuse-leap.dockerfile
+++ b/automation/build/suse/opensuse-leap.dockerfile
@@ -26,6 +26,7 @@ RUN zypper install -y --no-recommends \
         git \
         glib2-devel \
         glibc-devel \
+        # glibc-devel-32bit for Xen < 4.15
         glibc-devel-32bit \
         gzip \
         hostname \
diff --git a/automation/build/suse/opensuse-tumbleweed.dockerfile b/automation/build/suse/opensuse-tumbleweed.dockerfile
index 084cce0921..061173e751 100644
--- a/automation/build/suse/opensuse-tumbleweed.dockerfile
+++ b/automation/build/suse/opensuse-tumbleweed.dockerfile
@@ -26,6 +26,7 @@ RUN zypper install -y --no-recommends \
         git \
         glib2-devel \
         glibc-devel \
+        # glibc-devel-32bit for Xen < 4.15
         glibc-devel-32bit \
         gzip \
         hostname \
diff --git a/automation/build/ubuntu/bionic.dockerfile b/automation/build/ubuntu/bionic.dockerfile
index 712b2e4722..408063698c 100644
--- a/automation/build/ubuntu/bionic.dockerfile
+++ b/automation/build/ubuntu/bionic.dockerfile
@@ -32,6 +32,7 @@ RUN apt-get update && \
         bin86 \
         bcc \
         liblzma-dev \
+        # libc6-dev-i386 for Xen < 4.15
         libc6-dev-i386 \
         libnl-3-dev \
         ocaml-nox \
diff --git a/automation/build/ubuntu/focal.dockerfile b/automation/build/ubuntu/focal.dockerfile
index c1c1f8d58f..90b4001a6a 100644
--- a/automation/build/ubuntu/focal.dockerfile
+++ b/automation/build/ubuntu/focal.dockerfile
@@ -31,6 +31,7 @@ RUN apt-get update && \
         bin86 \
         bcc \
         liblzma-dev \
+        # libc6-dev-i386 for Xen < 4.15
         libc6-dev-i386 \
         libnl-3-dev \
         ocaml-nox \
diff --git a/automation/build/ubuntu/trusty.dockerfile b/automation/build/ubuntu/trusty.dockerfile
index 397a28061d..fd377d948f 100644
--- a/automation/build/ubuntu/trusty.dockerfile
+++ b/automation/build/ubuntu/trusty.dockerfile
@@ -32,6 +32,7 @@ RUN apt-get update && \
         bin86 \
         bcc \
         liblzma-dev \
+        # libc6-dev-i386 for Xen < 4.15
         libc6-dev-i386 \
         libnl-3-dev \
         ocaml-nox \
diff --git a/automation/build/ubuntu/xenial.dockerfile b/automation/build/ubuntu/xenial.dockerfile
index ce0e84fa2f..57a71eb8c6 100644
--- a/automation/build/ubuntu/xenial.dockerfile
+++ b/automation/build/ubuntu/xenial.dockerfile
@@ -32,6 +32,7 @@ RUN apt-get update && \
         bin86 \
         bcc \
         liblzma-dev \
+        # libc6-dev-i386 for Xen < 4.15
         libc6-dev-i386 \
         libnl-3-dev \
         ocaml-nox \
-- 
2.11.0

Re: [PATCH 1/3] tools/hvmloader: Drop machelf include as well

[Jan Beulich]

On 25.02.2021 21:30, Andrew Cooper wrote:
> The logic behind switching to elfstructs applies to sun builds as well.
> 
> Fixes: 81b2b328a2 ("hvmloader: use Xen private header for elf structs")
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

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

[Jan Beulich]

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

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

[Andrew Cooper]

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

Re: [PATCH 3/3] automation: Annotate that a 32bit libc is no longer a dependency

[Ian Jackson]

Andrew Cooper writes ("[PATCH 3/3] automation: Annotate that a 32bit libc is no longer a dependency"):
> We can't drop the 32bit libc from the existing containers, because they are
> used on older Xen branches as well.
> 
> However, we can avoid the dependency being propagated into newer conainers
> derived from our dockerfiles.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Doug Goldstein <cardoe@cardoe.com>
> CC: Wei Liu <wl@xen.org>
> CC: Ian Jackson <iwj@xenproject.org>
> 
> For 4.15.  Documentation changes only

Release-Acked-by: Ian Jackson <iwj@xenproject.org>

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

[Ian Jackson]

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>

Re: [PATCH 1/3] tools/hvmloader: Drop machelf include as well

[Ian Jackson]

Andrew Cooper writes ("[PATCH 1/3] tools/hvmloader: Drop machelf include as well"):
> The logic behind switching to elfstructs applies to sun builds as well.
> 
> Fixes: 81b2b328a2 ("hvmloader: use Xen private header for elf structs")
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

I would have preferred this patch to come after the one that is
actually justified for 4.15, so that it could be held off to after
4.15.  After all I see no substantive reason why this should get a
freeze exception.

However, it looks fine based on code review and I don't want to add
risk by asking you to transpose the two patches, so:

Release-Acked-by: Ian Jackson <iwj@xenproject.org>

Ian.

Re: [PATCH 1/3] tools/hvmloader: Drop machelf include as well

[Andrew Cooper]

On 01/03/2021 16:16, Ian Jackson wrote:
> Andrew Cooper writes ("[PATCH 1/3] tools/hvmloader: Drop machelf include as well"):
>> The logic behind switching to elfstructs applies to sun builds as well.
>>
>> Fixes: 81b2b328a2 ("hvmloader: use Xen private header for elf structs")
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> I would have preferred this patch to come after the one that is
> actually justified for 4.15, so that it could be held off to after
> 4.15.  After all I see no substantive reason why this should get a
> freeze exception.

It is a bugfix to a very recent patch which already already got a
release ack.

If this doesn't get an ack, then 81b2b328a2 should be reverted.  (Except
obviously that would be a bad move.)

>
> However, it looks fine based on code review and I don't want to add
> risk by asking you to transpose the two patches, so:
>
> Release-Acked-by: Ian Jackson <iwj@xenproject.org>

Thanks.

~Andrew