diff mbox series

[v2,1/1] sandbox: allow cross-compiling sandbox

Message ID 20210210175425.264251-1-xypron.glpk@gmx.de
State Accepted
Commit f7691a6d736bec7915c227ac14076f9993a27367
Delegated to: Simon Glass
Headers show
Series [v2,1/1] sandbox: allow cross-compiling sandbox | expand

Commit Message

Heinrich Schuchardt Feb. 10, 2021, 5:54 p.m. UTC
UEFI test files like helloworld.efi require an architecture specific
PE-COFF header.

Currently this does not work for cross compiling. If $CROSS_COMPILE is set,
use the first part of the architecture triplet from the variable to
choose the PE-COFF header.

Now we can cross-compile the sandbox, e.g.

    make sandbox_defconfig NO_SDL=1
    CROSS_COMPILE=/opt/bin/aarch64-linux-gnu- NO_SDL=1 MK_ARCH=aarch64 make

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v2:
	use $CROSS_COMPILE instead of an extra environment variable
---
 Makefile | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

--
2.30.0

Comments

Sean Anderson Feb. 10, 2021, 6:20 p.m. UTC | #1
On 2/10/21 12:54 PM, Heinrich Schuchardt wrote:
 > UEFI test files like helloworld.efi require an architecture specific
 > PE-COFF header.
 >
 > Currently this does not work for cross compiling. If $CROSS_COMPILE 
is set,
 > use the first part of the architecture triplet from the variable to
 > choose the PE-COFF header.
 >
 > Now we can cross-compile the sandbox, e.g.
 >
 >      make sandbox_defconfig NO_SDL=1
 >      CROSS_COMPILE=/opt/bin/aarch64-linux-gnu- NO_SDL=1 
MK_ARCH=aarch64 make
 >
 > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
 > ---
 > v2:
 > 	use $CROSS_COMPILE instead of an extra environment variable
 > ---
 >   Makefile | 10 +++++++---
 >   1 file changed, 7 insertions(+), 3 deletions(-)
 >
 > diff --git a/Makefile b/Makefile
 > index ebbedb1fb1..6c256a23b6 100644
 > --- a/Makefile
 > +++ b/Makefile
 > @@ -17,9 +17,13 @@ NAME =
 >   # o Look for make include files relative to root of kernel src
 >   MAKEFLAGS += -rR --include-dir=$(CURDIR)
 >
 > -# Determine host architecture
 > +# Determine target architecture for the sandbox
 >   include include/host_arch.h
 > -MK_ARCH="${shell uname -m}"
 > +ifeq ("", "$(CROSS_COMPILE)")
 > +  MK_ARCH="${shell uname -m}"
 > +else
 > +  MK_ARCH="${shell echo $(CROSS_COMPILE) | sed -n 
's/^\s*\([^\/]*\/\)*\([^-]*\)-\S*/\2/p'}"
 > +endif

Won't this break cross-compiling? E.g. if on my x86_64 machine I run
"CROSS_COMPILE=arm-linux-gnueabihf- make" my HOST_ARCH will be
HOST_ARCH_ARM, even though it should be HOST_ARCH_X86_64.

I think you need a separate variable for a "canadian cross." gcc uses
"build," "host," and "target," but as-is U-Boot's HOST_ARCH is gcc's
"build" arch.

--Sean

 >   unexport HOST_ARCH
 >   ifeq ("x86_64", $(MK_ARCH))
 >     export HOST_ARCH=$(HOST_ARCH_X86_64)
 > @@ -27,7 +31,7 @@ else ifneq (,$(findstring $(MK_ARCH), "i386" "i486" 
"i586" "i686"))
 >     export HOST_ARCH=$(HOST_ARCH_X86)
 >   else ifneq (,$(findstring $(MK_ARCH), "aarch64" "armv8l"))
 >     export HOST_ARCH=$(HOST_ARCH_AARCH64)
 > -else ifeq ("armv7l", $(MK_ARCH))
 > +else ifneq (,$(findstring $(MK_ARCH), "arm" "armv7" "armv7l"))
 >     export HOST_ARCH=$(HOST_ARCH_ARM)
 >   else ifeq ("riscv32", $(MK_ARCH))
 >     export HOST_ARCH=$(HOST_ARCH_RISCV32)
 > --
 > 2.30.0
 >
Sean Anderson Feb. 10, 2021, 6:30 p.m. UTC | #2
On 2/10/21 1:20 PM, Sean Anderson wrote:
> 
> 
> On 2/10/21 12:54 PM, Heinrich Schuchardt wrote:
>  > UEFI test files like helloworld.efi require an architecture specific
>  > PE-COFF header.
>  >
>  > Currently this does not work for cross compiling. If $CROSS_COMPILE 
> is set,
>  > use the first part of the architecture triplet from the variable to
>  > choose the PE-COFF header.
>  >
>  > Now we can cross-compile the sandbox, e.g.
>  >
>  >      make sandbox_defconfig NO_SDL=1
>  >      CROSS_COMPILE=/opt/bin/aarch64-linux-gnu- NO_SDL=1 
> MK_ARCH=aarch64 make
>  >
>  > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>  > ---
>  > v2:
>  >     use $CROSS_COMPILE instead of an extra environment variable
>  > ---
>  >   Makefile | 10 +++++++---
>  >   1 file changed, 7 insertions(+), 3 deletions(-)
>  >
>  > diff --git a/Makefile b/Makefile
>  > index ebbedb1fb1..6c256a23b6 100644
>  > --- a/Makefile
>  > +++ b/Makefile
>  > @@ -17,9 +17,13 @@ NAME =
>  >   # o Look for make include files relative to root of kernel src
>  >   MAKEFLAGS += -rR --include-dir=$(CURDIR)
>  >
>  > -# Determine host architecture
>  > +# Determine target architecture for the sandbox
>  >   include include/host_arch.h
>  > -MK_ARCH="${shell uname -m}"
>  > +ifeq ("", "$(CROSS_COMPILE)")
>  > +  MK_ARCH="${shell uname -m}"
>  > +else
>  > +  MK_ARCH="${shell echo $(CROSS_COMPILE) | sed -n 
> 's/^\s*\([^\/]*\/\)*\([^-]*\)-\S*/\2/p'}"
>  > +endif
> 
> Won't this break cross-compiling? E.g. if on my x86_64 machine I run
> "CROSS_COMPILE=arm-linux-gnueabihf- make" my HOST_ARCH will be
> HOST_ARCH_ARM, even though it should be HOST_ARCH_X86_64.

And it seems I've confused HOSTARCH with HOST_ARCH...

(probably a good candidate for a rename)

--Sean

> 
> I think you need a separate variable for a "canadian cross." gcc uses
> "build," "host," and "target," but as-is U-Boot's HOST_ARCH is gcc's
> "build" arch.
> 
> --Sean
> 
>  >   unexport HOST_ARCH
>  >   ifeq ("x86_64", $(MK_ARCH))
>  >     export HOST_ARCH=$(HOST_ARCH_X86_64)
>  > @@ -27,7 +31,7 @@ else ifneq (,$(findstring $(MK_ARCH), "i386" "i486" 
> "i586" "i686"))
>  >     export HOST_ARCH=$(HOST_ARCH_X86)
>  >   else ifneq (,$(findstring $(MK_ARCH), "aarch64" "armv8l"))
>  >     export HOST_ARCH=$(HOST_ARCH_AARCH64)
>  > -else ifeq ("armv7l", $(MK_ARCH))
>  > +else ifneq (,$(findstring $(MK_ARCH), "arm" "armv7" "armv7l"))
>  >     export HOST_ARCH=$(HOST_ARCH_ARM)
>  >   else ifeq ("riscv32", $(MK_ARCH))
>  >     export HOST_ARCH=$(HOST_ARCH_RISCV32)
>  > --
>  > 2.30.0
>  >
Simon Glass Feb. 10, 2021, 9:34 p.m. UTC | #3
On Wed, 10 Feb 2021 at 10:59, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> UEFI test files like helloworld.efi require an architecture specific
> PE-COFF header.
>
> Currently this does not work for cross compiling. If $CROSS_COMPILE is set,
> use the first part of the architecture triplet from the variable to
> choose the PE-COFF header.
>
> Now we can cross-compile the sandbox, e.g.
>
>     make sandbox_defconfig NO_SDL=1
>     CROSS_COMPILE=/opt/bin/aarch64-linux-gnu- NO_SDL=1 MK_ARCH=aarch64 make
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2:
>         use $CROSS_COMPILE instead of an extra environment variable
> ---
>  Makefile | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>
Simon Glass March 15, 2021, 7:25 a.m. UTC | #4
HI Heinrich,

On Thu, 11 Feb 2021 at 10:34, Simon Glass <sjg@chromium.org> wrote:
>
> On Wed, 10 Feb 2021 at 10:59, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >
> > UEFI test files like helloworld.efi require an architecture specific
> > PE-COFF header.
> >
> > Currently this does not work for cross compiling. If $CROSS_COMPILE is set,
> > use the first part of the architecture triplet from the variable to
> > choose the PE-COFF header.
> >
> > Now we can cross-compile the sandbox, e.g.
> >
> >     make sandbox_defconfig NO_SDL=1
> >     CROSS_COMPILE=/opt/bin/aarch64-linux-gnu- NO_SDL=1 MK_ARCH=aarch64 make
> >
> > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > ---
> > v2:
> >         use $CROSS_COMPILE instead of an extra environment variable
> > ---
> >  Makefile | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
>

With this patch if I try to build sandbox with CROSS_COMPILE=/bin/ it fails.

In fact 'buildman -A sandbox' will return '/bin/' so I'd like your
thoughts on what to do about this. Should we modify your patch or
perhaps make buildman return something else with -A sandbox?

Regards,
Simon

> Reviewed-by: Simon Glass <sjg@chromium.org>
Simon Glass March 16, 2021, 5:58 a.m. UTC | #5
HI Heinrich,

On Thu, 11 Feb 2021 at 10:34, Simon Glass <sjg@chromium.org> wrote:
>
> On Wed, 10 Feb 2021 at 10:59, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >
> > UEFI test files like helloworld.efi require an architecture specific
> > PE-COFF header.
> >
> > Currently this does not work for cross compiling. If $CROSS_COMPILE is set,
> > use the first part of the architecture triplet from the variable to
> > choose the PE-COFF header.
> >
> > Now we can cross-compile the sandbox, e.g.
> >
> >     make sandbox_defconfig NO_SDL=1
> >     CROSS_COMPILE=/opt/bin/aarch64-linux-gnu- NO_SDL=1 MK_ARCH=aarch64 make
> >
> > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > ---
> > v2:
> >         use $CROSS_COMPILE instead of an extra environment variable
> > ---
> >  Makefile | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
>

With this patch if I try to build sandbox with CROSS_COMPILE=/bin/ it fails.

In fact 'buildman -A sandbox' will return '/bin/' so I'd like your
thoughts on what to do about this. Should we modify your patch or
perhaps make buildman return something else with -A sandbox?

Regards,
Simon

> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot-dm/next, thanks!
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index ebbedb1fb1..6c256a23b6 100644
--- a/Makefile
+++ b/Makefile
@@ -17,9 +17,13 @@  NAME =
 # o Look for make include files relative to root of kernel src
 MAKEFLAGS += -rR --include-dir=$(CURDIR)

-# Determine host architecture
+# Determine target architecture for the sandbox
 include include/host_arch.h
-MK_ARCH="${shell uname -m}"
+ifeq ("", "$(CROSS_COMPILE)")
+  MK_ARCH="${shell uname -m}"
+else
+  MK_ARCH="${shell echo $(CROSS_COMPILE) | sed -n 's/^\s*\([^\/]*\/\)*\([^-]*\)-\S*/\2/p'}"
+endif
 unexport HOST_ARCH
 ifeq ("x86_64", $(MK_ARCH))
   export HOST_ARCH=$(HOST_ARCH_X86_64)
@@ -27,7 +31,7 @@  else ifneq (,$(findstring $(MK_ARCH), "i386" "i486" "i586" "i686"))
   export HOST_ARCH=$(HOST_ARCH_X86)
 else ifneq (,$(findstring $(MK_ARCH), "aarch64" "armv8l"))
   export HOST_ARCH=$(HOST_ARCH_AARCH64)
-else ifeq ("armv7l", $(MK_ARCH))
+else ifneq (,$(findstring $(MK_ARCH), "arm" "armv7" "armv7l"))
   export HOST_ARCH=$(HOST_ARCH_ARM)
 else ifeq ("riscv32", $(MK_ARCH))
   export HOST_ARCH=$(HOST_ARCH_RISCV32)