Re: [OE-core] [PATCH 8/9] runqemu: support UEFI with OVMF firmware

[Patrick Ohly]

On Mon, 2017-01-09 at 19:50 -0800, Ricardo Neri wrote:
> On Wed, 2017-01-04 at 10:43 +0100, Patrick Ohly wrote:
> > To
> > test that supporting separate code and variables also works, I've
> > implemented that locally so that ovmf.fd ovmf_secboot.fd, ovmf_code.fd,
> > ovmf_secboot_code.fd and ovmf_vars.fd get deployed and runqemu supports
> > more than one "ovmf" parameter - this worked nicely. Full change below.
> 
> ... Now that you have took the time to prototype the solution, we could
> put it to use.
> 
> > 
> > Now that I've implemented it, I wonder whether it would be worth
> > submitting that as part of rev2 of this patch series. Any opinions?
> > 
> > diff --git a/meta/recipes-core/ovmf/ovmf_git.bb 
> > b/meta/recipes-core/ovmf/ovmf_git.bb
> > index ef61b16..391274b 100644
> > --- a/meta/recipes-core/ovmf/ovmf_git.bb
> > +++ b/meta/recipes-core/ovmf/ovmf_git.bb
> > @@ -125,6 +125,8 @@ do_compile_class-target() {
> >  rm -rf ${S}/Build/Ovmf$OVMF_DIR_SUFFIX
> >  ${S}/OvmfPkg/build.sh $PARALLEL_JOBS -a $OVMF_ARCH -b RELEASE -t 
> > ${FIXED_GCCVER}
> >  ln ${build_dir}/FV/OVMF.fd ${WORKDIR}/ovmf/OVMF.fd
> > +ln ${build_dir}/FV/OVMF_CODE.fd ${WORKDIR}/ovmf/OVMF.code.fd
> > +ln ${build_dir}/FV/OVMF_VARS.fd ${WORKDIR}/ovmf/OVMF.vars.fd
> >  
> >  # See CryptoPkg/Library/OpensslLib/Patch-HOWTO.txt and
> >  # https://src.fedoraproject.org/cgit/rpms/edk2.git/tree/ for
> > @@ -137,6 +139,7 @@ do_compile_class-target() {
> >  ( cd ${S}/CryptoPkg/Library/OpensslLib/ && ./Install.sh )
> >  ${S}/OvmfPkg/build.sh $PARALLEL_JOBS -a $OVMF_ARCH -b RELEASE -t 
> > ${FIXED_GCCVER} ${OVMF_SECURE_BOOT_FLAGS}
> >  ln ${build_dir}/FV/OVMF.fd ${WORKDIR}/ovmf/OVMF.secboot.fd
> > +ln ${build_dir}/FV/OVMF_CODE.fd ${WORKDIR}/ovmf/OVMF.code.secboot.fd
> >  for i in Shell.efi EnrollDefaultKeys.efi; do
> >  ln ${build_dir}/${OVMF_ARCH}/$i ${WORKDIR}/ovmf/$i
> >  done
> > @@ -170,8 +173,9 @@ do_deploy() {
> >  }
> >  do_deploy_class-target() {
> >  # For use with "runqemu ovmf".
> > -qemu-img convert -f raw -O qcow2 ${WORKDIR}/ovmf/OVMF.fd 
> > ${DEPLOYDIR}/ovmf.qcow2
> > -qemu-img convert -f raw -O qcow2 ${WORKDIR}/ovmf/OVMF.secboot.fd 
> > ${DEPLOYDIR}/ovmf.secboot.qcow2
> > +for i in OVMF OVMF.secboot OVMF.code OVMF.vars OVMF.code.secboot; do
> > +qemu-img convert -f raw -O qcow2 ${WORKDIR}/ovmf/$i.fd 
> > ${DEPLOYDIR}/`echo $i | tr A-Z a-z`.qcow2
> 
> Will this preserve any previous OVMF_vars.fd that might exist in the
> directory.

No, it will overwrite ovmf.vars.qcow2 and any variables stored in that
get lost. That is consistent with rebuilding the disk image: a user who
wants to have a "persistent" virtual machine must copy the relevant file
and then use the full file paths. In this case, that means invoking
runqemu with the file path to the copy of ovmf.vars.qcow2 instead of
just "ovmf.vars".

> >  if self.ovmf_bios:
> > -print('OVMF: [%s]' % self.ovmf_bios)
> > +print('OVMF: %s' % self.ovmf_bios)
> 
> Is there a reason to remove the brackets here?

self.ovmf_bios is a list, so formatting it as string will add the
brackets. I found that more readable than the (from a semantic point of
view more correct) double brackets: [['.../ovmf.code.qcow2',
'.../ovmf.vars.qcow2']]

> > +for ovmf in self.ovmf_bios:
> > +format = ovmf.rsplit('.', 1)[-1]
> > +self.qemu_opt += ' -drive if=pflash,format=%s,file=%s' % 
> > (format, ovmf)
> >  if self.ovmf_bios:
> > -format = self.ovmf_bios.rsplit('.', 1)[-1]
> > -self.qemu_opt += ' -drive if=pflash,format=%s,file=%s' % 
> > (format, self.ovmf_bios)
> >  # OVMF only supports normal VGA, i.e. we need to override a 
> > -vga vmware
> >  # that gets added for example for normal qemux86.
> >  self.qemu_opt += ' -vga std'
> > 
> > 
> I think this solution looks good as having separate file does not pose
> an extra hassle in the user: the recipe builds all that is needed and
> runqemu takes all that it needs.

But not automatically. "runqemu ovmf" would expand "ovmf" to
tmp/deploy/images/.../ovmf.qcow2 and thus use the combined file while
"runqemu ovmf.code ovmf.vars" tells runqemu that it is supposed to use
two flash drives, one with the code and one with the variables.


>  If in the future people shows an
> interest in having unified images, maybe that can be added as another
> PACKAGECONFIG?

As the unified code+vars is slightly easier to use, I'd prefer to keep
it around and just offer both. That's not such a big deal in terms of
performance and disk usage.


-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.



-- 
___

Re: [OE-core] [PATCH 8/9] runqemu: support UEFI with OVMF firmware

[Ricardo Neri]

On Wed, 2017-01-04 at 10:43 +0100, Patrick Ohly wrote:
> On Wed, 2016-12-28 at 15:33 -0800, Ricardo Neri wrote:
> > On Wed, 2016-12-21 at 14:11 +0100, Patrick Ohly wrote:
> > > +# File name of a OVMF BIOS file, to be added with -drive
> > > if=pflash.
> > > +# Found in the same places as the rootfs, with or without one
> > > of
> > > +# these suffices: qcow2, bin.
> > > +# Setting one also adds "-vga std" because that is all that
> > > +# OVMF supports.
> > > +self.ovmf_bios = ''
> > 
> > runqemu has the options biosdir and biosfilename. Although the log for
> > these options was lost when the script was migrated to python,
> 
> You probably mean this:
> http://git.openembedded.org/openembedded-core/commit/?id=d302f5683dd736ac4cd4b601a046d22000d41e68
> http://git.openembedded.org/openembedded-core/commit/?id=29c9e6f44541b7f8731e21e9d1a0adca9da28e37
> 
> >  the
> > motivation of adding these options was to use OVMF. It uses the -L and
> > -bios options of qemu. To my knowledge, the only custom bios at the
> > moment is OVMF. Thus, you would ponder either removing or tweaking these
> > options with your approach; which makes more sense to me.
> 
> I have no personal opinion about the usefulness of the "biosdir" and
> "biosfilename" options. Just looking at what they do, they might have
> value also when not using OVMF (for example, the "VGA BIOS" that is
> mentioned in the first commit). But if no-one is actually using these
> options, then they should indeed be removed to simplify runqemu.
> 
> The problem just is to determine whether they are used :-/ As I don't
> know, I'd prefer to keep them for now and remove them separately.

This makes sense.
> 
> Regarding the approach that I proposed for the "ovmf" file(s): what's
> your opinion about that? I was a bit worried that too much "magic" is
> involved here (special keyword that expands to files and sets -vga), but
> it is convenient and quite naturally supports additional use cases
> (explicitly selecting files at non-standard locations, separate code and
> variable files).
> 
> Regarding that last argument: in the current patch series, only the
> combined ovmf.fd gets deployed and I argued that this is sufficient. 

It would be certainly enough for me :) as in most of my use cases I
always test brand new images without any variables in it. Also, you
kindly included facilitiesto lockdown the image. I can't speak for other
people but this is more than enough for me. If you pursue this path,
perhaps you can include a big warning in the recipe saying that people
will lose their variables if they rebuild OVMF. On the other hand...

> To
> test that supporting separate code and variables also works, I've
> implemented that locally so that ovmf.fd ovmf_secboot.fd, ovmf_code.fd,
> ovmf_secboot_code.fd and ovmf_vars.fd get deployed and runqemu supports
> more than one "ovmf" parameter - this worked nicely. Full change below.

... Now that you have took the time to prototype the solution, we could
put it to use.

> 
> Now that I've implemented it, I wonder whether it would be worth
> submitting that as part of rev2 of this patch series. Any opinions?
> 
> diff --git a/meta/recipes-core/ovmf/ovmf_git.bb 
> b/meta/recipes-core/ovmf/ovmf_git.bb
> index ef61b16..391274b 100644
> --- a/meta/recipes-core/ovmf/ovmf_git.bb
> +++ b/meta/recipes-core/ovmf/ovmf_git.bb
> @@ -125,6 +125,8 @@ do_compile_class-target() {
>  rm -rf ${S}/Build/Ovmf$OVMF_DIR_SUFFIX
>  ${S}/OvmfPkg/build.sh $PARALLEL_JOBS -a $OVMF_ARCH -b RELEASE -t 
> ${FIXED_GCCVER}
>  ln ${build_dir}/FV/OVMF.fd ${WORKDIR}/ovmf/OVMF.fd
> +ln ${build_dir}/FV/OVMF_CODE.fd ${WORKDIR}/ovmf/OVMF.code.fd
> +ln ${build_dir}/FV/OVMF_VARS.fd ${WORKDIR}/ovmf/OVMF.vars.fd
>  
>  # See CryptoPkg/Library/OpensslLib/Patch-HOWTO.txt and
>  # https://src.fedoraproject.org/cgit/rpms/edk2.git/tree/ for
> @@ -137,6 +139,7 @@ do_compile_class-target() {
>  ( cd ${S}/CryptoPkg/Library/OpensslLib/ && ./Install.sh )
>  ${S}/OvmfPkg/build.sh $PARALLEL_JOBS -a $OVMF_ARCH -b RELEASE -t 
> ${FIXED_GCCVER} ${OVMF_SECURE_BOOT_FLAGS}
>  ln ${build_dir}/FV/OVMF.fd ${WORKDIR}/ovmf/OVMF.secboot.fd
> +ln ${build_dir}/FV/OVMF_CODE.fd ${WORKDIR}/ovmf/OVMF.code.secboot.fd
>  for i in Shell.efi EnrollDefaultKeys.efi; do
>  ln ${build_dir}/${OVMF_ARCH}/$i ${WORKDIR}/ovmf/$i
>  done
> @@ -170,8 +173,9 @@ do_deploy() {
>  }
>  do_deploy_class-target() {
>  # For use with "runqemu ovmf".
> -qemu-img convert -f raw -O qcow2 ${WORKDIR}/ovmf/OVMF.fd 
> ${DEPLOYDIR}/ovmf.qcow2
> -qemu-img convert -f raw -O qcow2 ${WORKDIR}/ovmf/OVMF.secboot.fd 
> ${DEPLOYDIR}/ovmf.secboot.qcow2
> +for i in OVMF OVMF.secboot OVMF.code OVMF.vars OVMF.code.secboot; do
> +qemu-img convert -f raw -O qcow2 ${WORKDIR}/ovmf/$i.fd 
> ${DEPLOYDIR}/`echo $i | tr A-Z a-z`.qcow2

Will this preserve any previous OVMF_vars.fd that might exist in the
directory.

Re: [OE-core] [PATCH 8/9] runqemu: support UEFI with OVMF firmware

[Patrick Ohly]

On Wed, 2016-12-28 at 15:33 -0800, Ricardo Neri wrote:
> On Wed, 2016-12-21 at 14:11 +0100, Patrick Ohly wrote:
> > +# File name of a OVMF BIOS file, to be added with -drive
> > if=pflash.
> > +# Found in the same places as the rootfs, with or without one
> > of
> > +# these suffices: qcow2, bin.
> > +# Setting one also adds "-vga std" because that is all that
> > +# OVMF supports.
> > +self.ovmf_bios = ''
> 
> runqemu has the options biosdir and biosfilename. Although the log for
> these options was lost when the script was migrated to python,

You probably mean this:
http://git.openembedded.org/openembedded-core/commit/?id=d302f5683dd736ac4cd4b601a046d22000d41e68
http://git.openembedded.org/openembedded-core/commit/?id=29c9e6f44541b7f8731e21e9d1a0adca9da28e37

>  the
> motivation of adding these options was to use OVMF. It uses the -L and
> -bios options of qemu. To my knowledge, the only custom bios at the
> moment is OVMF. Thus, you would ponder either removing or tweaking these
> options with your approach; which makes more sense to me.

I have no personal opinion about the usefulness of the "biosdir" and
"biosfilename" options. Just looking at what they do, they might have
value also when not using OVMF (for example, the "VGA BIOS" that is
mentioned in the first commit). But if no-one is actually using these
options, then they should indeed be removed to simplify runqemu.

The problem just is to determine whether they are used :-/ As I don't
know, I'd prefer to keep them for now and remove them separately.

Regarding the approach that I proposed for the "ovmf" file(s): what's
your opinion about that? I was a bit worried that too much "magic" is
involved here (special keyword that expands to files and sets -vga), but
it is convenient and quite naturally supports additional use cases
(explicitly selecting files at non-standard locations, separate code and
variable files).

Regarding that last argument: in the current patch series, only the
combined ovmf.fd gets deployed and I argued that this is sufficient. To
test that supporting separate code and variables also works, I've
implemented that locally so that ovmf.fd ovmf_secboot.fd, ovmf_code.fd,
ovmf_secboot_code.fd and ovmf_vars.fd get deployed and runqemu supports
more than one "ovmf" parameter - this worked nicely. Full change below.

Now that I've implemented it, I wonder whether it would be worth
submitting that as part of rev2 of this patch series. Any opinions?

diff --git a/meta/recipes-core/ovmf/ovmf_git.bb 
b/meta/recipes-core/ovmf/ovmf_git.bb
index ef61b16..391274b 100644
--- a/meta/recipes-core/ovmf/ovmf_git.bb
+++ b/meta/recipes-core/ovmf/ovmf_git.bb
@@ -125,6 +125,8 @@ do_compile_class-target() {
 rm -rf ${S}/Build/Ovmf$OVMF_DIR_SUFFIX
 ${S}/OvmfPkg/build.sh $PARALLEL_JOBS -a $OVMF_ARCH -b RELEASE -t 
${FIXED_GCCVER}
 ln ${build_dir}/FV/OVMF.fd ${WORKDIR}/ovmf/OVMF.fd
+ln ${build_dir}/FV/OVMF_CODE.fd ${WORKDIR}/ovmf/OVMF.code.fd
+ln ${build_dir}/FV/OVMF_VARS.fd ${WORKDIR}/ovmf/OVMF.vars.fd
 
 # See CryptoPkg/Library/OpensslLib/Patch-HOWTO.txt and
 # https://src.fedoraproject.org/cgit/rpms/edk2.git/tree/ for
@@ -137,6 +139,7 @@ do_compile_class-target() {
 ( cd ${S}/CryptoPkg/Library/OpensslLib/ && ./Install.sh )
 ${S}/OvmfPkg/build.sh $PARALLEL_JOBS -a $OVMF_ARCH -b RELEASE -t 
${FIXED_GCCVER} ${OVMF_SECURE_BOOT_FLAGS}
 ln ${build_dir}/FV/OVMF.fd ${WORKDIR}/ovmf/OVMF.secboot.fd
+ln ${build_dir}/FV/OVMF_CODE.fd ${WORKDIR}/ovmf/OVMF.code.secboot.fd
 for i in Shell.efi EnrollDefaultKeys.efi; do
 ln ${build_dir}/${OVMF_ARCH}/$i ${WORKDIR}/ovmf/$i
 done
@@ -170,8 +173,9 @@ do_deploy() {
 }
 do_deploy_class-target() {
 # For use with "runqemu ovmf".
-qemu-img convert -f raw -O qcow2 ${WORKDIR}/ovmf/OVMF.fd 
${DEPLOYDIR}/ovmf.qcow2
-qemu-img convert -f raw -O qcow2 ${WORKDIR}/ovmf/OVMF.secboot.fd 
${DEPLOYDIR}/ovmf.secboot.qcow2
+for i in OVMF OVMF.secboot OVMF.code OVMF.vars OVMF.code.secboot; do
+qemu-img convert -f raw -O qcow2 ${WORKDIR}/ovmf/$i.fd 
${DEPLOYDIR}/`echo $i | tr A-Z a-z`.qcow2
+done
 }
 addtask do_deploy after do_compile before do_build
 
diff --git a/scripts/runqemu b/scripts/runqemu
index c8b7c8a..c3fed89 100755
--- a/scripts/runqemu
+++ b/scripts/runqemu
@@ -163,12 +163,12 @@ class BaseConfig(object):
 self.clean_nfs_dir = False
 self.nfs_server = ''
 self.rootfs = ''
-# File name of a OVMF BIOS file, to be added with -drive if=pflash.
+# File name(s) of a OVMF BIOS file or variable store, to be added with 
-drive if=pflash.
 # Found in the same places as the rootfs, with or without one of
 # these suffices: qcow2, bin.
 # Setting one also adds "-vga std" because that is all that
 # OVMF supports.
-self.ovmf_bios = ''
+self.ovmf_bios = []
 self.qemuboot = ''
 self.qbconfload = False
  

Re: [OE-core] [PATCH 8/9] runqemu: support UEFI with OVMF firmware

[Ricardo Neri]

On Wed, 2016-12-21 at 14:11 +0100, Patrick Ohly wrote:
> +# File name of a OVMF BIOS file, to be added with -drive
> if=pflash.
> +# Found in the same places as the rootfs, with or without one
> of
> +# these suffices: qcow2, bin.
> +# Setting one also adds "-vga std" because that is all that
> +# OVMF supports.
> +self.ovmf_bios = ''

runqemu has the options biosdir and biosfilename. Although the log for
these options was lost when the script was migrated to python, the
motivation of adding these options was to use OVMF. It uses the -L and
-bios options of qemu. To my knowledge, the only custom bios at the
moment is OVMF. Thus, you would ponder either removing or tweaking these
options with your approach; which makes more sense to me.

Thanks and BR,
Ricardo

-- 
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core

[OE-core] [PATCH 8/9] runqemu: support UEFI with OVMF firmware

[Patrick Ohly]

In the simplest case, "runqemu qemux86  qcow2 ovmf" for an
EFI-enabled image in the qcow2 format will locate the OVMF firmware file,
override the graphics hardware with "-vga std" because that is all
that OVMF supports, and boot with UEFI enabled. This depends on
"bitbake ovmf" deploying a "ovmf.qcow2" firmware file in the image deploy
directory.

The firmware file is activated as a flash drive instead of using the
qemu BIOS parameters, because that is the recommended method
(https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=764918#47) as it
allows storing UEFI variables in the file.

Instead of just "ovmf", a full path to an existing file can also be
used, just as with the rootfs. That may be useful when making a
permanent copy of the virtual machine data files.

Signed-off-by: Patrick Ohly 
---
 scripts/runqemu | 37 -
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/scripts/runqemu b/scripts/runqemu
index 203992a..257dcec 100755
--- a/scripts/runqemu
+++ b/scripts/runqemu
@@ -74,6 +74,7 @@ of the following environment variables (in any order):
 kvm-vhost - enable KVM with vhost when running x86/x86_64 (VT-capable CPU 
required)
 publicvnc - enable a VNC server open to all hosts
 audio - enable audio
+[*/]ovmf* - OVMF BIOS file or base name for booting with UEFI
   tcpserial= - specify tcp serial port number
   biosdir= - specify custom bios dir
   biosfilename= - specify bios filename
@@ -162,6 +163,12 @@ class BaseConfig(object):
 self.clean_nfs_dir = False
 self.nfs_server = ''
 self.rootfs = ''
+# File name of a OVMF BIOS file, to be added with -drive if=pflash.
+# Found in the same places as the rootfs, with or without one of
+# these suffices: qcow2, bin.
+# Setting one also adds "-vga std" because that is all that
+# OVMF supports.
+self.ovmf_bios = ''
 self.qemuboot = ''
 self.qbconfload = False
 self.kernel = ''
@@ -369,6 +376,8 @@ class BaseConfig(object):
 self.qemu_opt_script += ' %s' % arg[len('qemuparams='):]
 elif arg.startswith('bootparams='):
 self.kernel_cmdline_script += ' %s' % arg[len('bootparams='):]
+elif os.path.basename(arg).startswith('ovmf'):
+self.ovmf_bios = arg
 elif os.path.exists(arg) or (re.search(':', arg) and 
re.search('/', arg)):
 self.check_arg_path(os.path.abspath(arg))
 elif re.search('-image-', arg):
@@ -472,6 +481,20 @@ class BaseConfig(object):
 if not os.path.exists(self.rootfs):
 raise Exception("Can't find rootfs: %s" % self.rootfs)
 
+def check_ovmf(self):
+"""Check and set full path for OVMF BIOS file."""
+
+if self.ovmf_bios is None or os.path.exists(self.ovmf_bios):
+return
+
+for suffix in ('qcow2', 'bin'):
+ovmf_bios = '%s/%s.%s' % (self.get('DEPLOY_DIR_IMAGE'), 
self.ovmf_bios, suffix)
+if os.path.exists(ovmf_bios):
+self.ovmf_bios = ovmf_bios
+return
+
+raise Exception("Can't find OVMF BIOS: %s" % self.ovmf_bios)
+
 def check_kernel(self):
 """Check and set kernel, dtb"""
 # The vm image doesn't need a kernel
@@ -562,6 +585,7 @@ class BaseConfig(object):
 self.check_kvm()
 self.check_fstype()
 self.check_rootfs()
+self.check_ovmf()
 self.check_kernel()
 self.check_biosdir()
 self.check_mem()
@@ -670,6 +694,8 @@ class BaseConfig(object):
 print('NFS_DIR: [%s]' % self.nfs_dir)
 else:
 print('ROOTFS: [%s]' % self.rootfs)
+if self.ovmf_bios:
+print('OVMF: [%s]' % self.ovmf_bios)
 print('CONFFILE: [%s]' % self.qemuboot)
 print('')
 
@@ -926,7 +952,16 @@ class BaseConfig(object):
 
 check_libgl(qemu_bin)
 
-self.qemu_opt = "%s %s %s %s %s" % (qemu_bin, self.get('NETWORK_CMD'), 
self.get('ROOTFS_OPTIONS'), self.get('QB_OPT_APPEND'), self.qemu_opt_script)
+self.qemu_opt = "%s %s %s %s" % (qemu_bin, self.get('NETWORK_CMD'), 
self.get('ROOTFS_OPTIONS'), self.get('QB_OPT_APPEND'))
+
+if self.ovmf_bios:
+format = self.ovmf_bios.rsplit('.', 1)[-1]
+self.qemu_opt += ' -drive if=pflash,format=%s,file=%s' % (format, 
self.ovmf_bios)
+# OVMF only supports normal VGA, i.e. we need to override a -vga 
vmware
+# that gets added for example for normal qemux86.
+self.qemu_opt += ' -vga std'
+
+self.qemu_opt += ' ' + self.qemu_opt_script
 
 if self.snapshot:
 self.qemu_opt += " -snapshot"
-- 
2.1.4

-- 
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core