diff mbox series

[v2,1/3] qapi/machine-target: refactor machine-target

Message ID 20230404011956.90375-2-dinahbaum123@gmail.com
State New
Headers show
Series Enable -cpu <cpu>,help | expand

Commit Message

Dinah B April 4, 2023, 1:19 a.m. UTC 
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

Comments

Markus Armbruster May 11, 2023, 2:38 p.m. UTC | #1 
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.

[...]
diff mbox series

Patch

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',