RE: [PATCH 2/2] ARM: don't assume 32-bit when no boards are selected

[Lior Weintraub <liorw@pliops.com>]

Hi Ahmad,

Sorry for the confusion. My bad.
I used export ARCH=arm64 and that is probably the reason for the warnings I got when I run "make stm32mp_defconfig".
If I change to ARCH=arm, this command works well but then fail on "make" with the following errors:
  CREATE  include/config.h
  CC      arch/arm/lib/asm-offsets.s
aarch64-none-elf-gcc: error: unrecognized argument in option '-mabi=aapcs-linux'
aarch64-none-elf-gcc: note: valid arguments to '-mabi=' are: ilp32 lp64
aarch64-none-elf-gcc: error: unrecognized command-line option '-msoft-float'
aarch64-none-elf-gcc: error: unrecognized command-line option '-mthumb'
aarch64-none-elf-gcc: error: unrecognized command-line option '-mno-unaligned-access'

(using latest "next" branch with commit ID fdcc6754c0d0d7359b60e00e15921316b71764da)

Again, I apologize for my lack of experience.
When I try to run "make zynqmp_defconfig" and "make" it is working OK.

The only reason I tried using stm32mp in the first place was just because I thought it is more popular and should compile correctly.

Please ignore the "depends on HAVE_ARCH_STM32MP157".
It was only my attempt to introduce an error on Kbuild file.

Cheers,
Lior.

> -----Original Message-----
> From: Ahmad Fatoum <a.fatoum@pengutronix.de>
> Sent: Thursday, June 8, 2023 12:18 PM
> To: Lior Weintraub <liorw@pliops.com>; barebox@lists.infradead.org
> Cc: Ulrich Ölmann <u.oelmann@pengutronix.de>; Sascha Hauer
> <sha@pengutronix.de>
> Subject: Re: [PATCH 2/2] ARM: don't assume 32-bit when no boards are
> selected
> 
> CAUTION: External Sender
> 
> On 08.06.23 10:59, Lior Weintraub wrote:
> > Hi Ahmad,
> >
> > I think that replacing Kconfig with .config on the error message will not help
> either.
> > Maybe Kconfig is a good name because it suggest that the issue is there but
> again, not so helpful because it doesn't point to the Kconfig file and line that
> brock the configuration.
> 
> There is no single line that broke the configuration. The user is supposed
> to take an existing defconfig and manipulate that. If they write their
> own defconfig that doesn't enable any CPUs, then they get an error message
> that they enabled no CPUs instead of random compiler errors.
> 
> > As I said, I have no prior experience with Kbuild so it could be out of scope
> for the barebox project.
> 
> Changing the error message to something more useful is definitely in-scope.
> Suggestions are welcome.
> 
> > The patch I gave below just add this wrong line ("depends on
> HAVE_ARCH_STM32MP157") into arch/arm/mach-stm32mp/Kconfig
> > Then you can run:
> > "make stm32mp_defconfig" and see the strange warnings (which again
> doesn't help to pin point what is wrong with the Kconfig file).
> > Once you build with "make" you will see the new error ("#error No
> boards/CPUs selected in Kconfig").
> 
> There's no HAVE_ARCH_STM32MP157 symbol defined with barebox.
> (Symbols are defined e.g. with
>   config HAVE_ARCH_STM32MP157
>   bool
> ).
> 
> I don't think we can protect ourselves against arbitrary
> modification of the Kconfig files. Most users just take
> a defconfig and customize it with menuconfig.
> 
> In the past, developers had a bit harder time, but we have
> multi_v8_defconfig and multi_v7_defconfig, which can be used
> for new architectures as well (assuming the Kconfig of the
> new architecture opts in into CONFIG_ARCH_MULTIARCH).
> 
> > After reverting the wrong Kconfig line, I tried to run "make mrproper" but
> this didn't help.
> > I was still getting the same warning when I run "make stm32mp_defconfig"
> and the same error when I run "make".
> 
> I can't reproduce this. Even if you change arch/arm/mach-stm32mp/Kconfig
> as
> described, you will still have other STM32MP1 boards that enable CPUs.
> 
> Cheers,
> Ahmad
> 
> >
> > Hope this helps,
> > Cheers,
> > Lior.
> >
> >> -----Original Message-----
> >> From: Ahmad Fatoum <a.fatoum@pengutronix.de>
> >> Sent: Thursday, June 8, 2023 11:47 AM
> >> To: Lior Weintraub <liorw@pliops.com>; barebox@lists.infradead.org
> >> Cc: Ulrich Ölmann <u.oelmann@pengutronix.de>; Sascha Hauer
> >> <sha@pengutronix.de>
> >> Subject: Re: [PATCH 2/2] ARM: don't assume 32-bit when no boards are
> >> selected
> >>
> >> CAUTION: External Sender
> >>
> >> Hello Lior,
> >>
> >> On 08.06.23 10:20, Lior Weintraub wrote:
> >>> Hi Ahmad,
> >>>
> >>> Thanks for this fix!
> >>> Few comments:
> >>> 1. I was getting errors when I tried to apply your patch on either "master"
> or
> >> "next" branch but then realized your fix was already merged to "next" :-).
> >>
> >> Ye, Sascha was fast :-)
> >>
> >>> 2. Indeed this fix doesn't show the original error we saw but now it shows:
> >> "error: #error No boards/CPUs selected in Kconfig". It still doesn't help to
> find
> >> the culprit.
> >>
> >> How would you rephrase it to be more useful? Replace `Kconfig' with
> `.config'?
> >>
> >>> 3. During the investigation, I also found another issue which is related to
> >> temp\generated build files.
> >>>
> >>> The following patch introduces a mistake in Kconfig file.
> >>
> >> What defconfig do you source after applying the patch and
> >> what commands do you run?
> >>
> >>>
> >>> diff --git a/arch/arm/mach-stm32mp/Kconfig b/arch/arm/mach-
> >> stm32mp/Kconfig
> >>> index bc0a48d64c..97b61810d7 100644
> >>> --- a/arch/arm/mach-stm32mp/Kconfig
> >>> +++ b/arch/arm/mach-stm32mp/Kconfig
> >>> @@ -30,6 +30,7 @@ config MACH_LXA_MC1
> >>>       bool "Linux Automation MC-1 board"
> >>>
> >>>  config MACH_SEEED_ODYSSEY
> >>> +     depends on HAVE_ARCH_STM32MP157
> >>>       select ARCH_STM32MP157
> >>>       bool "Seeed Studio Odyssey"
> >>>
> >>> Now run make stm32mp_defconfig and see:
> >>>
> >>> WARNING: unmet direct dependencies detected for USB_DWC2
> >>>   Depends on [n]: USB_HOST [=y] && USB [=y] && HAS_DMA [=n]
> >>>   Selected by [y]:
> >>>   - USB_DWC2_HOST [=y] && USB_HOST [=y]
> >>>   - USB_DWC2_GADGET [=y] && USB_HOST [=y] && USB_GADGET [=y]
> >>>
> >>> WARNING: unmet direct dependencies detected for USB_DWC2
> >>>   Depends on [n]: USB_HOST [=y] && USB [=y] && HAS_DMA [=n]
> >>>   Selected by [y]:
> >>>   - USB_DWC2_HOST [=y] && USB_HOST [=y]
> >>>   - USB_DWC2_GADGET [=y] && USB_HOST [=y] && USB_GADGET [=y]
> >>>
> >>> (Not very helpful warnings but then again I have no experience with
> Kbuild)
> >>>
> >>> Then make -j will produce the error "#error No boards/CPUs selected in
> >> Kconfig".
> >>> Now even if you revert the change on Kconfig there is no way to fix the
> build
> >> (tried make clean) without getting a new clone into a fresh folder.
> >>
> >> make clean intentionally doesn't wipe the configuration, only the build
> >> artifacts.
> >>
> >>> I assume there is something left from previous build that is not cleaned
> >> correctly.
> >>
> >> If you want to source a defconfig, use
> >>
> >>   make my_platforms_defconfig
> >>
> >> If you want to delete everything, use
> >>
> >>   make mrproper
> >>
> >> Same applies to other Kbuild projects.
> >>
> >>>
> >>> Cheers,
> >>> Lior.
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Ahmad Fatoum <a.fatoum@pengutronix.de>
> >>>> Sent: Wednesday, June 7, 2023 6:32 PM
> >>>> To: barebox@lists.infradead.org
> >>>> Cc: Lior Weintraub <liorw@pliops.com>; Ulrich Ölmann
> >>>> <u.oelmann@pengutronix.de>; Ahmad Fatoum
> >> <a.fatoum@pengutronix.de>
> >>>> Subject: [PATCH 2/2] ARM: don't assume 32-bit when no boards are
> >> selected
> >>>>
> >>>> CAUTION: External Sender
> >>>>
> >>>> barebox build errors are very confusing if no board is selected.
> >>>> This should have been fixed with commit 14b296d2a7e6 ("arm: error
> >>>> out if __LINUX_ARM_ARCH__ is undefined"), but unfortunately that's
> >>>> only true for ARM32. On ARM64, the error message in question
> >>>> is not printed, because build aborts even earlier, because Kbuild
> >>>> assumes it should build for 32-bit ARM and thus passes the ARM64
> >>>> compiler options that it can't understand:
> >>>>
> >>>>   aarch64-oe-linux-gcc: error: unrecognized argument in option '-
> >> mabi=apcs-
> >>>> gnu'
> >>>>   aarch64-oe-linux-gcc: note: valid arguments to '-mabi=' are: ilp32 lp64
> >>>>   aarch64-oe-linux-gcc: error: unrecognized command-line option '-
> msoft-
> >>>> float'
> >>>>   aarch64-oe-linux-gcc: error: unrecognized command-line option '-mno-
> >>>> unaligned-access'
> >>>>
> >>>> Let's fix that for ARM64 builds by not assuming !CONFIG_CPU_64 to be
> 32-
> >>>> bit,
> >>>> but instead explicitly check that CONFIG_CPU_32 is set before doing
> >>>> 32-bit specific changes.
> >>>>
> >>>> This ensures we now fail during compilation on both ARM32 and ARM64
> if
> >>>> no boards were selected. We can't fail earlier via $(error ...) as this
> >>>> would impact use of targets like menuconfig.
> >>>>
> >>>> Reported-by: Lior Weintraub <liorw@pliops.com>
> >>>> Reported-by: Ulrich Ölmann <u.oelmann@pengutronix.de>
> >>>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> >>>> ---
> >>>>  arch/arm/Makefile | 35 ++++++++++++++---------------------
> >>>>  1 file changed, 14 insertions(+), 21 deletions(-)
> >>>>
> >>>> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> >>>> index 926af7387f7f..337b7e9095fa 100644
> >>>> --- a/arch/arm/Makefile
> >>>> +++ b/arch/arm/Makefile
> >>>> @@ -6,7 +6,8 @@ KBUILD_CPPFLAGS += -D__ARM__ -fno-strict-aliasing
> >>>>  # Explicitly specifiy 32-bit ARM ISA since toolchain default can be -
> mthumb:
> >>>>  ifeq ($(CONFIG_CPU_64),y)
> >>>>  KBUILD_CPPFLAGS        +=$(call cc-option,-maarch64,)
> >>>> -else
> >>>> +endif
> >>>> +ifeq ($(CONFIG_CPU_32),y)
> >>>>  KBUILD_CPPFLAGS        +=$(call cc-option,-marm,)
> >>>>  KBUILD_CPPFLAGS        += -msoft-float
> >>>>  endif
> >>>> @@ -27,14 +28,12 @@ endif
> >>>>  # at least some of the code would be executed with MMU off, lets be
> >>>>  # conservative and instruct the compiler not to generate any unaligned
> >>>>  # accesses
> >>>> -ifneq ($(CONFIG_CPU_64),y)
> >>>> +ifeq ($(CONFIG_CPU_32),y)
> >>>>  KBUILD_CFLAGS += -mno-unaligned-access
> >>>> -else
> >>>> -KBUILD_CFLAGS += -mstrict-align
> >>>>  endif
> >>>> -
> >>>> -# Prevent use of floating point and Advanced SIMD registers.
> >>>>  ifeq ($(CONFIG_CPU_64),y)
> >>>> +KBUILD_CFLAGS += -mstrict-align
> >>>> +# Prevent use of floating point and Advanced SIMD registers.
> >>>>  KBUILD_CFLAGS += -mgeneral-regs-only
> >>>>  endif
> >>>>
> >>>> @@ -54,19 +53,11 @@ tune-$(CONFIG_CPU_ARM920T)  :=-
> >>>> mtune=arm9tdmi
> >>>>  tune-$(CONFIG_CPU_ARM926T)     :=-mtune=arm9tdmi
> >>>>  tune-$(CONFIG_CPU_XSCALE)      :=$(call cc-option,-mtune=xscale,-
> >>>> mtune=strongarm110) -Wa,-mcpu=xscale
> >>>>
> >>>> -ifeq ($(CONFIG_CPU_64), y)
> >>>> -CFLAGS_ABI     :=-mabi=lp64
> >>>> -else
> >>>> -ifeq ($(CONFIG_AEABI),y)
> >>>> -CFLAGS_ABI     :=-mabi=aapcs-linux
> >>>> -else
> >>>> -CFLAGS_ABI     :=$(call cc-option,-mapcs-32,-mabi=apcs-gnu) $(call cc-
> >>>> option,-mno-thumb-interwork,)
> >>>> -endif
> >>>> -endif
> >>>> +CFLAGS_ABI-$(CONFIG_CPU_64)    :=-mabi=lp64
> >>>> +CFLAGS_ABI-$(CONFIG_CPU_32)    :=$(call cc-option,-mapcs-32,-
> >> mabi=apcs-
> >>>> gnu) $(call cc-option,-mno-thumb-interwork,)
> >>>> +CFLAGS_ABI-$(CONFIG_AEABI)     :=-mabi=aapcs-linux
> >>>>
> >>>> -ifeq ($(CONFIG_ARM_UNWIND),y)
> >>>> -CFLAGS_ABI     +=-funwind-tables
> >>>> -endif
> >>>> +CFLAGS_ABI-$(CONFIG_ARM_UNWIND)        +=-funwind-tables
> >>>>
> >>>>  ifeq ($(CONFIG_THUMB2_BAREBOX),y)
> >>>>  AFLAGS_AUTOIT  :=$(call as-option,-Wa$(comma)-mimplicit-it=always,-
> >>>> Wa$(comma)-mauto-it)
> >>>> @@ -75,13 +66,15 @@ CFLAGS_THUMB2       :=-mthumb
> >> $(AFLAGS_AUTOIT)
> >>>> $(AFLAGS_NOWARN)
> >>>>  AFLAGS_THUMB2  :=$(CFLAGS_THUMB2) -Wa$(comma)-mthumb
> >>>>  endif
> >>>>
> >>>> +KBUILD_CPPFLAGS += $(CFLAGS_ABI-y) $(arch-y) $(tune-y)
> >>>> +
> >>>>  ifeq ($(CONFIG_CPU_64), y)
> >>>> -KBUILD_CPPFLAGS += $(CFLAGS_ABI) $(arch-y) $(tune-y)
> >>>>  KBUILD_AFLAGS   += -include asm/unified.h
> >>>>  export S64_32 = 64
> >>>>  export S64 = 64
> >>>> -else
> >>>> -KBUILD_CPPFLAGS += $(CFLAGS_ABI) $(arch-y) $(tune-y)
> >>>> $(CFLAGS_THUMB2)
> >>>> +endif
> >>>> +ifeq ($(CONFIG_CPU_32), y)
> >>>> +KBUILD_CPPFLAGS += $(CFLAGS_THUMB2)
> >>>>  KBUILD_AFLAGS   += -include asm/unified.h -msoft-float
> >> $(AFLAGS_THUMB2)
> >>>>  export S64_32 = 32
> >>>>  export S32 = 32
> >>>> --
> >>>> 2.39.2
> >>>
> >>
> >> --
> >> Pengutronix e.K.                           |                             |
> >> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> >> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> >> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> >
> 
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |