Re: [boot-wrapper-aarch64 PATCH] configure: Do not force kernel-dir as a mandatory input

[Andre Przywara <andre.przywara@arm.com>]

On Thu, 23 May 2024 13:49:31 +0200
Akos Denke <akos.denke@arm.com> wrote:

Hi Akos,

> kernel-dir was required to fulfill the real dependencies:
>  * kernel image
>  * dtc
>  * dtb
> 
> There are already checks for all of these real dependencies, so passing
> the `--with-kernel-dir` does not need to be mandatory, in the case these
> dependencies are provided separately.
> 
> An autoconf `--with-kernel-image` argument was added which allows to
> specify the image with its exact location.
> The `dtc` can be already provided via the PATH variable, whilst we
> already have a `--with-dtb` argument to pass a specific `dtb` file.
> These facts together deprecates the mandatoriness of kernel-dir and
> allows to provide any arbitrary kernel image, with dtb, dtc and
> initramfs, without a need for an assumed kernel directory structure.
> 
> This makes the boot-wrapper building easier for prebuilt kernels, where
> we do not control the directory structure of the kernel artifacts.

So the patch itself looks fine to me, and it seems to work as described in
my quick testing.
The only thing I would like to change is the patch title: "dropping the
requirement for --with-kernel-dir" is more a side effect, the main
feature here seems to be --with-kernel-image, which indeed solves a
long-standing annoyance (so thanks for that!)

That's just a nit, though, so:

> Signed-off-by: Akos Denke <akos.denke@arm.com>

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre

> ---
>  configure.ac | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 9e3b722..768506f 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -27,14 +27,18 @@ AS_IF([test "x$BOOTWRAPPER_ES" = x32 -a "x$KERNEL_ES" != x32],
>  
>  # Allow a user to pass --with-kernel-dir
>  AC_ARG_WITH([kernel-dir],
> -	AS_HELP_STRING([--with-kernel-dir], [specify the root Linux kernel build directory (required)]),
> -	AC_SUBST([KERN_DIR], [$withval]),
> -	AC_MSG_ERROR([No kernel directory specified. Use --with-kernel-dir]))
> +	AS_HELP_STRING([--with-kernel-dir], [specify the root Linux kernel build directory]),
> +	AC_SUBST([KERN_DIR], [$withval]))
>  AS_IF([test "x$KERNEL_ES" = x32],
>  	[KERN_IMAGE=$KERN_DIR/arch/arm/boot/zImage],
>  	[KERN_IMAGE=$KERN_DIR/arch/arm64/boot/Image])
>  KERN_DTB=$KERN_DIR/arch/arm64/boot/dts/arm/fvp-base-revc.dtb
>  
> +# Allow a user to pass a specific kernel image file
> +AC_ARG_WITH([kernel-image],
> +	AS_HELP_STRING([--with-kernel-image], [specify kernel image]),
> +	AC_SUBST([KERN_IMAGE], [$withval]))
> +
>  # Allow the user to override the default DTB
>  AC_ARG_WITH([dtb],
>  	AS_HELP_STRING([--with-dtb], [Specify a particular DTB to use]),
> @@ -52,11 +56,6 @@ AS_IF([test "x$X_IMAGE" == "x"], [],
>  AC_SUBST([XEN_IMAGE], [$X_IMAGE])
>  AM_CONDITIONAL([XEN], [test "x$X_IMAGE" != "x"])
>  
> -# Ensure that the user has provided us with a sane kernel dir.
> -if ! test -d $KERN_DIR; then
> -	AC_MSG_ERROR([Could not find Linux kernel dir: $KERN_DIR.])
> -fi
> -
>  AC_MSG_CHECKING([whether DTB file exists])
>  if ! test -f $KERN_DTB; then
>  	AC_MSG_RESULT([no])
> @@ -126,7 +125,7 @@ AC_PROG_SED
>  AC_PROG_LN_S
>  AC_PATH_PROG([DTC], dtc, error, [$PATH$PATH_SEPARATOR$KERN_DIR/scripts/dtc])
>  if test "x$DTC" = "xerror"; then
> -	AC_MSG_ERROR([cannot find the device tree compiler (dtc)])
> +	AC_MSG_ERROR([cannot find the device tree compiler (dtc). Use --with-kernel-dir or put dtc on the PATH])
>  fi
>  AC_CHECK_TOOL(LD, ld)
>  
> @@ -139,8 +138,12 @@ echo ""
>  echo "  Boot wrapper configuration"
>  echo "  =========================="
>  echo ""
> +if test "x${KERN_DIR}" != "x"; then
>  echo "  Linux kernel build dir:            ${KERN_DIR}"
> +fi
> +echo "  Linux kernel image:                ${KERN_IMAGE}"
>  echo "  Device tree blob:                  ${KERN_DTB}"
> +echo "  Device tree compiler:              ${DTC}"
>  echo "  Linux kernel command line:         ${CMDLINE}"
>  echo "  Embedded initrd:                   ${FILESYSTEM:-NONE}"
>  echo "  Use PSCI?                          ${USE_PSCI}"


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel