[libvirt] [RFC 0/5] Allow object-add on X86CPU subclasses, for CPU model probing

This series allows management code to use object-add on X86CPU subclasses, so it 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(-) -- 1.9.0

In case something happens and prevents qemu_init_vcpu() from running after cpu_exec_init() was already called, this will let the rest of the VCPU handling code know that the CPU is not running yet. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- cpus.c | 1 - exec.c | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/cpus.c b/cpus.c index 7bbe153..69b0530 100644 --- a/cpus.c +++ b/cpus.c @@ -1184,7 +1184,6 @@ void qemu_init_vcpu(CPUState *cpu) { cpu->nr_cores = smp_cores; cpu->nr_threads = smp_threads; - cpu->stopped = true; if (kvm_enabled()) { qemu_kvm_start_vcpu(cpu); } else if (tcg_enabled()) { diff --git a/exec.c b/exec.c index 91513c6..e91decc 100644 --- a/exec.c +++ b/exec.c @@ -485,6 +485,7 @@ void cpu_exec_init(CPUArchState *env) } cpu->cpu_index = cpu_index; cpu->numa_node = 0; + cpu->stopped = true; QTAILQ_INIT(&cpu->breakpoints); QTAILQ_INIT(&cpu->watchpoints); #ifndef CONFIG_USER_ONLY -- 1.9.0

In case something happens and prevents qemu_init_vcpu() from running after cpu_exec_init() was already called, this will let the rest of the VCPU handling code know that the CPU is not running yet.
On Wed, 30 Apr 2014 17:29:29 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: perhaps default value should be set even earlier in qom/cpu.c:cpu_common_initfn()
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- cpus.c | 1 - exec.c | 1 + 2 files changed, 1 insertion(+), 1 deletion(-)
diff --git a/cpus.c b/cpus.c index 7bbe153..69b0530 100644 --- a/cpus.c +++ b/cpus.c @@ -1184,7 +1184,6 @@ void qemu_init_vcpu(CPUState *cpu) { cpu->nr_cores = smp_cores; cpu->nr_threads = smp_threads; - cpu->stopped = true; if (kvm_enabled()) { qemu_kvm_start_vcpu(cpu); } else if (tcg_enabled()) { diff --git a/exec.c b/exec.c index 91513c6..e91decc 100644 --- a/exec.c +++ b/exec.c @@ -485,6 +485,7 @@ void cpu_exec_init(CPUArchState *env) } cpu->cpu_index = cpu_index; cpu->numa_node = 0; + cpu->stopped = true; QTAILQ_INIT(&cpu->breakpoints); QTAILQ_INIT(&cpu->watchpoints); #ifndef CONFIG_USER_ONLY

This will make the pause_all_cpus() code not crash in case qemu_init_vcpu() was not called yet for a CPU (so it doesn't have cpu->thread set yet). Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- cpus.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/cpus.c b/cpus.c index 69b0530..ca3862b 100644 --- a/cpus.c +++ b/cpus.c @@ -1070,6 +1070,14 @@ static int all_vcpus_paused(void) return 1; } +static void cpu_kick_if_running(CPUState *cpu) +{ + if (!cpu->stopped) { + assert(cpu->thread); + qemu_cpu_kick(cpu); + } +} + void pause_all_vcpus(void) { CPUState *cpu; @@ -1077,7 +1085,7 @@ void pause_all_vcpus(void) qemu_clock_enable(QEMU_CLOCK_VIRTUAL, false); CPU_FOREACH(cpu) { cpu->stop = true; - qemu_cpu_kick(cpu); + cpu_kick_if_running(cpu); } if (qemu_in_vcpu_thread()) { @@ -1094,7 +1102,7 @@ void pause_all_vcpus(void) while (!all_vcpus_paused()) { qemu_cond_wait(&qemu_pause_cond, &qemu_global_mutex); CPU_FOREACH(cpu) { - qemu_cpu_kick(cpu); + cpu_kick_if_running(cpu); } } } -- 1.9.0

apic_accept_pic_intr() returns -1 if gets NULL as argument. So, instead of checking if apic_accept_pic_intr() returns non-zero, check for return value > 0. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- hw/i386/pc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 14f0d91..c747615 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -197,7 +197,7 @@ static void pic_irq_request(void *opaque, int irq, int level) if (cpu->apic_state) { CPU_FOREACH(cs) { cpu = X86_CPU(cs); - if (apic_accept_pic_intr(cpu->apic_state)) { + if (apic_accept_pic_intr(cpu->apic_state) > 0) { apic_deliver_pic_intr(cpu->apic_state, level); } } -- 1.9.0

Creating X86CPU objects using object-add will allow management code to probe for details on each CPU model when using "-machine none", and without having to restart QEMU. Note that object-add will _not_ create a running VCPU. It is not CPU hotplug, which will use device_add instead of object-add. It will just allow some information about the CPU object to be queried from the CPU object, without starting an actual VCPU thread. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- target-i386/cpu.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 8fd1497..2417fc8 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -43,6 +43,7 @@ #include "sysemu/sysemu.h" #include "hw/qdev-properties.h" #include "hw/cpu/icc_bus.h" +#include "qom/object_interfaces.h" #ifndef CONFIG_USER_ONLY #include "hw/xen/xen.h" #include "hw/i386/apic_internal.h" @@ -2832,6 +2833,10 @@ static const TypeInfo x86_cpu_type_info = { .abstract = true, .class_size = sizeof(X86CPUClass), .class_init = x86_cpu_common_class_init, + .interfaces = (InterfaceInfo[]) { + { TYPE_USER_CREATABLE }, + { } + }, }; static void x86_cpu_register_types(void) -- 1.9.0

The new qom-type attributed will be used to indicate that the object-add QMP command can be used to create CPU objects. This will let management code to create multiple CPU objects from different classes and CPU models and query for their information using QOM properties (like the "feature-words" properties) in a single QEMU instance, instead of having to re-run QEMU once for each CPU model. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- qapi-schema.json | 6 +++++- target-i386/cpu.c | 2 ++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/qapi-schema.json b/qapi-schema.json index 0b00427..7d8bec5 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -3438,11 +3438,15 @@ # Virtual CPU definition. # # @name: the name of the CPU definition +# @qom-type: the name of the QOM class for the CPU model. The presence +# of this attribute indicates that CPU objects can be created +# using object-add. +# (since: 2.1.0) # # Since: 1.2.0 ## { 'type': 'CpuDefinitionInfo', - 'data': { 'name': 'str' } } + 'data': { 'name': 'str', '*qom-type': 'str' } } ## # @query-cpu-definitions: diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 2417fc8..897caa4 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1828,6 +1828,8 @@ CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp) def = &builtin_x86_defs[i]; info = g_malloc0(sizeof(*info)); info->name = g_strdup(def->name); + info->has_qom_type = true; + info->qom_type = x86_cpu_type_name(def->name); entry = g_malloc0(sizeof(*entry)); entry->value = info; -- 1.9.0

On Wed, 30 Apr 2014 17:29:28 -0300 Eduardo Habkost <ehabkost@redhat.com> 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?
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(-)

On Fri, May 02, 2014 at 03:45:03PM +0200, Igor Mammedov wrote:
On Wed, 30 Apr 2014 17:29:28 -0300 Eduardo Habkost <ehabkost@redhat.com> 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 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

Il 02/05/2014 16:43, Eduardo Habkost ha scritto:
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).
I like this series in general. I have only some doubts about making the code somewhat future-proof, hence the three questions I have are really variations of this same doubt: * is it worthwhile to extend this to other devices, for management to query default values of the properties? * how does this interact with future QOMification of device hotplug where devices will be hotplugged with object-add? Should Device::UserCreatable::complete set realized to true in this case in the future? How will Device::UserCreatable::complete distinguish the two cases? * Related to this, if Device::UserCreatable::complete will set realized to true, how will we handle hotplug of interconnected devices where device 1 needs a link to device 2 and device 2 needs a link to device 1? Paolo

On Fri, May 02, 2014 at 04:54:00PM +0200, Paolo Bonzini wrote:
Il 02/05/2014 16:43, Eduardo Habkost ha scritto:
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).
I like this series in general. I have only some doubts about making the code somewhat future-proof, hence the three questions I have are really variations of this same doubt:
* is it worthwhile to extend this to other devices, for management to query default values of the properties?
Well, it was extended automatically to all devices for a few days on qemu.git, before TYPE_USER_CREATABLE was introduced. In practice many devices don't like being created without a bus, and/or outside the usual qdev hierarchy, and/or without getting realized=true set, so bad things could happen. Isn't that the reason TYPE_USER_CREATABLE was created? About default values of properties: if all you need is the default value of properties, that data is already present at class_init-time. We could allow class introspection to return that data without creating the objects. It would make sense to me, but I am not sure this would get accepted. See: http://marc.info/?l=qemu-devel&m=139170587709459 In the case of X86CPUs, all attempts to make the data introspectable at compile-time or class_init-time (without requiring CPU instances to be created) didn't work or were rejected in favour of calculating CPUID data at runtime. We are still slowly changing the X86CPU code in a way that would make that data instrospectable without creating the actual objects, but it may take a very long time, and we may never reach that goal.
* how does this interact with future QOMification of device hotplug where devices will be hotplugged with object-add? Should Device::UserCreatable::complete set realized to true in this case in the future? How will Device::UserCreatable::complete distinguish the two cases?
I don't know the answer to that. In my specific use case, I don't care if realized=true is set automatically in the future, as long as QEMU doesn't crash. At least in the case of x86 CPUs, it makes sense to set realized=true only after the CPU is connected to an icc-bus. So, I believe it makes sense to set realized=true only if/when the object is connected/linked to a bus/device that triggers realization, or explicitly using qom-set.
* Related to this, if Device::UserCreatable::complete will set realized to true, how will we handle hotplug of interconnected devices where device 1 needs a link to device 2 and device 2 needs a link to device 1?
How do we handle hotplug of interconnected devices today? -- Eduardo

On Fri, 2 May 2014 11:43:05 -0300 Eduardo Habkost <ehabkost@redhat.com> 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 <ehabkost@redhat.com> 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.
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(-)

On Tue, May 06, 2014 at 09:22:38AM +0200, Igor Mammedov wrote:
On Fri, 2 May 2014 11:43:05 -0300 Eduardo Habkost <ehabkost@redhat.com> 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 <ehabkost@redhat.com> 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). 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. 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). 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.
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

On Tue, May 06, 2014 at 09:22:38AM +0200, Igor Mammedov wrote:
On Fri, 2 May 2014 11:43:05 -0300 Eduardo Habkost <ehabkost@redhat.com> 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 <ehabkost@redhat.com> 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).
On Tue, 6 May 2014 11:42:56 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: 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.
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.
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.
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.
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

On Tue, May 06, 2014 at 10:01:11PM +0200, Igor Mammedov wrote:
On Tue, 6 May 2014 11:42:56 -0300 Eduardo Habkost <ehabkost@redhat.com> 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 <ehabkost@redhat.com> 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 <ehabkost@redhat.com> 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".
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.
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?
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
-- Eduardo

Am 06.05.2014 22:19, schrieb Eduardo Habkost:
On Tue, May 06, 2014 at 10:01:11PM +0200, Igor Mammedov 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 <ehabkost@redhat.com> 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 <ehabkost@redhat.com> 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).
On Tue, 6 May 2014 11:42:56 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: 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".
I see no reason why we couldn't *make* CPUs work on -machine none. The ICC bus and bridge were a hack to make APIC(?) hot-add work in face of SysBus; if that prohibits other valid uses now, then evaluating Igor's memory work for CPU might be an option. I'm not aware of any real X86CPU dependency on ICCBus, so we might as well make -device place it on SysBus if no ICCBus is available... probably much more invasive and potentially dangerous though. Regards, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

On Tue, May 06, 2014 at 10:29:24PM +0200, Andreas Färber wrote:
Am 06.05.2014 22:19, schrieb Eduardo Habkost:
On Tue, May 06, 2014 at 10:01:11PM +0200, Igor Mammedov 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 <ehabkost@redhat.com> 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 <ehabkost@redhat.com> 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).
On Tue, 6 May 2014 11:42:56 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: 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".
I see no reason why we couldn't *make* CPUs work on -machine none. The ICC bus and bridge were a hack to make APIC(?) hot-add work in face of SysBus; if that prohibits other valid uses now, then evaluating Igor's memory work for CPU might be an option.
I'm not aware of any real X86CPU dependency on ICCBus, so we might as well make -device place it on SysBus if no ICCBus is available... probably much more invasive and potentially dangerous though.
Doing that may be an option, too. But isn't the X86CPU bus type and object hierarchy part of the API? Anyway, I thought we were moving away from QDEV, and trying to use a purely QOM-based model where possible. So I don't see why making object-add working with X86CPU would be a bad thing, considering that the alternative requires redoing the X86CPU bus/device hierarchy again, just to work around device_add limitations. My other worry is if X86CPU/icc-bus hierarchy rework would be done in time for 2.1 (and not delay other work). The lack of proper host capability probing in the libvirt+QEMU stack is a bug, not just a nice new feature. -- Eduardo

On Tue, 06 May 2014 22:29:24 +0200 Andreas Färber <afaerber@suse.de> wrote:
Am 06.05.2014 22:19, schrieb Eduardo Habkost:
On Tue, May 06, 2014 at 10:01:11PM +0200, Igor Mammedov 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 <ehabkost@redhat.com> 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 <ehabkost@redhat.com> 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).
On Tue, 6 May 2014 11:42:56 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: 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".
I see no reason why we couldn't *make* CPUs work on -machine none. The ICC bus and bridge were a hack to make APIC(?) hot-add work in face of SysBus; if that prohibits other valid uses now, then evaluating Igor's memory work for CPU might be an option. Yep, if CPU is hot-plugged as bus-less device. There is a little concern of APIC device if we go that direction since in addition to hotpluggable BUS, BUS provides address-space for APIC MMIO. With that resolved, x86-cpu shouldn't depend on any bus and if there isn't any current user that uses QOM path to CPU for introspecting (Eduardo's ABI concern), then it could be done in time for 2.1.
PS: As side effect cpu/apic will disappear from "info qtree" HMP command output.
I'm not aware of any real X86CPU dependency on ICCBus, so we might as well make -device place it on SysBus if no ICCBus is available... probably much more invasive and potentially dangerous though.
Regards, Andreas

On Thu, May 15, 2014 at 02:35:01PM +0200, Igor Mammedov wrote:
On Tue, 06 May 2014 22:29:24 +0200 Andreas Färber <afaerber@suse.de> wrote:
Am 06.05.2014 22:19, schrieb Eduardo Habkost:
On Tue, May 06, 2014 at 10:01:11PM +0200, Igor Mammedov 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 <ehabkost@redhat.com> 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 <ehabkost@redhat.com> 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).
On Tue, 6 May 2014 11:42:56 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: 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".
I see no reason why we couldn't *make* CPUs work on -machine none. The ICC bus and bridge were a hack to make APIC(?) hot-add work in face of SysBus; if that prohibits other valid uses now, then evaluating Igor's memory work for CPU might be an option. Yep, if CPU is hot-plugged as bus-less device. There is a little concern of APIC device if we go that direction since in addition to hotpluggable BUS, BUS provides address-space for APIC MMIO. With that resolved, x86-cpu shouldn't depend on any bus and if there isn't any current user that uses QOM path to CPU for introspecting (Eduardo's ABI concern), then it could be done in time for 2.1.
Maybe there are no users of the current QOM path, but we do need a stable path to allow management to locate the CPU objects. Do we have one, already? -- Eduardo

Am 15.05.2014 15:07, schrieb Eduardo Habkost:
On Thu, May 15, 2014 at 02:35:01PM +0200, Igor Mammedov wrote:
On Tue, 06 May 2014 22:29:24 +0200 Andreas Färber <afaerber@suse.de> wrote:
Am 06.05.2014 22:19, schrieb Eduardo Habkost:
On Tue, May 06, 2014 at 10:01:11PM +0200, Igor Mammedov 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 <ehabkost@redhat.com> 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 <ehabkost@redhat.com> 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).
On Tue, 6 May 2014 11:42:56 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: 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".
I see no reason why we couldn't *make* CPUs work on -machine none. The ICC bus and bridge were a hack to make APIC(?) hot-add work in face of SysBus; if that prohibits other valid uses now, then evaluating Igor's memory work for CPU might be an option. Yep, if CPU is hot-plugged as bus-less device. There is a little concern of APIC device if we go that direction since in addition to hotpluggable BUS, BUS provides address-space for APIC MMIO. With that resolved, x86-cpu shouldn't depend on any bus and if there isn't any current user that uses QOM path to CPU for introspecting (Eduardo's ABI concern), then it could be done in time for 2.1.
Maybe there are no users of the current QOM path, but we do need a stable path to allow management to locate the CPU objects. Do we have one, already?
No, we don't. That question is intertwined with topology modeling. :/ Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

On Thu, 15 May 2014 10:07:51 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote:
On Thu, May 15, 2014 at 02:35:01PM +0200, Igor Mammedov wrote:
On Tue, 06 May 2014 22:29:24 +0200 Andreas Färber <afaerber@suse.de> wrote:
Am 06.05.2014 22:19, schrieb Eduardo Habkost:
On Tue, May 06, 2014 at 10:01:11PM +0200, Igor Mammedov 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 <ehabkost@redhat.com> 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 <ehabkost@redhat.com> 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).
On Tue, 6 May 2014 11:42:56 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: 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".
I see no reason why we couldn't *make* CPUs work on -machine none. The ICC bus and bridge were a hack to make APIC(?) hot-add work in face of SysBus; if that prohibits other valid uses now, then evaluating Igor's memory work for CPU might be an option. Yep, if CPU is hot-plugged as bus-less device. There is a little concern of APIC device if we go that direction since in addition to hotpluggable BUS, BUS provides address-space for APIC MMIO. With that resolved, x86-cpu shouldn't depend on any bus and if there isn't any current user that uses QOM path to CPU for introspecting (Eduardo's ABI concern), then it could be done in time for 2.1.
Maybe there are no users of the current QOM path, but we do need a stable path to allow management to locate the CPU objects. Do we have one, already?
Can't we add query-cpus QMP command or something like this to hide path from user.

On Thu, May 15, 2014 at 03:48:16PM +0200, Igor Mammedov wrote:
On Thu, 15 May 2014 10:07:51 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote:
On Thu, May 15, 2014 at 02:35:01PM +0200, Igor Mammedov wrote:
On Tue, 06 May 2014 22:29:24 +0200 Andreas Färber <afaerber@suse.de> wrote:
Am 06.05.2014 22:19, schrieb Eduardo Habkost:
On Tue, May 06, 2014 at 10:01:11PM +0200, Igor Mammedov wrote:
On Tue, 6 May 2014 11:42:56 -0300 Eduardo Habkost <ehabkost@redhat.com> 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 <ehabkost@redhat.com> 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 <ehabkost@redhat.com> 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".
I see no reason why we couldn't *make* CPUs work on -machine none. The ICC bus and bridge were a hack to make APIC(?) hot-add work in face of SysBus; if that prohibits other valid uses now, then evaluating Igor's memory work for CPU might be an option. Yep, if CPU is hot-plugged as bus-less device. There is a little concern of APIC device if we go that direction since in addition to hotpluggable BUS, BUS provides address-space for APIC MMIO. With that resolved, x86-cpu shouldn't depend on any bus and if there isn't any current user that uses QOM path to CPU for introspecting (Eduardo's ABI concern), then it could be done in time for 2.1.
Maybe there are no users of the current QOM path, but we do need a stable path to allow management to locate the CPU objects. Do we have one, already?
Can't we add query-cpus QMP command or something like this to hide path from user.
That would work, too. But why is a dedicated "query-cpus" command better than something like "qom-list path=/machine/cpus" (that could simply return a list of links to the actual CPU objects)? -- Eduardo

On Thu, 15 May 2014 11:03:49 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: > On Thu, May 15, 2014 at 03:48:16PM +0200, Igor Mammedov wrote: > > On Thu, 15 May 2014 10:07:51 -0300 > > Eduardo Habkost <ehabkost@redhat.com> wrote: > > > > > On Thu, May 15, 2014 at 02:35:01PM +0200, Igor Mammedov wrote: > > > > On Tue, 06 May 2014 22:29:24 +0200 > > > > Andreas Färber <afaerber@suse.de> wrote: > > > > > > > > > Am 06.05.2014 22:19, schrieb Eduardo Habkost: > > > > > > On Tue, May 06, 2014 at 10:01:11PM +0200, Igor Mammedov wrote: > > > > > >> On Tue, 6 May 2014 11:42:56 -0300 > > > > > >> Eduardo Habkost <ehabkost@redhat.com> 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 <ehabkost@redhat.com> 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 <ehabkost@redhat.com> 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". > > > > > > > > > > I see no reason why we couldn't *make* CPUs work on -machine none. The > > > > > ICC bus and bridge were a hack to make APIC(?) hot-add work in face of > > > > > SysBus; if that prohibits other valid uses now, then evaluating Igor's > > > > > memory work for CPU might be an option. > > > > Yep, if CPU is hot-plugged as bus-less device. > > > > There is a little concern of APIC device if we go that direction since > > > > in addition to hotpluggable BUS, BUS provides address-space for APIC MMIO. > > > > With that resolved, x86-cpu shouldn't depend on any bus and if there isn't > > > > any current user that uses QOM path to CPU for introspecting (Eduardo's > > > > ABI concern), then it could be done in time for 2.1. > > > > > > Maybe there are no users of the current QOM path, but we do need a > > > stable path to allow management to locate the CPU objects. Do we have > > > one, already? > > > > > > > Can't we add query-cpus QMP command or something like this to hide path > > from user. > > That would work, too. But why is a dedicated "query-cpus" command better > than something like "qom-list path=/machine/cpus" (that could simply > return a list of links to the actual CPU objects)? So that not to stall the work on deciding on - if exposing not yet stables QOM paths as stable ABI is a good thing, I recall Andreas objecting to using QOM paths with device hotplug - what paths to CPUs should be wrt stalled topology discussion > > -- > Eduardo > -- Regards, Igor

On Fri, May 16, 2014 at 04:52:21PM +0200, Igor Mammedov wrote: > On Thu, 15 May 2014 11:03:49 -0300 > Eduardo Habkost <ehabkost@redhat.com> wrote: > > On Thu, May 15, 2014 at 03:48:16PM +0200, Igor Mammedov wrote: [...] > > > > > > Can't we add query-cpus QMP command or something like this to hide path > > > from user. > > > > That would work, too. But why is a dedicated "query-cpus" command better > > than something like "qom-list path=/machine/cpus" (that could simply > > return a list of links to the actual CPU objects)? > So that not to stall the work on deciding on > - if exposing not yet stables QOM paths as stable ABI is a good thing, I > recall Andreas objecting to using QOM paths with device hotplug This surprises me. > - what paths to CPUs should be wrt stalled topology discussion > I don't see why it depends on the topology discussion: if we are capable of keeping "query-cpus" working after we introduce the topology hierarchy, I believe we are perfectly capable of keeping symlinks on "/machine/cpus" working, too. Even if we change the actual paths later and introduce a more complex QOM hierarchy somewhere else. Isn't the reuse of infrastructure for list/get/set operations the whole point of QOM? -- Eduardo

Am 15.05.2014 14:35, schrieb Igor Mammedov:
PS: As side effect cpu/apic will disappear from "info qtree" HMP command output.
Solutions are already on the list and in need of feedback: http://patchwork.ozlabs.org/patch/317224/ http://patchwork.ozlabs.org/patch/343136/ http://patchwork.ozlabs.org/patch/347064/ Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

On 05/06/2014 02:19 PM, Eduardo Habkost wrote:
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.
Therein is the reason libvirt isn't using what qemu already has - spawning multiple qemu instances (one per reported machine type) multiplied by the number of qemu binaries does not scale well, when compared to starting a single qemu -machine none, and doing all queries on that one machine. So the point of this patch is getting us closer to the point where libvirt can learn accurate cpu model information for multiple machines all from a single qemu invocation. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Tue, 6 May 2014 17:19:57 -0300 Eduardo Habkost <ehabkost@redhat.com> 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 <ehabkost@redhat.com> 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 <ehabkost@redhat.com> 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 <ehabkost@redhat.com> 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

On Thu, May 15, 2014 at 02:14:18PM +0200, Igor Mammedov wrote:
On Tue, 6 May 2014 17:19:57 -0300 Eduardo Habkost <ehabkost@redhat.com> 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 <ehabkost@redhat.com> 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 <ehabkost@redhat.com> 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 <ehabkost@redhat.com> 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 ??
I guess the explanation for "-machine none" is at: commit b4a738bf93c3137b92d532e59d60edccc4e1ea96 Author: Anthony Liguori <aliguori@us.ibm.com> Date: Wed Aug 22 15:22:05 2012 -0500 boards: add a 'none' machine type to all platforms This allows any QEMU binary to be executed with: $QEMU_BINARY -M none -qmp stdio Without errors from missing options that are required by various boards. This also provides a mode that we can use in the future to construct machines entirely through QMP commands. Cc: Daniel Berrange <berrange@redhat.com> Cc: Markus Armbruster <armbru@redhat.com> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> libvirt runs QEMU with "-machine none" before it knows anything about the QEMU binaries. There are many target architectures where the default machine-type won't work without extra options. 15 of the 26 qemu-system-* binaries on my Fedora 20 system failed to run as: $QEMU -nodefaults -no-user-config -nographic -monitor stdio (All of them ran happily when I added "-M none".)
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.
But not with "-machine none". -- Eduardo
participants (5)
-
Andreas Färber
-
Eduardo Habkost
-
Eric Blake
-
Igor Mammedov
-
Paolo Bonzini