Re: [Qemu-devel] [RFC 0/5] Allow object-add on X86CPU subclasses, for CPU model probing

[Igor Mammedov]

On Tue, 6 May 2014 17:19:57 -0300
Eduardo Habkost <address@hidden> wrote:

> On Tue, May 06, 2014 at 10:01:11PM +0200, Igor Mammedov wrote:
> > On Tue, 6 May 2014 11:42:56 -0300
> > Eduardo Habkost <address@hidden> wrote:
> > 
> > > On Tue, May 06, 2014 at 09:22:38AM +0200, Igor Mammedov wrote:
> > > > On Fri, 2 May 2014 11:43:05 -0300
> > > > Eduardo Habkost <address@hidden> wrote:
> > > > 
> > > > > On Fri, May 02, 2014 at 03:45:03PM +0200, Igor Mammedov wrote:
> > > > > > On Wed, 30 Apr 2014 17:29:28 -0300
> > > > > > Eduardo Habkost <address@hidden> wrote:
> > > > > > 
> > > > > > > This series allows management code to use object-add on X86CPU 
> > > > > > > subclasses, so it
> > > > > > Is there any reason why "device-add" couldn't be used?
> > > > > 
> > > > > It needs to work with "-machine none", device_add requires a bus to
> > > > > exist, and there is no icc-bus on machine_none.
> > > > The thing is that CPUID is a function of machine so using
> > > > "-machine none" will provide only approximately accurate data.
> > > > I'm not sure that retrieved possibly not accurate data are useful
> > > > for libvirt.
> > > 
> > > "-cpu host" doesn't depend on machine, and is the most important thing
> > > right now (because libvirt's checks for host QEMU/kernel/CPU
> > > capabilities is completely broken).
> > true, but device-add/-cpu host could be used right now to get the
> > same CPUID data wile using any machine type or default one without
> > any of this patches.
> 
> device_add can't be used with "-machine none".
Well, does caller need to use -machine in this case at all?
Why not call QEMU with default machine type and get the same info
using device_add ??

> 
> > 
> > > 
> > > About machine-type data: currently libvirt has no concept of
> > > per-machine-type CPU model data, anyway. We (and libvirt) will need to
> > > address this eventually, but considering our track record, I bet the
> > > QEMU community will take a few years to finally provide that info to
> > > libvirt.
> > I don't think QEMU is issue here, libvirt can use device-add to
> > probe accurate CPUID on all CPU models on a given machine type now.
> > libvirt should be fixed to care about machine type and use it to get
> > correct CPUID data that QEMU provides.
> 
> True, it should. But we still need a solution for the "-cpu host" problem.
As you've said before '-cpu host' doesn't depend on machine type so 
libvirt could use "device_add 'host-cpu'" with default or any other
PC machine type right now.

> 
> > 
> > > 
> > > In the meantime, we have a partial solution that fits the current
> > > libvirt data model and is better than the current state (where libvirt
> > > has to duplicate the CPU model data).
> > Replacing one set of inaccurate CPUIDs with another is hardly solution.
> 
> We could continue arguing about this, but let's ignore the issue about
> probing for CPU model information by now, and let's focus on host
> capability probing ("-cpu host"), then.
> 
> How do you propose fixing that in a reasonable time (in QEMU 2.1)
> without requiring libvirt to re-run QEMU?
Wouldn't "-cpu host" or alternative device_add command with default machine
be sufficient?

> 
> 
> > 
> > > 
> > > Maybe they will use this only for "host", maybe they will use this for
> > > all the other CPU models, we are just providing the mechanism, it's
> > > their decision to use it or not.
> > As I've said above libvirt could use device-add xxx-host or -cpu host
> > to get it and second point I object to is providing yet another way
> > to produce inaccurate CPUID info (libvirt has one already) and to do
> > so hack [patches 1-3] CPU infrastructure to run out of context it's
> > supposed to run in. While current API already allows to get correct
> > CPUID data.
> > 
> > IMHO, libvirt side should take advantage of information QEMU already
> > provides.
> > 
> 
> Current API requires re-running QEMU to query the information. This
> series allows it to be run with the "-machine none" QEMU instance that
> is already run by libvirt.
> 
> 
> > > 
> > > 
> > > > 
> > > > > 
> > > > > The first thing I considered was making icc-bus user-creatable. Then I
> > > > > noticed it wouldn't work because object-add always add objects to
> > > > > /objects, not inside the qdev hierarchy (that's where device_add looks
> > > > > for the bus).
> > > > > 
> > > > > So, allowing device_add could be possible, but would require changing
> > > > > more basic infrastructure: either allowing bus-less devices on
> > > > > device_add, or allowing device_add to add devices outside the qdev
> > > > > hierarchy, or allowing object-add to create objects outside /objects.
> > > > > 
> > > > > Simply making CPU objects work with object-add was much simpler and 
> > > > > less
> > > > > intrusive. And it had the interesting side-effect of _not_ doing 
> > > > > things
> > > > > that are not required for CPU model probing (like creating an actual
> > > > > VCPU thread).
> > > > > 
> > > > > > 
> > > > > > 
> > > > > > > can use it to probe for CPU model information without re-running 
> > > > > > > QEMU. The main
> > > > > > > use case for this is to allow management code to create CPU 
> > > > > > > objects and query
> > > > > > > the "feature-words" and "filtered-features" properties on the new 
> > > > > > > objects, to
> > > > > > > find out which features each CPU model needs, and to do the same 
> > > > > > > using the
> > > > > > > "host" CPU model to check which features can be enabled in a 
> > > > > > > given host.
> > > > > > > 
> > > > > > > There's experimental libvirt code to use the new command at:
> > > > > > >     
> > > > > > > https://github.com/ehabkost/libvirt/tree/work/cpu-feature-word-query
> > > > > > > The experimental code just create the CPU objects to query for 
> > > > > > > feature
> > > > > > > information, but doesn't do anything with that data.
> > > > > > > 
> > > > > > > Eduardo Habkost (5):
> > > > > > >   cpu: Initialize cpu->stopped=true earlier
> > > > > > >   cpu: Don't try to pause CPUs if they are already stopped
> > > > > > >   pc: Don't crash on apic_accept_pic_intr() if CPU has no 
> > > > > > > apic_state
> > > > > > >   target-i386: Make CPU objects user-creatable
> > > > > > >   target-i386: Report QOM class name for CPU definitions
> > > > > > > 
> > > > > > >  cpus.c            | 13 ++++++++++---
> > > > > > >  exec.c            |  1 +
> > > > > > >  hw/i386/pc.c      |  2 +-
> > > > > > >  qapi-schema.json  |  6 +++++-
> > > > > > >  target-i386/cpu.c |  7 +++++++
> > > > > > >  5 files changed, 24 insertions(+), 5 deletions(-)
> > > > > > > 
> > > > > > 
> > > > > 
> > > > 
> > > 
> > > -- 
> > > Eduardo
> > > 
> > 
> > 
> > -- 
> > Regards,
> >   Igor
>