[libvirt] [PATCH 0/3] qmp: query-host-cpu command
Add QMP command to allow management software to query for CPU information for the running host. The data returned by the command is in the form of a dictionary of QOM properties. This series depends on the "Add runnability info to query-cpu-definitions" series I sent 2 weeks ago. Git tree: https://github.com/ehabkost/qemu-hacks.git work/query-host-cpu Example output for x86: { "qom-properties": { "pfthreshold": false, "pku": false, "rtm": false, "tsc-deadline": true, "xstore-en": false, "cpuid-0xb": true, "abm": true, "ia64": false, "kvm-mmu": false, "xsaveopt": true, "hv-spinlocks": -1, "hotpluggable": true, "tce": false, "smep": true, "nx": true, "xcrypt": false, "sse4_2": true, "clflush": true, "sse4_1": true, "flushbyasid": false, "kvm-steal-time": true, "host-cache-info": false, "tsc": true, "adx": false, "ds": false, "fxsr": true, "tm": false, "hv-relaxed": false, "pclmuldq": true, "xgetbv1": false, "xstore": false, "migratable": true, "vme": true, "vendor": "GenuineIntel", "arat": true, "ffxsr": false, "de": true, "aes": true, "pse": true, "ds-cpl": false, "tbm": false, "sse": true, "phe-en": false, "f16c": true, "hotplugged": true, "mpx": false, "tsc-adjust": true, "avx512f": false, "avx2": true, "misalignsse": false, "level": 13, "pbe": false, "cx16": true, "avx512pf": false, "movbe": true, "perfctr-nb": false, "enforce": false, "ospke": false, "pmu": true, "fma4": false, "stepping": 1, "feature-words": [ { "cpuid-register": "EAX", "cpuid-input-eax": 6, "features": 4 }, { "cpuid-register": "EAX", "cpuid-input-eax": 13, "features": 1, "cpuid-input-ecx": 1 }, { "cpuid-register": "EDX", "cpuid-input-eax": 2147483658, "features": 0 }, { "cpuid-register": "EAX", "cpuid-input-eax": 1073741825, "features": 16777467 }, { "cpuid-register": "EDX", "cpuid-input-eax": 3221225473, "features": 0 }, { "cpuid-register": "EDX", "cpuid-input-eax": 2147483655, "features": 0 }, { "cpuid-register": "ECX", "cpuid-input-eax": 2147483649, "features": 33 }, { "cpuid-register": "EDX", "cpuid-input-eax": 2147483649, "features": 739248128 }, { "cpuid-register": "ECX", "cpuid-input-eax": 7, "features": 0, "cpuid-input-ecx": 0 }, { "cpuid-register": "EBX", "cpuid-input-eax": 7, "features": 1963, "cpuid-input-ecx": 0 }, { "cpuid-register": "ECX", "cpuid-input-eax": 1, "features": 4160369155 }, { "cpuid-register": "EDX", "cpuid-input-eax": 1, "features": 260832255 } ], "sse4a": false, "i64": true, "xsave": true, "hv-runtime": false, "pmm": false, "hle": false, "hv-crash": false, "est": false, "xop": false, "smx": false, "tsc-scale": false, "monitor": false, "avx512er": false, "apic": true, "sse4.1": true, "sse4.2": true, "hv-vapic": false, "pause-filter": false, "lahf-lm": true, "kvm-nopiodelay": true, "acpi": false, "mmx": true, "osxsave": false, "pcommit": false, "clwb": false, "dca": false, "pdcm": false, "xcrypt-en": false, "3dnow": false, "invtsc": false, "tm2": false, "hv-time": false, "hypervisor": true, "kvmclock-stable-bit": true, "xlevel": 2147483656, "fxsr-opt": false, "pcid": true, "lbrv": false, "wdt": false, "svm-lock": false, "popcnt": true, "nrip-save": false, "x2apic": true, "kvmclock": true, "smap": false, "pdpe1gb": true, "family": 6, "xlevel2": 0, "dtes64": false, "xd": true, "ace2": false, "xtpr": false, "lwp": false, "sep": true, "msr": true, "syscall": true, "decodeassists": false, "perfctr-core": false, "pge": true, "pn": false, "fma": true, "nodeid-msr": false, "hv-vpindex": false, "cx8": true, "mce": true, "avx512cd": false, "cr8legacy": false, "mca": true, "pni": true, "hv-vendor-id": "", "rdseed": false, "osvw": false, "fsgsbase": true, "model-id": "Intel(R) Core(TM) i7-4600U CPU @ 2.10GHz", "cmp-legacy": false, "kvm-pv-unhalt": true, "rdtscp": true, "mmxext": false, "cid": false, "type": "host-x86_64-cpu", "vmx": false, "ssse3": true, "extapic": false, "pse36": true, "mtrr": true, "ibs": false, "avx": true, "ace2-en": false, "invpcid": true, "bmi2": true, "vmcb-clean": false, "erms": true, "cmov": true, "check": true, "parent_bus": "", "clflushopt": false, "pat": true, "hv-synic": false, "hv-stimer": false, "3dnowprefetch": false, "fpu": true, "pae": true, "hv-reset": false, "skinit": false, "kvm": true, "pmm-en": false, "phe": false, "3dnowext": false, "ht": false, "tsc-frequency": 0, "kvm-pv-eoi": true, "npt": false, "apic-id": -1, "xsavec": false, "lm": true, "pclmulqdq": true, "svm": false, "sse3": true, "sse2": true, "ss": true, "topoext": false, "rdrand": true, "bmi1": true, "kvm-asyncpf": true, "xsaves": false, "model": 69 } } Eduardo Habkost (3): qmp: Add query-host-cpu command target-i386: Introduce x86_cpu_load_host_data() function target-i386: Implement arch_query_host_cpu_info() include/sysemu/arch_init.h | 1 + qapi-schema.json | 36 +++++++++++++++++ qmp-commands.hx | 6 +++ qmp.c | 13 ++++++ stubs/Makefile.objs | 1 + stubs/arch-query-host-cpu-info.c | 8 ++++ target-i386/cpu.c | 87 ++++++++++++++++++++++++++++++++++++---- 7 files changed, 145 insertions(+), 7 deletions(-) create mode 100644 stubs/arch-query-host-cpu-info.c -- 2.5.5
The command can be used to return host-specific CPU capabilities information. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- include/sysemu/arch_init.h | 1 + qapi-schema.json | 36 ++++++++++++++++++++++++++++++++++++ qmp-commands.hx | 6 ++++++ qmp.c | 13 +++++++++++++ stubs/Makefile.objs | 1 + stubs/arch-query-host-cpu-info.c | 8 ++++++++ 6 files changed, 65 insertions(+) create mode 100644 stubs/arch-query-host-cpu-info.c diff --git a/include/sysemu/arch_init.h b/include/sysemu/arch_init.h index d690dfa..54215ab 100644 --- a/include/sysemu/arch_init.h +++ b/include/sysemu/arch_init.h @@ -35,5 +35,6 @@ int kvm_available(void); int xen_available(void); CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp); +void arch_query_host_cpu_info(HostCPUInfo *r, bool migratable, Error **errp); #endif diff --git a/qapi-schema.json b/qapi-schema.json index 19e3ef2..d2f4879 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -3047,6 +3047,42 @@ ## { 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'] } + +## +# @HostCPUInfo: +# +# Information on CPU capabilities supported by the current host. +# +# @qom-properties: #optional Values of CPU QOM properties corresponding +# to CPU capabilities supported by the host. +# +# Most properties returned in qom-properties are boolean properties +# indicating if a feature can be enabled in the current host. Other +# non-boolean properties may be returned, the semantics of each property +# depend on the architecture-specific code that handle them. +# +# Since: 2.7.0 +## +{ 'struct': 'HostCPUInfo', + 'data': { '*qom-properties': 'any' } } + +## +# @query-host-cpu: +# +# @migratable: #optional If false, unmigratable features will be +# returned as well. If true, only migratable features +# will be returned. Defaults to true. +# +# Return information about CPU capabilities in the current host. +# The returned data may depend on machine and accelerator configuration. +# +# Returns: A HostCPUInfo object. +# +# Since: 2.7.0 +## +{ 'command': 'query-host-cpu', 'data': { '*migratable': 'bool' }, + 'returns': 'HostCPUInfo' } + # @AddfdInfo: # # Information about a file descriptor that was added to an fd set. diff --git a/qmp-commands.hx b/qmp-commands.hx index b444c20..d4c2ccd 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -3930,6 +3930,12 @@ EQMP }, { + .name = "query-host-cpu", + .args_type = "", + .mhandler.cmd_new = qmp_marshal_query_host_cpu, + }, + + { .name = "query-target", .args_type = "", .mhandler.cmd_new = qmp_marshal_query_target, diff --git a/qmp.c b/qmp.c index 7df6543..aec24d5 100644 --- a/qmp.c +++ b/qmp.c @@ -607,6 +607,19 @@ CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp) return arch_query_cpu_definitions(errp); } +HostCPUInfo *qmp_query_host_cpu(bool has_migratable, bool migratable, + Error **errp) +{ + HostCPUInfo *r = g_new0(HostCPUInfo, 1); + + if (!has_migratable) { + migratable = true; + } + + arch_query_host_cpu_info(r, migratable, errp); + return r; +} + void qmp_add_client(const char *protocol, const char *fdname, bool has_skipauth, bool skipauth, bool has_tls, bool tls, Error **errp) diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs index 4b258a6..eae0e89 100644 --- a/stubs/Makefile.objs +++ b/stubs/Makefile.objs @@ -1,4 +1,5 @@ stub-obj-y += arch-query-cpu-def.o +stub-obj-y += arch-query-host-cpu-info.o stub-obj-y += bdrv-next-monitor-owned.o stub-obj-y += blk-commit-all.o stub-obj-y += blockdev-close-all-bdrv-states.o diff --git a/stubs/arch-query-host-cpu-info.c b/stubs/arch-query-host-cpu-info.c new file mode 100644 index 0000000..b0c455c --- /dev/null +++ b/stubs/arch-query-host-cpu-info.c @@ -0,0 +1,8 @@ +#include "qemu/osdep.h" +#include "qemu-common.h" +#include "sysemu/arch_init.h" +#include "qapi/qmp/qerror.h" + +void arch_query_host_cpu_info(HostCPUInfo *r, bool migratable, Error **errp) +{ +} -- 2.5.5
On 06/20/2016 02:12 PM, Eduardo Habkost wrote:
The command can be used to return host-specific CPU capabilities information.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- include/sysemu/arch_init.h | 1 + qapi-schema.json | 36 ++++++++++++++++++++++++++++++++++++ qmp-commands.hx | 6 ++++++ qmp.c | 13 +++++++++++++ stubs/Makefile.objs | 1 + stubs/arch-query-host-cpu-info.c | 8 ++++++++ 6 files changed, 65 insertions(+) create mode 100644 stubs/arch-query-host-cpu-info.c
diff --git a/include/sysemu/arch_init.h b/include/sysemu/arch_init.h index d690dfa..54215ab 100644 --- a/include/sysemu/arch_init.h +++ b/include/sysemu/arch_init.h @@ -35,5 +35,6 @@ int kvm_available(void); int xen_available(void);
CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp); +void arch_query_host_cpu_info(HostCPUInfo *r, bool migratable, Error **errp);
#endif diff --git a/qapi-schema.json b/qapi-schema.json index 19e3ef2..d2f4879 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -3047,6 +3047,42 @@ ## { 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'] }
+ +## +# @HostCPUInfo: +# +# Information on CPU capabilities supported by the current host. +# +# @qom-properties: #optional Values of CPU QOM properties corresponding +# to CPU capabilities supported by the host. +# +# Most properties returned in qom-properties are boolean properties +# indicating if a feature can be enabled in the current host. Other +# non-boolean properties may be returned, the semantics of each property +# depend on the architecture-specific code that handle them. +# +# Since: 2.7.0
Most places in .json files list just 'Since: x.y' rather than 'x.y.z', but we aren't consistent enough to insist either way on including or excluding a micro release number.
+## +{ 'struct': 'HostCPUInfo', + 'data': { '*qom-properties': 'any' } }
This is a big hammer that makes the properties non-introspectible - a client can tell that properties will be returned, but cannot tell which properties to expect nor what format to expect for a given property name. I don't know that the interface could be made easily introspectible or not (it would probably require some QAPI unions, and a LOT more generated code). So it would be nice if we could explore how hard it would be to use a type-safe representation instead of 'any', before declaring that this is the best we can do. Or, it may be the sign of a bigger issue that we have no good way to introspect what qom properties to expect, in general (and that solving that would also solve this).
+ +## +# @query-host-cpu: +# +# @migratable: #optional If false, unmigratable features will be +# returned as well. If true, only migratable features +# will be returned. Defaults to true. +# +# Return information about CPU capabilities in the current host. +# The returned data may depend on machine and accelerator configuration. +# +# Returns: A HostCPUInfo object. +# +# Since: 2.7.0 +## +{ 'command': 'query-host-cpu', 'data': { '*migratable': 'bool' }, + 'returns': 'HostCPUInfo' } + # @AddfdInfo:
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Tue, Jun 21, 2016 at 03:48:58PM -0600, Eric Blake wrote:
On 06/20/2016 02:12 PM, Eduardo Habkost wrote:
The command can be used to return host-specific CPU capabilities information.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- include/sysemu/arch_init.h | 1 + qapi-schema.json | 36 ++++++++++++++++++++++++++++++++++++ qmp-commands.hx | 6 ++++++ qmp.c | 13 +++++++++++++ stubs/Makefile.objs | 1 + stubs/arch-query-host-cpu-info.c | 8 ++++++++ 6 files changed, 65 insertions(+) create mode 100644 stubs/arch-query-host-cpu-info.c
diff --git a/include/sysemu/arch_init.h b/include/sysemu/arch_init.h index d690dfa..54215ab 100644 --- a/include/sysemu/arch_init.h +++ b/include/sysemu/arch_init.h @@ -35,5 +35,6 @@ int kvm_available(void); int xen_available(void);
CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp); +void arch_query_host_cpu_info(HostCPUInfo *r, bool migratable, Error **errp);
#endif diff --git a/qapi-schema.json b/qapi-schema.json index 19e3ef2..d2f4879 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -3047,6 +3047,42 @@ ## { 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'] }
+ +## +# @HostCPUInfo: +# +# Information on CPU capabilities supported by the current host. +# +# @qom-properties: #optional Values of CPU QOM properties corresponding +# to CPU capabilities supported by the host. +# +# Most properties returned in qom-properties are boolean properties +# indicating if a feature can be enabled in the current host. Other +# non-boolean properties may be returned, the semantics of each property +# depend on the architecture-specific code that handle them. +# +# Since: 2.7.0
Most places in .json files list just 'Since: x.y' rather than 'x.y.z', but we aren't consistent enough to insist either way on including or excluding a micro release number.
+## +{ 'struct': 'HostCPUInfo', + 'data': { '*qom-properties': 'any' } }
This is a big hammer that makes the properties non-introspectible - a client can tell that properties will be returned, but cannot tell which properties to expect nor what format to expect for a given property name. I don't know that the interface could be made easily introspectible or not (it would probably require some QAPI unions, and a LOT more generated code). So it would be nice if we could explore how hard it would be to use a type-safe representation instead of 'any', before declaring that this is the best we can do. Or, it may be the sign of a bigger issue that we have no good way to introspect what qom properties to expect, in general (and that solving that would also solve this).
What I thought libvirt needed is different from what I though, so this series will be dropped by now (see the "s390x CPU models: exposing features" thread). But your comments may still apply when we look at the alternative "query-cpu-model-expansion" proposal. I believe QOM introspection is really the issue here. The CPU configuration is already based on QOM properties. Manually duplicating the existing QOM properties into the QAPI representation would be a waste of time, IMO. But I agree that the interface could be improved: we should document very clearly what can be done with the QOM property list being returned, and return only useful data. For example: we could only return properties that really makes sense when used with "-cpu" or "-global" (not every single QOM property), and document that. -- Eduardo
The code that loads host-specific information inside x86_cpu_realizefn() will be reused by the implementation of query-host-cpu, so move it to a separate function. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- target-i386/cpu.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index aadd0b9..3d3635d 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1491,6 +1491,20 @@ void x86_cpu_change_kvm_default(const char *prop, const char *value) static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w, bool migratable_only); +/* Load host-dependent CPU information, when applicable */ +static void x86_cpu_load_host_data(X86CPU *cpu) +{ + CPUX86State *env = &cpu->env; + FeatureWord w; + + if (cpu->host_features) { + for (w = 0; w < FEATURE_WORDS; w++) { + env->features[w] = + x86_cpu_get_supported_feature_word(w, cpu->migratable); + } + } +} + #ifdef CONFIG_KVM static int cpu_x86_fill_model_id(char *str) @@ -3012,18 +3026,13 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) return; } + x86_cpu_load_host_data(cpu); + /*TODO: cpu->host_features incorrectly overwrites features * set using "feat=on|off". Once we fix this, we can convert * plus_features & minus_features to global properties * inside x86_cpu_parse_featurestr() too. */ - if (cpu->host_features) { - for (w = 0; w < FEATURE_WORDS; w++) { - env->features[w] = - x86_cpu_get_supported_feature_word(w, cpu->migratable); - } - } - for (w = 0; w < FEATURE_WORDS; w++) { cpu->env.features[w] |= plus_features[w]; cpu->env.features[w] &= ~minus_features[w]; -- 2.5.5
The code that loads host-specific information inside x86_cpu_realizefn() will be reused by the implementation of query-host-cpu, so move it to a separate function.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- target-i386/cpu.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c index aadd0b9..3d3635d 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1491,6 +1491,20 @@ void x86_cpu_change_kvm_default(const char *prop, const char *value) static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w, bool migratable_only); +/* Load host-dependent CPU information, when applicable */ +static void x86_cpu_load_host_data(X86CPU *cpu) +{ + CPUX86State *env = &cpu->env; + FeatureWord w; + + if (cpu->host_features) { + for (w = 0; w < FEATURE_WORDS; w++) { + env->features[w] = + x86_cpu_get_supported_feature_word(w, cpu->migratable); + } + } +} + #ifdef CONFIG_KVM
static int cpu_x86_fill_model_id(char *str) @@ -3012,18 +3026,13 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) return; }
+ x86_cpu_load_host_data(cpu);
On Mon, 20 Jun 2016 17:12:43 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: this function should be below TODO comment as it applies to moved code. with this fixed Reviewed-by: Igor Mammedov <imammedo@redhat.com>
+ /*TODO: cpu->host_features incorrectly overwrites features * set using "feat=on|off". Once we fix this, we can convert * plus_features & minus_features to global properties * inside x86_cpu_parse_featurestr() too. */ - if (cpu->host_features) { - for (w = 0; w < FEATURE_WORDS; w++) { - env->features[w] = - x86_cpu_get_supported_feature_word(w, cpu->migratable); - } - } - for (w = 0; w < FEATURE_WORDS; w++) { cpu->env.features[w] |= plus_features[w]; cpu->env.features[w] &= ~minus_features[w];
On Thu, Jun 23, 2016 at 04:59:28PM +0200, Igor Mammedov wrote:
On Mon, 20 Jun 2016 17:12:43 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote:
The code that loads host-specific information inside x86_cpu_realizefn() will be reused by the implementation of query-host-cpu, so move it to a separate function.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- target-i386/cpu.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c index aadd0b9..3d3635d 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1491,6 +1491,20 @@ void x86_cpu_change_kvm_default(const char *prop, const char *value) static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w, bool migratable_only); +/* Load host-dependent CPU information, when applicable */ +static void x86_cpu_load_host_data(X86CPU *cpu) +{ + CPUX86State *env = &cpu->env; + FeatureWord w; + + if (cpu->host_features) { + for (w = 0; w < FEATURE_WORDS; w++) { + env->features[w] = + x86_cpu_get_supported_feature_word(w, cpu->migratable); + } + } +} + #ifdef CONFIG_KVM
static int cpu_x86_fill_model_id(char *str) @@ -3012,18 +3026,13 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) return; }
+ x86_cpu_load_host_data(cpu); this function should be below TODO comment as it applies to moved code.
It was on purpose. The comment is actually about the plus_features/minus_features code, that is the hack we want to remove after cpu->host_features is fixed. Placing the comment before the x86_cpu_load_host_data() call wouldn't make sense, as the host_features code is now hidden inside the function.
with this fixed Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Considering the above explanation, do you prefer that I keep the patch as-is, or move the comment inside x86_cpu_load_host_data()? (I will not move it before the x86_cpu_load_host_data() call)
+ /*TODO: cpu->host_features incorrectly overwrites features * set using "feat=on|off". Once we fix this, we can convert * plus_features & minus_features to global properties * inside x86_cpu_parse_featurestr() too. */ - if (cpu->host_features) { - for (w = 0; w < FEATURE_WORDS; w++) { - env->features[w] = - x86_cpu_get_supported_feature_word(w, cpu->migratable); - } - } - for (w = 0; w < FEATURE_WORDS; w++) { cpu->env.features[w] |= plus_features[w]; cpu->env.features[w] &= ~minus_features[w];
-- Eduardo
On Thu, 23 Jun 2016 13:04:53 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote:
On Thu, Jun 23, 2016 at 04:59:28PM +0200, Igor Mammedov wrote:
On Mon, 20 Jun 2016 17:12:43 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote:
The code that loads host-specific information inside x86_cpu_realizefn() will be reused by the implementation of query-host-cpu, so move it to a separate function.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- target-i386/cpu.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c index aadd0b9..3d3635d 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1491,6 +1491,20 @@ void x86_cpu_change_kvm_default(const char *prop, const char *value) static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w, bool migratable_only); +/* Load host-dependent CPU information, when applicable */ +static void x86_cpu_load_host_data(X86CPU *cpu) +{ + CPUX86State *env = &cpu->env; + FeatureWord w; + + if (cpu->host_features) { + for (w = 0; w < FEATURE_WORDS; w++) { + env->features[w] = + x86_cpu_get_supported_feature_word(w, cpu->migratable); + } + } +} + #ifdef CONFIG_KVM
static int cpu_x86_fill_model_id(char *str) @@ -3012,18 +3026,13 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) return; }
+ x86_cpu_load_host_data(cpu); this function should be below TODO comment as it applies to moved code.
It was on purpose. The comment is actually about the plus_features/minus_features code, that is the hack we want to remove after cpu->host_features is fixed.
Placing the comment before the x86_cpu_load_host_data() call wouldn't make sense, as the host_features code is now hidden inside the function.
with this fixed Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Considering the above explanation, do you prefer that I keep the patch as-is, or move the comment inside x86_cpu_load_host_data()? I prefer comment inside call as it is related to bug introduced by moving
env->features[w] = x86_cpu_get_supported_feature_word(w, cpu->migratable); into x86_cpu_parse_featurestr() for initfn(). plus_features/minus_features code in realize are side affect of above otherwise they could be converted at x86_cpu_parse_featurestr() time.
(I will not move it before the x86_cpu_load_host_data() call)
+ /*TODO: cpu->host_features incorrectly overwrites features * set using "feat=on|off". Once we fix this, we can convert * plus_features & minus_features to global properties * inside x86_cpu_parse_featurestr() too. */ - if (cpu->host_features) { - for (w = 0; w < FEATURE_WORDS; w++) { - env->features[w] = - x86_cpu_get_supported_feature_word(w, cpu->migratable); - } - } - for (w = 0; w < FEATURE_WORDS; w++) { cpu->env.features[w] |= plus_features[w]; cpu->env.features[w] &= ~minus_features[w];
On Thu, Jun 23, 2016 at 06:56:12PM +0200, Igor Mammedov wrote:
On Thu, 23 Jun 2016 13:04:53 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote:
On Thu, Jun 23, 2016 at 04:59:28PM +0200, Igor Mammedov wrote:
On Mon, 20 Jun 2016 17:12:43 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote:
The code that loads host-specific information inside x86_cpu_realizefn() will be reused by the implementation of query-host-cpu, so move it to a separate function.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- target-i386/cpu.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c index aadd0b9..3d3635d 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1491,6 +1491,20 @@ void x86_cpu_change_kvm_default(const char *prop, const char *value) static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w, bool migratable_only); +/* Load host-dependent CPU information, when applicable */ +static void x86_cpu_load_host_data(X86CPU *cpu) +{ + CPUX86State *env = &cpu->env; + FeatureWord w; + + if (cpu->host_features) { + for (w = 0; w < FEATURE_WORDS; w++) { + env->features[w] = + x86_cpu_get_supported_feature_word(w, cpu->migratable); + } + } +} + #ifdef CONFIG_KVM
static int cpu_x86_fill_model_id(char *str) @@ -3012,18 +3026,13 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) return; }
+ x86_cpu_load_host_data(cpu); this function should be below TODO comment as it applies to moved code.
It was on purpose. The comment is actually about the plus_features/minus_features code, that is the hack we want to remove after cpu->host_features is fixed.
Placing the comment before the x86_cpu_load_host_data() call wouldn't make sense, as the host_features code is now hidden inside the function.
with this fixed Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Considering the above explanation, do you prefer that I keep the patch as-is, or move the comment inside x86_cpu_load_host_data()? I prefer comment inside call as it is related to bug introduced by moving
env->features[w] = x86_cpu_get_supported_feature_word(w, cpu->migratable);
into x86_cpu_parse_featurestr() for initfn().
plus_features/minus_features code in realize are side affect of above otherwise they could be converted at x86_cpu_parse_featurestr() time.
OK, I will move it inside x86_cpu_load_host_data(). -- Eduardo
Return information on the host CPU using the "host" CPU model. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- target-i386/cpu.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 3d3635d..06b0b99 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -30,6 +30,7 @@ #include "qemu/config-file.h" #include "qapi/qmp/qerror.h" +#include "qom/qom-qobject.h" #include "qapi-types.h" #include "qapi-visit.h" #include "qapi/visitor.h" @@ -2222,6 +2223,69 @@ CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp) return cpu_list; } +/* Return host CPU info by instantiating a "host" CPU object, + * and returning all the default values for QOM properties + */ +void arch_query_host_cpu_info(HostCPUInfo *r, bool migratable, Error **errp) +{ + Object *obj = NULL; + ObjectPropertyIterator iter; + ObjectProperty *prop; + QDict *propdict = NULL; + Error *err = NULL; + + /* Host CPU information is returned only in KVM mode, by now */ + if (!kvm_enabled()) { + goto out; + } + + obj = object_new(X86_CPU_TYPE_NAME("host")); + + object_property_set_bool(obj, migratable, "migratable", &err); + if (err) { + goto out; + } + + x86_cpu_load_host_data(X86_CPU(obj)); + + propdict = qdict_new(); + + object_property_iter_init(&iter, obj); + while ((prop = object_property_iter_next(&iter))) { + QObject *v; + + /* Skip properties that aren't useful for the query because + * they don't make sense here: + * - "realized" doesn't make sense because we never realize + * the CPU object above. + * - "filtered-features" doesn't make sense because we + * never filter the feature list (as it is done inside + * realizefn). + */ + if (!strcmp(prop->name, "realized") || + !strcmp(prop->name, "filtered-features")) { + continue; + } + + v = object_property_get_qobject(obj, prop->name, &err); + if (err) { + goto out; + } + qdict_put_obj(propdict, prop->name, v); + } + + r->has_qom_properties = true; + r->qom_properties = QOBJECT(propdict); + +out: + object_unref(obj); + if (err) { + qobject_decref(QOBJECT(propdict)); + error_propagate(errp, err); + } + return; +} + static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w, bool migratable_only) { -- 2.5.5
Add QMP command to allow management software to query for CPU information for the running host.
The data returned by the command is in the form of a dictionary of QOM properties.
This series depends on the "Add runnability info to query-cpu-definitions" series I sent 2 weeks ago.
Git tree: https://github.com/ehabkost/qemu-hacks.git work/query-host-cpu
I like that interface, I'm going to post (maybe today? :) ) a similar interface that allows to also expand other cpu models, not just the host model. Maybe we can then decide which one makes sense for all of us. But in general, this interface is much better compared to what we had before. David
On Tue, Jun 21, 2016 at 08:20:40AM +0200, David Hildenbrand wrote:
Add QMP command to allow management software to query for CPU information for the running host.
The data returned by the command is in the form of a dictionary of QOM properties.
This series depends on the "Add runnability info to query-cpu-definitions" series I sent 2 weeks ago.
Git tree: https://github.com/ehabkost/qemu-hacks.git work/query-host-cpu
I like that interface, I'm going to post (maybe today? :) ) a similar interface that allows to also expand other cpu models, not just the host model.
In x86 I want to avoid exposing the details of other CPU models to libvirt because the details depend on machine-type. But if it is useful for you, I believe the same "qom-properties" dict could be returned in query-cpu-definitions.
Maybe we can then decide which one makes sense for all of us. But in general, this interface is much better compared to what we had before.
Maybe both? I think it's better to have a separate interface for querying "what exactly this host supports" and another one for querying for "what happens if I use -cpu host". In the case of x86, both are equivalent, but we can't guarantee this on all architectures. -- Eduardo
On Tue, Jun 21, 2016 at 08:20:40AM +0200, David Hildenbrand wrote:
Add QMP command to allow management software to query for CPU information for the running host.
The data returned by the command is in the form of a dictionary of QOM properties.
This series depends on the "Add runnability info to query-cpu-definitions" series I sent 2 weeks ago.
Git tree: https://github.com/ehabkost/qemu-hacks.git work/query-host-cpu
I like that interface, I'm going to post (maybe today? :) ) a similar interface that allows to also expand other cpu models, not just the host model.
In x86 I want to avoid exposing the details of other CPU models to libvirt because the details depend on machine-type.
But if it is useful for you, I believe the same "qom-properties" dict could be returned in query-cpu-definitions.
Maybe we can then decide which one makes sense for all of us. But in general, this interface is much better compared to what we had before.
Maybe both? I think it's better to have a separate interface for querying "what exactly this host supports" and another one for querying for "what happens if I use -cpu host". In the case of x86, both are equivalent, but we can't guarantee this on all architectures.
I'll post my patches in a couple of minutes, let's discuss it then. We might want to avoid having multiple interfaces carrying out the same task. David
On Tue, Jun 21, 2016 at 02:52:54PM +0200, David Hildenbrand wrote:
On Tue, Jun 21, 2016 at 08:20:40AM +0200, David Hildenbrand wrote:
Add QMP command to allow management software to query for CPU information for the running host.
The data returned by the command is in the form of a dictionary of QOM properties.
This series depends on the "Add runnability info to query-cpu-definitions" series I sent 2 weeks ago.
Git tree: https://github.com/ehabkost/qemu-hacks.git work/query-host-cpu
I like that interface, I'm going to post (maybe today? :) ) a similar interface that allows to also expand other cpu models, not just the host model.
In x86 I want to avoid exposing the details of other CPU models to libvirt because the details depend on machine-type.
But if it is useful for you, I believe the same "qom-properties" dict could be returned in query-cpu-definitions.
Maybe we can then decide which one makes sense for all of us. But in general, this interface is much better compared to what we had before.
Maybe both? I think it's better to have a separate interface for querying "what exactly this host supports" and another one for querying for "what happens if I use -cpu host". In the case of x86, both are equivalent, but we can't guarantee this on all architectures.
I'll post my patches in a couple of minutes, let's discuss it then.
We might want to avoid having multiple interfaces carrying out the same task.
OK, I will wait for the patches before discussing it. My assumption is that both look similar, but are actually different tasks. -- Eduardo
On Tue, Jun 21, 2016 at 02:52:54PM +0200, David Hildenbrand wrote:
On Tue, Jun 21, 2016 at 08:20:40AM +0200, David Hildenbrand wrote:
Add QMP command to allow management software to query for CPU information for the running host.
The data returned by the command is in the form of a dictionary of QOM properties.
This series depends on the "Add runnability info to query-cpu-definitions" series I sent 2 weeks ago.
Git tree: https://github.com/ehabkost/qemu-hacks.git work/query-host-cpu
I like that interface, I'm going to post (maybe today? :) ) a similar interface that allows to also expand other cpu models, not just the host model.
In x86 I want to avoid exposing the details of other CPU models to libvirt because the details depend on machine-type.
But if it is useful for you, I believe the same "qom-properties" dict could be returned in query-cpu-definitions.
Maybe we can then decide which one makes sense for all of us. But in general, this interface is much better compared to what we had before.
Maybe both? I think it's better to have a separate interface for querying "what exactly this host supports" and another one for querying for "what happens if I use -cpu host". In the case of x86, both are equivalent, but we can't guarantee this on all architectures.
I'll post my patches in a couple of minutes, let's discuss it then.
We might want to avoid having multiple interfaces carrying out the same task.
OK, I will wait for the patches before discussing it. My assumption is that both look similar, but are actually different tasks.
For us, also "-cpu host" and querying the host model is identical. Not sure if there would for now really be a requirement for some other architecture to handle this differently. Do you have anything specific already in mind? But let's see if it indeed makes sense to split this up after looking at our interface proposal. David
participants (4)
-
David Hildenbrand -
Eduardo Habkost -
Eric Blake -
Igor Mammedov