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

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

Hi Lior,

On 08.06.23 13:13, Lior Weintraub wrote:
> 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)

The STM32MP1 are 32-bit ARM SoCs. If you want to build that config you need
a 32-bit toolchain in CROSS_COMPILE.

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

ZynqMP are 64-bit ARM SoCs, so this is expected.

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

Unlike Linux, ARCH=arm64 is just an alias for ARCH=arm in barebox.
The only thing that determines whether barebox should target
ARMv4,5,6,7 or 8 is what boards are enabled, so if you have no
board enabled, there's not much one can do, besides print an
error message.

Cheers,
Ahmad 

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

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