Re: [PATCH v2 3/3] cpu, qdict, vl: Enable printing options for CPU type
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 (supporter:Machine core)
Marcel Apfelbaum (supporter:Machine core)
"Philippe Mathieu-Daudé" (reviewer:Machine core)
Yanan Wang (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 (maintainer:SPARC TCG CPUs)
Artyom Tarasenko (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 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
> ---
> 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
> +++
[PATCH v2 3/3] cpu, qdict, vl: Enable printing options for CPU type
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
---
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 cpuselect CPU ('-cpu help' for list)\n", QEMU_ARCH_ALL)
+"-cpu cpuselect 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 = {
},
[PATCH v2 3/3] cpu, qdict, vl: Enable printing options for CPU type
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
---
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 cpuselect CPU ('-cpu help' for list)\n", QEMU_ARCH_ALL)
+"-cpu cpuselect 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 = {
