[libvirt] [Qemu-devel] [PATCH 6/7] target-i386: add implementation of query-cpudefs

[Eduardo Habkost]

On Fri, Aug 10, 2012 at 09:43:21AM -0500, Anthony Liguori wrote:
> Eduardo Habkost <ehabkost at redhat.com> writes:
> 
> > On Fri, Jul 27, 2012 at 08:37:18AM -0500, Anthony Liguori wrote:
> >> Signed-off-by: Anthony Liguori <aliguori at us.ibm.com>
> >> ---
> >>  target-i386/cpu.c |   22 ++++++++++++++++++++++
> >>  1 files changed, 22 insertions(+), 0 deletions(-)
> >> 
> >> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> >> index 6b9659f..b398439 100644
> >> --- a/target-i386/cpu.c
> >> +++ b/target-i386/cpu.c
> >> @@ -28,6 +28,7 @@
> >>  #include "qemu-config.h"
> >>  
> >>  #include "qapi/qapi-visit-core.h"
> >> +#include "qmp-commands.h"
> >>  
> >>  #include "hyperv.h"
> >>  
> >> @@ -1123,6 +1124,27 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf, const char *optarg)
> >>      }
> >>  }
> >>  
> >> +CpuDefInfoList *qmp_query_cpudefs(Error **errp)
> >> +{
> >> +    CpuDefInfoList *cpu_list = NULL;
> >> +    x86_def_t *def;
> >> +
> >> +    for (def = x86_defs; def; def = def->next) {
> >> +        CpuDefInfoList *entry;
> >> +        CpuDefInfo *info;
> >> +
> >> +        info = g_malloc0(sizeof(*info));
> >> +        info->name = g_strdup(def->name);
> >> +
> >> +        entry = g_malloc0(sizeof(*entry));
> >> +        entry->value = info;
> >> +        entry->next = cpu_list;
> >> +        cpu_list = entry;
> >> +    }
> >> +
> >> +    return cpu_list;
> >> +}
> >
> > How would the interface look like once we:
> > - let libvirt know which features are available on each CPU model
> >   (libvirt needs that information[1]); and
> 
> I'm not sure I understand why libvirt needs this information.  Can you elaborate?

I see two reasons:

- The libvirt API has functions to tell the user which features are
  going to be enabled for each CPU model, so it needs to know which
  features are enabled or not, for each machine-type + cpu-model
  combination, so this information can be reported proeprly.
  - Also, if libvirt can enable/disable specific CPU features in the
    command-line, it just makes sens to know which ones are already
    enabled in each built-in CPU model.

- Probing for migration: libvirt needs to know if a given CPU model on a
  host can be migrated to another host. To know that, two pieces of
  information are needed:
  A) Which CPU features are visible to the guest for a specific
     configuration;
  B) Which of those features are really supported by the host
     hardware+kernel+QEMU, on the destination host, so it can
     know if migration is really possible.
  I am assuming that libvirt will query for A and B, and then combine it
  itself. But QEMU could also simply calculate (A&B) itself, and just
  have a boolean function that tells if a given model can be run on a
  specific host or not. But then it wouldn't solve the first item above
  (knowing which features are already enabled on a CPU model).
  - The problem is: even if QEMU does the check itself, the result of
    that "can this host run this VM?" probing function depends on the
    machine-type, too (see explanation below).


> 
> > - add machine-type-specific cpudef compatibility changes?
> 
> I think we've discussed this in IRC.  I don't think we need to worry
> about this.

I remember discussing a lot about the mechanism we will use to add the
compatibility changes, but I don t know how the query API will look
like, after we implement this mechanism.


> 
> > Would the command report different results depending on -machine?
> 
> No.

The problem is:

1) We need to introduce fixes on a CPU model that changes the set of
   guest-visible features (add or remove a feature)[1];
2) The fix has to keep compatibility, so older machine-types will
   keep exposing the old set of gues-visible features;
   - That means different machine-types will have different CPU
     features being exposed.
3) libvirt needs to control/know which guest-visible CPU features are
   available to the guest (see above);
4) Because of (2), the querying system used by libvirt need to depend on
   the CPU model and machine-type.


[1] Example:
    The SandyBridge model today has the "tsc-deadline" bit set, but
    QEMU-1.1 did not expose the tsc-deadline feature properly because of
    incorrect expectations about the GET_SUPPORTED_CPUID ioctl. This was
    fixed on qemu-1.2.
    
    That means "qemu-1.1 -machine pc-1.1 -cpu SandyBridge" does _not_
    expose tsc-deadline to the guest, and we need to make "qemu-1.2
    -machine pc-1.1 -cpu SandyBridge" _not_ expose it, too (otherwise
    migration from qemu-1.1 to qemu-1.2 will be broken).

> 
> >
> > Would the command return the latest cpudef without any machine-type
> > hacks, and libvirt would have to query for the cpudef compatibility data
> > for each machine-type and combine both pieces of information itself?
> 
> I'm not sure what you mean by compatibility data.

I mean any guest-visible compatibility bit that we will need to
introduce on older machine-types, when making changes on CPU models (see
the SandyBridge + tsc-deadline example above).

I see two options:
- Libvirt queries for a [f(machine_type, cpu_model) -> cpu_features]
  function, that will take into account the machine-type-specific
  compatibility bits.
- Libvirt queries for a [f(cpu_model) -> cpu_features] function and a
  [f(machine_type) -> compatibility_changes] function, and combine both.
  - I don't like this approach, I am just including it as a possible
    alternative.

> 
> Regards,
> 
> Anthony Liguori
> 
> >
> > [1] Note that it doesn't have to be low-level leaf-by-leaf
> >     register-by-register CPUID bits (I prefer a more high-level
> >     interface, myself), but it has to at least say "feature FOO is
> >     enabled/disabled" for a set of features libvirt cares about.
> >
> > -- 
> > Eduardo
> 

-- 
Eduardo