On Fri, Apr 12, 2024 at 12:23:42PM -0700, Calvin Owens wrote:
> 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.
I think your first assumption is likely correct although I have not
looked back at the history to confirm that. I am not as sure on the
second statement, mainly just because not all kernel images are
compressed so they wouldn't necessarily make sense as vmlinuz. I think
it just happens that many of the most popular architectures have default
compressed kernel images.
> > 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).
Yes, I think including that reasoning in the commit message makes sense,
since it is justification for changing the status quo.
> > > 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).
I see you already sent v2, which I will review shortly, but doing this
change certainly seems reasonable to me. We could add a comment above it
like
# Architectures with just a compressed KBUILD_IMAGE
> ---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}"
