diff mbox series

andes_ae350_45: Fix build issues of AE350

Message ID 20230812110359.27170-1-peterlin@andestech.com
State Changes Requested
Headers show
Series andes_ae350_45: Fix build issues of AE350 | expand

Commit Message

Yu Chien Peter Lin Aug. 12, 2023, 11:03 a.m. UTC
- Select RVA extension so the toolchain uses glibc by default
- Enable Zifencei and Zicsr for gcc-12 to fix U-Boot build failure

Fixes:
- https://gitlab.com/buildroot.org/buildroot/-/jobs/4839059655

Signed-off-by: Yu Chien Peter Lin <peterlin@andestech.com>
---
 configs/andes_ae350_45_defconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Kilian Zinnecker Aug. 12, 2023, noon UTC | #1
Hello Yu Chien Peter,

> - Select RVA extension so the toolchain uses glibc by default
> - Enable Zifencei and Zicsr for gcc-12 to fix U-Boot build failure

Yeah, this zifencei_zicsr extension flag thing is annoying ...

> Fixes:
> - https://gitlab.com/buildroot.org/buildroot/-/jobs/4839059655
> 
> Signed-off-by: Yu Chien Peter Lin <peterlin@andestech.com>
> ---
>  configs/andes_ae350_45_defconfig | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/configs/andes_ae350_45_defconfig
> b/configs/andes_ae350_45_defconfig index 998276635b..faf29b8b6b 100644
> --- a/configs/andes_ae350_45_defconfig
> +++ b/configs/andes_ae350_45_defconfig
> @@ -1,6 +1,7 @@
>  BR2_riscv=y
>  BR2_riscv_custom=y
>  BR2_RISCV_ISA_CUSTOM_RVM=y
> +BR2_RISCV_ISA_CUSTOM_RVA=y
>  BR2_RISCV_ISA_CUSTOM_RVF=y
>  BR2_RISCV_ISA_CUSTOM_RVD=y
>  BR2_RISCV_ISA_CUSTOM_RVC=y
> @@ -38,7 +39,7 @@ BR2_TARGET_UBOOT_NEEDS_OPENSBI=y
>  BR2_TARGET_UBOOT_FORMAT_CUSTOM=y
>  BR2_TARGET_UBOOT_FORMAT_CUSTOM_NAME="u-boot.itb"
>  BR2_TARGET_UBOOT_SPL=y
> -BR2_TARGET_UBOOT_CUSTOM_MAKEOPTS="ARCH_FLAGS=-march=rv64imafdc"
> +BR2_TARGET_UBOOT_CUSTOM_MAKEOPTS="ARCH_FLAGS=-march=rv64imafdc_zifencei_zic
> sr" BR2_PACKAGE_HOST_DOSFSTOOLS=y
>  BR2_PACKAGE_HOST_GENIMAGE=y
>  BR2_PACKAGE_HOST_MTOOLS=y

I am not sure, whether you need to specify custom makeopts for uboot after 
all: Looking at the current andes_ae350_45_defconfig (https://gitlab.com/
buildroot.org/buildroot/-/blob/4de60e41c3487946712da6d889ef89d325c2032a/
configs/andes_ae350_45_defconfig), it seems, that there is no special/custom gcc 
version defined. Hence the gcc version should be buildroot's current default 
version, i.e., v12.3.0. For this version, buildroot already adds the flags for 
zifencei and zicsr extension, see here:

https://gitlab.com/buildroot.org/buildroot/-/blob/
4de60e41c3487946712da6d889ef89d325c2032a/arch/arch.mk.riscv#L38-39

And I also believe, that these defines should be sufficient to set the rv64imafdc 
flags:

>  BR2_RISCV_ISA_CUSTOM_RVM=y
> +BR2_RISCV_ISA_CUSTOM_RVA=y
>  BR2_RISCV_ISA_CUSTOM_RVF=y
>  BR2_RISCV_ISA_CUSTOM_RVD=y
>  BR2_RISCV_ISA_CUSTOM_RVC=y

So I think setting BR2_RISCV_ISA_CUSTOM_RVA=y is correct. However, you could 
try whether it works, if you then completely remove the 
BR2_TARGET_UBOOT_CUSTOM_MAKEOPTS?

Best regards,
Kilian
Thomas Petazzoni Aug. 12, 2023, 12:25 p.m. UTC | #2
On Sat, 12 Aug 2023 19:03:59 +0800
Yu Chien Peter Lin <peterlin@andestech.com> wrote:

> - Select RVA extension so the toolchain uses glibc by default
> - Enable Zifencei and Zicsr for gcc-12 to fix U-Boot build failure

I think these should be two separate patches. Indeed, only the second
part is needed to fix the build failure. Could you check this, and if
it's the case, split into two patches?

Thanks!

Thomas
Thomas Petazzoni Aug. 12, 2023, 12:27 p.m. UTC | #3
On Sat, 12 Aug 2023 14:00:00 +0200
Kilian Zinnecker <kilian.zinnecker@mail.de> wrote:

> So I think setting BR2_RISCV_ISA_CUSTOM_RVA=y is correct. However, you could 
> try whether it works, if you then completely remove the 
> BR2_TARGET_UBOOT_CUSTOM_MAKEOPTS?

The U-Boot version used by the andes_ae350_45 defconfig indeed includes:

# Newer binutils versions default to ISA spec version 20191213 which moves some
# instructions from the I extension to the Zicsr and Zifencei extensions.
toolchain-need-zicsr-zifencei := $(call cc-option-yn, -mabi=$(ABI) -march=$(RISCV_MARCH)_zicsr_zifencei)
ifeq ($(toolchain-need-zicsr-zifencei),y)
        RISCV_MARCH := $(RISCV_MARCH)_zicsr_zifencei
endif

So it should already be doing the right thing automatically. However,
the fact that the defconfig forces its own ARCH_FLAGS=-march=rv64imafdc
is probably what breaks the build.

So I agree with Kilian here: please try without any:

BR2_TARGET_UBOOT_CUSTOM_MAKEOPTS="ARCH_FLAGS=-march=rv64imafdc"

and see if that helps. However in that case, you might need indeed to
keep the addition of BR2_RISCV_ISA_CUSTOM_RVA=y as part of the fix:
please double check that.

Thanks!

Thomas
Yu Chien Peter Lin Aug. 14, 2023, 2:19 a.m. UTC | #4
On Sat, Aug 12, 2023 at 02:27:59PM +0200, Thomas Petazzoni wrote:
> On Sat, 12 Aug 2023 14:00:00 +0200
> Kilian Zinnecker <kilian.zinnecker@mail.de> wrote:
> 
> > So I think setting BR2_RISCV_ISA_CUSTOM_RVA=y is correct. However, you could 
> > try whether it works, if you then completely remove the 
> > BR2_TARGET_UBOOT_CUSTOM_MAKEOPTS?
> 
> The U-Boot version used by the andes_ae350_45 defconfig indeed includes:
> 
> # Newer binutils versions default to ISA spec version 20191213 which moves some
> # instructions from the I extension to the Zicsr and Zifencei extensions.
> toolchain-need-zicsr-zifencei := $(call cc-option-yn, -mabi=$(ABI) -march=$(RISCV_MARCH)_zicsr_zifencei)
> ifeq ($(toolchain-need-zicsr-zifencei),y)
>         RISCV_MARCH := $(RISCV_MARCH)_zicsr_zifencei
> endif
> 
> So it should already be doing the right thing automatically. However,
> the fact that the defconfig forces its own ARCH_FLAGS=-march=rv64imafdc
> is probably what breaks the build.
> 
> So I agree with Kilian here: please try without any:
> 
> BR2_TARGET_UBOOT_CUSTOM_MAKEOPTS="ARCH_FLAGS=-march=rv64imafdc"
> 
> and see if that helps. However in that case, you might need indeed to
> keep the addition of BR2_RISCV_ISA_CUSTOM_RVA=y as part of the fix:
> please double check that.
> 
> Thanks!
> 
> Thomas
> -- 
> Thomas Petazzoni, co-owner and CEO, Bootlin
> Embedded Linux and Kernel engineering and training
> https://bootlin.com

Hi Thomas, Kilian,

Yeah, I should not override the ARCH_FLAGS with BR2_TARGET_UBOOT_CUSTOM_MAKEOPTS.
I've rebuilt it without any problems, so I will remove the configuration and
add a patch to set R2_RISCV_ISA_CUSTOM_RVA.

Thanks for your review!

Best regards,
Peter Lin
diff mbox series

Patch

diff --git a/configs/andes_ae350_45_defconfig b/configs/andes_ae350_45_defconfig
index 998276635b..faf29b8b6b 100644
--- a/configs/andes_ae350_45_defconfig
+++ b/configs/andes_ae350_45_defconfig
@@ -1,6 +1,7 @@ 
 BR2_riscv=y
 BR2_riscv_custom=y
 BR2_RISCV_ISA_CUSTOM_RVM=y
+BR2_RISCV_ISA_CUSTOM_RVA=y
 BR2_RISCV_ISA_CUSTOM_RVF=y
 BR2_RISCV_ISA_CUSTOM_RVD=y
 BR2_RISCV_ISA_CUSTOM_RVC=y
@@ -38,7 +39,7 @@  BR2_TARGET_UBOOT_NEEDS_OPENSBI=y
 BR2_TARGET_UBOOT_FORMAT_CUSTOM=y
 BR2_TARGET_UBOOT_FORMAT_CUSTOM_NAME="u-boot.itb"
 BR2_TARGET_UBOOT_SPL=y
-BR2_TARGET_UBOOT_CUSTOM_MAKEOPTS="ARCH_FLAGS=-march=rv64imafdc"
+BR2_TARGET_UBOOT_CUSTOM_MAKEOPTS="ARCH_FLAGS=-march=rv64imafdc_zifencei_zicsr"
 BR2_PACKAGE_HOST_DOSFSTOOLS=y
 BR2_PACKAGE_HOST_GENIMAGE=y
 BR2_PACKAGE_HOST_MTOOLS=y