qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/3] Enable -cpu <cpu>,help
@ 2023-03-14 10:00 Dinah Baum
  2023-03-14 10:00 ` [PATCH v2 1/3] qapi/machine-target: refactor machine-target Dinah Baum
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Dinah Baum @ 2023-03-14 10:00 UTC (permalink / raw)
  To: qemu-devel; +Cc: Dinah Baum

Part 1 is a refactor/code motion patch for
qapi/machine target required for setup of

Part 2 which enables query-cpu-model-expansion
on all architectures

Part 3 implements the '<cpu>,help' feature

Limitations:
Currently only 'FULL' expansion queries are implemented since
that's the only type enabled on the architectures that
allow feature probing

Unlike the 'device,help' command, default values aren't
printed

Dinah Baum (3):
  qapi/machine-target: refactor machine-target
  cpu, qapi, target/arm, i386, s390x: Generalize
    query-cpu-model-expansion
  cpu, qdict, vl: Enable printing options for CPU type

 MAINTAINERS                      |   1 +
 cpu.c                            |  61 +++++++++++++++
 include/exec/cpu-common.h        |  10 +++
 include/qapi/qmp/qdict.h         |   2 +
 qapi/machine-target-common.json  | 130 +++++++++++++++++++++++++++++++
 qapi/machine-target.json         | 129 +-----------------------------
 qapi/meson.build                 |   1 +
 qemu-options.hx                  |   7 +-
 qobject/qdict.c                  |   5 ++
 softmmu/vl.c                     |  36 ++++++++-
 target/arm/arm-qmp-cmds.c        |   7 +-
 target/arm/cpu.h                 |   7 +-
 target/i386/cpu-sysemu.c         |   7 +-
 target/i386/cpu.h                |   6 ++
 target/s390x/cpu.h               |   7 ++
 target/s390x/cpu_models_sysemu.c |   6 +-
 16 files changed, 278 insertions(+), 144 deletions(-)
 create mode 100644 qapi/machine-target-common.json

-- 
2.30.2



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 1/3] qapi/machine-target: refactor machine-target
  2023-03-14 10:00 [PATCH v2 0/3] Enable -cpu <cpu>,help Dinah Baum
@ 2023-03-14 10:00 ` Dinah Baum
  2023-03-14 10:29   ` Philippe Mathieu-Daudé
  2023-03-14 10:00 ` [PATCH v2 2/3] cpu, qapi, target/arm, i386, s390x: Generalize query-cpu-model-expansion Dinah Baum
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Dinah Baum @ 2023-03-14 10:00 UTC (permalink / raw)
  To: qemu-devel
  Cc: Dinah Baum, Eduardo Habkost, Marcel Apfelbaum,
	Philippe Mathieu-Daudé, Yanan Wang, Eric Blake,
	Markus Armbruster, Michael Roth

Moved architecture agnostic data types to their own
file to avoid "attempt to use poisoned TARGET_*"
error that results when including qapi header
with commands that aren't defined for all architectures.
Required to implement enabling `query-cpu-model-expansion`
on all architectures

Signed-off-by: Dinah Baum <dinahbaum123@gmail.com>
---
 MAINTAINERS                     |  1 +
 qapi/machine-target-common.json | 79 +++++++++++++++++++++++++++++++++
 qapi/machine-target.json        | 73 +-----------------------------
 qapi/meson.build                |  1 +
 4 files changed, 82 insertions(+), 72 deletions(-)
 create mode 100644 qapi/machine-target-common.json

diff --git a/MAINTAINERS b/MAINTAINERS
index 95c957d587..bc4a6ba399 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1750,6 +1750,7 @@ F: hw/core/numa.c
 F: hw/cpu/cluster.c
 F: qapi/machine.json
 F: qapi/machine-target.json
+F: qapi/machine-target-common.json
 F: include/hw/boards.h
 F: include/hw/core/cpu.h
 F: include/hw/cpu/cluster.h
diff --git a/qapi/machine-target-common.json b/qapi/machine-target-common.json
new file mode 100644
index 0000000000..1e6da3177d
--- /dev/null
+++ b/qapi/machine-target-common.json
@@ -0,0 +1,79 @@
+# -*- Mode: Python -*-
+# vim: filetype=python
+
+##
+# = Common data types for machine target commands
+##
+
+##
+# @CpuModelInfo:
+#
+# Virtual CPU model.
+#
+# A CPU model consists of the name of a CPU definition, to which
+# delta changes are applied (e.g. features added/removed). Most magic values
+# that an architecture might require should be hidden behind the name.
+# However, if required, architectures can expose relevant properties.
+#
+# @name: the name of the CPU definition the model is based on
+# @props: a dictionary of QOM properties to be applied
+#
+# Since: 2.8
+##
+{ 'struct': 'CpuModelInfo',
+'data': { 'name': 'str',
+          '*props': 'any' } }
+
+##
+# @CpuModelExpansionType:
+#
+# An enumeration of CPU model expansion types.
+#
+# @static: Expand to a static CPU model, a combination of a static base
+#          model name and property delta changes. As the static base model will
+#          never change, the expanded CPU model will be the same, independent of
+#          QEMU version, machine type, machine options, and accelerator options.
+#          Therefore, the resulting model can be used by tooling without having
+#          to specify a compatibility machine - e.g. when displaying the "host"
+#          model. The @static CPU models are migration-safe.
+
+# @full: Expand all properties. The produced model is not guaranteed to be
+#        migration-safe, but allows tooling to get an insight and work with
+#        model details.
+#
+# Note: When a non-migration-safe CPU model is expanded in static mode, some
+#       features enabled by the CPU model may be omitted, because they can't be
+#       implemented by a static CPU model definition (e.g. cache info passthrough and
+#       PMU passthrough in x86). If you need an accurate representation of the
+#       features enabled by a non-migration-safe CPU model, use @full. If you need a
+#       static representation that will keep ABI compatibility even when changing QEMU
+#       version or machine-type, use @static (but keep in mind that some features may
+#       be omitted).
+#
+# Since: 2.8
+##
+{ 'enum': 'CpuModelExpansionType',
+  'data': [ 'static', 'full' ] }
+
+##
+# @CpuModelCompareResult:
+#
+# An enumeration of CPU model comparison results. The result is usually
+# calculated using e.g. CPU features or CPU generations.
+#
+# @incompatible: If model A is incompatible to model B, model A is not
+#                guaranteed to run where model B runs and the other way around.
+#
+# @identical: If model A is identical to model B, model A is guaranteed to run
+#             where model B runs and the other way around.
+#
+# @superset: If model A is a superset of model B, model B is guaranteed to run
+#            where model A runs. There are no guarantees about the other way.
+#
+# @subset: If model A is a subset of model B, model A is guaranteed to run
+#          where model B runs. There are no guarantees about the other way.
+#
+# Since: 2.8
+##
+{ 'enum': 'CpuModelCompareResult',
+  'data': [ 'incompatible', 'identical', 'superset', 'subset' ] }
diff --git a/qapi/machine-target.json b/qapi/machine-target.json
index 2e267fa458..1cacfde88e 100644
--- a/qapi/machine-target.json
+++ b/qapi/machine-target.json
@@ -4,78 +4,7 @@
 # This work is licensed under the terms of the GNU GPL, version 2 or later.
 # See the COPYING file in the top-level directory.
 
-##
-# @CpuModelInfo:
-#
-# Virtual CPU model.
-#
-# A CPU model consists of the name of a CPU definition, to which
-# delta changes are applied (e.g. features added/removed). Most magic values
-# that an architecture might require should be hidden behind the name.
-# However, if required, architectures can expose relevant properties.
-#
-# @name: the name of the CPU definition the model is based on
-# @props: a dictionary of QOM properties to be applied
-#
-# Since: 2.8
-##
-{ 'struct': 'CpuModelInfo',
-  'data': { 'name': 'str',
-            '*props': 'any' } }
-
-##
-# @CpuModelExpansionType:
-#
-# An enumeration of CPU model expansion types.
-#
-# @static: Expand to a static CPU model, a combination of a static base
-#          model name and property delta changes. As the static base model will
-#          never change, the expanded CPU model will be the same, independent of
-#          QEMU version, machine type, machine options, and accelerator options.
-#          Therefore, the resulting model can be used by tooling without having
-#          to specify a compatibility machine - e.g. when displaying the "host"
-#          model. The @static CPU models are migration-safe.
-
-# @full: Expand all properties. The produced model is not guaranteed to be
-#        migration-safe, but allows tooling to get an insight and work with
-#        model details.
-#
-# Note: When a non-migration-safe CPU model is expanded in static mode, some
-#       features enabled by the CPU model may be omitted, because they can't be
-#       implemented by a static CPU model definition (e.g. cache info passthrough and
-#       PMU passthrough in x86). If you need an accurate representation of the
-#       features enabled by a non-migration-safe CPU model, use @full. If you need a
-#       static representation that will keep ABI compatibility even when changing QEMU
-#       version or machine-type, use @static (but keep in mind that some features may
-#       be omitted).
-#
-# Since: 2.8
-##
-{ 'enum': 'CpuModelExpansionType',
-  'data': [ 'static', 'full' ] }
-
-##
-# @CpuModelCompareResult:
-#
-# An enumeration of CPU model comparison results. The result is usually
-# calculated using e.g. CPU features or CPU generations.
-#
-# @incompatible: If model A is incompatible to model B, model A is not
-#                guaranteed to run where model B runs and the other way around.
-#
-# @identical: If model A is identical to model B, model A is guaranteed to run
-#             where model B runs and the other way around.
-#
-# @superset: If model A is a superset of model B, model B is guaranteed to run
-#            where model A runs. There are no guarantees about the other way.
-#
-# @subset: If model A is a subset of model B, model A is guaranteed to run
-#          where model B runs. There are no guarantees about the other way.
-#
-# Since: 2.8
-##
-{ 'enum': 'CpuModelCompareResult',
-  'data': [ 'incompatible', 'identical', 'superset', 'subset' ] }
+{ 'include': 'machine-target-common.json' }
 
 ##
 # @CpuModelBaselineInfo:
diff --git a/qapi/meson.build b/qapi/meson.build
index 9fd480c4d8..48be47170f 100644
--- a/qapi/meson.build
+++ b/qapi/meson.build
@@ -38,6 +38,7 @@ qapi_all_modules = [
   'job',
   'machine',
   'machine-target',
+  'machine-target-common',
   'migration',
   'misc',
   'misc-target',
-- 
2.30.2



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 2/3] cpu, qapi, target/arm, i386, s390x: Generalize query-cpu-model-expansion
  2023-03-14 10:00 [PATCH v2 0/3] Enable -cpu <cpu>,help Dinah Baum
  2023-03-14 10:00 ` [PATCH v2 1/3] qapi/machine-target: refactor machine-target Dinah Baum
@ 2023-03-14 10:00 ` Dinah Baum
  2023-03-14 10:00 ` [PATCH v2 3/3] cpu, qdict, vl: Enable printing options for CPU type Dinah Baum
  2023-03-22  3:16 ` [PATCH v2 0/3] Enable -cpu <cpu>,help Dinah B
  3 siblings, 0 replies; 8+ messages in thread
From: Dinah Baum @ 2023-03-14 10:00 UTC (permalink / raw)
  To: qemu-devel
  Cc: Dinah Baum, Eduardo Habkost, Marcel Apfelbaum,
	Philippe Mathieu-Daudé, Yanan Wang, Richard Henderson,
	Paolo Bonzini, Eric Blake, Markus Armbruster, Peter Maydell,
	David Hildenbrand, Ilya Leoshkevich, Thomas Huth,
	open list:ARM TCG CPUs, open list:S390 TCG CPUs

This patch enables 'query-cpu-model-expansion' on all
architectures. Only architectures that implement
the command will return results, others will return an
error message as before.

This patch lays the groundwork for parsing a
-cpu cpu,help option as specified in
https://gitlab.com/qemu-project/qemu/-/issues/1480

Signed-off-by: Dinah Baum <dinahbaum123@gmail.com>
---
 cpu.c                            | 20 ++++++++++++
 include/exec/cpu-common.h        |  8 +++++
 qapi/machine-target-common.json  | 51 +++++++++++++++++++++++++++++
 qapi/machine-target.json         | 56 --------------------------------
 target/arm/arm-qmp-cmds.c        |  7 ++--
 target/arm/cpu.h                 |  7 +++-
 target/i386/cpu-sysemu.c         |  7 ++--
 target/i386/cpu.h                |  6 ++++
 target/s390x/cpu.h               |  7 ++++
 target/s390x/cpu_models_sysemu.c |  6 ++--
 10 files changed, 108 insertions(+), 67 deletions(-)

diff --git a/cpu.c b/cpu.c
index 567b23af46..c09edc4556 100644
--- a/cpu.c
+++ b/cpu.c
@@ -291,6 +291,26 @@ void list_cpus(const char *optarg)
 #endif
 }
 
+CpuModelExpansionInfo *get_cpu_model_expansion_info(CpuModelExpansionType type,
+                                                    CpuModelInfo *model,
+                                                    Error **errp)
+{
+    /* XXX: implement cpu_model_expansion for targets that still miss it */
+#if defined(cpu_model_expansion)
+    return cpu_model_expansion(type, model, errp);
+#else
+    error_setg(errp, "Could not query cpu model information");
+    return NULL;
+#endif
+}
+
+CpuModelExpansionInfo *qmp_query_cpu_model_expansion(CpuModelExpansionType type,
+                                                     CpuModelInfo *model,
+                                                     Error **errp)
+{
+    return get_cpu_model_expansion_info(type, model, errp);
+}
+
 #if defined(CONFIG_USER_ONLY)
 void tb_invalidate_phys_addr(target_ulong addr)
 {
diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 6feaa40ca7..ec6024dfde 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -7,6 +7,8 @@
 #include "exec/hwaddr.h"
 #endif
 
+#include "qapi/qapi-commands-machine-target-common.h"
+
 /**
  * vaddr:
  * Type wide enough to contain any #target_ulong virtual address.
@@ -166,5 +168,11 @@ int cpu_memory_rw_debug(CPUState *cpu, vaddr addr,
 extern int singlestep;
 
 void list_cpus(const char *optarg);
+typedef void (*cpu_model_expansion_func)(CpuModelExpansionType type,
+                                         CpuModelInfo *model,
+                                         Error **errp);
+CpuModelExpansionInfo *get_cpu_model_expansion_info(CpuModelExpansionType type,
+                                                    CpuModelInfo *model,
+                                                    Error **errp);
 
 #endif /* CPU_COMMON_H */
diff --git a/qapi/machine-target-common.json b/qapi/machine-target-common.json
index 1e6da3177d..44713e9935 100644
--- a/qapi/machine-target-common.json
+++ b/qapi/machine-target-common.json
@@ -77,3 +77,54 @@
 ##
 { 'enum': 'CpuModelCompareResult',
   'data': [ 'incompatible', 'identical', 'superset', 'subset' ] }
+
+##
+# @CpuModelExpansionInfo:
+#
+# The result of a cpu model expansion.
+#
+# @model: the expanded CpuModelInfo.
+#
+# Since: 2.8
+##
+{ 'struct': 'CpuModelExpansionInfo',
+  'data': { 'model': 'CpuModelInfo' } }
+
+##
+# @query-cpu-model-expansion:
+#
+# Expands a given CPU model (or a combination of CPU model + additional options)
+# to different granularities, allowing tooling to get an understanding what a
+# specific CPU model looks like in QEMU under a certain configuration.
+#
+# This interface can be used to query the "host" CPU model.
+#
+# The data returned by this command may be affected by:
+#
+# * QEMU version: CPU models may look different depending on the QEMU version.
+#   (Except for CPU models reported as "static" in query-cpu-definitions.)
+# * machine-type: CPU model  may look different depending on the machine-type.
+#   (Except for CPU models reported as "static" in query-cpu-definitions.)
+# * machine options (including accelerator): in some architectures, CPU models
+#   may look different depending on machine and accelerator options. (Except for
+#   CPU models reported as "static" in query-cpu-definitions.)
+# * "-cpu" arguments and global properties: arguments to the -cpu option and
+#   global properties may affect expansion of CPU models. Using
+#   query-cpu-model-expansion while using these is not advised.
+#
+# Some architectures may not support all expansion types. s390x supports
+# "full" and "static". Arm only supports "full".
+#
+# Returns: a CpuModelExpansionInfo. Returns an error if expanding CPU models is
+#          not supported, if the model cannot be expanded, if the model contains
+#          an unknown CPU definition name, unknown properties or properties
+#          with a wrong type. Also returns an error if an expansion type is
+#          not supported.
+#
+# Since: 2.8
+##
+{ 'command': 'query-cpu-model-expansion',
+  'data': { 'type': 'CpuModelExpansionType',
+  'model': 'CpuModelInfo' },
+  'returns': 'CpuModelExpansionInfo' }
+
diff --git a/qapi/machine-target.json b/qapi/machine-target.json
index 1cacfde88e..cfc8b0c7f8 100644
--- a/qapi/machine-target.json
+++ b/qapi/machine-target.json
@@ -130,62 +130,6 @@
   'returns': 'CpuModelBaselineInfo',
   'if': 'TARGET_S390X' }
 
-##
-# @CpuModelExpansionInfo:
-#
-# The result of a cpu model expansion.
-#
-# @model: the expanded CpuModelInfo.
-#
-# Since: 2.8
-##
-{ 'struct': 'CpuModelExpansionInfo',
-  'data': { 'model': 'CpuModelInfo' },
-  'if': { 'any': [ 'TARGET_S390X',
-                   'TARGET_I386',
-                   'TARGET_ARM' ] } }
-
-##
-# @query-cpu-model-expansion:
-#
-# Expands a given CPU model (or a combination of CPU model + additional options)
-# to different granularities, allowing tooling to get an understanding what a
-# specific CPU model looks like in QEMU under a certain configuration.
-#
-# This interface can be used to query the "host" CPU model.
-#
-# The data returned by this command may be affected by:
-#
-# * QEMU version: CPU models may look different depending on the QEMU version.
-#   (Except for CPU models reported as "static" in query-cpu-definitions.)
-# * machine-type: CPU model  may look different depending on the machine-type.
-#   (Except for CPU models reported as "static" in query-cpu-definitions.)
-# * machine options (including accelerator): in some architectures, CPU models
-#   may look different depending on machine and accelerator options. (Except for
-#   CPU models reported as "static" in query-cpu-definitions.)
-# * "-cpu" arguments and global properties: arguments to the -cpu option and
-#   global properties may affect expansion of CPU models. Using
-#   query-cpu-model-expansion while using these is not advised.
-#
-# Some architectures may not support all expansion types. s390x supports
-# "full" and "static". Arm only supports "full".
-#
-# Returns: a CpuModelExpansionInfo. Returns an error if expanding CPU models is
-#          not supported, if the model cannot be expanded, if the model contains
-#          an unknown CPU definition name, unknown properties or properties
-#          with a wrong type. Also returns an error if an expansion type is
-#          not supported.
-#
-# Since: 2.8
-##
-{ 'command': 'query-cpu-model-expansion',
-  'data': { 'type': 'CpuModelExpansionType',
-            'model': 'CpuModelInfo' },
-  'returns': 'CpuModelExpansionInfo',
-  'if': { 'any': [ 'TARGET_S390X',
-                   'TARGET_I386',
-                   'TARGET_ARM' ] } }
-
 ##
 # @CpuDefinitionInfo:
 #
diff --git a/target/arm/arm-qmp-cmds.c b/target/arm/arm-qmp-cmds.c
index c8fa524002..e591e7c362 100644
--- a/target/arm/arm-qmp-cmds.c
+++ b/target/arm/arm-qmp-cmds.c
@@ -21,6 +21,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "cpu.h"
 #include "hw/boards.h"
 #include "kvm_arm.h"
 #include "qapi/error.h"
@@ -99,9 +100,9 @@ static const char *cpu_model_advertised_features[] = {
     NULL
 };
 
-CpuModelExpansionInfo *qmp_query_cpu_model_expansion(CpuModelExpansionType type,
-                                                     CpuModelInfo *model,
-                                                     Error **errp)
+CpuModelExpansionInfo *get_cpu_model_expansion(CpuModelExpansionType type,
+                                               CpuModelInfo *model,
+                                               Error **errp)
 {
     CpuModelExpansionInfo *expansion_info;
     const QDict *qdict_in = NULL;
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index c097cae988..d5dcc2ef11 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -2660,7 +2660,12 @@ bool write_cpustate_to_list(ARMCPU *cpu, bool kvm_sync);
 #define TYPE_ARM_HOST_CPU "host-" TYPE_ARM_CPU
 
 #define cpu_list arm_cpu_list
-
+#ifdef CONFIG_SOFTMMU
+CpuModelExpansionInfo *get_cpu_model_expansion(CpuModelExpansionType type,
+                                               CpuModelInfo *model,
+                                               Error **errp);
+#define cpu_model_expansion get_cpu_model_expansion
+#endif
 /* ARM has the following "translation regimes" (as the ARM ARM calls them):
  *
  * If EL3 is 64-bit:
diff --git a/target/i386/cpu-sysemu.c b/target/i386/cpu-sysemu.c
index 28115edf44..c01c85839c 100644
--- a/target/i386/cpu-sysemu.c
+++ b/target/i386/cpu-sysemu.c
@@ -176,10 +176,9 @@ out:
     return xc;
 }
 
-CpuModelExpansionInfo *
-qmp_query_cpu_model_expansion(CpuModelExpansionType type,
-                                                      CpuModelInfo *model,
-                                                      Error **errp)
+CpuModelExpansionInfo *get_cpu_model_expansion(CpuModelExpansionType type,
+                                               CpuModelInfo *model,
+                                               Error **errp)
 {
     X86CPU *xc = NULL;
     Error *err = NULL;
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index d243e290d3..7ab633c642 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -2209,6 +2209,12 @@ uint64_t cpu_get_tsc(CPUX86State *env);
 #endif
 
 #define cpu_list x86_cpu_list
+#ifdef CONFIG_SOFTMMU
+CpuModelExpansionInfo *get_cpu_model_expansion(CpuModelExpansionType type,
+                                               CpuModelInfo *model,
+                                               Error **errp);
+#define cpu_model_expansion get_cpu_model_expansion
+#endif
 
 /* MMU modes definitions */
 #define MMU_KSMAP_IDX   0
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 7d6d01325b..e67a2b386e 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -799,6 +799,13 @@ void s390_cpu_list(void);
 void s390_set_qemu_cpu_model(uint16_t type, uint8_t gen, uint8_t ec_ga,
                              const S390FeatInit feat_init);
 
+#ifdef CONFIG_SOFTMMU
+/* cpu_models-sysemu.c */
+CpuModelExpansionInfo *get_cpu_model_expansion(CpuModelExpansionType type,
+                                               CpuModelInfo *model,
+                                               Error **errp);
+#define cpu_model_expansion get_cpu_model_expansion
+#endif
 
 /* helper.c */
 #define S390_CPU_TYPE_SUFFIX "-" TYPE_S390_CPU
diff --git a/target/s390x/cpu_models_sysemu.c b/target/s390x/cpu_models_sysemu.c
index 63981bf36b..ef3845f11c 100644
--- a/target/s390x/cpu_models_sysemu.c
+++ b/target/s390x/cpu_models_sysemu.c
@@ -213,9 +213,9 @@ static void cpu_info_from_model(CpuModelInfo *info, const S390CPUModel *model,
     }
 }
 
-CpuModelExpansionInfo *qmp_query_cpu_model_expansion(CpuModelExpansionType type,
-                                                      CpuModelInfo *model,
-                                                      Error **errp)
+CpuModelExpansionInfo *get_cpu_model_expansion(CpuModelExpansionType type,
+                                               CpuModelInfo *model,
+                                               Error **errp)
 {
     Error *err = NULL;
     CpuModelExpansionInfo *expansion_info = NULL;
-- 
2.30.2



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 3/3] cpu, qdict, vl: Enable printing options for CPU type
  2023-03-14 10:00 [PATCH v2 0/3] Enable -cpu <cpu>,help Dinah Baum
  2023-03-14 10:00 ` [PATCH v2 1/3] qapi/machine-target: refactor machine-target Dinah Baum
  2023-03-14 10:00 ` [PATCH v2 2/3] cpu, qapi, target/arm, i386, s390x: Generalize query-cpu-model-expansion Dinah Baum
@ 2023-03-14 10:00 ` Dinah Baum
  2023-03-22  3:16 ` [PATCH v2 0/3] Enable -cpu <cpu>,help Dinah B
  3 siblings, 0 replies; 8+ messages in thread
From: Dinah Baum @ 2023-03-14 10:00 UTC (permalink / raw)
  To: qemu-devel
  Cc: Dinah Baum, Eduardo Habkost, Marcel Apfelbaum,
	Philippe Mathieu-Daudé, Yanan Wang, Richard Henderson,
	Paolo Bonzini, Markus Armbruster

Change parsing of -cpu argument to allow -cpu cpu,help
to print options for the CPU type similar to
how the '-device' option works.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1480

Signed-off-by: Dinah Baum <dinahbaum123@gmail.com>
---
 cpu.c                     | 41 +++++++++++++++++++++++++++++++++++++++
 include/exec/cpu-common.h |  2 ++
 include/qapi/qmp/qdict.h  |  2 ++
 qemu-options.hx           |  7 ++++---
 qobject/qdict.c           |  5 +++++
 softmmu/vl.c              | 36 ++++++++++++++++++++++++++++++++--
 6 files changed, 88 insertions(+), 5 deletions(-)

diff --git a/cpu.c b/cpu.c
index c09edc4556..fe33c51061 100644
--- a/cpu.c
+++ b/cpu.c
@@ -23,7 +23,9 @@
 #include "exec/target_page.h"
 #include "hw/qdev-core.h"
 #include "hw/qdev-properties.h"
+#include "qemu/cutils.h"
 #include "qemu/error-report.h"
+#include "qemu/qemu-print.h"
 #include "migration/vmstate.h"
 #ifdef CONFIG_USER_ONLY
 #include "qemu.h"
@@ -42,6 +44,8 @@
 #include "hw/core/accel-cpu.h"
 #include "trace/trace-root.h"
 #include "qemu/accel.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qobject.h"
 
 uintptr_t qemu_host_page_size;
 intptr_t qemu_host_page_mask;
@@ -311,6 +315,43 @@ CpuModelExpansionInfo *qmp_query_cpu_model_expansion(CpuModelExpansionType type,
     return get_cpu_model_expansion_info(type, model, errp);
 }
 
+void list_cpu_model_expansion(CpuModelExpansionType type,
+                              CpuModelInfo *model,
+                              Error **errp)
+{
+    CpuModelExpansionInfo *expansion_info;
+    QDict *qdict;
+    QDictEntry *qdict_entry;
+    const char *key;
+    QObject *obj;
+    QType q_type;
+    GPtrArray *array;
+    int i;
+    const char *type_name;
+
+    expansion_info = get_cpu_model_expansion_info(type, model, errp);
+    if (expansion_info) {
+        qdict = qobject_to(QDict, expansion_info->model->props);
+        if (qdict) {
+            qemu_printf("%s features:\n", model->name);
+            array = g_ptr_array_new();
+            for (qdict_entry = (QDictEntry *)qdict_first(qdict); qdict_entry;
+                 qdict_entry = (QDictEntry *)qdict_next(qdict, qdict_entry)) {
+                g_ptr_array_add(array, qdict_entry);
+            }
+            g_ptr_array_sort(array, (GCompareFunc)dict_key_compare);
+            for (i = 0; i < array->len; i++) {
+                qdict_entry = array->pdata[i];
+                key = qdict_entry_key(qdict_entry);
+                obj = qdict_get(qdict, key);
+                q_type = qobject_type(obj);
+                type_name = QType_str(q_type);
+                qemu_printf("  %s=<%s>\n", key, type_name);
+            }
+        }
+    }
+}
+
 #if defined(CONFIG_USER_ONLY)
 void tb_invalidate_phys_addr(target_ulong addr)
 {
diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index ec6024dfde..8fc05307ad 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -174,5 +174,7 @@ typedef void (*cpu_model_expansion_func)(CpuModelExpansionType type,
 CpuModelExpansionInfo *get_cpu_model_expansion_info(CpuModelExpansionType type,
                                                     CpuModelInfo *model,
                                                     Error **errp);
+void list_cpu_model_expansion(CpuModelExpansionType type,
+                              CpuModelInfo *model, Error **errp);
 
 #endif /* CPU_COMMON_H */
diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
index 82e90fc072..1ff9523a13 100644
--- a/include/qapi/qmp/qdict.h
+++ b/include/qapi/qmp/qdict.h
@@ -68,4 +68,6 @@ const char *qdict_get_try_str(const QDict *qdict, const char *key);
 
 QDict *qdict_clone_shallow(const QDict *src);
 
+int dict_key_compare(QDictEntry **entry1, QDictEntry **entry2);
+
 #endif /* QDICT_H */
diff --git a/qemu-options.hx b/qemu-options.hx
index d42f60fb91..d5284b9330 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -169,11 +169,12 @@ SRST
 ERST
 
 DEF("cpu", HAS_ARG, QEMU_OPTION_cpu,
-    "-cpu cpu        select CPU ('-cpu help' for list)\n", QEMU_ARCH_ALL)
+    "-cpu cpu        select CPU ('-cpu help' for list)\n"
+    "                use '-cpu cpu,help' to print possible properties\n", QEMU_ARCH_ALL)
 SRST
 ``-cpu model``
-    Select CPU model (``-cpu help`` for list and additional feature
-    selection)
+    Select CPU model (``-cpu help`` and ``-cpu cpu,help``) for list and additional feature
+    selection
 ERST
 
 DEF("accel", HAS_ARG, QEMU_OPTION_accel,
diff --git a/qobject/qdict.c b/qobject/qdict.c
index 8faff230d3..31407e62f6 100644
--- a/qobject/qdict.c
+++ b/qobject/qdict.c
@@ -447,3 +447,8 @@ void qdict_unref(QDict *q)
 {
     qobject_unref(q);
 }
+
+int dict_key_compare(QDictEntry **entry1, QDictEntry **entry2)
+{
+    return g_strcmp0(qdict_entry_key(*entry1), qdict_entry_key(*entry2));
+}
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 3340f63c37..a9d70e559e 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -500,6 +500,15 @@ static QemuOptsList qemu_action_opts = {
     },
 };
 
+static QemuOptsList qemu_cpu_opts = {
+    .name = "cpu",
+    .implied_opt_name = "cpu",
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_cpu_opts.head),
+    .desc = {
+        { /* end of list */ }
+    },
+};
+
 const char *qemu_get_vm_name(void)
 {
     return qemu_name;
@@ -1147,6 +1156,26 @@ static int device_init_func(void *opaque, QemuOpts *opts, Error **errp)
     return 0;
 }
 
+static int cpu_help_func(void *opaque, QemuOpts *opts, Error **errp)
+{
+    CpuModelInfo *model;
+
+    if (cpu_option && is_help_option(cpu_option)) {
+        list_cpus(cpu_option);
+        return 1;
+    }
+
+    if (!cpu_option || !qemu_opt_has_help_opt(opts)) {
+        return 0;
+    }
+
+    model = g_new0(CpuModelInfo, 1);
+    model->name = (char *)qemu_opt_get(opts, "cpu");
+    /* TODO: handle other expansion cases */
+    list_cpu_model_expansion(CPU_MODEL_EXPANSION_TYPE_FULL, model, errp);
+    return 1;
+}
+
 static int chardev_init_func(void *opaque, QemuOpts *opts, Error **errp)
 {
     Error *local_err = NULL;
@@ -2431,8 +2460,9 @@ static void qemu_process_help_options(void)
      * type and the user did not specify one, so that the user doesn't need
      * to say '-cpu help -machine something'.
      */
-    if (cpu_option && is_help_option(cpu_option)) {
-        list_cpus(cpu_option);
+    Error *errp = NULL;
+    if (qemu_opts_foreach(qemu_find_opts("cpu"),
+                          cpu_help_func, NULL, &errp)) {
         exit(0);
     }
 
@@ -2672,6 +2702,7 @@ void qemu_init(int argc, char **argv)
     qemu_add_opts(&qemu_semihosting_config_opts);
     qemu_add_opts(&qemu_fw_cfg_opts);
     qemu_add_opts(&qemu_action_opts);
+    qemu_add_opts(&qemu_cpu_opts);
     module_call_init(MODULE_INIT_OPTS);
 
     error_init(argv[0]);
@@ -2723,6 +2754,7 @@ void qemu_init(int argc, char **argv)
             switch(popt->index) {
             case QEMU_OPTION_cpu:
                 /* hw initialization will check this */
+                qemu_opts_parse_noisily(qemu_find_opts("cpu"), optarg, true);
                 cpu_option = optarg;
                 break;
             case QEMU_OPTION_hda:
-- 
2.30.2



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/3] qapi/machine-target: refactor machine-target
  2023-03-14 10:00 ` [PATCH v2 1/3] qapi/machine-target: refactor machine-target Dinah Baum
@ 2023-03-14 10:29   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-03-14 10:29 UTC (permalink / raw)
  To: Dinah Baum, qemu-devel
  Cc: Eduardo Habkost, Marcel Apfelbaum, Yanan Wang, Eric Blake,
	Markus Armbruster, Michael Roth

On 14/3/23 11:00, Dinah Baum wrote:
> Moved architecture agnostic data types to their own
> file to avoid "attempt to use poisoned TARGET_*"
> error that results when including qapi header
> with commands that aren't defined for all architectures.
> Required to implement enabling `query-cpu-model-expansion`
> on all architectures
> 
> Signed-off-by: Dinah Baum <dinahbaum123@gmail.com>
> ---
>   MAINTAINERS                     |  1 +
>   qapi/machine-target-common.json | 79 +++++++++++++++++++++++++++++++++
>   qapi/machine-target.json        | 73 +-----------------------------
>   qapi/meson.build                |  1 +
>   4 files changed, 82 insertions(+), 72 deletions(-)
>   create mode 100644 qapi/machine-target-common.json

> diff --git a/qapi/meson.build b/qapi/meson.build
> index 9fd480c4d8..48be47170f 100644
> --- a/qapi/meson.build
> +++ b/qapi/meson.build
> @@ -38,6 +38,7 @@ qapi_all_modules = [
>     'job',
>     'machine',
>     'machine-target',
> +  'machine-target-common',
>     'migration',
>     'misc',
>     'misc-target',

Eh, this reminds me of
https://lore.kernel.org/qemu-devel/20220204152924.6253-5-f4bug@amsat.org/

I wonder between -common / -any / -all. Anyhow,

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 0/3] Enable -cpu <cpu>,help
  2023-03-14 10:00 [PATCH v2 0/3] Enable -cpu <cpu>,help Dinah Baum
                   ` (2 preceding siblings ...)
  2023-03-14 10:00 ` [PATCH v2 3/3] cpu, qdict, vl: Enable printing options for CPU type Dinah Baum
@ 2023-03-22  3:16 ` Dinah B
  3 siblings, 0 replies; 8+ messages in thread
From: Dinah B @ 2023-03-22  3:16 UTC (permalink / raw)
  To: qemu-devel

[-- Attachment #1: Type: text/plain, Size: 1878 bytes --]

Friendly ping for code review on this patch series.

Full series:
https://lore.kernel.org/qemu-devel/20230314100026.536079-1-dinahbaum123@gmail.com/

Thanks,
-DInah

On Tue, Mar 14, 2023 at 6:00 AM Dinah Baum <dinahbaum123@gmail.com> wrote:

> Part 1 is a refactor/code motion patch for
> qapi/machine target required for setup of
>
> Part 2 which enables query-cpu-model-expansion
> on all architectures
>
> Part 3 implements the '<cpu>,help' feature
>
> Limitations:
> Currently only 'FULL' expansion queries are implemented since
> that's the only type enabled on the architectures that
> allow feature probing
>
> Unlike the 'device,help' command, default values aren't
> printed
>
> Dinah Baum (3):
>   qapi/machine-target: refactor machine-target
>   cpu, qapi, target/arm, i386, s390x: Generalize
>     query-cpu-model-expansion
>   cpu, qdict, vl: Enable printing options for CPU type
>
>  MAINTAINERS                      |   1 +
>  cpu.c                            |  61 +++++++++++++++
>  include/exec/cpu-common.h        |  10 +++
>  include/qapi/qmp/qdict.h         |   2 +
>  qapi/machine-target-common.json  | 130 +++++++++++++++++++++++++++++++
>  qapi/machine-target.json         | 129 +-----------------------------
>  qapi/meson.build                 |   1 +
>  qemu-options.hx                  |   7 +-
>  qobject/qdict.c                  |   5 ++
>  softmmu/vl.c                     |  36 ++++++++-
>  target/arm/arm-qmp-cmds.c        |   7 +-
>  target/arm/cpu.h                 |   7 +-
>  target/i386/cpu-sysemu.c         |   7 +-
>  target/i386/cpu.h                |   6 ++
>  target/s390x/cpu.h               |   7 ++
>  target/s390x/cpu_models_sysemu.c |   6 +-
>  16 files changed, 278 insertions(+), 144 deletions(-)
>  create mode 100644 qapi/machine-target-common.json
>
> --
> 2.30.2
>
>

[-- Attachment #2: Type: text/html, Size: 2612 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 3/3] cpu, qdict, vl: Enable printing options for CPU type
  2023-04-04  1:19 [RESEND PATCH " Dinah Baum
@ 2023-04-04  1:19 ` Dinah Baum
  2023-05-26  6:07   ` Markus Armbruster
  0 siblings, 1 reply; 8+ messages in thread
From: Dinah Baum @ 2023-04-04  1:19 UTC (permalink / raw)
  To: qemu-devel
  Cc: Dinah Baum, Eduardo Habkost, Marcel Apfelbaum,
	Philippe Mathieu-Daudé, Yanan Wang, Richard Henderson,
	Paolo Bonzini, Markus Armbruster

Change parsing of -cpu argument to allow -cpu cpu,help
to print options for the CPU type similar to
how the '-device' option works.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1480

Signed-off-by: Dinah Baum <dinahbaum123@gmail.com>
---
 cpu.c                     | 41 +++++++++++++++++++++++++++++++++++++++
 include/exec/cpu-common.h |  2 ++
 include/qapi/qmp/qdict.h  |  2 ++
 qemu-options.hx           |  7 ++++---
 qobject/qdict.c           |  5 +++++
 softmmu/vl.c              | 36 ++++++++++++++++++++++++++++++++--
 6 files changed, 88 insertions(+), 5 deletions(-)

diff --git a/cpu.c b/cpu.c
index daf4e1ff0d..5f8a72e51f 100644
--- a/cpu.c
+++ b/cpu.c
@@ -23,7 +23,9 @@
 #include "exec/target_page.h"
 #include "hw/qdev-core.h"
 #include "hw/qdev-properties.h"
+#include "qemu/cutils.h"
 #include "qemu/error-report.h"
+#include "qemu/qemu-print.h"
 #include "migration/vmstate.h"
 #ifdef CONFIG_USER_ONLY
 #include "qemu.h"
@@ -43,6 +45,8 @@
 #include "trace/trace-root.h"
 #include "qemu/accel.h"
 #include "qemu/plugin.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qobject.h"
 
 uintptr_t qemu_host_page_size;
 intptr_t qemu_host_page_mask;
@@ -312,6 +316,43 @@ CpuModelExpansionInfo *qmp_query_cpu_model_expansion(CpuModelExpansionType type,
     return get_cpu_model_expansion_info(type, model, errp);
 }
 
+void list_cpu_model_expansion(CpuModelExpansionType type,
+                              CpuModelInfo *model,
+                              Error **errp)
+{
+    CpuModelExpansionInfo *expansion_info;
+    QDict *qdict;
+    QDictEntry *qdict_entry;
+    const char *key;
+    QObject *obj;
+    QType q_type;
+    GPtrArray *array;
+    int i;
+    const char *type_name;
+
+    expansion_info = get_cpu_model_expansion_info(type, model, errp);
+    if (expansion_info) {
+        qdict = qobject_to(QDict, expansion_info->model->props);
+        if (qdict) {
+            qemu_printf("%s features:\n", model->name);
+            array = g_ptr_array_new();
+            for (qdict_entry = (QDictEntry *)qdict_first(qdict); qdict_entry;
+                 qdict_entry = (QDictEntry *)qdict_next(qdict, qdict_entry)) {
+                g_ptr_array_add(array, qdict_entry);
+            }
+            g_ptr_array_sort(array, (GCompareFunc)dict_key_compare);
+            for (i = 0; i < array->len; i++) {
+                qdict_entry = array->pdata[i];
+                key = qdict_entry_key(qdict_entry);
+                obj = qdict_get(qdict, key);
+                q_type = qobject_type(obj);
+                type_name = QType_str(q_type);
+                qemu_printf("  %s=<%s>\n", key, type_name);
+            }
+        }
+    }
+}
+
 #if defined(CONFIG_USER_ONLY)
 void tb_invalidate_phys_addr(target_ulong addr)
 {
diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index ec6024dfde..8fc05307ad 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -174,5 +174,7 @@ typedef void (*cpu_model_expansion_func)(CpuModelExpansionType type,
 CpuModelExpansionInfo *get_cpu_model_expansion_info(CpuModelExpansionType type,
                                                     CpuModelInfo *model,
                                                     Error **errp);
+void list_cpu_model_expansion(CpuModelExpansionType type,
+                              CpuModelInfo *model, Error **errp);
 
 #endif /* CPU_COMMON_H */
diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
index 82e90fc072..1ff9523a13 100644
--- a/include/qapi/qmp/qdict.h
+++ b/include/qapi/qmp/qdict.h
@@ -68,4 +68,6 @@ const char *qdict_get_try_str(const QDict *qdict, const char *key);
 
 QDict *qdict_clone_shallow(const QDict *src);
 
+int dict_key_compare(QDictEntry **entry1, QDictEntry **entry2);
+
 #endif /* QDICT_H */
diff --git a/qemu-options.hx b/qemu-options.hx
index 59bdf67a2c..10601626b7 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -169,11 +169,12 @@ SRST
 ERST
 
 DEF("cpu", HAS_ARG, QEMU_OPTION_cpu,
-    "-cpu cpu        select CPU ('-cpu help' for list)\n", QEMU_ARCH_ALL)
+    "-cpu cpu        select CPU ('-cpu help' for list)\n"
+    "                use '-cpu cpu,help' to print possible properties\n", QEMU_ARCH_ALL)
 SRST
 ``-cpu model``
-    Select CPU model (``-cpu help`` for list and additional feature
-    selection)
+    Select CPU model (``-cpu help`` and ``-cpu cpu,help``) for list and additional feature
+    selection
 ERST
 
 DEF("accel", HAS_ARG, QEMU_OPTION_accel,
diff --git a/qobject/qdict.c b/qobject/qdict.c
index 8faff230d3..31407e62f6 100644
--- a/qobject/qdict.c
+++ b/qobject/qdict.c
@@ -447,3 +447,8 @@ void qdict_unref(QDict *q)
 {
     qobject_unref(q);
 }
+
+int dict_key_compare(QDictEntry **entry1, QDictEntry **entry2)
+{
+    return g_strcmp0(qdict_entry_key(*entry1), qdict_entry_key(*entry2));
+}
diff --git a/softmmu/vl.c b/softmmu/vl.c
index ea20b23e4c..af6753a7e3 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -500,6 +500,15 @@ static QemuOptsList qemu_action_opts = {
     },
 };
 
+static QemuOptsList qemu_cpu_opts = {
+    .name = "cpu",
+    .implied_opt_name = "cpu",
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_cpu_opts.head),
+    .desc = {
+        { /* end of list */ }
+    },
+};
+
 const char *qemu_get_vm_name(void)
 {
     return qemu_name;
@@ -1147,6 +1156,26 @@ static int device_init_func(void *opaque, QemuOpts *opts, Error **errp)
     return 0;
 }
 
+static int cpu_help_func(void *opaque, QemuOpts *opts, Error **errp)
+{
+    CpuModelInfo *model;
+
+    if (cpu_option && is_help_option(cpu_option)) {
+        list_cpus(cpu_option);
+        return 1;
+    }
+
+    if (!cpu_option || !qemu_opt_has_help_opt(opts)) {
+        return 0;
+    }
+
+    model = g_new0(CpuModelInfo, 1);
+    model->name = (char *)qemu_opt_get(opts, "cpu");
+    /* TODO: handle other expansion cases */
+    list_cpu_model_expansion(CPU_MODEL_EXPANSION_TYPE_FULL, model, errp);
+    return 1;
+}
+
 static int chardev_init_func(void *opaque, QemuOpts *opts, Error **errp)
 {
     Error *local_err = NULL;
@@ -2431,8 +2460,9 @@ static void qemu_process_help_options(void)
      * type and the user did not specify one, so that the user doesn't need
      * to say '-cpu help -machine something'.
      */
-    if (cpu_option && is_help_option(cpu_option)) {
-        list_cpus(cpu_option);
+    Error *errp = NULL;
+    if (qemu_opts_foreach(qemu_find_opts("cpu"),
+                          cpu_help_func, NULL, &errp)) {
         exit(0);
     }
 
@@ -2673,6 +2703,7 @@ void qemu_init(int argc, char **argv)
     qemu_add_opts(&qemu_semihosting_config_opts);
     qemu_add_opts(&qemu_fw_cfg_opts);
     qemu_add_opts(&qemu_action_opts);
+    qemu_add_opts(&qemu_cpu_opts);
     module_call_init(MODULE_INIT_OPTS);
 
     error_init(argv[0]);
@@ -2724,6 +2755,7 @@ void qemu_init(int argc, char **argv)
             switch(popt->index) {
             case QEMU_OPTION_cpu:
                 /* hw initialization will check this */
+                qemu_opts_parse_noisily(qemu_find_opts("cpu"), optarg, true);
                 cpu_option = optarg;
                 break;
             case QEMU_OPTION_hda:
-- 
2.30.2



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 3/3] cpu, qdict, vl: Enable printing options for CPU type
  2023-04-04  1:19 ` [PATCH v2 3/3] cpu, qdict, vl: Enable printing options for CPU type Dinah Baum
@ 2023-05-26  6:07   ` Markus Armbruster
  0 siblings, 0 replies; 8+ messages in thread
From: Markus Armbruster @ 2023-05-26  6:07 UTC (permalink / raw)
  To: Dinah Baum
  Cc: qemu-devel, Eduardo Habkost, Marcel Apfelbaum,
	Philippe Mathieu-Daudé, Yanan Wang, Richard Henderson,
	Paolo Bonzini

This is really, really, *really* for maintainers of the code parsing
-cpu to review.  Code parsing -cpu:

* parse_cpu_option() in cpu.c

  Eduardo Habkost <eduardo@habkost.net> (supporter:Machine core)
  Marcel Apfelbaum <marcel.apfelbaum@gmail.com> (supporter:Machine core)
  "Philippe Mathieu-Daudé" <philmd@linaro.org> (reviewer:Machine core)
  Yanan Wang <wangyanan55@huawei.com> (reviewer:Machine core)

* cpu_common_parse_features() in hw/core/cpu-common.c

  No maintainers *boggle*

* x86_cpu_parse_featurestr() in qemu/target/i386/cpu.c

  No maintainers *BOGGLE*

* sparc_cpu_parse_features() in target/sparc/cpu.c

  Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> (maintainer:SPARC TCG CPUs)
  Artyom Tarasenko <atar4qemu@gmail.com> (maintainer:SPARC TCG CPUs)

Paolo, Richard, Eduardo, care to get these covered in MAINTAINERS?

Since the patch has been waiting for review for so long, I'll give it a
try, even though I'm only passingly familiar with -cpu parsing.

Paolo, I have a question for you further down.

Dinah Baum <dinahbaum123@gmail.com> writes:

> Change parsing of -cpu argument to allow -cpu cpu,help
> to print options for the CPU type similar to
> how the '-device' option works.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1480
>
> Signed-off-by: Dinah Baum <dinahbaum123@gmail.com>
> ---
>  cpu.c                     | 41 +++++++++++++++++++++++++++++++++++++++
>  include/exec/cpu-common.h |  2 ++
>  include/qapi/qmp/qdict.h  |  2 ++
>  qemu-options.hx           |  7 ++++---
>  qobject/qdict.c           |  5 +++++
>  softmmu/vl.c              | 36 ++++++++++++++++++++++++++++++++--
>  6 files changed, 88 insertions(+), 5 deletions(-)
>
> diff --git a/cpu.c b/cpu.c
> index daf4e1ff0d..5f8a72e51f 100644
> --- a/cpu.c
> +++ b/cpu.c
> @@ -23,7 +23,9 @@
>  #include "exec/target_page.h"
>  #include "hw/qdev-core.h"
>  #include "hw/qdev-properties.h"
> +#include "qemu/cutils.h"
>  #include "qemu/error-report.h"
> +#include "qemu/qemu-print.h"
>  #include "migration/vmstate.h"
>  #ifdef CONFIG_USER_ONLY
>  #include "qemu.h"
> @@ -43,6 +45,8 @@
>  #include "trace/trace-root.h"
>  #include "qemu/accel.h"
>  #include "qemu/plugin.h"
> +#include "qapi/qmp/qdict.h"
> +#include "qapi/qmp/qobject.h"
>  
>  uintptr_t qemu_host_page_size;
>  intptr_t qemu_host_page_mask;
> @@ -312,6 +316,43 @@ CpuModelExpansionInfo *qmp_query_cpu_model_expansion(CpuModelExpansionType type,
>      return get_cpu_model_expansion_info(type, model, errp);
>  }
>  
> +void list_cpu_model_expansion(CpuModelExpansionType type,
> +                              CpuModelInfo *model,
> +                              Error **errp)
> +{
> +    CpuModelExpansionInfo *expansion_info;
> +    QDict *qdict;
> +    QDictEntry *qdict_entry;
> +    const char *key;
> +    QObject *obj;
> +    QType q_type;
> +    GPtrArray *array;
> +    int i;
> +    const char *type_name;
> +
> +    expansion_info = get_cpu_model_expansion_info(type, model, errp);
> +    if (expansion_info) {

Avoid nesting:

       if (!expansion_info) {
           return;
       }

       ... work with expansion_info ...

> +        qdict = qobject_to(QDict, expansion_info->model->props);
> +        if (qdict) {

Likewise.

> +            qemu_printf("%s features:\n", model->name);
> +            array = g_ptr_array_new();

Name it @props, please.

> +            for (qdict_entry = (QDictEntry *)qdict_first(qdict); qdict_entry;
> +                 qdict_entry = (QDictEntry *)qdict_next(qdict, qdict_entry)) {
> +                g_ptr_array_add(array, qdict_entry);
> +            }

@qdict can change while we're using it here (if it could, your code
would be wrong).  So, no need for a flexible array.  Create a dynamic
one with g_new(QDictEntry, qdict_size(qdict), fill it, then sort with
qsort().

> +            g_ptr_array_sort(array, (GCompareFunc)dict_key_compare);

Casting function pointers is iffy.  The clean way is to define the
function so it is a GCompareFunc exactly, and have it cast its arguments
if necessary.

> +            for (i = 0; i < array->len; i++) {
> +                qdict_entry = array->pdata[i];
> +                key = qdict_entry_key(qdict_entry);
> +                obj = qdict_get(qdict, key);
> +                q_type = qobject_type(obj);
> +                type_name = QType_str(q_type);
> +                qemu_printf("  %s=<%s>\n", key, type_name);

Contract to

                   qemu_printf("  %s=<%s>\n",
                               key, QType_str(qobject_type(obj)));

Actually, don't use QType_str(), because the type comes out as "qnum",
"qstring", "qbool" (bad), or as "qdict", "qlist" (worse), or as "qnull"
(still worse, but impossible, I think).

Is CpuModelInfo the appropriate source?  Could we get properties
straight from QOM instead, like we do for "-device TYPE,help" and
"-object TYPE,help"?  I guess this question is for Paolo.

> +            }
> +        }
> +    }
> +}
> +
>  #if defined(CONFIG_USER_ONLY)
>  void tb_invalidate_phys_addr(target_ulong addr)
>  {
> diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
> index ec6024dfde..8fc05307ad 100644
> --- a/include/exec/cpu-common.h
> +++ b/include/exec/cpu-common.h
> @@ -174,5 +174,7 @@ typedef void (*cpu_model_expansion_func)(CpuModelExpansionType type,
>  CpuModelExpansionInfo *get_cpu_model_expansion_info(CpuModelExpansionType type,
>                                                      CpuModelInfo *model,
>                                                      Error **errp);
> +void list_cpu_model_expansion(CpuModelExpansionType type,
> +                              CpuModelInfo *model, Error **errp);
>  
>  #endif /* CPU_COMMON_H */
> diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
> index 82e90fc072..1ff9523a13 100644
> --- a/include/qapi/qmp/qdict.h
> +++ b/include/qapi/qmp/qdict.h
> @@ -68,4 +68,6 @@ const char *qdict_get_try_str(const QDict *qdict, const char *key);
>  
>  QDict *qdict_clone_shallow(const QDict *src);
>  
> +int dict_key_compare(QDictEntry **entry1, QDictEntry **entry2);
> +
>  #endif /* QDICT_H */
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 59bdf67a2c..10601626b7 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -169,11 +169,12 @@ SRST
>  ERST
>  
>  DEF("cpu", HAS_ARG, QEMU_OPTION_cpu,
> -    "-cpu cpu        select CPU ('-cpu help' for list)\n", QEMU_ARCH_ALL)
> +    "-cpu cpu        select CPU ('-cpu help' for list)\n"
> +    "                use '-cpu cpu,help' to print possible properties\n", QEMU_ARCH_ALL)
>  SRST
>  ``-cpu model``
> -    Select CPU model (``-cpu help`` for list and additional feature
> -    selection)
> +    Select CPU model (``-cpu help`` and ``-cpu cpu,help``) for list and additional feature
> +    selection
>  ERST
>  
>  DEF("accel", HAS_ARG, QEMU_OPTION_accel,
> diff --git a/qobject/qdict.c b/qobject/qdict.c
> index 8faff230d3..31407e62f6 100644
> --- a/qobject/qdict.c
> +++ b/qobject/qdict.c
> @@ -447,3 +447,8 @@ void qdict_unref(QDict *q)
>  {
>      qobject_unref(q);
>  }
> +
> +int dict_key_compare(QDictEntry **entry1, QDictEntry **entry2)
> +{
> +    return g_strcmp0(qdict_entry_key(*entry1), qdict_entry_key(*entry2));
> +}

This file's external functions start with qdict_, not dict_.

There is just one caller.  Let's put the function next to it, and make
it static.

> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index ea20b23e4c..af6753a7e3 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -500,6 +500,15 @@ static QemuOptsList qemu_action_opts = {
>      },
>  };
>  
> +static QemuOptsList qemu_cpu_opts = {
> +    .name = "cpu",
> +    .implied_opt_name = "cpu",
> +    .head = QTAILQ_HEAD_INITIALIZER(qemu_cpu_opts.head),
> +    .desc = {
> +        { /* end of list */ }
> +    },
> +};
> +
>  const char *qemu_get_vm_name(void)
>  {
>      return qemu_name;
> @@ -1147,6 +1156,26 @@ static int device_init_func(void *opaque, QemuOpts *opts, Error **errp)
>      return 0;
>  }
>  
> +static int cpu_help_func(void *opaque, QemuOpts *opts, Error **errp)
> +{
> +    CpuModelInfo *model;
> +
> +    if (cpu_option && is_help_option(cpu_option)) {
> +        list_cpus(cpu_option);
> +        return 1;
> +    }
> +
> +    if (!cpu_option || !qemu_opt_has_help_opt(opts)) {
> +        return 0;
> +    }
> +
> +    model = g_new0(CpuModelInfo, 1);
> +    model->name = (char *)qemu_opt_get(opts, "cpu");
> +    /* TODO: handle other expansion cases */
> +    list_cpu_model_expansion(CPU_MODEL_EXPANSION_TYPE_FULL, model, errp);
> +    return 1;
> +}
> +
>  static int chardev_init_func(void *opaque, QemuOpts *opts, Error **errp)
>  {
>      Error *local_err = NULL;
> @@ -2431,8 +2460,9 @@ static void qemu_process_help_options(void)
>       * type and the user did not specify one, so that the user doesn't need
>       * to say '-cpu help -machine something'.
>       */
> -    if (cpu_option && is_help_option(cpu_option)) {
> -        list_cpus(cpu_option);
> +    Error *errp = NULL;
> +    if (qemu_opts_foreach(qemu_find_opts("cpu"),
> +                          cpu_help_func, NULL, &errp)) {
>          exit(0);
>      }
>  
> @@ -2673,6 +2703,7 @@ void qemu_init(int argc, char **argv)
>      qemu_add_opts(&qemu_semihosting_config_opts);
>      qemu_add_opts(&qemu_fw_cfg_opts);
>      qemu_add_opts(&qemu_action_opts);
> +    qemu_add_opts(&qemu_cpu_opts);
>      module_call_init(MODULE_INIT_OPTS);
>  
>      error_init(argv[0]);
> @@ -2724,6 +2755,7 @@ void qemu_init(int argc, char **argv)
>              switch(popt->index) {
>              case QEMU_OPTION_cpu:
>                  /* hw initialization will check this */
> +                qemu_opts_parse_noisily(qemu_find_opts("cpu"), optarg, true);

No :)

We have bespoke parsers for the argument of -cpu: parse_cpu_option()
together with CPUClass methods parse_features().  The syntax they parse
is superficially similar to QemuOpts (parts separated with comma), but
it's not the same.  If it was, we'd use QemuOpts and ditch the bespoke
parsers.

If qemu_opts_parse_noisily() rejects @optarg here, it reports an error,
and we continue anyway.

If parse_cpu_option() also rejects @optarg later on, the error is
reported twice.  Bad.

If it doesn't, the error qemu_opts_parse_noisily() reported is bogus.

If both succeed, they may well yield different parse results.  I can try
to dig up examples if necessary.

As far as I can see, you use the result of qemu_opts_parse_noisily()
only with cpu_help_func().  Can we slot the help feature into the
bespoke parser instead?  Let's have a look.

When the argument of -cpu is "help", qemu_process_help_options() shows
help and exits before we call parse_cpu_option().

parse_cpu_option() splits the argument of -cpu at the first comma into
CPU class name and features.

If you factor the splitting out of parse_cpu_option(), you can call it
from qemu_process_help_options(), then check whether the features are
"help".

>                  cpu_option = optarg;
>                  break;
>              case QEMU_OPTION_hda:



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-05-26  6:08 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-14 10:00 [PATCH v2 0/3] Enable -cpu <cpu>,help Dinah Baum
2023-03-14 10:00 ` [PATCH v2 1/3] qapi/machine-target: refactor machine-target Dinah Baum
2023-03-14 10:29   ` Philippe Mathieu-Daudé
2023-03-14 10:00 ` [PATCH v2 2/3] cpu, qapi, target/arm, i386, s390x: Generalize query-cpu-model-expansion Dinah Baum
2023-03-14 10:00 ` [PATCH v2 3/3] cpu, qdict, vl: Enable printing options for CPU type Dinah Baum
2023-03-22  3:16 ` [PATCH v2 0/3] Enable -cpu <cpu>,help Dinah B
  -- strict thread matches above, loose matches on Subject: below --
2023-04-04  1:19 [RESEND PATCH " Dinah Baum
2023-04-04  1:19 ` [PATCH v2 3/3] cpu, qdict, vl: Enable printing options for CPU type Dinah Baum
2023-05-26  6:07   ` Markus Armbruster

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).