diff mbox series

mips: implement qmp query-cpu-definitions command

Message ID 154937205518.29984.9188603364499998604.stgit@pasha-VirtualBox (mailing list archive)
State New, archived
Headers show
Series mips: implement qmp query-cpu-definitions command | expand

Commit Message

Pavel Dovgalyuk Feb. 5, 2019, 1:07 p.m. UTC
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>
---
 monitor.c            |    2 +-
 target/mips/helper.c |   33 +++++++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 1 deletion(-)

Comments

Pavel Dovgalyuk Feb. 11, 2019, 5:34 a.m. UTC | #1
Ping.


Pavel Dovgalyuk

> -----Original Message-----
> From: Pavel Dovgalyuk [mailto:Pavel.Dovgaluk@ispras.ru]
> Sent: Tuesday, February 05, 2019 4:08 PM
> To: qemu-devel@nongnu.org
> Cc: pavel.dovgaluk@ispras.ru; arikalo@wavecomp.com; mdroth@linux.vnet.ibm.com;
> armbru@redhat.com; dovgaluk@ispras.ru; natalia.fursova@ispras.ru; eblake@redhat.com;
> aurelien@aurel32.net
> Subject: [PATCH] mips: implement qmp query-cpu-definitions command
> 
> 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>
> ---
>  monitor.c            |    2 +-
>  target/mips/helper.c |   33 +++++++++++++++++++++++++++++++++
>  2 files changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/monitor.c b/monitor.c
> index c09fa63940..25d3b141ad 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -1165,7 +1165,7 @@ static void qmp_unregister_commands_hack(void)
>      qmp_unregister_command(&qmp_commands, "query-cpu-model-comparison");
>  #endif
>  #if !defined(TARGET_PPC) && !defined(TARGET_ARM) && !defined(TARGET_I386) \
> -    && !defined(TARGET_S390X)
> +    && !defined(TARGET_S390X) && !defined(TARGET_MIPS)
>      qmp_unregister_command(&qmp_commands, "query-cpu-definitions");
>  #endif
>  }
> diff --git a/target/mips/helper.c b/target/mips/helper.c
> index 8988452dbd..c84d056c09 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 "sysemu/arch_init.h"
> 
>  enum {
>      TLBRET_XI = -6,
> @@ -1472,3 +1473,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 *arch_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;
> +}
Philippe Mathieu-Daudé Feb. 16, 2019, 6:34 p.m. UTC | #2
Hi Pavel,

On 2/11/19 6:34 AM, Pavel Dovgalyuk wrote:
> Ping.

You forgot to Cc Aleksandar, to get his MIPS maintainer Ack-by:

./scripts/get_maintainer.pl -f target/mips/helper.c
Aleksandar Markovic <amarkovic@wavecomp.com> (maintainer:MIPS)

> 
> Pavel Dovgalyuk
> 
>> -----Original Message-----
>> From: Pavel Dovgalyuk [mailto:Pavel.Dovgaluk@ispras.ru]
>> Sent: Tuesday, February 05, 2019 4:08 PM
>> To: qemu-devel@nongnu.org
>> Cc: pavel.dovgaluk@ispras.ru; arikalo@wavecomp.com; mdroth@linux.vnet.ibm.com;
>> armbru@redhat.com; dovgaluk@ispras.ru; natalia.fursova@ispras.ru; eblake@redhat.com;
>> aurelien@aurel32.net
>> Subject: [PATCH] mips: implement qmp query-cpu-definitions command
>>
>> This patch enables QMP-based querying of the available CPU types for MIPS
>> and MIPS64 platforms.

Your patch is a simple copy of the ARM code, so:

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Also:

Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>

However this clashes with Marc-André's "qapi: make query-cpu-definitions
depend on specific targets" posted here by Markus:
https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg03831.html

I believe your patch will go thru the QMP tree, so you might want to
rebase on top of the series Markus sent; or see if Markus is OK to do
the manual cleanup when applying.

Regards,

Phil.

>>
>> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
>> ---
>>  monitor.c            |    2 +-
>>  target/mips/helper.c |   33 +++++++++++++++++++++++++++++++++
>>  2 files changed, 34 insertions(+), 1 deletion(-)
>>
>> diff --git a/monitor.c b/monitor.c
>> index c09fa63940..25d3b141ad 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -1165,7 +1165,7 @@ static void qmp_unregister_commands_hack(void)
>>      qmp_unregister_command(&qmp_commands, "query-cpu-model-comparison");
>>  #endif
>>  #if !defined(TARGET_PPC) && !defined(TARGET_ARM) && !defined(TARGET_I386) \
>> -    && !defined(TARGET_S390X)
>> +    && !defined(TARGET_S390X) && !defined(TARGET_MIPS)
>>      qmp_unregister_command(&qmp_commands, "query-cpu-definitions");
>>  #endif
>>  }
>> diff --git a/target/mips/helper.c b/target/mips/helper.c
>> index 8988452dbd..c84d056c09 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 "sysemu/arch_init.h"
>>
>>  enum {
>>      TLBRET_XI = -6,
>> @@ -1472,3 +1473,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 *arch_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;
>> +}
> 
> 
>
Aleksandar Markovic Feb. 18, 2019, 6:47 a.m. UTC | #3
> From: Pavel Dovgalyuk [mailto:Pavel.Dovgaluk@ispras.ru]
> Sent: Tuesday, February 05, 2019 4:08 PM
> To: qemu-devel@nongnu.org
> Cc: pavel.dovgaluk@ispras.ru; arikalo@wavecomp.com; mdroth@linux.vnet.ibm.com;
> armbru@redhat.com; dovgaluk@ispras.ru; natalia.fursova@ispras.ru; eblake@redhat.com;
> aurelien@aurel32.net
> Subject: [PATCH] mips: implement qmp query-cpu-definitions command
>
> 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>
> ---
>  monitor.c            |    2 +-
>  target/mips/helper.c |   33 +++++++++++++++++++++++++++++++++
>  2 files changed, 34 insertions(+), 1 deletion(-)
>

Hello, Pavel,

Thanks for involving in this area!

I have just a couple of question:

1) What are the effects of these two patches on the end user?

2) What is the context of these patches? Do you intend to send more related patches in the future? Are these patches preconditions for some other not yet implemented features?

3) Why is only target MIPS included? Do other targets need similar improvements?

Thanks,
Aleksandar


> diff --git a/monitor.c b/monitor.c
> index c09fa63940..25d3b141ad 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -1165,7 +1165,7 @@ static void qmp_unregister_commands_hack(void)
>      qmp_unregister_command(&qmp_commands, "query-cpu-model-comparison");
>  #endif
>  #if !defined(TARGET_PPC) && !defined(TARGET_ARM) && !defined(TARGET_I386) \
> -    && !defined(TARGET_S390X)
> +    && !defined(TARGET_S390X) && !defined(TARGET_MIPS)
>      qmp_unregister_command(&qmp_commands, "query-cpu-definitions");
>  #endif
>  }
> diff --git a/target/mips/helper.c b/target/mips/helper.c
> index 8988452dbd..c84d056c09 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 "sysemu/arch_init.h"
>
>  enum {
>      TLBRET_XI = -6,
> @@ -1472,3 +1473,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 *arch_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;
> +}
Pavel Dovgalyuk Feb. 18, 2019, 7:01 a.m. UTC | #4
> From: Aleksandar Markovic [mailto:amarkovic@wavecomp.com]
> > From: Pavel Dovgalyuk [mailto: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>
> > ---
> >  monitor.c            |    2 +-
> >  target/mips/helper.c |   33 +++++++++++++++++++++++++++++++++
> >  2 files changed, 34 insertions(+), 1 deletion(-)
> >
> 
> Hello, Pavel,
> 
> Thanks for involving in this area!
> 
> I have just a couple of question:
> 
> 1) What are the effects of these two patches on the end user?

This patch make qmp query-cpu-definitions available for the MIPS users.
This command allows requesting possible CPU models with QMP.

> 2) What is the context of these patches? Do you intend to send more related patches in the
> future? Are these patches preconditions for some other not yet implemented features?

Not yet.
We are developing GUI for virtual machine management and debugging
with record-replay feature: https://github.com/ispras/qemu-gui
Therefore we need to request possible CPU and hardware options.

> 3) Why is only target MIPS included? Do other targets need similar improvements?

We use MIPS in our projects. Some other targets need similar improvements, but we do
not focus on them right now.

Pavel Dovgalyuk

> 
> Thanks,
> Aleksandar
> 
> 
> > diff --git a/monitor.c b/monitor.c
> > index c09fa63940..25d3b141ad 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -1165,7 +1165,7 @@ static void qmp_unregister_commands_hack(void)
> >      qmp_unregister_command(&qmp_commands, "query-cpu-model-comparison");
> >  #endif
> >  #if !defined(TARGET_PPC) && !defined(TARGET_ARM) && !defined(TARGET_I386) \
> > -    && !defined(TARGET_S390X)
> > +    && !defined(TARGET_S390X) && !defined(TARGET_MIPS)
> >      qmp_unregister_command(&qmp_commands, "query-cpu-definitions");
> >  #endif
> >  }
> > diff --git a/target/mips/helper.c b/target/mips/helper.c
> > index 8988452dbd..c84d056c09 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 "sysemu/arch_init.h"
> >
> >  enum {
> >      TLBRET_XI = -6,
> > @@ -1472,3 +1473,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 *arch_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;
> > +}
> 
>
Markus Armbruster Feb. 19, 2019, 7:28 a.m. UTC | #5
Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> Hi Pavel,
>
> On 2/11/19 6:34 AM, Pavel Dovgalyuk wrote:
>> Ping.
>
> You forgot to Cc Aleksandar, to get his MIPS maintainer Ack-by:
>
> ./scripts/get_maintainer.pl -f target/mips/helper.c
> Aleksandar Markovic <amarkovic@wavecomp.com> (maintainer:MIPS)
>
>> 
>> Pavel Dovgalyuk
>> 
>>> -----Original Message-----
>>> From: Pavel Dovgalyuk [mailto:Pavel.Dovgaluk@ispras.ru]
>>> Sent: Tuesday, February 05, 2019 4:08 PM
>>> To: qemu-devel@nongnu.org
>>> Cc: pavel.dovgaluk@ispras.ru; arikalo@wavecomp.com; mdroth@linux.vnet.ibm.com;
>>> armbru@redhat.com; dovgaluk@ispras.ru; natalia.fursova@ispras.ru; eblake@redhat.com;
>>> aurelien@aurel32.net
>>> Subject: [PATCH] mips: implement qmp query-cpu-definitions command
>>>
>>> This patch enables QMP-based querying of the available CPU types for MIPS
>>> and MIPS64 platforms.
>
> Your patch is a simple copy of the ARM code, so:
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
> Also:
>
> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
> However this clashes with Marc-André's "qapi: make query-cpu-definitions
> depend on specific targets" posted here by Markus:
> https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg03831.html

This is now in master, merge commit a0430dd8abb.

> I believe your patch will go thru the QMP tree, so you might want to
> rebase on top of the series Markus sent; or see if Markus is OK to do
> the manual cleanup when applying.

Please rebase.  Let me know if you need help.
Aleksandar Markovic Feb. 19, 2019, 7:19 p.m. UTC | #6
> From: Markus Armbruster <armbru@redhat.com>
> Subject: Re: [Qemu-devel] [PATCH] mips: implement qmp query-cpu-definitions command

> Please rebase.  Let me know if you need help.

Hi, Markus.

Pavel was probably busy today, so I took the liberty to rebase the patch,
and here is the v2:

https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg05158.html

I would really appreciate if you, or somebody else knowledgeable in QAPI,
take a look. I did only build tests.

Regards,
Aleksandar
Pavel Dovgalyuk Feb. 20, 2019, 5:20 a.m. UTC | #7
> From: Aleksandar Markovic [mailto:amarkovic@wavecomp.com]
> > From: Markus Armbruster <armbru@redhat.com>
> > Subject: Re: [Qemu-devel] [PATCH] mips: implement qmp query-cpu-definitions command
> 
> > Please rebase.  Let me know if you need help.
> 
> Hi, Markus.
> 
> Pavel was probably busy today, so I took the liberty to rebase the patch,
> and here is the v2:
> 
> https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg05158.html

Thank you!

Pavel Dovgalyuk
diff mbox series

Patch

diff --git a/monitor.c b/monitor.c
index c09fa63940..25d3b141ad 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1165,7 +1165,7 @@  static void qmp_unregister_commands_hack(void)
     qmp_unregister_command(&qmp_commands, "query-cpu-model-comparison");
 #endif
 #if !defined(TARGET_PPC) && !defined(TARGET_ARM) && !defined(TARGET_I386) \
-    && !defined(TARGET_S390X)
+    && !defined(TARGET_S390X) && !defined(TARGET_MIPS)
     qmp_unregister_command(&qmp_commands, "query-cpu-definitions");
 #endif
 }
diff --git a/target/mips/helper.c b/target/mips/helper.c
index 8988452dbd..c84d056c09 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 "sysemu/arch_init.h"
 
 enum {
     TLBRET_XI = -6,
@@ -1472,3 +1473,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 *arch_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;
+}