Patchwork [3/3] S390: Enable -cpu help and QMP query-cpu-definitions

login
register
mail settings
Submitter Jens Freimann
Date Dec. 14, 2012, 4:46 p.m.
Message ID <1355503606-54131-4-git-send-email-jfrei@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/206524/
State New
Headers show

Comments

Jens Freimann - Dec. 14, 2012, 4:46 p.m.
From: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>

This enables qemu -cpu help to return a list of supported CPU models
on s390 and also to query for cpu definitions in the monitor.
Initially only cpu model = host is returned. This needs to be reworked
into a full-fledged CPU model handling later on.
This change is needed to allow libvirt exploiters (like OpenStack)
to specify a CPU model.

Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
v1 -> v2:
* only print output with CONFIG_KVM

---
 hw/s390-virtio.c   |  6 +++++-
 target-s390x/cpu.c | 23 +++++++++++++++++++++++
 target-s390x/cpu.h |  3 +++
 3 files changed, 31 insertions(+), 1 deletion(-)
Andreas Färber - Dec. 16, 2012, 3:42 p.m.
Am 14.12.2012 17:46, schrieb Jens Freimann:
> From: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
> 
> This enables qemu -cpu help to return a list of supported CPU models
> on s390 and also to query for cpu definitions in the monitor.
> Initially only cpu model = host is returned. This needs to be reworked
> into a full-fledged CPU model handling later on.
> This change is needed to allow libvirt exploiters (like OpenStack)
> to specify a CPU model.
> 
> Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>

I guess we can live with this solution for now,

Reviewed-by: Andreas Färber <afaerber@suse.de>

Can you follow-up with a strcmp(cpu_model, "host") check for
!kvm_enabled() in cpu_s390x_init() please?

Thanks,
Andreas
Alexander Graf - Dec. 17, 2012, 2:47 p.m.
On 14.12.2012, at 17:46, Jens Freimann wrote:

> From: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
> 
> This enables qemu -cpu help to return a list of supported CPU models
> on s390 and also to query for cpu definitions in the monitor.
> Initially only cpu model = host is returned. This needs to be reworked
> into a full-fledged CPU model handling later on.
> This change is needed to allow libvirt exploiters (like OpenStack)
> to specify a CPU model.
> 
> Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
> v1 -> v2:
> * only print output with CONFIG_KVM
> 
> ---
> hw/s390-virtio.c   |  6 +++++-
> target-s390x/cpu.c | 23 +++++++++++++++++++++++
> target-s390x/cpu.h |  3 +++
> 3 files changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
> index a350430..60fde26 100644
> --- a/hw/s390-virtio.c
> +++ b/hw/s390-virtio.c
> @@ -2,6 +2,7 @@
>  * QEMU S390 virtio target
>  *
>  * Copyright (c) 2009 Alexander Graf <agraf@suse.de>
> + * Copyright IBM Corp 2012
>  *
>  * This library is free software; you can redistribute it and/or
>  * modify it under the terms of the GNU Lesser General Public
> @@ -13,7 +14,10 @@
>  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>  * Lesser General Public License for more details.
>  *
> - * You should have received a copy of the GNU Lesser General Public
> + * Contributions after 2012-10-29 are licensed under the terms of the
> + * GNU GPL, version 2 or (at your option) any later version.
> + *
> + * You should have received a copy of the GNU (Lesser) General Public
>  * License along with this library; if not, see <http://www.gnu.org/licenses/>.
>  */
> 
> diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> index 75d4036..adca789 100644
> --- a/target-s390x/cpu.c
> +++ b/target-s390x/cpu.c
> @@ -28,8 +28,31 @@
> #include "hw/hw.h"
> #include "qemu-common.h"
> #include "qemu-timer.h"
> +#include "arch_init.h"
> 
> 
> +/* generate CPU information for cpu -? */
> +void s390_cpu_list(FILE *f, fprintf_function cpu_fprintf)
> +{
> +#ifdef CONFIG_KVM
> +    (*cpu_fprintf)(f, "s390 %16s\n", "host");
> +#endif
> +}
> +

static const struct CpuDefinitionInfo cpu_info[] = {
#ifdef CONFIG_KVM
    {
        .name = "host",
    },
#endif
};

static const struct CPUDefinitionInfoList cpu_entry = {
    value = cpu_info,
};

> +CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
> +{
> +    CpuDefinitionInfoList *entry;
> +    CpuDefinitionInfo *info;
> +
> +    info = g_malloc0(sizeof(*info));
> +    info->name = g_strdup("host");
> +
> +    entry = g_malloc0(sizeof(*entry));
> +    entry->value = info;

return &entry;

(completely untested, don't you think the above would work? The code as is looks quite leaky.)

> +
> +    return entry;
> +}
> +
> /* CPUClass::reset() */
> static void s390_cpu_reset(CPUState *s)
> {
> diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
> index 0f9a1f7..3513976 100644
> --- a/target-s390x/cpu.h
> +++ b/target-s390x/cpu.h
> @@ -350,6 +350,9 @@ static inline void cpu_set_tls(CPUS390XState *env, target_ulong newtls)
> #define cpu_gen_code cpu_s390x_gen_code
> #define cpu_signal_handler cpu_s390x_signal_handler
> 
> +void s390_cpu_list(FILE *f, fprintf_function cpu_fprintf);
> +#define cpu_list s390_cpu_list
> +
> #include "exec-all.h"
> 
> #ifdef CONFIG_USER_ONLY
> -- 
> 1.7.12.4
>
Andreas Färber - Dec. 17, 2012, 5:32 p.m.
Am 17.12.2012 15:47, schrieb Alexander Graf:
> 
> On 14.12.2012, at 17:46, Jens Freimann wrote:
> 
>> From: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
>>
>> This enables qemu -cpu help to return a list of supported CPU models
>> on s390 and also to query for cpu definitions in the monitor.
>> Initially only cpu model = host is returned. This needs to be reworked
>> into a full-fledged CPU model handling later on.
>> This change is needed to allow libvirt exploiters (like OpenStack)
>> to specify a CPU model.
>>
>> Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
>> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
>> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> ---
>> v1 -> v2:
>> * only print output with CONFIG_KVM
>>
>> ---
>> hw/s390-virtio.c   |  6 +++++-
>> target-s390x/cpu.c | 23 +++++++++++++++++++++++
>> target-s390x/cpu.h |  3 +++
>> 3 files changed, 31 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
>> index a350430..60fde26 100644
>> --- a/hw/s390-virtio.c
>> +++ b/hw/s390-virtio.c
>> @@ -2,6 +2,7 @@
>>  * QEMU S390 virtio target
>>  *
>>  * Copyright (c) 2009 Alexander Graf <agraf@suse.de>
>> + * Copyright IBM Corp 2012
>>  *
>>  * This library is free software; you can redistribute it and/or
>>  * modify it under the terms of the GNU Lesser General Public
>> @@ -13,7 +14,10 @@
>>  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>  * Lesser General Public License for more details.
>>  *
>> - * You should have received a copy of the GNU Lesser General Public
>> + * Contributions after 2012-10-29 are licensed under the terms of the
>> + * GNU GPL, version 2 or (at your option) any later version.
>> + *
>> + * You should have received a copy of the GNU (Lesser) General Public
>>  * License along with this library; if not, see <http://www.gnu.org/licenses/>.
>>  */
>>
>> diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
>> index 75d4036..adca789 100644
>> --- a/target-s390x/cpu.c
>> +++ b/target-s390x/cpu.c
>> @@ -28,8 +28,31 @@
>> #include "hw/hw.h"
>> #include "qemu-common.h"
>> #include "qemu-timer.h"
>> +#include "arch_init.h"
>>
>>
>> +/* generate CPU information for cpu -? */
>> +void s390_cpu_list(FILE *f, fprintf_function cpu_fprintf)
>> +{
>> +#ifdef CONFIG_KVM
>> +    (*cpu_fprintf)(f, "s390 %16s\n", "host");
>> +#endif
>> +}
>> +
> 
> static const struct CpuDefinitionInfo cpu_info[] = {
> #ifdef CONFIG_KVM
>     {
>         .name = "host",
>     },
> #endif
> };
> 
> static const struct CPUDefinitionInfoList cpu_entry = {
>     value = cpu_info,
> };
> 
>> +CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
>> +{
>> +    CpuDefinitionInfoList *entry;
>> +    CpuDefinitionInfo *info;
>> +
>> +    info = g_malloc0(sizeof(*info));
>> +    info->name = g_strdup("host");
>> +
>> +    entry = g_malloc0(sizeof(*entry));
>> +    entry->value = info;
> 
> return &entry;
> 
> (completely untested, don't you think the above would work? The code as is looks quite leaky.)

target-i386 does it the same way (well, not hardcoding "host"
obviously), so if there's leaks they need to be solved in generic code.

Mid-term we want to generate this list on the fly from CPU subclasses,
so I don't see the utility of starting a static model list for QMP only.
Given that subclasses are really easy to introduce, we might as well do
that. We could leave s390-cpu non-abstract as fallback for the non-host
cpu_models plus one host-s390-cpu for -cpu host; only thing to keep in
mind then is that the base class is not automatically filtered out, as
relied on for other targets.

Andreas

Patch

diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
index a350430..60fde26 100644
--- a/hw/s390-virtio.c
+++ b/hw/s390-virtio.c
@@ -2,6 +2,7 @@ 
  * QEMU S390 virtio target
  *
  * Copyright (c) 2009 Alexander Graf <agraf@suse.de>
+ * Copyright IBM Corp 2012
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -13,7 +14,10 @@ 
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
  * Lesser General Public License for more details.
  *
- * You should have received a copy of the GNU Lesser General Public
+ * Contributions after 2012-10-29 are licensed under the terms of the
+ * GNU GPL, version 2 or (at your option) any later version.
+ *
+ * You should have received a copy of the GNU (Lesser) General Public
  * License along with this library; if not, see <http://www.gnu.org/licenses/>.
  */
 
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 75d4036..adca789 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -28,8 +28,31 @@ 
 #include "hw/hw.h"
 #include "qemu-common.h"
 #include "qemu-timer.h"
+#include "arch_init.h"
 
 
+/* generate CPU information for cpu -? */
+void s390_cpu_list(FILE *f, fprintf_function cpu_fprintf)
+{
+#ifdef CONFIG_KVM
+    (*cpu_fprintf)(f, "s390 %16s\n", "host");
+#endif
+}
+
+CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
+{
+    CpuDefinitionInfoList *entry;
+    CpuDefinitionInfo *info;
+
+    info = g_malloc0(sizeof(*info));
+    info->name = g_strdup("host");
+
+    entry = g_malloc0(sizeof(*entry));
+    entry->value = info;
+
+    return entry;
+}
+
 /* CPUClass::reset() */
 static void s390_cpu_reset(CPUState *s)
 {
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 0f9a1f7..3513976 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -350,6 +350,9 @@  static inline void cpu_set_tls(CPUS390XState *env, target_ulong newtls)
 #define cpu_gen_code cpu_s390x_gen_code
 #define cpu_signal_handler cpu_s390x_signal_handler
 
+void s390_cpu_list(FILE *f, fprintf_function cpu_fprintf);
+#define cpu_list s390_cpu_list
+
 #include "exec-all.h"
 
 #ifdef CONFIG_USER_ONLY