* Re: [PATCH 00/11] cpu model bug fixes and definition corrections (v2)
2011-06-03 14:38 ` [PATCH 00/11] cpu model bug fixes and definition corrections (v2) Eduardo Habkost
@ 2011-06-03 14:51 ` Jan Kiszka
2011-06-09 6:43 ` [Qemu-devel] " Markus Armbruster
0 siblings, 1 reply; 3+ messages in thread
From: Jan Kiszka @ 2011-06-03 14:51 UTC (permalink / raw)
To: Anthony Liguori, qemu-devel, Marcelo Tosatti, Avi Kivity, kvm
On 2011-06-03 16:38, Eduardo Habkost wrote:
> (CCing Marcelo, Avi, and kvm mailing list, so they can help answering
> the uq/master patch flow question)
>
> On Fri, Jun 03, 2011 at 12:51:42AM +0200, Jan Kiszka wrote:
>> On 2011-06-02 21:34, Eduardo Habkost wrote:
>>> Ouch, the subject prefix is completely wrong because of broken
>>> git-send-email config on my side, sorry.
>>>
>>> Please ignore the 'RHEL6 qemu-kvm' prefix, it is actually supposed to go
>>> to the main Qemu tree.
>>
>> Some of my review comments on John's original version still apply. Same
>> for the advice on the patch flow (uq/master for kvm stuff).
>
> Just to make sure I didn't miss anything:
>
> 1) uq/master flow: considering that most of the series is not
> KVM-specific but depends on patch 02/11 (Allow an optional
> qemu_early_init_vcpu()) what is the best approach? Should the whole
> series go through uq/master, or just patch 02/11? In the case of the
> latter, shall the rest of the series wait for the patch to be merged
> upstream, or should patch 02/11 go to both branches at the same time?
I would suggest to break out those patches that touch KVM
infrastructure, post them for uq/master, and declare the rest of the
series to depend on them.
>
> 2) Reviewing cpu_x86_cpuid() cpuid hacking code & dropping
> -enable-nesting: should it hold the series, or may it be addressed
> after this series enter the tree?
No that does not need to block the series. I would just recommend
checking if there is anything in that diff directly related. If not,
let's address it separately.
>
> 3) Other recommendations for the qemu_early_init_vcpu() code
> (checkpatch.sh, return code evaluation, KVMState vs. VCPU): I will
> address those issues and send a new version.
Find some proposal for a refactored kvm_arch_get_supported_cpuid API
below.
>
> Something else I may have missed?
>
Nothing critical, I'm just hoping someone finds the time to fix
sysconfigs loading when starting qemu from a build directory. :)
Thanks,
Jan
-------8<-------
From: Jan Kiszka <jan.kiszka@siemens.com>
kvm: x86: Pass KVMState to kvm_arch_get_supported_cpuid
kvm_arch_get_supported_cpuid checks for global cpuid restrictions, it
does not require any CPUState reference. Changing its interface allows
to call it before any VCPU is initialized.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
kvm.h | 2 +-
target-i386/cpuid.c | 20 ++++++++++++--------
target-i386/kvm.c | 30 +++++++++++++++---------------
3 files changed, 28 insertions(+), 24 deletions(-)
diff --git a/kvm.h b/kvm.h
index d565dba..243b063 100644
--- a/kvm.h
+++ b/kvm.h
@@ -157,7 +157,7 @@ bool kvm_arch_stop_on_emulation_error(CPUState *env);
int kvm_check_extension(KVMState *s, unsigned int extension);
-uint32_t kvm_arch_get_supported_cpuid(CPUState *env, uint32_t function,
+uint32_t kvm_arch_get_supported_cpuid(KVMState *env, uint32_t function,
uint32_t index, int reg);
void kvm_cpu_synchronize_state(CPUState *env);
void kvm_cpu_synchronize_post_reset(CPUState *env);
diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index 79e7580..e1ae3af 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -1144,10 +1144,12 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
break;
case 7:
if (kvm_enabled()) {
- *eax = kvm_arch_get_supported_cpuid(env, 0x7, count, R_EAX);
- *ebx = kvm_arch_get_supported_cpuid(env, 0x7, count, R_EBX);
- *ecx = kvm_arch_get_supported_cpuid(env, 0x7, count, R_ECX);
- *edx = kvm_arch_get_supported_cpuid(env, 0x7, count, R_EDX);
+ KVMState *s = env->kvm_state;
+
+ *eax = kvm_arch_get_supported_cpuid(s, 0x7, count, R_EAX);
+ *ebx = kvm_arch_get_supported_cpuid(s, 0x7, count, R_EBX);
+ *ecx = kvm_arch_get_supported_cpuid(s, 0x7, count, R_ECX);
+ *edx = kvm_arch_get_supported_cpuid(s, 0x7, count, R_EDX);
} else {
*eax = 0;
*ebx = 0;
@@ -1179,10 +1181,12 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
break;
}
if (kvm_enabled()) {
- *eax = kvm_arch_get_supported_cpuid(env, 0xd, count, R_EAX);
- *ebx = kvm_arch_get_supported_cpuid(env, 0xd, count, R_EBX);
- *ecx = kvm_arch_get_supported_cpuid(env, 0xd, count, R_ECX);
- *edx = kvm_arch_get_supported_cpuid(env, 0xd, count, R_EDX);
+ KVMState *s = env->kvm_state;
+
+ *eax = kvm_arch_get_supported_cpuid(s, 0xd, count, R_EAX);
+ *ebx = kvm_arch_get_supported_cpuid(s, 0xd, count, R_EBX);
+ *ecx = kvm_arch_get_supported_cpuid(s, 0xd, count, R_ECX);
+ *edx = kvm_arch_get_supported_cpuid(s, 0xd, count, R_EDX);
} else {
*eax = 0;
*ebx = 0;
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 1ae2d61..fbdf612 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -106,12 +106,12 @@ struct kvm_para_features {
{ -1, -1 }
};
-static int get_para_features(CPUState *env)
+static int get_para_features(KVMState *s)
{
int i, features = 0;
for (i = 0; i < ARRAY_SIZE(para_features) - 1; i++) {
- if (kvm_check_extension(env->kvm_state, para_features[i].cap)) {
+ if (kvm_check_extension(s, para_features[i].cap)) {
features |= (1 << para_features[i].feature);
}
}
@@ -121,7 +121,7 @@ static int get_para_features(CPUState *env)
#endif
-uint32_t kvm_arch_get_supported_cpuid(CPUState *env, uint32_t function,
+uint32_t kvm_arch_get_supported_cpuid(KVMState *s, uint32_t function,
uint32_t index, int reg)
{
struct kvm_cpuid2 *cpuid;
@@ -133,7 +133,7 @@ uint32_t kvm_arch_get_supported_cpuid(CPUState *env, uint32_t function,
#endif
max = 1;
- while ((cpuid = try_get_cpuid(env->kvm_state, max)) == NULL) {
+ while ((cpuid = try_get_cpuid(s, max)) == NULL) {
max *= 2;
}
@@ -166,7 +166,7 @@ uint32_t kvm_arch_get_supported_cpuid(CPUState *env, uint32_t function,
/* On Intel, kvm returns cpuid according to the Intel spec,
* so add missing bits according to the AMD spec:
*/
- cpuid_1_edx = kvm_arch_get_supported_cpuid(env, 1, 0, R_EDX);
+ cpuid_1_edx = kvm_arch_get_supported_cpuid(s, 1, 0, R_EDX);
ret |= cpuid_1_edx & 0x183f7ff;
break;
}
@@ -180,7 +180,7 @@ uint32_t kvm_arch_get_supported_cpuid(CPUState *env, uint32_t function,
#ifdef CONFIG_KVM_PARA
/* fallback for older kernels */
if (!has_kvm_features && (function == KVM_CPUID_FEATURES)) {
- ret = get_para_features(env);
+ ret = get_para_features(s);
}
#endif
@@ -374,6 +374,7 @@ int kvm_arch_init_vcpu(CPUState *env)
struct kvm_cpuid2 cpuid;
struct kvm_cpuid_entry2 entries[100];
} __attribute__((packed)) cpuid_data;
+ KVMState *s = env->kvm_state;
uint32_t limit, i, j, cpuid_i;
uint32_t unused;
struct kvm_cpuid_entry2 *c;
@@ -381,20 +382,19 @@ int kvm_arch_init_vcpu(CPUState *env)
uint32_t signature[3];
#endif
- env->cpuid_features &= kvm_arch_get_supported_cpuid(env, 1, 0, R_EDX);
+ env->cpuid_features &= kvm_arch_get_supported_cpuid(s, 1, 0, R_EDX);
i = env->cpuid_ext_features & CPUID_EXT_HYPERVISOR;
- env->cpuid_ext_features &= kvm_arch_get_supported_cpuid(env, 1, 0, R_ECX);
+ env->cpuid_ext_features &= kvm_arch_get_supported_cpuid(s, 1, 0, R_ECX);
env->cpuid_ext_features |= i;
- env->cpuid_ext2_features &= kvm_arch_get_supported_cpuid(env, 0x80000001,
+ env->cpuid_ext2_features &= kvm_arch_get_supported_cpuid(s, 0x80000001,
0, R_EDX);
- env->cpuid_ext3_features &= kvm_arch_get_supported_cpuid(env, 0x80000001,
+ env->cpuid_ext3_features &= kvm_arch_get_supported_cpuid(s, 0x80000001,
0, R_ECX);
- env->cpuid_svm_features &= kvm_arch_get_supported_cpuid(env, 0x8000000A,
+ env->cpuid_svm_features &= kvm_arch_get_supported_cpuid(s, 0x8000000A,
0, R_EDX);
-
cpuid_i = 0;
#ifdef CONFIG_KVM_PARA
@@ -411,8 +411,8 @@ int kvm_arch_init_vcpu(CPUState *env)
c = &cpuid_data.entries[cpuid_i++];
memset(c, 0, sizeof(*c));
c->function = KVM_CPUID_FEATURES;
- c->eax = env->cpuid_kvm_features & kvm_arch_get_supported_cpuid(env,
- KVM_CPUID_FEATURES, 0, R_EAX);
+ c->eax = env->cpuid_kvm_features &
+ kvm_arch_get_supported_cpuid(s, KVM_CPUID_FEATURES, 0, R_EAX);
#ifdef KVM_CAP_ASYNC_PF
has_msr_async_pf_en = c->eax & (1 << KVM_FEATURE_ASYNC_PF);
@@ -485,7 +485,7 @@ int kvm_arch_init_vcpu(CPUState *env)
/* Call Centaur's CPUID instructions they are supported. */
if (env->cpuid_xlevel2 > 0) {
env->cpuid_ext4_features &=
- kvm_arch_get_supported_cpuid(env, 0xC0000001, 0, R_EDX);
+ kvm_arch_get_supported_cpuid(s, 0xC0000001, 0, R_EDX);
cpu_x86_cpuid(env, 0xC0000000, 0, &limit, &unused, &unused, &unused);
for (i = 0xC0000000; i <= limit; i++) {
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
^ permalink raw reply related [flat|nested] 3+ messages in thread