[PATCH] runqemu: support uImage kernel

[PATCH] runqemu: support uImage kernel

[Adrian Freihofer]

u-boot's uImage kernel format is not supported by Qemu. Extract the
kernel from the uImage to a temporary directory.

Signed-off-by: Adrian Freihofer <adrian.freihofer@siemens.com>
---
 scripts/runqemu | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/scripts/runqemu b/scripts/runqemu
index ba0b701aff..fa630fc7a4 100755
--- a/scripts/runqemu
+++ b/scripts/runqemu
@@ -163,6 +163,7 @@ class BaseConfig(object):
         self.bios = ''
         self.kernel_cmdline = ''
         self.kernel_cmdline_script = ''
+        self.temp_kernel_dir = None
         self.bootparams = ''
         self.dtb = ''
         self.fstype = ''
@@ -1422,10 +1423,26 @@ class BaseConfig(object):
         self.setup_serial()
         self.setup_vga()
 
+    def dump_uimage(self):
+        if self.kernel and self.get('KERNEL_IMAGETYPE') == 'uImage':
+            import tempfile
+            self.temp_kernel_dir = tempfile.mkdtemp()
+            kernel = os.path.join(self.temp_kernel_dir, 'image')
+            dumpimage_bin = os.path.join(self.bindir_native, 'dumpimage')
+            image_link_name = self.get('IMAGE_LINK_NAME')
+            logger.info('Running: %s' % str([dumpimage_bin, "-o ", kernel, self.kernel]))
+            staging_bindir_native = '%s/usr/bin' % self.get('STAGING_DIR_NATIVE')
+            dumpimage_bin = os.path.join(staging_bindir_native, 'dumpimage')
+            subprocess.run([dumpimage_bin, "-p", "0", "-o", kernel, self.kernel])
+            return kernel
+        return self.kernel
+
     def start_qemu(self):
         import shlex
-        if self.kernel:
-            kernel_opts = "-kernel %s -append '%s %s %s %s'" % (self.kernel, self.kernel_cmdline,
+
+        kernel = self.dump_uimage()
+        if kernel:
+            kernel_opts = "-kernel %s -append '%s %s %s %s'" % (kernel, self.kernel_cmdline,
                                                                 self.kernel_cmdline_script, self.get('QB_KERNEL_CMDLINE_APPEND'),
                                                                 self.bootparams)
             if self.bios:
@@ -1481,6 +1498,9 @@ class BaseConfig(object):
             shutil.rmtree(self.rootfs)
             shutil.rmtree('%s.pseudo_state' % self.rootfs)
 
+        if self.temp_kernel_dir:
+            shutil.rmtree(self.temp_kernel_dir)
+
         self.cleaned = True
 
     def run_bitbake_env(self, mach=None):
-- 
2.26.3

Re: [OE-core] [PATCH] runqemu: support uImage kernel

[Bruce Ashfield]

On Fri, Apr 16, 2021 at 12:43 PM Adrian Freihofer
<adrian.freihofer@gmail.com> wrote:
>
> u-boot's uImage kernel format is not supported by Qemu. Extract the
> kernel from the uImage to a temporary directory.
>

I'd suggest that we should document in the commit log why someone
wouldn't just build a kernel image format that is already supported by
runqemu (which is what I typically do). I'm guessing it is some sort
of h/w boot being re-purposed to qemu emulation, and this is to avoid
building a second kernel image ?

Additionally, this should have some sort of test, or the code is going
to bitrot due to not being run by anything in core.

> Signed-off-by: Adrian Freihofer <adrian.freihofer@siemens.com>
> ---
>  scripts/runqemu | 24 ++++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/runqemu b/scripts/runqemu
> index ba0b701aff..fa630fc7a4 100755
> --- a/scripts/runqemu
> +++ b/scripts/runqemu
> @@ -163,6 +163,7 @@ class BaseConfig(object):
>          self.bios = ''
>          self.kernel_cmdline = ''
>          self.kernel_cmdline_script = ''
> +        self.temp_kernel_dir = None
>          self.bootparams = ''
>          self.dtb = ''
>          self.fstype = ''
> @@ -1422,10 +1423,26 @@ class BaseConfig(object):
>          self.setup_serial()
>          self.setup_vga()
>
> +    def dump_uimage(self):
> +        if self.kernel and self.get('KERNEL_IMAGETYPE') == 'uImage':
> +            import tempfile
> +            self.temp_kernel_dir = tempfile.mkdtemp()
> +            kernel = os.path.join(self.temp_kernel_dir, 'image')
> +            dumpimage_bin = os.path.join(self.bindir_native, 'dumpimage')
> +            image_link_name = self.get('IMAGE_LINK_NAME')
> +            logger.info('Running: %s' % str([dumpimage_bin, "-o ", kernel, self.kernel]))
> +            staging_bindir_native = '%s/usr/bin' % self.get('STAGING_DIR_NATIVE')
> +            dumpimage_bin = os.path.join(staging_bindir_native, 'dumpimage')
> +            subprocess.run([dumpimage_bin, "-p", "0", "-o", kernel, self.kernel])
> +            return kernel
> +        return self.kernel
> +
>      def start_qemu(self):
>          import shlex
> -        if self.kernel:
> -            kernel_opts = "-kernel %s -append '%s %s %s %s'" % (self.kernel, self.kernel_cmdline,
> +
> +        kernel = self.dump_uimage()
> +        if kernel:
> +            kernel_opts = "-kernel %s -append '%s %s %s %s'" % (kernel, self.kernel_cmdline,
>                                                                  self.kernel_cmdline_script, self.get('QB_KERNEL_CMDLINE_APPEND'),
>                                                                  self.bootparams)

If the kernel never fails from the dup_uimage() command, why test for
it ? I don't see how the routine could return something that doesn't
pass the test. If so, aren't we going to run into more cryptic errors
(or traceback) later if there was some sort of failure.

The existing code doesn't look like it is completely error proofed
either, that just came to mind when looking at the patch.

Bruce

>              if self.bios:
> @@ -1481,6 +1498,9 @@ class BaseConfig(object):
>              shutil.rmtree(self.rootfs)
>              shutil.rmtree('%s.pseudo_state' % self.rootfs)
>
> +        if self.temp_kernel_dir:
> +            shutil.rmtree(self.temp_kernel_dir)
> +
>          self.cleaned = True
>
>      def run_bitbake_env(self, mach=None):
> --
> 2.26.3
>
>
> 
>


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

Re: [OE-core] [PATCH] runqemu: support uImage kernel

[Adrian Freihofer]

Hi Bruce,

Thank you for asking the right questions. This patch should be ignored.

On Fri, 2021-04-16 at 14:06 -0400, Bruce Ashfield wrote:
> On Fri, Apr 16, 2021 at 12:43 PM Adrian Freihofer
> <adrian.freihofer@gmail.com> wrote:
> > u-boot's uImage kernel format is not supported by Qemu. Extract the
> > kernel from the uImage to a temporary directory.
> > 
> 
> I'd suggest that we should document in the commit log why someone
> wouldn't just build a kernel image format that is already supported by
> runqemu (which is what I typically do). I'm guessing it is some sort
> of h/w boot being re-purposed to qemu emulation, and this is to avoid
> building a second kernel image ?
> 
The runqemu script explicitly checks for the uImage type. However, my
conclusion so far is that Qemu does not support this format, or at
least it is broken. I've been thinking about adding more reasonable
feedback to the user instead of just launching the uImage and running
into an infinite loop of nothing. However this would need deeper
understanding of Qemus's support for e.g. uImage format or better ctest
cases covering this use cases.

> Additionally, this should have some sort of test, or the code is going
> to bitrot due to not being run by anything in core.
The current runquem-related test cases only cover the x86_64 MACHINE.Regardless of the discussion around this patch, it might be useful to
cover more machines there as well. This would make it possible to test
some more image formats, such as uImage and fitImage, which are more
widely used on ARM machines. I will have a look at that.

Regards,
Adrian
> 
> > Signed-off-by: Adrian Freihofer <adrian.freihofer@siemens.com>
> > ---
> >  scripts/runqemu | 24 ++++++++++++++++++++++--
> >  1 file changed, 22 insertions(+), 2 deletions(-)
> > 
> > diff --git a/scripts/runqemu b/scripts/runqemu
> > index ba0b701aff..fa630fc7a4 100755
> > --- a/scripts/runqemu
> > +++ b/scripts/runqemu
> > @@ -163,6 +163,7 @@ class BaseConfig(object):
> >          self.bios = ''
> >          self.kernel_cmdline = ''
> >          self.kernel_cmdline_script = ''
> > +        self.temp_kernel_dir = None
> >          self.bootparams = ''
> >          self.dtb = ''
> >          self.fstype = ''
> > @@ -1422,10 +1423,26 @@ class BaseConfig(object):
> >          self.setup_serial()
> >          self.setup_vga()
> > 
> > +    def dump_uimage(self):
> > +        if self.kernel and self.get('KERNEL_IMAGETYPE') == 'uImage':
> > +            import tempfile
> > +            self.temp_kernel_dir = tempfile.mkdtemp()
> > +            kernel = os.path.join(self.temp_kernel_dir, 'image')
> > +            dumpimage_bin = os.path.join(self.bindir_native, 'dumpimage')
> > +            image_link_name = self.get('IMAGE_LINK_NAME')
> > +            logger.info('Running: %s' % str([dumpimage_bin, "-o ", kernel, self.kernel]))
> > +            staging_bindir_native = '%s/usr/bin' % self.get('STAGING_DIR_NATIVE')
> > +            dumpimage_bin = os.path.join(staging_bindir_native, 'dumpimage')
> > +            subprocess.run([dumpimage_bin, "-p", "0", "-o", kernel, self.kernel])
> > +            return kernel
> > +        return self.kernel
> > +
> >      def start_qemu(self):
> >          import shlex
> > -        if self.kernel:
> > -            kernel_opts = "-kernel %s -append '%s %s %s %s'" % (self.kernel, self.kernel_cmdline,
> > +
> > +        kernel = self.dump_uimage()
> > +        if kernel:
> > +            kernel_opts = "-kernel %s -append '%s %s %s %s'" % (kernel, self.kernel_cmdline,
> >                                                                  self.kernel_cmdline_script, self.get('QB_KERNEL_CMDLINE_APPEND'),
> >                                                                  self.bootparams)
> 
> If the kernel never fails from the dup_uimage() command, why test for
> it ? I don't see how the routine could return something that doesn't
> pass the test. If so, aren't we going to run into more cryptic errors
> (or traceback) later if there was some sort of failure.
> 
> The existing code doesn't look like it is completely error proofed
> either, that just came to mind when looking at the patch.
> 
> Bruce
> 
> >              if self.bios:
> > @@ -1481,6 +1498,9 @@ class BaseConfig(object):
> >              shutil.rmtree(self.rootfs)
> >              shutil.rmtree('%s.pseudo_state' % self.rootfs)
> > 
> > +        if self.temp_kernel_dir:
> > +            shutil.rmtree(self.temp_kernel_dir)
> > +
> >          self.cleaned = True
> > 
> >      def run_bitbake_env(self, mach=None):
> > --
> > 2.26.3
> > 
> > 
> > 
> > 
> 
>