[PATCH 0 of 2] Add the OVMF UEFI support to hvmloader

[PATCH 0 of 2] Add the OVMF UEFI support to hvmloader

[Attilio Rao]

The attached patches rewamp an early SoC 2011 work by Bei Guan and
add the support for OVMF UEFI in hvmloader.

In order to use the OVMF the guest configuration file must specify it
via the introduced bios_override option and the Tianocore roms must be
built and copied manually into the specific location (as explained in
the comments about ovmf_infrastructure.patch).

An open point is how to deal with download and building of the rom
themselves.
We should likely mirror the edk2 svn tree (or a part of it) via an
utility as git-svn and download with git as we already do with projects
as seabios.

[PATCH 1 of 2] Add the support for Xen to include OVMF UEFI support and directly use it

[Attilio Rao]

when specified in the guest configuration file.
This work is somewhat based on Bei Guan effort during the SoC 2011 and
relies on upstream edk2/ovmf Tianocore ROM to be built separately and
manually copied as:
Build/OvmfX64/DEBUG_GCC44/FV/OVMF.fd -> tools/firmware/ovmf/ovmf-x64.bin
Build/OvmfIa32/DEBUG_GCC44/FV/OVMF.fd -> toolf/firmware/ovmf/ovmf-ia32.bin

A way to integrate OVMF build directly into XEN has still be discussed
on the mailing list appropriately.

Signed-off-by: Attilio Rao <attilio.rao@citrix.com>

diff -r a88ba599add1 -r 032fea10f8d1 Config.mk
--- a/Config.mk	Tue Feb 21 17:45:59 2012 +0000
+++ b/Config.mk	Wed Feb 22 18:54:03 2012 +0000
@@ -224,6 +224,7 @@ SEABIOS_UPSTREAM_TAG ?= rel-1.6.3.1
 
 ETHERBOOT_NICS ?= rtl8139 8086100e
 
+CONFIG_OVMF ?= y
 CONFIG_ROMBIOS ?= y
 CONFIG_SEABIOS ?= y
 
diff -r a88ba599add1 -r 032fea10f8d1 tools/firmware/hvmloader/Makefile
--- a/tools/firmware/hvmloader/Makefile	Tue Feb 21 17:45:59 2012 +0000
+++ b/tools/firmware/hvmloader/Makefile	Wed Feb 22 18:54:03 2012 +0000
@@ -37,6 +37,7 @@ endif
 
 CIRRUSVGA_DEBUG ?= n
 
+OVMF_DIR := ../ovmf
 ROMBIOS_DIR := ../rombios
 SEABIOS_DIR := ../seabios-dir
 
@@ -52,6 +53,14 @@ endif
 
 ROMS := 
 
+ifeq ($(CONFIG_OVMF),y)
+OBJS += ovmf.o
+CFLAGS += -DENABLE_OVMF32 -DENABLE_OVMF64
+OVMF32_ROM := $(OVMF_DIR)/ovmf-ia32.bin
+OVMF64_ROM := $(OVMF_DIR)/ovmf-x64.bin
+ROMS += $(OVMF32_ROM) $(OVMF64_ROM)
+endif
+
 ifeq ($(CONFIG_ROMBIOS),y)
 OBJS += optionroms.o 32bitbios_support.o rombios.o
 CFLAGS += -DENABLE_ROMBIOS
@@ -70,7 +79,7 @@ endif
 all: subdirs-all
 	$(MAKE) hvmloader
 
-rombios.o seabios.o hvmloader.o: roms.inc
+ovmf.o rombios.o seabios.o hvmloader.o: roms.inc
 smbios.o: CFLAGS += -D__SMBIOS_DATE__="\"$(shell date +%m/%d/%Y)\""
 
 hvmloader: $(OBJS) acpi/acpi.a
@@ -93,6 +102,18 @@ ifneq ($(SEABIOS_ROM),)
 	echo "#endif" >> $@.new
 endif
 
+ifneq ($(OVMF32_ROM),)
+	echo "#ifdef ROM_INCLUDE_OVMF32" >> $@.new
+	sh ./mkhex ovmf32 $(OVMF32_ROM) >> $@.new
+	echo "#endif" >> $@.new	
+endif 
+
+ifneq ($(OVMF64_ROM),)
+	echo "#ifdef ROM_INCLUDE_OVMF64" >> $@.new
+	sh ./mkhex ovmf64 $(OVMF64_ROM) >> $@.new
+	echo "#endif" >> $@.new	
+endif 
+
 ifneq ($(STDVGA_ROM),)
 	echo "#ifdef ROM_INCLUDE_VGABIOS" >> $@.new
 	sh ./mkhex vgabios_stdvga $(STDVGA_ROM) >> $@.new
diff -r a88ba599add1 -r 032fea10f8d1 tools/firmware/hvmloader/config.h
--- a/tools/firmware/hvmloader/config.h	Tue Feb 21 17:45:59 2012 +0000
+++ b/tools/firmware/hvmloader/config.h	Wed Feb 22 18:54:03 2012 +0000
@@ -35,6 +35,8 @@ struct bios_config {
 
 extern struct bios_config rombios_config;
 extern struct bios_config seabios_config;
+extern struct bios_config ovmf32_config;
+extern struct bios_config ovmf64_config;
 
 #define PAGE_SHIFT 12
 #define PAGE_SIZE  (1ul << PAGE_SHIFT)
diff -r a88ba599add1 -r 032fea10f8d1 tools/firmware/hvmloader/hvmloader.c
--- a/tools/firmware/hvmloader/hvmloader.c	Tue Feb 21 17:45:59 2012 +0000
+++ b/tools/firmware/hvmloader/hvmloader.c	Wed Feb 22 18:54:03 2012 +0000
@@ -212,6 +212,12 @@ struct bios_info {
 #ifdef ENABLE_SEABIOS
     { "seabios", &seabios_config, },
 #endif
+#ifdef ENABLE_OVMF32
+    { "ovmf-ia32", &ovmf32_config, },
+#endif
+#ifdef ENABLE_OVMF64
+    { "ovmf-x64", &ovmf64_config, },
+#endif
     { NULL, NULL }
 };
 
diff -r a88ba599add1 -r 032fea10f8d1 tools/firmware/hvmloader/ovmf.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/tools/firmware/hvmloader/ovmf.c	Wed Feb 22 18:54:03 2012 +0000
@@ -0,0 +1,147 @@
+/*
+ * HVM OVMF UEFI support.
+ *
+ * Bei Guan, gbtju85@gmail.com
+ * Andrei Warkentin, andreiw@motorola.com
+ * Leendert van Doorn, leendert@watson.ibm.com
+ * Copyright (c) 2005, International Business Machines Corporation.
+ * Copyright (c) 2006, Keir Fraser, XenSource Inc.
+ * Copyright (c) 2011, Citrix Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc., 59 Temple
+ * Place - Suite 330, Boston, MA 02111-1307 USA.
+ */
+
+#include "config.h"
+#include "smbios_types.h"
+#include "acpi/acpi2_0.h"
+#include "apic_regs.h"
+#include "../rombios/config.h"
+#include "util.h"
+#include "pci_regs.h"
+#include "hypercall.h"
+
+#include <xen/hvm/params.h>
+#include <xen/hvm/ioreq.h>
+#include <xen/memory.h>
+
+#define ROM_INCLUDE_OVMF32
+#define ROM_INCLUDE_OVMF64
+#include "roms.inc"
+
+#define OVMF_BEGIN              0xFFF00000ULL
+#define OVMF_SIZE               0x00100000ULL
+#define OVMF_MAXOFFSET          0x000FFFFFULL
+#define OVMF_END                (OVMF_BEGIN + OVMF_SIZE)
+#define LOWCHUNK_BEGIN          0x000F0000
+#define LOWCHUNK_SIZE           0x00010000
+#define LOWCHUNK_MAXOFFSET      0x0000FFFF
+#define LOWCHUNK_END            (OVMF_BEGIN + OVMF_SIZE)
+
+extern unsigned char dsdt_anycpu[], dsdt_15cpu[];
+extern int dsdt_anycpu_len, dsdt_15cpu_len;
+
+static void ovmf_load(const struct bios_config *config)
+{
+    xen_pfn_t mfn;
+    uint64_t addr = OVMF_BEGIN;
+
+    /* Copy low-reset vector portion. */
+    memcpy((void *) LOWCHUNK_BEGIN, (uint8_t *) config->image
+           + OVMF_SIZE
+           - LOWCHUNK_SIZE,
+           LOWCHUNK_SIZE);
+
+    /* Ensure we have backing page prior to moving FD. */
+    while ((addr >> PAGE_SHIFT) != (OVMF_END >> PAGE_SHIFT)) {
+        mfn = (uint32_t) (addr >> PAGE_SHIFT);
+        addr += PAGE_SIZE;
+	mem_hole_populate_ram(mfn, 1);
+    }
+
+    /* Copy FD. */
+    memcpy((void *) OVMF_BEGIN, config->image, OVMF_SIZE);
+}
+
+static void ovmf_acpi_build_tables(void)
+{
+    struct acpi_config config = {
+        .dsdt_anycpu = dsdt_anycpu,
+        .dsdt_anycpu_len = dsdt_anycpu_len,
+        .dsdt_15cpu = dsdt_15cpu,
+        .dsdt_15cpu_len = dsdt_15cpu_len,
+    };
+
+    acpi_build_tables(&config, ACPI_PHYSICAL_ADDRESS);
+}
+
+static void ovmf_create_smbios_tables(void)
+{
+    hvm_write_smbios_tables(SMBIOS_PHYSICAL_ADDRESS,
+                            SMBIOS_PHYSICAL_ADDRESS + sizeof(struct smbios_entry_point),
+                            SMBIOS_PHYSICAL_END);
+}
+
+struct bios_config ovmf32_config =  {
+    .name = "OVMF-IA32",
+
+    .image = ovmf32,
+    .image_size = sizeof(ovmf32),
+
+    .bios_address = 0,
+    .bios_load = ovmf_load,
+
+    .load_roms = 0,
+
+    .bios_info_setup = NULL,
+    .bios_info_finish = NULL,
+
+    .e820_setup = NULL,
+
+    .acpi_build_tables = ovmf_acpi_build_tables,
+    .create_mp_tables = NULL,
+    .create_smbios_tables = ovmf_create_smbios_tables,
+    .create_pir_tables = NULL,
+};
+
+struct bios_config ovmf64_config =  {
+    .name = "OVMF-X64",
+
+    .image = ovmf64,
+    .image_size = sizeof(ovmf64),
+
+    .bios_address = 0,
+    .bios_load = ovmf_load,
+
+    .load_roms = 0,
+
+    .bios_info_setup = NULL,
+    .bios_info_finish = NULL,
+
+    .e820_setup = NULL,
+
+    .acpi_build_tables = ovmf_acpi_build_tables,
+    .create_mp_tables = NULL,
+    .create_smbios_tables = ovmf_create_smbios_tables,
+    .create_pir_tables = NULL,
+};
+
+/*
+ * Local variables:
+ * mode: C
+ * c-set-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */

Re: [PATCH 1 of 2] Add the support for Xen to include OVMF UEFI support and directly use it

[Ian Campbell]

On Wed, 2012-02-22 at 19:17 +0000, Attilio Rao wrote:
> A way to integrate OVMF build directly into XEN has still be discussed
> on the mailing list appropriately.

AIUI OVMF is maintained in SVN. Our normal procedure for adding an
external dependency would be for us to mirror it on xenbits as a
convenience to our users, who don't need to get stuff from multiple
places, and as a courtesy to our upstreams, so our users don't consume
their resources.

I don't much fancy setting the necessary webdav or whatever stuff on
xenbits and integrating SVN support into our build system though. What
do people think about using git-svn to manage our mirror in git instead?
Or better: perhaps OVMF have an official git or hg mirror?

Anyone have any thoughts/opinions/better ideas etc?

> diff -r a88ba599add1 -r 032fea10f8d1 tools/firmware/hvmloader/config.h
> --- a/tools/firmware/hvmloader/config.h	Tue Feb 21 17:45:59 2012 +0000
> +++ b/tools/firmware/hvmloader/config.h	Wed Feb 22 18:54:03 2012 +0000
> @@ -35,6 +35,8 @@ struct bios_config {
>  
>  extern struct bios_config rombios_config;
>  extern struct bios_config seabios_config;
> +extern struct bios_config ovmf32_config;
> +extern struct bios_config ovmf64_config;

Can you confirm that you need an OVMF which matches the OS bit-width you
are installing. i..e that there is no support for booting a 32 bit EFI
OS (or bootloader, shell, whatever it is called) on a 64 bit OVMF?

[...]
> +static void ovmf_acpi_build_tables(void)
> +{
> +    struct acpi_config config = {
> +        .dsdt_anycpu = dsdt_anycpu,
> +        .dsdt_anycpu_len = dsdt_anycpu_len,
> +        .dsdt_15cpu = dsdt_15cpu,
> +        .dsdt_15cpu_len = dsdt_15cpu_len,
> +    };

IIRC the 15cpu tables are there to workaround a bug in some old version
of Windows (2k?). I think therefore you can omit these on the basis that
no version of Windows with that bug also supports EFI. seabios.c does
this too.

[...]

Re: [PATCH 1 of 2] Add the support for Xen to include OVMF UEFI support and directly use it

[Attilio Rao]

On 23/02/12 10:07, Ian Campbell wrote:
> On Wed, 2012-02-22 at 19:17 +0000, Attilio Rao wrote:
>    
>> A way to integrate OVMF build directly into XEN has still be discussed
>> on the mailing list appropriately.
>>      
> AIUI OVMF is maintained in SVN. Our normal procedure for adding an
> external dependency would be for us to mirror it on xenbits as a
> convenience to our users, who don't need to get stuff from multiple
> places, and as a courtesy to our upstreams, so our users don't consume
> their resources.
>
> I don't much fancy setting the necessary webdav or whatever stuff on
> xenbits and integrating SVN support into our build system though. What
> do people think about using git-svn to manage our mirror in git instead?
> Or better: perhaps OVMF have an official git or hg mirror?
>
> Anyone have any thoughts/opinions/better ideas etc?
>
>    
>> diff -r a88ba599add1 -r 032fea10f8d1 tools/firmware/hvmloader/config.h
>> --- a/tools/firmware/hvmloader/config.h	Tue Feb 21 17:45:59 2012 +0000
>> +++ b/tools/firmware/hvmloader/config.h	Wed Feb 22 18:54:03 2012 +0000
>> @@ -35,6 +35,8 @@ struct bios_config {
>>
>>   extern struct bios_config rombios_config;
>>   extern struct bios_config seabios_config;
>> +extern struct bios_config ovmf32_config;
>> +extern struct bios_config ovmf64_config;
>>      
> Can you confirm that you need an OVMF which matches the OS bit-width you
> are installing. i..e that there is no support for booting a 32 bit EFI
> OS (or bootloader, shell, whatever it is called) on a 64 bit OVMF?
>
>    

I didn't test this case, really, but I would think OVMF-64 / OS-32 could 
possibly work.
You are suggesting if this is the case we should just ship the 64-bit 
emulation?
> [...]
>    
>> +static void ovmf_acpi_build_tables(void)
>> +{
>> +    struct acpi_config config = {
>> +        .dsdt_anycpu = dsdt_anycpu,
>> +        .dsdt_anycpu_len = dsdt_anycpu_len,
>> +        .dsdt_15cpu = dsdt_15cpu,
>> +        .dsdt_15cpu_len = dsdt_15cpu_len,
>> +    };
>>      
> IIRC the 15cpu tables are there to workaround a bug in some old version
> of Windows (2k?). I think therefore you can omit these on the basis that
> no version of Windows with that bug also supports EFI. seabios.c does
> this too.
>    

Yes, I was just unsure which of the 2 approaches (use dsdt_15cpu/skip 
completely) was preferred here so I went with the more 'complete' but it 
makes sense.

Thanks,
Attilio

Re: [PATCH 1 of 2] Add the support for Xen to include OVMF UEFI support and directly use it

[Ian Campbell]

On Thu, 2012-02-23 at 10:18 +0000, Attilio Rao wrote:
> On 23/02/12 10:07, Ian Campbell wrote:
> > On Wed, 2012-02-22 at 19:17 +0000, Attilio Rao wrote:
> >    
> >> A way to integrate OVMF build directly into XEN has still be discussed
> >> on the mailing list appropriately.
> >>      
> > AIUI OVMF is maintained in SVN. Our normal procedure for adding an
> > external dependency would be for us to mirror it on xenbits as a
> > convenience to our users, who don't need to get stuff from multiple
> > places, and as a courtesy to our upstreams, so our users don't consume
> > their resources.
> >
> > I don't much fancy setting the necessary webdav or whatever stuff on
> > xenbits and integrating SVN support into our build system though. What
> > do people think about using git-svn to manage our mirror in git instead?
> > Or better: perhaps OVMF have an official git or hg mirror?
> >
> > Anyone have any thoughts/opinions/better ideas etc?
> >
> >    
> >> diff -r a88ba599add1 -r 032fea10f8d1 tools/firmware/hvmloader/config.h
> >> --- a/tools/firmware/hvmloader/config.h	Tue Feb 21 17:45:59 2012 +0000
> >> +++ b/tools/firmware/hvmloader/config.h	Wed Feb 22 18:54:03 2012 +0000
> >> @@ -35,6 +35,8 @@ struct bios_config {
> >>
> >>   extern struct bios_config rombios_config;
> >>   extern struct bios_config seabios_config;
> >> +extern struct bios_config ovmf32_config;
> >> +extern struct bios_config ovmf64_config;
> >>      
> > Can you confirm that you need an OVMF which matches the OS bit-width you
> > are installing. i..e that there is no support for booting a 32 bit EFI
> > OS (or bootloader, shell, whatever it is called) on a 64 bit OVMF?
> >
> >    
> 
> I didn't test this case, really, but I would think OVMF-64 / OS-32 could 
> possibly work.
> You are suggesting if this is the case we should just ship the 64-bit 
> emulation?

That would be preferable if possible. I thought this didn't work though
(but I may have misunderstood or misremembered something).

Ian.

Re: [PATCH 1 of 2] Add the support for Xen to include OVMF UEFI support and directly use it

[Jan Beulich]

>>> Attilio Rao <attilio.rao@citrix.com> 02/23/12 11:18 AM >>>
>On 23/02/12 10:07, Ian Campbell wrote:
>> On Wed, 2012-02-22 at 19:17 +0000, Attilio Rao wrote:
>> Can you confirm that you need an OVMF which matches the OS bit-width you
>> are installing. i..e that there is no support for booting a 32 bit EFI
>> OS (or bootloader, shell, whatever it is called) on a 64 bit OVMF?
>
>I didn't test this case, really, but I would think OVMF-64 / OS-32 could 
>possibly work.

Native EFI requires bit-matched OS loaders, and I can't see how OVMF would
be able to get around this (but yes, it is theoretically possible, just that I
doubt anyone cared to implement this).

Jan

Re: [PATCH 1 of 2] Add the support for Xen to include OVMF UEFI support and directly use it

[Ian Campbell]

On Thu, 2012-02-23 at 14:48 +0000, Jan Beulich wrote:
> >>> Attilio Rao <attilio.rao@citrix.com> 02/23/12 11:18 AM >>>
> >On 23/02/12 10:07, Ian Campbell wrote:
> >> On Wed, 2012-02-22 at 19:17 +0000, Attilio Rao wrote:
> >> Can you confirm that you need an OVMF which matches the OS bit-width you
> >> are installing. i..e that there is no support for booting a 32 bit EFI
> >> OS (or bootloader, shell, whatever it is called) on a 64 bit OVMF?
> >
> >I didn't test this case, really, but I would think OVMF-64 / OS-32 could 
> >possibly work.
> 
> Native EFI requires bit-matched OS loaders,

Is that a shortcoming of EFI generally or just this implementation?

Surely people don't reflash their "BIOS" to be able to run a 32 vs 64
bit OS? Or do most OSes (even 32 bit ones) have a 64 bit loader capable
of loading a 32 bit OS?

Ian.

Re: [PATCH 1 of 2] Add the support for Xen to include OVMF UEFI support and directly use it

[Jordan Justen]

On Thu, Feb 23, 2012 at 07:37, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Thu, 2012-02-23 at 14:48 +0000, Jan Beulich wrote:
>> >>> Attilio Rao <attilio.rao@citrix.com> 02/23/12 11:18 AM >>>
>> >On 23/02/12 10:07, Ian Campbell wrote:
>> >> On Wed, 2012-02-22 at 19:17 +0000, Attilio Rao wrote:
>> >> Can you confirm that you need an OVMF which matches the OS bit-width you
>> >> are installing. i..e that there is no support for booting a 32 bit EFI
>> >> OS (or bootloader, shell, whatever it is called) on a 64 bit OVMF?
>> >
>> >I didn't test this case, really, but I would think OVMF-64 / OS-32 could
>> >possibly work.
>>
>> Native EFI requires bit-matched OS loaders,
>
> Is that a shortcoming of EFI generally or just this implementation?

This is basically built into the design of UEFI.  The issue is that
the firmware boot/runtime calls are only available for the native
architecture.

If the OS or loader had support for changing to the native
architecture before calling these services, it can run in another
mode.

> Surely people don't reflash their "BIOS" to be able to run a 32 vs 64
> bit OS? Or do most OSes (even 32 bit ones) have a 64 bit loader capable
> of loading a 32 bit OS?

I don't think people often update their firmware to switch modes.  In
fact, I think in most cases they don't have that option.  For example,
most desktop/server systems with UEFI support will likely only support
UEFI X64.

The loader or OS could have special support for running under this
environment.  It would just need to change to the proper CPU mode
before calling the firmware runtime services.

In the case of UEFI Linux and Windows x86, I think they only support
calling UEFI IA32.  In the case of UEFI Linux and Windows x86-64, I
think they only support calling UEFI X64.

-Jordan

Re: [PATCH 1 of 2] Add the support for Xen to include OVMF UEFI support and directly use it

[Ian Campbell]

On Thu, 2012-02-23 at 17:33 +0000, Jordan Justen wrote:
> On Thu, Feb 23, 2012 at 07:37, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Thu, 2012-02-23 at 14:48 +0000, Jan Beulich wrote:
> >> >>> Attilio Rao <attilio.rao@citrix.com> 02/23/12 11:18 AM >>>
> >> >On 23/02/12 10:07, Ian Campbell wrote:
> >> >> On Wed, 2012-02-22 at 19:17 +0000, Attilio Rao wrote:
> >> >> Can you confirm that you need an OVMF which matches the OS bit-width you
> >> >> are installing. i..e that there is no support for booting a 32 bit EFI
> >> >> OS (or bootloader, shell, whatever it is called) on a 64 bit OVMF?
> >> >
> >> >I didn't test this case, really, but I would think OVMF-64 / OS-32 could
> >> >possibly work.
> >>
> >> Native EFI requires bit-matched OS loaders,
> >
> > Is that a shortcoming of EFI generally or just this implementation?
> 
> This is basically built into the design of UEFI.  The issue is that
> the firmware boot/runtime calls are only available for the native
> architecture.

Thanks for confirming. I suppose in principal there could be some sort
of compat layer (such as we use for running 32 bit guests on a 64 bit
h/v) but from what you say below it doesn't sound like it would be worth
the time to implement.

> If the OS or loader had support for changing to the native
> architecture before calling these services, it can run in another
> mode.
> 
> > Surely people don't reflash their "BIOS" to be able to run a 32 vs 64
> > bit OS? Or do most OSes (even 32 bit ones) have a 64 bit loader capable
> > of loading a 32 bit OS?
> 
> I don't think people often update their firmware to switch modes.  In
> fact, I think in most cases they don't have that option.  For example,
> most desktop/server systems with UEFI support will likely only support
> UEFI X64.
> 
> The loader or OS could have special support for running under this
> environment.  It would just need to change to the proper CPU mode
> before calling the firmware runtime services.
> 
> In the case of UEFI Linux and Windows x86, I think they only support
> calling UEFI IA32.  In the case of UEFI Linux and Windows x86-64, I
> think they only support calling UEFI X64.

Out of interest do you know any OSes which have good support for 32 bit
UEFI?

Win7 and Ubuntu have support for 64 bit but the 32 bit equivalents don't
appear to.

I'm tempted to suggest that we only support 64 bit UEFI, at least
initially and defer 32 bit support until we have an actual user request
and rationale for that feature.

Ian.

Re: [PATCH 1 of 2] Add the support for Xen to include OVMF UEFI support and directly use it

[Jan Beulich]

>>> On 23.02.12 at 16:37, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Thu, 2012-02-23 at 14:48 +0000, Jan Beulich wrote:
>> >>> Attilio Rao <attilio.rao@citrix.com> 02/23/12 11:18 AM >>>
>> >On 23/02/12 10:07, Ian Campbell wrote:
>> >> On Wed, 2012-02-22 at 19:17 +0000, Attilio Rao wrote:
>> >> Can you confirm that you need an OVMF which matches the OS bit-width you
>> >> are installing. i..e that there is no support for booting a 32 bit EFI
>> >> OS (or bootloader, shell, whatever it is called) on a 64 bit OVMF?
>> >
>> >I didn't test this case, really, but I would think OVMF-64 / OS-32 could 
>> >possibly work.
>> 
>> Native EFI requires bit-matched OS loaders,
> 
> Is that a shortcoming of EFI generally or just this implementation?

I'd assume more the former.

> Surely people don't reflash their "BIOS" to be able to run a 32 vs 64
> bit OS? Or do most OSes (even 32 bit ones) have a 64 bit loader capable
> of loading a 32 bit OS?

When I asked the same question, I was told that the assumption is that
on modern (read: UEFI) systems people aren't expected to run 32-bit
OSes.

And no, I don't think any 32-bit OS comes with a 64-bit bootloader.
Furthermore, to make EFI really work, such an OS would also need
64-bit stubs to call the EFI services - not something that's realistically
going to happen I would think.

If anything, I would have expected that things could work out the
other way around: A 64-bit OS having a 32-bit loader and 32-bit
stubs (which is how we implemented this when, on x86, there was
no 64-bit EFI yet).

Jan

Re: [PATCH 1 of 2] Add the support for Xen to include OVMF UEFI support and directly use it

[Ian Campbell]

On Fri, 2012-02-24 at 08:35 +0000, Jan Beulich wrote:
> >>> On 23.02.12 at 16:37, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Thu, 2012-02-23 at 14:48 +0000, Jan Beulich wrote:
> >> >>> Attilio Rao <attilio.rao@citrix.com> 02/23/12 11:18 AM >>>
> >> >On 23/02/12 10:07, Ian Campbell wrote:
> >> >> On Wed, 2012-02-22 at 19:17 +0000, Attilio Rao wrote:
> >> >> Can you confirm that you need an OVMF which matches the OS bit-width you
> >> >> are installing. i..e that there is no support for booting a 32 bit EFI
> >> >> OS (or bootloader, shell, whatever it is called) on a 64 bit OVMF?
> >> >
> >> >I didn't test this case, really, but I would think OVMF-64 / OS-32 could 
> >> >possibly work.
> >> 
> >> Native EFI requires bit-matched OS loaders,
> > 
> > Is that a shortcoming of EFI generally or just this implementation?
> 
> I'd assume more the former.
> 
> > Surely people don't reflash their "BIOS" to be able to run a 32 vs 64
> > bit OS? Or do most OSes (even 32 bit ones) have a 64 bit loader capable
> > of loading a 32 bit OS?
> 
> When I asked the same question, I was told that the assumption is that
> on modern (read: UEFI) systems people aren't expected to run 32-bit
> OSes.

Doesn't that apply to running those OSes under Xen to some extent as
well? Can we get away with only supporting 64 bit EFI?

> And no, I don't think any 32-bit OS comes with a 64-bit bootloader.
> Furthermore, to make EFI really work, such an OS would also need
> 64-bit stubs to call the EFI services - not something that's realistically
> going to happen I would think.
> 
> If anything, I would have expected that things could work out the
> other way around: A 64-bit OS having a 32-bit loader and 32-bit
> stubs (which is how we implemented this when, on x86, there was
> no 64-bit EFI yet).

Yes, that does sound more plausible. Does anyone still do that though?
What about in a situation where they could just as easily run a
traditional BIOS (like in a Xen HVM guest)? Why would anyone insist on
being able to use 32 bit EFI if they could just use SeaBIOS? I suppose
the underlying question is: are there any OSes which _only_ support 32
bit EFI and don't support a traditional BIOS?

Ian.
Ian.

Re: [PATCH 1 of 2] Add the support for Xen to include OVMF UEFI support and directly use it

[Jan Beulich]

>>> On 24.02.12 at 09:43, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Fri, 2012-02-24 at 08:35 +0000, Jan Beulich wrote:
>> >>> On 23.02.12 at 16:37, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>> > On Thu, 2012-02-23 at 14:48 +0000, Jan Beulich wrote:
>> >> >>> Attilio Rao <attilio.rao@citrix.com> 02/23/12 11:18 AM >>>
>> >> >On 23/02/12 10:07, Ian Campbell wrote:
>> >> >> On Wed, 2012-02-22 at 19:17 +0000, Attilio Rao wrote:
>> >> >> Can you confirm that you need an OVMF which matches the OS bit-width you
>> >> >> are installing. i..e that there is no support for booting a 32 bit EFI
>> >> >> OS (or bootloader, shell, whatever it is called) on a 64 bit OVMF?
>> >> >
>> >> >I didn't test this case, really, but I would think OVMF-64 / OS-32 could 
>> >> >possibly work.
>> >> 
>> >> Native EFI requires bit-matched OS loaders,
>> > 
>> > Is that a shortcoming of EFI generally or just this implementation?
>> 
>> I'd assume more the former.
>> 
>> > Surely people don't reflash their "BIOS" to be able to run a 32 vs 64
>> > bit OS? Or do most OSes (even 32 bit ones) have a 64 bit loader capable
>> > of loading a 32 bit OS?
>> 
>> When I asked the same question, I was told that the assumption is that
>> on modern (read: UEFI) systems people aren't expected to run 32-bit
>> OSes.
> 
> Doesn't that apply to running those OSes under Xen to some extent as
> well? Can we get away with only supporting 64 bit EFI?

If we don't care about 32-bit OSes using EFI, yes.

>> And no, I don't think any 32-bit OS comes with a 64-bit bootloader.
>> Furthermore, to make EFI really work, such an OS would also need
>> 64-bit stubs to call the EFI services - not something that's realistically
>> going to happen I would think.
>> 
>> If anything, I would have expected that things could work out the
>> other way around: A 64-bit OS having a 32-bit loader and 32-bit
>> stubs (which is how we implemented this when, on x86, there was
>> no 64-bit EFI yet).
> 
> Yes, that does sound more plausible. Does anyone still do that though?

I don't think so (and what we did never got released in any form).

> What about in a situation where they could just as easily run a
> traditional BIOS (like in a Xen HVM guest)? Why would anyone insist on
> being able to use 32 bit EFI if they could just use SeaBIOS? I suppose
> the underlying question is: are there any OSes which _only_ support 32
> bit EFI and don't support a traditional BIOS?

I'm unaware of such OSes (and it would make pretty little sense to me
for the next couple of years). But it is clear that without legacy BIOS
support (which the ultimate goal of UEFI is), there's not going to be
a legacy (BIOS) boot method, and hence 32-bit OSes will be impossible
to run on such systems.

Jan

Re: [PATCH 1 of 2] Add the support for Xen to include OVMF UEFI support and directly use it

[Jordan Justen]

On Thu, Feb 23, 2012 at 02:07, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Wed, 2012-02-22 at 19:17 +0000, Attilio Rao wrote:
>> A way to integrate OVMF build directly into XEN has still be discussed
>> on the mailing list appropriately.
>
> AIUI OVMF is maintained in SVN. Our normal procedure for adding an
> external dependency would be for us to mirror it on xenbits as a
> convenience to our users, who don't need to get stuff from multiple
> places, and as a courtesy to our upstreams, so our users don't consume
> their resources.
>
> I don't much fancy setting the necessary webdav or whatever stuff on
> xenbits and integrating SVN support into our build system though. What
> do people think about using git-svn to manage our mirror in git instead?
> Or better: perhaps OVMF have an official git or hg mirror?

These are not 100% official, but I have been mirroring to git for over
a year now:
http://sourceforge.net/apps/mediawiki/tianocore/index.php?title=Source_Control#Mirrors

-Jordan

Re: [PATCH 1 of 2] Add the support for Xen to include OVMF UEFI support and directly use it

[Ian Campbell]

On Thu, 2012-02-23 at 16:21 +0000, Jordan Justen wrote:
> On Thu, Feb 23, 2012 at 02:07, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Wed, 2012-02-22 at 19:17 +0000, Attilio Rao wrote:
> >> A way to integrate OVMF build directly into XEN has still be discussed
> >> on the mailing list appropriately.
> >
> > AIUI OVMF is maintained in SVN. Our normal procedure for adding an
> > external dependency would be for us to mirror it on xenbits as a
> > convenience to our users, who don't need to get stuff from multiple
> > places, and as a courtesy to our upstreams, so our users don't consume
> > their resources.
> >
> > I don't much fancy setting the necessary webdav or whatever stuff on
> > xenbits and integrating SVN support into our build system though. What
> > do people think about using git-svn to manage our mirror in git instead?
> > Or better: perhaps OVMF have an official git or hg mirror?
> 
> These are not 100% official, but I have been mirroring to git for over
> a year now:
> http://sourceforge.net/apps/mediawiki/tianocore/index.php?title=Source_Control#Mirrors

More official than anything we would have come up with so I think this
is what we should use.

Thanks Jordan!

> 
> -Jordan

Re: [PATCH 1 of 2] Add the support for Xen to include OVMF UEFI support and directly use it

[Keir Fraser]

On 22/02/2012 19:17, "Attilio Rao" <attilio.rao@citrix.com> wrote:

> when specified in the guest configuration file.
> This work is somewhat based on Bei Guan effort during the SoC 2011 and
> relies on upstream edk2/ovmf Tianocore ROM to be built separately and
> manually copied as:
> Build/OvmfX64/DEBUG_GCC44/FV/OVMF.fd -> tools/firmware/ovmf/ovmf-x64.bin
> Build/OvmfIa32/DEBUG_GCC44/FV/OVMF.fd -> toolf/firmware/ovmf/ovmf-ia32.bin
> 
> A way to integrate OVMF build directly into XEN has still be discussed
> on the mailing list appropriately.

I applied this patch but changed CONFIG_OVMF to 'n' so that we don't break
the default build while ovmf is not integrated into our build process.

I have not applied the other patch (to libxl), but leave that to be reviewed
by libxl maintainers.

 -- Keir

> Signed-off-by: Attilio Rao <attilio.rao@citrix.com>
> 
> diff -r a88ba599add1 -r 032fea10f8d1 Config.mk
> --- a/Config.mk Tue Feb 21 17:45:59 2012 +0000
> +++ b/Config.mk Wed Feb 22 18:54:03 2012 +0000
> @@ -224,6 +224,7 @@ SEABIOS_UPSTREAM_TAG ?= rel-1.6.3.1
>  
>  ETHERBOOT_NICS ?= rtl8139 8086100e
>  
> +CONFIG_OVMF ?= y
>  CONFIG_ROMBIOS ?= y
>  CONFIG_SEABIOS ?= y
>  
> diff -r a88ba599add1 -r 032fea10f8d1 tools/firmware/hvmloader/Makefile
> --- a/tools/firmware/hvmloader/Makefile Tue Feb 21 17:45:59 2012 +0000
> +++ b/tools/firmware/hvmloader/Makefile Wed Feb 22 18:54:03 2012 +0000
> @@ -37,6 +37,7 @@ endif
>  
>  CIRRUSVGA_DEBUG ?= n
>  
> +OVMF_DIR := ../ovmf
>  ROMBIOS_DIR := ../rombios
>  SEABIOS_DIR := ../seabios-dir
>  
> @@ -52,6 +53,14 @@ endif
>  
>  ROMS := 
>  
> +ifeq ($(CONFIG_OVMF),y)
> +OBJS += ovmf.o
> +CFLAGS += -DENABLE_OVMF32 -DENABLE_OVMF64
> +OVMF32_ROM := $(OVMF_DIR)/ovmf-ia32.bin
> +OVMF64_ROM := $(OVMF_DIR)/ovmf-x64.bin
> +ROMS += $(OVMF32_ROM) $(OVMF64_ROM)
> +endif
> +
>  ifeq ($(CONFIG_ROMBIOS),y)
>  OBJS += optionroms.o 32bitbios_support.o rombios.o
>  CFLAGS += -DENABLE_ROMBIOS
> @@ -70,7 +79,7 @@ endif
>  all: subdirs-all
> $(MAKE) hvmloader
>  
> -rombios.o seabios.o hvmloader.o: roms.inc
> +ovmf.o rombios.o seabios.o hvmloader.o: roms.inc
>  smbios.o: CFLAGS += -D__SMBIOS_DATE__="\"$(shell date +%m/%d/%Y)\""
>  
>  hvmloader: $(OBJS) acpi/acpi.a
> @@ -93,6 +102,18 @@ ifneq ($(SEABIOS_ROM),)
> echo "#endif" >> $@.new
>  endif
>  
> +ifneq ($(OVMF32_ROM),)
> + echo "#ifdef ROM_INCLUDE_OVMF32" >> $@.new
> + sh ./mkhex ovmf32 $(OVMF32_ROM) >> $@.new
> + echo "#endif" >> $@.new 
> +endif 
> +
> +ifneq ($(OVMF64_ROM),)
> + echo "#ifdef ROM_INCLUDE_OVMF64" >> $@.new
> + sh ./mkhex ovmf64 $(OVMF64_ROM) >> $@.new
> + echo "#endif" >> $@.new 
> +endif 
> +
>  ifneq ($(STDVGA_ROM),)
> echo "#ifdef ROM_INCLUDE_VGABIOS" >> $@.new
> sh ./mkhex vgabios_stdvga $(STDVGA_ROM) >> $@.new
> diff -r a88ba599add1 -r 032fea10f8d1 tools/firmware/hvmloader/config.h
> --- a/tools/firmware/hvmloader/config.h Tue Feb 21 17:45:59 2012 +0000
> +++ b/tools/firmware/hvmloader/config.h Wed Feb 22 18:54:03 2012 +0000
> @@ -35,6 +35,8 @@ struct bios_config {
>  
>  extern struct bios_config rombios_config;
>  extern struct bios_config seabios_config;
> +extern struct bios_config ovmf32_config;
> +extern struct bios_config ovmf64_config;
>  
>  #define PAGE_SHIFT 12
>  #define PAGE_SIZE  (1ul << PAGE_SHIFT)
> diff -r a88ba599add1 -r 032fea10f8d1 tools/firmware/hvmloader/hvmloader.c
> --- a/tools/firmware/hvmloader/hvmloader.c Tue Feb 21 17:45:59 2012 +0000
> +++ b/tools/firmware/hvmloader/hvmloader.c Wed Feb 22 18:54:03 2012 +0000
> @@ -212,6 +212,12 @@ struct bios_info {
>  #ifdef ENABLE_SEABIOS
>      { "seabios", &seabios_config, },
>  #endif
> +#ifdef ENABLE_OVMF32
> +    { "ovmf-ia32", &ovmf32_config, },
> +#endif
> +#ifdef ENABLE_OVMF64
> +    { "ovmf-x64", &ovmf64_config, },
> +#endif
>      { NULL, NULL }
>  };
>  
> diff -r a88ba599add1 -r 032fea10f8d1 tools/firmware/hvmloader/ovmf.c
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++ b/tools/firmware/hvmloader/ovmf.c Wed Feb 22 18:54:03 2012 +0000
> @@ -0,0 +1,147 @@
> +/*
> + * HVM OVMF UEFI support.
> + *
> + * Bei Guan, gbtju85@gmail.com
> + * Andrei Warkentin, andreiw@motorola.com
> + * Leendert van Doorn, leendert@watson.ibm.com
> + * Copyright (c) 2005, International Business Machines Corporation.
> + * Copyright (c) 2006, Keir Fraser, XenSource Inc.
> + * Copyright (c) 2011, Citrix Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> with
> + * this program; if not, write to the Free Software Foundation, Inc., 59
> Temple
> + * Place - Suite 330, Boston, MA 02111-1307 USA.
> + */
> +
> +#include "config.h"
> +#include "smbios_types.h"
> +#include "acpi/acpi2_0.h"
> +#include "apic_regs.h"
> +#include "../rombios/config.h"
> +#include "util.h"
> +#include "pci_regs.h"
> +#include "hypercall.h"
> +
> +#include <xen/hvm/params.h>
> +#include <xen/hvm/ioreq.h>
> +#include <xen/memory.h>
> +
> +#define ROM_INCLUDE_OVMF32
> +#define ROM_INCLUDE_OVMF64
> +#include "roms.inc"
> +
> +#define OVMF_BEGIN              0xFFF00000ULL
> +#define OVMF_SIZE               0x00100000ULL
> +#define OVMF_MAXOFFSET          0x000FFFFFULL
> +#define OVMF_END                (OVMF_BEGIN + OVMF_SIZE)
> +#define LOWCHUNK_BEGIN          0x000F0000
> +#define LOWCHUNK_SIZE           0x00010000
> +#define LOWCHUNK_MAXOFFSET      0x0000FFFF
> +#define LOWCHUNK_END            (OVMF_BEGIN + OVMF_SIZE)
> +
> +extern unsigned char dsdt_anycpu[], dsdt_15cpu[];
> +extern int dsdt_anycpu_len, dsdt_15cpu_len;
> +
> +static void ovmf_load(const struct bios_config *config)
> +{
> +    xen_pfn_t mfn;
> +    uint64_t addr = OVMF_BEGIN;
> +
> +    /* Copy low-reset vector portion. */
> +    memcpy((void *) LOWCHUNK_BEGIN, (uint8_t *) config->image
> +           + OVMF_SIZE
> +           - LOWCHUNK_SIZE,
> +           LOWCHUNK_SIZE);
> +
> +    /* Ensure we have backing page prior to moving FD. */
> +    while ((addr >> PAGE_SHIFT) != (OVMF_END >> PAGE_SHIFT)) {
> +        mfn = (uint32_t) (addr >> PAGE_SHIFT);
> +        addr += PAGE_SIZE;
> + mem_hole_populate_ram(mfn, 1);
> +    }
> +
> +    /* Copy FD. */
> +    memcpy((void *) OVMF_BEGIN, config->image, OVMF_SIZE);
> +}
> +
> +static void ovmf_acpi_build_tables(void)
> +{
> +    struct acpi_config config = {
> +        .dsdt_anycpu = dsdt_anycpu,
> +        .dsdt_anycpu_len = dsdt_anycpu_len,
> +        .dsdt_15cpu = dsdt_15cpu,
> +        .dsdt_15cpu_len = dsdt_15cpu_len,
> +    };
> +
> +    acpi_build_tables(&config, ACPI_PHYSICAL_ADDRESS);
> +}
> +
> +static void ovmf_create_smbios_tables(void)
> +{
> +    hvm_write_smbios_tables(SMBIOS_PHYSICAL_ADDRESS,
> +                            SMBIOS_PHYSICAL_ADDRESS + sizeof(struct
> smbios_entry_point),
> +                            SMBIOS_PHYSICAL_END);
> +}
> +
> +struct bios_config ovmf32_config =  {
> +    .name = "OVMF-IA32",
> +
> +    .image = ovmf32,
> +    .image_size = sizeof(ovmf32),
> +
> +    .bios_address = 0,
> +    .bios_load = ovmf_load,
> +
> +    .load_roms = 0,
> +
> +    .bios_info_setup = NULL,
> +    .bios_info_finish = NULL,
> +
> +    .e820_setup = NULL,
> +
> +    .acpi_build_tables = ovmf_acpi_build_tables,
> +    .create_mp_tables = NULL,
> +    .create_smbios_tables = ovmf_create_smbios_tables,
> +    .create_pir_tables = NULL,
> +};
> +
> +struct bios_config ovmf64_config =  {
> +    .name = "OVMF-X64",
> +
> +    .image = ovmf64,
> +    .image_size = sizeof(ovmf64),
> +
> +    .bios_address = 0,
> +    .bios_load = ovmf_load,
> +
> +    .load_roms = 0,
> +
> +    .bios_info_setup = NULL,
> +    .bios_info_finish = NULL,
> +
> +    .e820_setup = NULL,
> +
> +    .acpi_build_tables = ovmf_acpi_build_tables,
> +    .create_mp_tables = NULL,
> +    .create_smbios_tables = ovmf_create_smbios_tables,
> +    .create_pir_tables = NULL,
> +};
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-set-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

[PATCH 2 of 2] Add the ability to specify the option "bios_override" in the guest

[Attilio Rao]

configuration file.
An example is given by:
bios_override="ovmf-x64"

which forces BIOS to be the OVMF UEFI for 64 bits architectures.

Signed-off-by: Attilio Rao <attilio.rao@citrix.com>

diff -r 032fea10f8d1 -r 520ba1527b7a tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c	Wed Feb 22 18:54:03 2012 +0000
+++ b/tools/libxl/libxl_create.c	Wed Feb 22 18:56:22 2012 +0000
@@ -89,6 +89,7 @@ int libxl_init_build_info(libxl_ctx *ctx
     case LIBXL_DOMAIN_TYPE_HVM:
         b_info->video_memkb = 8 * 1024;
         b_info->u.hvm.firmware = NULL;
+        b_info->u.hvm.bios = NULL;
         b_info->u.hvm.pae = 1;
         b_info->u.hvm.apic = 1;
         b_info->u.hvm.acpi = 1;
diff -r 032fea10f8d1 -r 520ba1527b7a tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c	Wed Feb 22 18:54:03 2012 +0000
+++ b/tools/libxl/libxl_dm.c	Wed Feb 22 18:56:22 2012 +0000
@@ -66,6 +66,8 @@ const char *libxl__domain_device_model(l
 static const char *libxl__domain_bios(libxl__gc *gc,
                                 const libxl_domain_build_info *info)
 {
+    if (info->u.hvm.bios)
+       return info->u.hvm.bios;
     switch (info->device_model_version) {
     case 1: return "rombios";
     case 2: return "seabios";
diff -r 032fea10f8d1 -r 520ba1527b7a tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl	Wed Feb 22 18:54:03 2012 +0000
+++ b/tools/libxl/libxl_types.idl	Wed Feb 22 18:56:22 2012 +0000
@@ -228,6 +228,7 @@ libxl_domain_build_info = Struct("domain
 
     ("u", KeyedUnion(None, libxl_domain_type, "type",
                 [("hvm", Struct(None, [("firmware", string),
+                                       ("bios", string),
                                        ("pae", bool),
                                        ("apic", bool),
                                        ("acpi", bool),
diff -r 032fea10f8d1 -r 520ba1527b7a tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Wed Feb 22 18:54:03 2012 +0000
+++ b/tools/libxl/xl_cmdimpl.c	Wed Feb 22 18:56:22 2012 +0000
@@ -704,6 +704,8 @@ static void parse_config_data(const char
 
         xlu_cfg_replace_string (config, "firmware_override",
                                 &b_info->u.hvm.firmware, 0);
+        xlu_cfg_replace_string (config, "bios_override",
+                                &b_info->u.hvm.bios, 0);
         if (!xlu_cfg_get_long (config, "pae", &l, 0))
             b_info->u.hvm.pae = l;
         if (!xlu_cfg_get_long (config, "apic", &l, 0))

Re: [PATCH 2 of 2] Add the ability to specify the option "bios_override" in the guest

[Ian Campbell]

On Wed, 2012-02-22 at 19:17 +0000, Attilio Rao wrote:
> configuration file.
> An example is given by:
> bios_override="ovmf-x64"

I'm not sure I would use the override suffix here -- that is generally
for settings where we recommend that the user accepts the default. In
this case though a user has perfectly valid reasons for selecting
ovmf-{32,64} or seabios etc.

Also please update docs/man/xl.cfg.pod.5 to document the new setting.

> diff -r 032fea10f8d1 -r 520ba1527b7a tools/libxl/libxl_dm.c
> --- a/tools/libxl/libxl_dm.c	Wed Feb 22 18:54:03 2012 +0000
> +++ b/tools/libxl/libxl_dm.c	Wed Feb 22 18:56:22 2012 +0000
> @@ -66,6 +66,8 @@ const char *libxl__domain_device_model(l
>  static const char *libxl__domain_bios(libxl__gc *gc,
>                                  const libxl_domain_build_info *info)
>  {
> +    if (info->u.hvm.bios)
> +       return info->u.hvm.bios;
>      switch (info->device_model_version) {
>      case 1: return "rombios";
>      case 2: return "seabios";

Should integrate this function into libxl__domain_build_info_setdefaults
from my "libxl: improved handling for default values in API" series.

Also it would be good to use that same infrastructure to enforce that
qemu-xen-traditional can only use rombios and that seabios and ovmf-*
are only valid with qemu-xen.

> diff -r 032fea10f8d1 -r 520ba1527b7a tools/libxl/libxl_types.idl
> --- a/tools/libxl/libxl_types.idl	Wed Feb 22 18:54:03 2012 +0000
> +++ b/tools/libxl/libxl_types.idl	Wed Feb 22 18:56:22 2012 +0000
> @@ -228,6 +228,7 @@ libxl_domain_build_info = Struct("domain
>  
>      ("u", KeyedUnion(None, libxl_domain_type, "type",
>                  [("hvm", Struct(None, [("firmware", string),
> +                                       ("bios", string),

I think an Enumeration would be better here.

>                                         ("pae", bool),
>                                         ("apic", bool),
>                                         ("acpi", bool),
> diff -r 032fea10f8d1 -r 520ba1527b7a tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c	Wed Feb 22 18:54:03 2012 +0000
> +++ b/tools/libxl/xl_cmdimpl.c	Wed Feb 22 18:56:22 2012 +0000
> @@ -704,6 +704,8 @@ static void parse_config_data(const char
>  
>          xlu_cfg_replace_string (config, "firmware_override",
>                                  &b_info->u.hvm.firmware, 0);
> +        xlu_cfg_replace_string (config, "bios_override",
> +                                &b_info->u.hvm.bios, 0);
>          if (!xlu_cfg_get_long (config, "pae", &l, 0))
>              b_info->u.hvm.pae = l;
>          if (!xlu_cfg_get_long (config, "apic", &l, 0))
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH 2 of 2] Add the ability to specify the option "bios_override" in the guest

[Attilio Rao]

On 23/02/12 10:13, Ian Campbell wrote:
> On Wed, 2012-02-22 at 19:17 +0000, Attilio Rao wrote:
>    
>> configuration file.
>> An example is given by:
>> bios_override="ovmf-x64"
>>      
> I'm not sure I would use the override suffix here -- that is generally
> for settings where we recommend that the user accepts the default. In
> this case though a user has perfectly valid reasons for selecting
> ovmf-{32,64} or seabios etc.
>
> Also please update docs/man/xl.cfg.pod.5 to document the new setting.
>    

It makes sense, thanks.

>> diff -r 032fea10f8d1 -r 520ba1527b7a tools/libxl/libxl_dm.c
>> --- a/tools/libxl/libxl_dm.c	Wed Feb 22 18:54:03 2012 +0000
>> +++ b/tools/libxl/libxl_dm.c	Wed Feb 22 18:56:22 2012 +0000
>> @@ -66,6 +66,8 @@ const char *libxl__domain_device_model(l
>>   static const char *libxl__domain_bios(libxl__gc *gc,
>>                                   const libxl_domain_build_info *info)
>>   {
>> +    if (info->u.hvm.bios)
>> +       return info->u.hvm.bios;
>>       switch (info->device_model_version) {
>>       case 1: return "rombios";
>>       case 2: return "seabios";
>>      
> Should integrate this function into libxl__domain_build_info_setdefaults
> from my "libxl: improved handling for default values in API" series.
>
>    

I really wanted to do this type of integration in a separate step.

> Also it would be good to use that same infrastructure to enforce that
> qemu-xen-traditional can only use rombios and that seabios and ovmf-*
> are only valid with qemu-xen.
>
>    
>> diff -r 032fea10f8d1 -r 520ba1527b7a tools/libxl/libxl_types.idl
>> --- a/tools/libxl/libxl_types.idl	Wed Feb 22 18:54:03 2012 +0000
>> +++ b/tools/libxl/libxl_types.idl	Wed Feb 22 18:56:22 2012 +0000
>> @@ -228,6 +228,7 @@ libxl_domain_build_info = Struct("domain
>>
>>       ("u", KeyedUnion(None, libxl_domain_type, "type",
>>                   [("hvm", Struct(None, [("firmware", string),
>> +                                       ("bios", string),
>>      
> I think an Enumeration would be better here.
> libxl__domain_bios

I just found it quicker/cleaner to just store the string and pass it 
down, rather than translating to an Enumeration and then translate it 
back to a string into libxl__domain_bios(), don't you think?
However I'm open to do what you feel is better.

Thanks,
Attilio

Re: [PATCH 2 of 2] Add the ability to specify the option "bios_override" in the guest

[Ian Campbell]

On Thu, 2012-02-23 at 10:31 +0000, Attilio Rao wrote:
> On 23/02/12 10:13, Ian Campbell wrote:
> > On Wed, 2012-02-22 at 19:17 +0000, Attilio Rao wrote:
> >    
> >> configuration file.
> >> An example is given by:
> >> bios_override="ovmf-x64"
> >>      
> > I'm not sure I would use the override suffix here -- that is generally
> > for settings where we recommend that the user accepts the default. In
> > this case though a user has perfectly valid reasons for selecting
> > ovmf-{32,64} or seabios etc.
> >
> > Also please update docs/man/xl.cfg.pod.5 to document the new setting.
> >    
> 
> It makes sense, thanks.
> 
> >> diff -r 032fea10f8d1 -r 520ba1527b7a tools/libxl/libxl_dm.c
> >> --- a/tools/libxl/libxl_dm.c	Wed Feb 22 18:54:03 2012 +0000
> >> +++ b/tools/libxl/libxl_dm.c	Wed Feb 22 18:56:22 2012 +0000
> >> @@ -66,6 +66,8 @@ const char *libxl__domain_device_model(l
> >>   static const char *libxl__domain_bios(libxl__gc *gc,
> >>                                   const libxl_domain_build_info *info)
> >>   {
> >> +    if (info->u.hvm.bios)
> >> +       return info->u.hvm.bios;
> >>       switch (info->device_model_version) {
> >>       case 1: return "rombios";
> >>       case 2: return "seabios";
> >>      
> > Should integrate this function into libxl__domain_build_info_setdefaults
> > from my "libxl: improved handling for default values in API" series.
> >
> >    
> 
> I really wanted to do this type of integration in a separate step.

Sure, either you or I can take a look after both series are in.

> > Also it would be good to use that same infrastructure to enforce that
> > qemu-xen-traditional can only use rombios and that seabios and ovmf-*
> > are only valid with qemu-xen.
> >
> >    
> >> diff -r 032fea10f8d1 -r 520ba1527b7a tools/libxl/libxl_types.idl
> >> --- a/tools/libxl/libxl_types.idl	Wed Feb 22 18:54:03 2012 +0000
> >> +++ b/tools/libxl/libxl_types.idl	Wed Feb 22 18:56:22 2012 +0000
> >> @@ -228,6 +228,7 @@ libxl_domain_build_info = Struct("domain
> >>
> >>       ("u", KeyedUnion(None, libxl_domain_type, "type",
> >>                   [("hvm", Struct(None, [("firmware", string),
> >> +                                       ("bios", string),
> >>      
> > I think an Enumeration would be better here.
> > libxl__domain_bios
> 
> I just found it quicker/cleaner to just store the string and pass it 
> down, rather than translating to an Enumeration and then translate it 
> back to a string into libxl__domain_bios(), don't you think?

The IDL provides helpers for doing this (..._{to,from}_string). You
would end up just calling that instead of libxl__domain_bios(). i.e.
libxl__domain_bios goes away because the logic for setting
info->u.hvm.bios (in the future in setdefaults) ensures it is always a
specific valid value.

> However I'm open to do what you feel is better.
> 
> Thanks,
> Attilio