From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43032) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1btxTM-0006QM-Mi for qemu-devel@nongnu.org; Tue, 11 Oct 2016 09:52:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1btxTH-0000ld-Ue for qemu-devel@nongnu.org; Tue, 11 Oct 2016 09:52:00 -0400 Received: from mx1.redhat.com ([209.132.183.28]:1964) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1btxTH-0000lF-LJ for qemu-devel@nongnu.org; Tue, 11 Oct 2016 09:51:55 -0400 Date: Tue, 11 Oct 2016 15:51:51 +0200 From: Igor Mammedov Message-ID: <20161011155151.36876570@nial.brq.redhat.com> In-Reply-To: <20161011132438.GB3847@thinpad.lan.raisama.net> References: <1475872142-3986-1-git-send-email-ehabkost@redhat.com> <1475872142-3986-4-git-send-email-ehabkost@redhat.com> <20161010142749.47282a0a@nial.brq.redhat.com> <20161010170110.GE22870@thinpad.lan.raisama.net> <20161011134521.60ae8d70@nial.brq.redhat.com> <20161011115802.GA3847@thinpad.lan.raisama.net> <20161011152105.16269348@nial.brq.redhat.com> <20161011132438.GB3847@thinpad.lan.raisama.net> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v6 3/3] target-i386: Return runnability information on query-cpu-definitions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: qemu-devel@nongnu.org, dahi@linux.vnet.ibm.com, Paolo Bonzini , Jiri Denemark , Markus Armbruster , Richard Henderson , libvir-list@redhat.com On Tue, 11 Oct 2016 10:24:38 -0300 Eduardo Habkost wrote: > On Tue, Oct 11, 2016 at 03:21:05PM +0200, Igor Mammedov wrote: > > On Tue, 11 Oct 2016 08:58:02 -0300 > > Eduardo Habkost wrote: > > =20 > > > On Tue, Oct 11, 2016 at 01:45:21PM +0200, Igor Mammedov wrote: =20 > > > > On Mon, 10 Oct 2016 14:01:10 -0300 > > > > Eduardo Habkost wrote: =20 > > > > > On Mon, Oct 10, 2016 at 02:27:49PM +0200, Igor Mammedov wrote: = =20 > > > > > > On Fri, 7 Oct 2016 17:29:02 -0300 > > > > > > Eduardo Habkost wrote: =20 > > > [...] =20 > > > > > > > +static void x86_cpu_class_check_missing_features(X86CPUClass= *xcc, > > > > > > > + strList **m= issing_feats) > > > > > > > +{ > > > > > > > + X86CPU *xc; > > > > > > > + FeatureWord w; > > > > > > > + Error *err =3D NULL; > > > > > > > + strList **next =3D missing_feats; > > > > > > > + > > > > > > > + if (xcc->kvm_required && !kvm_enabled()) { > > > > > > > + strList *new =3D g_new0(strList, 1); > > > > > > > + new->value =3D g_strdup("kvm");; > > > > > > > + *missing_feats =3D new; > > > > > > > + return; > > > > > > > + } > > > > > > > + > > > > > > > + xc =3D X86_CPU(object_new(object_class_get_name(OBJECT_C= LASS(xcc)))); > > > > > > > + > > > > > > > + x86_cpu_load_features(xc, &err); > > > > > > > + if (err) { > > > > > > > + /* Errors at x86_cpu_load_features should never happ= en, > > > > > > > + * but in case it does, just report the model as not > > > > > > > + * runnable at all using the "type" property. > > > > > > > + */ > > > > > > > + strList *new =3D g_new0(strList, 1); > > > > > > > + new->value =3D g_strdup("type"); > > > > > > > + *next =3D new; > > > > > > > + next =3D &new->next; > > > > > > > + } > > > > > > > + > > > > > > > + x86_cpu_filter_features(xc); > > > > > > > + > > > > > > > + for (w =3D 0; w < FEATURE_WORDS; w++) { > > > > > > > + uint32_t filtered =3D xc->filtered_features[w]; > > > > > > > + int i; > > > > > > > + for (i =3D 0; i < 32; i++) { > > > > > > > + if (filtered & (1UL << i)) { > > > > > > > + strList *new =3D g_new0(strList, 1); > > > > > > > + new->value =3D g_strdup(x86_cpu_feature_name= (w, i)); > > > > > > > + *next =3D new; > > > > > > > + next =3D &new->next; > > > > > > > + } > > > > > > > + } > > > > > > > + } =20 > > > > > > Shouldn't you add=20 > > > > > > if (IS_AMD_CPU(env)) {=20 > > > > > > fixup here, that realize does right after calling x86_cpu_filte= r_features() =20 > > > > >=20 > > > > > What would it be useful for? The IS_AMD_CPU fixup runs after > > > > > x86_cpu_filter_features() (so it doesn't affect filtered_features > > > > > at all), and filtered_features is the only field used as input to > > > > > build missing_feats. =20 > > > > For completeness of features returned by query-cpu-definitions, I'd= guess. > > > > So that returned cpu definitions would match actually created cpus.= =20 > > >=20 > > > For completeness of which query-cpu-definitions field, exactly? > > > There's no field in the return value of query-cpu-definitions > > > that would be affected by the AMD aliases. The AMD aliases don't > > > affect runnability of the CPU model because they aren't included > > > in the GET_SUPPORTED_CPUID check[1]. > > >=20 > > > You would be right if we did return a copy of the low-level CPUID > > > data that's seen by the guest, or if the AMD aliases were set up > > > before x86_cpu_filter_features() (so they could affect > > > filtered_features/unavailable-features), but that's not the case. > > >=20 > > > [1] They aren't included in the GET_SUPPORTED_CPUID check because > > > the existence of the AMD aliases depend only on the > > > configured guest vendor ID, not on the host CPU. > > > =20 > > Got it. > >=20 > > I've tried to build with this patch but build fails with > >=20 > > make -j32 > > CHK version_gen.h > > CC i386-linux-user/target-i386/cpu.o > > target-i386/cpu.c: In function =E2=80=98x86_cpu_definition_en= try=E2=80=99: > > target-i386/cpu.c:2199:51: error: =E2=80=98CpuDefinitionInfo= =E2=80=99 has no member named =E2=80=98unavailable_features=E2=80=99 > > x86_cpu_class_check_missing_features(cc, &info->unavailable_featur= es); > > ^ > > target-i386/cpu.c:2200:9: error: =E2=80=98CpuDefinitionInfo= =E2=80=99 has no member named =E2=80=98has_unavailable_features=E2=80=99 > > info->has_unavailable_features =3D true; > >=20 > > Probably series misses a patch that adds it. =20 >=20 > See git URLs on cover letter. Series is based on my x86-next branch. >=20 > ] This series can be seen in the git branch at: > ] https://github.com/ehabkost/qemu-hacks.git work/query-cpu-definitions= -runnable-info > ] > ] The series is based on my x86-next branch: > ] https://github.com/ehabkost/qemu.git x86-next I've used this one from yesterday as base and it didn't have "qmp: Add runnability information to query-cpu-definitions" I'll refetch and try again.