* [Qemu-devel] [uq/master PATCH 1/7] target-i386: Eliminate CONFIG_KVM #ifdefs
2014-01-30 19:48 [Qemu-devel] [uq/master PATCH 0/7] x86 CPU subclasses, take 7 Eduardo Habkost
@ 2014-01-30 19:48 ` Eduardo Habkost
2014-01-31 11:42 ` Paolo Bonzini
2014-01-30 19:48 ` [Qemu-devel] [uq/master PATCH 2/7] target-i386: Don't change x86_def_t struct on cpu_x86_register() Eduardo Habkost
` (7 subsequent siblings)
8 siblings, 1 reply; 45+ messages in thread
From: Eduardo Habkost @ 2014-01-30 19:48 UTC (permalink / raw)
To: qemu-devel
Cc: kvm, libvir-list, Igor Mammedov, Paolo Bonzini, Jiri Denemark,
Andreas Färber
The compiler is already able to eliminate the kvm_arch_get_supported_cpuid()
calls in kvm_cpu_fill_host() and filter_features_for_kvm(), so we can
eliminate the CONFIG_KVM #ifdefs there.
Also, kvm_cpu_fill_host() and host_cpuid() don't need to check
CONFIG_KVM, as they don't have any KVM-specific function calls.
Tested to build successfully with CONFIG_KVM disabled, using the
following CFLAGS combinations: "-DNDEBUG", "-DNDEBUG -O', "-DNDEBUG
-O0", "-DNDEBUG -O1", "-DNDEBUG -O2".
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v2:
* Check for __i386__ on host_cpuid() so the code compiles properly
on non-x86 hosts.
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Change v3:
* Don't add assert(kvm_enabled()) line, which is not necessary to help
the compiler (and wouldn't help it if using -DNDEBUG, anyway).
Reported-by: Richard Henderson <rth@twiddle.net>
* Commit message update
---
target-i386/cpu.c | 15 +++------------
1 file changed, 3 insertions(+), 12 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 1f30efd..8425212 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -374,7 +374,6 @@ void disable_kvm_pv_eoi(void)
void host_cpuid(uint32_t function, uint32_t count,
uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx)
{
-#if defined(CONFIG_KVM)
uint32_t vec[4];
#ifdef __x86_64__
@@ -382,7 +381,7 @@ void host_cpuid(uint32_t function, uint32_t count,
: "=a"(vec[0]), "=b"(vec[1]),
"=c"(vec[2]), "=d"(vec[3])
: "0"(function), "c"(count) : "cc");
-#else
+#elif defined(__i386__)
asm volatile("pusha \n\t"
"cpuid \n\t"
"mov %%eax, 0(%2) \n\t"
@@ -392,6 +391,8 @@ void host_cpuid(uint32_t function, uint32_t count,
"popa"
: : "a"(function), "c"(count), "S"(vec)
: "memory", "cc");
+#else
+ abort();
#endif
if (eax)
@@ -402,7 +403,6 @@ void host_cpuid(uint32_t function, uint32_t count,
*ecx = vec[2];
if (edx)
*edx = vec[3];
-#endif
}
#define iswhite(c) ((c) && ((c) <= ' ' || '~' < (c)))
@@ -1119,7 +1119,6 @@ void x86_cpu_compat_set_features(const char *cpu_model, FeatureWord w,
}
}
-#ifdef CONFIG_KVM
static int cpu_x86_fill_model_id(char *str)
{
uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
@@ -1134,7 +1133,6 @@ static int cpu_x86_fill_model_id(char *str)
}
return 0;
}
-#endif
/* Fill a x86_def_t struct with information about the host CPU, and
* the CPU features supported by the host hardware + host kernel
@@ -1143,7 +1141,6 @@ static int cpu_x86_fill_model_id(char *str)
*/
static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
{
-#ifdef CONFIG_KVM
KVMState *s = kvm_state;
uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
@@ -1173,8 +1170,6 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
kvm_arch_get_supported_cpuid(s, wi->cpuid_eax, wi->cpuid_ecx,
wi->cpuid_reg);
}
-
-#endif /* CONFIG_KVM */
}
static int unavailable_host_feature(FeatureWordInfo *f, uint32_t mask)
@@ -1817,7 +1812,6 @@ CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
return cpu_list;
}
-#ifdef CONFIG_KVM
static void filter_features_for_kvm(X86CPU *cpu)
{
CPUX86State *env = &cpu->env;
@@ -1834,7 +1828,6 @@ static void filter_features_for_kvm(X86CPU *cpu)
cpu->filtered_features[w] = requested_features & ~env->features[w];
}
}
-#endif
static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp)
{
@@ -2545,9 +2538,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
"Host's CPU doesn't support requested features");
goto out;
}
-#ifdef CONFIG_KVM
filter_features_for_kvm(cpu);
-#endif
}
#ifndef CONFIG_USER_ONLY
--
1.8.4.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [uq/master PATCH 1/7] target-i386: Eliminate CONFIG_KVM #ifdefs
2014-01-30 19:48 ` [Qemu-devel] [uq/master PATCH 1/7] target-i386: Eliminate CONFIG_KVM #ifdefs Eduardo Habkost
@ 2014-01-31 11:42 ` Paolo Bonzini
0 siblings, 0 replies; 45+ messages in thread
From: Paolo Bonzini @ 2014-01-31 11:42 UTC (permalink / raw)
To: Eduardo Habkost, qemu-devel
Cc: libvir-list, Igor Mammedov, Jiri Denemark, Andreas Färber,
kvm
Il 30/01/2014 20:48, Eduardo Habkost ha scritto:
> The compiler is already able to eliminate the kvm_arch_get_supported_cpuid()
> calls in kvm_cpu_fill_host() and filter_features_for_kvm(), so we can
> eliminate the CONFIG_KVM #ifdefs there.
>
> Also, kvm_cpu_fill_host() and host_cpuid() don't need to check
> CONFIG_KVM, as they don't have any KVM-specific function calls.
>
> Tested to build successfully with CONFIG_KVM disabled, using the
> following CFLAGS combinations: "-DNDEBUG", "-DNDEBUG -O', "-DNDEBUG
> -O0", "-DNDEBUG -O1", "-DNDEBUG -O2".
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> Changes v2:
> * Check for __i386__ on host_cpuid() so the code compiles properly
> on non-x86 hosts.
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
>
> Change v3:
> * Don't add assert(kvm_enabled()) line, which is not necessary to help
> the compiler (and wouldn't help it if using -DNDEBUG, anyway).
> Reported-by: Richard Henderson <rth@twiddle.net>
> * Commit message update
> ---
> target-i386/cpu.c | 15 +++------------
> 1 file changed, 3 insertions(+), 12 deletions(-)
>
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 1f30efd..8425212 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -374,7 +374,6 @@ void disable_kvm_pv_eoi(void)
> void host_cpuid(uint32_t function, uint32_t count,
> uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx)
> {
> -#if defined(CONFIG_KVM)
> uint32_t vec[4];
>
> #ifdef __x86_64__
> @@ -382,7 +381,7 @@ void host_cpuid(uint32_t function, uint32_t count,
> : "=a"(vec[0]), "=b"(vec[1]),
> "=c"(vec[2]), "=d"(vec[3])
> : "0"(function), "c"(count) : "cc");
> -#else
> +#elif defined(__i386__)
> asm volatile("pusha \n\t"
> "cpuid \n\t"
> "mov %%eax, 0(%2) \n\t"
> @@ -392,6 +391,8 @@ void host_cpuid(uint32_t function, uint32_t count,
> "popa"
> : : "a"(function), "c"(count), "S"(vec)
> : "memory", "cc");
> +#else
> + abort();
> #endif
>
> if (eax)
> @@ -402,7 +403,6 @@ void host_cpuid(uint32_t function, uint32_t count,
> *ecx = vec[2];
> if (edx)
> *edx = vec[3];
> -#endif
> }
>
> #define iswhite(c) ((c) && ((c) <= ' ' || '~' < (c)))
> @@ -1119,7 +1119,6 @@ void x86_cpu_compat_set_features(const char *cpu_model, FeatureWord w,
> }
> }
>
> -#ifdef CONFIG_KVM
> static int cpu_x86_fill_model_id(char *str)
> {
> uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
> @@ -1134,7 +1133,6 @@ static int cpu_x86_fill_model_id(char *str)
> }
> return 0;
> }
> -#endif
>
> /* Fill a x86_def_t struct with information about the host CPU, and
> * the CPU features supported by the host hardware + host kernel
> @@ -1143,7 +1141,6 @@ static int cpu_x86_fill_model_id(char *str)
> */
> static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
> {
> -#ifdef CONFIG_KVM
> KVMState *s = kvm_state;
> uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
>
> @@ -1173,8 +1170,6 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
> kvm_arch_get_supported_cpuid(s, wi->cpuid_eax, wi->cpuid_ecx,
> wi->cpuid_reg);
> }
> -
> -#endif /* CONFIG_KVM */
> }
>
> static int unavailable_host_feature(FeatureWordInfo *f, uint32_t mask)
> @@ -1817,7 +1812,6 @@ CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
> return cpu_list;
> }
>
> -#ifdef CONFIG_KVM
> static void filter_features_for_kvm(X86CPU *cpu)
> {
> CPUX86State *env = &cpu->env;
> @@ -1834,7 +1828,6 @@ static void filter_features_for_kvm(X86CPU *cpu)
> cpu->filtered_features[w] = requested_features & ~env->features[w];
> }
> }
> -#endif
>
> static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp)
> {
> @@ -2545,9 +2538,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
> "Host's CPU doesn't support requested features");
> goto out;
> }
> -#ifdef CONFIG_KVM
> filter_features_for_kvm(cpu);
> -#endif
> }
>
> #ifndef CONFIG_USER_ONLY
>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
^ permalink raw reply [flat|nested] 45+ messages in thread
* [Qemu-devel] [uq/master PATCH 2/7] target-i386: Don't change x86_def_t struct on cpu_x86_register()
2014-01-30 19:48 [Qemu-devel] [uq/master PATCH 0/7] x86 CPU subclasses, take 7 Eduardo Habkost
2014-01-30 19:48 ` [Qemu-devel] [uq/master PATCH 1/7] target-i386: Eliminate CONFIG_KVM #ifdefs Eduardo Habkost
@ 2014-01-30 19:48 ` Eduardo Habkost
2014-01-31 11:42 ` Paolo Bonzini
2014-01-30 19:48 ` [Qemu-devel] [uq/master PATCH 3/7] target-i386: Move KVM default-vendor hack to instance_init Eduardo Habkost
` (6 subsequent siblings)
8 siblings, 1 reply; 45+ messages in thread
From: Eduardo Habkost @ 2014-01-30 19:48 UTC (permalink / raw)
To: qemu-devel
Cc: kvm, libvir-list, Igor Mammedov, Paolo Bonzini, Jiri Denemark,
Andreas Färber
As eventually the x86_def_t data is going to be provided by the CPU
class, it's better to not touch it, and handle the special cases on the
X86CPU object itself.
Current behavior of the code should stay exactly the same.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
target-i386/cpu.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 8425212..be54f84 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1841,11 +1841,6 @@ static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp)
return;
}
- if (kvm_enabled()) {
- def->features[FEAT_KVM] |= kvm_default_features;
- }
- def->features[FEAT_1_ECX] |= CPUID_EXT_HYPERVISOR;
-
object_property_set_str(OBJECT(cpu), def->vendor, "vendor", errp);
object_property_set_int(OBJECT(cpu), def->level, "level", errp);
object_property_set_int(OBJECT(cpu), def->family, "family", errp);
@@ -1864,6 +1859,12 @@ static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp)
cpu->cache_info_passthrough = def->cache_info_passthrough;
object_property_set_str(OBJECT(cpu), def->model_id, "model-id", errp);
+
+ /* Special cases not set in the x86_def_t structs: */
+ if (kvm_enabled()) {
+ env->features[FEAT_KVM] |= kvm_default_features;
+ }
+ env->features[FEAT_1_ECX] |= CPUID_EXT_HYPERVISOR;
}
X86CPU *cpu_x86_create(const char *cpu_model, DeviceState *icc_bridge,
--
1.8.4.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [uq/master PATCH 2/7] target-i386: Don't change x86_def_t struct on cpu_x86_register()
2014-01-30 19:48 ` [Qemu-devel] [uq/master PATCH 2/7] target-i386: Don't change x86_def_t struct on cpu_x86_register() Eduardo Habkost
@ 2014-01-31 11:42 ` Paolo Bonzini
0 siblings, 0 replies; 45+ messages in thread
From: Paolo Bonzini @ 2014-01-31 11:42 UTC (permalink / raw)
To: Eduardo Habkost, qemu-devel
Cc: libvir-list, Igor Mammedov, Jiri Denemark, Andreas Färber,
kvm
Il 30/01/2014 20:48, Eduardo Habkost ha scritto:
> As eventually the x86_def_t data is going to be provided by the CPU
> class, it's better to not touch it, and handle the special cases on the
> X86CPU object itself.
>
> Current behavior of the code should stay exactly the same.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> target-i386/cpu.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 8425212..be54f84 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1841,11 +1841,6 @@ static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp)
> return;
> }
>
> - if (kvm_enabled()) {
> - def->features[FEAT_KVM] |= kvm_default_features;
> - }
> - def->features[FEAT_1_ECX] |= CPUID_EXT_HYPERVISOR;
> -
> object_property_set_str(OBJECT(cpu), def->vendor, "vendor", errp);
> object_property_set_int(OBJECT(cpu), def->level, "level", errp);
> object_property_set_int(OBJECT(cpu), def->family, "family", errp);
> @@ -1864,6 +1859,12 @@ static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp)
> cpu->cache_info_passthrough = def->cache_info_passthrough;
>
> object_property_set_str(OBJECT(cpu), def->model_id, "model-id", errp);
> +
> + /* Special cases not set in the x86_def_t structs: */
> + if (kvm_enabled()) {
> + env->features[FEAT_KVM] |= kvm_default_features;
> + }
> + env->features[FEAT_1_ECX] |= CPUID_EXT_HYPERVISOR;
> }
>
> X86CPU *cpu_x86_create(const char *cpu_model, DeviceState *icc_bridge,
>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
^ permalink raw reply [flat|nested] 45+ messages in thread
* [Qemu-devel] [uq/master PATCH 3/7] target-i386: Move KVM default-vendor hack to instance_init
2014-01-30 19:48 [Qemu-devel] [uq/master PATCH 0/7] x86 CPU subclasses, take 7 Eduardo Habkost
2014-01-30 19:48 ` [Qemu-devel] [uq/master PATCH 1/7] target-i386: Eliminate CONFIG_KVM #ifdefs Eduardo Habkost
2014-01-30 19:48 ` [Qemu-devel] [uq/master PATCH 2/7] target-i386: Don't change x86_def_t struct on cpu_x86_register() Eduardo Habkost
@ 2014-01-30 19:48 ` Eduardo Habkost
2014-01-31 11:42 ` Paolo Bonzini
2014-01-30 19:48 ` [Qemu-devel] [uq/master PATCH 4/7] target-i386: Rename cpu_x86_register() to x86_cpu_load_def() Eduardo Habkost
` (5 subsequent siblings)
8 siblings, 1 reply; 45+ messages in thread
From: Eduardo Habkost @ 2014-01-30 19:48 UTC (permalink / raw)
To: qemu-devel
Cc: kvm, libvir-list, Igor Mammedov, Paolo Bonzini, Jiri Denemark,
Andreas Färber
As we will not have a cpu_x86_find_by_name() function anymore,
move the KVM default-vendor hack to instance_init.
Unfortunately we can't move that code to class_init because it depends
on KVM being initialized.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
target-i386/cpu.c | 32 +++++++++++++++++++-------------
1 file changed, 19 insertions(+), 13 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index be54f84..0e8812a 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1601,18 +1601,6 @@ static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def,
def = &builtin_x86_defs[i];
if (strcmp(name, def->name) == 0) {
memcpy(x86_cpu_def, def, sizeof(*def));
- /* sysenter isn't supported in compatibility mode on AMD,
- * syscall isn't supported in compatibility mode on Intel.
- * Normally we advertise the actual CPU vendor, but you can
- * override this using the 'vendor' property if you want to use
- * KVM's sysenter/syscall emulation in compatibility mode and
- * when doing cross vendor migration
- */
- if (kvm_enabled()) {
- uint32_t ebx = 0, ecx = 0, edx = 0;
- host_cpuid(0, 0, NULL, &ebx, &ecx, &edx);
- x86_cpu_vendor_words2str(x86_cpu_def->vendor, ebx, edx, ecx);
- }
return 0;
}
}
@@ -1841,7 +1829,6 @@ static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp)
return;
}
- object_property_set_str(OBJECT(cpu), def->vendor, "vendor", errp);
object_property_set_int(OBJECT(cpu), def->level, "level", errp);
object_property_set_int(OBJECT(cpu), def->family, "family", errp);
object_property_set_int(OBJECT(cpu), def->model, "model", errp);
@@ -1865,6 +1852,25 @@ static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp)
env->features[FEAT_KVM] |= kvm_default_features;
}
env->features[FEAT_1_ECX] |= CPUID_EXT_HYPERVISOR;
+
+ /* sysenter isn't supported in compatibility mode on AMD,
+ * syscall isn't supported in compatibility mode on Intel.
+ * Normally we advertise the actual CPU vendor, but you can
+ * override this using the 'vendor' property if you want to use
+ * KVM's sysenter/syscall emulation in compatibility mode and
+ * when doing cross vendor migration
+ */
+ const char *vendor = def->vendor;
+ char host_vendor[CPUID_VENDOR_SZ + 1];
+ if (kvm_enabled()) {
+ uint32_t ebx = 0, ecx = 0, edx = 0;
+ host_cpuid(0, 0, NULL, &ebx, &ecx, &edx);
+ x86_cpu_vendor_words2str(host_vendor, ebx, edx, ecx);
+ vendor = host_vendor;
+ }
+
+ object_property_set_str(OBJECT(cpu), vendor, "vendor", errp);
+
}
X86CPU *cpu_x86_create(const char *cpu_model, DeviceState *icc_bridge,
--
1.8.4.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [uq/master PATCH 3/7] target-i386: Move KVM default-vendor hack to instance_init
2014-01-30 19:48 ` [Qemu-devel] [uq/master PATCH 3/7] target-i386: Move KVM default-vendor hack to instance_init Eduardo Habkost
@ 2014-01-31 11:42 ` Paolo Bonzini
0 siblings, 0 replies; 45+ messages in thread
From: Paolo Bonzini @ 2014-01-31 11:42 UTC (permalink / raw)
To: Eduardo Habkost, qemu-devel
Cc: libvir-list, Igor Mammedov, Jiri Denemark, Andreas Färber,
kvm
Il 30/01/2014 20:48, Eduardo Habkost ha scritto:
> As we will not have a cpu_x86_find_by_name() function anymore,
> move the KVM default-vendor hack to instance_init.
>
> Unfortunately we can't move that code to class_init because it depends
> on KVM being initialized.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> target-i386/cpu.c | 32 +++++++++++++++++++-------------
> 1 file changed, 19 insertions(+), 13 deletions(-)
>
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index be54f84..0e8812a 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1601,18 +1601,6 @@ static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def,
> def = &builtin_x86_defs[i];
> if (strcmp(name, def->name) == 0) {
> memcpy(x86_cpu_def, def, sizeof(*def));
> - /* sysenter isn't supported in compatibility mode on AMD,
> - * syscall isn't supported in compatibility mode on Intel.
> - * Normally we advertise the actual CPU vendor, but you can
> - * override this using the 'vendor' property if you want to use
> - * KVM's sysenter/syscall emulation in compatibility mode and
> - * when doing cross vendor migration
> - */
> - if (kvm_enabled()) {
> - uint32_t ebx = 0, ecx = 0, edx = 0;
> - host_cpuid(0, 0, NULL, &ebx, &ecx, &edx);
> - x86_cpu_vendor_words2str(x86_cpu_def->vendor, ebx, edx, ecx);
> - }
> return 0;
> }
> }
> @@ -1841,7 +1829,6 @@ static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp)
> return;
> }
>
> - object_property_set_str(OBJECT(cpu), def->vendor, "vendor", errp);
> object_property_set_int(OBJECT(cpu), def->level, "level", errp);
> object_property_set_int(OBJECT(cpu), def->family, "family", errp);
> object_property_set_int(OBJECT(cpu), def->model, "model", errp);
> @@ -1865,6 +1852,25 @@ static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp)
> env->features[FEAT_KVM] |= kvm_default_features;
> }
> env->features[FEAT_1_ECX] |= CPUID_EXT_HYPERVISOR;
> +
> + /* sysenter isn't supported in compatibility mode on AMD,
> + * syscall isn't supported in compatibility mode on Intel.
> + * Normally we advertise the actual CPU vendor, but you can
> + * override this using the 'vendor' property if you want to use
> + * KVM's sysenter/syscall emulation in compatibility mode and
> + * when doing cross vendor migration
> + */
> + const char *vendor = def->vendor;
> + char host_vendor[CPUID_VENDOR_SZ + 1];
> + if (kvm_enabled()) {
> + uint32_t ebx = 0, ecx = 0, edx = 0;
> + host_cpuid(0, 0, NULL, &ebx, &ecx, &edx);
> + x86_cpu_vendor_words2str(host_vendor, ebx, edx, ecx);
> + vendor = host_vendor;
> + }
> +
> + object_property_set_str(OBJECT(cpu), vendor, "vendor", errp);
> +
> }
>
> X86CPU *cpu_x86_create(const char *cpu_model, DeviceState *icc_bridge,
>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
^ permalink raw reply [flat|nested] 45+ messages in thread
* [Qemu-devel] [uq/master PATCH 4/7] target-i386: Rename cpu_x86_register() to x86_cpu_load_def()
2014-01-30 19:48 [Qemu-devel] [uq/master PATCH 0/7] x86 CPU subclasses, take 7 Eduardo Habkost
` (2 preceding siblings ...)
2014-01-30 19:48 ` [Qemu-devel] [uq/master PATCH 3/7] target-i386: Move KVM default-vendor hack to instance_init Eduardo Habkost
@ 2014-01-30 19:48 ` Eduardo Habkost
2014-01-31 11:42 ` Paolo Bonzini
2014-01-30 19:48 ` [Qemu-devel] [uq/master PATCH 5/7] target-i386: Call x86_cpu_load_def() earlier Eduardo Habkost
` (4 subsequent siblings)
8 siblings, 1 reply; 45+ messages in thread
From: Eduardo Habkost @ 2014-01-30 19:48 UTC (permalink / raw)
To: qemu-devel
Cc: kvm, libvir-list, Igor Mammedov, Paolo Bonzini, Jiri Denemark,
Andreas Färber
There isn't any kind of "registration" involved in cpu_x86_register()
anymore: it is simply looking up a CPU model name and loading the model
definition data into the X86CPU object. Rename it to x86_cpu_load_def()
to reflect what it does.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
target-i386/cpu.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 0e8812a..58b4c71 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1817,7 +1817,9 @@ static void filter_features_for_kvm(X86CPU *cpu)
}
}
-static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp)
+/* Load CPU definition for a given CPU model name
+ */
+static void x86_cpu_load_def(X86CPU *cpu, const char *name, Error **errp)
{
CPUX86State *env = &cpu->env;
x86_def_t def1, *def = &def1;
@@ -1900,7 +1902,7 @@ X86CPU *cpu_x86_create(const char *cpu_model, DeviceState *icc_bridge,
object_unref(OBJECT(cpu));
#endif
- cpu_x86_register(cpu, name, &error);
+ x86_cpu_load_def(cpu, name, &error);
if (error) {
goto out;
}
--
1.8.4.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [uq/master PATCH 4/7] target-i386: Rename cpu_x86_register() to x86_cpu_load_def()
2014-01-30 19:48 ` [Qemu-devel] [uq/master PATCH 4/7] target-i386: Rename cpu_x86_register() to x86_cpu_load_def() Eduardo Habkost
@ 2014-01-31 11:42 ` Paolo Bonzini
2014-02-10 0:03 ` Andreas Färber
0 siblings, 1 reply; 45+ messages in thread
From: Paolo Bonzini @ 2014-01-31 11:42 UTC (permalink / raw)
To: Eduardo Habkost, qemu-devel
Cc: libvir-list, Igor Mammedov, Jiri Denemark, Andreas Färber,
kvm
Il 30/01/2014 20:48, Eduardo Habkost ha scritto:
> There isn't any kind of "registration" involved in cpu_x86_register()
> anymore: it is simply looking up a CPU model name and loading the model
> definition data into the X86CPU object. Rename it to x86_cpu_load_def()
> to reflect what it does.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> target-i386/cpu.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 0e8812a..58b4c71 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1817,7 +1817,9 @@ static void filter_features_for_kvm(X86CPU *cpu)
> }
> }
>
> -static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp)
> +/* Load CPU definition for a given CPU model name
> + */
> +static void x86_cpu_load_def(X86CPU *cpu, const char *name, Error **errp)
> {
> CPUX86State *env = &cpu->env;
> x86_def_t def1, *def = &def1;
> @@ -1900,7 +1902,7 @@ X86CPU *cpu_x86_create(const char *cpu_model, DeviceState *icc_bridge,
> object_unref(OBJECT(cpu));
> #endif
>
> - cpu_x86_register(cpu, name, &error);
> + x86_cpu_load_def(cpu, name, &error);
> if (error) {
> goto out;
> }
>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [uq/master PATCH 4/7] target-i386: Rename cpu_x86_register() to x86_cpu_load_def()
2014-01-31 11:42 ` Paolo Bonzini
@ 2014-02-10 0:03 ` Andreas Färber
0 siblings, 0 replies; 45+ messages in thread
From: Andreas Färber @ 2014-02-10 0:03 UTC (permalink / raw)
To: Paolo Bonzini, Eduardo Habkost, qemu-devel
Cc: libvir-list, Igor Mammedov, Jiri Denemark, kvm
Am 31.01.2014 12:42, schrieb Paolo Bonzini:
> Il 30/01/2014 20:48, Eduardo Habkost ha scritto:
>> There isn't any kind of "registration" involved in cpu_x86_register()
>> anymore: it is simply looking up a CPU model name and loading the model
>> definition data into the X86CPU object. Rename it to x86_cpu_load_def()
>> to reflect what it does.
>>
>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
[...]
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Thanks, applied to qom-cpu:
https://github.com/afaerber/qemu-cpu/commits/qom-cpu
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 45+ messages in thread
* [Qemu-devel] [uq/master PATCH 5/7] target-i386: Call x86_cpu_load_def() earlier
2014-01-30 19:48 [Qemu-devel] [uq/master PATCH 0/7] x86 CPU subclasses, take 7 Eduardo Habkost
` (3 preceding siblings ...)
2014-01-30 19:48 ` [Qemu-devel] [uq/master PATCH 4/7] target-i386: Rename cpu_x86_register() to x86_cpu_load_def() Eduardo Habkost
@ 2014-01-30 19:48 ` Eduardo Habkost
2014-02-10 0:13 ` Andreas Färber
2014-01-30 19:48 ` [Qemu-devel] [uq/master PATCH 6/7] target-i386: Rename x86_def_t to X86CPUDefinition Eduardo Habkost
` (3 subsequent siblings)
8 siblings, 1 reply; 45+ messages in thread
From: Eduardo Habkost @ 2014-01-30 19:48 UTC (permalink / raw)
To: qemu-devel
Cc: kvm, libvir-list, Igor Mammedov, Paolo Bonzini, Jiri Denemark,
Andreas Färber
As we will initialize the X86CPU fields on instance_init eventually,
move the code that initializes the X86CPU data based on the CPU model
name closer to the object_new() call.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
target-i386/cpu.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 58b4c71..5c13ed6 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1893,6 +1893,11 @@ X86CPU *cpu_x86_create(const char *cpu_model, DeviceState *icc_bridge,
features = model_pieces[1];
cpu = X86_CPU(object_new(TYPE_X86_CPU));
+ x86_cpu_load_def(cpu, name, &error);
+ if (error) {
+ goto out;
+ }
+
#ifndef CONFIG_USER_ONLY
if (icc_bridge == NULL) {
error_setg(&error, "Invalid icc-bridge value");
@@ -1902,11 +1907,6 @@ X86CPU *cpu_x86_create(const char *cpu_model, DeviceState *icc_bridge,
object_unref(OBJECT(cpu));
#endif
- x86_cpu_load_def(cpu, name, &error);
- if (error) {
- goto out;
- }
-
/* Emulate per-model subclasses for global properties */
typename = g_strdup_printf("%s-" TYPE_X86_CPU, name);
qdev_prop_set_globals_for_type(DEVICE(cpu), typename, &error);
--
1.8.4.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [uq/master PATCH 5/7] target-i386: Call x86_cpu_load_def() earlier
2014-01-30 19:48 ` [Qemu-devel] [uq/master PATCH 5/7] target-i386: Call x86_cpu_load_def() earlier Eduardo Habkost
@ 2014-02-10 0:13 ` Andreas Färber
0 siblings, 0 replies; 45+ messages in thread
From: Andreas Färber @ 2014-02-10 0:13 UTC (permalink / raw)
To: Eduardo Habkost, qemu-devel
Cc: libvir-list, Igor Mammedov, Jiri Denemark, kvm, Paolo Bonzini
Am 30.01.2014 20:48, schrieb Eduardo Habkost:
> As we will initialize the X86CPU fields on instance_init eventually,
> move the code that initializes the X86CPU data based on the CPU model
> name closer to the object_new() call.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Thanks, applied to qom-cpu:
https://github.com/afaerber/qemu-cpu/commits/qom-cpu
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 45+ messages in thread
* [Qemu-devel] [uq/master PATCH 6/7] target-i386: Rename x86_def_t to X86CPUDefinition
2014-01-30 19:48 [Qemu-devel] [uq/master PATCH 0/7] x86 CPU subclasses, take 7 Eduardo Habkost
` (4 preceding siblings ...)
2014-01-30 19:48 ` [Qemu-devel] [uq/master PATCH 5/7] target-i386: Call x86_cpu_load_def() earlier Eduardo Habkost
@ 2014-01-30 19:48 ` Eduardo Habkost
2014-01-31 11:42 ` Paolo Bonzini
2014-01-30 19:48 ` [Qemu-devel] [uq/master PATCH 7/7] target-i386: CPU model subclasses Eduardo Habkost
` (2 subsequent siblings)
8 siblings, 1 reply; 45+ messages in thread
From: Eduardo Habkost @ 2014-01-30 19:48 UTC (permalink / raw)
To: qemu-devel
Cc: kvm, libvir-list, Igor Mammedov, Paolo Bonzini, Jiri Denemark,
Andreas Färber
As the new X86CPU subclass code is going to change lots of the code
invoving x86_def_t, let's rename the struct to match coding style first.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
target-i386/cpu.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 5c13ed6..6659527 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -484,7 +484,7 @@ static void add_flagname_to_bitmaps(const char *flagname,
}
}
-typedef struct x86_def_t {
+typedef struct X86CPUDefinition {
const char *name;
uint32_t level;
uint32_t xlevel;
@@ -497,7 +497,7 @@ typedef struct x86_def_t {
FeatureWordArray features;
char model_id[48];
bool cache_info_passthrough;
-} x86_def_t;
+} X86CPUDefinition;
#define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
#define PENTIUM_FEATURES (I486_FEATURES | CPUID_DE | CPUID_TSC | \
@@ -549,7 +549,7 @@ typedef struct x86_def_t {
/* built-in CPU model definitions
*/
-static x86_def_t builtin_x86_defs[] = {
+static X86CPUDefinition builtin_x86_defs[] = {
{
.name = "qemu64",
.level = 4,
@@ -1108,7 +1108,7 @@ static x86_def_t builtin_x86_defs[] = {
void x86_cpu_compat_set_features(const char *cpu_model, FeatureWord w,
uint32_t feat_add, uint32_t feat_remove)
{
- x86_def_t *def;
+ X86CPUDefinition *def;
int i;
for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); i++) {
def = &builtin_x86_defs[i];
@@ -1134,12 +1134,12 @@ static int cpu_x86_fill_model_id(char *str)
return 0;
}
-/* Fill a x86_def_t struct with information about the host CPU, and
+/* Fill a X86CPUDefinition struct with information about the host CPU, and
* the CPU features supported by the host hardware + host kernel
*
* This function may be called only if KVM is enabled.
*/
-static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
+static void kvm_cpu_fill_host(X86CPUDefinition *x86_cpu_def)
{
KVMState *s = kvm_state;
uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
@@ -1582,10 +1582,10 @@ static PropertyInfo qdev_prop_spinlocks = {
.set = x86_set_hv_spinlocks,
};
-static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def,
+static int cpu_x86_find_by_name(X86CPU *cpu, X86CPUDefinition *x86_cpu_def,
const char *name)
{
- x86_def_t *def;
+ X86CPUDefinition *def;
int i;
if (name == NULL) {
@@ -1753,7 +1753,7 @@ static void listflags(char *buf, int bufsize, uint32_t fbits,
/* generate CPU information. */
void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf)
{
- x86_def_t *def;
+ X86CPUDefinition *def;
char buf[256];
int i;
@@ -1780,7 +1780,7 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf)
CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
{
CpuDefinitionInfoList *cpu_list = NULL;
- x86_def_t *def;
+ X86CPUDefinition *def;
int i;
for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); i++) {
@@ -1822,7 +1822,7 @@ static void filter_features_for_kvm(X86CPU *cpu)
static void x86_cpu_load_def(X86CPU *cpu, const char *name, Error **errp)
{
CPUX86State *env = &cpu->env;
- x86_def_t def1, *def = &def1;
+ X86CPUDefinition def1, *def = &def1;
memset(def, 0, sizeof(*def));
@@ -1849,7 +1849,7 @@ static void x86_cpu_load_def(X86CPU *cpu, const char *name, Error **errp)
object_property_set_str(OBJECT(cpu), def->model_id, "model-id", errp);
- /* Special cases not set in the x86_def_t structs: */
+ /* Special cases not set in the X86CPUDefinition structs: */
if (kvm_enabled()) {
env->features[FEAT_KVM] |= kvm_default_features;
}
@@ -1971,7 +1971,7 @@ void x86_cpudef_setup(void)
static const char *model_with_versions[] = { "qemu32", "qemu64", "athlon" };
for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); ++i) {
- x86_def_t *def = &builtin_x86_defs[i];
+ X86CPUDefinition *def = &builtin_x86_defs[i];
/* Look for specific "cpudef" models that */
/* have the QEMU version in .model_id */
--
1.8.4.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [uq/master PATCH 6/7] target-i386: Rename x86_def_t to X86CPUDefinition
2014-01-30 19:48 ` [Qemu-devel] [uq/master PATCH 6/7] target-i386: Rename x86_def_t to X86CPUDefinition Eduardo Habkost
@ 2014-01-31 11:42 ` Paolo Bonzini
2014-02-10 0:14 ` Andreas Färber
0 siblings, 1 reply; 45+ messages in thread
From: Paolo Bonzini @ 2014-01-31 11:42 UTC (permalink / raw)
To: Eduardo Habkost, qemu-devel
Cc: libvir-list, Igor Mammedov, Jiri Denemark, Andreas Färber,
kvm
Il 30/01/2014 20:48, Eduardo Habkost ha scritto:
> As the new X86CPU subclass code is going to change lots of the code
> invoving x86_def_t, let's rename the struct to match coding style first.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> target-i386/cpu.c | 26 +++++++++++++-------------
> 1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 5c13ed6..6659527 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -484,7 +484,7 @@ static void add_flagname_to_bitmaps(const char *flagname,
> }
> }
>
> -typedef struct x86_def_t {
> +typedef struct X86CPUDefinition {
> const char *name;
> uint32_t level;
> uint32_t xlevel;
> @@ -497,7 +497,7 @@ typedef struct x86_def_t {
> FeatureWordArray features;
> char model_id[48];
> bool cache_info_passthrough;
> -} x86_def_t;
> +} X86CPUDefinition;
>
> #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
> #define PENTIUM_FEATURES (I486_FEATURES | CPUID_DE | CPUID_TSC | \
> @@ -549,7 +549,7 @@ typedef struct x86_def_t {
>
> /* built-in CPU model definitions
> */
> -static x86_def_t builtin_x86_defs[] = {
> +static X86CPUDefinition builtin_x86_defs[] = {
> {
> .name = "qemu64",
> .level = 4,
> @@ -1108,7 +1108,7 @@ static x86_def_t builtin_x86_defs[] = {
> void x86_cpu_compat_set_features(const char *cpu_model, FeatureWord w,
> uint32_t feat_add, uint32_t feat_remove)
> {
> - x86_def_t *def;
> + X86CPUDefinition *def;
> int i;
> for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); i++) {
> def = &builtin_x86_defs[i];
> @@ -1134,12 +1134,12 @@ static int cpu_x86_fill_model_id(char *str)
> return 0;
> }
>
> -/* Fill a x86_def_t struct with information about the host CPU, and
> +/* Fill a X86CPUDefinition struct with information about the host CPU, and
> * the CPU features supported by the host hardware + host kernel
> *
> * This function may be called only if KVM is enabled.
> */
> -static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
> +static void kvm_cpu_fill_host(X86CPUDefinition *x86_cpu_def)
> {
> KVMState *s = kvm_state;
> uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
> @@ -1582,10 +1582,10 @@ static PropertyInfo qdev_prop_spinlocks = {
> .set = x86_set_hv_spinlocks,
> };
>
> -static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def,
> +static int cpu_x86_find_by_name(X86CPU *cpu, X86CPUDefinition *x86_cpu_def,
> const char *name)
> {
> - x86_def_t *def;
> + X86CPUDefinition *def;
> int i;
>
> if (name == NULL) {
> @@ -1753,7 +1753,7 @@ static void listflags(char *buf, int bufsize, uint32_t fbits,
> /* generate CPU information. */
> void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf)
> {
> - x86_def_t *def;
> + X86CPUDefinition *def;
> char buf[256];
> int i;
>
> @@ -1780,7 +1780,7 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf)
> CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
> {
> CpuDefinitionInfoList *cpu_list = NULL;
> - x86_def_t *def;
> + X86CPUDefinition *def;
> int i;
>
> for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); i++) {
> @@ -1822,7 +1822,7 @@ static void filter_features_for_kvm(X86CPU *cpu)
> static void x86_cpu_load_def(X86CPU *cpu, const char *name, Error **errp)
> {
> CPUX86State *env = &cpu->env;
> - x86_def_t def1, *def = &def1;
> + X86CPUDefinition def1, *def = &def1;
>
> memset(def, 0, sizeof(*def));
>
> @@ -1849,7 +1849,7 @@ static void x86_cpu_load_def(X86CPU *cpu, const char *name, Error **errp)
>
> object_property_set_str(OBJECT(cpu), def->model_id, "model-id", errp);
>
> - /* Special cases not set in the x86_def_t structs: */
> + /* Special cases not set in the X86CPUDefinition structs: */
> if (kvm_enabled()) {
> env->features[FEAT_KVM] |= kvm_default_features;
> }
> @@ -1971,7 +1971,7 @@ void x86_cpudef_setup(void)
> static const char *model_with_versions[] = { "qemu32", "qemu64", "athlon" };
>
> for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); ++i) {
> - x86_def_t *def = &builtin_x86_defs[i];
> + X86CPUDefinition *def = &builtin_x86_defs[i];
>
> /* Look for specific "cpudef" models that */
> /* have the QEMU version in .model_id */
>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [uq/master PATCH 6/7] target-i386: Rename x86_def_t to X86CPUDefinition
2014-01-31 11:42 ` Paolo Bonzini
@ 2014-02-10 0:14 ` Andreas Färber
0 siblings, 0 replies; 45+ messages in thread
From: Andreas Färber @ 2014-02-10 0:14 UTC (permalink / raw)
To: Paolo Bonzini, Eduardo Habkost, qemu-devel
Cc: libvir-list, Igor Mammedov, Jiri Denemark, kvm
Am 31.01.2014 12:42, schrieb Paolo Bonzini:
> Il 30/01/2014 20:48, Eduardo Habkost ha scritto:
>> As the new X86CPU subclass code is going to change lots of the code
>> invoving x86_def_t, let's rename the struct to match coding style first.
>>
>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
[...]
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Thanks, applied to qom-cpu:
https://github.com/afaerber/qemu-cpu/commits/qom-cpu
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 45+ messages in thread
* [Qemu-devel] [uq/master PATCH 7/7] target-i386: CPU model subclasses
2014-01-30 19:48 [Qemu-devel] [uq/master PATCH 0/7] x86 CPU subclasses, take 7 Eduardo Habkost
` (5 preceding siblings ...)
2014-01-30 19:48 ` [Qemu-devel] [uq/master PATCH 6/7] target-i386: Rename x86_def_t to X86CPUDefinition Eduardo Habkost
@ 2014-01-30 19:48 ` Eduardo Habkost
2014-01-31 17:20 ` Eduardo Habkost
2014-01-31 18:13 ` [Qemu-devel] [uq/master PATCH 7/7 v8] " Eduardo Habkost
2014-01-30 21:47 ` [Qemu-devel] [uq/master PATCH 0/7] x86 CPU subclasses, take 7 Paolo Bonzini
2014-01-31 14:48 ` Igor Mammedov
8 siblings, 2 replies; 45+ messages in thread
From: Eduardo Habkost @ 2014-01-30 19:48 UTC (permalink / raw)
To: qemu-devel
Cc: kvm, libvir-list, Igor Mammedov, Paolo Bonzini, Jiri Denemark,
Andreas Färber
Register separate QOM classes for each x86 CPU model.
This will allow management code to more easily probe what each CPU model
provides, by simply creating objects using the appropriate class name,
without having to restart QEMU.
This also allows us to eliminate the qdev_prop_set_globals_for_type()
hack to set CPU-model-specific global properties.
Instead of creating separate class_init functions for each class, I just
used class_dat to store a pointer to the X86CPUDefinition struct for
each CPU model. This should make the patch shorter and easier to review.
Later we can gradually convert each X86CPUDefinition field to lists of
per-class property defaults.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
This version is closer to the version sent by Andrea and then later
resubmitted by Igor as "[RFC v5] target-i386: Slim conversion to X86CPU
subclasses + KVM subclasses", as it doesn't create one new class_init
function for each subclass. One main difference is that this version
does not use KVM-specific subclasses, to keep things simpler.
---
target-i386/cpu-qom.h | 13 ++
target-i386/cpu.c | 334 +++++++++++++++++++++++++++++++-------------------
target-i386/cpu.h | 2 -
3 files changed, 222 insertions(+), 127 deletions(-)
diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index 722f11a..60c5c32 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -37,6 +37,9 @@
#define X86_CPU_GET_CLASS(obj) \
OBJECT_GET_CLASS(X86CPUClass, (obj), TYPE_X86_CPU)
+
+typedef struct X86CPUDefinition X86CPUDefinition;
+
/**
* X86CPUClass:
* @parent_realize: The parent class' realize handler.
@@ -49,6 +52,16 @@ typedef struct X86CPUClass {
CPUClass parent_class;
/*< public >*/
+ /* CPU model definition
+ * Should be eventually replaced by subclass-specific property defaults
+ */
+ X86CPUDefinition *cpu_def;
+ /* CPU model requires KVM to be enabled */
+ bool kvm_required;
+ /* Optional description of CPU model.
+ * If unavailable, cpu_def->model_id is used */
+ const char *model_description;
+
DeviceRealize parent_realize;
void (*parent_reset)(CPUState *cpu);
} X86CPUClass;
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 6659527..4d88dfd 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -484,7 +484,10 @@ static void add_flagname_to_bitmaps(const char *flagname,
}
}
-typedef struct X86CPUDefinition {
+/* CPU model definition data that was not converted to QOM per-subclass
+ * property defaults yet.
+ */
+struct X86CPUDefinition {
const char *name;
uint32_t level;
uint32_t xlevel;
@@ -497,7 +500,7 @@ typedef struct X86CPUDefinition {
FeatureWordArray features;
char model_id[48];
bool cache_info_passthrough;
-} X86CPUDefinition;
+};
#define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
#define PENTIUM_FEATURES (I486_FEATURES | CPUID_DE | CPUID_TSC | \
@@ -547,8 +550,41 @@ typedef struct X86CPUDefinition {
CPUID_7_0_EBX_ERMS, CPUID_7_0_EBX_INVPCID, CPUID_7_0_EBX_RTM,
CPUID_7_0_EBX_RDSEED */
-/* built-in CPU model definitions
+/* CPU class name definitions: */
+
+#define X86_CPU_CLASS_SUFFIX "-" TYPE_X86_CPU
+#define CPU_CLASS_NAME(name) (name X86_CPU_CLASS_SUFFIX)
+
+
+static char *x86_cpu_class_get_model_name(X86CPUClass *cc)
+{
+ const char *class_name = object_class_get_name(OBJECT_CLASS(cc));
+ if (g_str_has_suffix(class_name, X86_CPU_CLASS_SUFFIX)) {
+ return g_strndup(class_name,
+ strlen(class_name) - strlen(X86_CPU_CLASS_SUFFIX));
+ } else {
+ return g_strdup(class_name);
+ }
+}
+
+/* Return class name for a given CPU model name
+ * Caller is responsible for freeing the returned string.
*/
+static char *x86_cpu_class_name(const char *model_name)
+{
+ return g_strdup_printf(CPU_CLASS_NAME("%s"), model_name);
+}
+
+/* Return X86CPUClass for a CPU model name */
+static X86CPUClass *x86_cpu_class_by_name(const char *name)
+{
+ X86CPUClass *cc;
+ char *class_name = x86_cpu_class_name(name);
+ cc = X86_CPU_CLASS(object_class_by_name(class_name));
+ g_free(class_name);
+ return cc;
+}
+
static X86CPUDefinition builtin_x86_defs[] = {
{
.name = "qemu64",
@@ -1093,6 +1129,33 @@ static X86CPUDefinition builtin_x86_defs[] = {
},
};
+static void x86_cpu_class_init_cpudef(ObjectClass *oc, void *data)
+{
+ X86CPUDefinition *cpudef = data;
+ X86CPUClass *xcc = X86_CPU_CLASS(oc);
+ xcc->cpu_def = cpudef;
+}
+
+static void x86_register_cpudef_classes(void)
+{
+ int i;
+ for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); i++) {
+ X86CPUDefinition *def = &builtin_x86_defs[i];
+ char *class_name = x86_cpu_class_name(def->name);
+ TypeInfo ti = {
+ .name = class_name,
+ .parent = TYPE_X86_CPU,
+ .instance_size = sizeof(X86CPU),
+ .abstract = false,
+ .class_size = sizeof(X86CPUClass),
+ .class_init = x86_cpu_class_init_cpudef,
+ .class_data = def,
+ };
+ type_register(&ti);
+ g_free(class_name);
+ }
+}
+
/**
* x86_cpu_compat_set_features:
* @cpu_model: CPU model name to be changed. If NULL, all CPU models are changed
@@ -1134,44 +1197,74 @@ static int cpu_x86_fill_model_id(char *str)
return 0;
}
-/* Fill a X86CPUDefinition struct with information about the host CPU, and
- * the CPU features supported by the host hardware + host kernel
+static X86CPUDefinition host_cpudef;
+
+/* class_init for the "host" CPU model
*
- * This function may be called only if KVM is enabled.
+ * This function may be called before KVM is initialized.
*/
-static void kvm_cpu_fill_host(X86CPUDefinition *x86_cpu_def)
+static void x86_cpu_class_init_host(ObjectClass *oc, void *data)
{
- KVMState *s = kvm_state;
+ X86CPUClass *xcc = X86_CPU_CLASS(oc);
uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
- assert(kvm_enabled());
+ xcc->kvm_required = true;
- x86_cpu_def->name = "host";
- x86_cpu_def->cache_info_passthrough = true;
host_cpuid(0x0, 0, &eax, &ebx, &ecx, &edx);
- x86_cpu_vendor_words2str(x86_cpu_def->vendor, ebx, edx, ecx);
+ x86_cpu_vendor_words2str(host_cpudef.vendor, ebx, edx, ecx);
host_cpuid(0x1, 0, &eax, &ebx, &ecx, &edx);
- x86_cpu_def->family = ((eax >> 8) & 0x0F) + ((eax >> 20) & 0xFF);
- x86_cpu_def->model = ((eax >> 4) & 0x0F) | ((eax & 0xF0000) >> 12);
- x86_cpu_def->stepping = eax & 0x0F;
+ host_cpudef.family = ((eax >> 8) & 0x0F) + ((eax >> 20) & 0xFF);
+ host_cpudef.model = ((eax >> 4) & 0x0F) | ((eax & 0xF0000) >> 12);
+ host_cpudef.stepping = eax & 0x0F;
- x86_cpu_def->level = kvm_arch_get_supported_cpuid(s, 0x0, 0, R_EAX);
- x86_cpu_def->xlevel = kvm_arch_get_supported_cpuid(s, 0x80000000, 0, R_EAX);
- x86_cpu_def->xlevel2 =
- kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX);
+ cpu_x86_fill_model_id(host_cpudef.model_id);
- cpu_x86_fill_model_id(x86_cpu_def->model_id);
+ xcc->cpu_def = &host_cpudef;
+ xcc->model_description =
+ "KVM processor with all supported host features "
+ "(only available in KVM mode)";
+
+ host_cpudef.cache_info_passthrough = true;
+
+ /* level, xlevel, xlevel2, and the feature words are initialized on
+ * instance_init, because they require KVM to be initialized.
+ */
+}
+
+static void x86_cpu_instance_init_host(Object *obj)
+{
+ X86CPU *cpu = X86_CPU(obj);
+ CPUX86State *env = &cpu->env;
+ KVMState *s = kvm_state;
+
+ assert(kvm_enabled());
+
+ env->cpuid_level = kvm_arch_get_supported_cpuid(s, 0x0, 0, R_EAX);
+ env->cpuid_xlevel = kvm_arch_get_supported_cpuid(s, 0x80000000, 0, R_EAX);
+ env->cpuid_xlevel2 = kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX);
FeatureWord w;
for (w = 0; w < FEATURE_WORDS; w++) {
FeatureWordInfo *wi = &feature_word_info[w];
- x86_cpu_def->features[w] =
+ env->features[w] =
kvm_arch_get_supported_cpuid(s, wi->cpuid_eax, wi->cpuid_ecx,
wi->cpuid_reg);
}
+ object_property_set_bool(OBJECT(cpu), true, "pmu", &error_abort);
}
+
+static const TypeInfo x86_cpu_host_type_info = {
+ .name = CPU_CLASS_NAME("host"),
+ .parent = TYPE_X86_CPU,
+ .instance_size = sizeof(X86CPU),
+ .instance_init = x86_cpu_instance_init_host,
+ .abstract = false,
+ .class_size = sizeof(X86CPUClass),
+ .class_init = x86_cpu_class_init_host,
+};
+
static int unavailable_host_feature(FeatureWordInfo *f, uint32_t mask)
{
int i;
@@ -1582,32 +1675,6 @@ static PropertyInfo qdev_prop_spinlocks = {
.set = x86_set_hv_spinlocks,
};
-static int cpu_x86_find_by_name(X86CPU *cpu, X86CPUDefinition *x86_cpu_def,
- const char *name)
-{
- X86CPUDefinition *def;
- int i;
-
- if (name == NULL) {
- return -1;
- }
- if (kvm_enabled() && strcmp(name, "host") == 0) {
- kvm_cpu_fill_host(x86_cpu_def);
- object_property_set_bool(OBJECT(cpu), true, "pmu", &error_abort);
- return 0;
- }
-
- for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); i++) {
- def = &builtin_x86_defs[i];
- if (strcmp(name, def->name) == 0) {
- memcpy(x86_cpu_def, def, sizeof(*def));
- return 0;
- }
- }
-
- return -1;
-}
-
/* Convert all '_' in a feature string option name to '-', to make feature
* name conform to QOM property naming rule, which uses '-' instead of '_'.
*/
@@ -1750,23 +1817,63 @@ static void listflags(char *buf, int bufsize, uint32_t fbits,
}
}
-/* generate CPU information. */
+/* Sort alphabetically by type name, listing kvm_required models last. */
+static gint x86_cpu_list_compare(gconstpointer a, gconstpointer b)
+{
+ ObjectClass *class_a = (ObjectClass *)a;
+ ObjectClass *class_b = (ObjectClass *)b;
+ X86CPUClass *cc_a = X86_CPU_CLASS(class_a);
+ X86CPUClass *cc_b = X86_CPU_CLASS(class_b);
+ const char *name_a, *name_b;
+
+ if (cc_a->kvm_required != cc_b->kvm_required) {
+ /* kvm_required items go last */
+ return cc_a->kvm_required ? 1 : -1;
+ } else {
+ name_a = object_class_get_name(class_a);
+ name_b = object_class_get_name(class_b);
+ return strcmp(name_a, name_b);
+ }
+}
+
+static GSList *get_sorted_cpu_model_list(void)
+{
+ GSList *list = object_class_get_list(TYPE_X86_CPU, false);
+ list = g_slist_sort(list, x86_cpu_list_compare);
+ return list;
+}
+
+static void x86_cpu_list_entry(gpointer data, gpointer user_data)
+{
+ ObjectClass *oc = data;
+ X86CPUClass *cc = X86_CPU_CLASS(oc);
+ CPUListState *s = user_data;
+ char *name = x86_cpu_class_get_model_name(cc);
+ const char *desc = cc->model_description;
+ if (!desc) {
+ desc = cc->cpu_def->model_id;
+ }
+
+ (*s->cpu_fprintf)(s->file, "x86 %16s %-48s\n",
+ name, desc);
+ g_free(name);
+}
+
+/* list available CPU models and flags */
void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf)
{
- X86CPUDefinition *def;
- char buf[256];
int i;
+ CPUListState s = {
+ .file = f,
+ .cpu_fprintf = cpu_fprintf,
+ };
+ GSList *list;
+ char buf[256];
- for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); i++) {
- def = &builtin_x86_defs[i];
- snprintf(buf, sizeof(buf), "%s", def->name);
- (*cpu_fprintf)(f, "x86 %16s %-48s\n", buf, def->model_id);
- }
-#ifdef CONFIG_KVM
- (*cpu_fprintf)(f, "x86 %16s %-48s\n", "host",
- "KVM processor with all supported host features "
- "(only available in KVM mode)");
-#endif
+ (*cpu_fprintf)(f, "Available CPUs:\n");
+ list = get_sorted_cpu_model_list();
+ g_slist_foreach(list, x86_cpu_list_entry, &s);
+ g_slist_free(list);
(*cpu_fprintf)(f, "\nRecognized CPUID flags:\n");
for (i = 0; i < ARRAY_SIZE(feature_word_info); i++) {
@@ -1777,26 +1884,29 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf)
}
}
-CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
+static void x86_cpu_definition_entry(gpointer data, gpointer user_data)
{
- CpuDefinitionInfoList *cpu_list = NULL;
- X86CPUDefinition *def;
- int i;
-
- for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); i++) {
- CpuDefinitionInfoList *entry;
- CpuDefinitionInfo *info;
+ ObjectClass *oc = data;
+ X86CPUClass *cc = X86_CPU_CLASS(oc);
+ CpuDefinitionInfoList **cpu_list = user_data;
+ CpuDefinitionInfoList *entry;
+ CpuDefinitionInfo *info;
- def = &builtin_x86_defs[i];
- info = g_malloc0(sizeof(*info));
- info->name = g_strdup(def->name);
+ info = g_malloc0(sizeof(*info));
+ info->name = x86_cpu_class_get_model_name(cc);
- entry = g_malloc0(sizeof(*entry));
- entry->value = info;
- entry->next = cpu_list;
- cpu_list = entry;
- }
+ entry = g_malloc0(sizeof(*entry));
+ entry->value = info;
+ entry->next = *cpu_list;
+ *cpu_list = entry;
+}
+CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
+{
+ CpuDefinitionInfoList *cpu_list = NULL;
+ GSList *list = get_sorted_cpu_model_list();
+ g_slist_foreach(list, x86_cpu_definition_entry, &cpu_list);
+ g_slist_free(list);
return cpu_list;
}
@@ -1817,19 +1927,11 @@ static void filter_features_for_kvm(X86CPU *cpu)
}
}
-/* Load CPU definition for a given CPU model name
+/* Load data from X86CPUDefinition
*/
-static void x86_cpu_load_def(X86CPU *cpu, const char *name, Error **errp)
+static void x86_cpu_load_def(X86CPU *cpu, X86CPUDefinition *def, Error **errp)
{
CPUX86State *env = &cpu->env;
- X86CPUDefinition def1, *def = &def1;
-
- memset(def, 0, sizeof(*def));
-
- if (cpu_x86_find_by_name(cpu, def, name) < 0) {
- error_setg(errp, "Unable to find CPU definition: %s", name);
- return;
- }
object_property_set_int(OBJECT(cpu), def->level, "level", errp);
object_property_set_int(OBJECT(cpu), def->family, "family", errp);
@@ -1881,7 +1983,6 @@ X86CPU *cpu_x86_create(const char *cpu_model, DeviceState *icc_bridge,
X86CPU *cpu = NULL;
gchar **model_pieces;
char *name, *features;
- char *typename;
Error *error = NULL;
model_pieces = g_strsplit(cpu_model, ",", 2);
@@ -1892,12 +1993,19 @@ X86CPU *cpu_x86_create(const char *cpu_model, DeviceState *icc_bridge,
name = model_pieces[0];
features = model_pieces[1];
- cpu = X86_CPU(object_new(TYPE_X86_CPU));
- x86_cpu_load_def(cpu, name, &error);
- if (error) {
+ X86CPUClass *cc = x86_cpu_class_by_name(name);
+ if (!cc) {
+ error_setg(&error, "Unable to find CPU definition: %s", name);
+ goto out;
+ }
+
+ if (cc->kvm_required && !kvm_enabled()) {
+ error_setg(&error, "CPU model '%s' requires KVM", name);
goto out;
}
+ cpu = X86_CPU(object_new(object_class_get_name(OBJECT_CLASS(cc))));
+
#ifndef CONFIG_USER_ONLY
if (icc_bridge == NULL) {
error_setg(&error, "Invalid icc-bridge value");
@@ -1907,14 +2015,6 @@ X86CPU *cpu_x86_create(const char *cpu_model, DeviceState *icc_bridge,
object_unref(OBJECT(cpu));
#endif
- /* Emulate per-model subclasses for global properties */
- typename = g_strdup_printf("%s-" TYPE_X86_CPU, name);
- qdev_prop_set_globals_for_type(DEVICE(cpu), typename, &error);
- g_free(typename);
- if (error) {
- goto out;
- }
-
cpu_x86_parse_featurestr(cpu, features, &error);
if (error) {
goto out;
@@ -1923,8 +2023,10 @@ X86CPU *cpu_x86_create(const char *cpu_model, DeviceState *icc_bridge,
out:
if (error != NULL) {
error_propagate(errp, error);
- object_unref(OBJECT(cpu));
- cpu = NULL;
+ if (cpu) {
+ object_unref(OBJECT(cpu));
+ cpu = NULL;
+ }
}
g_strfreev(model_pieces);
return cpu;
@@ -1963,30 +2065,6 @@ void cpu_clear_apic_feature(CPUX86State *env)
#endif /* !CONFIG_USER_ONLY */
-/* Initialize list of CPU models, filling some non-static fields if necessary
- */
-void x86_cpudef_setup(void)
-{
- int i, j;
- static const char *model_with_versions[] = { "qemu32", "qemu64", "athlon" };
-
- for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); ++i) {
- X86CPUDefinition *def = &builtin_x86_defs[i];
-
- /* Look for specific "cpudef" models that */
- /* have the QEMU version in .model_id */
- for (j = 0; j < ARRAY_SIZE(model_with_versions); j++) {
- if (strcmp(model_with_versions[j], def->name) == 0) {
- pstrcpy(def->model_id, sizeof(def->model_id),
- "QEMU Virtual CPU version ");
- pstrcat(def->model_id, sizeof(def->model_id),
- qemu_get_version());
- break;
- }
- }
- }
-}
-
static void get_cpuid_vendor(CPUX86State *env, uint32_t *ebx,
uint32_t *ecx, uint32_t *edx)
{
@@ -2615,6 +2693,7 @@ static void x86_cpu_initfn(Object *obj)
{
CPUState *cs = CPU(obj);
X86CPU *cpu = X86_CPU(obj);
+ X86CPUClass *xcc = X86_CPU_GET_CLASS(cpu);
CPUX86State *env = &cpu->env;
static int inited;
@@ -2666,6 +2745,9 @@ static void x86_cpu_initfn(Object *obj)
cpu_set_debug_excp_handler(breakpoint_handler);
#endif
}
+
+ X86CPUDefinition *def = xcc->cpu_def;
+ x86_cpu_load_def(cpu, def, &error_abort);
}
static int64_t x86_cpu_get_arch_id(CPUState *cs)
@@ -2748,7 +2830,7 @@ static const TypeInfo x86_cpu_type_info = {
.parent = TYPE_CPU,
.instance_size = sizeof(X86CPU),
.instance_init = x86_cpu_initfn,
- .abstract = false,
+ .abstract = true,
.class_size = sizeof(X86CPUClass),
.class_init = x86_cpu_common_class_init,
};
@@ -2756,6 +2838,8 @@ static const TypeInfo x86_cpu_type_info = {
static void x86_cpu_register_types(void)
{
type_register_static(&x86_cpu_type_info);
+ x86_register_cpudef_classes();
+ type_register_static(&x86_cpu_host_type_info);
}
type_init(x86_cpu_register_types)
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 1b94f0f..d683941 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -944,7 +944,6 @@ X86CPU *cpu_x86_create(const char *cpu_model, DeviceState *icc_bridge,
Error **errp);
int cpu_x86_exec(CPUX86State *s);
void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf);
-void x86_cpudef_setup(void);
int cpu_x86_support_mca_broadcast(CPUX86State *env);
int cpu_get_pic_interrupt(CPUX86State *s);
@@ -1137,7 +1136,6 @@ static inline CPUX86State *cpu_init(const char *cpu_model)
#define cpu_gen_code cpu_x86_gen_code
#define cpu_signal_handler cpu_x86_signal_handler
#define cpu_list x86_cpu_list
-#define cpudef_setup x86_cpudef_setup
/* MMU modes definitions */
#define MMU_MODE0_SUFFIX _kernel
--
1.8.4.2
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [uq/master PATCH 7/7] target-i386: CPU model subclasses
2014-01-30 19:48 ` [Qemu-devel] [uq/master PATCH 7/7] target-i386: CPU model subclasses Eduardo Habkost
@ 2014-01-31 17:20 ` Eduardo Habkost
2014-01-31 18:13 ` [Qemu-devel] [uq/master PATCH 7/7 v8] " Eduardo Habkost
1 sibling, 0 replies; 45+ messages in thread
From: Eduardo Habkost @ 2014-01-31 17:20 UTC (permalink / raw)
To: qemu-devel
Cc: kvm, libvir-list, Paolo Bonzini, Igor Mammedov, Jiri Denemark,
Andreas Färber
On Thu, Jan 30, 2014 at 05:48:59PM -0200, Eduardo Habkost wrote:
> Register separate QOM classes for each x86 CPU model.
>
> This will allow management code to more easily probe what each CPU model
> provides, by simply creating objects using the appropriate class name,
> without having to restart QEMU.
>
> This also allows us to eliminate the qdev_prop_set_globals_for_type()
> hack to set CPU-model-specific global properties.
>
> Instead of creating separate class_init functions for each class, I just
> used class_dat to store a pointer to the X86CPUDefinition struct for
> each CPU model. This should make the patch shorter and easier to review.
> Later we can gradually convert each X86CPUDefinition field to lists of
> per-class property defaults.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> This version is closer to the version sent by Andrea and then later
> resubmitted by Igor as "[RFC v5] target-i386: Slim conversion to X86CPU
> subclasses + KVM subclasses", as it doesn't create one new class_init
> function for each subclass. One main difference is that this version
> does not use KVM-specific subclasses, to keep things simpler.
I will submit a new version of this patch later, as I will:
* Split some changes that can be made in a separate patch, after
the conversion (the x86_cpudef_setup() removal and the CPU listing
code);
* Add proper attribution to Andreas and Igor, who wrote
"[RFC v5] target-i386: Slim conversion to X86CPU subclasses + KVM
subclasses".
--
Eduardo
^ permalink raw reply [flat|nested] 45+ messages in thread
* [Qemu-devel] [uq/master PATCH 7/7 v8] target-i386: CPU model subclasses
2014-01-30 19:48 ` [Qemu-devel] [uq/master PATCH 7/7] target-i386: CPU model subclasses Eduardo Habkost
2014-01-31 17:20 ` Eduardo Habkost
@ 2014-01-31 18:13 ` Eduardo Habkost
2014-02-10 0:23 ` Andreas Färber
1 sibling, 1 reply; 45+ messages in thread
From: Eduardo Habkost @ 2014-01-31 18:13 UTC (permalink / raw)
To: qemu-devel
Cc: kvm, libvir-list, Paolo Bonzini, Igor Mammedov, Jiri Denemark,
Andreas Färber
Register separate QOM classes for each x86 CPU model.
This will allow management code to more easily probe what each CPU model
provides, by simply creating objects using the appropriate class name,
without having to restart QEMU.
This also allows us to eliminate the qdev_prop_set_globals_for_type()
hack to set CPU-model-specific global properties.
Instead of creating separate class_init functions for each class, I just
used class_data to store a pointer to the X86CPUDefinition struct for
each CPU model. This should make the patch shorter and easier to review.
Later we can gradually convert each X86CPUDefinition field to lists of
per-class property defaults.
Written based on the ideas from the patch "[RFC v5] target-i386: Slim
conversion to X86CPU subclasses + KVM subclasses" written by Andreas
Färber <afaerber@suse.de>, Igor Mammedov <imammedo@redhat.com>.
The "host" CPU model is special, as the feature flags depend on KVM
being initialized. So it has its own class_init and instance_init
function, and feature flags are set on instance_init instead of
class_init.
Signed-off-by: Andreas Färber <afaerber@suse.de>
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
This patch is similar to the one sent by Andrea and then later
resubmitted by Igor as "[RFC v5] target-i386: Slim conversion to X86CPU
subclasses + KVM subclasses", as it doesn't create one new class_init
function for each subclass.
Main differences v5 -> v6 are:
* Code was written from scratch (instead of using the previous patches
as base)
* I didn't mean to rewrite it entirely, but when doing additional
simplification of the CPU init logic on other patches, I ended up
rewriting it.
* I chose to keep the Signed-off-by lines because I built upon
Andreas's and Igor's ideas. Is that OK?
* No KVM-specific subclasses, to keep things simpler.
* No embedding of X86CPUDefinition (x86_def_t) inside the class struct,
instead keeping a pointer to the existing X86CPUDefinition struct.
* The "host" class is registered on cpu.c, but the CPUID data
is filled on instance_init instead of class_init (because KVM has to
be initialized already).
* kvm_required field introduced to make sure the "host" class can't
be used without KVM.
Changes v6 -> v7:
* Rebase
Changes v7 -> v8:
* Removed CPU listing code (will be sent as a separate patch)
* Kept x86_cpudef_setup() (will be addressed in a separate patch)
---
target-i386/cpu-qom.h | 13 ++++
target-i386/cpu.c | 197 ++++++++++++++++++++++++++++++++------------------
2 files changed, 138 insertions(+), 72 deletions(-)
diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index 722f11a..60c5c32 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -37,6 +37,9 @@
#define X86_CPU_GET_CLASS(obj) \
OBJECT_GET_CLASS(X86CPUClass, (obj), TYPE_X86_CPU)
+
+typedef struct X86CPUDefinition X86CPUDefinition;
+
/**
* X86CPUClass:
* @parent_realize: The parent class' realize handler.
@@ -49,6 +52,16 @@ typedef struct X86CPUClass {
CPUClass parent_class;
/*< public >*/
+ /* CPU model definition
+ * Should be eventually replaced by subclass-specific property defaults
+ */
+ X86CPUDefinition *cpu_def;
+ /* CPU model requires KVM to be enabled */
+ bool kvm_required;
+ /* Optional description of CPU model.
+ * If unavailable, cpu_def->model_id is used */
+ const char *model_description;
+
DeviceRealize parent_realize;
void (*parent_reset)(CPUState *cpu);
} X86CPUClass;
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 6659527..bb72e5b 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -484,7 +484,10 @@ static void add_flagname_to_bitmaps(const char *flagname,
}
}
-typedef struct X86CPUDefinition {
+/* CPU model definition data that was not converted to QOM per-subclass
+ * property defaults yet.
+ */
+struct X86CPUDefinition {
const char *name;
uint32_t level;
uint32_t xlevel;
@@ -497,7 +500,7 @@ typedef struct X86CPUDefinition {
FeatureWordArray features;
char model_id[48];
bool cache_info_passthrough;
-} X86CPUDefinition;
+};
#define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
#define PENTIUM_FEATURES (I486_FEATURES | CPUID_DE | CPUID_TSC | \
@@ -547,8 +550,29 @@ typedef struct X86CPUDefinition {
CPUID_7_0_EBX_ERMS, CPUID_7_0_EBX_INVPCID, CPUID_7_0_EBX_RTM,
CPUID_7_0_EBX_RDSEED */
-/* built-in CPU model definitions
+/* CPU class name definitions: */
+
+#define X86_CPU_CLASS_SUFFIX "-" TYPE_X86_CPU
+#define CPU_CLASS_NAME(name) (name X86_CPU_CLASS_SUFFIX)
+
+/* Return class name for a given CPU model name
+ * Caller is responsible for freeing the returned string.
*/
+static char *x86_cpu_class_name(const char *model_name)
+{
+ return g_strdup_printf(CPU_CLASS_NAME("%s"), model_name);
+}
+
+/* Return X86CPUClass for a CPU model name */
+static X86CPUClass *x86_cpu_class_by_name(const char *name)
+{
+ X86CPUClass *cc;
+ char *class_name = x86_cpu_class_name(name);
+ cc = X86_CPU_CLASS(object_class_by_name(class_name));
+ g_free(class_name);
+ return cc;
+}
+
static X86CPUDefinition builtin_x86_defs[] = {
{
.name = "qemu64",
@@ -1093,6 +1117,33 @@ static X86CPUDefinition builtin_x86_defs[] = {
},
};
+static void x86_cpu_class_init_cpudef(ObjectClass *oc, void *data)
+{
+ X86CPUDefinition *cpudef = data;
+ X86CPUClass *xcc = X86_CPU_CLASS(oc);
+ xcc->cpu_def = cpudef;
+}
+
+static void x86_register_cpudef_classes(void)
+{
+ int i;
+ for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); i++) {
+ X86CPUDefinition *def = &builtin_x86_defs[i];
+ char *class_name = x86_cpu_class_name(def->name);
+ TypeInfo ti = {
+ .name = class_name,
+ .parent = TYPE_X86_CPU,
+ .instance_size = sizeof(X86CPU),
+ .abstract = false,
+ .class_size = sizeof(X86CPUClass),
+ .class_init = x86_cpu_class_init_cpudef,
+ .class_data = def,
+ };
+ type_register(&ti);
+ g_free(class_name);
+ }
+}
+
/**
* x86_cpu_compat_set_features:
* @cpu_model: CPU model name to be changed. If NULL, all CPU models are changed
@@ -1134,44 +1185,74 @@ static int cpu_x86_fill_model_id(char *str)
return 0;
}
-/* Fill a X86CPUDefinition struct with information about the host CPU, and
- * the CPU features supported by the host hardware + host kernel
+static X86CPUDefinition host_cpudef;
+
+/* class_init for the "host" CPU model
*
- * This function may be called only if KVM is enabled.
+ * This function may be called before KVM is initialized.
*/
-static void kvm_cpu_fill_host(X86CPUDefinition *x86_cpu_def)
+static void x86_cpu_class_init_host(ObjectClass *oc, void *data)
{
- KVMState *s = kvm_state;
+ X86CPUClass *xcc = X86_CPU_CLASS(oc);
uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
- assert(kvm_enabled());
+ xcc->kvm_required = true;
- x86_cpu_def->name = "host";
- x86_cpu_def->cache_info_passthrough = true;
host_cpuid(0x0, 0, &eax, &ebx, &ecx, &edx);
- x86_cpu_vendor_words2str(x86_cpu_def->vendor, ebx, edx, ecx);
+ x86_cpu_vendor_words2str(host_cpudef.vendor, ebx, edx, ecx);
host_cpuid(0x1, 0, &eax, &ebx, &ecx, &edx);
- x86_cpu_def->family = ((eax >> 8) & 0x0F) + ((eax >> 20) & 0xFF);
- x86_cpu_def->model = ((eax >> 4) & 0x0F) | ((eax & 0xF0000) >> 12);
- x86_cpu_def->stepping = eax & 0x0F;
+ host_cpudef.family = ((eax >> 8) & 0x0F) + ((eax >> 20) & 0xFF);
+ host_cpudef.model = ((eax >> 4) & 0x0F) | ((eax & 0xF0000) >> 12);
+ host_cpudef.stepping = eax & 0x0F;
+
+ cpu_x86_fill_model_id(host_cpudef.model_id);
+
+ xcc->cpu_def = &host_cpudef;
+ xcc->model_description =
+ "KVM processor with all supported host features "
+ "(only available in KVM mode)";
+
+ host_cpudef.cache_info_passthrough = true;
- x86_cpu_def->level = kvm_arch_get_supported_cpuid(s, 0x0, 0, R_EAX);
- x86_cpu_def->xlevel = kvm_arch_get_supported_cpuid(s, 0x80000000, 0, R_EAX);
- x86_cpu_def->xlevel2 =
- kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX);
+ /* level, xlevel, xlevel2, and the feature words are initialized on
+ * instance_init, because they require KVM to be initialized.
+ */
+}
+
+static void x86_cpu_instance_init_host(Object *obj)
+{
+ X86CPU *cpu = X86_CPU(obj);
+ CPUX86State *env = &cpu->env;
+ KVMState *s = kvm_state;
+
+ assert(kvm_enabled());
- cpu_x86_fill_model_id(x86_cpu_def->model_id);
+ env->cpuid_level = kvm_arch_get_supported_cpuid(s, 0x0, 0, R_EAX);
+ env->cpuid_xlevel = kvm_arch_get_supported_cpuid(s, 0x80000000, 0, R_EAX);
+ env->cpuid_xlevel2 = kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX);
FeatureWord w;
for (w = 0; w < FEATURE_WORDS; w++) {
FeatureWordInfo *wi = &feature_word_info[w];
- x86_cpu_def->features[w] =
+ env->features[w] =
kvm_arch_get_supported_cpuid(s, wi->cpuid_eax, wi->cpuid_ecx,
wi->cpuid_reg);
}
+ object_property_set_bool(OBJECT(cpu), true, "pmu", &error_abort);
}
+
+static const TypeInfo x86_cpu_host_type_info = {
+ .name = CPU_CLASS_NAME("host"),
+ .parent = TYPE_X86_CPU,
+ .instance_size = sizeof(X86CPU),
+ .instance_init = x86_cpu_instance_init_host,
+ .abstract = false,
+ .class_size = sizeof(X86CPUClass),
+ .class_init = x86_cpu_class_init_host,
+};
+
static int unavailable_host_feature(FeatureWordInfo *f, uint32_t mask)
{
int i;
@@ -1582,32 +1663,6 @@ static PropertyInfo qdev_prop_spinlocks = {
.set = x86_set_hv_spinlocks,
};
-static int cpu_x86_find_by_name(X86CPU *cpu, X86CPUDefinition *x86_cpu_def,
- const char *name)
-{
- X86CPUDefinition *def;
- int i;
-
- if (name == NULL) {
- return -1;
- }
- if (kvm_enabled() && strcmp(name, "host") == 0) {
- kvm_cpu_fill_host(x86_cpu_def);
- object_property_set_bool(OBJECT(cpu), true, "pmu", &error_abort);
- return 0;
- }
-
- for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); i++) {
- def = &builtin_x86_defs[i];
- if (strcmp(name, def->name) == 0) {
- memcpy(x86_cpu_def, def, sizeof(*def));
- return 0;
- }
- }
-
- return -1;
-}
-
/* Convert all '_' in a feature string option name to '-', to make feature
* name conform to QOM property naming rule, which uses '-' instead of '_'.
*/
@@ -1817,19 +1872,11 @@ static void filter_features_for_kvm(X86CPU *cpu)
}
}
-/* Load CPU definition for a given CPU model name
+/* Load data from X86CPUDefinition
*/
-static void x86_cpu_load_def(X86CPU *cpu, const char *name, Error **errp)
+static void x86_cpu_load_def(X86CPU *cpu, X86CPUDefinition *def, Error **errp)
{
CPUX86State *env = &cpu->env;
- X86CPUDefinition def1, *def = &def1;
-
- memset(def, 0, sizeof(*def));
-
- if (cpu_x86_find_by_name(cpu, def, name) < 0) {
- error_setg(errp, "Unable to find CPU definition: %s", name);
- return;
- }
object_property_set_int(OBJECT(cpu), def->level, "level", errp);
object_property_set_int(OBJECT(cpu), def->family, "family", errp);
@@ -1881,7 +1928,6 @@ X86CPU *cpu_x86_create(const char *cpu_model, DeviceState *icc_bridge,
X86CPU *cpu = NULL;
gchar **model_pieces;
char *name, *features;
- char *typename;
Error *error = NULL;
model_pieces = g_strsplit(cpu_model, ",", 2);
@@ -1892,12 +1938,19 @@ X86CPU *cpu_x86_create(const char *cpu_model, DeviceState *icc_bridge,
name = model_pieces[0];
features = model_pieces[1];
- cpu = X86_CPU(object_new(TYPE_X86_CPU));
- x86_cpu_load_def(cpu, name, &error);
- if (error) {
+ X86CPUClass *cc = x86_cpu_class_by_name(name);
+ if (!cc) {
+ error_setg(&error, "Unable to find CPU definition: %s", name);
+ goto out;
+ }
+
+ if (cc->kvm_required && !kvm_enabled()) {
+ error_setg(&error, "CPU model '%s' requires KVM", name);
goto out;
}
+ cpu = X86_CPU(object_new(object_class_get_name(OBJECT_CLASS(cc))));
+
#ifndef CONFIG_USER_ONLY
if (icc_bridge == NULL) {
error_setg(&error, "Invalid icc-bridge value");
@@ -1907,14 +1960,6 @@ X86CPU *cpu_x86_create(const char *cpu_model, DeviceState *icc_bridge,
object_unref(OBJECT(cpu));
#endif
- /* Emulate per-model subclasses for global properties */
- typename = g_strdup_printf("%s-" TYPE_X86_CPU, name);
- qdev_prop_set_globals_for_type(DEVICE(cpu), typename, &error);
- g_free(typename);
- if (error) {
- goto out;
- }
-
cpu_x86_parse_featurestr(cpu, features, &error);
if (error) {
goto out;
@@ -1923,8 +1968,10 @@ X86CPU *cpu_x86_create(const char *cpu_model, DeviceState *icc_bridge,
out:
if (error != NULL) {
error_propagate(errp, error);
- object_unref(OBJECT(cpu));
- cpu = NULL;
+ if (cpu) {
+ object_unref(OBJECT(cpu));
+ cpu = NULL;
+ }
}
g_strfreev(model_pieces);
return cpu;
@@ -2615,6 +2662,7 @@ static void x86_cpu_initfn(Object *obj)
{
CPUState *cs = CPU(obj);
X86CPU *cpu = X86_CPU(obj);
+ X86CPUClass *xcc = X86_CPU_GET_CLASS(cpu);
CPUX86State *env = &cpu->env;
static int inited;
@@ -2666,6 +2714,9 @@ static void x86_cpu_initfn(Object *obj)
cpu_set_debug_excp_handler(breakpoint_handler);
#endif
}
+
+ X86CPUDefinition *def = xcc->cpu_def;
+ x86_cpu_load_def(cpu, def, &error_abort);
}
static int64_t x86_cpu_get_arch_id(CPUState *cs)
@@ -2748,7 +2799,7 @@ static const TypeInfo x86_cpu_type_info = {
.parent = TYPE_CPU,
.instance_size = sizeof(X86CPU),
.instance_init = x86_cpu_initfn,
- .abstract = false,
+ .abstract = true,
.class_size = sizeof(X86CPUClass),
.class_init = x86_cpu_common_class_init,
};
@@ -2756,6 +2807,8 @@ static const TypeInfo x86_cpu_type_info = {
static void x86_cpu_register_types(void)
{
type_register_static(&x86_cpu_type_info);
+ x86_register_cpudef_classes();
+ type_register_static(&x86_cpu_host_type_info);
}
type_init(x86_cpu_register_types)
--
1.8.4.2
--
Eduardo
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [uq/master PATCH 7/7 v8] target-i386: CPU model subclasses
2014-01-31 18:13 ` [Qemu-devel] [uq/master PATCH 7/7 v8] " Eduardo Habkost
@ 2014-02-10 0:23 ` Andreas Färber
2014-02-10 8:19 ` Eduardo Habkost
2014-02-10 9:48 ` [Qemu-devel] [uq/master PATCH 7/7 v8] " Igor Mammedov
0 siblings, 2 replies; 45+ messages in thread
From: Andreas Färber @ 2014-02-10 0:23 UTC (permalink / raw)
To: Eduardo Habkost, qemu-devel
Cc: libvir-list, Paolo Bonzini, Jiri Denemark, kvm, Igor Mammedov
Am 31.01.2014 19:13, schrieb Eduardo Habkost:
> Register separate QOM classes for each x86 CPU model.
>
> This will allow management code to more easily probe what each CPU model
> provides, by simply creating objects using the appropriate class name,
> without having to restart QEMU.
>
> This also allows us to eliminate the qdev_prop_set_globals_for_type()
> hack to set CPU-model-specific global properties.
>
> Instead of creating separate class_init functions for each class, I just
> used class_data to store a pointer to the X86CPUDefinition struct for
> each CPU model. This should make the patch shorter and easier to review.
> Later we can gradually convert each X86CPUDefinition field to lists of
> per-class property defaults.
>
> Written based on the ideas from the patch "[RFC v5] target-i386: Slim
> conversion to X86CPU subclasses + KVM subclasses" written by Andreas
> Färber <afaerber@suse.de>, Igor Mammedov <imammedo@redhat.com>.
>
> The "host" CPU model is special, as the feature flags depend on KVM
> being initialized. So it has its own class_init and instance_init
> function, and feature flags are set on instance_init instead of
> class_init.
>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> This patch is similar to the one sent by Andrea and then later
> resubmitted by Igor as "[RFC v5] target-i386: Slim conversion to X86CPU
> subclasses + KVM subclasses", as it doesn't create one new class_init
> function for each subclass.
>
> Main differences v5 -> v6 are:
> * Code was written from scratch (instead of using the previous patches
> as base)
> * I didn't mean to rewrite it entirely, but when doing additional
> simplification of the CPU init logic on other patches, I ended up
> rewriting it.
> * I chose to keep the Signed-off-by lines because I built upon
> Andreas's and Igor's ideas. Is that OK?
Yes, your From and our Sobs in order is the expected way in this case.
If Igor agrees I would propose to drop the textual repetition of this.
I am ~1/3 through reviewing this and it looks pretty promising so far!
Thanks a lot for your efforts. Meanwhile one cleanup idea inline...
> * No KVM-specific subclasses, to keep things simpler.
> * No embedding of X86CPUDefinition (x86_def_t) inside the class struct,
> instead keeping a pointer to the existing X86CPUDefinition struct.
> * The "host" class is registered on cpu.c, but the CPUID data
> is filled on instance_init instead of class_init (because KVM has to
> be initialized already).
> * kvm_required field introduced to make sure the "host" class can't
> be used without KVM.
>
> Changes v6 -> v7:
> * Rebase
>
> Changes v7 -> v8:
> * Removed CPU listing code (will be sent as a separate patch)
> * Kept x86_cpudef_setup() (will be addressed in a separate patch)
> ---
> target-i386/cpu-qom.h | 13 ++++
> target-i386/cpu.c | 197 ++++++++++++++++++++++++++++++++------------------
> 2 files changed, 138 insertions(+), 72 deletions(-)
>
> diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
> index 722f11a..60c5c32 100644
> --- a/target-i386/cpu-qom.h
> +++ b/target-i386/cpu-qom.h
> @@ -37,6 +37,9 @@
> #define X86_CPU_GET_CLASS(obj) \
> OBJECT_GET_CLASS(X86CPUClass, (obj), TYPE_X86_CPU)
>
> +
> +typedef struct X86CPUDefinition X86CPUDefinition;
> +
> /**
> * X86CPUClass:
> * @parent_realize: The parent class' realize handler.
> @@ -49,6 +52,16 @@ typedef struct X86CPUClass {
> CPUClass parent_class;
> /*< public >*/
>
> + /* CPU model definition
> + * Should be eventually replaced by subclass-specific property defaults
> + */
> + X86CPUDefinition *cpu_def;
> + /* CPU model requires KVM to be enabled */
> + bool kvm_required;
> + /* Optional description of CPU model.
> + * If unavailable, cpu_def->model_id is used */
> + const char *model_description;
Here I wondered why you needed this? For PowerPCCPU subclasses we have
reused DeviceClass::desc.
Regards,
Andreas
> +
> DeviceRealize parent_realize;
> void (*parent_reset)(CPUState *cpu);
> } X86CPUClass;
[snip]
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [uq/master PATCH 7/7 v8] target-i386: CPU model subclasses
2014-02-10 0:23 ` Andreas Färber
@ 2014-02-10 8:19 ` Eduardo Habkost
2014-02-10 8:26 ` Eduardo Habkost
2014-02-10 9:48 ` [Qemu-devel] [uq/master PATCH 7/7 v8] " Igor Mammedov
1 sibling, 1 reply; 45+ messages in thread
From: Eduardo Habkost @ 2014-02-10 8:19 UTC (permalink / raw)
To: Andreas Färber
Cc: kvm, libvir-list, qemu-devel, Igor Mammedov, Paolo Bonzini,
Jiri Denemark
On Mon, Feb 10, 2014 at 01:23:37AM +0100, Andreas Färber wrote:
[...]
> > /**
> > * X86CPUClass:
> > * @parent_realize: The parent class' realize handler.
> > @@ -49,6 +52,16 @@ typedef struct X86CPUClass {
> > CPUClass parent_class;
> > /*< public >*/
> >
> > + /* CPU model definition
> > + * Should be eventually replaced by subclass-specific property defaults
> > + */
> > + X86CPUDefinition *cpu_def;
> > + /* CPU model requires KVM to be enabled */
> > + bool kvm_required;
> > + /* Optional description of CPU model.
> > + * If unavailable, cpu_def->model_id is used */
> > + const char *model_description;
>
> Here I wondered why you needed this? For PowerPCCPU subclasses we have
> reused DeviceClass::desc.
I was not aware of DeviceClass::desc. We can use it instead.
Do you prefer a respin, or an additional patch?
--
Eduardo
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [uq/master PATCH 7/7 v8] target-i386: CPU model subclasses
2014-02-10 8:19 ` Eduardo Habkost
@ 2014-02-10 8:26 ` Eduardo Habkost
2014-02-10 10:21 ` [Qemu-devel] [qom-cpu PATCH 7/7 v9] " Eduardo Habkost
0 siblings, 1 reply; 45+ messages in thread
From: Eduardo Habkost @ 2014-02-10 8:26 UTC (permalink / raw)
To: Andreas Färber
Cc: kvm, libvir-list, qemu-devel, Paolo Bonzini, Igor Mammedov,
Jiri Denemark
On Mon, Feb 10, 2014 at 06:19:58AM -0200, Eduardo Habkost wrote:
> On Mon, Feb 10, 2014 at 01:23:37AM +0100, Andreas Färber wrote:
> [...]
> > > /**
> > > * X86CPUClass:
> > > * @parent_realize: The parent class' realize handler.
> > > @@ -49,6 +52,16 @@ typedef struct X86CPUClass {
> > > CPUClass parent_class;
> > > /*< public >*/
> > >
> > > + /* CPU model definition
> > > + * Should be eventually replaced by subclass-specific property defaults
> > > + */
> > > + X86CPUDefinition *cpu_def;
> > > + /* CPU model requires KVM to be enabled */
> > > + bool kvm_required;
> > > + /* Optional description of CPU model.
> > > + * If unavailable, cpu_def->model_id is used */
> > > + const char *model_description;
> >
> > Here I wondered why you needed this? For PowerPCCPU subclasses we have
> > reused DeviceClass::desc.
>
> I was not aware of DeviceClass::desc. We can use it instead.
>
> Do you prefer a respin, or an additional patch?
>
Actually we don't even need model_description or DeviceClass::desc yet,
because the code to list CPU models using
object_class_get_list(TYPE_X86_CPU) was moved to a separate patch I will
send later.
I will send a new version of this patch without model_description, then
change the new CPU model listing code to use DeviceClass::desc.
--
Eduardo
^ permalink raw reply [flat|nested] 45+ messages in thread* [Qemu-devel] [qom-cpu PATCH 7/7 v9] target-i386: CPU model subclasses
2014-02-10 8:26 ` Eduardo Habkost
@ 2014-02-10 10:21 ` Eduardo Habkost
2014-02-10 22:39 ` Andreas Färber
0 siblings, 1 reply; 45+ messages in thread
From: Eduardo Habkost @ 2014-02-10 10:21 UTC (permalink / raw)
To: Andreas Färber
Cc: kvm, libvir-list, qemu-devel, Igor Mammedov, Paolo Bonzini,
Jiri Denemark
Register separate QOM classes for each x86 CPU model.
This will allow management code to more easily probe what each CPU model
provides, by simply creating objects using the appropriate class name,
without having to restart QEMU.
This also allows us to eliminate the qdev_prop_set_globals_for_type()
hack to set CPU-model-specific global properties.
Instead of creating separate class_init functions for each class, I just
used class_data to store a pointer to the X86CPUDefinition struct for
each CPU model. This should make the patch shorter and easier to review.
Later we can gradually convert each X86CPUDefinition field to lists of
per-class property defaults.
Written based on the ideas from the patch "[RFC v5] target-i386: Slim
conversion to X86CPU subclasses + KVM subclasses" written by Andreas
Färber <afaerber@suse.de>, Igor Mammedov <imammedo@redhat.com>.
The "host" CPU model is special, as the feature flags depend on KVM
being initialized. So it has its own class_init and instance_init
function, and feature flags are set on instance_init instead of
class_init.
Signed-off-by: Andreas Färber <afaerber@suse.de>
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
This patch is similar to the one sent by Andrea and then later
resubmitted by Igor as "[RFC v5] target-i386: Slim conversion to X86CPU
subclasses + KVM subclasses", as it doesn't create one new class_init
function for each subclass.
Main differences v5 -> v6 are:
* Code was written from scratch (instead of using the previous patches
as base)
* I didn't mean to rewrite it entirely, but when doing additional
simplification of the CPU init logic on other patches, I ended up
rewriting it.
* I chose to keep the Signed-off-by lines because I built upon
Andreas's and Igor's ideas. Is that OK?
* No KVM-specific subclasses, to keep things simpler.
* No embedding of X86CPUDefinition (x86_def_t) inside the class struct,
instead keeping a pointer to the existing X86CPUDefinition struct.
* The "host" class is registered on cpu.c, but the CPUID data
is filled on instance_init instead of class_init (because KVM has to
be initialized already).
* kvm_required field introduced to make sure the "host" class can't
be used without KVM.
Changes v6 -> v7:
* Rebase
Changes v7 -> v8:
* Removed CPU listing code (will be sent as a separate patch)
* Kept x86_cpudef_setup() (will be addressed in a separate patch)
Changes v8 -> v9:
* Remove model_desc field from X86CPUClass (it is not necessary yet)
---
target-i386/cpu-qom.h | 10 +++
target-i386/cpu.c | 193 +++++++++++++++++++++++++++++++-------------------
2 files changed, 131 insertions(+), 72 deletions(-)
diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index 722f11a..00ad5a4 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -37,6 +37,9 @@
#define X86_CPU_GET_CLASS(obj) \
OBJECT_GET_CLASS(X86CPUClass, (obj), TYPE_X86_CPU)
+
+typedef struct X86CPUDefinition X86CPUDefinition;
+
/**
* X86CPUClass:
* @parent_realize: The parent class' realize handler.
@@ -49,6 +52,13 @@ typedef struct X86CPUClass {
CPUClass parent_class;
/*< public >*/
+ /* CPU model definition
+ * Should be eventually replaced by subclass-specific property defaults
+ */
+ X86CPUDefinition *cpu_def;
+ /* CPU model requires KVM to be enabled */
+ bool kvm_required;
+
DeviceRealize parent_realize;
void (*parent_reset)(CPUState *cpu);
} X86CPUClass;
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 5a530b5..0b6be20 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -484,7 +484,10 @@ static void add_flagname_to_bitmaps(const char *flagname,
}
}
-typedef struct X86CPUDefinition {
+/* CPU model definition data that was not converted to QOM per-subclass
+ * property defaults yet.
+ */
+struct X86CPUDefinition {
const char *name;
uint32_t level;
uint32_t xlevel;
@@ -497,7 +500,7 @@ typedef struct X86CPUDefinition {
FeatureWordArray features;
char model_id[48];
bool cache_info_passthrough;
-} X86CPUDefinition;
+};
#define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
#define PENTIUM_FEATURES (I486_FEATURES | CPUID_DE | CPUID_TSC | \
@@ -547,8 +550,29 @@ typedef struct X86CPUDefinition {
CPUID_7_0_EBX_ERMS, CPUID_7_0_EBX_INVPCID, CPUID_7_0_EBX_RTM,
CPUID_7_0_EBX_RDSEED */
-/* built-in CPU model definitions
+/* CPU class name definitions: */
+
+#define X86_CPU_CLASS_SUFFIX "-" TYPE_X86_CPU
+#define CPU_CLASS_NAME(name) (name X86_CPU_CLASS_SUFFIX)
+
+/* Return class name for a given CPU model name
+ * Caller is responsible for freeing the returned string.
*/
+static char *x86_cpu_class_name(const char *model_name)
+{
+ return g_strdup_printf(CPU_CLASS_NAME("%s"), model_name);
+}
+
+/* Return X86CPUClass for a CPU model name */
+static X86CPUClass *x86_cpu_class_by_name(const char *name)
+{
+ X86CPUClass *cc;
+ char *class_name = x86_cpu_class_name(name);
+ cc = X86_CPU_CLASS(object_class_by_name(class_name));
+ g_free(class_name);
+ return cc;
+}
+
static X86CPUDefinition builtin_x86_defs[] = {
{
.name = "qemu64",
@@ -1093,6 +1117,33 @@ static X86CPUDefinition builtin_x86_defs[] = {
},
};
+static void x86_cpu_class_init_cpudef(ObjectClass *oc, void *data)
+{
+ X86CPUDefinition *cpudef = data;
+ X86CPUClass *xcc = X86_CPU_CLASS(oc);
+ xcc->cpu_def = cpudef;
+}
+
+static void x86_register_cpudef_classes(void)
+{
+ int i;
+ for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); i++) {
+ X86CPUDefinition *def = &builtin_x86_defs[i];
+ char *class_name = x86_cpu_class_name(def->name);
+ TypeInfo ti = {
+ .name = class_name,
+ .parent = TYPE_X86_CPU,
+ .instance_size = sizeof(X86CPU),
+ .abstract = false,
+ .class_size = sizeof(X86CPUClass),
+ .class_init = x86_cpu_class_init_cpudef,
+ .class_data = def,
+ };
+ type_register(&ti);
+ g_free(class_name);
+ }
+}
+
/**
* x86_cpu_compat_set_features:
* @cpu_model: CPU model name to be changed. If NULL, all CPU models are changed
@@ -1134,44 +1185,70 @@ static int cpu_x86_fill_model_id(char *str)
return 0;
}
-/* Fill a X86CPUDefinition struct with information about the host CPU, and
- * the CPU features supported by the host hardware + host kernel
+static X86CPUDefinition host_cpudef;
+
+/* class_init for the "host" CPU model
*
- * This function may be called only if KVM is enabled.
+ * This function may be called before KVM is initialized.
*/
-static void kvm_cpu_fill_host(X86CPUDefinition *x86_cpu_def)
+static void x86_cpu_class_init_host(ObjectClass *oc, void *data)
{
- KVMState *s = kvm_state;
+ X86CPUClass *xcc = X86_CPU_CLASS(oc);
uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
- assert(kvm_enabled());
+ xcc->kvm_required = true;
- x86_cpu_def->name = "host";
- x86_cpu_def->cache_info_passthrough = true;
host_cpuid(0x0, 0, &eax, &ebx, &ecx, &edx);
- x86_cpu_vendor_words2str(x86_cpu_def->vendor, ebx, edx, ecx);
+ x86_cpu_vendor_words2str(host_cpudef.vendor, ebx, edx, ecx);
host_cpuid(0x1, 0, &eax, &ebx, &ecx, &edx);
- x86_cpu_def->family = ((eax >> 8) & 0x0F) + ((eax >> 20) & 0xFF);
- x86_cpu_def->model = ((eax >> 4) & 0x0F) | ((eax & 0xF0000) >> 12);
- x86_cpu_def->stepping = eax & 0x0F;
+ host_cpudef.family = ((eax >> 8) & 0x0F) + ((eax >> 20) & 0xFF);
+ host_cpudef.model = ((eax >> 4) & 0x0F) | ((eax & 0xF0000) >> 12);
+ host_cpudef.stepping = eax & 0x0F;
+
+ cpu_x86_fill_model_id(host_cpudef.model_id);
- x86_cpu_def->level = kvm_arch_get_supported_cpuid(s, 0x0, 0, R_EAX);
- x86_cpu_def->xlevel = kvm_arch_get_supported_cpuid(s, 0x80000000, 0, R_EAX);
- x86_cpu_def->xlevel2 =
- kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX);
+ xcc->cpu_def = &host_cpudef;
+ host_cpudef.cache_info_passthrough = true;
- cpu_x86_fill_model_id(x86_cpu_def->model_id);
+ /* level, xlevel, xlevel2, and the feature words are initialized on
+ * instance_init, because they require KVM to be initialized.
+ */
+}
+
+static void x86_cpu_instance_init_host(Object *obj)
+{
+ X86CPU *cpu = X86_CPU(obj);
+ CPUX86State *env = &cpu->env;
+ KVMState *s = kvm_state;
+
+ assert(kvm_enabled());
+
+ env->cpuid_level = kvm_arch_get_supported_cpuid(s, 0x0, 0, R_EAX);
+ env->cpuid_xlevel = kvm_arch_get_supported_cpuid(s, 0x80000000, 0, R_EAX);
+ env->cpuid_xlevel2 = kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX);
FeatureWord w;
for (w = 0; w < FEATURE_WORDS; w++) {
FeatureWordInfo *wi = &feature_word_info[w];
- x86_cpu_def->features[w] =
+ env->features[w] =
kvm_arch_get_supported_cpuid(s, wi->cpuid_eax, wi->cpuid_ecx,
wi->cpuid_reg);
}
+ object_property_set_bool(OBJECT(cpu), true, "pmu", &error_abort);
}
+
+static const TypeInfo x86_cpu_host_type_info = {
+ .name = CPU_CLASS_NAME("host"),
+ .parent = TYPE_X86_CPU,
+ .instance_size = sizeof(X86CPU),
+ .instance_init = x86_cpu_instance_init_host,
+ .abstract = false,
+ .class_size = sizeof(X86CPUClass),
+ .class_init = x86_cpu_class_init_host,
+};
+
static int unavailable_host_feature(FeatureWordInfo *f, uint32_t mask)
{
int i;
@@ -1582,32 +1659,6 @@ static PropertyInfo qdev_prop_spinlocks = {
.set = x86_set_hv_spinlocks,
};
-static int cpu_x86_find_by_name(X86CPU *cpu, X86CPUDefinition *x86_cpu_def,
- const char *name)
-{
- X86CPUDefinition *def;
- int i;
-
- if (name == NULL) {
- return -1;
- }
- if (kvm_enabled() && strcmp(name, "host") == 0) {
- kvm_cpu_fill_host(x86_cpu_def);
- object_property_set_bool(OBJECT(cpu), true, "pmu", &error_abort);
- return 0;
- }
-
- for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); i++) {
- def = &builtin_x86_defs[i];
- if (strcmp(name, def->name) == 0) {
- memcpy(x86_cpu_def, def, sizeof(*def));
- return 0;
- }
- }
-
- return -1;
-}
-
/* Convert all '_' in a feature string option name to '-', to make feature
* name conform to QOM property naming rule, which uses '-' instead of '_'.
*/
@@ -1817,19 +1868,11 @@ static void filter_features_for_kvm(X86CPU *cpu)
}
}
-/* Load CPU definition for a given CPU model name
+/* Load data from X86CPUDefinition
*/
-static void x86_cpu_load_def(X86CPU *cpu, const char *name, Error **errp)
+static void x86_cpu_load_def(X86CPU *cpu, X86CPUDefinition *def, Error **errp)
{
CPUX86State *env = &cpu->env;
- X86CPUDefinition def1, *def = &def1;
-
- memset(def, 0, sizeof(*def));
-
- if (cpu_x86_find_by_name(cpu, def, name) < 0) {
- error_setg(errp, "Unable to find CPU definition: %s", name);
- return;
- }
object_property_set_int(OBJECT(cpu), def->level, "level", errp);
object_property_set_int(OBJECT(cpu), def->family, "family", errp);
@@ -1881,7 +1924,6 @@ X86CPU *cpu_x86_create(const char *cpu_model, DeviceState *icc_bridge,
X86CPU *cpu = NULL;
gchar **model_pieces;
char *name, *features;
- char *typename;
Error *error = NULL;
model_pieces = g_strsplit(cpu_model, ",", 2);
@@ -1892,12 +1934,19 @@ X86CPU *cpu_x86_create(const char *cpu_model, DeviceState *icc_bridge,
name = model_pieces[0];
features = model_pieces[1];
- cpu = X86_CPU(object_new(TYPE_X86_CPU));
- x86_cpu_load_def(cpu, name, &error);
- if (error) {
+ X86CPUClass *cc = x86_cpu_class_by_name(name);
+ if (!cc) {
+ error_setg(&error, "Unable to find CPU definition: %s", name);
+ goto out;
+ }
+
+ if (cc->kvm_required && !kvm_enabled()) {
+ error_setg(&error, "CPU model '%s' requires KVM", name);
goto out;
}
+ cpu = X86_CPU(object_new(object_class_get_name(OBJECT_CLASS(cc))));
+
#ifndef CONFIG_USER_ONLY
if (icc_bridge == NULL) {
error_setg(&error, "Invalid icc-bridge value");
@@ -1907,14 +1956,6 @@ X86CPU *cpu_x86_create(const char *cpu_model, DeviceState *icc_bridge,
object_unref(OBJECT(cpu));
#endif
- /* Emulate per-model subclasses for global properties */
- typename = g_strdup_printf("%s-" TYPE_X86_CPU, name);
- qdev_prop_set_globals_for_type(DEVICE(cpu), typename, &error);
- g_free(typename);
- if (error) {
- goto out;
- }
-
cpu_x86_parse_featurestr(cpu, features, &error);
if (error) {
goto out;
@@ -1923,8 +1964,10 @@ X86CPU *cpu_x86_create(const char *cpu_model, DeviceState *icc_bridge,
out:
if (error != NULL) {
error_propagate(errp, error);
- object_unref(OBJECT(cpu));
- cpu = NULL;
+ if (cpu) {
+ object_unref(OBJECT(cpu));
+ cpu = NULL;
+ }
}
g_strfreev(model_pieces);
return cpu;
@@ -2615,6 +2658,7 @@ static void x86_cpu_initfn(Object *obj)
{
CPUState *cs = CPU(obj);
X86CPU *cpu = X86_CPU(obj);
+ X86CPUClass *xcc = X86_CPU_GET_CLASS(cpu);
CPUX86State *env = &cpu->env;
static int inited;
@@ -2666,6 +2710,9 @@ static void x86_cpu_initfn(Object *obj)
cpu_set_debug_excp_handler(breakpoint_handler);
#endif
}
+
+ X86CPUDefinition *def = xcc->cpu_def;
+ x86_cpu_load_def(cpu, def, &error_abort);
}
static int64_t x86_cpu_get_arch_id(CPUState *cs)
@@ -2763,7 +2810,7 @@ static const TypeInfo x86_cpu_type_info = {
.parent = TYPE_CPU,
.instance_size = sizeof(X86CPU),
.instance_init = x86_cpu_initfn,
- .abstract = false,
+ .abstract = true,
.class_size = sizeof(X86CPUClass),
.class_init = x86_cpu_common_class_init,
};
@@ -2771,6 +2818,8 @@ static const TypeInfo x86_cpu_type_info = {
static void x86_cpu_register_types(void)
{
type_register_static(&x86_cpu_type_info);
+ x86_register_cpudef_classes();
+ type_register_static(&x86_cpu_host_type_info);
}
type_init(x86_cpu_register_types)
--
1.8.5.3
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [qom-cpu PATCH 7/7 v9] target-i386: CPU model subclasses
2014-02-10 10:21 ` [Qemu-devel] [qom-cpu PATCH 7/7 v9] " Eduardo Habkost
@ 2014-02-10 22:39 ` Andreas Färber
2014-02-11 8:05 ` Eduardo Habkost
2014-02-11 8:07 ` Paolo Bonzini
0 siblings, 2 replies; 45+ messages in thread
From: Andreas Färber @ 2014-02-10 22:39 UTC (permalink / raw)
To: Eduardo Habkost
Cc: kvm, libvir-list, qemu-devel, Igor Mammedov, Paolo Bonzini,
Jiri Denemark
Am 10.02.2014 11:21, schrieb Eduardo Habkost:
> +static const TypeInfo x86_cpu_host_type_info = {
> + .name = CPU_CLASS_NAME("host"),
> + .parent = TYPE_X86_CPU,
> + .instance_size = sizeof(X86CPU),
> + .instance_init = x86_cpu_instance_init_host,
> + .abstract = false,
> + .class_size = sizeof(X86CPUClass),
> + .class_init = x86_cpu_class_init_host,
> +};
This looks broken: .class_data is not set but the loading of the cpudef
happens in the TYPE_X86_CPU initfn. My preferred solution would be to
move the cpudef-loading from TYPE_X86_CPU's instance_init to a separate
one specified for the models only, allowing non-cpudef-based models. Not
finished investigating yet.
For now I've prepended a patch implementing my generalized
CPUClass::class_by_name instead of a custom x86_cpu_class_by_name().
Other style nits that I'm working on cleaning up are declarations in the
middle of blocks, keeping _class_init naming convention (pretty sure my
patches always had the most-specific-to-least-specific naming), strictly
distinguishing between type and class, adding to my gtk-style
documentation rather than new custom comments, placing struct
documentation in the header and keeping the diff nicely readable AFAP.
I'd further like to keep some other conventions from previous CPU
subclasses, like pulling the model for loop out of the model
registration function.
My patches had always tried to turn what is now x86_cpu_load_def() into
an instance_init function rather than calling it from one - did you have
reasons not to?
Did you consider converting the host model in a first step to make the
patch smaller?
I'd rather finish my investigations and discuss my v10 patches but Paolo
is already asking whether Eduardo should send a PULL, so here's my
textual reply informing of some thoughts and WIP. ;)
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
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [qom-cpu PATCH 7/7 v9] target-i386: CPU model subclasses
2014-02-10 22:39 ` Andreas Färber
@ 2014-02-11 8:05 ` Eduardo Habkost
2014-02-11 8:07 ` Paolo Bonzini
1 sibling, 0 replies; 45+ messages in thread
From: Eduardo Habkost @ 2014-02-11 8:05 UTC (permalink / raw)
To: Andreas Färber
Cc: kvm, libvir-list, qemu-devel, Igor Mammedov, Paolo Bonzini,
Jiri Denemark
On Mon, Feb 10, 2014 at 11:39:51PM +0100, Andreas Färber wrote:
> Am 10.02.2014 11:21, schrieb Eduardo Habkost:
> > +static const TypeInfo x86_cpu_host_type_info = {
> > + .name = CPU_CLASS_NAME("host"),
> > + .parent = TYPE_X86_CPU,
> > + .instance_size = sizeof(X86CPU),
> > + .instance_init = x86_cpu_instance_init_host,
> > + .abstract = false,
> > + .class_size = sizeof(X86CPUClass),
> > + .class_init = x86_cpu_class_init_host,
> > +};
>
> This looks broken: .class_data is not set but the loading of the cpudef
> happens in the TYPE_X86_CPU initfn. My preferred solution would be to
> move the cpudef-loading from TYPE_X86_CPU's instance_init to a separate
> one specified for the models only, allowing non-cpudef-based models. Not
> finished investigating yet.
class_data is not set because x86_cpu_class_init_host doesn't use it.
x86_cpu_class_init_host() simply points fills host_cpu_def and makes
xcc->cpu_def point to it. Why would we need a separate instance_init
only for the other models? x86_cpu_initfn() already works for everything
except the ->features field (that is then handled by
x86_cpu_instance_init_host()).
>
> For now I've prepended a patch implementing my generalized
> CPUClass::class_by_name instead of a custom x86_cpu_class_by_name().
Sounds good to me.
>
> Other style nits that I'm working on cleaning up are declarations in the
> middle of blocks, keeping _class_init naming convention (pretty sure my
> patches always had the most-specific-to-least-specific naming), strictly
> distinguishing between type and class, adding to my gtk-style
> documentation rather than new custom comments, placing struct
> documentation in the header and keeping the diff nicely readable AFAP.
> I'd further like to keep some other conventions from previous CPU
> subclasses, like pulling the model for loop out of the model
> registration function.
>
> My patches had always tried to turn what is now x86_cpu_load_def() into
> an instance_init function rather than calling it from one - did you have
> reasons not to?
I like to keep the "simply move stuff from X86CPUDefinition to X86CPU"
logic in a separate place. I think it makes the code more readable (and
it also made the code movements more obvious because I didn't have to
move all the code that's inside load_def(), just the load_def() call.
But if you want to inline it inside instance_init, I wouldn't mind.
>
> Did you consider converting the host model in a first step to make the
> patch smaller?
Converting the host model would require moving some logic to
instance_init but keeping the strcmp(name, "host") hack. I thought it
would involve more complex intermediate steps so I chose not to try it.
--
Eduardo
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [qom-cpu PATCH 7/7 v9] target-i386: CPU model subclasses
2014-02-10 22:39 ` Andreas Färber
2014-02-11 8:05 ` Eduardo Habkost
@ 2014-02-11 8:07 ` Paolo Bonzini
1 sibling, 0 replies; 45+ messages in thread
From: Paolo Bonzini @ 2014-02-11 8:07 UTC (permalink / raw)
To: Andreas Färber, Eduardo Habkost
Cc: libvir-list, Igor Mammedov, Jiri Denemark, qemu-devel, kvm
Il 10/02/2014 23:39, Andreas Färber ha scritto:
> I'd rather finish my investigations and discuss my v10 patches but Paolo
> is already asking whether Eduardo should send a PULL, so here's my
> textual reply informing of some thoughts and WIP. ;)
Don't worry, :) I was asking you if you are fine with getting PULL
requests for qom-next instead of patch series, since Peter said he
prefers pull requests. I didn't refer to any series in particular!
Anyway, I'm happy to see that we're sorting out the issues you raised in
the last few days.
Regards,
Paolo
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [uq/master PATCH 7/7 v8] target-i386: CPU model subclasses
2014-02-10 0:23 ` Andreas Färber
2014-02-10 8:19 ` Eduardo Habkost
@ 2014-02-10 9:48 ` Igor Mammedov
1 sibling, 0 replies; 45+ messages in thread
From: Igor Mammedov @ 2014-02-10 9:48 UTC (permalink / raw)
To: Andreas Färber
Cc: Eduardo Habkost, kvm, libvir-list, qemu-devel, Paolo Bonzini,
Jiri Denemark
On Mon, 10 Feb 2014 01:23:37 +0100
Andreas Färber <afaerber@suse.de> wrote:
> Am 31.01.2014 19:13, schrieb Eduardo Habkost:
> > Register separate QOM classes for each x86 CPU model.
> >
> > This will allow management code to more easily probe what each CPU model
> > provides, by simply creating objects using the appropriate class name,
> > without having to restart QEMU.
> >
> > This also allows us to eliminate the qdev_prop_set_globals_for_type()
> > hack to set CPU-model-specific global properties.
> >
> > Instead of creating separate class_init functions for each class, I just
> > used class_data to store a pointer to the X86CPUDefinition struct for
> > each CPU model. This should make the patch shorter and easier to review.
> > Later we can gradually convert each X86CPUDefinition field to lists of
> > per-class property defaults.
> >
> > Written based on the ideas from the patch "[RFC v5] target-i386: Slim
> > conversion to X86CPU subclasses + KVM subclasses" written by Andreas
> > Färber <afaerber@suse.de>, Igor Mammedov <imammedo@redhat.com>.
> >
> > The "host" CPU model is special, as the feature flags depend on KVM
> > being initialized. So it has its own class_init and instance_init
> > function, and feature flags are set on instance_init instead of
> > class_init.
> >
> > Signed-off-by: Andreas Färber <afaerber@suse.de>
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> > This patch is similar to the one sent by Andrea and then later
> > resubmitted by Igor as "[RFC v5] target-i386: Slim conversion to X86CPU
> > subclasses + KVM subclasses", as it doesn't create one new class_init
> > function for each subclass.
> >
> > Main differences v5 -> v6 are:
> > * Code was written from scratch (instead of using the previous patches
> > as base)
> > * I didn't mean to rewrite it entirely, but when doing additional
> > simplification of the CPU init logic on other patches, I ended up
> > rewriting it.
> > * I chose to keep the Signed-off-by lines because I built upon
> > Andreas's and Igor's ideas. Is that OK?
>
> Yes, your From and our Sobs in order is the expected way in this case.
> If Igor agrees I would propose to drop the textual repetition of this.
I'm ok with it, but it doesn't matter since this part is under ---, so
it's dropped at commit time anyway.
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [uq/master PATCH 0/7] x86 CPU subclasses, take 7
2014-01-30 19:48 [Qemu-devel] [uq/master PATCH 0/7] x86 CPU subclasses, take 7 Eduardo Habkost
` (6 preceding siblings ...)
2014-01-30 19:48 ` [Qemu-devel] [uq/master PATCH 7/7] target-i386: CPU model subclasses Eduardo Habkost
@ 2014-01-30 21:47 ` Paolo Bonzini
2014-01-31 11:30 ` Andreas Färber
2014-01-31 14:48 ` Igor Mammedov
8 siblings, 1 reply; 45+ messages in thread
From: Paolo Bonzini @ 2014-01-30 21:47 UTC (permalink / raw)
To: Eduardo Habkost, qemu-devel
Cc: libvir-list, Igor Mammedov, Jiri Denemark, Andreas Färber,
kvm
Il 30/01/2014 20:48, Eduardo Habkost ha scritto:
> Is there any hope to get this into QEMU 2.0, or it is now too late? I got
> almost no feedback on take 6 (submitted November 27).
It's not too late, not for me at least. I wanted to send the next
uq/master pull request tomorrow or Tuesday, after I've done some more
testing on enlightenments. I can squeeze this in too.
Paolo
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [uq/master PATCH 0/7] x86 CPU subclasses, take 7
2014-01-30 21:47 ` [Qemu-devel] [uq/master PATCH 0/7] x86 CPU subclasses, take 7 Paolo Bonzini
@ 2014-01-31 11:30 ` Andreas Färber
2014-01-31 11:42 ` Paolo Bonzini
` (2 more replies)
0 siblings, 3 replies; 45+ messages in thread
From: Andreas Färber @ 2014-01-31 11:30 UTC (permalink / raw)
To: Paolo Bonzini, Eduardo Habkost, qemu-devel
Cc: libvir-list, Igor Mammedov, Jiri Denemark, kvm
Am 30.01.2014 22:47, schrieb Paolo Bonzini:
> Il 30/01/2014 20:48, Eduardo Habkost ha scritto:
>> Is there any hope to get this into QEMU 2.0, or it is now too late? I got
>> almost no feedback on take 6 (submitted November 27).
>
> It's not too late, not for me at least. I wanted to send the next
> uq/master pull request tomorrow or Tuesday, after I've done some more
> testing on enlightenments. I can squeeze this in too.
Negative, not without my review. It's clearly a CPU series, and apart
from having been on vacation pretty much all of December, Eduardo and
others have objected to my subclass series the last 2 *years*, so 2
months is peanuts by comparison.
Further, I was under the impression that this series depends on Igor's
feature property series, which I haven't found time to rework and
haven't noticed anyone else do either. So if there's no prereqs (why
uq/master?) I'll happily start reviewing and queuing it.
As Eduardo points out below the commit message in the final patch, his
conversion is very similar to one of my earlier patch series, so
committing that with Eduardo as author via uq/master without crediting
me via uq/master would leave a bad taste.
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
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [uq/master PATCH 0/7] x86 CPU subclasses, take 7
2014-01-31 11:30 ` Andreas Färber
@ 2014-01-31 11:42 ` Paolo Bonzini
2014-01-31 12:17 ` Eduardo Habkost
2014-01-31 12:14 ` Eduardo Habkost
2014-01-31 14:36 ` Igor Mammedov
2 siblings, 1 reply; 45+ messages in thread
From: Paolo Bonzini @ 2014-01-31 11:42 UTC (permalink / raw)
To: Andreas Färber, Eduardo Habkost, qemu-devel
Cc: libvir-list, Igor Mammedov, Jiri Denemark, kvm
Il 31/01/2014 12:30, Andreas Färber ha scritto:
> Further, I was under the impression that this series depends on Igor's
> feature property series, which I haven't found time to rework and
> haven't noticed anyone else do either. So if there's no prereqs (why
> uq/master?) I'll happily start reviewing and queuing it.
Yeah, I was also wondering why uq/master too, mostly for patches 5 and 7.
The first 3 patches are KVM-ish, and I can apply them if you prefer.
Patches 4 and 6 are mostly trivial, but I'll leave them to you.
> As Eduardo points out below the commit message in the final patch, his
> conversion is very similar to one of my earlier patch series, so
> committing that with Eduardo as author via uq/master without crediting
> me via uq/master would leave a bad taste.
I'll leave this to you to sort out.
Paolo
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [uq/master PATCH 0/7] x86 CPU subclasses, take 7
2014-01-31 11:42 ` Paolo Bonzini
@ 2014-01-31 12:17 ` Eduardo Habkost
0 siblings, 0 replies; 45+ messages in thread
From: Eduardo Habkost @ 2014-01-31 12:17 UTC (permalink / raw)
To: Paolo Bonzini
Cc: kvm, libvir-list, qemu-devel, Igor Mammedov, Jiri Denemark,
Andreas Färber
On Fri, Jan 31, 2014 at 12:42:08PM +0100, Paolo Bonzini wrote:
> Il 31/01/2014 12:30, Andreas Färber ha scritto:
> >Further, I was under the impression that this series depends on Igor's
> >feature property series, which I haven't found time to rework and
> >haven't noticed anyone else do either. So if there's no prereqs (why
> >uq/master?) I'll happily start reviewing and queuing it.
>
> Yeah, I was also wondering why uq/master too, mostly for patches 5 and 7.
I just wanted to indicate that the series can't be applied into
master yet because it depends on patches that are still on uq/master.
(In the past I have sent patches without a branch identifier in the
Subject, and started receiving notifications from some robot saying that
the patch doesn't apply to master. I don't know if that robot still
exists.)
--
Eduardo
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [uq/master PATCH 0/7] x86 CPU subclasses, take 7
2014-01-31 11:30 ` Andreas Färber
2014-01-31 11:42 ` Paolo Bonzini
@ 2014-01-31 12:14 ` Eduardo Habkost
2014-01-31 14:36 ` Igor Mammedov
2 siblings, 0 replies; 45+ messages in thread
From: Eduardo Habkost @ 2014-01-31 12:14 UTC (permalink / raw)
To: Andreas Färber
Cc: kvm, libvir-list, qemu-devel, Igor Mammedov, Paolo Bonzini,
Jiri Denemark
On Fri, Jan 31, 2014 at 12:30:17PM +0100, Andreas Färber wrote:
> Am 30.01.2014 22:47, schrieb Paolo Bonzini:
> > Il 30/01/2014 20:48, Eduardo Habkost ha scritto:
> >> Is there any hope to get this into QEMU 2.0, or it is now too late? I got
> >> almost no feedback on take 6 (submitted November 27).
> >
> > It's not too late, not for me at least. I wanted to send the next
> > uq/master pull request tomorrow or Tuesday, after I've done some more
> > testing on enlightenments. I can squeeze this in too.
>
> Negative, not without my review. It's clearly a CPU series, and apart
> from having been on vacation pretty much all of December, Eduardo and
> others have objected to my subclass series the last 2 *years*, so 2
> months is peanuts by comparison.
I don't remember objecting to your approach, but we simply had lots of
details to understand and questions settle. Then in February we
(Andreas, Igor, and me) stopped submitting new versions while we focused
in other stuff.
I was also not aware how the lack of subclasses would badly affect
libvirt's ability to use the new QOM properties we have added. I only
noticed that while talking to Jiri last November, that's why I resumed
the subclass work after months, to try to get it into QEMU 2.0.
>
> Further, I was under the impression that this series depends on Igor's
> feature property series, which I haven't found time to rework and
> haven't noticed anyone else do either. So if there's no prereqs (why
> uq/master?) I'll happily start reviewing and queuing it.
It doesn't depend on the feature properties at all.
But the series is based on uq/master only because it depends on
KVM-specific patches that are already on uq/master (that's why I added
uq/master to the Subject. It doesn't mean it has to be pulled through
uq/master necessarily).
>
> As Eduardo points out below the commit message in the final patch, his
> conversion is very similar to one of my earlier patch series, so
> committing that with Eduardo as author via uq/master without crediting
> me via uq/master would leave a bad taste.
Sorry, what would be the proper way to give proper attribution on the
commit message in this case? I often don't know if it is appropriate to
keep a Signed-off-by line if most of the code is new.
Also, I was incorrectly assuming the whole patch was re-written from
scratch by me, but now I have noticed I have copied
x86_cpu_list_compare() (and other parts) from your code without proper
attribution, I am very sorry for that. My poor excuse is that I have 3
or 4 local git branches where I was experimenting with different
approaches, and it was hard to keep track of everything.
--
Eduardo
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [uq/master PATCH 0/7] x86 CPU subclasses, take 7
2014-01-31 11:30 ` Andreas Färber
2014-01-31 11:42 ` Paolo Bonzini
2014-01-31 12:14 ` Eduardo Habkost
@ 2014-01-31 14:36 ` Igor Mammedov
2 siblings, 0 replies; 45+ messages in thread
From: Igor Mammedov @ 2014-01-31 14:36 UTC (permalink / raw)
To: Andreas Färber
Cc: Eduardo Habkost, kvm, libvir-list, qemu-devel, Paolo Bonzini,
Jiri Denemark
On Fri, 31 Jan 2014 12:30:17 +0100
Andreas Färber <afaerber@suse.de> wrote:
>
> Further, I was under the impression that this series depends on Igor's
> feature property series, which I haven't found time to rework and
> haven't noticed anyone else do either. So if there's no prereqs (why
> uq/master?) I'll happily start reviewing and queuing it.
It doesn't depend on properties series.
due to recent changes properties series should be reworked anyway
and probably should be based on top of this one.
[...]
> 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
--
Regards,
Igor
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [uq/master PATCH 0/7] x86 CPU subclasses, take 7
2014-01-30 19:48 [Qemu-devel] [uq/master PATCH 0/7] x86 CPU subclasses, take 7 Eduardo Habkost
` (7 preceding siblings ...)
2014-01-30 21:47 ` [Qemu-devel] [uq/master PATCH 0/7] x86 CPU subclasses, take 7 Paolo Bonzini
@ 2014-01-31 14:48 ` Igor Mammedov
2014-01-31 14:50 ` Paolo Bonzini
2014-01-31 15:10 ` [Qemu-devel] " Eduardo Habkost
8 siblings, 2 replies; 45+ messages in thread
From: Igor Mammedov @ 2014-01-31 14:48 UTC (permalink / raw)
To: Eduardo Habkost
Cc: kvm, libvir-list, qemu-devel, Paolo Bonzini, Jiri Denemark,
Andreas Färber
On Thu, 30 Jan 2014 17:48:52 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:
> Is there any hope to get this into QEMU 2.0, or it is now too late? I got
> almost no feedback on take 6 (submitted November 27).
>
> This is the main blocker to allow libvirt finally implement an equivalent to the
> "enforce" flag, finally making the CPU configuration safe and sane (today
> libvirt simply ignores GET_SUPPORTED_CPUID information, and features are
> silently disabled because "enforce" is not used).
>
> This blocks libvirt because the current available interfaces requires re-running
> QEMU for each CPU model to be probed. Having the x86 CPU subclasses allow
> libvirt to simply create and destroy CPU objects from each available CPU class,
> and query for the results using QMP.
>
> Demonstration of how this can be used, below:
>
> Running QEMU as:
> $ qemu-system-x86_64 -enable-kvm -machine none -monitor stdio -qmp unix:/tmp/qmp,server,nowait -nographic
>
> Then running qmp-shell as:
> $ ./scripts/qmp/qmp-shell /tmp/qmp
> [...]
> (QEMU) object-add qom-type=host-x86_64-cpu id=probing-host-cpu-type
[...]
> (QEMU) qom-list path=/objects/
that's abusing of object-add interface and due to recent changes, object-add
won't accept arbitrary objects.
see "[PATCH v1 3/4] add optional 2nd stage initialization to
-object/object-add commands"
libvirt probably could use device_add instead to the same effect.
BTW how libvirt would discover values for qom-type=foo?
> {u'return': [{u'type': u'child<Haswell-x86_64-cpu>', u'name': u'probing-cpu-type-Haswell'}, {u'type': u'child<Westmere-x86_64-cpu>', u'name': u'probing-cpu-type-Westmere'}, {u'type': u'child<Nehalem-x86_64-cpu>', u'name': u'probing-cpu-type-Nehalem'}, {u'type': u'child<host-x86_64-cpu>', u'name': u'probing-host-cpu-type'}, {u'type': u'string', u'name': u'type'}]}
> (QEMU) qom-list path=/objects/probing-cpu-type-Haswell/
> {u'return': [{u'type': u'X86CPUFeatureWordInfo', u'name': u'filtered-features'}, {u'type': u'X86CPUFeatureWordInfo', u'name': u'feature-words'}, {u'type': u'int', u'name': u'apic-id'}, {u'type': u'int', u'name': u'tsc-frequency'}, {u'type': u'string', u'name': u'model-id'}, {u'type': u'string', u'name': u'vendor'}, {u'type': u'int', u'name': u'xlevel'}, {u'type': u'int', u'name': u'level'}, {u'type': u'int', u'name': u'stepping'}, {u'type': u'int', u'name': u'model'}, {u'type': u'int', u'name': u'family'}, {u'type': u'link<bus>', u'name': u'parent_bus'}, {u'type': u'boolean', u'name': u'enforce'}, {u'type': u'boolean', u'name': u'check'}, {u'type': u'boolean', u'name': u'hv-time'}, {u'type': u'boolean', u'name': u'hv-vapic'}, {u'type': u'boolean', u'name': u'hv-relaxed'}, {u'type': u'int', u'name': u'hv-spinlocks'}, {u'type': u'boolean', u'name': u'pmu'}, {u'type': u'bool', u'name': u'realized'}, {u'type': u'string', u'name': u'type'}]}
> (QEMU) qom-get path=/objects/probing-cpu-type-Haswell property=feature-words
> {u'return': [{u'cpuid-register': u'EDX', u'cpuid-input-eax': 2147483658, u'features': 0}, {u'cpuid-register': u'EAX', u'cpuid-input-eax': 1073741825, u'features': 16777339}, {u'cpuid-register': u'EDX', u'cpuid-input-eax': 3221225473, u'features': 0}, {u'cpuid-register': u'ECX', u'cpuid-input-eax': 2147483649, u'features': 1}, {u'cpuid-register': u'EDX', u'cpuid-input-eax': 2147483649, u'features': 672139264}, {u'cpuid-register': u'EBX', u'cpuid-input-eax': 7, u'features': 4025, u'cpuid-input-ecx': 0}, {u'cpuid-register': u'ECX', u'cpuid-input-eax': 1, u'features': 2549756419}, {u'cpuid-register': u'EDX', u'cpuid-input-eax': 1, u'features': 126614525}]}
> (QEMU) qom-get path=/objects/probing-cpu-type-Haswell property=filtered-features
> {u'return': [{u'cpuid-register': u'EDX', u'cpuid-input-eax': 2147483658, u'features': 0}, {u'cpuid-register': u'EAX', u'cpuid-input-eax': 1073741825, u'features': 0}, {u'cpuid-register': u'EDX', u'cpuid-input-eax': 3221225473, u'features': 0}, {u'cpuid-register': u'ECX', u'cpuid-input-eax': 2147483649, u'features': 0}, {u'cpuid-register': u'EDX', u'cpuid-input-eax': 2147483649, u'features': 0}, {u'cpuid-register': u'EBX', u'cpuid-input-eax': 7, u'features': 0, u'cpuid-input-ecx': 0}, {u'cpuid-register': u'ECX', u'cpuid-input-eax': 1, u'features': 0}, {u'cpuid-register': u'EDX', u'cpuid-input-eax': 1, u'features': 0}]}
> (QEMU) qom-get path=/objects/probing-host-cpu-type property=feature-words
> {u'return': [{u'cpuid-register': u'EDX', u'cpuid-input-eax': 2147483658, u'features': 0}, {u'cpuid-register': u'EAX', u'cpuid-input-eax': 1073741825, u'features': 16777467}, {u'cpuid-register': u'EDX', u'cpuid-input-eax': 3221225473, u'features': 0}, {u'cpuid-register': u'ECX', u'cpuid-input-eax': 2147483649, u'features': 1}, {u'cpuid-register': u'EDX', u'cpuid-input-eax': 2147483649, u'features': 697564159}, {u'cpuid-register': u'EBX', u'cpuid-input-eax': 7, u'features': 2, u'cpuid-input-ecx': 0}, {u'cpuid-register': u'ECX', u'cpuid-input-eax': 1, u'features': 2193236483}, {u'cpuid-register': u'EDX', u'cpuid-input-eax': 1, u'features': 260832255}]}
> (QEMU) qom-get path=/objects/probing-host-cpu-type property=filtered-features
> {u'return': [{u'cpuid-register': u'EDX', u'cpuid-input-eax': 2147483658, u'features': 0}, {u'cpuid-register': u'EAX', u'cpuid-input-eax': 1073741825, u'features': 0}, {u'cpuid-register': u'EDX', u'cpuid-input-eax': 3221225473, u'features': 0}, {u'cpuid-register': u'ECX', u'cpuid-input-eax': 2147483649, u'features': 0}, {u'cpuid-register': u'EDX', u'cpuid-input-eax': 2147483649, u'features': 0}, {u'cpuid-register': u'EBX', u'cpuid-input-eax': 7, u'features': 0, u'cpuid-input-ecx': 0}, {u'cpuid-register': u'ECX', u'cpuid-input-eax': 1, u'features': 0}, {u'cpuid-register': u'EDX', u'cpuid-input-eax': 1, u'features': 0}]}
>
> Changes from take 6:
>
> * Rebase against uq/master
> * Patch 1/7:
> * Check for __i386__ on host_cpuid() so the code compiles properly
> on non-x86 hosts.
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> * Don't add assert(kvm_enabled()) line, which is not necessary to help
> the compiler (and wouldn't help it if using -DNDEBUG, anyway).
> Reported-by: Richard Henderson <rth@twiddle.net>
> * Commit message update
>
>
> Eduardo Habkost (7):
> target-i386: Eliminate CONFIG_KVM #ifdefs
> target-i386: Don't change x86_def_t struct on cpu_x86_register()
> target-i386: Move KVM default-vendor hack to instance_init
> target-i386: Rename cpu_x86_register() to x86_cpu_load_def()
> target-i386: Call x86_cpu_load_def() earlier
> target-i386: Rename x86_def_t to X86CPUDefinition
> target-i386: CPU model subclasses
>
> target-i386/cpu-qom.h | 13 ++
> target-i386/cpu.c | 402 ++++++++++++++++++++++++++++++--------------------
> target-i386/cpu.h | 2 -
> 3 files changed, 256 insertions(+), 161 deletions(-)
>
> --
> 1.8.4.2
>
>
--
Regards,
Igor
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [uq/master PATCH 0/7] x86 CPU subclasses, take 7
2014-01-31 14:48 ` Igor Mammedov
@ 2014-01-31 14:50 ` Paolo Bonzini
2014-01-31 15:17 ` Eduardo Habkost
2014-01-31 15:10 ` [Qemu-devel] " Eduardo Habkost
1 sibling, 1 reply; 45+ messages in thread
From: Paolo Bonzini @ 2014-01-31 14:50 UTC (permalink / raw)
To: Igor Mammedov, Eduardo Habkost
Cc: libvir-list, Jiri Denemark, qemu-devel, kvm, Andreas Färber
Il 31/01/2014 15:48, Igor Mammedov ha scritto:
> that's abusing of object-add interface and due to recent changes, object-add
> won't accept arbitrary objects.
I hope that sooner or later device hotplug will be doable with
object-add too. But yes, in the meanwhile device_add could work, and
this patch series is in the right direction anyway.
> see "[PATCH v1 3/4] add optional 2nd stage initialization to
> -object/object-add commands"
>
> libvirt probably could use device_add instead to the same effect.
>
> BTW how libvirt would discover values for qom-type=foo?
>
With qom-list-types.
Paolo
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [uq/master PATCH 0/7] x86 CPU subclasses, take 7
2014-01-31 14:50 ` Paolo Bonzini
@ 2014-01-31 15:17 ` Eduardo Habkost
2014-01-31 16:06 ` Igor Mammedov
0 siblings, 1 reply; 45+ messages in thread
From: Eduardo Habkost @ 2014-01-31 15:17 UTC (permalink / raw)
To: Paolo Bonzini
Cc: kvm, libvir-list, qemu-devel, Igor Mammedov, Jiri Denemark,
Andreas Färber
On Fri, Jan 31, 2014 at 03:50:23PM +0100, Paolo Bonzini wrote:
> Il 31/01/2014 15:48, Igor Mammedov ha scritto:
> >that's abusing of object-add interface and due to recent changes, object-add
> >won't accept arbitrary objects.
>
> I hope that sooner or later device hotplug will be doable with
> object-add too. But yes, in the meanwhile device_add could work,
> and this patch series is in the right direction anyway.
In that case, what is holding us from setting
cannot_instantiate_with_device_add_yet on TYPE_X86_CPU? I don't think we
can recommend using "-device" for CPUs just yet, but we would need to
allow it in case object-add doesn't work.
(But I liked the fact that object-add worked and (it looks like) it
didn't run realize(). All libvirt needs is to be able to create the
object and get instance_init() run, no need for realize() to run.)
I still need to read the reasoning behind the object-add changes. But is
there some chance we could allow object-add to work for TYPE_X86_CPU
subclasses, to avoid the device_add/cannot_instantiate_with_device_add_yet issues?
--
Eduardo
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [uq/master PATCH 0/7] x86 CPU subclasses, take 7
2014-01-31 15:17 ` Eduardo Habkost
@ 2014-01-31 16:06 ` Igor Mammedov
2014-01-31 16:42 ` Eduardo Habkost
0 siblings, 1 reply; 45+ messages in thread
From: Igor Mammedov @ 2014-01-31 16:06 UTC (permalink / raw)
To: Eduardo Habkost
Cc: kvm, libvir-list, qemu-devel, Paolo Bonzini, Jiri Denemark,
Andreas Färber
On Fri, 31 Jan 2014 13:17:53 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Fri, Jan 31, 2014 at 03:50:23PM +0100, Paolo Bonzini wrote:
> > Il 31/01/2014 15:48, Igor Mammedov ha scritto:
> > >that's abusing of object-add interface and due to recent changes, object-add
> > >won't accept arbitrary objects.
> >
> > I hope that sooner or later device hotplug will be doable with
> > object-add too. But yes, in the meanwhile device_add could work,
> > and this patch series is in the right direction anyway.
>
> In that case, what is holding us from setting
> cannot_instantiate_with_device_add_yet on TYPE_X86_CPU? I don't think we
> can recommend using "-device" for CPUs just yet, but we would need to
> allow it in case object-add doesn't work.
>
> (But I liked the fact that object-add worked and (it looks like) it
> didn't run realize(). All libvirt needs is to be able to create the
> object and get instance_init() run, no need for realize() to run.)
>
> I still need to read the reasoning behind the object-add changes. But is
> there some chance we could allow object-add to work for TYPE_X86_CPU
> subclasses, to avoid the device_add/cannot_instantiate_with_device_add_yet issues?
you can hack around by inheriting from UserCreatable interface,
but question is what kind of information libvirt would expect from
-object xxx-cpu
if it's going to read/interpret feature words then CPU.instance_init()
is not sufficient, since properties (read as compat props) and
realize() itself are changing feature words and CPU model guest is going
to see is very different from what -object would create.
It looks like only -device would be able to create actual CPU models,
but for -device to work we need as minimum this series and conversion
of CPU features to properties in tree. Then I guess we can override
cannot_instantiate_with_device_add_yet for x86 CPUs.
>
> --
> Eduardo
--
Regards,
Igor
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [uq/master PATCH 0/7] x86 CPU subclasses, take 7
2014-01-31 16:06 ` Igor Mammedov
@ 2014-01-31 16:42 ` Eduardo Habkost
2014-01-31 16:52 ` Paolo Bonzini
0 siblings, 1 reply; 45+ messages in thread
From: Eduardo Habkost @ 2014-01-31 16:42 UTC (permalink / raw)
To: Igor Mammedov
Cc: kvm, libvir-list, qemu-devel, Paolo Bonzini, Jiri Denemark,
Andreas Färber
On Fri, Jan 31, 2014 at 05:06:21PM +0100, Igor Mammedov wrote:
> On Fri, 31 Jan 2014 13:17:53 -0200
> Eduardo Habkost <ehabkost@redhat.com> wrote:
>
> > On Fri, Jan 31, 2014 at 03:50:23PM +0100, Paolo Bonzini wrote:
> > > Il 31/01/2014 15:48, Igor Mammedov ha scritto:
> > > >that's abusing of object-add interface and due to recent changes, object-add
> > > >won't accept arbitrary objects.
> > >
> > > I hope that sooner or later device hotplug will be doable with
> > > object-add too. But yes, in the meanwhile device_add could work,
> > > and this patch series is in the right direction anyway.
> >
> > In that case, what is holding us from setting
> > cannot_instantiate_with_device_add_yet on TYPE_X86_CPU? I don't think we
> > can recommend using "-device" for CPUs just yet, but we would need to
> > allow it in case object-add doesn't work.
> >
> > (But I liked the fact that object-add worked and (it looks like) it
> > didn't run realize(). All libvirt needs is to be able to create the
> > object and get instance_init() run, no need for realize() to run.)
> >
> > I still need to read the reasoning behind the object-add changes. But is
> > there some chance we could allow object-add to work for TYPE_X86_CPU
> > subclasses, to avoid the device_add/cannot_instantiate_with_device_add_yet issues?
>
> you can hack around by inheriting from UserCreatable interface,
> but question is what kind of information libvirt would expect from
> -object xxx-cpu
>
> if it's going to read/interpret feature words then CPU.instance_init()
> is not sufficient, since properties (read as compat props) and
> realize() itself are changing feature words and CPU model guest is going
> to see is very different from what -object would create.
I am not sure yet, but maybe that's a good thing?
I mean: libvirt has no concept of CPU models changing depending on
machine-type yet, and this will be addressed later. In this case, not
having the global properties set on the CPU object could be useful.
>
> It looks like only -device would be able to create actual CPU models,
> but for -device to work we need as minimum this series and conversion
> of CPU features to properties in tree. Then I guess we can override
> cannot_instantiate_with_device_add_yet for x86 CPUs.
Setting cannot_instantiate_with_device_add_yet=false looks much simpler
than implementing UserCreatable. My question is: may we do that, already
(once this series gets included), or is there something else holding us
from doing it?
--
Eduardo
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [uq/master PATCH 0/7] x86 CPU subclasses, take 7
2014-01-31 16:42 ` Eduardo Habkost
@ 2014-01-31 16:52 ` Paolo Bonzini
2014-01-31 18:51 ` Eduardo Habkost
0 siblings, 1 reply; 45+ messages in thread
From: Paolo Bonzini @ 2014-01-31 16:52 UTC (permalink / raw)
To: Eduardo Habkost, Igor Mammedov
Cc: libvir-list, Jiri Denemark, qemu-devel, kvm, Andreas Färber
Il 31/01/2014 17:42, Eduardo Habkost ha scritto:
>> > It looks like only -device would be able to create actual CPU models,
>> > but for -device to work we need as minimum this series and conversion
>> > of CPU features to properties in tree. Then I guess we can override
>> > cannot_instantiate_with_device_add_yet for x86 CPUs.
> Setting cannot_instantiate_with_device_add_yet=false looks much simpler
> than implementing UserCreatable. My question is: may we do that, already
> (once this series gets included), or is there something else holding us
> from doing it?
Once you make something creatable with "-object", the API must not
change anymore. So we would have to think twice about that.
Allowing -device may be okay, since in the (very?) long term -device can
be replaced by -object. But -object is definitive.
Paolo
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [uq/master PATCH 0/7] x86 CPU subclasses, take 7
2014-01-31 16:52 ` Paolo Bonzini
@ 2014-01-31 18:51 ` Eduardo Habkost
2014-01-31 18:56 ` [Qemu-devel] [libvirt] " Eric Blake
0 siblings, 1 reply; 45+ messages in thread
From: Eduardo Habkost @ 2014-01-31 18:51 UTC (permalink / raw)
To: Paolo Bonzini
Cc: kvm, libvir-list, qemu-devel, Igor Mammedov, Jiri Denemark,
Andreas Färber
On Fri, Jan 31, 2014 at 05:52:57PM +0100, Paolo Bonzini wrote:
> Il 31/01/2014 17:42, Eduardo Habkost ha scritto:
> >>> It looks like only -device would be able to create actual CPU models,
> >>> but for -device to work we need as minimum this series and conversion
> >>> of CPU features to properties in tree. Then I guess we can override
> >>> cannot_instantiate_with_device_add_yet for x86 CPUs.
> >Setting cannot_instantiate_with_device_add_yet=false looks much simpler
> >than implementing UserCreatable. My question is: may we do that, already
> >(once this series gets included), or is there something else holding us
> >from doing it?
>
> Once you make something creatable with "-object", the API must not
> change anymore. So we would have to think twice about that.
>
> Allowing -device may be okay, since in the (very?) long term -device
> can be replaced by -object. But -object is definitive.
OK, one additional reason to try device_add first.
But then we have one additional problem:
* We want to allow libvirt to probe for CPU model information when
running QEMU using "-machine none" (because libvirt already does
that, and we don't want to require libvirt to run QEMU multiple
times)
* "device_add driver=<model>-x86_64-cpu" requires an icc-bus to be present
* -machine none doesn't have any bus
* I don't see a way to create an icc-bus through QMP (is there a way?)
--
Eduardo
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [libvirt] [uq/master PATCH 0/7] x86 CPU subclasses, take 7
2014-01-31 18:51 ` Eduardo Habkost
@ 2014-01-31 18:56 ` Eric Blake
2014-01-31 19:08 ` Eduardo Habkost
2014-01-31 19:18 ` Igor Mammedov
0 siblings, 2 replies; 45+ messages in thread
From: Eric Blake @ 2014-01-31 18:56 UTC (permalink / raw)
To: Eduardo Habkost, Paolo Bonzini
Cc: kvm, libvir-list, qemu-devel, Igor Mammedov, Jiri Denemark,
Andreas Färber
[-- Attachment #1: Type: text/plain, Size: 1325 bytes --]
On 01/31/2014 11:51 AM, Eduardo Habkost wrote:
>> Allowing -device may be okay, since in the (very?) long term -device
>> can be replaced by -object. But -object is definitive.
>
> OK, one additional reason to try device_add first.
>
> But then we have one additional problem:
>
> * We want to allow libvirt to probe for CPU model information when
> running QEMU using "-machine none" (because libvirt already does
> that, and we don't want to require libvirt to run QEMU multiple
> times)
> * "device_add driver=<model>-x86_64-cpu" requires an icc-bus to be present
> * -machine none doesn't have any bus
> * I don't see a way to create an icc-bus through QMP (is there a way?)
Is the icc-bus something that makes sense for all architectures, so that
libvirt could just blindly request a command line that uses '-machine
none' but also instantiates the icc-bus? Even if icc-bus is
x86-specific, libvirt DOES have some notion of what architecture a qemu
executable will be targetting, and could modify the command line based
on what architecture it guesses the binary will support, if only for the
purpose of minimizing qemu invocations for its probe of supported cpus.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [libvirt] [uq/master PATCH 0/7] x86 CPU subclasses, take 7
2014-01-31 18:56 ` [Qemu-devel] [libvirt] " Eric Blake
@ 2014-01-31 19:08 ` Eduardo Habkost
2014-01-31 19:18 ` Igor Mammedov
1 sibling, 0 replies; 45+ messages in thread
From: Eduardo Habkost @ 2014-01-31 19:08 UTC (permalink / raw)
To: Eric Blake
Cc: kvm, libvir-list, qemu-devel, Igor Mammedov, Paolo Bonzini,
Jiri Denemark, Andreas Färber
On Fri, Jan 31, 2014 at 11:56:18AM -0700, Eric Blake wrote:
> On 01/31/2014 11:51 AM, Eduardo Habkost wrote:
>
> >> Allowing -device may be okay, since in the (very?) long term -device
> >> can be replaced by -object. But -object is definitive.
> >
> > OK, one additional reason to try device_add first.
> >
> > But then we have one additional problem:
> >
> > * We want to allow libvirt to probe for CPU model information when
> > running QEMU using "-machine none" (because libvirt already does
> > that, and we don't want to require libvirt to run QEMU multiple
> > times)
> > * "device_add driver=<model>-x86_64-cpu" requires an icc-bus to be present
> > * -machine none doesn't have any bus
> > * I don't see a way to create an icc-bus through QMP (is there a way?)
>
> Is the icc-bus something that makes sense for all architectures, so that
> libvirt could just blindly request a command line that uses '-machine
> none' but also instantiates the icc-bus? Even if icc-bus is
> x86-specific, libvirt DOES have some notion of what architecture a qemu
> executable will be targetting, and could modify the command line based
> on what architecture it guesses the binary will support, if only for the
> purpose of minimizing qemu invocations for its probe of supported cpus.
I don't know if it is possible to instantiate icc-bus from the
command-line if using -machine none, either. Does anybody know if it's
already possible?
--
Eduardo
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [libvirt] [uq/master PATCH 0/7] x86 CPU subclasses, take 7
2014-01-31 18:56 ` [Qemu-devel] [libvirt] " Eric Blake
2014-01-31 19:08 ` Eduardo Habkost
@ 2014-01-31 19:18 ` Igor Mammedov
2014-01-31 19:25 ` Eduardo Habkost
1 sibling, 1 reply; 45+ messages in thread
From: Igor Mammedov @ 2014-01-31 19:18 UTC (permalink / raw)
To: Eric Blake
Cc: Eduardo Habkost, kvm, libvir-list, qemu-devel, Paolo Bonzini,
Jiri Denemark, Andreas Färber
On Fri, 31 Jan 2014 11:56:18 -0700
Eric Blake <eblake@redhat.com> wrote:
> On 01/31/2014 11:51 AM, Eduardo Habkost wrote:
>
> >> Allowing -device may be okay, since in the (very?) long term -device
> >> can be replaced by -object. But -object is definitive.
> >
> > OK, one additional reason to try device_add first.
> >
> > But then we have one additional problem:
> >
> > * We want to allow libvirt to probe for CPU model information when
> > running QEMU using "-machine none" (because libvirt already does
> > that, and we don't want to require libvirt to run QEMU multiple
> > times)
> > * "device_add driver=<model>-x86_64-cpu" requires an icc-bus to be present
> > * -machine none doesn't have any bus
> > * I don't see a way to create an icc-bus through QMP (is there a way?)
>
> Is the icc-bus something that makes sense for all architectures, so that
> libvirt could just blindly request a command line that uses '-machine
> none' but also instantiates the icc-bus? Even if icc-bus is
> x86-specific, libvirt DOES have some notion of what architecture a qemu
> executable will be targetting, and could modify the command line based
> on what architecture it guesses the binary will support, if only for the
> purpose of minimizing qemu invocations for its probe of supported cpus.
Since -machine none, will not produce accurate CPUID info anyway and libvirt
knows in advance that it's going to create x86 machine, it might probe with
default machine type. It would be more accurate than -machine none,
but still might be not accurate if another machine type will be eventually
used for running guest.
>
> --
> Eric Blake eblake redhat com +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
--
Regards,
Igor
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [libvirt] [uq/master PATCH 0/7] x86 CPU subclasses, take 7
2014-01-31 19:18 ` Igor Mammedov
@ 2014-01-31 19:25 ` Eduardo Habkost
0 siblings, 0 replies; 45+ messages in thread
From: Eduardo Habkost @ 2014-01-31 19:25 UTC (permalink / raw)
To: Igor Mammedov
Cc: kvm, libvir-list, qemu-devel, Paolo Bonzini, Jiri Denemark,
Andreas Färber
On Fri, Jan 31, 2014 at 08:18:39PM +0100, Igor Mammedov wrote:
> On Fri, 31 Jan 2014 11:56:18 -0700
> Eric Blake <eblake@redhat.com> wrote:
>
> > On 01/31/2014 11:51 AM, Eduardo Habkost wrote:
> >
> > >> Allowing -device may be okay, since in the (very?) long term -device
> > >> can be replaced by -object. But -object is definitive.
> > >
> > > OK, one additional reason to try device_add first.
> > >
> > > But then we have one additional problem:
> > >
> > > * We want to allow libvirt to probe for CPU model information when
> > > running QEMU using "-machine none" (because libvirt already does
> > > that, and we don't want to require libvirt to run QEMU multiple
> > > times)
> > > * "device_add driver=<model>-x86_64-cpu" requires an icc-bus to be present
> > > * -machine none doesn't have any bus
> > > * I don't see a way to create an icc-bus through QMP (is there a way?)
> >
> > Is the icc-bus something that makes sense for all architectures, so that
> > libvirt could just blindly request a command line that uses '-machine
> > none' but also instantiates the icc-bus? Even if icc-bus is
> > x86-specific, libvirt DOES have some notion of what architecture a qemu
> > executable will be targetting, and could modify the command line based
> > on what architecture it guesses the binary will support, if only for the
> > purpose of minimizing qemu invocations for its probe of supported cpus.
> Since -machine none, will not produce accurate CPUID info anyway and libvirt
> knows in advance that it's going to create x86 machine, it might probe with
> default machine type. It would be more accurate than -machine none,
> but still might be not accurate if another machine type will be eventually
> used for running guest.
AFAIK, libvirt logic about CPU model features doesn't even take into
account that it may change depending machine-type. I believe getting the
non-machine-type-specific data will be better than the current state
where libvirt relies on the data from cpu_map.xml, which duplicates (and
sometimes don't match) what's inside QEMU.
We will also need a way to figure out machine-type-specific information
about each CPU model without re-running QEMU, but I think we can do this
one step at a time.
--
Eduardo
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [uq/master PATCH 0/7] x86 CPU subclasses, take 7
2014-01-31 14:48 ` Igor Mammedov
2014-01-31 14:50 ` Paolo Bonzini
@ 2014-01-31 15:10 ` Eduardo Habkost
2014-01-31 15:11 ` Paolo Bonzini
1 sibling, 1 reply; 45+ messages in thread
From: Eduardo Habkost @ 2014-01-31 15:10 UTC (permalink / raw)
To: Igor Mammedov
Cc: luiz, kvm, libvir-list, qemu-devel, Paolo Bonzini, Jiri Denemark,
Andreas Färber
(CCing Luiz, in case he can give some advice about the expectations of
QMP semantics stability)
On Fri, Jan 31, 2014 at 03:48:53PM +0100, Igor Mammedov wrote:
> On Thu, 30 Jan 2014 17:48:52 -0200
> Eduardo Habkost <ehabkost@redhat.com> wrote:
>
> > Is there any hope to get this into QEMU 2.0, or it is now too late? I got
> > almost no feedback on take 6 (submitted November 27).
> >
> > This is the main blocker to allow libvirt finally implement an equivalent to the
> > "enforce" flag, finally making the CPU configuration safe and sane (today
> > libvirt simply ignores GET_SUPPORTED_CPUID information, and features are
> > silently disabled because "enforce" is not used).
> >
> > This blocks libvirt because the current available interfaces requires re-running
> > QEMU for each CPU model to be probed. Having the x86 CPU subclasses allow
> > libvirt to simply create and destroy CPU objects from each available CPU class,
> > and query for the results using QMP.
> >
> > Demonstration of how this can be used, below:
> >
> > Running QEMU as:
> > $ qemu-system-x86_64 -enable-kvm -machine none -monitor stdio -qmp unix:/tmp/qmp,server,nowait -nographic
> >
> > Then running qmp-shell as:
> > $ ./scripts/qmp/qmp-shell /tmp/qmp
> > [...]
> > (QEMU) object-add qom-type=host-x86_64-cpu id=probing-host-cpu-type
> [...]
> > (QEMU) qom-list path=/objects/
> that's abusing of object-add interface and due to recent changes, object-add
> won't accept arbitrary objects.
>
> see "[PATCH v1 3/4] add optional 2nd stage initialization to
> -object/object-add commands"
>
> libvirt probably could use device_add instead to the same effect.
I don't mind which command is used, as long as we have the same effect.
I used object-add in my example because device_add rejects the CPU
classes by now (because they have cannot_instantiate_with_device_add_yet=true).
But now I have one question: how can people know which parts of the QMP
semantics will be stable, and which parts are subject to change?
>
> BTW how libvirt would discover values for qom-type=foo?
I don't know, but I assume there's an appropriate command to list qdev
and/or QOM classes, already?
In the worst case, libvirt can already use the query-cpu-definitions
command. But in this case, libvirt would have to encode the assumption
that the CPU class names will be "<model>-<arch>-cpu". Or we could
provide a new property on the query-cpu-definitions output containing
the class name (that sounds appropriate, considering that the "-cpu"
option doesn't get a class name (yet?)).
The specifics are not set in stone, and I expect other developers to
help me guide the libvirt developers and give them recommendations on
how to query for the needed information (because there are multiple ways
to do the same thing in QMP, as far as I can see).
>
> > {u'return': [{u'type': u'child<Haswell-x86_64-cpu>', u'name': u'probing-cpu-type-Haswell'}, {u'type': u'child<Westmere-x86_64-cpu>', u'name': u'probing-cpu-type-Westmere'}, {u'type': u'child<Nehalem-x86_64-cpu>', u'name': u'probing-cpu-type-Nehalem'}, {u'type': u'child<host-x86_64-cpu>', u'name': u'probing-host-cpu-type'}, {u'type': u'string', u'name': u'type'}]}
> > (QEMU) qom-list path=/objects/probing-cpu-type-Haswell/
> > {u'return': [{u'type': u'X86CPUFeatureWordInfo', u'name': u'filtered-features'}, {u'type': u'X86CPUFeatureWordInfo', u'name': u'feature-words'}, {u'type': u'int', u'name': u'apic-id'}, {u'type': u'int', u'name': u'tsc-frequency'}, {u'type': u'string', u'name': u'model-id'}, {u'type': u'string', u'name': u'vendor'}, {u'type': u'int', u'name': u'xlevel'}, {u'type': u'int', u'name': u'level'}, {u'type': u'int', u'name': u'stepping'}, {u'type': u'int', u'name': u'model'}, {u'type': u'int', u'name': u'family'}, {u'type': u'link<bus>', u'name': u'parent_bus'}, {u'type': u'boolean', u'name': u'enforce'}, {u'type': u'boolean', u'name': u'check'}, {u'type': u'boolean', u'name': u'hv-time'}, {u'type': u'boolean', u'name': u'hv-vapic'}, {u'type': u'boolean', u'name': u'hv-relaxed'}, {u'type': u'int', u'name': u'hv-spinlocks'}, {u'type': u'boolean', u'name': u'pmu'}, {u'type': u'bool', u'name': u'realized'}, {u'type': u'string', u'name': u'type'}]}
> > (QEMU) qom-get path=/objects/probing-cpu-type-Haswell property=feature-words
> > {u'return': [{u'cpuid-register': u'EDX', u'cpuid-input-eax': 2147483658, u'features': 0}, {u'cpuid-register': u'EAX', u'cpuid-input-eax': 1073741825, u'features': 16777339}, {u'cpuid-register': u'EDX', u'cpuid-input-eax': 3221225473, u'features': 0}, {u'cpuid-register': u'ECX', u'cpuid-input-eax': 2147483649, u'features': 1}, {u'cpuid-register': u'EDX', u'cpuid-input-eax': 2147483649, u'features': 672139264}, {u'cpuid-register': u'EBX', u'cpuid-input-eax': 7, u'features': 4025, u'cpuid-input-ecx': 0}, {u'cpuid-register': u'ECX', u'cpuid-input-eax': 1, u'features': 2549756419}, {u'cpuid-register': u'EDX', u'cpuid-input-eax': 1, u'features': 126614525}]}
> > (QEMU) qom-get path=/objects/probing-cpu-type-Haswell property=filtered-features
> > {u'return': [{u'cpuid-register': u'EDX', u'cpuid-input-eax': 2147483658, u'features': 0}, {u'cpuid-register': u'EAX', u'cpuid-input-eax': 1073741825, u'features': 0}, {u'cpuid-register': u'EDX', u'cpuid-input-eax': 3221225473, u'features': 0}, {u'cpuid-register': u'ECX', u'cpuid-input-eax': 2147483649, u'features': 0}, {u'cpuid-register': u'EDX', u'cpuid-input-eax': 2147483649, u'features': 0}, {u'cpuid-register': u'EBX', u'cpuid-input-eax': 7, u'features': 0, u'cpuid-input-ecx': 0}, {u'cpuid-register': u'ECX', u'cpuid-input-eax': 1, u'features': 0}, {u'cpuid-register': u'EDX', u'cpuid-input-eax': 1, u'features': 0}]}
> > (QEMU) qom-get path=/objects/probing-host-cpu-type property=feature-words
> > {u'return': [{u'cpuid-register': u'EDX', u'cpuid-input-eax': 2147483658, u'features': 0}, {u'cpuid-register': u'EAX', u'cpuid-input-eax': 1073741825, u'features': 16777467}, {u'cpuid-register': u'EDX', u'cpuid-input-eax': 3221225473, u'features': 0}, {u'cpuid-register': u'ECX', u'cpuid-input-eax': 2147483649, u'features': 1}, {u'cpuid-register': u'EDX', u'cpuid-input-eax': 2147483649, u'features': 697564159}, {u'cpuid-register': u'EBX', u'cpuid-input-eax': 7, u'features': 2, u'cpuid-input-ecx': 0}, {u'cpuid-register': u'ECX', u'cpuid-input-eax': 1, u'features': 2193236483}, {u'cpuid-register': u'EDX', u'cpuid-input-eax': 1, u'features': 260832255}]}
> > (QEMU) qom-get path=/objects/probing-host-cpu-type property=filtered-features
> > {u'return': [{u'cpuid-register': u'EDX', u'cpuid-input-eax': 2147483658, u'features': 0}, {u'cpuid-register': u'EAX', u'cpuid-input-eax': 1073741825, u'features': 0}, {u'cpuid-register': u'EDX', u'cpuid-input-eax': 3221225473, u'features': 0}, {u'cpuid-register': u'ECX', u'cpuid-input-eax': 2147483649, u'features': 0}, {u'cpuid-register': u'EDX', u'cpuid-input-eax': 2147483649, u'features': 0}, {u'cpuid-register': u'EBX', u'cpuid-input-eax': 7, u'features': 0, u'cpuid-input-ecx': 0}, {u'cpuid-register': u'ECX', u'cpuid-input-eax': 1, u'features': 0}, {u'cpuid-register': u'EDX', u'cpuid-input-eax': 1, u'features': 0}]}
> >
> > Changes from take 6:
> >
> > * Rebase against uq/master
> > * Patch 1/7:
> > * Check for __i386__ on host_cpuid() so the code compiles properly
> > on non-x86 hosts.
> > Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> > * Don't add assert(kvm_enabled()) line, which is not necessary to help
> > the compiler (and wouldn't help it if using -DNDEBUG, anyway).
> > Reported-by: Richard Henderson <rth@twiddle.net>
> > * Commit message update
> >
> >
> > Eduardo Habkost (7):
> > target-i386: Eliminate CONFIG_KVM #ifdefs
> > target-i386: Don't change x86_def_t struct on cpu_x86_register()
> > target-i386: Move KVM default-vendor hack to instance_init
> > target-i386: Rename cpu_x86_register() to x86_cpu_load_def()
> > target-i386: Call x86_cpu_load_def() earlier
> > target-i386: Rename x86_def_t to X86CPUDefinition
> > target-i386: CPU model subclasses
> >
> > target-i386/cpu-qom.h | 13 ++
> > target-i386/cpu.c | 402 ++++++++++++++++++++++++++++++--------------------
> > target-i386/cpu.h | 2 -
> > 3 files changed, 256 insertions(+), 161 deletions(-)
> >
> > --
> > 1.8.4.2
> >
> >
>
>
> --
> Regards,
> Igor
--
Eduardo
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [uq/master PATCH 0/7] x86 CPU subclasses, take 7
2014-01-31 15:10 ` [Qemu-devel] " Eduardo Habkost
@ 2014-01-31 15:11 ` Paolo Bonzini
0 siblings, 0 replies; 45+ messages in thread
From: Paolo Bonzini @ 2014-01-31 15:11 UTC (permalink / raw)
To: Eduardo Habkost, Igor Mammedov
Cc: luiz, kvm, libvir-list, qemu-devel, Jiri Denemark,
Andreas Färber
Il 31/01/2014 16:10, Eduardo Habkost ha scritto:
> I don't mind which command is used, as long as we have the same effect.
> I used object-add in my example because device_add rejects the CPU
> classes by now (because they have cannot_instantiate_with_device_add_yet=true).
>
> But now I have one question: how can people know which parts of the QMP
> semantics will be stable, and which parts are subject to change?
object-add was added in 2.0, so it was never stable.
Paolo
^ permalink raw reply [flat|nested] 45+ messages in thread