| Message ID | 20220107212446.132386-3-saul.wold@windriver.com |
|---|---|
| State | New |
| Headers | show |
| Series | Extend create-spdx to build kernel spdx info | expand |
On Fri, 2022-01-07 at 13:24 -0800, Saul Wold wrote: > Move the do_strip() functionality to a more common location in the > package split_and_strip_files() flow. This makes it possible for the > extended packaging data to be generated correctly for the kernel and > kernel modules. The KERNEL_IMAGE_STRIP_EXTRA_SECTIONS is reused in > the runstrip() part of package stripping. > > Signed-off-by: Saul Wold <saul.wold@windriver.com> > --- > meta/classes/kernel.bbclass | 35 +++-------------------------------- > 1 file changed, 3 insertions(+), 32 deletions(-) As I mentioned in previous mails, I think this will mean the do_deploy output will change and become unstripped which may cause a problem for some platforms. We likely need to keep a strip task but call the new strip function from it? I personally think this is a reasonable direction apart from that though, at least in principle. Cheers, Richard
On Fri, Jan 7, 2022 at 6:19 PM Richard Purdie < richard.purdie@linuxfoundation.org> wrote: > On Fri, 2022-01-07 at 13:24 -0800, Saul Wold wrote: > > Move the do_strip() functionality to a more common location in the > > package split_and_strip_files() flow. This makes it possible for the > > extended packaging data to be generated correctly for the kernel and > > kernel modules. The KERNEL_IMAGE_STRIP_EXTRA_SECTIONS is reused in > > the runstrip() part of package stripping. > > > > Signed-off-by: Saul Wold <saul.wold@windriver.com> > > --- > > meta/classes/kernel.bbclass | 35 +++-------------------------------- > > 1 file changed, 3 insertions(+), 32 deletions(-) > > As I mentioned in previous mails, I think this will mean the do_deploy > output > will change and become unstripped which may cause a problem for some > platforms. > > We likely need to keep a strip task but call the new strip function from > it? > > I personally think this is a reasonable direction apart from that though, > at > least in principle. > Agreed. It looks ok to me as well, with us making sure that do_deploy can remain as it was. Bruce > > Cheers, > > Richard > >
On 1/8/22 11:27, Bruce Ashfield wrote: > > > On Fri, Jan 7, 2022 at 6:19 PM Richard Purdie > <richard.purdie@linuxfoundation.org > <mailto:richard.purdie@linuxfoundation.org>> wrote: > > On Fri, 2022-01-07 at 13:24 -0800, Saul Wold wrote: > > Move the do_strip() functionality to a more common location in the > > package split_and_strip_files() flow. This makes it possible for the > > extended packaging data to be generated correctly for the kernel and > > kernel modules. The KERNEL_IMAGE_STRIP_EXTRA_SECTIONS is reused in > > the runstrip() part of package stripping. > > > > Signed-off-by: Saul Wold <saul.wold@windriver.com > <mailto:saul.wold@windriver.com>> > > --- > > meta/classes/kernel.bbclass | 35 +++-------------------------------- > > 1 file changed, 3 insertions(+), 32 deletions(-) > > As I mentioned in previous mails, I think this will mean the > do_deploy output > will change and become unstripped which may cause a problem for some > platforms. > > We likely need to keep a strip task but call the new strip function > from it? > > I personally think this is a reasonable direction apart from that > though, at > least in principle. > > > Agreed. It looks ok to me as well, with us making sure that do_deploy > can remain as it was. > I have been looking into this and realized it's maybe a little more complex since the do_sizecheck() needs to run also before do_deploy. Currently, I believe the ordering is: do_compile() do_compile_kernelmodules() do_kernel_linux_images() do_strip() do_sizecheck() do_install do_package() do_packagedata() do_deploy() If I move the strip() process to package, I also have to move when do_sizecheck() runs. As you mention we need to keep what gets deployed to be the same with and without the package split_and_strip_files(). I understand what your talking about having the vmlinux image (which is the one getting stripped) is the same before and after this changeset, I don't think it's an issue of the deployed vmlinux being un-stripped but the vmlinux being stripped in the deploy dir. We can't really call the package.bbclass strip from the kernel bbclass since it's part of the package process and called in a multiprocess() way. One thought is to have a copy of the vmlinux that gets packaged and stripped via the package process vs the vmlinux binary that gets deployed and optionally stripped when KERNEL_IMAGE_STRIP_EXTRA_SECTIONS is set. Maybe we can talk though this during the Tech call Tuesday. Sau! > Bruce > > > Cheers, > > Richard > > > > -- > - Thou shalt not follow the NULL pointer, for chaos and madness await > thee at its end > - "Use the force Harry" - Gandalf, Star Trek II >
On Mon, 2022-01-10 at 16:42 -0800, Saul Wold wrote: > > On 1/8/22 11:27, Bruce Ashfield wrote: > > > > > > On Fri, Jan 7, 2022 at 6:19 PM Richard Purdie > > <richard.purdie@linuxfoundation.org > > <mailto:richard.purdie@linuxfoundation.org>> wrote: > > > > On Fri, 2022-01-07 at 13:24 -0800, Saul Wold wrote: > > > Move the do_strip() functionality to a more common location in the > > > package split_and_strip_files() flow. This makes it possible for the > > > extended packaging data to be generated correctly for the kernel and > > > kernel modules. The KERNEL_IMAGE_STRIP_EXTRA_SECTIONS is reused in > > > the runstrip() part of package stripping. > > > > > > Signed-off-by: Saul Wold <saul.wold@windriver.com > > <mailto:saul.wold@windriver.com>> > > > --- > > > meta/classes/kernel.bbclass | 35 +++-------------------------------- > > > 1 file changed, 3 insertions(+), 32 deletions(-) > > > > As I mentioned in previous mails, I think this will mean the > > do_deploy output > > will change and become unstripped which may cause a problem for some > > platforms. > > > > We likely need to keep a strip task but call the new strip function > > from it? > > > > I personally think this is a reasonable direction apart from that > > though, at > > least in principle. > > > > > > Agreed. It looks ok to me as well, with us making sure that do_deploy > > can remain as it was. > > > I have been looking into this and realized it's maybe a little more > complex since the do_sizecheck() needs to run also before do_deploy. > > Currently, I believe the ordering is: > > do_compile() > do_compile_kernelmodules() > do_kernel_linux_images() > do_strip() > do_sizecheck() > do_install > do_package() > do_packagedata() > do_deploy() > > If I move the strip() process to package, I also have to move when > do_sizecheck() runs. As you mention we need to keep what gets deployed > to be the same with and without the package split_and_strip_files(). > > I understand what your talking about having the vmlinux image (which is > the one getting stripped) is the same before and after this changeset, I > don't think it's an issue of the deployed vmlinux being un-stripped but > the vmlinux being stripped in the deploy dir. > > We can't really call the package.bbclass strip from the kernel bbclass > since it's part of the package process and called in a multiprocess() way. This bit puzzles me. Why can't we manually call the function from kernel bbclass? It might not be particularly nice given the multiprocess bits but I'm sure it can be done and likely the lesser of several evils. If it works in a multiprocess context, it should work without? (the other way around is much harder). Cheers, Richard
On 1/11/22 03:27, Richard Purdie wrote: > On Mon, 2022-01-10 at 16:42 -0800, Saul Wold wrote: >> >> On 1/8/22 11:27, Bruce Ashfield wrote: >>> >>> >>> On Fri, Jan 7, 2022 at 6:19 PM Richard Purdie >>> <richard.purdie@linuxfoundation.org >>> <mailto:richard.purdie@linuxfoundation.org>> wrote: >>> >>> On Fri, 2022-01-07 at 13:24 -0800, Saul Wold wrote: >>> > Move the do_strip() functionality to a more common location in the >>> > package split_and_strip_files() flow. This makes it possible for the >>> > extended packaging data to be generated correctly for the kernel and >>> > kernel modules. The KERNEL_IMAGE_STRIP_EXTRA_SECTIONS is reused in >>> > the runstrip() part of package stripping. >>> > >>> > Signed-off-by: Saul Wold <saul.wold@windriver.com >>> <mailto:saul.wold@windriver.com>> >>> > --- >>> > meta/classes/kernel.bbclass | 35 +++-------------------------------- >>> > 1 file changed, 3 insertions(+), 32 deletions(-) >>> >>> As I mentioned in previous mails, I think this will mean the >>> do_deploy output >>> will change and become unstripped which may cause a problem for some >>> platforms. >>> >>> We likely need to keep a strip task but call the new strip function >>> from it? >>> >>> I personally think this is a reasonable direction apart from that >>> though, at >>> least in principle. >>> >>> >>> Agreed. It looks ok to me as well, with us making sure that do_deploy >>> can remain as it was. >>> >> I have been looking into this and realized it's maybe a little more >> complex since the do_sizecheck() needs to run also before do_deploy. >> >> Currently, I believe the ordering is: >> >> do_compile() >> do_compile_kernelmodules() >> do_kernel_linux_images() >> do_strip() >> do_sizecheck() >> do_install >> do_package() >> do_packagedata() >> do_deploy() >> >> If I move the strip() process to package, I also have to move when >> do_sizecheck() runs. As you mention we need to keep what gets deployed >> to be the same with and without the package split_and_strip_files(). >> >> I understand what your talking about having the vmlinux image (which is >> the one getting stripped) is the same before and after this changeset, I >> don't think it's an issue of the deployed vmlinux being un-stripped but >> the vmlinux being stripped in the deploy dir. >> >> We can't really call the package.bbclass strip from the kernel bbclass >> since it's part of the package process and called in a multiprocess() way. > > This bit puzzles me. Why can't we manually call the function from kernel > bbclass? It might not be particularly nice given the multiprocess bits but I'm > sure it can be done and likely the lesser of several evils. If it works in a > multiprocess context, it should work without? (the other way around is much > harder). > I might not of explained myself well, since I did not mention the gathering of debug info (extended packagedata) occurs during the do_package() phase. Yes, I can use the runstrip() function from oe.package in kernel.bbclass, but it still means the kernel image is stripped by the time it gets to the do_package when we want to gather the debuginfo, but can't from the already stripped kernel image. It's a task ordering issue in combination with what happens in the split_and_strip_files() to get the extended packagedata that the create-spdx bbclass then uses. The kernel do_strip() happens on the vmlinux image in B, while the package stripping happens in the PKGD directory, while do_deploy goes back to B for the kernel image (KERNEL_IMAGETTYPES) which might not be the vmlinux. It's a twisty little process with back and forth. I think this is where I got a little confused also, I thought do_deploy was from either D (WORKDIR/image) until I re-read the run.do_deploy script itself! (my bad). I could add special handling in the kernel to do the strip/split Hope that makes better sense. Sau! > Cheers, > > Richard >
diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass index 7685c6921fa..30e67abb936 100644 --- a/meta/classes/kernel.bbclass +++ b/meta/classes/kernel.bbclass @@ -421,7 +421,7 @@ do_compile_kernelmodules() { bbnote "no modules to compile" fi } -addtask compile_kernelmodules after do_compile before do_strip +addtask compile_kernelmodules after do_compile before do_sizecheck kernel_do_install() { # @@ -698,36 +698,7 @@ do_kernel_link_images() { ln -sf ../../../vmlinux.64 fi } -addtask kernel_link_images after do_compile before do_strip - -do_strip() { - if [ -n "${KERNEL_IMAGE_STRIP_EXTRA_SECTIONS}" ]; then - if ! (echo "${KERNEL_IMAGETYPES}" | grep -wq "vmlinux"); then - bbwarn "image type(s) will not be stripped (not supported): ${KERNEL_IMAGETYPES}" - return - fi - - cd ${B} - headers=`"$CROSS_COMPILE"readelf -S ${KERNEL_OUTPUT_DIR}/vmlinux | \ - grep "^ \{1,\}\[[0-9 ]\{1,\}\] [^ ]" | \ - sed "s/^ \{1,\}\[[0-9 ]\{1,\}\] //" | \ - gawk '{print $1}'` - - for str in ${KERNEL_IMAGE_STRIP_EXTRA_SECTIONS}; do { - if ! (echo "$headers" | grep -q "^$str$"); then - bbwarn "Section not found: $str"; - fi - - "$CROSS_COMPILE"strip -s -R $str ${KERNEL_OUTPUT_DIR}/vmlinux - }; done - - bbnote "KERNEL_IMAGE_STRIP_EXTRA_SECTIONS is set, stripping sections:" \ - "${KERNEL_IMAGE_STRIP_EXTRA_SECTIONS}" - fi; -} -do_strip[dirs] = "${B}" - -addtask strip before do_sizecheck after do_kernel_link_images +addtask kernel_link_images after do_compile before do_sizecheck # Support checking the kernel size since some kernels need to reside in partitions # with a fixed length or there is a limit in transferring the kernel to memory. @@ -755,7 +726,7 @@ do_sizecheck() { } do_sizecheck[dirs] = "${B}" -addtask sizecheck before do_install after do_strip +addtask sizecheck before do_install after do_kernel_link_images inherit kernel-artifact-names
Move the do_strip() functionality to a more common location in the package split_and_strip_files() flow. This makes it possible for the extended packaging data to be generated correctly for the kernel and kernel modules. The KERNEL_IMAGE_STRIP_EXTRA_SECTIONS is reused in the runstrip() part of package stripping. Signed-off-by: Saul Wold <saul.wold@windriver.com> --- meta/classes/kernel.bbclass | 35 +++-------------------------------- 1 file changed, 3 insertions(+), 32 deletions(-)