diff mbox series

[RFC,PATCH-for-9.1,21/21] qapi: Make @query-cpu-definitions target-agnostic

Message ID 20240315130910.15750-22-philmd@linaro.org
State New
Headers show
Series qapi: Make @query-cpu-definitions command target-agnostic | expand

Commit Message

Philippe Mathieu-Daudé March 15, 2024, 1:09 p.m. UTC 
All targets use the generic_query_cpu_definitions() method,
which is not target-specific. Make the command target agnostic
by moving it to machine.json. Rename generic_query_cpu_definitions
as qmp_query_cpu_definitions.

This is an introspection change for the target that were not
implementing qmp_query_cpu_definitions(): now query-cpu-definitions
returns an their CPUs list.

Example with SH4 before:

  { "execute": "query-cpu-definitions" }

  { "error": {"class": "CommandNotFound", "desc": "The command query-cpu-definitions has not been found"} }

and after:

  { "execute": "query-cpu-definitions" }

  { "return": [
          {"name": "sh7751r", "typename": "sh7751r-superh-cpu", "static": false, "deprecated": false},
          {"name": "sh7750r", "typename": "sh7750r-superh-cpu", "static": false, "deprecated": false},
          {"name": "sh7785", "typename": "sh7785-superh-cpu", "static": false, "deprecated": false}
      ]
  }

However this allows heterogeneous emulation to return a correct list.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
Well, not all target got converted, I left the s390x one for later :)
---
 MAINTAINERS                           |  1 -
 qapi/machine-target.json              | 18 ------------------
 qapi/machine.json                     | 11 +++++++++++
 include/qapi/commands-target-compat.h | 14 --------------
 system/cpu-qmp-cmds.c                 |  4 ++--
 target/arm/arm-qmp-cmds.c             |  6 ------
 target/i386/cpu.c                     |  6 ------
 target/loongarch/loongarch-qmp-cmds.c |  6 ------
 target/mips/sysemu/mips-qmp-cmds.c    |  9 ---------
 target/ppc/ppc-qmp-cmds.c             |  7 -------
 target/riscv/riscv-qmp-cmds.c         |  6 ------
 11 files changed, 13 insertions(+), 75 deletions(-)
 delete mode 100644 include/qapi/commands-target-compat.h

Comments

Markus Armbruster March 26, 2024, 1:32 p.m. UTC | #1 
Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> All targets use the generic_query_cpu_definitions() method,
> which is not target-specific. Make the command target agnostic
> by moving it to machine.json. Rename generic_query_cpu_definitions
> as qmp_query_cpu_definitions.
>
> This is an introspection change for the target that were not
> implementing qmp_query_cpu_definitions(): now query-cpu-definitions
> returns an their CPUs list.

Do you mean "returns their CPUs list"?

>
> Example with SH4 before:
>
>   { "execute": "query-cpu-definitions" }
>
>   { "error": {"class": "CommandNotFound", "desc": "The command query-cpu-definitions has not been found"} }
>
> and after:
>
>   { "execute": "query-cpu-definitions" }
>
>   { "return": [
>           {"name": "sh7751r", "typename": "sh7751r-superh-cpu", "static": false, "deprecated": false},
>           {"name": "sh7750r", "typename": "sh7750r-superh-cpu", "static": false, "deprecated": false},
>           {"name": "sh7785", "typename": "sh7785-superh-cpu", "static": false, "deprecated": false}
>       ]
>   }

Does the result make sense?  What do the callbacks add for the targets
that define them, and why do the other targets don't need them?

Conversion steps:

0. Create a generic version of the command, with optional callbacks
   [PATCH 14]

1. Turn the target-specific versions of the command into thin wrappers
   around the generic version one by one, supplying callbacks to
   preserve behavior [PATCH 15-20]

2. Peel off the wrappers [this patch]

3. Enable the command for all the other targets [also this patch]

Correct?

Split this patch?  Question, not a demand.

I think the commit messages of the step 1 patches should explain the
target-specific behavior, i.e. the difference between their callbacks
and the default behavior.

Obvious for cpu_list_compare(): sorted vs. unsorted.

Not obvious for add_definition().

> However this allows heterogeneous emulation to return a correct list.

Cryptic.  It made me go back to PATCH 14, where I discovered the loop
over the arch bits.

> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> Well, not all target got converted, I left the s390x one for later :)

Haha!
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 39d7c14d98..71f446311b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1895,7 +1895,6 @@  F: qapi/machine-target.json
 F: include/hw/boards.h
 F: include/hw/core/cpu.h
 F: include/hw/cpu/cluster.h
-F: include/qapi/commands-target-compat.h
 F: include/sysemu/numa.h
 F: tests/unit/test-smp-parse.c
 T: git https://gitlab.com/ehabkost/qemu.git machine-next
diff --git a/qapi/machine-target.json b/qapi/machine-target.json
index 7480921d33..2065972d8a 100644
--- a/qapi/machine-target.json
+++ b/qapi/machine-target.json
@@ -212,24 +212,6 @@ 
                    'TARGET_LOONGARCH64',
                    'TARGET_RISCV' ] } }
 
-##
-# @query-cpu-definitions:
-#
-# Return a list of supported virtual CPU definitions
-#
-# Returns: a list of CpuDefinitionInfo
-#
-# Since: 1.2
-##
-{ 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'],
-  'if': { 'any': [ 'TARGET_PPC',
-                   'TARGET_ARM',
-                   'TARGET_I386',
-                   'TARGET_S390X',
-                   'TARGET_MIPS',
-                   'TARGET_LOONGARCH64',
-                   'TARGET_RISCV' ] } }
-
 ##
 # @CpuS390Polarization:
 #
diff --git a/qapi/machine.json b/qapi/machine.json
index 9c44b8fa82..987c64f8e7 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -73,6 +73,17 @@ 
             'deprecated' : 'bool' }
 }
 
+##
+# @query-cpu-definitions:
+#
+# Return a list of supported virtual CPU definitions
+#
+# Returns: a list of CpuDefinitionInfo
+#
+# Since: 1.2
+##
+{ 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'] }
+
 ##
 # @CpuModelInfo:
 #
diff --git a/include/qapi/commands-target-compat.h b/include/qapi/commands-target-compat.h
deleted file mode 100644
index 86d45d8fcc..0000000000
--- a/include/qapi/commands-target-compat.h
+++ /dev/null
@@ -1,14 +0,0 @@ 
-/*
- * QAPI helpers for target specific QMP commands
- *
- * SPDX-FileCopyrightText: 2024 Linaro Ltd.
- * SPDX-License-Identifier: GPL-2.0-or-later
- */
-#ifndef QAPI_COMPAT_TARGET_H
-#define QAPI_COMPAT_TARGET_H
-
-#include "qapi/qapi-types-machine.h"
-
-CpuDefinitionInfoList *generic_query_cpu_definitions(Error **errp);
-
-#endif
diff --git a/system/cpu-qmp-cmds.c b/system/cpu-qmp-cmds.c
index daeb131159..049e8b9d35 100644
--- a/system/cpu-qmp-cmds.c
+++ b/system/cpu-qmp-cmds.c
@@ -7,7 +7,7 @@ 
 
 #include "qemu/osdep.h"
 #include "qom/object.h"
-#include "qapi/commands-target-compat.h"
+#include "qapi/qapi-commands-machine.h"
 #include "sysemu/arch_init.h"
 #include "hw/core/cpu.h"
 #include "hw/core/sysemu-cpu-ops.h"
@@ -53,7 +53,7 @@  static void arch_add_cpu_definitions(CpuDefinitionInfoList **cpu_list,
     }
 }
 
-CpuDefinitionInfoList *generic_query_cpu_definitions(Error **errp)
+CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
 {
     CpuDefinitionInfoList *cpu_list = NULL;
 
diff --git a/target/arm/arm-qmp-cmds.c b/target/arm/arm-qmp-cmds.c
index c5091e64ec..ac8d890bc0 100644
--- a/target/arm/arm-qmp-cmds.c
+++ b/target/arm/arm-qmp-cmds.c
@@ -28,7 +28,6 @@ 
 #include "qapi/qobject-input-visitor.h"
 #include "qapi/qapi-commands-machine-target.h"
 #include "qapi/qapi-commands-misc-target.h"
-#include "qapi/commands-target-compat.h"
 #include "qapi/qmp/qdict.h"
 #include "qom/qom-qobject.h"
 
@@ -220,8 +219,3 @@  CpuModelExpansionInfo *qmp_query_cpu_model_expansion(CpuModelExpansionType type,
 
     return expansion_info;
 }
-
-CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
-{
-    return generic_query_cpu_definitions(errp);
-}
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index e5dbd307d8..f26adefd37 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -38,7 +38,6 @@ 
 #ifndef CONFIG_USER_ONLY
 #include "sysemu/reset.h"
 #include "qapi/qapi-commands-machine-target.h"
-#include "qapi/commands-target-compat.h"
 #include "exec/address-spaces.h"
 #include "hw/boards.h"
 #include "hw/i386/sgx-epc.h"
@@ -5666,11 +5665,6 @@  static void x86_cpu_definition_entry(gpointer data, gpointer user_data)
     QAPI_LIST_PREPEND(*cpu_list, info);
 }
 
-CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
-{
-    return generic_query_cpu_definitions(errp);
-}
-
 #endif /* !CONFIG_USER_ONLY */
 
 uint64_t x86_cpu_get_supported_feature_word(FeatureWord w,
diff --git a/target/loongarch/loongarch-qmp-cmds.c b/target/loongarch/loongarch-qmp-cmds.c
index ef5aedc1cd..aaed7bbf93 100644
--- a/target/loongarch/loongarch-qmp-cmds.c
+++ b/target/loongarch/loongarch-qmp-cmds.c
@@ -9,17 +9,11 @@ 
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qapi/qapi-commands-machine-target.h"
-#include "qapi/commands-target-compat.h"
 #include "cpu.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qobject-input-visitor.h"
 #include "qom/qom-qobject.h"
 
-CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
-{
-    return generic_query_cpu_definitions(errp);
-}
-
 static const char *cpu_model_advertised_features[] = {
     "lsx", "lasx", NULL
 };
diff --git a/target/mips/sysemu/mips-qmp-cmds.c b/target/mips/sysemu/mips-qmp-cmds.c
index 329db3a028..c3c775acc6 100644
--- a/target/mips/sysemu/mips-qmp-cmds.c
+++ b/target/mips/sysemu/mips-qmp-cmds.c
@@ -5,12 +5,3 @@ 
  *
  * SPDX-License-Identifier: LGPL-2.1-or-later
  */
-
-#include "qemu/osdep.h"
-#include "qapi/qapi-commands-machine-target.h"
-#include "qapi/commands-target-compat.h"
-
-CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
-{
-    return generic_query_cpu_definitions(errp);
-}
diff --git a/target/ppc/ppc-qmp-cmds.c b/target/ppc/ppc-qmp-cmds.c
index 267dd84718..f85c4560f9 100644
--- a/target/ppc/ppc-qmp-cmds.c
+++ b/target/ppc/ppc-qmp-cmds.c
@@ -28,8 +28,6 @@ 
 #include "qemu/ctype.h"
 #include "monitor/hmp-target.h"
 #include "monitor/hmp.h"
-#include "qapi/qapi-commands-machine-target.h"
-#include "qapi/commands-target-compat.h"
 #include "cpu-models.h"
 #include "cpu-qom.h"
 
@@ -195,8 +193,3 @@  void ppc_add_alias_definitions(CpuDefinitionInfoList **cpu_list)
         QAPI_LIST_PREPEND(*cpu_list, info);
     }
 }
-
-CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
-{
-    return generic_query_cpu_definitions(errp);
-}
diff --git a/target/riscv/riscv-qmp-cmds.c b/target/riscv/riscv-qmp-cmds.c
index 45adc90d3b..170d720b24 100644
--- a/target/riscv/riscv-qmp-cmds.c
+++ b/target/riscv/riscv-qmp-cmds.c
@@ -26,7 +26,6 @@ 
 
 #include "qapi/error.h"
 #include "qapi/qapi-commands-machine-target.h"
-#include "qapi/commands-target-compat.h"
 #include "qapi/qmp/qbool.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qobject-input-visitor.h"
@@ -54,11 +53,6 @@  void riscv_cpu_add_definition(gpointer data, gpointer user_data)
     QAPI_LIST_PREPEND(*cpu_list, info);
 }
 
-CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
-{
-    return generic_query_cpu_definitions(errp);
-}
-
 static void riscv_check_if_cpu_available(RISCVCPU *cpu, Error **errp)
 {
     if (!riscv_cpu_accelerator_compatible(cpu)) {