On Mon, 10 Oct 2016 14:01:10 -0300
Eduardo Habkost <ehabkost@xxxxxxxxxx> 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@xxxxxxxxxx> wrote:
> >
> > > Fill the "unavailable-features" field on the x86 implementation
> > > of query-cpu-definitions.
> > >
> > > Cc: Jiri Denemark <jdenemar@xxxxxxxxxx>
> > > Cc: libvir-list@xxxxxxxxxx
> > > Signed-off-by: Eduardo Habkost <ehabkost@xxxxxxxxxx>
> > > ---
> > > Changes v5 -> v6:
> > > * Call x86_cpu_filter_features(), now that x86_cpu_load_features()
> > > won't run it automatically
> > >
> > > Changes v4 -> v5:
> > > * (none)
> > >
> > > Changes v3 -> v4:
> > > * Handle missing XSAVE components cleanly, but looking up
> > > the original feature that required it
> > > * Use x86_cpu_load_features() function
> > >
> > > Changes v2 -> v3:
> > > * Create a x86_cpu_feature_name() function, to
> > > isolate the code that returns the property name
> > >
> > > Changes v1 -> v2:
> > > * Updated to the new schema: no @runnable field, and
> > > always report @unavailable-features as present
> > > ---
> > > target-i386/cpu.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 76 insertions(+)
> > >
> > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > > index 23cc19b..63330ce 100644
> > > --- a/target-i386/cpu.c
> > > +++ b/target-i386/cpu.c
> > > @@ -1945,6 +1945,27 @@ static inline void feat2prop(char *s)
> > > }
> > > }
> > >
> > > +/* Return the feature property name for a feature flag bit */
> > > +static const char *x86_cpu_feature_name(FeatureWord w, int bitnr)
> > > +{
> > > + /* XSAVE components are automatically enabled by other features,
> > > + * so return the original feature name instead
> > > + */
> > > + if (w == FEAT_XSAVE_COMP_LO || w == FEAT_XSAVE_COMP_HI) {
> > > + int comp = (w == FEAT_XSAVE_COMP_HI) ? bitnr + 32 : bitnr;
> > > +
> > > + if (comp < ARRAY_SIZE(x86_ext_save_areas) &&
> > > + x86_ext_save_areas[comp].bits) {
> > > + w = x86_ext_save_areas[comp].feature;
> > > + bitnr = ctz32(x86_ext_save_areas[comp].bits);
> > > + }
> > > + }
> > > +
> > > + assert(bitnr < 32);
> > > + assert(w < FEATURE_WORDS);
> > > + return feature_word_info[w].feat_names[bitnr];
> > > +}
> > > +
> > > /* Compatibily hack to maintain legacy +-feat semantic,
> > > * where +-feat overwrites any feature set by
> > > * feat=on|feat even if the later is parsed after +-feat
> > > @@ -2030,6 +2051,59 @@ static void x86_cpu_parse_featurestr(const char *typename, char *features,
> > > }
> > > }
> > >
> > > +static void x86_cpu_load_features(X86CPU *cpu, Error **errp);
> > > +static int x86_cpu_filter_features(X86CPU *cpu);
> > > +
> > > +/* Check for missing features that may prevent the CPU class from
> > > + * running using the current machine and accelerator.
> > > + */
> > > +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.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list