Received-SPF: pass (google.com: domain of linux-kernel+bounces-139509-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Wed, 10 Apr 2024 15:56:11 -0700
From: Calvin Owens <calvin@wbinvd.org>
To: Nathan Chancellor <nathan@kernel.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: <ZhcZCwMmANQS39s2@mozart.vkv.me>
References: <e7c14a0d329e28bdcda21376b54a43c85a4aaf3f.1712682861.git.calvin@wbinvd.org>
 <20240410170450.GA1828262@dev-arch.thelio-3990X>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20240410170450.GA1828262@dev-arch.thelio-3990X>

On Wednesday 04/10 at 10:04 -0700, Nathan Chancellor wrote:
> 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?

Exactly. I assumed (maybe incorrectly?) the vmlinux-kbuild-* name was
generic "unimplemented" filler that was meant to be replaced. It seems
like the vmlinuz-* naming has sort of become a de facto standard in the
tar-pkgs.

The context for me is a pile of scripts that build kernels and boot them
with QEMU on arm and arm64: it's convenient if the tar-pkg structure is
consistent between the two (and across other architectures too).

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

Makes sense, thanks. Although...

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

..it ends up looking almost identical to the default case. Does it make
make more sense to change the destination in the default case and remove
the warning? I'm not sure if anything might rely on the current
behavior, it goes all the way back (git sha 6d983feab809).

Thanks,
Calvin

> > -- 
> > 2.39.2
> > 
> 
> Cheers,
> Nathan