[PATCH v5 0/4] qapi: Restrict X86 features to X86 targets

[PATCH v5 0/4] qapi: Restrict X86 features to X86 targets

[Philippe Mathieu-Daudé]

Since v4:
- Rebase on Claudio's work to avoid stub

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).

v4: https://www.mail-archive.com/qemu-devel@nongnu.org/msg746628.html

Supersedes: <20201001144152.1555659-1-philmd@redhat.com>
Based-on: <20210224133428.14071-1-cfontana@suse.de>

Philippe Mathieu-Daudé (4):
  target/i386/cpu: Introduce get_register_enum_32() helper
  target/i386/cpu: Restrict x86_cpu_get_feature_words to sysemu
  qapi: Move X86 specific types to machine-target.json
  qapi/machine-target: Restrict X86 features to X86 targets

 qapi/machine-target.json   | 45 ++++++++++++++++++++++++++++++++++++++
 qapi/machine.json          | 42 -----------------------------------
 target/i386/cpu-internal.h |  7 ++++++
 target/i386/cpu-softmmu.c  | 36 ++++++++++++++++++++++++++++++
 target/i386/cpu.c          | 45 +++++++-------------------------------
 5 files changed, 96 insertions(+), 79 deletions(-)

-- 
2.26.2

[PATCH v5 1/4] target/i386/cpu: Introduce get_register_enum_32() helper

[Philippe Mathieu-Daudé]

Introduce get_register_enum_32(), similar to get_register_name_32().

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/i386/cpu-internal.h | 4 ++++
 target/i386/cpu.c          | 7 ++++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/target/i386/cpu-internal.h b/target/i386/cpu-internal.h
index be688cb393e..9df24c482ea 100644
--- a/target/i386/cpu-internal.h
+++ b/target/i386/cpu-internal.h
@@ -20,6 +20,8 @@
 #ifndef I386_CPU_INTERNAL_H
 #define I386_CPU_INTERNAL_H
 
+#include "qapi/qapi-types-machine.h"
+
 typedef enum FeatureWordType {
    CPUID_FEATURE_WORD,
    MSR_FEATURE_WORD,
@@ -55,6 +57,8 @@ typedef struct FeatureWordInfo {
 
 extern FeatureWordInfo feature_word_info[];
 
+X86CPURegister32 get_register_enum_32(FeatureWord w);
+
 void x86_cpu_expand_features(X86CPU *cpu, Error **errp);
 
 #ifndef CONFIG_USER_ONLY
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 219d7652524..ffa342171ba 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1476,6 +1476,11 @@ static const char *get_register_name_32(unsigned int reg)
     return x86_reg_info_32[reg].name;
 }
 
+X86CPURegister32 get_register_enum_32(FeatureWord w)
+{
+    return x86_reg_info_32[feature_word_info[w].cpuid.reg].qapi_enum;
+}
+
 /*
  * Returns the set of feature flags that are supported and migratable by
  * QEMU, for a given FeatureWord.
@@ -4548,7 +4553,7 @@ static void x86_cpu_get_feature_words(Object *obj, Visitor *v,
         qwi->cpuid_input_eax = wi->cpuid.eax;
         qwi->has_cpuid_input_ecx = wi->cpuid.needs_ecx;
         qwi->cpuid_input_ecx = wi->cpuid.ecx;
-        qwi->cpuid_register = x86_reg_info_32[wi->cpuid.reg].qapi_enum;
+        qwi->cpuid_register = get_register_enum_32(w);
         qwi->features = array[w];
 
         /* List will be in reverse order, but order shouldn't matter */
-- 
2.26.2

[PATCH v5 2/4] target/i386/cpu: Restrict x86_cpu_get_feature_words to sysemu

[Philippe Mathieu-Daudé]

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.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/i386/cpu-internal.h |  3 +++
 target/i386/cpu-softmmu.c  | 36 ++++++++++++++++++++++++++++++++++
 target/i386/cpu.c          | 40 +++-----------------------------------
 3 files changed, 42 insertions(+), 37 deletions(-)

diff --git a/target/i386/cpu-internal.h b/target/i386/cpu-internal.h
index 9df24c482ea..9355f86f6e1 100644
--- a/target/i386/cpu-internal.h
+++ b/target/i386/cpu-internal.h
@@ -69,6 +69,9 @@ void x86_cpu_get_crash_info_qom(Object *obj, Visitor *v,
 void x86_cpu_apic_create(X86CPU *cpu, Error **errp);
 void x86_cpu_apic_realize(X86CPU *cpu, Error **errp);
 void x86_cpu_machine_reset_cb(void *opaque);
+void x86_cpu_get_feature_words(Object *obj, Visitor *v,
+                               const char *name, void *opaque,
+                               Error **errp);
 #endif /* CONFIG_USER_ONLY */
 
 #endif /* CPU_INTERNAL_H */
diff --git a/target/i386/cpu-softmmu.c b/target/i386/cpu-softmmu.c
index c824408e8eb..34fd7b0de02 100644
--- a/target/i386/cpu-softmmu.c
+++ b/target/i386/cpu-softmmu.c
@@ -23,6 +23,7 @@
 #include "sysemu/whpx.h"
 #include "kvm/kvm_i386.h"
 #include "qapi/error.h"
+#include "qapi/qapi-visit-machine.h"
 #include "qapi/qapi-visit-run-state.h"
 #include "qapi/qmp/qdict.h"
 #include "qom/qom-qobject.h"
@@ -350,3 +351,38 @@ void x86_cpu_get_crash_info_qom(Object *obj, Visitor *v,
     qapi_free_GuestPanicInformation(panic_info);
 }
 
+/* Generic getter for "feature-words" and "filtered-features" properties */
+void x86_cpu_get_feature_words(Object *obj, Visitor *v,
+                               const char *name, void *opaque,
+                               Error **errp)
+{
+    uint64_t *array = (uint64_t *)opaque;
+    FeatureWord w;
+    X86CPUFeatureWordInfo word_infos[FEATURE_WORDS] = { };
+    X86CPUFeatureWordInfoList list_entries[FEATURE_WORDS] = { };
+    X86CPUFeatureWordInfoList *list = NULL;
+
+    for (w = 0; w < FEATURE_WORDS; w++) {
+        FeatureWordInfo *wi = &feature_word_info[w];
+        /*
+         * We didn't have MSR features when "feature-words" was
+         *  introduced. Therefore skipped other type entries.
+         */
+        if (wi->type != CPUID_FEATURE_WORD) {
+            continue;
+        }
+        X86CPUFeatureWordInfo *qwi = &word_infos[w];
+        qwi->cpuid_input_eax = wi->cpuid.eax;
+        qwi->has_cpuid_input_ecx = wi->cpuid.needs_ecx;
+        qwi->cpuid_input_ecx = wi->cpuid.ecx;
+        qwi->cpuid_register = get_register_enum_32(w);
+        qwi->features = array[w];
+
+        /* List will be in reverse order, but order shouldn't matter */
+        list_entries[w].next = list;
+        list_entries[w].value = &word_infos[w];
+        list = &list_entries[w];
+    }
+
+    visit_type_X86CPUFeatureWordInfoList(v, "feature-words", &list, errp);
+}
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index ffa342171ba..c088bcf3113 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -27,7 +27,7 @@
 #include "sysemu/hvf.h"
 #include "kvm/kvm_i386.h"
 #include "sev_i386.h"
-#include "qapi/qapi-visit-machine.h"
+#include "qapi/qapi-builtin-visit.h"
 #include "qapi/qmp/qerror.h"
 #include "qapi/qapi-commands-machine-target.h"
 #include "standard-headers/asm-x86/kvm_para.h"
@@ -4529,42 +4529,6 @@ 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;
 }
 
-/* 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,
-                                      Error **errp)
-{
-    uint64_t *array = (uint64_t *)opaque;
-    FeatureWord w;
-    X86CPUFeatureWordInfo word_infos[FEATURE_WORDS] = { };
-    X86CPUFeatureWordInfoList list_entries[FEATURE_WORDS] = { };
-    X86CPUFeatureWordInfoList *list = NULL;
-
-    for (w = 0; w < FEATURE_WORDS; w++) {
-        FeatureWordInfo *wi = &feature_word_info[w];
-        /*
-                * We didn't have MSR features when "feature-words" was
-                *  introduced. Therefore skipped other type entries.
-                */
-        if (wi->type != CPUID_FEATURE_WORD) {
-            continue;
-        }
-        X86CPUFeatureWordInfo *qwi = &word_infos[w];
-        qwi->cpuid_input_eax = wi->cpuid.eax;
-        qwi->has_cpuid_input_ecx = wi->cpuid.needs_ecx;
-        qwi->cpuid_input_ecx = wi->cpuid.ecx;
-        qwi->cpuid_register = get_register_enum_32(w);
-        qwi->features = array[w];
-
-        /* List will be in reverse order, but order shouldn't matter */
-        list_entries[w].next = list;
-        list_entries[w].value = &word_infos[w];
-        list = &list_entries[w];
-    }
-
-    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 '_'.
  */
@@ -6422,12 +6386,14 @@ static void x86_cpu_initfn(Object *obj)
     env->nr_dies = 1;
     cpu_set_cpustate_pointers(cpu);
 
+#if !defined(CONFIG_USER_ONLY)
     object_property_add(obj, "feature-words", "X86CPUFeatureWordInfo",
                         x86_cpu_get_feature_words,
                         NULL, NULL, (void *)env->features);
     object_property_add(obj, "filtered-features", "X86CPUFeatureWordInfo",
                         x86_cpu_get_feature_words,
                         NULL, NULL, (void *)cpu->filtered_features);
+#endif  /* !CONFIG_USER_ONLY */
 
     object_property_add_alias(obj, "sse3", obj, "pni");
     object_property_add_alias(obj, "pclmuldq", obj, "pclmulqdq");
-- 
2.26.2

[PATCH v5 3/4] qapi: Move X86 specific types to machine-target.json

[Philippe Mathieu-Daudé]

X86CPURegister32 enum and X86CPUFeatureWordInfo structure
are specific to the x86 architecture, move these entries
to machine-target.json.

Restricting the x86-specific commands to machine-target.json
pulls less QAPI-generated code into non-x86 targets.

Acked-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 qapi/machine-target.json  | 42 +++++++++++++++++++++++++++++++++++++++
 qapi/machine.json         | 42 ---------------------------------------
 target/i386/cpu-softmmu.c |  2 +-
 3 files changed, 43 insertions(+), 43 deletions(-)

diff --git a/qapi/machine-target.json b/qapi/machine-target.json
index e7811654b72..106fbd2e9ed 100644
--- a/qapi/machine-target.json
+++ b/qapi/machine-target.json
@@ -329,3 +329,45 @@
 ##
 { 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'],
   'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_I386) || defined(TARGET_S390X) || defined(TARGET_MIPS)' }
+
+##
+# @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'] } }
diff --git a/qapi/machine.json b/qapi/machine.json
index 330189efe3d..8053803a0e0 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -681,48 +681,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-softmmu.c b/target/i386/cpu-softmmu.c
index 34fd7b0de02..a7afa7963db 100644
--- a/target/i386/cpu-softmmu.c
+++ b/target/i386/cpu-softmmu.c
@@ -23,7 +23,7 @@
 #include "sysemu/whpx.h"
 #include "kvm/kvm_i386.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 "qom/qom-qobject.h"
-- 
2.26.2

[PATCH v5 4/4] qapi/machine-target: Restrict X86 features to X86 targets

[Philippe Mathieu-Daudé]

X86CPURegister32 enum and X86CPUFeatureWordInfo structure
are specific to the x86 architecture, restrict them to the
X86 targets.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 qapi/machine-target.json | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/qapi/machine-target.json b/qapi/machine-target.json
index 106fbd2e9ed..a8a5b3f86f6 100644
--- a/qapi/machine-target.json
+++ b/qapi/machine-target.json
@@ -338,7 +338,8 @@
 # Since: 1.5
 ##
 { 'enum': 'X86CPURegister32',
-  'data': [ 'EAX', 'EBX', 'ECX', 'EDX', 'ESP', 'EBP', 'ESI', 'EDI' ] }
+  'data': [ 'EAX', 'EBX', 'ECX', 'EDX', 'ESP', 'EBP', 'ESI', 'EDI' ],
+  'if': 'defined(TARGET_I386)' }
 
 ##
 # @X86CPUFeatureWordInfo:
@@ -360,7 +361,8 @@
   'data': { 'cpuid-input-eax': 'int',
             '*cpuid-input-ecx': 'int',
             'cpuid-register': 'X86CPURegister32',
-            'features': 'int' } }
+            'features': 'int' },
+  'if': 'defined(TARGET_I386)' }
 
 ##
 # @DummyForceArrays:
@@ -370,4 +372,5 @@
 # Since: 2.5
 ##
 { 'struct': 'DummyForceArrays',
-  'data': { 'unused': ['X86CPUFeatureWordInfo'] } }
+  'data': { 'unused': ['X86CPUFeatureWordInfo'] },
+  'if': 'defined(TARGET_I386)' }
-- 
2.26.2

Re: [PATCH v5 0/4] qapi: Restrict X86 features to X86 targets

[Philippe Mathieu-Daudé]

ping?

On 2/24/21 11:46 PM, Philippe Mathieu-Daudé wrote:
> Since v4:
> - Rebase on Claudio's work to avoid stub
> 
> 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).
> 
> v4: https://www.mail-archive.com/qemu-devel@nongnu.org/msg746628.html
> 
> Supersedes: <20201001144152.1555659-1-philmd@redhat.com>
> Based-on: <20210224133428.14071-1-cfontana@suse.de>
> 
> Philippe Mathieu-Daudé (4):
>   target/i386/cpu: Introduce get_register_enum_32() helper
>   target/i386/cpu: Restrict x86_cpu_get_feature_words to sysemu
>   qapi: Move X86 specific types to machine-target.json
>   qapi/machine-target: Restrict X86 features to X86 targets
> 
>  qapi/machine-target.json   | 45 ++++++++++++++++++++++++++++++++++++++
>  qapi/machine.json          | 42 -----------------------------------
>  target/i386/cpu-internal.h |  7 ++++++
>  target/i386/cpu-softmmu.c  | 36 ++++++++++++++++++++++++++++++
>  target/i386/cpu.c          | 45 +++++++-------------------------------
>  5 files changed, 96 insertions(+), 79 deletions(-)
>