From patchwork Fri Jun 3 14:51:15 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Kiszka X-Patchwork-Id: 847132 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter2.kernel.org (8.14.4/8.14.3) with ESMTP id p53EpTXB014167 for ; Fri, 3 Jun 2011 14:51:29 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753392Ab1FCOv0 (ORCPT ); Fri, 3 Jun 2011 10:51:26 -0400 Received: from david.siemens.de ([192.35.17.14]:25237 "EHLO david.siemens.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752370Ab1FCOvY (ORCPT ); Fri, 3 Jun 2011 10:51:24 -0400 Received: from mail1.siemens.de (localhost [127.0.0.1]) by david.siemens.de (8.13.6/8.13.6) with ESMTP id p53EpG1G007797; Fri, 3 Jun 2011 16:51:16 +0200 Received: from mchn199C.mchp.siemens.de ([139.25.109.49]) by mail1.siemens.de (8.13.6/8.13.6) with ESMTP id p53EpFfB024442; Fri, 3 Jun 2011 16:51:15 +0200 Message-ID: <4DE8F4E3.1000300@siemens.com> Date: Fri, 03 Jun 2011 16:51:15 +0200 From: Jan Kiszka User-Agent: Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666 MIME-Version: 1.0 To: Anthony Liguori , qemu-devel@nongnu.org, Marcelo Tosatti , Avi Kivity , kvm@vger.kernel.org Subject: Re: [PATCH 00/11] cpu model bug fixes and definition corrections (v2) References: <1307041990-26194-1-git-send-email-ehabkost@redhat.com> <20110602193458.GA7807@otherpad.lan.raisama.net> <4DE813FE.5030505@web.de> <20110603143839.GA26905@otherpad.lan.raisama.net> In-Reply-To: <20110603143839.GA26905@otherpad.lan.raisama.net> Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter2.kernel.org [140.211.167.43]); Fri, 03 Jun 2011 14:51:30 +0000 (UTC) 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 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 --- 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++) {