* [Qemu-devel] [PATCH 1/7] target-i386: kvm_cpu_fill_host(): Kill unused code
2014-01-20 16:41 [Qemu-devel] [PATCH 0/7 RESEND] target-i386: Simplify kvm_cpu_fill_host() and kvm_check_features_against_host() Eduardo Habkost
@ 2014-01-20 16:41 ` Eduardo Habkost
2014-01-20 16:41 ` [Qemu-devel] [PATCH 2/7] target-i386: kvm_cpu_fill_host(): No need to check level Eduardo Habkost
` (6 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Eduardo Habkost @ 2014-01-20 16:41 UTC (permalink / raw)
To: qemu-devel; +Cc: Igor Mammedov, Andreas Färber, Michael S. Tsirkin
Those host_cpuid() calls are useless. They are leftovers from when the
old code using host_cpuid() was removed.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
target-i386/cpu.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 0eea8c7..6f27273 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1178,12 +1178,10 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
/* Call Centaur's CPUID instruction. */
if (!strcmp(x86_cpu_def->vendor, CPUID_VENDOR_VIA)) {
- host_cpuid(0xC0000000, 0, &eax, &ebx, &ecx, &edx);
eax = kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX);
if (eax >= 0xC0000001) {
/* Support VIA max extended level */
x86_cpu_def->xlevel2 = eax;
- host_cpuid(0xC0000001, 0, &eax, &ebx, &ecx, &edx);
x86_cpu_def->features[FEAT_C000_0001_EDX] =
kvm_arch_get_supported_cpuid(s, 0xC0000001, 0, R_EDX);
}
--
1.8.4.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 2/7] target-i386: kvm_cpu_fill_host(): No need to check level
2014-01-20 16:41 [Qemu-devel] [PATCH 0/7 RESEND] target-i386: Simplify kvm_cpu_fill_host() and kvm_check_features_against_host() Eduardo Habkost
2014-01-20 16:41 ` [Qemu-devel] [PATCH 1/7] target-i386: kvm_cpu_fill_host(): Kill unused code Eduardo Habkost
@ 2014-01-20 16:41 ` Eduardo Habkost
2014-01-20 16:41 ` [Qemu-devel] [PATCH 3/7] target-i386: kvm_cpu_fill_host(): No need to check CPU vendor Eduardo Habkost
` (5 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Eduardo Habkost @ 2014-01-20 16:41 UTC (permalink / raw)
To: qemu-devel; +Cc: Igor Mammedov, Andreas Färber, Michael S. Tsirkin
There's no need to check level (CPUID[0].EAX) before calling
kvm_arch_get_supported_cpuid(s, 0x7, 0, R_EBX), because:
* The kernel won't return any entry for CPUID 7 if CPUID[0].EAX is < 7
(See kvm_dev_ioctl_get_supported_cpuid() on the kernel code);
* kvm_arch_get_supported_cpuid() will return 0 if no entry is returned
by the kernel for the requested leaf.
This will simplify the kvm_cpu_fill_host() code a little.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
target-i386/cpu.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 6f27273..aec0107 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1161,12 +1161,8 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
x86_cpu_def->features[FEAT_1_ECX] =
kvm_arch_get_supported_cpuid(s, 0x1, 0, R_ECX);
- if (x86_cpu_def->level >= 7) {
- x86_cpu_def->features[FEAT_7_0_EBX] =
- kvm_arch_get_supported_cpuid(s, 0x7, 0, R_EBX);
- } else {
- x86_cpu_def->features[FEAT_7_0_EBX] = 0;
- }
+ x86_cpu_def->features[FEAT_7_0_EBX] =
+ kvm_arch_get_supported_cpuid(s, 0x7, 0, R_EBX);
x86_cpu_def->xlevel = kvm_arch_get_supported_cpuid(s, 0x80000000, 0, R_EAX);
x86_cpu_def->features[FEAT_8000_0001_EDX] =
--
1.8.4.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 3/7] target-i386: kvm_cpu_fill_host(): No need to check CPU vendor
2014-01-20 16:41 [Qemu-devel] [PATCH 0/7 RESEND] target-i386: Simplify kvm_cpu_fill_host() and kvm_check_features_against_host() Eduardo Habkost
2014-01-20 16:41 ` [Qemu-devel] [PATCH 1/7] target-i386: kvm_cpu_fill_host(): Kill unused code Eduardo Habkost
2014-01-20 16:41 ` [Qemu-devel] [PATCH 2/7] target-i386: kvm_cpu_fill_host(): No need to check level Eduardo Habkost
@ 2014-01-20 16:41 ` Eduardo Habkost
2014-01-20 16:41 ` [Qemu-devel] [PATCH 4/7] target-i386: kvm_cpu_fill_host(): No need to check xlevel2 Eduardo Habkost
` (4 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Eduardo Habkost @ 2014-01-20 16:41 UTC (permalink / raw)
To: qemu-devel; +Cc: Igor Mammedov, Andreas Färber, Michael S. Tsirkin
There's no need to check CPU vendor before calling
kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX), because:
* The kernel won't return any entry for 0xC0000000 if host CPU vendor
is not Centaur (See kvm_dev_ioctl_get_supported_cpuid() on the kernel
code);
* kvm_arch_get_supported_cpuid() will return 0 if no entry is returned
by the kernel for the requested leaf.
This will simplify the kvm_cpu_fill_host() code a little.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
target-i386/cpu.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index aec0107..1a86bcf 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1173,14 +1173,12 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
cpu_x86_fill_model_id(x86_cpu_def->model_id);
/* Call Centaur's CPUID instruction. */
- if (!strcmp(x86_cpu_def->vendor, CPUID_VENDOR_VIA)) {
- eax = kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX);
- if (eax >= 0xC0000001) {
- /* Support VIA max extended level */
- x86_cpu_def->xlevel2 = eax;
- x86_cpu_def->features[FEAT_C000_0001_EDX] =
- kvm_arch_get_supported_cpuid(s, 0xC0000001, 0, R_EDX);
- }
+ eax = kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX);
+ if (eax >= 0xC0000001) {
+ /* Support VIA max extended level */
+ x86_cpu_def->xlevel2 = eax;
+ x86_cpu_def->features[FEAT_C000_0001_EDX] =
+ kvm_arch_get_supported_cpuid(s, 0xC0000001, 0, R_EDX);
}
/* Other KVM-specific feature fields: */
--
1.8.4.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 4/7] target-i386: kvm_cpu_fill_host(): No need to check xlevel2
2014-01-20 16:41 [Qemu-devel] [PATCH 0/7 RESEND] target-i386: Simplify kvm_cpu_fill_host() and kvm_check_features_against_host() Eduardo Habkost
` (2 preceding siblings ...)
2014-01-20 16:41 ` [Qemu-devel] [PATCH 3/7] target-i386: kvm_cpu_fill_host(): No need to check CPU vendor Eduardo Habkost
@ 2014-01-20 16:41 ` Eduardo Habkost
2014-01-20 21:16 ` Andreas Färber
2014-01-21 10:52 ` Paolo Bonzini
2014-01-20 16:41 ` [Qemu-devel] [PATCH 5/7] target-i386: kvm_cpu_fill_host(): Set all feature words at end of function Eduardo Habkost
` (3 subsequent siblings)
7 siblings, 2 replies; 16+ messages in thread
From: Eduardo Habkost @ 2014-01-20 16:41 UTC (permalink / raw)
To: qemu-devel; +Cc: Igor Mammedov, Andreas Färber, Michael S. Tsirkin
There's no need to check CPU xlevel2 before calling
kvm_arch_get_supported_cpuid(s, 0xC0000001, 0, R_EDX), because:
* The kernel won't return any entry for 0xC0000000 if host CPU vendor
is not Centaur (See kvm_dev_ioctl_get_supported_cpuid() on the kernel
code)
* Similarly, the kernel won't return any entry for 0xC0000001 if
CPUID[0xC0000000].EAX is < 0xC0000001
* kvm_arch_get_supported_cpuid() will return 0 if no entry is returned
by the kernel for the requested leaf
For similar reasons, we can simply set x86_cpu_def->xlevel2 directly
instead of making it conditional, because it will be set to 0 CPU vendor
is not Centaur.
This will simplify the kvm_cpu_fill_host() code a little.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
target-i386/cpu.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 1a86bcf..8bc72c2 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1173,13 +1173,11 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
cpu_x86_fill_model_id(x86_cpu_def->model_id);
/* Call Centaur's CPUID instruction. */
- eax = kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX);
- if (eax >= 0xC0000001) {
- /* Support VIA max extended level */
- x86_cpu_def->xlevel2 = eax;
- x86_cpu_def->features[FEAT_C000_0001_EDX] =
- kvm_arch_get_supported_cpuid(s, 0xC0000001, 0, R_EDX);
- }
+ x86_cpu_def->xlevel2 =
+ kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX);
+ /* Support VIA max extended level */
+ x86_cpu_def->features[FEAT_C000_0001_EDX] =
+ kvm_arch_get_supported_cpuid(s, 0xC0000001, 0, R_EDX);
/* Other KVM-specific feature fields: */
x86_cpu_def->features[FEAT_SVM] =
--
1.8.4.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 4/7] target-i386: kvm_cpu_fill_host(): No need to check xlevel2
2014-01-20 16:41 ` [Qemu-devel] [PATCH 4/7] target-i386: kvm_cpu_fill_host(): No need to check xlevel2 Eduardo Habkost
@ 2014-01-20 21:16 ` Andreas Färber
2014-01-21 10:52 ` Paolo Bonzini
1 sibling, 0 replies; 16+ messages in thread
From: Andreas Färber @ 2014-01-20 21:16 UTC (permalink / raw)
To: Eduardo Habkost, qemu-devel; +Cc: Igor Mammedov, Michael S. Tsirkin
Am 20.01.2014 17:41, schrieb Eduardo Habkost:
> There's no need to check CPU xlevel2 before calling
> kvm_arch_get_supported_cpuid(s, 0xC0000001, 0, R_EDX), because:
>
> * The kernel won't return any entry for 0xC0000000 if host CPU vendor
> is not Centaur (See kvm_dev_ioctl_get_supported_cpuid() on the kernel
> code)
> * Similarly, the kernel won't return any entry for 0xC0000001 if
> CPUID[0xC0000000].EAX is < 0xC0000001
> * kvm_arch_get_supported_cpuid() will return 0 if no entry is returned
> by the kernel for the requested leaf
>
> For similar reasons, we can simply set x86_cpu_def->xlevel2 directly
> instead of making it conditional, because it will be set to 0 CPU vendor
> is not Centaur.
Commented on the wrong version:
"... if CPU vendor is not ..." I guess. :)
>
> This will simplify the kvm_cpu_fill_host() code a little.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> target-i386/cpu.c | 12 +++++-------
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 1a86bcf..8bc72c2 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1173,13 +1173,11 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
> cpu_x86_fill_model_id(x86_cpu_def->model_id);
>
> /* Call Centaur's CPUID instruction. */
> - eax = kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX);
> - if (eax >= 0xC0000001) {
> - /* Support VIA max extended level */
> - x86_cpu_def->xlevel2 = eax;
> - x86_cpu_def->features[FEAT_C000_0001_EDX] =
> - kvm_arch_get_supported_cpuid(s, 0xC0000001, 0, R_EDX);
> - }
> + x86_cpu_def->xlevel2 =
> + kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX);
> + /* Support VIA max extended level */
> + x86_cpu_def->features[FEAT_C000_0001_EDX] =
> + kvm_arch_get_supported_cpuid(s, 0xC0000001, 0, R_EDX);
>
> /* Other KVM-specific feature fields: */
> x86_cpu_def->features[FEAT_SVM] =
Otherwise looks okay to me.
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] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 4/7] target-i386: kvm_cpu_fill_host(): No need to check xlevel2
2014-01-20 16:41 ` [Qemu-devel] [PATCH 4/7] target-i386: kvm_cpu_fill_host(): No need to check xlevel2 Eduardo Habkost
2014-01-20 21:16 ` Andreas Färber
@ 2014-01-21 10:52 ` Paolo Bonzini
1 sibling, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2014-01-21 10:52 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Igor Mammedov, Michael S. Tsirkin, qemu-devel,
Andreas Färber
Il 20/01/2014 17:41, Eduardo Habkost ha scritto:
> - eax = kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX);
> - if (eax >= 0xC0000001) {
> - /* Support VIA max extended level */
> - x86_cpu_def->xlevel2 = eax;
> - x86_cpu_def->features[FEAT_C000_0001_EDX] =
> - kvm_arch_get_supported_cpuid(s, 0xC0000001, 0, R_EDX);
> - }
> + x86_cpu_def->xlevel2 =
> + kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX);
> + /* Support VIA max extended level */
> + x86_cpu_def->features[FEAT_C000_0001_EDX] =
> + kvm_arch_get_supported_cpuid(s, 0xC0000001, 0, R_EDX);
>
I'm removing this comment before applying the patch (I believe the
comment was not really grammatical, and it meant something like "VIA max
extended level supported").
Paolo
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 5/7] target-i386: kvm_cpu_fill_host(): Set all feature words at end of function
2014-01-20 16:41 [Qemu-devel] [PATCH 0/7 RESEND] target-i386: Simplify kvm_cpu_fill_host() and kvm_check_features_against_host() Eduardo Habkost
` (3 preceding siblings ...)
2014-01-20 16:41 ` [Qemu-devel] [PATCH 4/7] target-i386: kvm_cpu_fill_host(): No need to check xlevel2 Eduardo Habkost
@ 2014-01-20 16:41 ` Eduardo Habkost
2014-01-20 16:41 ` [Qemu-devel] [PATCH 6/7] target-i386: kvm_cpu_fill_host(): Fill feature words in a loop Eduardo Habkost
` (2 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Eduardo Habkost @ 2014-01-20 16:41 UTC (permalink / raw)
To: qemu-devel; +Cc: Igor Mammedov, Andreas Färber, Michael S. Tsirkin
Reorder the code so all the code that sets x86_cpu_def->features is at
the end of the function.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
target-i386/cpu.c | 24 +++++++++---------------
1 file changed, 9 insertions(+), 15 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 8bc72c2..8183be0 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1156,30 +1156,24 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
x86_cpu_def->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(x86_cpu_def->model_id);
+
x86_cpu_def->features[FEAT_1_EDX] =
kvm_arch_get_supported_cpuid(s, 0x1, 0, R_EDX);
x86_cpu_def->features[FEAT_1_ECX] =
kvm_arch_get_supported_cpuid(s, 0x1, 0, R_ECX);
-
x86_cpu_def->features[FEAT_7_0_EBX] =
- kvm_arch_get_supported_cpuid(s, 0x7, 0, R_EBX);
-
- x86_cpu_def->xlevel = kvm_arch_get_supported_cpuid(s, 0x80000000, 0, R_EAX);
+ kvm_arch_get_supported_cpuid(s, 0x7, 0, R_EBX);
x86_cpu_def->features[FEAT_8000_0001_EDX] =
- kvm_arch_get_supported_cpuid(s, 0x80000001, 0, R_EDX);
+ kvm_arch_get_supported_cpuid(s, 0x80000001, 0, R_EDX);
x86_cpu_def->features[FEAT_8000_0001_ECX] =
- kvm_arch_get_supported_cpuid(s, 0x80000001, 0, R_ECX);
-
- cpu_x86_fill_model_id(x86_cpu_def->model_id);
-
- /* Call Centaur's CPUID instruction. */
- x86_cpu_def->xlevel2 =
- kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX);
- /* Support VIA max extended level */
+ kvm_arch_get_supported_cpuid(s, 0x80000001, 0, R_ECX);
x86_cpu_def->features[FEAT_C000_0001_EDX] =
kvm_arch_get_supported_cpuid(s, 0xC0000001, 0, R_EDX);
-
- /* Other KVM-specific feature fields: */
x86_cpu_def->features[FEAT_SVM] =
kvm_arch_get_supported_cpuid(s, 0x8000000A, 0, R_EDX);
x86_cpu_def->features[FEAT_KVM] =
--
1.8.4.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 6/7] target-i386: kvm_cpu_fill_host(): Fill feature words in a loop
2014-01-20 16:41 [Qemu-devel] [PATCH 0/7 RESEND] target-i386: Simplify kvm_cpu_fill_host() and kvm_check_features_against_host() Eduardo Habkost
` (4 preceding siblings ...)
2014-01-20 16:41 ` [Qemu-devel] [PATCH 5/7] target-i386: kvm_cpu_fill_host(): Set all feature words at end of function Eduardo Habkost
@ 2014-01-20 16:41 ` Eduardo Habkost
2014-01-20 16:41 ` [Qemu-devel] [PATCH 7/7 v2] target-i386: kvm_check_features_against_host(): Kill feature word array Eduardo Habkost
2014-01-20 18:36 ` [Qemu-devel] [PATCH 0/7 RESEND] target-i386: Simplify kvm_cpu_fill_host() and kvm_check_features_against_host() Eduardo Habkost
7 siblings, 0 replies; 16+ messages in thread
From: Eduardo Habkost @ 2014-01-20 16:41 UTC (permalink / raw)
To: qemu-devel; +Cc: Igor Mammedov, Andreas Färber, Michael S. Tsirkin
Now that the kvm_cpu_fill_host() code is simplified, we can simply set
the feature word array using a simple loop.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
target-i386/cpu.c | 23 +++++++----------------
1 file changed, 7 insertions(+), 16 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 8183be0..e76ed1e 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1162,22 +1162,13 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
cpu_x86_fill_model_id(x86_cpu_def->model_id);
- x86_cpu_def->features[FEAT_1_EDX] =
- kvm_arch_get_supported_cpuid(s, 0x1, 0, R_EDX);
- x86_cpu_def->features[FEAT_1_ECX] =
- kvm_arch_get_supported_cpuid(s, 0x1, 0, R_ECX);
- x86_cpu_def->features[FEAT_7_0_EBX] =
- kvm_arch_get_supported_cpuid(s, 0x7, 0, R_EBX);
- x86_cpu_def->features[FEAT_8000_0001_EDX] =
- kvm_arch_get_supported_cpuid(s, 0x80000001, 0, R_EDX);
- x86_cpu_def->features[FEAT_8000_0001_ECX] =
- kvm_arch_get_supported_cpuid(s, 0x80000001, 0, R_ECX);
- x86_cpu_def->features[FEAT_C000_0001_EDX] =
- kvm_arch_get_supported_cpuid(s, 0xC0000001, 0, R_EDX);
- x86_cpu_def->features[FEAT_SVM] =
- kvm_arch_get_supported_cpuid(s, 0x8000000A, 0, R_EDX);
- x86_cpu_def->features[FEAT_KVM] =
- kvm_arch_get_supported_cpuid(s, KVM_CPUID_FEATURES, 0, R_EAX);
+ FeatureWord w;
+ for (w = 0; w < FEATURE_WORDS; w++) {
+ FeatureWordInfo *wi = &feature_word_info[w];
+ x86_cpu_def->features[w] =
+ kvm_arch_get_supported_cpuid(s, wi->cpuid_eax, wi->cpuid_ecx,
+ wi->cpuid_reg);
+ }
#endif /* CONFIG_KVM */
}
--
1.8.4.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 7/7 v2] target-i386: kvm_check_features_against_host(): Kill feature word array
2014-01-20 16:41 [Qemu-devel] [PATCH 0/7 RESEND] target-i386: Simplify kvm_cpu_fill_host() and kvm_check_features_against_host() Eduardo Habkost
` (5 preceding siblings ...)
2014-01-20 16:41 ` [Qemu-devel] [PATCH 6/7] target-i386: kvm_cpu_fill_host(): Fill feature words in a loop Eduardo Habkost
@ 2014-01-20 16:41 ` Eduardo Habkost
2014-01-20 18:36 ` [Qemu-devel] [PATCH 0/7 RESEND] target-i386: Simplify kvm_cpu_fill_host() and kvm_check_features_against_host() Eduardo Habkost
7 siblings, 0 replies; 16+ messages in thread
From: Eduardo Habkost @ 2014-01-20 16:41 UTC (permalink / raw)
To: qemu-devel; +Cc: Igor Mammedov, Andreas Färber, Michael S. Tsirkin
We don't need the ft[] array on kvm_check_features_against_host()
anymore, as we can simply use the feature_word_info[] array, that has
everything we need.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Change v2:
* Rebase and solve simple conflict with recent changes in the check/enforce code
---
target-i386/cpu.c | 48 ++++++++++++------------------------------------
1 file changed, 12 insertions(+), 36 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index e76ed1e..583adfe 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1197,48 +1197,23 @@ static int unavailable_host_feature(FeatureWordInfo *f, uint32_t mask)
*
* This function may be called only if KVM is enabled.
*/
-static int kvm_check_features_against_host(X86CPU *cpu)
+static int kvm_check_features_against_host(KVMState *s, X86CPU *cpu)
{
CPUX86State *env = &cpu->env;
- x86_def_t host_def;
- uint32_t mask;
- int rv, i;
- struct model_features_t ft[] = {
- {&env->features[FEAT_1_EDX],
- &host_def.features[FEAT_1_EDX],
- FEAT_1_EDX },
- {&env->features[FEAT_1_ECX],
- &host_def.features[FEAT_1_ECX],
- FEAT_1_ECX },
- {&env->features[FEAT_8000_0001_EDX],
- &host_def.features[FEAT_8000_0001_EDX],
- FEAT_8000_0001_EDX },
- {&env->features[FEAT_8000_0001_ECX],
- &host_def.features[FEAT_8000_0001_ECX],
- FEAT_8000_0001_ECX },
- {&env->features[FEAT_C000_0001_EDX],
- &host_def.features[FEAT_C000_0001_EDX],
- FEAT_C000_0001_EDX },
- {&env->features[FEAT_7_0_EBX],
- &host_def.features[FEAT_7_0_EBX],
- FEAT_7_0_EBX },
- {&env->features[FEAT_SVM],
- &host_def.features[FEAT_SVM],
- FEAT_SVM },
- {&env->features[FEAT_KVM],
- &host_def.features[FEAT_KVM],
- FEAT_KVM },
- };
+ int rv = 0;
+ FeatureWord w;
assert(kvm_enabled());
- kvm_cpu_fill_host(&host_def);
- for (rv = 0, i = 0; i < ARRAY_SIZE(ft); ++i) {
- FeatureWord w = ft[i].feat_word;
+ for (w = 0; w < FEATURE_WORDS; w++) {
FeatureWordInfo *wi = &feature_word_info[w];
+ uint32_t guest_feat = env->features[w];
+ uint32_t host_feat = kvm_arch_get_supported_cpuid(s, wi->cpuid_eax,
+ wi->cpuid_ecx,
+ wi->cpuid_reg);
+ uint32_t mask;
for (mask = 1; mask; mask <<= 1) {
- if (*ft[i].guest_feat & mask &&
- !(*ft[i].host_feat & mask)) {
+ if (guest_feat & mask && !(host_feat & mask)) {
unavailable_host_feature(wi, mask);
rv = 1;
}
@@ -2556,8 +2531,9 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
env->features[FEAT_8000_0001_ECX] &= TCG_EXT3_FEATURES;
env->features[FEAT_SVM] &= TCG_SVM_FEATURES;
} else {
+ KVMState *s = kvm_state;
if ((cpu->check_cpuid || cpu->enforce_cpuid)
- && kvm_check_features_against_host(cpu) && cpu->enforce_cpuid) {
+ && kvm_check_features_against_host(s, cpu) && cpu->enforce_cpuid) {
error_setg(&local_err,
"Host's CPU doesn't support requested features");
goto out;
--
1.8.4.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 0/7 RESEND] target-i386: Simplify kvm_cpu_fill_host() and kvm_check_features_against_host()
2014-01-20 16:41 [Qemu-devel] [PATCH 0/7 RESEND] target-i386: Simplify kvm_cpu_fill_host() and kvm_check_features_against_host() Eduardo Habkost
` (6 preceding siblings ...)
2014-01-20 16:41 ` [Qemu-devel] [PATCH 7/7 v2] target-i386: kvm_check_features_against_host(): Kill feature word array Eduardo Habkost
@ 2014-01-20 18:36 ` Eduardo Habkost
2014-01-20 20:39 ` Michael S. Tsirkin
7 siblings, 1 reply; 16+ messages in thread
From: Eduardo Habkost @ 2014-01-20 18:36 UTC (permalink / raw)
To: qemu-devel
Cc: Igor Mammedov, Paolo Bonzini, Andreas Färber, kvm,
Michael S. Tsirkin
On Mon, Jan 20, 2014 at 02:41:07PM -0200, Eduardo Habkost wrote:
> Resend of series submitted on 24 November 2013, that didn't get any reply. Only
> change is a trivial conflict on patch 7/7.
Question: which tree is the most appropriate to get this in? qom-cpu?
kvm?
>
> This series simplifies kvm_cpu_fill_host() and
> kvm_check_features_against_host() to simply use FeatureWord & feature_word_info
> loops to fill/check feature words.
>
> The initial motivation for this was to avoid hacks involving the "host" CPU
> class on the forthcoming conversion of CPU models to be X86CPU subclasses.
> Instead of requiring the kvm_arch_get_supported_cpuid() results to be stored in
> the class struct for "host" (thus requiring KVM initialization hacks).
>
> Eduardo Habkost (7):
> target-i386: kvm_cpu_fill_host(): Kill unused code
> target-i386: kvm_cpu_fill_host(): No need to check level
> target-i386: kvm_cpu_fill_host(): No need to check CPU vendor
> target-i386: kvm_cpu_fill_host(): No need to check xlevel2
> target-i386: kvm_cpu_fill_host(): Set all feature words at end of
> function
> target-i386: kvm_cpu_fill_host(): Fill feature words in a loop
> target-i386: kvm_check_features_against_host(): Kill feature word
> array
>
> target-i386/cpu.c | 89 +++++++++++++------------------------------------------
> 1 file changed, 20 insertions(+), 69 deletions(-)
>
> --
> 1.8.4.2
>
>
--
Eduardo
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 0/7 RESEND] target-i386: Simplify kvm_cpu_fill_host() and kvm_check_features_against_host()
2014-01-20 18:36 ` [Qemu-devel] [PATCH 0/7 RESEND] target-i386: Simplify kvm_cpu_fill_host() and kvm_check_features_against_host() Eduardo Habkost
@ 2014-01-20 20:39 ` Michael S. Tsirkin
2014-01-20 21:07 ` Andreas Färber
2014-01-21 10:03 ` Paolo Bonzini
0 siblings, 2 replies; 16+ messages in thread
From: Michael S. Tsirkin @ 2014-01-20 20:39 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Igor Mammedov, Paolo Bonzini, qemu-devel, kvm,
Andreas Färber
On Mon, Jan 20, 2014 at 04:36:56PM -0200, Eduardo Habkost wrote:
> On Mon, Jan 20, 2014 at 02:41:07PM -0200, Eduardo Habkost wrote:
> > Resend of series submitted on 24 November 2013, that didn't get any reply. Only
> > change is a trivial conflict on patch 7/7.
>
> Question: which tree is the most appropriate to get this in? qom-cpu?
> kvm?
Either kvm or my pc tree.
Seems unrelated to qom.
Paolo - want to review and take this?
>
> >
> > This series simplifies kvm_cpu_fill_host() and
> > kvm_check_features_against_host() to simply use FeatureWord & feature_word_info
> > loops to fill/check feature words.
> >
> > The initial motivation for this was to avoid hacks involving the "host" CPU
> > class on the forthcoming conversion of CPU models to be X86CPU subclasses.
> > Instead of requiring the kvm_arch_get_supported_cpuid() results to be stored in
> > the class struct for "host" (thus requiring KVM initialization hacks).
> >
> > Eduardo Habkost (7):
> > target-i386: kvm_cpu_fill_host(): Kill unused code
> > target-i386: kvm_cpu_fill_host(): No need to check level
> > target-i386: kvm_cpu_fill_host(): No need to check CPU vendor
> > target-i386: kvm_cpu_fill_host(): No need to check xlevel2
> > target-i386: kvm_cpu_fill_host(): Set all feature words at end of
> > function
> > target-i386: kvm_cpu_fill_host(): Fill feature words in a loop
> > target-i386: kvm_check_features_against_host(): Kill feature word
> > array
> >
> > target-i386/cpu.c | 89 +++++++++++++------------------------------------------
> > 1 file changed, 20 insertions(+), 69 deletions(-)
> >
> > --
> > 1.8.4.2
> >
> >
>
> --
> Eduardo
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 0/7 RESEND] target-i386: Simplify kvm_cpu_fill_host() and kvm_check_features_against_host()
2014-01-20 20:39 ` Michael S. Tsirkin
@ 2014-01-20 21:07 ` Andreas Färber
2014-01-20 22:51 ` Michael S. Tsirkin
2014-01-21 10:03 ` Paolo Bonzini
1 sibling, 1 reply; 16+ messages in thread
From: Andreas Färber @ 2014-01-20 21:07 UTC (permalink / raw)
To: Michael S. Tsirkin, Eduardo Habkost
Cc: Igor Mammedov, qemu-devel, kvm, Paolo Bonzini
Am 20.01.2014 21:39, schrieb Michael S. Tsirkin:
> On Mon, Jan 20, 2014 at 04:36:56PM -0200, Eduardo Habkost wrote:
>> On Mon, Jan 20, 2014 at 02:41:07PM -0200, Eduardo Habkost wrote:
>>> Resend of series submitted on 24 November 2013, that didn't get any reply. Only
>>> change is a trivial conflict on patch 7/7.
>>
>> Question: which tree is the most appropriate to get this in? qom-cpu?
>> kvm?
>
> Either kvm or my pc tree.
> Seems unrelated to qom.
Seems unrelated to PC. ;) I've been maintaining target-i386/cpu.c as
part of my QOM CPU tree according to MAINTAINERS.
I don't mind whether this goes through Paolo's or my tree, but for me to
take KVM related changes, I expect review from the KVM side.
> Paolo - want to review and take this?
Regards,
Andreas
>>> target-i386/cpu.c | 89 +++++++++++++------------------------------------------
>>> 1 file changed, 20 insertions(+), 69 deletions(-)
--
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] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 0/7 RESEND] target-i386: Simplify kvm_cpu_fill_host() and kvm_check_features_against_host()
2014-01-20 21:07 ` Andreas Färber
@ 2014-01-20 22:51 ` Michael S. Tsirkin
0 siblings, 0 replies; 16+ messages in thread
From: Michael S. Tsirkin @ 2014-01-20 22:51 UTC (permalink / raw)
To: Andreas Färber
Cc: Igor Mammedov, Paolo Bonzini, Eduardo Habkost, kvm, qemu-devel
On Mon, Jan 20, 2014 at 10:07:47PM +0100, Andreas Färber wrote:
> Am 20.01.2014 21:39, schrieb Michael S. Tsirkin:
> > On Mon, Jan 20, 2014 at 04:36:56PM -0200, Eduardo Habkost wrote:
> >> On Mon, Jan 20, 2014 at 02:41:07PM -0200, Eduardo Habkost wrote:
> >>> Resend of series submitted on 24 November 2013, that didn't get any reply. Only
> >>> change is a trivial conflict on patch 7/7.
> >>
> >> Question: which tree is the most appropriate to get this in? qom-cpu?
> >> kvm?
> >
> > Either kvm or my pc tree.
> > Seems unrelated to qom.
>
> Seems unrelated to PC. ;) I've been maintaining target-i386/cpu.c as
> part of my QOM CPU tree according to MAINTAINERS.
Ah, right. Cool, the less work for me the better.
> I don't mind whether this goes through Paolo's or my tree, but for me to
> take KVM related changes, I expect review from the KVM side.
>
> > Paolo - want to review and take this?
>
> Regards,
> Andreas
>
> >>> target-i386/cpu.c | 89 +++++++++++++------------------------------------------
> >>> 1 file changed, 20 insertions(+), 69 deletions(-)
>
> --
> 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] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 0/7 RESEND] target-i386: Simplify kvm_cpu_fill_host() and kvm_check_features_against_host()
2014-01-20 20:39 ` Michael S. Tsirkin
2014-01-20 21:07 ` Andreas Färber
@ 2014-01-21 10:03 ` Paolo Bonzini
2014-01-21 10:20 ` Andreas Färber
1 sibling, 1 reply; 16+ messages in thread
From: Paolo Bonzini @ 2014-01-21 10:03 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Igor Mammedov, kvm, Eduardo Habkost, Andreas Färber,
qemu-devel
Il 20/01/2014 21:39, Michael S. Tsirkin ha scritto:
>>> > > Resend of series submitted on 24 November 2013, that didn't get any reply. Only
>>> > > change is a trivial conflict on patch 7/7.
>> >
>> > Question: which tree is the most appropriate to get this in? qom-cpu?
>> > kvm?
> Either kvm or my pc tree.
> Seems unrelated to qom.
> Paolo - want to review and take this?
>
Yup, will include in the next uq/master pull request.
Paolo
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 0/7 RESEND] target-i386: Simplify kvm_cpu_fill_host() and kvm_check_features_against_host()
2014-01-21 10:03 ` Paolo Bonzini
@ 2014-01-21 10:20 ` Andreas Färber
0 siblings, 0 replies; 16+ messages in thread
From: Andreas Färber @ 2014-01-21 10:20 UTC (permalink / raw)
To: Paolo Bonzini
Cc: qemu-devel, Igor Mammedov, Eduardo Habkost, kvm,
Michael S. Tsirkin
Am 21.01.2014 11:03, schrieb Paolo Bonzini:
> Il 20/01/2014 21:39, Michael S. Tsirkin ha scritto:
>>>>>> Resend of series submitted on 24 November 2013, that didn't get any reply. Only
>>>>>> change is a trivial conflict on patch 7/7.
>>>>
>>>> Question: which tree is the most appropriate to get this in? qom-cpu?
>>>> kvm?
>> Either kvm or my pc tree.
>> Seems unrelated to qom.
>> Paolo - want to review and take this?
>>
>
> Yup, will include in the next uq/master pull request.
Thanks!
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] 16+ messages in thread