On Wednesday 04/10 at 15:56 -0700, Calvin Owens wrote:
> 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@xxxxxxxxxx>
> >
> > 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).
What I'm trying to say is: using KBUILD_IMAGE like you suggest allows
more of the existing cases to be combined, like the below (and probably
alpha too, at least).
Thanks,
Calvin
---8<---
diff --git a/scripts/package/buildtar b/scripts/package/buildtar
index 72c91a1b832f..66b4d8d308b6 100755
--- a/scripts/package/buildtar
+++ b/scripts/package/buildtar
@@ -54,8 +54,8 @@ cp -v -- "${objtree}/vmlinux" "${tmpdir}/boot/vmlinux-${KERNELRELEASE}"
# Install arch-specific kernel image(s)
#
case "${ARCH}" in
- x86|i386|x86_64)
- [ -f "${objtree}/arch/x86/boot/bzImage" ] && cp -v -- "${objtree}/arch/x86/boot/bzImage" "${tmpdir}/boot/vmlinuz-${KERNELRELEASE}"
+ x86|i386|x86_64|arm)
+ [ -f "${KBUILD_IMAGE}" ] && cp -v -- "${KBUILD_IMAGE}" "${tmpdir}/boot/vmlinuz-${KERNELRELEASE}"
;;
alpha)
[ -f "${objtree}/arch/alpha/boot/vmlinux.gz" ] && cp -v -- "${objtree}/arch/alpha/boot/vmlinux.gz" "${tmpdir}/boot/vmlinuz-${KERNELRELEASE}"
