Re: [Xen-devel] [PATCH v2 1/3] x86: remove PVHv1 code

[Roger Pau Monne <roger.pau@xxxxxxxxxx>]

On Wed, Mar 01, 2017 at 02:18:16PM +0000, Ian Jackson wrote:
> Roger Pau Monne writes ("Re: [PATCH v2 1/3] x86: remove PVHv1 code"):
> > On Wed, Mar 01, 2017 at 01:53:29PM +0000, Ian Jackson wrote:
> > > Well, PVHv2 is in the process of becoming properly supported, so now
> > > is the time to decide the "official" way.
> > 
> > I prefer builder="hvm" device_model_version="none" because I think
> > it's clearer from a user PoV that a HVM guest it being
> > created.
> 
> Err, but a PVH guest is not an HVM guest in the sense that the user
> will expect.  Whenever I explain to anyone the difference between PV
> and HVM, the explanation is that "HVM provides a complete emulated PC"
> and "PV needs a guest operating systemn modified to work under Xen".
> By both these measures, a PVH guest is more like PV than HVM.
> 
> The use of the CPU extensions which originally only enabled support
> for HVM is a detail which most people will not be so interested in.
> The details of API, ABI and so on are not of interest to the writer of
> the xl domain config file.

It's not just the CPU extensions, we are also providing it with a local APIC
and ACPI tables, and it looks like we will also have OVMF firmware support for
PVH in the near future.

This is all just a question of taste at the end, but to me it tastes much more
like HVM rather than PV.

> > OTOH, using pvh=1 it's more obscure, and it isn't clear
> > which kind of guest you are creating, and which options apply to
> > it. Although all that can be fixed in the man page, I think it's
> > less intuitive.
> 
> The explanation we have been giving to ordinary users is that there
> are going to be three kinds of guest: PV, HVM, and the new PVH.
> 
> > TBH, I'm not even sure we should keep the "pvh" option, the same kernel that
> > previously worked with pvh=1 might not work anymore when this patch is 
> > applied.
> 
> PVHv1 was never supported so there is no need to worry about this.

Oh, right.

> > > When you say "it will basically fill the PV side", what is "it" ?
> > > Do you mean xl_parse.c ?
> > 
> > Yes, parse_config_data. With the current code in parse_config_data
> > domain type (c_info->type) is set to PV when pvh=1 is set in the
> > config file. Then in the same function, further below, options like
> > nestedhvm are simply ignored.
> > 
> > I don't any other way to solve this rather than forcing domain type to HVM 
> > in
> > parse_config_data when pvh=1 is set.
> > 
> > > Isn't this what libxl_domain_build_info_init_type is for ?
> > 
> > libxl_domain_build_info_init_type is not be able to re-parse the config 
> > file.
> 
> libxl_domain_build_info_init_type is called in xl_parse.c before any
> of the type-specific fields are set.  That's it's whole purpose.
> 
> So I think having libxl_domain_build_info_init_type set type to HVM if
> pvh=1 would solve the problem.

I've looked at it again, and no, libxl_domain_build_info_init_type when called
from parse_config_data is not able to set the type of the guest to HVM. This is
it's prototype:

libxl_domain_build_info_init_type(libxl_domain_build_info *p, libxl_domain_type 
type);

Type (second parameter) is PV when pvh=1 is set, and that's all the info
provided. libxl_domain_build_info_init_type has no way to know whether the info
it's initializing is for a PV or PVH guest, because it's lacking context.

Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel