[RFC PATCH 0/3] Extend create-spdx to build kernel spdx info

[RFC PATCH 0/3] Extend create-spdx to build kernel spdx info

[Saul Wold]

This set of changes moves the kernel:do_strip() functionality to 
the packaging code related to split_and_strip_files(). The code
checks the KENREL_IMAGE_STRIP_EXTRA_SECTIONS for the kernel specific
path. This also needs to strip out the "/usr/src/kernel" path since
the kernel uses work-shared, which is added to the search path.

I am still working on testing this, but want to get more eyes on it.

I am most concerned with the change to package.bbclass to handle the
kernel and modules. As mentioned in the other patch thread, there was
some concern about the stripping being too agressive. I am guessing 
that part of the change might need some tuning.

Thanks

Sau!

Saul Wold (3):
  create-spdx: Add kernel work-shared source
  kernel.bbclass: remove do_strip() method
  package: Add support for kernel stripping

 meta/classes/create-spdx.bbclass |  6 +++++-
 meta/classes/kernel.bbclass      | 35 +++-----------------------------
 meta/classes/package.bbclass     | 23 +++++++++++----------
 meta/lib/oe/package.py           |  7 +++++--
 4 files changed, 25 insertions(+), 46 deletions(-)

-- 
2.31.1

[RFC PATCH 1/3] create-spdx: Add kernel work-shared source

[Saul Wold]

Since the kernel source is stored in work-shared, we need to add it
to the search path so the kernel and kernel-modules source code can
be found correctly.

Signed-off-by: Saul Wold <saul.wold@windriver.com>
---
 meta/classes/create-spdx.bbclass | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/meta/classes/create-spdx.bbclass b/meta/classes/create-spdx.bbclass
index 0a4db80aba5..e1ff2bbb787 100644
--- a/meta/classes/create-spdx.bbclass
+++ b/meta/classes/create-spdx.bbclass
@@ -254,6 +254,7 @@ def add_package_sources_from_debug(d, package_doc, spdx_package, package, packag
         Path(d.getVar('PKGD')),
         Path(d.getVar('STAGING_DIR_TARGET')),
         Path(d.getVar('STAGING_DIR_NATIVE')),
+        Path(d.getVar('STAGING_KERNEL_DIR')),
     ]
 
     pkg_data = oe.packagedata.read_subpkgdata_extended(package, d)
@@ -275,7 +276,10 @@ def add_package_sources_from_debug(d, package_doc, spdx_package, package, packag
         for debugsrc in file_data["debugsrc"]:
             ref_id = "NOASSERTION"
             for search in debug_search_paths:
-                debugsrc_path = search / debugsrc.lstrip("/")
+                if debugsrc.startswith("/usr/src/kernel"):
+                    debugsrc_path = search / debugsrc.replace('/usr/src/kernel/', '')
+                else:
+                    debugsrc_path = search / debugsrc.lstrip("/")
                 if not debugsrc_path.exists():
                     continue
 
-- 
2.31.1

[RFC PATCH 2/3] kernel.bbclass: remove do_strip() method

[Saul Wold]

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(-)

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

[RFC PATCH 3/3] package: Add support for kernel stripping

[Saul Wold]

This moves the KERNEL_IMAGE_STRIP_EXTRA_SECTIONS from kernel.bbclass
to the split_and_strip_files() flow. Since the multiprocess_launch is
used here the variable needs to be passed as an argument.

Since is_elf() understands kernel modules there is no need to keep a
seperate list for kernmodules or hardcode the values to runstrip.

Signed-off-by: Saul Wold <saul.wold@windriver.com>
---
 meta/classes/package.bbclass | 23 ++++++++++++-----------
 meta/lib/oe/package.py       |  7 +++++--
 2 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
index 09cd376f4af..2f6863be7ac 100644
--- a/meta/classes/package.bbclass
+++ b/meta/classes/package.bbclass
@@ -1122,7 +1122,6 @@ python split_and_strip_files () {
     #
     elffiles = {}
     symlinks = {}
-    kernmods = []
     staticlibs = []
     inodes = {}
     libdir = os.path.abspath(dvar + os.sep + d.getVar("libdir"))
@@ -1145,9 +1144,6 @@ python split_and_strip_files () {
                 if file in skipfiles:
                     continue
 
-                if file.endswith(".ko") and file.find("/lib/modules/") != -1:
-                    kernmods.append(file)
-                    continue
                 if oe.package.is_static_lib(file):
                     staticlibs.append(file)
                     continue
@@ -1164,8 +1160,11 @@ python split_and_strip_files () {
                 if not s:
                     continue
                 # Check its an executable
-                if (s[stat.ST_MODE] & stat.S_IXUSR) or (s[stat.ST_MODE] & stat.S_IXGRP) or (s[stat.ST_MODE] & stat.S_IXOTH) \
-                        or ((file.startswith(libdir) or file.startswith(baselibdir)) and (".so" in f or ".node" in f)):
+                if (s[stat.ST_MODE] & stat.S_IXUSR) or (s[stat.ST_MODE] & stat.S_IXGRP) \
+                        or (s[stat.ST_MODE] & stat.S_IXOTH) \
+                        or ((file.startswith(libdir) or file.startswith(baselibdir)) \
+                        and (".so" in f or ".node" in f)) \
+                        or (f.startswith('vmlinux') or ".ko" in f):
 
                     if cpath.islink(file):
                         checkelflinks[file] = ltarget
@@ -1310,13 +1309,15 @@ python split_and_strip_files () {
         sfiles = []
         for file in elffiles:
             elf_file = int(elffiles[file])
-            #bb.note("Strip %s" % file)
-            sfiles.append((file, elf_file, strip))
-        for f in kernmods:
-            sfiles.append((f, 16, strip))
+            bb.note("Strip %s" % file)
+            extra_sections = d.getVar('KERNEL_IMAGE_STRIP_EXTRA_SECTIONS')
+            if (extra_sections is not None and file.find('boot/vmlinux')):
+                sfiles.append((file, elf_file, strip, extra_sections))
+            else:
+                sfiles.append((file, elf_file, strip, ''))
         if (d.getVar('PACKAGE_STRIP_STATIC') == '1' or d.getVar('PACKAGE_DEBUG_STATIC_SPLIT') == '1'):
             for f in staticlibs:
-                sfiles.append((f, 16, strip))
+                sfiles.append((f, 16, strip, ''))
 
         oe.utils.multiprocess_launch(oe.package.runstrip, sfiles, d)
 
diff --git a/meta/lib/oe/package.py b/meta/lib/oe/package.py
index dd700cbb0c9..47bc85a4266 100644
--- a/meta/lib/oe/package.py
+++ b/meta/lib/oe/package.py
@@ -16,7 +16,7 @@ def runstrip(arg):
     # 8 - shared library
     # 16 - kernel module
 
-    (file, elftype, strip) = arg
+    (file, elftype, strip, extra_strip_sections) = arg
 
     newmode = None
     if not os.access(file, os.W_OK) or os.access(file, os.R_OK):
@@ -40,6 +40,9 @@ def runstrip(arg):
     # shared or executable:
     elif elftype & 8 or elftype & 4:
         stripcmd.extend(["--remove-section=.comment", "--remove-section=.note"])
+        if "boot/vmlinux" in file and extra_strip_sections != '':
+            for section in extra_strip_sections.split():
+                stripcmd.extend(["--remove-section=" + section])
 
     stripcmd.append(file)
     bb.debug(1, "runstrip: %s" % stripcmd)
@@ -172,7 +175,7 @@ def strip_execs(pn, dstdir, strip_cmd, libdir, base_libdir, d, qa_already_stripp
     sfiles = []
     for file in elffiles:
         elf_file = int(elffiles[file])
-        sfiles.append((file, elf_file, strip_cmd))
+        sfiles.append((file, elf_file, strip_cmd, ''))
 
     oe.utils.multiprocess_launch(runstrip, sfiles, d)
 
-- 
2.31.1

Re: [OE-core] [RFC PATCH 2/3] kernel.bbclass: remove do_strip() method

[Richard Purdie]

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

Re: [OE-core] [RFC PATCH 2/3] kernel.bbclass: remove do_strip() method

[Bruce Ashfield]

[-- Attachment #1: Type: text/plain, Size: 1303 bytes --]

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

-- 
- Thou shalt not follow the NULL pointer, for chaos and madness await thee
at its end
- "Use the force Harry" - Gandalf, Star Trek II

[-- Attachment #2: Type: text/html, Size: 2290 bytes --]

Re: [OE-core] [RFC PATCH 2/3] kernel.bbclass: remove do_strip() method

[Saul Wold]

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
> 

-- 
Sau!

Re: [OE-core] [RFC PATCH 2/3] kernel.bbclass: remove do_strip() method

[Richard Purdie]

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

Re: [OE-core] [RFC PATCH 2/3] kernel.bbclass: remove do_strip() method

[Saul Wold]

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
> 

-- 
Sau!