qapi/target.json | 4 ++-- target/mips/helper.c | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-)
From: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
This patch enables QMP-based querying of the available CPU types for
MIPS and MIPS64 platforms.
Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
v1->v2:
- rebased to the latest QAPI code
qapi/target.json | 4 ++--
target/mips/helper.c | 33 +++++++++++++++++++++++++++++++++
2 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/qapi/target.json b/qapi/target.json
index da7b4be..1d4d54b 100644
--- a/qapi/target.json
+++ b/qapi/target.json
@@ -499,7 +499,7 @@
'static': 'bool',
'*unavailable-features': [ 'str' ],
'typename': 'str' },
- 'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_I386) || defined(TARGET_S390X)' }
+ 'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_I386) || defined(TARGET_S390X) || defined(TARGET_MIPS)' }
##
# @query-cpu-definitions:
@@ -511,4 +511,4 @@
# Since: 1.2.0
##
{ 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'],
- 'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_I386) || defined(TARGET_S390X)' }
+ 'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_I386) || defined(TARGET_S390X) || defined(TARGET_MIPS)' }
diff --git a/target/mips/helper.c b/target/mips/helper.c
index 944f094..c44cdca 100644
--- a/target/mips/helper.c
+++ b/target/mips/helper.c
@@ -24,6 +24,7 @@
#include "exec/cpu_ldst.h"
#include "exec/log.h"
#include "hw/mips/cpudevs.h"
+#include "qapi/qapi-commands-target.h"
enum {
TLBRET_XI = -6,
@@ -1470,3 +1471,35 @@ void QEMU_NORETURN do_raise_exception_err(CPUMIPSState *env,
cpu_loop_exit_restore(cs, pc);
}
+
+static void mips_cpu_add_definition(gpointer data, gpointer user_data)
+{
+ ObjectClass *oc = data;
+ CpuDefinitionInfoList **cpu_list = user_data;
+ CpuDefinitionInfoList *entry;
+ CpuDefinitionInfo *info;
+ const char *typename;
+
+ typename = object_class_get_name(oc);
+ info = g_malloc0(sizeof(*info));
+ info->name = g_strndup(typename,
+ strlen(typename) - strlen("-" TYPE_MIPS_CPU));
+ info->q_typename = g_strdup(typename);
+
+ entry = g_malloc0(sizeof(*entry));
+ entry->value = info;
+ entry->next = *cpu_list;
+ *cpu_list = entry;
+}
+
+CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
+{
+ CpuDefinitionInfoList *cpu_list = NULL;
+ GSList *list;
+
+ list = object_class_get_list(TYPE_MIPS_CPU, false);
+ g_slist_foreach(list, mips_cpu_add_definition, &cpu_list);
+ g_slist_free(list);
+
+ return cpu_list;
+}
--
2.7.4
On 2/19/19 8:15 PM, Aleksandar Markovic wrote: > From: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru> > > This patch enables QMP-based querying of the available CPU types for > MIPS and MIPS64 platforms. > > Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru> > Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com> > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com> Aleksandar, FYI I tested v1 and your v2 that way: $ mips64el-softmmu/qemu-system-mips64el -S \ -qmp tcp:localhost:4444,server,nowait $ telnet localhost 4444 {"QMP": {"version": {"qemu": {"micro": 50, "minor": 1, "major": 3}, "package": "v3.1.0-1924-gc7b700fafb"}, "capabilities": ["oob"]}} { "execute": "qmp_capabilities" } {"return": {}} { "execute": "query-cpu-definitions" } {"return": [{"name": "24KEc", "typename": "24KEc-mips64-cpu", "static": false}, {"name": "MIPS64R2-generic", "typename": "MIPS64R2-generic-mips64-cpu", "static": false}, {"name": "VR5432", "typename": "VR5432-mips64-cpu", "static": false}, {"name": "M14Kc", "typename": "M14Kc-mips64-cpu", "static": false}, {"name": "20Kc", "typename": "20Kc-mips64-cpu", "static": false}, {"name": "4Km", "typename": "4Km-mips64-cpu", "static": false}, {"name": "mips64dspr2", "typename": "mips64dspr2-mips64-cpu", "static": false}, {"name": "I6400", "typename": "I6400-mips64-cpu", "static": false}, {"name": "Loongson-2E", "typename": "Loongson-2E-mips64-cpu", "static": false}, {"name": "I6500", "typename": "I6500-mips64-cpu", "static": false}, {"name": "4KEcR1", "typename": "4KEcR1-mips64-cpu", "static": false}, {"name": "74Kf", "typename": "74Kf-mips64-cpu", "static": false}, {"name": "24Kf", "typename": "24Kf-mips64-cpu", "static": false}, {"name": "5KEc", "typename": "5KEc-mips64-cpu", "static": false}, {"name": "P5600", "typename": "P5600-mips64-cpu", "static": false}, {"name": "I7200", "typename": "I7200-mips64-cpu", "static": false}, {"name": "M14K", "typename": "M14K-mips64-cpu", "static": false}, {"name": "5KEf", "typename": "5KEf-mips64-cpu", "static": false}, {"name": "24Kc", "typename": "24Kc-mips64-cpu", "static": false}, {"name": "4KEm", "typename": "4KEm-mips64-cpu", "static": false}, {"name": "4Kc", "typename": "4Kc-mips64-cpu", "static": false}, {"name": "5Kc", "typename": "5Kc-mips64-cpu", "static": false}, {"name": "4KEmR1", "typename": "4KEmR1-mips64-cpu", "static": false}, {"name": "R4000", "typename": "R4000-mips64-cpu", "static": false}, {"name": "5Kf", "typename": "5Kf-mips64-cpu", "static": false}, {"name": "mips32r6-generic", "typename": "mips32r6-generic-mips64-cpu", "static": false}, {"name": "4KEc", "typename": "4KEc-mips64-cpu", "static": false}, {"name": "34Kf", "typename": "34Kf-mips64-cpu", "static": false}, {"name": "Loongson-2F", "typename": "Loongson-2F-mips64-cpu", "static": false}]} See: https://wiki.qemu.org/Documentation/QMP Regards, Phil. > --- > > v1->v2: > > - rebased to the latest QAPI code > > qapi/target.json | 4 ++-- > target/mips/helper.c | 33 +++++++++++++++++++++++++++++++++ > 2 files changed, 35 insertions(+), 2 deletions(-) > > diff --git a/qapi/target.json b/qapi/target.json > index da7b4be..1d4d54b 100644 > --- a/qapi/target.json > +++ b/qapi/target.json > @@ -499,7 +499,7 @@ > 'static': 'bool', > '*unavailable-features': [ 'str' ], > 'typename': 'str' }, > - 'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_I386) || defined(TARGET_S390X)' } > + 'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_I386) || defined(TARGET_S390X) || defined(TARGET_MIPS)' } > > ## > # @query-cpu-definitions: > @@ -511,4 +511,4 @@ > # Since: 1.2.0 > ## > { 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'], > - 'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_I386) || defined(TARGET_S390X)' } > + 'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_I386) || defined(TARGET_S390X) || defined(TARGET_MIPS)' } > diff --git a/target/mips/helper.c b/target/mips/helper.c > index 944f094..c44cdca 100644 > --- a/target/mips/helper.c > +++ b/target/mips/helper.c > @@ -24,6 +24,7 @@ > #include "exec/cpu_ldst.h" > #include "exec/log.h" > #include "hw/mips/cpudevs.h" > +#include "qapi/qapi-commands-target.h" > > enum { > TLBRET_XI = -6, > @@ -1470,3 +1471,35 @@ void QEMU_NORETURN do_raise_exception_err(CPUMIPSState *env, > > cpu_loop_exit_restore(cs, pc); > } > + > +static void mips_cpu_add_definition(gpointer data, gpointer user_data) > +{ > + ObjectClass *oc = data; > + CpuDefinitionInfoList **cpu_list = user_data; > + CpuDefinitionInfoList *entry; > + CpuDefinitionInfo *info; > + const char *typename; > + > + typename = object_class_get_name(oc); > + info = g_malloc0(sizeof(*info)); > + info->name = g_strndup(typename, > + strlen(typename) - strlen("-" TYPE_MIPS_CPU)); > + info->q_typename = g_strdup(typename); > + > + entry = g_malloc0(sizeof(*entry)); > + entry->value = info; > + entry->next = *cpu_list; > + *cpu_list = entry; > +} > + > +CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp) > +{ > + CpuDefinitionInfoList *cpu_list = NULL; > + GSList *list; > + > + list = object_class_get_list(TYPE_MIPS_CPU, false); > + g_slist_foreach(list, mips_cpu_add_definition, &cpu_list); > + g_slist_free(list); > + > + return cpu_list; > +} >
> From: Philippe Mathieu-Daudé <philmd@redhat.com> > Subject: Re: [PATCH v2] target/mips: implement QMP query-cpu-definitions command > > On 2/19/19 8:15 PM, Aleksandar Markovic wrote: > > From: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru> > > > > This patch enables QMP-based querying of the available CPU types for > > MIPS and MIPS64 platforms. > > > > Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru> > > Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com> > > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > > Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com> > > Aleksandar, FYI I tested v1 and your v2 that way: > > $ mips64el-softmmu/qemu-system-mips64el -S \ > -qmp tcp:localhost:4444,server,nowait > > $ telnet localhost 4444 > {"QMP": {"version": {"qemu": {"micro": 50, "minor": 1, "major": 3}, > "package": "v3.1.0-1924-gc7b700fafb"}, "capabilities": ["oob"]}} ... That's fabulous! Thanks, Philippe! Aleksandar
On 2/19/19 1:15 PM, Aleksandar Markovic wrote: > From: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru> > > This patch enables QMP-based querying of the available CPU types for > MIPS and MIPS64 platforms. > > Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru> > Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com> > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > +++ b/qapi/target.json > @@ -499,7 +499,7 @@ > 'static': 'bool', > '*unavailable-features': [ 'str' ], > 'typename': 'str' }, > - 'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_I386) || defined(TARGET_S390X)' } > + 'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_I386) || defined(TARGET_S390X) || defined(TARGET_MIPS)' } Hmm. Long line; however, the argument to 'if' is pasted literally to an #if directive, which would break if we added newlines in the middle. And we can't use literal newlines in the middle of a JSON string. About the only thing I could think of that might allow for more manageable line lengths would be permitting: 'if': [ 'defined(TARGET_PPC)', 'defined(TARGET_ARM)' ...] where the QAPI generator would in turn form the disjunction of supplying the || between each term when the 'if' is an array of strings. But that feels like a lot of effort for little gain compared to just living with the long lines. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Hi Eric, On 2/19/19 9:12 PM, Eric Blake wrote: > On 2/19/19 1:15 PM, Aleksandar Markovic wrote: >> From: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru> >> >> This patch enables QMP-based querying of the available CPU types for >> MIPS and MIPS64 platforms. >> >> Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru> >> Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com> >> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> --- > >> +++ b/qapi/target.json >> @@ -499,7 +499,7 @@ >> 'static': 'bool', >> '*unavailable-features': [ 'str' ], >> 'typename': 'str' }, >> - 'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_I386) || defined(TARGET_S390X)' } >> + 'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_I386) || defined(TARGET_S390X) || defined(TARGET_MIPS)' } > > Hmm. Long line; however, the argument to 'if' is pasted literally to an > #if directive, which would break if we added newlines in the middle. > And we can't use literal newlines in the middle of a JSON string. About > the only thing I could think of that might allow for more manageable > line lengths would be permitting: > > 'if': [ 'defined(TARGET_PPC)', > 'defined(TARGET_ARM)' ...] Can this be: 'if': [ 'defined(TARGET_PPC)', 'defined(TARGET_ARM)', ... ] ? > where the QAPI generator would in turn form the disjunction of supplying > the || between each term when the 'if' is an array of strings. But that > feels like a lot of effort for little gain compared to just living with > the long lines. Single line looks easier thus less bug prone. Multiline improves git diff/blame. The former looks safer.
Eric Blake <eblake@redhat.com> writes: > On 2/19/19 1:15 PM, Aleksandar Markovic wrote: >> From: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru> >> >> This patch enables QMP-based querying of the available CPU types for >> MIPS and MIPS64 platforms. >> >> Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru> >> Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com> >> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> --- > >> +++ b/qapi/target.json >> @@ -499,7 +499,7 @@ >> 'static': 'bool', >> '*unavailable-features': [ 'str' ], >> 'typename': 'str' }, >> - 'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_I386) || defined(TARGET_S390X)' } >> + 'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_I386) || defined(TARGET_S390X) || defined(TARGET_MIPS)' } > > Hmm. Long line; however, the argument to 'if' is pasted literally to an > #if directive, which would break if we added newlines in the middle. > And we can't use literal newlines in the middle of a JSON string. About > the only thing I could think of that might allow for more manageable > line lengths would be permitting: > > 'if': [ 'defined(TARGET_PPC)', > 'defined(TARGET_ARM)' ...] > > where the QAPI generator would in turn form the disjunction of supplying > the || between each term when the 'if' is an array of strings. But that > feels like a lot of effort for little gain compared to just living with > the long lines. I hate long lines as much as anyone, but we already burned the list syntax on conjunction: docs/devel/qapi-code-gen.txt: === Configuring the schema === The 'struct', 'enum', 'union', 'alternate', 'command' and 'event' top-level expressions can take an 'if' key. Its value must be a string or a list of strings. A string is shorthand for a list containing just that string. The code generated for the top-level expression will then be guarded by #if COND for each COND in the list. Example: a conditional struct { 'struct': 'IfStruct', 'data': { 'foo': 'int' }, 'if': ['defined(CONFIG_FOO)', 'defined(HAVE_BAR)'] } gets its generated code guarded like this: #if defined(CONFIG_FOO) #if defined(HAVE_BAR) ... generated code ... #endif /* defined(HAVE_BAR) */ #endif /* defined(CONFIG_FOO) */ The problem of long strings requiring long lines is not limited to 'if'. The common solution in programming languages is to concatenate adjacent string literals. A more user-friendly QAPI language would support something like that. By choosing JSON, a language designed "for the serialization of structured data" (RFC 4627, 7159, 8259), we opted against user-friendly. We then admitted our regrets halfheartedly by making our version of JSON incompatible enough to defeat tools like javascript-mode. Correcting our fundamental mistake now would be a lot of churn.
Aleksandar Markovic <aleksandar.markovic@rt-rk.com> writes: > From: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru> > > This patch enables QMP-based querying of the available CPU types for > MIPS and MIPS64 platforms. > > Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru> > Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com> > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com> Almost identical to the ARM code. Factoring out the common core doesn't seem worth the bother for just two copies. Perhaps if we grow more copies. Reviewed-by: Markus Armbruster <armbru@redhat.com>
> From: Markus Armbruster <armbru@redhat.com> > Subject: Re: [Qemu-devel] [PATCH v2] target/mips: implement QMP query-cpu-definitions command ... > Reviewed-by: Markus Armbruster <armbru@redhat.com> Unless there is somebody's objection, I am going to accept this patch into the next MIPS pull request, planned to be sent in next few days. Thanks, Aleksandar
Aleksandar Markovic <amarkovic@wavecomp.com> writes: >> From: Markus Armbruster <armbru@redhat.com> >> Subject: Re: [Qemu-devel] [PATCH v2] target/mips: implement QMP query-cpu-definitions command > > ... > >> Reviewed-by: Markus Armbruster <armbru@redhat.com> > > Unless there is somebody's objection, I am going to accept this patch into the next > MIPS pull request, planned to be sent in next few days. As far as I'm concerned: go right ahead.
> From: Markus Armbruster <armbru@redhat.com> > Subject: Re: [Qemu-devel] [PATCH v2] target/mips: implement QMP query-cpu-definitions command > > Aleksandar Markovic <amarkovic@wavecomp.com> writes: > > >> From: Markus Armbruster <armbru@redhat.com> > >> Subject: Re: [Qemu-devel] [PATCH v2] target/mips: implement QMP query-cpu-definitions command > > > > ... > > > >> Reviewed-by: Markus Armbruster <armbru@redhat.com> > > > > Unless there is somebody's objection, I am going to accept this patch into the next > > MIPS pull request, planned to be sent in next few days. > > As far as I'm concerned: go right ahead. This patch was selected to be in the MIPS queue, and was subsequently integrated in the main tree today. Thanks to all folks involved! Aleksandar
© 2016 - 2024 Red Hat, Inc.