mips: implement qmp query-cpu-definitions command

[Qemu-devel] [PATCH] mips: implement qmp query-cpu-definitions command

Posted by Pavel Dovgalyuk 5 years, 2 months ago

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;
+}

Re: [Qemu-devel] [PATCH] mips: implement qmp query-cpu-definitions command

Posted by Pavel Dovgalyuk 5 years, 2 months ago

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;
> +}

Re: [Qemu-devel] [PATCH] mips: implement qmp query-cpu-definitions command

Posted by Philippe Mathieu-Daudé 5 years, 2 months ago

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;
>> +}
> 
> 
>

Re: [Qemu-devel] [PATCH] mips: implement qmp query-cpu-definitions command

Posted by Aleksandar Markovic 5 years, 2 months ago

> 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;
> +}

Re: [Qemu-devel] [PATCH] mips: implement qmp query-cpu-definitions command

Posted by Pavel Dovgalyuk 5 years, 2 months ago

> 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;
> > +}
> 
>

Re: [Qemu-devel] [PATCH] mips: implement qmp query-cpu-definitions command

Posted by Markus Armbruster 5 years, 1 month ago

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.

Re: [Qemu-devel] [PATCH] mips: implement qmp query-cpu-definitions command

Posted by Aleksandar Markovic 5 years, 1 month ago

> 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

Re: [Qemu-devel] [PATCH] mips: implement qmp query-cpu-definitions command

Posted by Pavel Dovgalyuk 5 years, 1 month ago

> 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