The feature-words properties are not used in user-mode emulation,
restrict it to system-mode.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
target/i386/cpu.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 2eec53ca22..9f72342506 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4607,6 +4607,7 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor *v, const char *name,
cpu->env.tsc_khz = cpu->env.user_tsc_khz = value / 1000;
}
+#if !defined(CONFIG_USER_ONLY)
/* Generic getter for "feature-words" and "filtered-features" properties */
static void x86_cpu_get_feature_words(Object *obj, Visitor *v,
const char *name, void *opaque,
@@ -4666,6 +4667,7 @@ static const char *x86_cpu_feature_name(FeatureWord w, int bitnr)
assert(bitnr < 32 || !(name && feature_word_info[w].type == CPUID_FEATURE_WORD));
return name;
}
+#endif /* CONFIG_USER_ONLY */
/*
* Convert all '_' in a feature string option name to '-', to make feature
@@ -4783,6 +4785,7 @@ static void x86_cpu_parse_featurestr(const char *typename, char *features,
static void x86_cpu_expand_features(X86CPU *cpu, Error **errp);
static void x86_cpu_filter_features(X86CPU *cpu, bool verbose);
+#if !defined(CONFIG_USER_ONLY)
/* Build a list with the name of all features on a feature word array */
static void x86_cpu_list_feature_names(FeatureWordArray features,
strList **feat_names)
@@ -4853,6 +4856,7 @@ static void x86_cpu_class_check_missing_features(X86CPUClass *xcc,
object_unref(OBJECT(xc));
}
+#endif /* CONFIG_USER_ONLY */
/* Print all cpuid feature names in featureset
*/
@@ -4987,7 +4991,9 @@ static void x86_cpu_definition_entry(gpointer data, gpointer user_data)
info = g_malloc0(sizeof(*info));
info->name = x86_cpu_class_get_model_name(cc);
+#if !defined(CONFIG_USER_ONLY)
x86_cpu_class_check_missing_features(cc, &info->unavailable_features);
+#endif
info->has_unavailable_features = true;
info->q_typename = g_strdup(object_class_get_name(oc));
info->migration_safe = cc->migration_safe;
@@ -6941,6 +6947,7 @@ static void x86_cpu_initfn(Object *obj)
object_property_add(obj, "tsc-frequency", "int",
x86_cpuid_get_tsc_freq,
x86_cpuid_set_tsc_freq, NULL, NULL);
+#if !defined(CONFIG_USER_ONLY)
object_property_add(obj, "feature-words", "X86CPUFeatureWordInfo",
x86_cpu_get_feature_words,
NULL, NULL, (void *)env->features);
@@ -6957,7 +6964,6 @@ static void x86_cpu_initfn(Object *obj)
x86_cpu_get_unavailable_features,
NULL, NULL, NULL);
-#if !defined(CONFIG_USER_ONLY)
object_property_add(obj, "crash-information", "GuestPanicInformation",
x86_cpu_get_crash_info_qom, NULL, NULL, NULL);
#endif
--
2.26.2
On 10/1/20 4:41 PM, Philippe Mathieu-Daudé wrote: > The feature-words properties are not used in user-mode emulation, > restrict it to system-mode. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > target/i386/cpu.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > index 2eec53ca22..9f72342506 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -4607,6 +4607,7 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor *v, const char *name, > cpu->env.tsc_khz = cpu->env.user_tsc_khz = value / 1000; > } > > +#if !defined(CONFIG_USER_ONLY) > /* Generic getter for "feature-words" and "filtered-features" properties */ > static void x86_cpu_get_feature_words(Object *obj, Visitor *v, > const char *name, void *opaque, > @@ -4666,6 +4667,7 @@ static const char *x86_cpu_feature_name(FeatureWord w, int bitnr) > assert(bitnr < 32 || !(name && feature_word_info[w].type == CPUID_FEATURE_WORD)); > return name; > } > +#endif /* CONFIG_USER_ONLY */ > > /* > * Convert all '_' in a feature string option name to '-', to make feature > @@ -4783,6 +4785,7 @@ static void x86_cpu_parse_featurestr(const char *typename, char *features, > static void x86_cpu_expand_features(X86CPU *cpu, Error **errp); > static void x86_cpu_filter_features(X86CPU *cpu, bool verbose); > > +#if !defined(CONFIG_USER_ONLY) > /* Build a list with the name of all features on a feature word array */ > static void x86_cpu_list_feature_names(FeatureWordArray features, > strList **feat_names) > @@ -4853,6 +4856,7 @@ static void x86_cpu_class_check_missing_features(X86CPUClass *xcc, > > object_unref(OBJECT(xc)); > } > +#endif /* CONFIG_USER_ONLY */ > > /* Print all cpuid feature names in featureset > */ > @@ -4987,7 +4991,9 @@ static void x86_cpu_definition_entry(gpointer data, gpointer user_data) > > info = g_malloc0(sizeof(*info)); > info->name = x86_cpu_class_get_model_name(cc); > +#if !defined(CONFIG_USER_ONLY) > x86_cpu_class_check_missing_features(cc, &info->unavailable_features); > +#endif Buh... > info->has_unavailable_features = true; ... I guess here goes the: #endif > info->q_typename = g_strdup(object_class_get_name(oc)); > info->migration_safe = cc->migration_safe; > @@ -6941,6 +6947,7 @@ static void x86_cpu_initfn(Object *obj) > object_property_add(obj, "tsc-frequency", "int", > x86_cpuid_get_tsc_freq, > x86_cpuid_set_tsc_freq, NULL, NULL); > +#if !defined(CONFIG_USER_ONLY) > object_property_add(obj, "feature-words", "X86CPUFeatureWordInfo", > x86_cpu_get_feature_words, > NULL, NULL, (void *)env->features); > @@ -6957,7 +6964,6 @@ static void x86_cpu_initfn(Object *obj) > x86_cpu_get_unavailable_features, > NULL, NULL, NULL); > > -#if !defined(CONFIG_USER_ONLY) > object_property_add(obj, "crash-information", "GuestPanicInformation", > x86_cpu_get_crash_info_qom, NULL, NULL, NULL); > #endif >
On 01/10/20 16:41, Philippe Mathieu-Daudé wrote: > The feature-words properties are not used in user-mode emulation, > restrict it to system-mode. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> This seems a bit pointless honestly. It's going to be a few KBs of code at most. I would end the work with the patches that have already been queued. Paolo > --- > target/i386/cpu.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > index 2eec53ca22..9f72342506 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -4607,6 +4607,7 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor *v, const char *name, > cpu->env.tsc_khz = cpu->env.user_tsc_khz = value / 1000; > } > > +#if !defined(CONFIG_USER_ONLY) > /* Generic getter for "feature-words" and "filtered-features" properties */ > static void x86_cpu_get_feature_words(Object *obj, Visitor *v, > const char *name, void *opaque, > @@ -4666,6 +4667,7 @@ static const char *x86_cpu_feature_name(FeatureWord w, int bitnr) > assert(bitnr < 32 || !(name && feature_word_info[w].type == CPUID_FEATURE_WORD)); > return name; > } > +#endif /* CONFIG_USER_ONLY */ > > /* > * Convert all '_' in a feature string option name to '-', to make feature > @@ -4783,6 +4785,7 @@ static void x86_cpu_parse_featurestr(const char *typename, char *features, > static void x86_cpu_expand_features(X86CPU *cpu, Error **errp); > static void x86_cpu_filter_features(X86CPU *cpu, bool verbose); > > +#if !defined(CONFIG_USER_ONLY) > /* Build a list with the name of all features on a feature word array */ > static void x86_cpu_list_feature_names(FeatureWordArray features, > strList **feat_names) > @@ -4853,6 +4856,7 @@ static void x86_cpu_class_check_missing_features(X86CPUClass *xcc, > > object_unref(OBJECT(xc)); > } > +#endif /* CONFIG_USER_ONLY */ > > /* Print all cpuid feature names in featureset > */ > @@ -4987,7 +4991,9 @@ static void x86_cpu_definition_entry(gpointer data, gpointer user_data) > > info = g_malloc0(sizeof(*info)); > info->name = x86_cpu_class_get_model_name(cc); > +#if !defined(CONFIG_USER_ONLY) > x86_cpu_class_check_missing_features(cc, &info->unavailable_features); > +#endif > info->has_unavailable_features = true; > info->q_typename = g_strdup(object_class_get_name(oc)); > info->migration_safe = cc->migration_safe; > @@ -6941,6 +6947,7 @@ static void x86_cpu_initfn(Object *obj) > object_property_add(obj, "tsc-frequency", "int", > x86_cpuid_get_tsc_freq, > x86_cpuid_set_tsc_freq, NULL, NULL); > +#if !defined(CONFIG_USER_ONLY) > object_property_add(obj, "feature-words", "X86CPUFeatureWordInfo", > x86_cpu_get_feature_words, > NULL, NULL, (void *)env->features); > @@ -6957,7 +6964,6 @@ static void x86_cpu_initfn(Object *obj) > x86_cpu_get_unavailable_features, > NULL, NULL, NULL); > > -#if !defined(CONFIG_USER_ONLY) > object_property_add(obj, "crash-information", "GuestPanicInformation", > x86_cpu_get_crash_info_qom, NULL, NULL, NULL); > #endif >
On 10/1/20 5:14 PM, Paolo Bonzini wrote: > On 01/10/20 16:41, Philippe Mathieu-Daudé wrote: >> The feature-words properties are not used in user-mode emulation, >> restrict it to system-mode. >> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > > This seems a bit pointless honestly. It's going to be a few KBs of code > at most. I would end the work with the patches that have already been > queued. I'll let Eduardo and you discuss that, as he suggested the #ifdef instead of the stub. The two options are on the list, whichever you guys prefer :) > > Paolo > >> --- >> target/i386/cpu.c | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/target/i386/cpu.c b/target/i386/cpu.c >> index 2eec53ca22..9f72342506 100644 >> --- a/target/i386/cpu.c >> +++ b/target/i386/cpu.c >> @@ -4607,6 +4607,7 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor *v, const char *name, >> cpu->env.tsc_khz = cpu->env.user_tsc_khz = value / 1000; >> } >> >> +#if !defined(CONFIG_USER_ONLY) >> /* Generic getter for "feature-words" and "filtered-features" properties */ >> static void x86_cpu_get_feature_words(Object *obj, Visitor *v, >> const char *name, void *opaque, >> @@ -4666,6 +4667,7 @@ static const char *x86_cpu_feature_name(FeatureWord w, int bitnr) >> assert(bitnr < 32 || !(name && feature_word_info[w].type == CPUID_FEATURE_WORD)); >> return name; >> } >> +#endif /* CONFIG_USER_ONLY */ >> >> /* >> * Convert all '_' in a feature string option name to '-', to make feature >> @@ -4783,6 +4785,7 @@ static void x86_cpu_parse_featurestr(const char *typename, char *features, >> static void x86_cpu_expand_features(X86CPU *cpu, Error **errp); >> static void x86_cpu_filter_features(X86CPU *cpu, bool verbose); >> >> +#if !defined(CONFIG_USER_ONLY) >> /* Build a list with the name of all features on a feature word array */ >> static void x86_cpu_list_feature_names(FeatureWordArray features, >> strList **feat_names) >> @@ -4853,6 +4856,7 @@ static void x86_cpu_class_check_missing_features(X86CPUClass *xcc, >> >> object_unref(OBJECT(xc)); >> } >> +#endif /* CONFIG_USER_ONLY */ >> >> /* Print all cpuid feature names in featureset >> */ >> @@ -4987,7 +4991,9 @@ static void x86_cpu_definition_entry(gpointer data, gpointer user_data) >> >> info = g_malloc0(sizeof(*info)); >> info->name = x86_cpu_class_get_model_name(cc); >> +#if !defined(CONFIG_USER_ONLY) >> x86_cpu_class_check_missing_features(cc, &info->unavailable_features); >> +#endif >> info->has_unavailable_features = true; >> info->q_typename = g_strdup(object_class_get_name(oc)); >> info->migration_safe = cc->migration_safe; >> @@ -6941,6 +6947,7 @@ static void x86_cpu_initfn(Object *obj) >> object_property_add(obj, "tsc-frequency", "int", >> x86_cpuid_get_tsc_freq, >> x86_cpuid_set_tsc_freq, NULL, NULL); >> +#if !defined(CONFIG_USER_ONLY) >> object_property_add(obj, "feature-words", "X86CPUFeatureWordInfo", >> x86_cpu_get_feature_words, >> NULL, NULL, (void *)env->features); >> @@ -6957,7 +6964,6 @@ static void x86_cpu_initfn(Object *obj) >> x86_cpu_get_unavailable_features, >> NULL, NULL, NULL); >> >> -#if !defined(CONFIG_USER_ONLY) >> object_property_add(obj, "crash-information", "GuestPanicInformation", >> x86_cpu_get_crash_info_qom, NULL, NULL, NULL); >> #endif >> >
On Thu, Oct 01, 2020 at 05:27:32PM +0200, Philippe Mathieu-Daudé wrote: > On 10/1/20 5:14 PM, Paolo Bonzini wrote: > > On 01/10/20 16:41, Philippe Mathieu-Daudé wrote: > >> The feature-words properties are not used in user-mode emulation, > >> restrict it to system-mode. > >> > >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > > > > This seems a bit pointless honestly. It's going to be a few KBs of code > > at most. I would end the work with the patches that have already been > > queued. > > I'll let Eduardo and you discuss that, as he suggested the #ifdef > instead of the stub. The two options are on the list, whichever > you guys prefer :) Having neither the stub or the #ifdef is even better. :) I assumed the stub was necessary as a means for an end, but now I don't understand why the stub was necessary in the first place. -- Eduardo
On 10/1/20 5:37 PM, Eduardo Habkost wrote: > On Thu, Oct 01, 2020 at 05:27:32PM +0200, Philippe Mathieu-Daudé wrote: >> On 10/1/20 5:14 PM, Paolo Bonzini wrote: >>> On 01/10/20 16:41, Philippe Mathieu-Daudé wrote: >>>> The feature-words properties are not used in user-mode emulation, >>>> restrict it to system-mode. >>>> >>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >>> >>> This seems a bit pointless honestly. It's going to be a few KBs of code >>> at most. I would end the work with the patches that have already been >>> queued. >> >> I'll let Eduardo and you discuss that, as he suggested the #ifdef >> instead of the stub. The two options are on the list, whichever >> you guys prefer :) > > Having neither the stub or the #ifdef is even better. :) > > I assumed the stub was necessary as a means for an end, but now I > don't understand why the stub was necessary in the first place. x86_cpu_get_feature_words() calls QAPI-generated visit_type_X86CPUFeatureWordInfoList(). Since "Only qemu-system-FOO and qemu-storage-daemon provide QMP monitors", this code is unreachable on user-mode. I tried to get a "no ifdef/no stub" codebase by not removing the QAPI uses from user-mode code, and build it without QAPI-generated files. I could only start the work and realized it is a huge effort. In particular since nobody sees the point of doing this cleanup... Regards, Phil.
On Thu, Oct 01, 2020 at 05:57:14PM +0200, Philippe Mathieu-Daudé wrote: > On 10/1/20 5:37 PM, Eduardo Habkost wrote: > > On Thu, Oct 01, 2020 at 05:27:32PM +0200, Philippe Mathieu-Daudé wrote: > >> On 10/1/20 5:14 PM, Paolo Bonzini wrote: > >>> On 01/10/20 16:41, Philippe Mathieu-Daudé wrote: > >>>> The feature-words properties are not used in user-mode emulation, > >>>> restrict it to system-mode. > >>>> > >>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > >>> > >>> This seems a bit pointless honestly. It's going to be a few KBs of code > >>> at most. I would end the work with the patches that have already been > >>> queued. > >> > >> I'll let Eduardo and you discuss that, as he suggested the #ifdef > >> instead of the stub. The two options are on the list, whichever > >> you guys prefer :) > > > > Having neither the stub or the #ifdef is even better. :) > > > > I assumed the stub was necessary as a means for an end, but now I > > don't understand why the stub was necessary in the first place. > > x86_cpu_get_feature_words() calls QAPI-generated > visit_type_X86CPUFeatureWordInfoList(). > > Since "Only qemu-system-FOO and qemu-storage-daemon provide QMP > monitors", this code is unreachable on user-mode. This is true, but you are shaving a tiny fraction of target/i386/cpu.c code unreachable in user-mode. I agree with Paolo that it's pointless. > > I tried to get a "no ifdef/no stub" codebase by not removing the > QAPI uses from user-mode code, and build it without QAPI-generated > files. I could only start the work and realized it is a huge effort. > In particular since nobody sees the point of doing this cleanup... I don't see the point of removing QAPI-generated files from user-mode. Long term, I'd expect to see more QAPI usage in QEMU, which will probably include code used by user-mode. -- Eduardo
© 2016 - 2026 Red Hat, Inc.