linux-kbuild.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] kbuild: buildtar: Add arm support
@ 2024-04-09 17:17 Calvin Owens
  2024-04-10 17:04 ` Nathan Chancellor
  0 siblings, 1 reply; 7+ messages in thread
From: Calvin Owens @ 2024-04-09 17:17 UTC (permalink / raw)
  To: linux-kernel
  Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, linux-kbuild

Make 'make tar-pkg' and friends work on 32-bit arm.

Signed-off-by: Calvin Owens <calvin@wbinvd.org>
---
 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}"
+		;;
 	*)
 		[ -f "${KBUILD_IMAGE}" ] && cp -v -- "${KBUILD_IMAGE}" "${tmpdir}/boot/vmlinux-kbuild-${KERNELRELEASE}"
 		echo "" >&2
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] kbuild: buildtar: Add arm support
  2024-04-09 17:17 [PATCH] kbuild: buildtar: Add arm support Calvin Owens
@ 2024-04-10 17:04 ` Nathan Chancellor
  2024-04-10 22:56   ` Calvin Owens
  0 siblings, 1 reply; 7+ messages in thread
From: Nathan Chancellor @ 2024-04-10 17:04 UTC (permalink / raw)
  To: Calvin Owens; +Cc: linux-kernel, Masahiro Yamada, Nicolas Schier, linux-kbuild

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] kbuild: buildtar: Add arm support
  2024-04-10 17:04 ` Nathan Chancellor
@ 2024-04-10 22:56   ` Calvin Owens
  2024-04-12 19:23     ` Calvin Owens
  2024-04-12 19:26     ` [PATCH v2] kbuild: buildtar: Add explicit 32-bit " Calvin Owens
  0 siblings, 2 replies; 7+ messages in thread
From: Calvin Owens @ 2024-04-10 22:56 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: linux-kernel, Masahiro Yamada, Nicolas Schier, linux-kbuild

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] kbuild: buildtar: Add arm support
  2024-04-10 22:56   ` Calvin Owens
@ 2024-04-12 19:23     ` Calvin Owens
  2024-04-12 21:47       ` Nathan Chancellor
  2024-04-12 19:26     ` [PATCH v2] kbuild: buildtar: Add explicit 32-bit " Calvin Owens
  1 sibling, 1 reply; 7+ messages in thread
From: Calvin Owens @ 2024-04-12 19:23 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: linux-kernel, Masahiro Yamada, Nicolas Schier, linux-kbuild

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@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).

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}"

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2] kbuild: buildtar: Add explicit 32-bit arm support
  2024-04-10 22:56   ` Calvin Owens
  2024-04-12 19:23     ` Calvin Owens
@ 2024-04-12 19:26     ` Calvin Owens
  2024-04-12 21:48       ` Nathan Chancellor
  1 sibling, 1 reply; 7+ messages in thread
From: Calvin Owens @ 2024-04-12 19:26 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: linux-kernel, Masahiro Yamada, Nicolas Schier, linux-kbuild

Implement buildtar for 32-bit arm, so the zImage (or xipimage) appears
at boot/vmlinuz-$foo, rather than at boot/vmlinux-kbuild-$foo, matching
the structure of the tar-pkg on arm64 and other architectures.

Link: https://lore.kernel.org/all/e7c14a0d329e28bdcda21376b54a43c85a4aaf3f.1712682861.git.calvin@wbinvd.org/
Signed-off-by: Calvin Owens <calvin@wbinvd.org>
---
 scripts/package/buildtar | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/scripts/package/buildtar b/scripts/package/buildtar
index 72c91a1b832f..86035c990aec 100755
--- a/scripts/package/buildtar
+++ b/scripts/package/buildtar
@@ -101,6 +101,9 @@ case "${ARCH}" in
 			fi
 		done
 		;;
+	arm)
+		[ -f "${KBUILD_IMAGE}" ] && cp -v -- "${KBUILD_IMAGE}" "${tmpdir}/boot/vmlinuz-${KERNELRELEASE}"
+		;;
 	*)
 		[ -f "${KBUILD_IMAGE}" ] && cp -v -- "${KBUILD_IMAGE}" "${tmpdir}/boot/vmlinux-kbuild-${KERNELRELEASE}"
 		echo "" >&2
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] kbuild: buildtar: Add arm support
  2024-04-12 19:23     ` Calvin Owens
@ 2024-04-12 21:47       ` Nathan Chancellor
  0 siblings, 0 replies; 7+ messages in thread
From: Nathan Chancellor @ 2024-04-12 21:47 UTC (permalink / raw)
  To: Calvin Owens; +Cc: linux-kernel, Masahiro Yamada, Nicolas Schier, linux-kbuild

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@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.

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}"

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] kbuild: buildtar: Add explicit 32-bit arm support
  2024-04-12 19:26     ` [PATCH v2] kbuild: buildtar: Add explicit 32-bit " Calvin Owens
@ 2024-04-12 21:48       ` Nathan Chancellor
  0 siblings, 0 replies; 7+ messages in thread
From: Nathan Chancellor @ 2024-04-12 21:48 UTC (permalink / raw)
  To: Calvin Owens; +Cc: linux-kernel, Masahiro Yamada, Nicolas Schier, linux-kbuild

On Fri, Apr 12, 2024 at 12:26:06PM -0700, Calvin Owens wrote:
> Implement buildtar for 32-bit arm, so the zImage (or xipimage) appears
> at boot/vmlinuz-$foo, rather than at boot/vmlinux-kbuild-$foo, matching
> the structure of the tar-pkg on arm64 and other architectures.
> 
> Link: https://lore.kernel.org/all/e7c14a0d329e28bdcda21376b54a43c85a4aaf3f.1712682861.git.calvin@wbinvd.org/
> Signed-off-by: Calvin Owens <calvin@wbinvd.org>

Looks good to me, especially with the added justification of matching
other architectures.

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  scripts/package/buildtar | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/scripts/package/buildtar b/scripts/package/buildtar
> index 72c91a1b832f..86035c990aec 100755
> --- a/scripts/package/buildtar
> +++ b/scripts/package/buildtar
> @@ -101,6 +101,9 @@ case "${ARCH}" in
>  			fi
>  		done
>  		;;
> +	arm)
> +		[ -f "${KBUILD_IMAGE}" ] && cp -v -- "${KBUILD_IMAGE}" "${tmpdir}/boot/vmlinuz-${KERNELRELEASE}"
> +		;;
>  	*)
>  		[ -f "${KBUILD_IMAGE}" ] && cp -v -- "${KBUILD_IMAGE}" "${tmpdir}/boot/vmlinux-kbuild-${KERNELRELEASE}"
>  		echo "" >&2
> -- 
> 2.39.2
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2024-04-12 21:48 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-09 17:17 [PATCH] kbuild: buildtar: Add arm support Calvin Owens
2024-04-10 17:04 ` Nathan Chancellor
2024-04-10 22:56   ` Calvin Owens
2024-04-12 19:23     ` Calvin Owens
2024-04-12 21:47       ` Nathan Chancellor
2024-04-12 19:26     ` [PATCH v2] kbuild: buildtar: Add explicit 32-bit " Calvin Owens
2024-04-12 21:48       ` Nathan Chancellor

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).