Re: [Xen-devel] [PATCH] [PATCH v2] Add the bios option

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

> [Patch v2] Add the bios option to specify the bios to load.
> 
> Please note 2 things:
> - I've used the name of 'OVMF' rather than adding the arch postfix 
> because we are going to make this code arch-size agnostic (in another
> patch)
> - Other seems missing in xl.cfg.pod5, like firmware_override

:-( If you notice such please add them, with an XXX is fine. In this
case I thought I saw a patch from Olaf adding a bunch of missing
options.

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

> On Fri, 2012-02-24 at 18:24 +0000, Attilio Rao wrote:
> diff -r adcd6ab160fa -r 20ea6f881776 docs/man/xl.cfg.pod.5
> --- a/docs/man/xl.cfg.pod.5   Thu Feb 23 10:29:27 2012 +0000
> +++ b/docs/man/xl.cfg.pod.5   Fri Feb 24 18:19:56 2012 +0000
> @@ -430,6 +430,12 @@ accept the defaults for these options wh
>  
>  =over 4
>  
> +=item B<bios="STRING">

Can we enumerate the valid values of "STRING" here and their
requirements (even if we aren't enforcing them) WRT which
device_model_version they can be used with.

> +Load the specified BIOS in HVMLOADER. By default, HVMLOADER will try
> +to do a guess about the BIOS to run, but sometimes it may be useful
> +to force a different one, like OVMF UEFI.

I don't think we need to mention the implementation detail that
HVMLOADER is involved. I think it will be sufficient to simply say that
this selects the virtual firmware which is exposed to the guest.

> +
>  =item B<pae=BOOLEAN>
>  
>  Hide or expose the IA32 Physical Address Extensions. These extensions
> diff -r adcd6ab160fa -r 20ea6f881776 tools/libxl/libxl_create.c
> --- a/tools/libxl/libxl_create.c      Thu Feb 23 10:29:27 2012 +0000
> +++ b/tools/libxl/libxl_create.c      Fri Feb 24 18:19:56 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 = 0;
>          b_info->u.hvm.pae = 1;
>          b_info->u.hvm.apic = 1;
>          b_info->u.hvm.acpi = 1;
> diff -r adcd6ab160fa -r 20ea6f881776 tools/libxl/libxl_dm.c
> --- a/tools/libxl/libxl_dm.c  Thu Feb 23 10:29:27 2012 +0000
> +++ b/tools/libxl/libxl_dm.c  Fri Feb 24 18:19:56 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 libxl_bios_types_to_string(info->u.hvm.bios);
>      switch (info->device_model_version) {
>      case 1: return "rombios";
>      case 2: return "seabios";
> diff -r adcd6ab160fa -r 20ea6f881776 tools/libxl/libxl_types.idl
> --- a/tools/libxl/libxl_types.idl     Thu Feb 23 10:29:27 2012 +0000
> +++ b/tools/libxl/libxl_types.idl     Fri Feb 24 18:19:56 2012 +0000
> @@ -99,6 +99,12 @@ libxl_timer_mode = Enumeration("timer_mo
>      (3, "one_missed_tick_pending"),
>      ])
>  
> +libxl_bios_types = Enumeration("bios_types", [

Whether by coincidence or design we seem to have been using the singular
form for our enum names, so this would be "libxl_bios_type"

Ian.

> +    (1, "rombios"),
> +    (2, "seabios"),
> +    (3, "ovmf"),
> +    ])
> +
>  #
>  # Complex libxl types
>  #
> @@ -228,6 +234,7 @@ libxl_domain_build_info = Struct("domain
>  
>      ("u", KeyedUnion(None, libxl_domain_type, "type",
>                  [("hvm", Struct(None, [("firmware", string),
> +                                       ("bios", libxl_bios_types),
>                                         ("pae", bool),
>                                         ("apic", bool),
>                                         ("acpi", bool),
> diff -r adcd6ab160fa -r 20ea6f881776 tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c        Thu Feb 23 10:29:27 2012 +0000
> +++ b/tools/libxl/xl_cmdimpl.c        Fri Feb 24 18:19:56 2012 +0000
> @@ -704,6 +704,12 @@ static void parse_config_data(const char
>  
>          xlu_cfg_replace_string (config, "firmware_override",
>                                  &b_info->u.hvm.firmware, 0);
> +        if (!xlu_cfg_get_string(config, "bios", &buf, 0) &&
> +            libxl_bios_types_from_string(buf, &b_info->u.hvm.bios)) {
> +                fprintf(stderr, "ERROR: invalid value \"%s\" for \"bios\"\n",
> +                    buf);
> +                exit (1);
> +        }
>          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@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel