Re: [PATCH v3 09/10] RISC-V: Add to build system

[Bin Meng]

Hi Alex,

On Thu, Nov 15, 2018 at 1:27 AM Alexander Graf <address@hidden> wrote:
>
> This patch adds support for RISC-V to the grub build system. With this
> patch, I can successfully build grub on RISC-V as a UEFI application.
>
> Signed-off-by: Alexander Graf <address@hidden>
> Reviewed-by: Alistair Francis <address@hidden>
>
> ---
>
> v2 -> v3:
>
>   - Fix riscv32 target
> ---
>  configure.ac                 | 28 ++++++++++++++++++++++++++--
>  gentpl.py                    | 11 +++++++----
>  grub-core/Makefile.am        | 12 ++++++++++++
>  grub-core/Makefile.core.def  | 29 +++++++++++++++++++++++++++++
>  grub-core/commands/file.c    | 14 +++++++++++++-
>  grub-core/kern/compiler-rt.c |  6 ++++--
>  grub-core/kern/efi/mm.c      |  2 +-
>  grub-core/kern/emu/cache.c   |  6 ++++++
>  grub-core/kern/emu/cache_s.S |  1 +
>  grub-core/kern/emu/lite.c    |  2 ++
>  grub-core/lib/efi/halt.c     |  3 ++-
>  grub-core/lib/setjmp.S       |  2 ++
>  include/grub/compiler-rt.h   | 12 ++++++++----
>  include/grub/efi/api.h       |  3 ++-
>  include/grub/efi/efi.h       |  2 +-
>  include/grub/misc.h          |  3 ++-
>  include/grub/util/install.h  |  2 ++
>  util/grub-install-common.c   |  2 ++
>  util/grub-install.c          | 28 ++++++++++++++++++++++++++++
>  util/grub-mknetdir.c         |  4 +++-
>  util/grub-mkrescue.c         | 16 +++++++++++++++-
>  util/mkimage.c               | 32 ++++++++++++++++++++++++++++++++
>  22 files changed, 200 insertions(+), 20 deletions(-)
>

Reviewed-by: Bin Meng <address@hidden>
Tested-by: Bin Meng <address@hidden>

But please see one comment below:

> diff --git a/configure.ac b/configure.ac
> index 5e63c4af3..2cc2ffb2c 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -104,6 +104,12 @@ case "$target_cpu" in
>    aarch64*)
>                 target_cpu=arm64
>                 ;;
> +  riscv32*)
> +               target_cpu=riscv32
> +               ;;
> +  riscv64*)
> +               target_cpu=riscv64
> +               ;;
>  esac
>
>  # Specify the platform (such as firmware).
> @@ -127,6 +133,8 @@ if test "x$with_platform" = x; then
>      ia64-*) platform=efi ;;
>      arm-*) platform=uboot ;;
>      arm64-*) platform=efi ;;
> +    riscv32-*) platform=efi ;;
> +    riscv64-*) platform=efi ;;
>      *)
>        AC_MSG_WARN([unsupported CPU: "$target_cpu" - only building utilities])
>        platform=none
> @@ -174,6 +182,8 @@ case "$target_cpu"-"$platform" in
>    arm-coreboot) ;;
>    arm-efi) ;;
>    arm64-efi) ;;
> +  riscv32-efi) ;;
> +  riscv64-efi) ;;
>    *-emu) ;;
>    *-none) ;;
>    *) AC_MSG_ERROR([platform "$platform" is not supported for target CPU 
> "$target_cpu"]) ;;
> @@ -826,6 +836,16 @@ if test x"$platform" != xemu ; then
>         AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[]], [[]])],
>                          
> [grub_cv_target_cc_soft_float="-mgeneral-regs-only"], [])
>      fi
> +    if test "x$target_cpu" = xriscv32; then
> +       CFLAGS="$TARGET_CFLAGS -march=rv32imac -mabi=ilp32 -Werror"
> +       AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[]], [[]])],
> +                        [grub_cv_target_cc_soft_float="-march=rv32imac 
> -mabi=ilp32"], [])
> +    fi
> +    if test "x$target_cpu" = xriscv64; then
> +       CFLAGS="$TARGET_CFLAGS -march=rv64imac -mabi=lp64 -Werror"
> +       AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[]], [[]])],
> +                        [grub_cv_target_cc_soft_float="-march=rv64imac 
> -mabi=lp64"], [])
> +    fi
>      if test "x$target_cpu" = xia64; then
>         CFLAGS="$TARGET_CFLAGS -mno-inline-float-divide -mno-inline-sqrt 
> -Werror"
>         AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[]], [[]])],
> @@ -1141,7 +1161,7 @@ AC_SUBST(TARGET_LDFLAGS_OLDMAGIC)
>
>  LDFLAGS="$TARGET_LDFLAGS"
>
> -if test "$target_cpu" = x86_64 || test "$target_cpu" = sparc64 ; then
> +if test "$target_cpu" = x86_64 || test "$target_cpu" = sparc64 || test 
> "$target_cpu" = riscv64 ; then
>    # Use large model to support 4G memory
>    AC_CACHE_CHECK([whether option -mcmodel=large works], grub_cv_cc_mcmodel, [
>      CFLAGS="$TARGET_CFLAGS -mcmodel=large"
> @@ -1151,7 +1171,7 @@ if test "$target_cpu" = x86_64 || test "$target_cpu" = 
> sparc64 ; then
>    ])
>    if test "x$grub_cv_cc_mcmodel" = xyes; then
>      TARGET_CFLAGS="$TARGET_CFLAGS -mcmodel=large"
> -  elif test "$target_cpu" = sparc64; then
> +  elif test "$target_cpu" = sparc64 || test "$target_cpu" = riscv64; then
>      TARGET_CFLAGS="$TARGET_CFLAGS -mcmodel=medany"

The patch does not indicate the code model explicitly for riscv32.
It's better we pass the -mcmodel=medlow (suitable for riscv32) to the
compiler in case the compiler is configured with a different code
model by default.

>    fi
>  fi

[snip]

Regards,
Bin