Received-SPF: pass (google.com: domain of linux-kernel+bounces-139035-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Wed, 10 Apr 2024 10:04:50 -0700
From: Nathan Chancellor <nathan@kernel.org>
To: Calvin Owens <calvin@wbinvd.org>
Cc: linux-kernel@vger.kernel.org, Masahiro Yamada <masahiroy@kernel.org>,
	Nicolas Schier <nicolas@fjasle.eu>, linux-kbuild@vger.kernel.org
Subject: Re: [PATCH] kbuild: buildtar: Add arm support
Message-ID: <20240410170450.GA1828262@dev-arch.thelio-3990X>
References: <e7c14a0d329e28bdcda21376b54a43c85a4aaf3f.1712682861.git.calvin@wbinvd.org>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <e7c14a0d329e28bdcda21376b54a43c85a4aaf3f.1712682861.git.calvin@wbinvd.org>

Hi Calvin,

Thanks for the patch!

On Tue, Apr 09, 2024 at 10:17:07AM -0700, Calvin Owens wrote:
> Make 'make tar-pkg' and friends work on 32-bit arm.
> 
> Signed-off-by: Calvin Owens <calvin@wbinvd.org>

Technically speaking, buildtar works for 32-bit ARM right now (I use it
almost daily), this is just explicitly adding it to the supported list
to avoid the warning and putting zImage at vmlinuz-${KERNELRELEASE}
instead of vmlinux-kbuild-${KERNELRELEASE}, right?

That said, looks mostly fine to me, one comment below.

Before:

  './System.map' -> 'tar-install/boot/System.map-6.9.0-rc3-00023-g2c71fdf02a95'
  '.config' -> 'tar-install/boot/config-6.9.0-rc3-00023-g2c71fdf02a95'
  './vmlinux' -> 'tar-install/boot/vmlinux-6.9.0-rc3-00023-g2c71fdf02a95'
  'arch/arm/boot/zImage' -> 'tar-install/boot/vmlinux-kbuild-6.9.0-rc3-00023-g2c71fdf02a95'

  ** ** **  WARNING  ** ** **

  Your architecture did not define any architecture-dependent files
  to be placed into the tarball. Please add those to scripts/package/buildtar ...

After:

  './System.map' -> 'tar-install/boot/System.map-6.9.0-rc3-00023-g2c71fdf02a95-dirty'
  '.config' -> 'tar-install/boot/config-6.9.0-rc3-00023-g2c71fdf02a95-dirty'
  './vmlinux' -> 'tar-install/boot/vmlinux-6.9.0-rc3-00023-g2c71fdf02a95-dirty'
  './arch/arm/boot/zImage' -> 'tar-install/boot/vmlinuz-6.9.0-rc3-00023-g2c71fdf02a95-dirty'

and the location of zImage is the only thing that changes as expected.

> ---
>  scripts/package/buildtar | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/scripts/package/buildtar b/scripts/package/buildtar
> index 72c91a1b832f..0939f9eabbf2 100755
> --- a/scripts/package/buildtar
> +++ b/scripts/package/buildtar
> @@ -101,6 +101,9 @@ case "${ARCH}" in
>  			fi
>  		done
>  		;;
> +	arm)
> +		[ -f "${objtree}/arch/arm/boot/zImage" ] && cp -v -- "${objtree}/arch/arm/boot/zImage" "${tmpdir}/boot/vmlinuz-${KERNELRELEASE}"

While it probably does not matter too much, it would be more proper to
make this

  [ -f "${KBUILD_IMAGE}" ] && cp -v -- "${KBUILD_IMAGE}" "${tmpdir}/boot/vmlinuz-${KERNELRELEASE}"

as the current line does not work with CONFIG_XIP_KERNEL=y, since zImage
does not exist (KBUILD_IMAGE is arch/arm/boot/xipImage with this
configuration)

  $ ls arch/arm/boot
  compressed  dts  xipImage

resulting in buildtar failing because

  [ -f "${objtree}/arch/arm/boot/zImage" ]

fails and is the last statement that runs in the script (and the tar
package is not really complete in this configuration anyways).

Prior to this change, the correct image would get placed into the
tarball.

  'arch/arm/boot/xipImage' -> 'tar-install/boot/vmlinux-kbuild-6.9.0-rc3-00023-g2c71fdf02a95'

> +		;;
>  	*)
>  		[ -f "${KBUILD_IMAGE}" ] && cp -v -- "${KBUILD_IMAGE}" "${tmpdir}/boot/vmlinux-kbuild-${KERNELRELEASE}"
>  		echo "" >&2
> -- 
> 2.39.2
> 

Cheers,
Nathan