* [PATCH v2 1/3] qapi/machine-target: refactor machine-target
2023-04-04 1:19 [RESEND PATCH v2 0/3] Enable -cpu <cpu>,help Dinah Baum
@ 2023-04-04 1:19 ` Dinah Baum
2023-05-11 14:38 ` Markus Armbruster
2023-04-04 1:19 ` [PATCH v2 2/3] cpu, qapi, target/arm, i386, s390x: Generalize query-cpu-model-expansion Dinah Baum
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Dinah Baum @ 2023-04-04 1:19 UTC (permalink / raw)
To: qemu-devel
Cc: Dinah Baum, Philippe Mathieu-Daudé, Eric Blake,
Markus Armbruster, Eduardo Habkost, Marcel Apfelbaum, Yanan Wang,
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>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
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 ef45b5e71e..fbc4d7be66 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1751,6 +1751,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] 11+ messages in thread* Re: [PATCH v2 1/3] qapi/machine-target: refactor machine-target
2023-04-04 1:19 ` [PATCH v2 1/3] qapi/machine-target: refactor machine-target Dinah Baum
@ 2023-05-11 14:38 ` Markus Armbruster
0 siblings, 0 replies; 11+ messages in thread
From: Markus Armbruster @ 2023-05-11 14:38 UTC (permalink / raw)
To: Dinah Baum
Cc: qemu-devel, Philippe Mathieu-Daudé, Eric Blake,
Eduardo Habkost, Marcel Apfelbaum, Yanan Wang, Michael Roth
First of all, my sincere apologies for the delayed review.
The patch series needs a rebase. But let me review it first.
Dinah Baum <dinahbaum123@gmail.com> writes:
> 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>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> 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
Do we really want to create qapi/machine-target-common.json? Before we
can answer this, I think I should explain how we use QAPI modules so
far.
You already know about target-independent vs. target-dependent code.
In target-dependent code, a multitude of additional macros are
available, such as TARGET_S390X, TARGET_I386, TARGET_ARM, ... We poison
them to prevent accidental use in target-independent code.
Since target-dependent code needs to be compiled per target, we try to
keep as much code as we can target-independent.
QAPI-generated code is target-independent except for code generated for
QAPI modules whose name ends with "-target". Yes, that's a bit of a
hack. See qapi/meson.build.
When a subsystem needs QAPI schema stuff, we generally put it into its
own module. For instance, the PCI subsystem's QAPI schema is in the pci
module (qapi/pci.json). See MAINTAINERS for more.
Most subsystems' QAPI schema is entirely target-independent. If a
subsystem needs some target-dependence in its schema, we split the QAPI
module into a target-dependent and a target-independent part. We have
two such pairs: misc and misc-target, machine and machine-target.
Can we stick to this convention? I.e. move to existing machine.json
instead to new machine-target-common.json. Let's have a closer look.
This patch moves a few types from (machine-dependent)
machine-target.json to new (and machine-independent)
machine-target-common.json.
The next patch moves another type and a command after removing their
machine-dependence.
After both moves, machine-target.json needs to include
machine-target-common.json for CpuModelInfo and CpuModelCompareResult.
Aside: the latter is only ever used in machine-target.json. We could
keep it there.
If we move to machine.json instead, then machine-target.json needs to
include that.
Would that work?
If not: I think the name machine-target-common.json is unfortunate,
because it kind of suggests machine-dependence.
[...]
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 2/3] cpu, qapi, target/arm, i386, s390x: Generalize query-cpu-model-expansion
2023-04-04 1:19 [RESEND PATCH v2 0/3] Enable -cpu <cpu>,help Dinah Baum
2023-04-04 1:19 ` [PATCH v2 1/3] qapi/machine-target: refactor machine-target Dinah Baum
@ 2023-04-04 1:19 ` Dinah Baum
2023-05-11 17:41 ` Markus Armbruster
2023-04-04 1:19 ` [PATCH v2 3/3] cpu, qdict, vl: Enable printing options for CPU type Dinah Baum
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ 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, 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 849bac062c..daf4e1ff0d 100644
--- a/cpu.c
+++ b/cpu.c
@@ -292,6 +292,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 c47e7adcb1..da5d9b4bf9 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -810,6 +810,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] 11+ messages in thread* Re: [PATCH v2 2/3] cpu, qapi, target/arm, i386, s390x: Generalize query-cpu-model-expansion
2023-04-04 1:19 ` [PATCH v2 2/3] cpu, qapi, target/arm, i386, s390x: Generalize query-cpu-model-expansion Dinah Baum
@ 2023-05-11 17:41 ` Markus Armbruster
0 siblings, 0 replies; 11+ messages in thread
From: Markus Armbruster @ 2023-05-11 17:41 UTC (permalink / raw)
To: Dinah Baum
Cc: qemu-devel, Eduardo Habkost, Marcel Apfelbaum,
Philippe Mathieu-Daudé, Yanan Wang, Richard Henderson,
Paolo Bonzini, Eric Blake, Peter Maydell, David Hildenbrand,
Ilya Leoshkevich, Thomas Huth, open list:ARM TCG CPUs,
open list:S390 TCG CPUs
Dinah Baum <dinahbaum123@gmail.com> writes:
> 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
Yes, but why does the change make sense for QMP?
Any idea how hard implementing the thing for more targets would be?
Question, not a demand!
> 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 849bac062c..daf4e1ff0d 100644
> --- a/cpu.c
> +++ b/cpu.c
> @@ -292,6 +292,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");
This is vague enough to leave the user wondering what could be done to
avoid this error and by whom.
Before the patch, it's clear enough: "The command
query-cpu-model-expansion has not been found".
You could go with something like "command not supported for this
target".
The error class changes from CommandNotFound to GenericError. Please
verify libvirt is fine with that.
> + 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' },
Please use the opportunity to fix the indentation.
> + 'returns': 'CpuModelExpansionInfo' }
> +
[Remainder skipped for now...]
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 3/3] cpu, qdict, vl: Enable printing options for CPU type
2023-04-04 1:19 [RESEND PATCH v2 0/3] Enable -cpu <cpu>,help Dinah Baum
2023-04-04 1:19 ` [PATCH v2 1/3] qapi/machine-target: refactor machine-target Dinah Baum
2023-04-04 1:19 ` [PATCH v2 2/3] cpu, qapi, target/arm, i386, s390x: Generalize query-cpu-model-expansion Dinah Baum
@ 2023-04-04 1:19 ` Dinah Baum
2023-05-26 6:07 ` Markus Armbruster
2023-05-11 12:16 ` [RESEND PATCH v2 0/3] Enable -cpu <cpu>,help Peter Maydell
2023-05-26 14:28 ` Igor Mammedov
4 siblings, 1 reply; 11+ 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] 11+ 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; 11+ 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] 11+ messages in thread
* Re: [RESEND PATCH v2 0/3] Enable -cpu <cpu>,help
2023-04-04 1:19 [RESEND PATCH v2 0/3] Enable -cpu <cpu>,help Dinah Baum
` (2 preceding siblings ...)
2023-04-04 1:19 ` [PATCH v2 3/3] cpu, qdict, vl: Enable printing options for CPU type Dinah Baum
@ 2023-05-11 12:16 ` Peter Maydell
2023-05-11 13:51 ` Markus Armbruster
2023-05-26 14:28 ` Igor Mammedov
4 siblings, 1 reply; 11+ messages in thread
From: Peter Maydell @ 2023-05-11 12:16 UTC (permalink / raw)
To: Dinah Baum; +Cc: qemu-devel, Markus Armbruster
Markus, I think you said you would review this patchset?
thanks
-- PMM
On Tue, 4 Apr 2023 at 02:22, 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
>
> Changes since v2: Rebase
>
> 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] 11+ messages in thread* Re: [RESEND PATCH v2 0/3] Enable -cpu <cpu>,help
2023-04-04 1:19 [RESEND PATCH v2 0/3] Enable -cpu <cpu>,help Dinah Baum
` (3 preceding siblings ...)
2023-05-11 12:16 ` [RESEND PATCH v2 0/3] Enable -cpu <cpu>,help Peter Maydell
@ 2023-05-26 14:28 ` Igor Mammedov
2023-05-26 19:47 ` Peter Maydell
4 siblings, 1 reply; 11+ messages in thread
From: Igor Mammedov @ 2023-05-26 14:28 UTC (permalink / raw)
To: Dinah Baum; +Cc: qemu-devel
On Mon, 3 Apr 2023 21:19:53 -0400
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
what's wrong with 'device,help' if it's used for cpu devices?
>
> Changes since v2: Rebase
>
> 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
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [RESEND PATCH v2 0/3] Enable -cpu <cpu>,help
2023-05-26 14:28 ` Igor Mammedov
@ 2023-05-26 19:47 ` Peter Maydell
0 siblings, 0 replies; 11+ messages in thread
From: Peter Maydell @ 2023-05-26 19:47 UTC (permalink / raw)
To: Igor Mammedov; +Cc: Dinah Baum, qemu-devel
On Fri, 26 May 2023 at 15:28, Igor Mammedov <imammedo@redhat.com> wrote:
>
> On Mon, 3 Apr 2023 21:19:53 -0400
> 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
>
> what's wrong with 'device,help' if it's used for cpu devices?
Nothing, but almost no creation/configuration of CPUs
is done with -device. -cpu is by far the more usual way,
so '-cpu foo,help' should work...
-- PMM
^ permalink raw reply [flat|nested] 11+ messages in thread