* [PATCH v4 0/3] target/i386: Restrict 'feature-words' property to x86 machines
@ 2020-10-01 14:41 Philippe Mathieu-Daudé
2020-10-01 14:41 ` [PATCH v4 1/3] target/i386/cpu: Trivial code movement Philippe Mathieu-Daudé
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-01 14:41 UTC (permalink / raw)
To: Laurent Vivier, qemu-devel
Cc: Eduardo Habkost, qemu-trivial, Philippe Mathieu-Daudé,
Michael Tokarev, Markus Armbruster, Paolo Bonzini,
Richard Henderson
This series restrict the 'feature-words' property to the x86
architecture (other archs don't have it), and to system-mode
(user-mode doesn't use it).
Extracted from "user-mode: Prune build dependencies (part 3)" v3:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg746423.html
Philippe Mathieu-Daudé (3):
target/i386/cpu: Trivial code movement
target/i386/cpu: Restrict some of feature-words uses to system-mode
target/i386: Restrict X86CPUFeatureWord to X86 targets
qapi/machine-target.json | 45 ++++++++++++++++++++++++++++++++++++++++
qapi/machine.json | 42 -------------------------------------
target/i386/cpu.c | 31 ++++++++++++++++-----------
3 files changed, 64 insertions(+), 54 deletions(-)
--
2.26.2
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v4 1/3] target/i386/cpu: Trivial code movement
2020-10-01 14:41 [PATCH v4 0/3] target/i386: Restrict 'feature-words' property to x86 machines Philippe Mathieu-Daudé
@ 2020-10-01 14:41 ` Philippe Mathieu-Daudé
2020-10-01 14:41 ` [PATCH v4 2/3] target/i386/cpu: Restrict some of feature-words uses to system-mode Philippe Mathieu-Daudé
2020-10-01 14:41 ` [PATCH v4 3/3] target/i386: Restrict X86CPUFeatureWord to X86 targets Philippe Mathieu-Daudé
2 siblings, 0 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-01 14:41 UTC (permalink / raw)
To: Laurent Vivier, qemu-devel
Cc: Eduardo Habkost, qemu-trivial, Philippe Mathieu-Daudé,
Michael Tokarev, Markus Armbruster, Paolo Bonzini,
Richard Henderson
Trivial code movement to reduce #ifdef'ry in the next commit.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
target/i386/cpu.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 3ffd877dd5..2eec53ca22 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4643,16 +4643,6 @@ static void x86_cpu_get_feature_words(Object *obj, Visitor *v,
visit_type_X86CPUFeatureWordInfoList(v, "feature-words", &list, errp);
}
-/* Convert all '_' in a feature string option name to '-', to make feature
- * name conform to QOM property naming rule, which uses '-' instead of '_'.
- */
-static inline void feat2prop(char *s)
-{
- while ((s = strchr(s, '_'))) {
- *s = '-';
- }
-}
-
/* Return the feature property name for a feature flag bit */
static const char *x86_cpu_feature_name(FeatureWord w, int bitnr)
{
@@ -4677,6 +4667,17 @@ static const char *x86_cpu_feature_name(FeatureWord w, int bitnr)
return name;
}
+/*
+ * Convert all '_' in a feature string option name to '-', to make feature
+ * name conform to QOM property naming rule, which uses '-' instead of '_'.
+ */
+static inline void feat2prop(char *s)
+{
+ while ((s = strchr(s, '_'))) {
+ *s = '-';
+ }
+}
+
/* 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
--
2.26.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v4 2/3] target/i386/cpu: Restrict some of feature-words uses to system-mode
2020-10-01 14:41 [PATCH v4 0/3] target/i386: Restrict 'feature-words' property to x86 machines Philippe Mathieu-Daudé
2020-10-01 14:41 ` [PATCH v4 1/3] target/i386/cpu: Trivial code movement Philippe Mathieu-Daudé
@ 2020-10-01 14:41 ` Philippe Mathieu-Daudé
2020-10-01 14:44 ` Philippe Mathieu-Daudé
2020-10-01 15:14 ` Paolo Bonzini
2020-10-01 14:41 ` [PATCH v4 3/3] target/i386: Restrict X86CPUFeatureWord to X86 targets Philippe Mathieu-Daudé
2 siblings, 2 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-01 14:41 UTC (permalink / raw)
To: Laurent Vivier, qemu-devel
Cc: Eduardo Habkost, qemu-trivial, Philippe Mathieu-Daudé,
Michael Tokarev, Markus Armbruster, Paolo Bonzini,
Richard Henderson
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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v4 3/3] target/i386: Restrict X86CPUFeatureWord to X86 targets
2020-10-01 14:41 [PATCH v4 0/3] target/i386: Restrict 'feature-words' property to x86 machines Philippe Mathieu-Daudé
2020-10-01 14:41 ` [PATCH v4 1/3] target/i386/cpu: Trivial code movement Philippe Mathieu-Daudé
2020-10-01 14:41 ` [PATCH v4 2/3] target/i386/cpu: Restrict some of feature-words uses to system-mode Philippe Mathieu-Daudé
@ 2020-10-01 14:41 ` Philippe Mathieu-Daudé
2020-10-01 15:29 ` Eduardo Habkost
2 siblings, 1 reply; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-01 14:41 UTC (permalink / raw)
To: Laurent Vivier, qemu-devel
Cc: Eduardo Habkost, qemu-trivial, Philippe Mathieu-Daudé,
Michael Tokarev, Markus Armbruster, Paolo Bonzini,
Richard Henderson, Richard Henderson
Only qemu-system-FOO and qemu-storage-daemon provide QMP
monitors, therefore such declarations and definitions are
irrelevant for user-mode emulation.
Restricting the x86-specific commands to machine-target.json
pulls less QAPI-generated code into user-mode.
Acked-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
qapi/machine-target.json | 45 ++++++++++++++++++++++++++++++++++++++++
qapi/machine.json | 42 -------------------------------------
target/i386/cpu.c | 2 +-
3 files changed, 46 insertions(+), 43 deletions(-)
diff --git a/qapi/machine-target.json b/qapi/machine-target.json
index 698850cc78..b4d769a53b 100644
--- a/qapi/machine-target.json
+++ b/qapi/machine-target.json
@@ -4,6 +4,51 @@
# This work is licensed under the terms of the GNU GPL, version 2 or later.
# See the COPYING file in the top-level directory.
+##
+# @X86CPURegister32:
+#
+# A X86 32-bit register
+#
+# Since: 1.5
+##
+{ 'enum': 'X86CPURegister32',
+ 'data': [ 'EAX', 'EBX', 'ECX', 'EDX', 'ESP', 'EBP', 'ESI', 'EDI' ],
+ 'if': 'defined(TARGET_I386)' }
+
+##
+# @X86CPUFeatureWordInfo:
+#
+# Information about a X86 CPU feature word
+#
+# @cpuid-input-eax: Input EAX value for CPUID instruction for that feature word
+#
+# @cpuid-input-ecx: Input ECX value for CPUID instruction for that
+# feature word
+#
+# @cpuid-register: Output register containing the feature bits
+#
+# @features: value of output register, containing the feature bits
+#
+# Since: 1.5
+##
+{ 'struct': 'X86CPUFeatureWordInfo',
+ 'data': { 'cpuid-input-eax': 'int',
+ '*cpuid-input-ecx': 'int',
+ 'cpuid-register': 'X86CPURegister32',
+ 'features': 'int' },
+ 'if': 'defined(TARGET_I386)' }
+
+##
+# @DummyForceArrays:
+#
+# Not used by QMP; hack to let us use X86CPUFeatureWordInfoList internally
+#
+# Since: 2.5
+##
+{ 'struct': 'DummyForceArrays',
+ 'data': { 'unused': ['X86CPUFeatureWordInfo'] },
+ 'if': 'defined(TARGET_I386)' }
+
##
# @CpuModelInfo:
#
diff --git a/qapi/machine.json b/qapi/machine.json
index 756dacb06f..995e972858 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -574,48 +574,6 @@
'dst': 'uint16',
'val': 'uint8' }}
-##
-# @X86CPURegister32:
-#
-# A X86 32-bit register
-#
-# Since: 1.5
-##
-{ 'enum': 'X86CPURegister32',
- 'data': [ 'EAX', 'EBX', 'ECX', 'EDX', 'ESP', 'EBP', 'ESI', 'EDI' ] }
-
-##
-# @X86CPUFeatureWordInfo:
-#
-# Information about a X86 CPU feature word
-#
-# @cpuid-input-eax: Input EAX value for CPUID instruction for that feature word
-#
-# @cpuid-input-ecx: Input ECX value for CPUID instruction for that
-# feature word
-#
-# @cpuid-register: Output register containing the feature bits
-#
-# @features: value of output register, containing the feature bits
-#
-# Since: 1.5
-##
-{ 'struct': 'X86CPUFeatureWordInfo',
- 'data': { 'cpuid-input-eax': 'int',
- '*cpuid-input-ecx': 'int',
- 'cpuid-register': 'X86CPURegister32',
- 'features': 'int' } }
-
-##
-# @DummyForceArrays:
-#
-# Not used by QMP; hack to let us use X86CPUFeatureWordInfoList internally
-#
-# Since: 2.5
-##
-{ 'struct': 'DummyForceArrays',
- 'data': { 'unused': ['X86CPUFeatureWordInfo'] } }
-
##
# @NumaCpuOptions:
#
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 9f72342506..848a65ded2 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -38,7 +38,7 @@
#include "qemu/option.h"
#include "qemu/config-file.h"
#include "qapi/error.h"
-#include "qapi/qapi-visit-machine.h"
+#include "qapi/qapi-visit-machine-target.h"
#include "qapi/qapi-visit-run-state.h"
#include "qapi/qmp/qdict.h"
#include "qapi/qmp/qerror.h"
--
2.26.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v4 2/3] target/i386/cpu: Restrict some of feature-words uses to system-mode
2020-10-01 14:41 ` [PATCH v4 2/3] target/i386/cpu: Restrict some of feature-words uses to system-mode Philippe Mathieu-Daudé
@ 2020-10-01 14:44 ` Philippe Mathieu-Daudé
2020-10-01 15:14 ` Paolo Bonzini
1 sibling, 0 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-01 14:44 UTC (permalink / raw)
To: Laurent Vivier, qemu-devel
Cc: Eduardo Habkost, qemu-trivial, Michael Tokarev,
Markus Armbruster, Paolo Bonzini, Richard Henderson
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
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 2/3] target/i386/cpu: Restrict some of feature-words uses to system-mode
2020-10-01 14:41 ` [PATCH v4 2/3] target/i386/cpu: Restrict some of feature-words uses to system-mode Philippe Mathieu-Daudé
2020-10-01 14:44 ` Philippe Mathieu-Daudé
@ 2020-10-01 15:14 ` Paolo Bonzini
2020-10-01 15:27 ` Philippe Mathieu-Daudé
1 sibling, 1 reply; 14+ messages in thread
From: Paolo Bonzini @ 2020-10-01 15:14 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Laurent Vivier, qemu-devel
Cc: Eduardo Habkost, qemu-trivial, Michael Tokarev,
Markus Armbruster, Richard Henderson
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
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 2/3] target/i386/cpu: Restrict some of feature-words uses to system-mode
2020-10-01 15:14 ` Paolo Bonzini
@ 2020-10-01 15:27 ` Philippe Mathieu-Daudé
2020-10-01 15:37 ` Eduardo Habkost
0 siblings, 1 reply; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-01 15:27 UTC (permalink / raw)
To: Paolo Bonzini, Laurent Vivier, qemu-devel
Cc: Eduardo Habkost, qemu-trivial, Michael Tokarev,
Markus Armbruster, Richard Henderson
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
>>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 3/3] target/i386: Restrict X86CPUFeatureWord to X86 targets
2020-10-01 14:41 ` [PATCH v4 3/3] target/i386: Restrict X86CPUFeatureWord to X86 targets Philippe Mathieu-Daudé
@ 2020-10-01 15:29 ` Eduardo Habkost
2020-10-01 15:37 ` Philippe Mathieu-Daudé
2020-10-02 6:55 ` Markus Armbruster
0 siblings, 2 replies; 14+ messages in thread
From: Eduardo Habkost @ 2020-10-01 15:29 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, Richard Henderson, Michael Tokarev,
Markus Armbruster, Laurent Vivier, qemu-trivial, Paolo Bonzini,
Richard Henderson
On Thu, Oct 01, 2020 at 04:41:52PM +0200, Philippe Mathieu-Daudé wrote:
> Only qemu-system-FOO and qemu-storage-daemon provide QMP
> monitors, therefore such declarations and definitions are
> irrelevant for user-mode emulation.
>
> Restricting the x86-specific commands to machine-target.json
> pulls less QAPI-generated code into user-mode.
Is this still true without "qapi: Restrict code generated for
user-mode"?
Markus, Eric: what's the difference between machine.json and
machine-target.json? commit 7f7b4e7abef4 ("qapi: Split
machine-target.json off target.json and misc.json") explains what
but not why.
>
> Acked-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Why the visit_type_X86CPUFeatureWordInfoList() stub was required
in v3, but not in this version?
> ---
> qapi/machine-target.json | 45 ++++++++++++++++++++++++++++++++++++++++
> qapi/machine.json | 42 -------------------------------------
> target/i386/cpu.c | 2 +-
> 3 files changed, 46 insertions(+), 43 deletions(-)
>
> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
> index 698850cc78..b4d769a53b 100644
> --- a/qapi/machine-target.json
> +++ b/qapi/machine-target.json
> @@ -4,6 +4,51 @@
> # This work is licensed under the terms of the GNU GPL, version 2 or later.
> # See the COPYING file in the top-level directory.
>
> +##
> +# @X86CPURegister32:
> +#
> +# A X86 32-bit register
> +#
> +# Since: 1.5
> +##
> +{ 'enum': 'X86CPURegister32',
> + 'data': [ 'EAX', 'EBX', 'ECX', 'EDX', 'ESP', 'EBP', 'ESI', 'EDI' ],
> + 'if': 'defined(TARGET_I386)' }
> +
> +##
> +# @X86CPUFeatureWordInfo:
> +#
> +# Information about a X86 CPU feature word
> +#
> +# @cpuid-input-eax: Input EAX value for CPUID instruction for that feature word
> +#
> +# @cpuid-input-ecx: Input ECX value for CPUID instruction for that
> +# feature word
> +#
> +# @cpuid-register: Output register containing the feature bits
> +#
> +# @features: value of output register, containing the feature bits
> +#
> +# Since: 1.5
> +##
> +{ 'struct': 'X86CPUFeatureWordInfo',
> + 'data': { 'cpuid-input-eax': 'int',
> + '*cpuid-input-ecx': 'int',
> + 'cpuid-register': 'X86CPURegister32',
> + 'features': 'int' },
> + 'if': 'defined(TARGET_I386)' }
> +
> +##
> +# @DummyForceArrays:
> +#
> +# Not used by QMP; hack to let us use X86CPUFeatureWordInfoList internally
> +#
> +# Since: 2.5
> +##
> +{ 'struct': 'DummyForceArrays',
> + 'data': { 'unused': ['X86CPUFeatureWordInfo'] },
> + 'if': 'defined(TARGET_I386)' }
> +
> ##
> # @CpuModelInfo:
> #
> diff --git a/qapi/machine.json b/qapi/machine.json
> index 756dacb06f..995e972858 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -574,48 +574,6 @@
> 'dst': 'uint16',
> 'val': 'uint8' }}
>
> -##
> -# @X86CPURegister32:
> -#
> -# A X86 32-bit register
> -#
> -# Since: 1.5
> -##
> -{ 'enum': 'X86CPURegister32',
> - 'data': [ 'EAX', 'EBX', 'ECX', 'EDX', 'ESP', 'EBP', 'ESI', 'EDI' ] }
> -
> -##
> -# @X86CPUFeatureWordInfo:
> -#
> -# Information about a X86 CPU feature word
> -#
> -# @cpuid-input-eax: Input EAX value for CPUID instruction for that feature word
> -#
> -# @cpuid-input-ecx: Input ECX value for CPUID instruction for that
> -# feature word
> -#
> -# @cpuid-register: Output register containing the feature bits
> -#
> -# @features: value of output register, containing the feature bits
> -#
> -# Since: 1.5
> -##
> -{ 'struct': 'X86CPUFeatureWordInfo',
> - 'data': { 'cpuid-input-eax': 'int',
> - '*cpuid-input-ecx': 'int',
> - 'cpuid-register': 'X86CPURegister32',
> - 'features': 'int' } }
> -
> -##
> -# @DummyForceArrays:
> -#
> -# Not used by QMP; hack to let us use X86CPUFeatureWordInfoList internally
> -#
> -# Since: 2.5
> -##
> -{ 'struct': 'DummyForceArrays',
> - 'data': { 'unused': ['X86CPUFeatureWordInfo'] } }
> -
> ##
> # @NumaCpuOptions:
> #
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 9f72342506..848a65ded2 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -38,7 +38,7 @@
> #include "qemu/option.h"
> #include "qemu/config-file.h"
> #include "qapi/error.h"
> -#include "qapi/qapi-visit-machine.h"
> +#include "qapi/qapi-visit-machine-target.h"
> #include "qapi/qapi-visit-run-state.h"
> #include "qapi/qmp/qdict.h"
> #include "qapi/qmp/qerror.h"
> --
> 2.26.2
>
--
Eduardo
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 2/3] target/i386/cpu: Restrict some of feature-words uses to system-mode
2020-10-01 15:27 ` Philippe Mathieu-Daudé
@ 2020-10-01 15:37 ` Eduardo Habkost
2020-10-01 15:57 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 14+ messages in thread
From: Eduardo Habkost @ 2020-10-01 15:37 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, Michael Tokarev, Laurent Vivier, Markus Armbruster,
qemu-trivial, Paolo Bonzini, Richard Henderson
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 3/3] target/i386: Restrict X86CPUFeatureWord to X86 targets
2020-10-01 15:29 ` Eduardo Habkost
@ 2020-10-01 15:37 ` Philippe Mathieu-Daudé
2020-10-01 15:53 ` Eduardo Habkost
2020-10-02 6:55 ` Markus Armbruster
1 sibling, 1 reply; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-01 15:37 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, Richard Henderson, Michael Tokarev,
Markus Armbruster, Laurent Vivier, qemu-trivial, Paolo Bonzini,
Richard Henderson
On 10/1/20 5:29 PM, Eduardo Habkost wrote:
> On Thu, Oct 01, 2020 at 04:41:52PM +0200, Philippe Mathieu-Daudé wrote:
>> Only qemu-system-FOO and qemu-storage-daemon provide QMP
>> monitors, therefore such declarations and definitions are
>> irrelevant for user-mode emulation.
>>
>> Restricting the x86-specific commands to machine-target.json
>> pulls less QAPI-generated code into user-mode.
>
> Is this still true without "qapi: Restrict code generated for
> user-mode"?
The correct description so far is:
"Restricting the x86-specific commands to machine-target.json
pulls less QAPI-generated code into non-x86 targets.", as of
this commit the X86CPURegister32 definitions are not built in
the other ARM/S390/PPC/... targets.
I still have some hope "qapi: Restrict code generated for user-mode"
get merged some day.
>
> Markus, Eric: what's the difference between machine.json and
> machine-target.json? commit 7f7b4e7abef4 ("qapi: Split
> machine-target.json off target.json and misc.json") explains what
> but not why.
>
>>
>> Acked-by: Richard Henderson <richard.henderson@linaro.org>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
> Why the visit_type_X86CPUFeatureWordInfoList() stub was required
> in v3, but not in this version?
Because it is ifdef'ed out in the previous (new) patch:
"target/i386/cpu: Restrict some of feature-words uses to system-mode".
>
>
>> ---
>> qapi/machine-target.json | 45 ++++++++++++++++++++++++++++++++++++++++
>> qapi/machine.json | 42 -------------------------------------
>> target/i386/cpu.c | 2 +-
>> 3 files changed, 46 insertions(+), 43 deletions(-)
>>
>> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
>> index 698850cc78..b4d769a53b 100644
>> --- a/qapi/machine-target.json
>> +++ b/qapi/machine-target.json
>> @@ -4,6 +4,51 @@
>> # This work is licensed under the terms of the GNU GPL, version 2 or later.
>> # See the COPYING file in the top-level directory.
>>
>> +##
>> +# @X86CPURegister32:
>> +#
>> +# A X86 32-bit register
>> +#
>> +# Since: 1.5
>> +##
>> +{ 'enum': 'X86CPURegister32',
>> + 'data': [ 'EAX', 'EBX', 'ECX', 'EDX', 'ESP', 'EBP', 'ESI', 'EDI' ],
>> + 'if': 'defined(TARGET_I386)' }
>> +
>> +##
>> +# @X86CPUFeatureWordInfo:
>> +#
>> +# Information about a X86 CPU feature word
>> +#
>> +# @cpuid-input-eax: Input EAX value for CPUID instruction for that feature word
>> +#
>> +# @cpuid-input-ecx: Input ECX value for CPUID instruction for that
>> +# feature word
>> +#
>> +# @cpuid-register: Output register containing the feature bits
>> +#
>> +# @features: value of output register, containing the feature bits
>> +#
>> +# Since: 1.5
>> +##
>> +{ 'struct': 'X86CPUFeatureWordInfo',
>> + 'data': { 'cpuid-input-eax': 'int',
>> + '*cpuid-input-ecx': 'int',
>> + 'cpuid-register': 'X86CPURegister32',
>> + 'features': 'int' },
>> + 'if': 'defined(TARGET_I386)' }
>> +
>> +##
>> +# @DummyForceArrays:
>> +#
>> +# Not used by QMP; hack to let us use X86CPUFeatureWordInfoList internally
>> +#
>> +# Since: 2.5
>> +##
>> +{ 'struct': 'DummyForceArrays',
>> + 'data': { 'unused': ['X86CPUFeatureWordInfo'] },
>> + 'if': 'defined(TARGET_I386)' }
>> +
>> ##
>> # @CpuModelInfo:
>> #
>> diff --git a/qapi/machine.json b/qapi/machine.json
>> index 756dacb06f..995e972858 100644
>> --- a/qapi/machine.json
>> +++ b/qapi/machine.json
>> @@ -574,48 +574,6 @@
>> 'dst': 'uint16',
>> 'val': 'uint8' }}
>>
>> -##
>> -# @X86CPURegister32:
>> -#
>> -# A X86 32-bit register
>> -#
>> -# Since: 1.5
>> -##
>> -{ 'enum': 'X86CPURegister32',
>> - 'data': [ 'EAX', 'EBX', 'ECX', 'EDX', 'ESP', 'EBP', 'ESI', 'EDI' ] }
>> -
>> -##
>> -# @X86CPUFeatureWordInfo:
>> -#
>> -# Information about a X86 CPU feature word
>> -#
>> -# @cpuid-input-eax: Input EAX value for CPUID instruction for that feature word
>> -#
>> -# @cpuid-input-ecx: Input ECX value for CPUID instruction for that
>> -# feature word
>> -#
>> -# @cpuid-register: Output register containing the feature bits
>> -#
>> -# @features: value of output register, containing the feature bits
>> -#
>> -# Since: 1.5
>> -##
>> -{ 'struct': 'X86CPUFeatureWordInfo',
>> - 'data': { 'cpuid-input-eax': 'int',
>> - '*cpuid-input-ecx': 'int',
>> - 'cpuid-register': 'X86CPURegister32',
>> - 'features': 'int' } }
>> -
>> -##
>> -# @DummyForceArrays:
>> -#
>> -# Not used by QMP; hack to let us use X86CPUFeatureWordInfoList internally
>> -#
>> -# Since: 2.5
>> -##
>> -{ 'struct': 'DummyForceArrays',
>> - 'data': { 'unused': ['X86CPUFeatureWordInfo'] } }
>> -
>> ##
>> # @NumaCpuOptions:
>> #
>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>> index 9f72342506..848a65ded2 100644
>> --- a/target/i386/cpu.c
>> +++ b/target/i386/cpu.c
>> @@ -38,7 +38,7 @@
>> #include "qemu/option.h"
>> #include "qemu/config-file.h"
>> #include "qapi/error.h"
>> -#include "qapi/qapi-visit-machine.h"
>> +#include "qapi/qapi-visit-machine-target.h"
>> #include "qapi/qapi-visit-run-state.h"
>> #include "qapi/qmp/qdict.h"
>> #include "qapi/qmp/qerror.h"
>> --
>> 2.26.2
>>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 3/3] target/i386: Restrict X86CPUFeatureWord to X86 targets
2020-10-01 15:37 ` Philippe Mathieu-Daudé
@ 2020-10-01 15:53 ` Eduardo Habkost
0 siblings, 0 replies; 14+ messages in thread
From: Eduardo Habkost @ 2020-10-01 15:53 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, Richard Henderson, Michael Tokarev,
Markus Armbruster, Laurent Vivier, qemu-trivial, Paolo Bonzini,
Richard Henderson
On Thu, Oct 01, 2020 at 05:37:26PM +0200, Philippe Mathieu-Daudé wrote:
> On 10/1/20 5:29 PM, Eduardo Habkost wrote:
> > On Thu, Oct 01, 2020 at 04:41:52PM +0200, Philippe Mathieu-Daudé wrote:
> >> Only qemu-system-FOO and qemu-storage-daemon provide QMP
> >> monitors, therefore such declarations and definitions are
> >> irrelevant for user-mode emulation.
> >>
> >> Restricting the x86-specific commands to machine-target.json
> >> pulls less QAPI-generated code into user-mode.
> >
> > Is this still true without "qapi: Restrict code generated for
> > user-mode"?
>
> The correct description so far is:
>
> "Restricting the x86-specific commands to machine-target.json
> pulls less QAPI-generated code into non-x86 targets.", as of
> this commit the X86CPURegister32 definitions are not built in
> the other ARM/S390/PPC/... targets.
This seems to have other side effects in addition to remove the
code from non-x86 targets and I don't understand why.
>
> I still have some hope "qapi: Restrict code generated for user-mode"
> get merged some day.
>
> >
> > Markus, Eric: what's the difference between machine.json and
> > machine-target.json? commit 7f7b4e7abef4 ("qapi: Split
> > machine-target.json off target.json and misc.json") explains what
> > but not why.
[1]
> >
> >>
> >> Acked-by: Richard Henderson <richard.henderson@linaro.org>
> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> >
> > Why the visit_type_X86CPUFeatureWordInfoList() stub was required
> > in v3, but not in this version?
>
> Because it is ifdef'ed out in the previous (new) patch:
> "target/i386/cpu: Restrict some of feature-words uses to system-mode".
Sorry for the confusion, I thought this patch was _before_
"Restrict some of feature-words ..".
Now, what I don't understand is: why moving the code from
machine.json to machine-target.json makes patch 2/3 necessary?
I assume the answer to the question above[1] would clarify this.
>
> >
> >
> >> ---
> >> qapi/machine-target.json | 45 ++++++++++++++++++++++++++++++++++++++++
> >> qapi/machine.json | 42 -------------------------------------
> >> target/i386/cpu.c | 2 +-
> >> 3 files changed, 46 insertions(+), 43 deletions(-)
> >>
> >> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
> >> index 698850cc78..b4d769a53b 100644
> >> --- a/qapi/machine-target.json
> >> +++ b/qapi/machine-target.json
> >> @@ -4,6 +4,51 @@
> >> # This work is licensed under the terms of the GNU GPL, version 2 or later.
> >> # See the COPYING file in the top-level directory.
> >>
> >> +##
> >> +# @X86CPURegister32:
> >> +#
> >> +# A X86 32-bit register
> >> +#
> >> +# Since: 1.5
> >> +##
> >> +{ 'enum': 'X86CPURegister32',
> >> + 'data': [ 'EAX', 'EBX', 'ECX', 'EDX', 'ESP', 'EBP', 'ESI', 'EDI' ],
> >> + 'if': 'defined(TARGET_I386)' }
> >> +
> >> +##
> >> +# @X86CPUFeatureWordInfo:
> >> +#
> >> +# Information about a X86 CPU feature word
> >> +#
> >> +# @cpuid-input-eax: Input EAX value for CPUID instruction for that feature word
> >> +#
> >> +# @cpuid-input-ecx: Input ECX value for CPUID instruction for that
> >> +# feature word
> >> +#
> >> +# @cpuid-register: Output register containing the feature bits
> >> +#
> >> +# @features: value of output register, containing the feature bits
> >> +#
> >> +# Since: 1.5
> >> +##
> >> +{ 'struct': 'X86CPUFeatureWordInfo',
> >> + 'data': { 'cpuid-input-eax': 'int',
> >> + '*cpuid-input-ecx': 'int',
> >> + 'cpuid-register': 'X86CPURegister32',
> >> + 'features': 'int' },
> >> + 'if': 'defined(TARGET_I386)' }
> >> +
> >> +##
> >> +# @DummyForceArrays:
> >> +#
> >> +# Not used by QMP; hack to let us use X86CPUFeatureWordInfoList internally
> >> +#
> >> +# Since: 2.5
> >> +##
> >> +{ 'struct': 'DummyForceArrays',
> >> + 'data': { 'unused': ['X86CPUFeatureWordInfo'] },
> >> + 'if': 'defined(TARGET_I386)' }
> >> +
> >> ##
> >> # @CpuModelInfo:
> >> #
> >> diff --git a/qapi/machine.json b/qapi/machine.json
> >> index 756dacb06f..995e972858 100644
> >> --- a/qapi/machine.json
> >> +++ b/qapi/machine.json
> >> @@ -574,48 +574,6 @@
> >> 'dst': 'uint16',
> >> 'val': 'uint8' }}
> >>
> >> -##
> >> -# @X86CPURegister32:
> >> -#
> >> -# A X86 32-bit register
> >> -#
> >> -# Since: 1.5
> >> -##
> >> -{ 'enum': 'X86CPURegister32',
> >> - 'data': [ 'EAX', 'EBX', 'ECX', 'EDX', 'ESP', 'EBP', 'ESI', 'EDI' ] }
> >> -
> >> -##
> >> -# @X86CPUFeatureWordInfo:
> >> -#
> >> -# Information about a X86 CPU feature word
> >> -#
> >> -# @cpuid-input-eax: Input EAX value for CPUID instruction for that feature word
> >> -#
> >> -# @cpuid-input-ecx: Input ECX value for CPUID instruction for that
> >> -# feature word
> >> -#
> >> -# @cpuid-register: Output register containing the feature bits
> >> -#
> >> -# @features: value of output register, containing the feature bits
> >> -#
> >> -# Since: 1.5
> >> -##
> >> -{ 'struct': 'X86CPUFeatureWordInfo',
> >> - 'data': { 'cpuid-input-eax': 'int',
> >> - '*cpuid-input-ecx': 'int',
> >> - 'cpuid-register': 'X86CPURegister32',
> >> - 'features': 'int' } }
> >> -
> >> -##
> >> -# @DummyForceArrays:
> >> -#
> >> -# Not used by QMP; hack to let us use X86CPUFeatureWordInfoList internally
> >> -#
> >> -# Since: 2.5
> >> -##
> >> -{ 'struct': 'DummyForceArrays',
> >> - 'data': { 'unused': ['X86CPUFeatureWordInfo'] } }
> >> -
> >> ##
> >> # @NumaCpuOptions:
> >> #
> >> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> >> index 9f72342506..848a65ded2 100644
> >> --- a/target/i386/cpu.c
> >> +++ b/target/i386/cpu.c
> >> @@ -38,7 +38,7 @@
> >> #include "qemu/option.h"
> >> #include "qemu/config-file.h"
> >> #include "qapi/error.h"
> >> -#include "qapi/qapi-visit-machine.h"
> >> +#include "qapi/qapi-visit-machine-target.h"
> >> #include "qapi/qapi-visit-run-state.h"
> >> #include "qapi/qmp/qdict.h"
> >> #include "qapi/qmp/qerror.h"
> >> --
> >> 2.26.2
> >>
> >
>
--
Eduardo
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 2/3] target/i386/cpu: Restrict some of feature-words uses to system-mode
2020-10-01 15:37 ` Eduardo Habkost
@ 2020-10-01 15:57 ` Philippe Mathieu-Daudé
2020-10-01 16:15 ` Eduardo Habkost
0 siblings, 1 reply; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-01 15:57 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, Michael Tokarev, Laurent Vivier, Markus Armbruster,
qemu-trivial, Paolo Bonzini, Richard Henderson
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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 2/3] target/i386/cpu: Restrict some of feature-words uses to system-mode
2020-10-01 15:57 ` Philippe Mathieu-Daudé
@ 2020-10-01 16:15 ` Eduardo Habkost
0 siblings, 0 replies; 14+ messages in thread
From: Eduardo Habkost @ 2020-10-01 16:15 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, Michael Tokarev, Laurent Vivier, Markus Armbruster,
qemu-trivial, Paolo Bonzini, Richard Henderson
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 3/3] target/i386: Restrict X86CPUFeatureWord to X86 targets
2020-10-01 15:29 ` Eduardo Habkost
2020-10-01 15:37 ` Philippe Mathieu-Daudé
@ 2020-10-02 6:55 ` Markus Armbruster
1 sibling, 0 replies; 14+ messages in thread
From: Markus Armbruster @ 2020-10-02 6:55 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-trivial, Michael Tokarev, Richard Henderson, qemu-devel,
Laurent Vivier, Paolo Bonzini, Philippe Mathieu-Daudé,
Richard Henderson
Eduardo Habkost <ehabkost@redhat.com> writes:
> On Thu, Oct 01, 2020 at 04:41:52PM +0200, Philippe Mathieu-Daudé wrote:
>> Only qemu-system-FOO and qemu-storage-daemon provide QMP
>> monitors, therefore such declarations and definitions are
>> irrelevant for user-mode emulation.
>>
>> Restricting the x86-specific commands to machine-target.json
>> pulls less QAPI-generated code into user-mode.
>
> Is this still true without "qapi: Restrict code generated for
> user-mode"?
>
> Markus, Eric: what's the difference between machine.json and
> machine-target.json? commit 7f7b4e7abef4 ("qapi: Split
> machine-target.json off target.json and misc.json") explains what
> but not why.
My bad.
Short explanation: machine-target.json can use preprocessor symbols with
'if' that are only available in target-specific code: TARGET_S390X,
TARGET_I386, TARGET_ARM, ... This requires compiling per target.
machine.json can't use these, because it's compiled just once.
Same for misc-target.json and misc.json.
Now the long explanation.
The clue is buried in the patch:
diff --git a/qapi/Makefile.objs b/qapi/Makefile.objs
index 01dced01aa..4e87bef6e1 100644
--- a/qapi/Makefile.objs
+++ b/qapi/Makefile.objs
@@ -9,7 +9,7 @@ QAPI_COMMON_MODULES = audio authz block-core block char common crypto
QAPI_COMMON_MODULES += introspect job machine migration misc net
QAPI_COMMON_MODULES += qdev qom rdma rocker run-state sockets tpm
QAPI_COMMON_MODULES += trace transaction ui
-QAPI_TARGET_MODULES = target
+QAPI_TARGET_MODULES = machine-target target
QAPI_MODULES = $(QAPI_COMMON_MODULES) $(QAPI_TARGET_MODULES)
util-obj-y += qapi-builtin-types.o
QAPI_TARGET_MODULES appears in commit 61eb9e80d5:
qapi: New module target.json
We can't add appropriate target-specific conditionals to misc.json,
because that would make all of misc.json unusable in
target-independent code. To keep misc.json target-independent, we
need to split off target-dependent target.json.
This commit doesn't actually split off anything, it merely creates the
empty module. The next few patches will move stuff from misc.json
there.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Aha: it's about compiling per target, to make target-specific
conditionals available.
This part of qapi/Makefile.objs has since morphed into
qapi_inputs = []
qapi_specific_outputs = []
foreach module : qapi_all_modules
qapi_inputs += [ files(module + '.json') ]
qapi_module_outputs = [
...
]
if module.endswith('-target')
qapi_specific_outputs += qapi_module_outputs
else
qapi_util_outputs += qapi_module_outputs
endif
endforeach
in qapi/meson.build. Further down:
# Now go through all the outputs and add them to the right sourceset.
# These loops must be synchronized with the output of the above custom target.
i = 0
foreach output : qapi_util_outputs
if output.endswith('.h')
genh += qapi_files[i]
endif
util_ss.add(qapi_files[i])
i = i + 1
endforeach
foreach output : qapi_specific_outputs + qapi_nonmodule_outputs
if output.endswith('.h')
genh += qapi_files[i]
endif
specific_ss.add(when: 'CONFIG_SOFTMMU', if_true: qapi_files[i])
i = i + 1
endforeach
The source files generated for the "common" modules are added to
sourceset util_ss, and the ones for the "target" modules are added to
sourceset specific_ss.
The former gets compiled just once, and the .o get linked into pretty
much everything.
The latter gets compiled per target, and the .o get linked into
target-specific executables.
Clear as mud?
[...]
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-10-02 6:56 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-01 14:41 [PATCH v4 0/3] target/i386: Restrict 'feature-words' property to x86 machines Philippe Mathieu-Daudé
2020-10-01 14:41 ` [PATCH v4 1/3] target/i386/cpu: Trivial code movement Philippe Mathieu-Daudé
2020-10-01 14:41 ` [PATCH v4 2/3] target/i386/cpu: Restrict some of feature-words uses to system-mode Philippe Mathieu-Daudé
2020-10-01 14:44 ` Philippe Mathieu-Daudé
2020-10-01 15:14 ` Paolo Bonzini
2020-10-01 15:27 ` Philippe Mathieu-Daudé
2020-10-01 15:37 ` Eduardo Habkost
2020-10-01 15:57 ` Philippe Mathieu-Daudé
2020-10-01 16:15 ` Eduardo Habkost
2020-10-01 14:41 ` [PATCH v4 3/3] target/i386: Restrict X86CPUFeatureWord to X86 targets Philippe Mathieu-Daudé
2020-10-01 15:29 ` Eduardo Habkost
2020-10-01 15:37 ` Philippe Mathieu-Daudé
2020-10-01 15:53 ` Eduardo Habkost
2020-10-02 6:55 ` Markus Armbruster
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.