From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:57768) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TlNFk-0008DO-4G for qemu-devel@nongnu.org; Wed, 19 Dec 2012 12:16:21 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TlNFe-0003d2-Lm for qemu-devel@nongnu.org; Wed, 19 Dec 2012 12:16:20 -0500 Received: from mx1.redhat.com ([209.132.183.28]:35942) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TlNFe-0003cA-Eu for qemu-devel@nongnu.org; Wed, 19 Dec 2012 12:16:14 -0500 Date: Wed, 19 Dec 2012 18:16:08 +0100 From: Igor Mammedov Message-ID: <20121219181608.5f0e50d1@nial.usersys.redhat.com> In-Reply-To: <20121219164230.GK5334@otherpad.lan.raisama.net> References: <1355760092-18755-1-git-send-email-imammedo@redhat.com> <1355760092-18755-9-git-send-email-imammedo@redhat.com> <20121219164230.GK5334@otherpad.lan.raisama.net> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 08/20] target-i386: compile kvm only functions if CONFIG_KVM is defined List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: Don@CloudSwitch.com, qemu-devel@nongnu.org, afaerber@suse.de On Wed, 19 Dec 2012 14:42:31 -0200 Eduardo Habkost wrote: > On Mon, Dec 17, 2012 at 05:01:20PM +0100, Igor Mammedov wrote: > [...] > > > > static void x86_cpuid_version_get_family(Object *obj, Visitor *v, void > > *opaque, const char *name, Error **errp) > > @@ -1273,7 +1271,9 @@ static int cpu_x86_find_by_name(x86_def_t > > *x86_cpu_def, const char *name) } > > } > > if (kvm_enabled() && name && strcmp(name, "host") == 0) { > > +#ifdef CONFIG_KVM > > kvm_cpu_fill_host(x86_cpu_def); > > +#endif > > Is this really better than the existing code that generates an empty > stub function (that will never be called anyway)? Following patch that moves kvm_check_features_against_host() inside of #ifdef CONFIG_KVM in realize_fn(), makes build failure *-user with warning that kvm_check_features_against_host() is unused and if we make stub from kvm_check_features_against_host() as well then we will have to ifdef unavailable_host_feature() as well. As result it makes +2 more ifdefs one of which excludes whole function. This patch makes one big ifdef block of kvm specific functions and the next one keep amount of ifdef the same as before these patches. And as bonus we get cleaner build and won't get unused symbols & calls to empty functions on debug builds. > > I am not strongly inclined either way, but I prefer the existing style. > > > > } else if (!def) { > > return -1; > > } else { > > @@ -1428,10 +1428,12 @@ static int cpu_x86_parse_featurestr(x86_def_t > > *x86_cpu_def, char *features) x86_cpu_def->kvm_features &= > > ~minus_kvm_features; x86_cpu_def->svm_features &= ~minus_svm_features; > > x86_cpu_def->cpuid_7_0_ebx_features &= ~minus_7_0_ebx_features; > > +#ifdef CONFIG_KVM > > if (check_cpuid && kvm_enabled()) { > > if (kvm_check_features_against_host(x86_cpu_def) && > > enforce_cpuid) goto error; > > } > > +#endif > > return 0; > > > > error: > > -- > > 1.7.1 > > > > >