From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=34727 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Q2hlV-0006ac-LZ for qemu-devel@nongnu.org; Thu, 24 Mar 2011 06:27:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Q2hlP-0001Go-CR for qemu-devel@nongnu.org; Thu, 24 Mar 2011 06:27:36 -0400 Received: from mx1.redhat.com ([209.132.183.28]:13085) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q2hlO-0001GU-97 for qemu-devel@nongnu.org; Thu, 24 Mar 2011 06:27:34 -0400 Message-ID: <4D8B1C8F.7060302@redhat.com> Date: Thu, 24 Mar 2011 12:27:27 +0200 From: Avi Kivity MIME-Version: 1.0 References: <1300401727-5235-1-git-send-email-glommer@redhat.com> <1300401727-5235-2-git-send-email-glommer@redhat.com> In-Reply-To: <1300401727-5235-2-git-send-email-glommer@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH 1/3] use kernel-provided para_features instead of statically coming up with new capabilities List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Glauber Costa Cc: mtosatti@redhat.com, qemu-devel@nongnu.org, kvm@vger.kernel.org On 03/18/2011 12:42 AM, Glauber Costa wrote: > According to Avi's comments over my last submission, I decided to take a > different, and more correct direction - we hope. > > This patch is now using the features provided by KVM_GET_SUPPORTED_CPUID directly to > mask out features from guest-visible cpuid. > > The old get_para_features() mechanism is kept for older kernels that do not implement it. > > > > +#ifdef CONFIG_KVM_PARA > +struct kvm_para_features { > + int cap; > + int feature; > +} para_features[] = { > + { KVM_CAP_CLOCKSOURCE, KVM_FEATURE_CLOCKSOURCE }, > + { KVM_CAP_NOP_IO_DELAY, KVM_FEATURE_NOP_IO_DELAY }, > + { KVM_CAP_PV_MMU, KVM_FEATURE_MMU_OP }, > +#ifdef KVM_CAP_ASYNC_PF > + { KVM_CAP_ASYNC_PF, KVM_FEATURE_ASYNC_PF }, > +#endif Shouldn't the others get the same #ifdef treatment? Yes, we depend on a kernels of a certain age, but let's not add more dependencies. > + { -1, -1 } > +}; Since you use ARRAY_SIZE() later, don't need the guard here. > + > +static int get_para_features(CPUState *env) > +{ > + int i, features = 0; > + > + for (i = 0; i< ARRAY_SIZE(para_features) - 1; i++) { So you can drop the - 1. > + if (kvm_check_extension(env->kvm_state, para_features[i].cap)) { > + features |= (1<< para_features[i].feature); > + } > + } > + > + return features; > +} > +#endif > + > + > uint32_t kvm_arch_get_supported_cpuid(CPUState *env, uint32_t function, > uint32_t index, int reg) > { > > > -#ifdef CONFIG_KVM_PARA > -struct kvm_para_features { > - int cap; > - int feature; > -} para_features[] = { > - { KVM_CAP_CLOCKSOURCE, KVM_FEATURE_CLOCKSOURCE }, > - { KVM_CAP_NOP_IO_DELAY, KVM_FEATURE_NOP_IO_DELAY }, > - { KVM_CAP_PV_MMU, KVM_FEATURE_MMU_OP }, > -#ifdef KVM_CAP_ASYNC_PF > - { KVM_CAP_ASYNC_PF, KVM_FEATURE_ASYNC_PF }, > -#endif > - { -1, -1 } > -}; > - > -static int get_para_features(CPUState *env) > -{ > - 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)) { > - features |= (1<< para_features[i].feature); > - } > - } > -#ifdef KVM_CAP_ASYNC_PF > - has_msr_async_pf_en = features& (1<< KVM_FEATURE_ASYNC_PF); > -#endif > - return features; > -} > -#endif > - Oh. The whole thing was copied wholesale. Nevermind. -- error compiling committee.c: too many arguments to function