| Message ID | 20220722141731.64039-3-jean-philippe@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Makefile and virtio fixes | expand |
Hi, Thank you for fixing this, I've come across it several times in the past. On Fri, Jul 22, 2022 at 03:17:30PM +0100, Jean-Philippe Brucker wrote: > Variables set on the command-line are not overridden by normal > assignments. So when passing ARCH=x86_64 on the command-line, build > fails: > > Makefile:227: *** This architecture (x86_64) is not supported in kvmtool. > > Use the 'override' directive to force the ARCH reassignment. > > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> > --- > Makefile | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/Makefile b/Makefile > index f0df76f4..faae0da2 100644 > --- a/Makefile > +++ b/Makefile > @@ -115,11 +115,11 @@ ARCH ?= $(shell uname -m | sed -e s/i.86/i386/ -e s/ppc.*/powerpc/ \ > -e s/riscv64/riscv/ -e s/riscv32/riscv/) > > ifeq ($(ARCH),i386) As far as I know, there are several versions of the x86 architecture: i386, i486, i586 and i686. It looks rather arbitrary to have i386 as a valid ARCH value, but not the other ix86 versions. I wonder if we should just drop it and keep only x86 and x86_64, like the kernel. Regardless, the patch looks good to me: Tested-by: Alexandru Elisei <alexandru.elisei@arm.com> Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com> Thanks, Alex > - ARCH := x86 > + override ARCH = x86 > DEFINES += -DCONFIG_X86_32 > endif > ifeq ($(ARCH),x86_64) > - ARCH := x86 > + override ARCH = x86 > DEFINES += -DCONFIG_X86_64 > ARCH_PRE_INIT = x86/init.S > endif > -- > 2.37.1 >
On Mon, Jul 25, 2022 at 12:06:38PM +0100, Alexandru Elisei wrote: > Hi, > > Thank you for fixing this, I've come across it several times in the past. > > On Fri, Jul 22, 2022 at 03:17:30PM +0100, Jean-Philippe Brucker wrote: > > Variables set on the command-line are not overridden by normal > > assignments. So when passing ARCH=x86_64 on the command-line, build > > fails: > > > > Makefile:227: *** This architecture (x86_64) is not supported in kvmtool. > > > > Use the 'override' directive to force the ARCH reassignment. > > > > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> > > --- > > Makefile | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/Makefile b/Makefile > > index f0df76f4..faae0da2 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -115,11 +115,11 @@ ARCH ?= $(shell uname -m | sed -e s/i.86/i386/ -e s/ppc.*/powerpc/ \ > > -e s/riscv64/riscv/ -e s/riscv32/riscv/) > > > > ifeq ($(ARCH),i386) > > As far as I know, there are several versions of the x86 architecture: i386, > i486, i586 and i686. The discovery from "uname -m" does "sed -e s/i.86/i386/" to catch those > It looks rather arbitrary to have i386 as a valid ARCH value, but not the other > ix86 versions. I wonder if we should just drop it and keep only x86 and x86_64, > like the kernel. "i386" does seem to be the conventional way of selecting 32-bit x86 in the kernel, since it has a i386_defconfig selected by ARCH=i386. > > Regardless, the patch looks good to me: > > Tested-by: Alexandru Elisei <alexandru.elisei@arm.com> > Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com> Thanks! Jean > > Thanks, > Alex > > > - ARCH := x86 > > + override ARCH = x86 > > DEFINES += -DCONFIG_X86_32 > > endif > > ifeq ($(ARCH),x86_64) > > - ARCH := x86 > > + override ARCH = x86 > > DEFINES += -DCONFIG_X86_64 > > ARCH_PRE_INIT = x86/init.S > > endif > > -- > > 2.37.1 > >
Hi, On Tue, Aug 09, 2022 at 10:56:50AM +0100, Jean-Philippe Brucker wrote: > On Mon, Jul 25, 2022 at 12:06:38PM +0100, Alexandru Elisei wrote: > > Hi, > > > > Thank you for fixing this, I've come across it several times in the past. > > > > On Fri, Jul 22, 2022 at 03:17:30PM +0100, Jean-Philippe Brucker wrote: > > > Variables set on the command-line are not overridden by normal > > > assignments. So when passing ARCH=x86_64 on the command-line, build > > > fails: > > > > > > Makefile:227: *** This architecture (x86_64) is not supported in kvmtool. > > > > > > Use the 'override' directive to force the ARCH reassignment. > > > > > > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> > > > --- > > > Makefile | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/Makefile b/Makefile > > > index f0df76f4..faae0da2 100644 > > > --- a/Makefile > > > +++ b/Makefile > > > @@ -115,11 +115,11 @@ ARCH ?= $(shell uname -m | sed -e s/i.86/i386/ -e s/ppc.*/powerpc/ \ > > > -e s/riscv64/riscv/ -e s/riscv32/riscv/) > > > > > > ifeq ($(ARCH),i386) > > > > As far as I know, there are several versions of the x86 architecture: i386, > > i486, i586 and i686. > > The discovery from "uname -m" does "sed -e s/i.86/i386/" to catch those I was referring when the user sets ARCH to something other than i386: $ make ARCH=i486 clean Makefile:233: *** This architecture (i486) is not supported in kvmtool. Stop. Looks like that's because ARCH is assigned to the output of that uname -m oneliner with the ?= operator, which according to gnu make: "This is called a conditional variable assignment operator, because it only has an effect if the variable is not yet defined". But yeah, I checked for the kernel and only i386 works (never compiled the kernel for something other than x86_64), so I guess it's OK to leave kvmtool's Makefile as it is. Thanks, Alex > > > It looks rather arbitrary to have i386 as a valid ARCH value, but not the other > > ix86 versions. I wonder if we should just drop it and keep only x86 and x86_64, > > like the kernel. > > "i386" does seem to be the conventional way of selecting 32-bit x86 in the > kernel, since it has a i386_defconfig selected by ARCH=i386. > > > > > Regardless, the patch looks good to me: > > > > Tested-by: Alexandru Elisei <alexandru.elisei@arm.com> > > Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com> > > Thanks! > Jean > > > > > Thanks, > > Alex > > > > > - ARCH := x86 > > > + override ARCH = x86 > > > DEFINES += -DCONFIG_X86_32 > > > endif > > > ifeq ($(ARCH),x86_64) > > > - ARCH := x86 > > > + override ARCH = x86 > > > DEFINES += -DCONFIG_X86_64 > > > ARCH_PRE_INIT = x86/init.S > > > endif > > > -- > > > 2.37.1 > > >
diff --git a/Makefile b/Makefile index f0df76f4..faae0da2 100644 --- a/Makefile +++ b/Makefile @@ -115,11 +115,11 @@ ARCH ?= $(shell uname -m | sed -e s/i.86/i386/ -e s/ppc.*/powerpc/ \ -e s/riscv64/riscv/ -e s/riscv32/riscv/) ifeq ($(ARCH),i386) - ARCH := x86 + override ARCH = x86 DEFINES += -DCONFIG_X86_32 endif ifeq ($(ARCH),x86_64) - ARCH := x86 + override ARCH = x86 DEFINES += -DCONFIG_X86_64 ARCH_PRE_INIT = x86/init.S endif
Variables set on the command-line are not overridden by normal assignments. So when passing ARCH=x86_64 on the command-line, build fails: Makefile:227: *** This architecture (x86_64) is not supported in kvmtool. Use the 'override' directive to force the ARCH reassignment. Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)