Re: [libvirt] [PATCH v6 3/3] target-i386: Return runnability information on query-cpu-definitions
On Tue, Oct 11, 2016 at 01:45:21PM +0200, Igor Mammedov wrote:
On Mon, 10 Oct 2016 14:01:10 -0300
Eduardo Habkost <ehabkost(a)redhat.com> wrote:
> On Mon, Oct 10, 2016 at 02:27:49PM +0200, Igor Mammedov wrote:
> > On Fri, 7 Oct 2016 17:29:02 -0300
> > Eduardo Habkost <ehabkost(a)redhat.com> wrote:
[...]
> > > +static void
x86_cpu_class_check_missing_features(X86CPUClass *xcc,
> > > + strList
**missing_feats)
> > > +{
> > > + X86CPU *xc;
> > > + FeatureWord w;
> > > + Error *err = NULL;
> > > + strList **next = missing_feats;
> > > +
> > > + if (xcc->kvm_required && !kvm_enabled()) {
> > > + strList *new = g_new0(strList, 1);
> > > + new->value = g_strdup("kvm");;
> > > + *missing_feats = new;
> > > + return;
> > > + }
> > > +
> > > + xc = X86_CPU(object_new(object_class_get_name(OBJECT_CLASS(xcc))));
> > > +
> > > + x86_cpu_load_features(xc, &err);
> > > + if (err) {
> > > + /* Errors at x86_cpu_load_features should never happen,
> > > + * but in case it does, just report the model as not
> > > + * runnable at all using the "type" property.
> > > + */
> > > + strList *new = g_new0(strList, 1);
> > > + new->value = g_strdup("type");
> > > + *next = new;
> > > + next = &new->next;
> > > + }
> > > +
> > > + x86_cpu_filter_features(xc);
> > > +
> > > + for (w = 0; w < FEATURE_WORDS; w++) {
> > > + uint32_t filtered = xc->filtered_features[w];
> > > + int i;
> > > + for (i = 0; i < 32; i++) {
> > > + if (filtered & (1UL << i)) {
> > > + strList *new = g_new0(strList, 1);
> > > + new->value = g_strdup(x86_cpu_feature_name(w, i));
> > > + *next = new;
> > > + next = &new->next;
> > > + }
> > > + }
> > > + }
> > Shouldn't you add
> > if (IS_AMD_CPU(env)) {
> > fixup here, that realize does right after calling x86_cpu_filter_features()
>
> 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.
For completeness of features returned by query-cpu-definitions, I'd guess.
So that returned cpu definitions would match actually created cpus.
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].
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.
[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.
--
Eduardo