* [Qemu-devel] [PATCH 3/3] S390: Enable -cpu help and QMP query-cpu-definitions
2012-12-12 13:08 [Qemu-devel] [PATCH 0/3] s390: ipl device, cpu reset handler and cpu model support Jens Freimann
@ 2012-12-12 13:08 ` Jens Freimann
2012-12-12 13:40 ` Alexander Graf
2012-12-12 13:51 ` Andreas Färber
0 siblings, 2 replies; 26+ messages in thread
From: Jens Freimann @ 2012-12-12 13:08 UTC (permalink / raw)
To: Alexander Graf
Cc: Heinz Graalfs, qemu-devel, Andreas Faerber, Christian Borntraeger,
Jens Freimann, Cornelia Huck, Viktor Mihajlovski, Einar Lueck
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>
---
hw/s390-virtio.c | 6 +++++-
target-s390x/cpu.c | 21 +++++++++++++++++++++
target-s390x/cpu.h | 3 +++
3 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
index 18050b1..5d4dd43 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 75f60b3..4a8562b 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -28,8 +28,29 @@
#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)
+{
+ (*cpu_fprintf)(f, "s390 %16s\n", "[host]");
+}
+
+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
--
1.7.12.4
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] S390: Enable -cpu help and QMP query-cpu-definitions
2012-12-12 13:08 ` [Qemu-devel] [PATCH 3/3] S390: Enable -cpu help and QMP query-cpu-definitions Jens Freimann
@ 2012-12-12 13:40 ` Alexander Graf
2012-12-12 13:51 ` Andreas Färber
1 sibling, 0 replies; 26+ messages in thread
From: Alexander Graf @ 2012-12-12 13:40 UTC (permalink / raw)
To: Jens Freimann
Cc: Heinz Graalfs, qemu-devel, Andreas Faerber, Christian Borntraeger,
Cornelia Huck, Viktor Mihajlovski, Einar Lueck
On 12.12.2012, at 14:08, 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>
> ---
> hw/s390-virtio.c | 6 +++++-
> target-s390x/cpu.c | 21 +++++++++++++++++++++
> target-s390x/cpu.h | 3 +++
> 3 files changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
> index 18050b1..5d4dd43 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 75f60b3..4a8562b 100644
> --- a/target-s390x/cpu.c
> +++ b/target-s390x/cpu.c
> @@ -28,8 +28,29 @@
> #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)
> +{
> + (*cpu_fprintf)(f, "s390 %16s\n", "[host]");
> +}
> +
> +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;
Can't we keep all the above information in a const struct in .rodata?
Alex
> +}
> +
> /* 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
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] S390: Enable -cpu help and QMP query-cpu-definitions
2012-12-12 13:08 ` [Qemu-devel] [PATCH 3/3] S390: Enable -cpu help and QMP query-cpu-definitions Jens Freimann
2012-12-12 13:40 ` Alexander Graf
@ 2012-12-12 13:51 ` Andreas Färber
2012-12-12 14:03 ` Alexander Graf
2012-12-12 15:05 ` Viktor Mihajlovski
1 sibling, 2 replies; 26+ messages in thread
From: Andreas Färber @ 2012-12-12 13:51 UTC (permalink / raw)
To: Jens Freimann
Cc: Heinz Graalfs, Alexander Graf, qemu-devel, Christian Borntraeger,
Cornelia Huck, Viktor Mihajlovski, Einar Lueck, Richard Henderson
Am 12.12.2012 14:08, 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>
> ---
> hw/s390-virtio.c | 6 +++++-
> target-s390x/cpu.c | 21 +++++++++++++++++++++
> target-s390x/cpu.h | 3 +++
> 3 files changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
> index 18050b1..5d4dd43 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 75f60b3..4a8562b 100644
> --- a/target-s390x/cpu.c
> +++ b/target-s390x/cpu.c
> @@ -28,8 +28,29 @@
> #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)
> +{
> + (*cpu_fprintf)(f, "s390 %16s\n", "[host]");
Note that the square-bracket notation was specific to x86 when it
distinguished between built-in and config-based models.
> +}
> +
> +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;
> +}
"host" only makes sense for KVM, not for TCG. So we would need one other
placeholder model for libvirt.
In cpu_s390x_init() a check should be added to reject "host" if
!kvm_enabled().
Background is that in the CPU subclasses world we would want to derive a
"host-s390-cpu" here and not let that be instantiated outside KVM. Then
we do need one always instantiatible model though.
Regards,
Andreas
> +
> /* 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
>
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] S390: Enable -cpu help and QMP query-cpu-definitions
2012-12-12 13:51 ` Andreas Färber
@ 2012-12-12 14:03 ` Alexander Graf
2012-12-12 14:57 ` Viktor Mihajlovski
2012-12-12 17:52 ` Richard Henderson
2012-12-12 15:05 ` Viktor Mihajlovski
1 sibling, 2 replies; 26+ messages in thread
From: Alexander Graf @ 2012-12-12 14:03 UTC (permalink / raw)
To: Andreas Färber
Cc: Heinz Graalfs, qemu-devel, Viktor Mihajlovski,
Christian Borntraeger, Jens Freimann, Cornelia Huck, Einar Lueck,
Richard Henderson
On 12.12.2012, at 14:51, Andreas Färber wrote:
> Am 12.12.2012 14:08, 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>
>> ---
>> hw/s390-virtio.c | 6 +++++-
>> target-s390x/cpu.c | 21 +++++++++++++++++++++
>> target-s390x/cpu.h | 3 +++
>> 3 files changed, 29 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
>> index 18050b1..5d4dd43 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 75f60b3..4a8562b 100644
>> --- a/target-s390x/cpu.c
>> +++ b/target-s390x/cpu.c
>> @@ -28,8 +28,29 @@
>> #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)
>> +{
>> + (*cpu_fprintf)(f, "s390 %16s\n", "[host]");
>
> Note that the square-bracket notation was specific to x86 when it
> distinguished between built-in and config-based models.
>
>> +}
>> +
>> +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;
>> +}
>
> "host" only makes sense for KVM, not for TCG. So we would need one other
> placeholder model for libvirt.
>
> In cpu_s390x_init() a check should be added to reject "host" if
> !kvm_enabled().
>
> Background is that in the CPU subclasses world we would want to derive a
> "host-s390-cpu" here and not let that be instantiated outside KVM. Then
> we do need one always instantiatible model though.
The TCG model is a "z900". We can just have a "z900" model and a "host" model with tcg_enabled() vs kvm_enabled() checks on each.
Alex
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] S390: Enable -cpu help and QMP query-cpu-definitions
2012-12-12 14:03 ` Alexander Graf
@ 2012-12-12 14:57 ` Viktor Mihajlovski
2012-12-12 17:52 ` Richard Henderson
1 sibling, 0 replies; 26+ messages in thread
From: Viktor Mihajlovski @ 2012-12-12 14:57 UTC (permalink / raw)
To: Alexander Graf
Cc: Heinz Graalfs, qemu-devel, Einar Lueck, Christian Borntraeger,
Jens Freimann, Cornelia Huck, Andreas Färber,
Richard Henderson
On 12/12/2012 03:03 PM, Alexander Graf wrote:
>
>
>
> The TCG model is a "z900". We can just have a "z900" model and a "host" model with tcg_enabled() vs kvm_enabled() checks on each.
>
>
> Alex
>
I would advise to not define 'artificial' names, i.e. such that cannot
be retrieved via a user-space interface on the host in order to make it
easier for systems management tools to figure out the right CPU model
in the future.
Further, if 'z900' was the host CPU model, then it should also be
accepted in kvm mode in order to behave as on x86. This can be deferred
to a future point in time.
Having said this, I have no idea whether 'z900' would be a correct
choice for a CPU model or not.
--
Mit freundlichen Grüßen/Kind Regards
Viktor Mihajlovski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] S390: Enable -cpu help and QMP query-cpu-definitions
2012-12-12 13:51 ` Andreas Färber
2012-12-12 14:03 ` Alexander Graf
@ 2012-12-12 15:05 ` Viktor Mihajlovski
2012-12-12 15:50 ` Alexander Graf
2012-12-12 16:28 ` Andreas Färber
1 sibling, 2 replies; 26+ messages in thread
From: Viktor Mihajlovski @ 2012-12-12 15:05 UTC (permalink / raw)
To: Andreas Färber
Cc: Heinz Graalfs, Alexander Graf, qemu-devel, Christian Borntraeger,
Jens Freimann, Cornelia Huck, Einar Lueck, Richard Henderson
On 12/12/2012 02:51 PM, Andreas Färber wrote:
>> + (*cpu_fprintf)(f, "s390 %16s\n", "[host]");
>
> Note that the square-bracket notation was specific to x86 when it
> distinguished between built-in and config-based models.
>
OK, since libvirt capable of dealing with s390 cpu models will
never see this, we can change it any way that is wanted.
So, host without brackets?
> "host" only makes sense for KVM, not for TCG. So we would need one other
> placeholder model for libvirt.
see my reply to Alex' comment: the placeholder name must be chosen
carefully, i.e. future-proof
--
Mit freundlichen Grüßen/Kind Regards
Viktor Mihajlovski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] S390: Enable -cpu help and QMP query-cpu-definitions
2012-12-12 15:05 ` Viktor Mihajlovski
@ 2012-12-12 15:50 ` Alexander Graf
2012-12-12 16:28 ` Andreas Färber
1 sibling, 0 replies; 26+ messages in thread
From: Alexander Graf @ 2012-12-12 15:50 UTC (permalink / raw)
To: Viktor Mihajlovski
Cc: Heinz Graalfs, qemu-devel, Einar Lueck, Christian Borntraeger,
Jens Freimann, Cornelia Huck, Andreas Färber,
Richard Henderson
On 12/12/2012 04:05 PM, Viktor Mihajlovski wrote:
> On 12/12/2012 02:51 PM, Andreas Färber wrote:
>>> + (*cpu_fprintf)(f, "s390 %16s\n", "[host]");
>>
>> Note that the square-bracket notation was specific to x86 when it
>> distinguished between built-in and config-based models.
>>
> OK, since libvirt capable of dealing with s390 cpu models will
> never see this, we can change it any way that is wanted.
> So, host without brackets?
>> "host" only makes sense for KVM, not for TCG. So we would need one other
>> placeholder model for libvirt.
> see my reply to Alex' comment: the placeholder name must be chosen
> carefully, i.e. future-proof
Yeah, it's perfectly fine for me to ignore the model name in the code
today (as we do iirc) and only add the host target for starters. Though
that one should do a kvm_enabled() check.
Alex
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] S390: Enable -cpu help and QMP query-cpu-definitions
2012-12-12 15:05 ` Viktor Mihajlovski
2012-12-12 15:50 ` Alexander Graf
@ 2012-12-12 16:28 ` Andreas Färber
2012-12-12 18:23 ` Alexander Graf
1 sibling, 1 reply; 26+ messages in thread
From: Andreas Färber @ 2012-12-12 16:28 UTC (permalink / raw)
To: Viktor Mihajlovski, Alexander Graf
Cc: Heinz Graalfs, qemu-devel, Christian Borntraeger, Jens Freimann,
Cornelia Huck, Einar Lueck, Richard Henderson
Am 12.12.2012 16:05, schrieb Viktor Mihajlovski:
> On 12/12/2012 02:51 PM, Andreas Färber wrote:
>>> + (*cpu_fprintf)(f, "s390 %16s\n", "[host]");
>>
>> Note that the square-bracket notation was specific to x86 when it
>> distinguished between built-in and config-based models.
>>
> OK, since libvirt capable of dealing with s390 cpu models will
> never see this, we can change it any way that is wanted.
> So, host without brackets?
Yes, but see below...
>> "host" only makes sense for KVM, not for TCG. So we would need one other
>> placeholder model for libvirt.
> see my reply to Alex' comment: the placeholder name must be chosen
> carefully, i.e. future-proof
On further thoughts, didn't we discuss that the issue libvirt wants to
address is that migration from z10 to z9 must fail? That's not solved
with -cpu host, we would need two other models then. IMO ideally -cpu
host should have the same semantics as on x86, that is passing the host
features through mostly 1:1. IIUC there is currently no way to not do so?
What about future-wise having -cpu host not be a subclass and instead
behaving like Alex' -cpu best, given the above semantics? What I am just
worried about with this patch is cementing the use of -cpu host into
libvirt when that is not a mid-term solution.
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] S390: Enable -cpu help and QMP query-cpu-definitions
2012-12-12 14:03 ` Alexander Graf
2012-12-12 14:57 ` Viktor Mihajlovski
@ 2012-12-12 17:52 ` Richard Henderson
2012-12-12 18:25 ` Alexander Graf
1 sibling, 1 reply; 26+ messages in thread
From: Richard Henderson @ 2012-12-12 17:52 UTC (permalink / raw)
To: Alexander Graf
Cc: Heinz Graalfs, Einar Lueck, qemu-devel, Viktor Mihajlovski,
Christian Borntraeger, Jens Freimann, Cornelia Huck,
Andreas Färber
On 12/12/2012 06:03 AM, Alexander Graf wrote:
> The TCG model is a "z900". We can just have a "z900" model and a
> "host" model with tcg_enabled() vs kvm_enabled() checks on each.
My target-s390x tcg rewrite targets something just shy of z196.
The main thing that's missing is actually STORE FACILITY LIST EXTENDED.
Which I always assumed would go along with a re-vamp of the cpu model,
just as we're considering here, so that we could in fact emulate z900
or anything in between.
r~
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] S390: Enable -cpu help and QMP query-cpu-definitions
2012-12-12 16:28 ` Andreas Färber
@ 2012-12-12 18:23 ` Alexander Graf
0 siblings, 0 replies; 26+ messages in thread
From: Alexander Graf @ 2012-12-12 18:23 UTC (permalink / raw)
To: Andreas Färber
Cc: Heinz Graalfs, qemu-devel, Viktor Mihajlovski,
Christian Borntraeger, Jens Freimann, Cornelia Huck, Einar Lueck,
Richard Henderson
On 12/12/2012 05:28 PM, Andreas Färber wrote:
> Am 12.12.2012 16:05, schrieb Viktor Mihajlovski:
>> On 12/12/2012 02:51 PM, Andreas Färber wrote:
>>>> + (*cpu_fprintf)(f, "s390 %16s\n", "[host]");
>>> Note that the square-bracket notation was specific to x86 when it
>>> distinguished between built-in and config-based models.
>>>
>> OK, since libvirt capable of dealing with s390 cpu models will
>> never see this, we can change it any way that is wanted.
>> So, host without brackets?
> Yes, but see below...
Yes, and with #ifdef CONFIG_KVM :). That way the TCG behavior stays
untouched.
>
>>> "host" only makes sense for KVM, not for TCG. So we would need one other
>>> placeholder model for libvirt.
>> see my reply to Alex' comment: the placeholder name must be chosen
>> carefully, i.e. future-proof
> On further thoughts, didn't we discuss that the issue libvirt wants to
> address is that migration from z10 to z9 must fail? That's not solved
> with -cpu host, we would need two other models then. IMO ideally -cpu
> host should have the same semantics as on x86, that is passing the host
> features through mostly 1:1. IIUC there is currently no way to not do so?
That is another problem that we need to solve, but one thing at a time.
> What about future-wise having -cpu host not be a subclass and instead
> behaving like Alex' -cpu best, given the above semantics? What I am just
> worried about with this patch is cementing the use of -cpu host into
> libvirt when that is not a mid-term solution.
I think on s390 we can get away with the same method as ppc for -cpu
host, which basically is -cpu best without fuzziness.
Alex
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] S390: Enable -cpu help and QMP query-cpu-definitions
2012-12-12 17:52 ` Richard Henderson
@ 2012-12-12 18:25 ` Alexander Graf
0 siblings, 0 replies; 26+ messages in thread
From: Alexander Graf @ 2012-12-12 18:25 UTC (permalink / raw)
To: Richard Henderson
Cc: Heinz Graalfs, Einar Lueck, qemu-devel, Viktor Mihajlovski,
Christian Borntraeger, Jens Freimann, Cornelia Huck,
Andreas Färber
On 12/12/2012 06:52 PM, Richard Henderson wrote:
> On 12/12/2012 06:03 AM, Alexander Graf wrote:
>> The TCG model is a "z900". We can just have a "z900" model and a
>> "host" model with tcg_enabled() vs kvm_enabled() checks on each.
> My target-s390x tcg rewrite targets something just shy of z196.
>
> The main thing that's missing is actually STORE FACILITY LIST EXTENDED.
> Which I always assumed would go along with a re-vamp of the cpu model,
> just as we're considering here, so that we could in fact emulate z900
> or anything in between.
Yes, and it's definitely something we need to do. I'm just concerned
about timing. Do we want to hold back a bug fix (libvirt can break with
empty cpu lists) for a feature (proper cpu list for tcg targets)?
I'd say let's make the -cpu host target work and whoever gets around to
it first can implement cpu models, store facility list extended, and tcg
feature masks.
Alex
^ permalink raw reply [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH 3/3] S390: Enable -cpu help and QMP query-cpu-definitions
2012-12-14 16:46 [Qemu-devel] [PATCH 0/3] s390: ipl device, cpu reset handler and cpu model support Jens Freimann
@ 2012-12-14 16:46 ` Jens Freimann
2012-12-16 15:42 ` Andreas Färber
2012-12-17 14:47 ` Alexander Graf
0 siblings, 2 replies; 26+ messages in thread
From: Jens Freimann @ 2012-12-14 16:46 UTC (permalink / raw)
To: Alexander Graf
Cc: Heinz Graalfs, qemu-devel, Andreas Faerber, Christian Borntraeger,
Jens Freimann, Cornelia Huck, Viktor Mihajlovski, Einar Lueck
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
+}
+
+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
--
1.7.12.4
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] S390: Enable -cpu help and QMP query-cpu-definitions
2012-12-14 16:46 ` [Qemu-devel] [PATCH 3/3] S390: Enable -cpu help and QMP query-cpu-definitions Jens Freimann
@ 2012-12-16 15:42 ` Andreas Färber
2012-12-17 14:47 ` Alexander Graf
1 sibling, 0 replies; 26+ messages in thread
From: Andreas Färber @ 2012-12-16 15:42 UTC (permalink / raw)
To: Jens Freimann, Viktor Mihajlovski
Cc: Heinz Graalfs, Alexander Graf, qemu-devel, Christian Borntraeger,
Cornelia Huck, Einar Lueck
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
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] S390: Enable -cpu help and QMP query-cpu-definitions
2012-12-14 16:46 ` [Qemu-devel] [PATCH 3/3] S390: Enable -cpu help and QMP query-cpu-definitions Jens Freimann
2012-12-16 15:42 ` Andreas Färber
@ 2012-12-17 14:47 ` Alexander Graf
2012-12-17 17:32 ` Andreas Färber
1 sibling, 1 reply; 26+ messages in thread
From: Alexander Graf @ 2012-12-17 14:47 UTC (permalink / raw)
To: Jens Freimann
Cc: Heinz Graalfs, qemu-devel, Andreas Faerber, Christian Borntraeger,
Cornelia Huck, Viktor Mihajlovski, Einar Lueck
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
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] S390: Enable -cpu help and QMP query-cpu-definitions
2012-12-17 14:47 ` Alexander Graf
@ 2012-12-17 17:32 ` Andreas Färber
0 siblings, 0 replies; 26+ messages in thread
From: Andreas Färber @ 2012-12-17 17:32 UTC (permalink / raw)
To: Alexander Graf
Cc: Heinz Graalfs, qemu-devel, Viktor Mihajlovski,
Christian Borntraeger, Jens Freimann, Cornelia Huck, Einar Lueck
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
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH 0/3] s390: ipl device, cpu reset handler and cpu model support
@ 2012-12-18 17:50 Jens Freimann
2012-12-18 17:50 ` [Qemu-devel] [PATCH 1/3] s390: Move IPL code into a separate device Jens Freimann
` (2 more replies)
0 siblings, 3 replies; 26+ messages in thread
From: Jens Freimann @ 2012-12-18 17:50 UTC (permalink / raw)
To: Alexander Graf
Cc: Heinz Graalfs, qemu-devel, Andreas Faerber, Christian Borntraeger,
Jens Freimann, Cornelia Huck, Einar Lueck
Hi Alex,
here's a reworked version of the ipl-device patch, the cpu reset handler
and the patch to query cpu definitions via QMP.
Patch 1: Creates a new ipl device and moves ipl code from s390_virtio.c
Patch 2: Adds a cpu reset handler to
Patch 3 Allow to query cpu types via commandline -? argument as well
as via qmp
Christian Borntraeger (1):
s390: Move IPL code into a separate device
Jens Freimann (1):
s390: Add CPU reset handler
Viktor Mihajlovski (1):
S390: Enable -cpu help and QMP query-cpu-definitions
hw/s390-virtio.c | 104 +++++------------------------
hw/s390x/Makefile.objs | 1 +
hw/s390x/ipl.c | 174 +++++++++++++++++++++++++++++++++++++++++++++++++
target-s390x/cpu.c | 58 ++++++++++++++++-
target-s390x/cpu.h | 3 +
target-s390x/kvm.c | 9 ++-
6 files changed, 257 insertions(+), 92 deletions(-)
create mode 100644 hw/s390x/ipl.c
--
1.7.12.4
^ permalink raw reply [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH 1/3] s390: Move IPL code into a separate device
2012-12-18 17:50 [Qemu-devel] [PATCH 0/3] s390: ipl device, cpu reset handler and cpu model support Jens Freimann
@ 2012-12-18 17:50 ` Jens Freimann
2013-01-03 12:47 ` Alexander Graf
2012-12-18 17:50 ` [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler Jens Freimann
2012-12-18 17:50 ` [Qemu-devel] [PATCH 3/3] S390: Enable -cpu help and QMP query-cpu-definitions Jens Freimann
2 siblings, 1 reply; 26+ messages in thread
From: Jens Freimann @ 2012-12-18 17:50 UTC (permalink / raw)
To: Alexander Graf
Cc: Heinz Graalfs, qemu-devel, Andreas Faerber, Christian Borntraeger,
Jens Freimann, Cornelia Huck, Einar Lueck
From: Christian Borntraeger <borntraeger@de.ibm.com>
Lets move the code to setup IPL for external kernel
or via the zipl rom into a separate file. This allows to
- define a reboot handler, setting up the PSW appropriately
- enhance the boot code to IPL disks that contain a bootmap that
was created with zipl under LPAR or z/VM (future patch)
- reuse that code for several machines (e.g. virtio-ccw and virtio-s390)
- allow different machines to provide different defaults
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
---
v2 -> v3:
* changed include <sysemu.h> -> "sysemu.h"
* make S390IPLState non-anonymous struct
* add QOM cast macro S390_IPL(dev)
* remove trailing whitespace
v1 -> v2:
* get rid of ipl.h
* move defines to ipl.c and make s390_ipl_cpu static
---
---
hw/s390-virtio.c | 98 +++-------------------------
hw/s390x/Makefile.objs | 1 +
hw/s390x/ipl.c | 174 +++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 185 insertions(+), 88 deletions(-)
create mode 100644 hw/s390x/ipl.c
diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
index ca1bb09..a350430 100644
--- a/hw/s390-virtio.c
+++ b/hw/s390-virtio.c
@@ -25,7 +25,6 @@
#include "boards.h"
#include "monitor.h"
#include "loader.h"
-#include "elf.h"
#include "hw/virtio.h"
#include "hw/sysbus.h"
#include "kvm.h"
@@ -48,17 +47,6 @@
#define KVM_S390_VIRTIO_RESET 1
#define KVM_S390_VIRTIO_SET_STATUS 2
-#define KERN_IMAGE_START 0x010000UL
-#define KERN_PARM_AREA 0x010480UL
-#define INITRD_START 0x800000UL
-#define INITRD_PARM_START 0x010408UL
-#define INITRD_PARM_SIZE 0x010410UL
-#define PARMFILE_START 0x001000UL
-
-#define ZIPL_START 0x009000UL
-#define ZIPL_LOAD_ADDR 0x009000UL
-#define ZIPL_FILENAME "s390-zipl.rom"
-
#define MAX_BLK_DEVS 10
static VirtIOS390Bus *s390_bus;
@@ -156,15 +144,10 @@ static void s390_init(QEMUMachineInitArgs *args)
{
ram_addr_t my_ram_size = args->ram_size;
const char *cpu_model = args->cpu_model;
- const char *kernel_filename = args->kernel_filename;
- const char *kernel_cmdline = args->kernel_cmdline;
- const char *initrd_filename = args->initrd_filename;
CPUS390XState *env = NULL;
+ DeviceState *dev;
MemoryRegion *sysmem = get_system_memory();
MemoryRegion *ram = g_new(MemoryRegion, 1);
- ram_addr_t kernel_size = 0;
- ram_addr_t initrd_offset;
- ram_addr_t initrd_size = 0;
int shift = 0;
uint8_t *storage_keys;
void *virtio_region;
@@ -185,6 +168,15 @@ static void s390_init(QEMUMachineInitArgs *args)
/* get a BUS */
s390_bus = s390_virtio_bus_init(&my_ram_size);
s390_sclp_init();
+ dev = qdev_create(NULL, "s390-ipl");
+ if (args->kernel_filename) {
+ qdev_prop_set_string(dev, "kernel", args->kernel_filename);
+ }
+ if (args->initrd_filename) {
+ qdev_prop_set_string(dev, "initrd", args->initrd_filename);
+ }
+ qdev_prop_set_string(dev, "cmdline", args->kernel_cmdline);
+ qdev_init_nofail(dev);
/* allocate RAM */
memory_region_init_ram(ram, "s390.ram", my_ram_size);
@@ -225,76 +217,6 @@ static void s390_init(QEMUMachineInitArgs *args)
tmp_env->storage_keys = storage_keys;
}
- /* One CPU has to run */
- s390_add_running_cpu(env);
-
- if (kernel_filename) {
-
- kernel_size = load_elf(kernel_filename, NULL, NULL, NULL, NULL,
- NULL, 1, ELF_MACHINE, 0);
- if (kernel_size == -1UL) {
- kernel_size = load_image_targphys(kernel_filename, 0, ram_size);
- }
- if (kernel_size == -1UL) {
- fprintf(stderr, "qemu: could not load kernel '%s'\n",
- kernel_filename);
- exit(1);
- }
- /*
- * we can not rely on the ELF entry point, since up to 3.2 this
- * value was 0x800 (the SALIPL loader) and it wont work. For
- * all (Linux) cases 0x10000 (KERN_IMAGE_START) should be fine.
- */
- env->psw.addr = KERN_IMAGE_START;
- env->psw.mask = 0x0000000180000000ULL;
- } else {
- ram_addr_t bios_size = 0;
- char *bios_filename;
-
- /* Load zipl bootloader */
- if (bios_name == NULL) {
- bios_name = ZIPL_FILENAME;
- }
-
- bios_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
- bios_size = load_image_targphys(bios_filename, ZIPL_LOAD_ADDR, 4096);
- g_free(bios_filename);
-
- if ((long)bios_size < 0) {
- hw_error("could not load bootloader '%s'\n", bios_name);
- }
-
- if (bios_size > 4096) {
- hw_error("stage1 bootloader is > 4k\n");
- }
-
- env->psw.addr = ZIPL_START;
- env->psw.mask = 0x0000000180000000ULL;
- }
-
- if (initrd_filename) {
- initrd_offset = INITRD_START;
- while (kernel_size + 0x100000 > initrd_offset) {
- initrd_offset += 0x100000;
- }
- initrd_size = load_image_targphys(initrd_filename, initrd_offset,
- ram_size - initrd_offset);
- if (initrd_size == -1UL) {
- fprintf(stderr, "qemu: could not load initrd '%s'\n",
- initrd_filename);
- exit(1);
- }
-
- /* we have to overwrite values in the kernel image, which are "rom" */
- stq_p(rom_ptr(INITRD_PARM_START), initrd_offset);
- stq_p(rom_ptr(INITRD_PARM_SIZE), initrd_size);
- }
-
- if (rom_ptr(KERN_PARM_AREA)) {
- /* we have to overwrite values in the kernel image, which are "rom" */
- memcpy(rom_ptr(KERN_PARM_AREA), kernel_cmdline,
- strlen(kernel_cmdline) + 1);
- }
/* Create VirtIO network adapters */
for(i = 0; i < nb_nics; i++) {
diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
index 096dfcd..4a5a5d8 100644
--- a/hw/s390x/Makefile.objs
+++ b/hw/s390x/Makefile.objs
@@ -4,3 +4,4 @@ obj-y := $(addprefix ../,$(obj-y))
obj-y += sclp.o
obj-y += event-facility.o
obj-y += sclpquiesce.o sclpconsole.o
+obj-y += ipl.o
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
new file mode 100644
index 0000000..7670079
--- /dev/null
+++ b/hw/s390x/ipl.c
@@ -0,0 +1,174 @@
+/*
+ * bootloader support
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ * Christian Borntraeger <borntraeger@de.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at your
+ * option) any later version. See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "sysemu.h"
+#include "cpu.h"
+#include "elf.h"
+#include "hw/loader.h"
+#include "hw/sysbus.h"
+
+#define KERN_IMAGE_START 0x010000UL
+#define KERN_PARM_AREA 0x010480UL
+#define INITRD_START 0x800000UL
+#define INITRD_PARM_START 0x010408UL
+#define INITRD_PARM_SIZE 0x010410UL
+#define PARMFILE_START 0x001000UL
+#define ZIPL_FILENAME "s390-zipl.rom"
+#define ZIPL_IMAGE_START 0x009000UL
+#define IPL_PSW_MASK 0x0000000180000000ULL
+
+#define TYPE_S390_IPL "s390-ipl"
+#define S390_IPL(obj) \
+ OBJECT_CHECK(S390IPLState, (obj), TYPE_S390_IPL)
+#if 0
+#define S390_IPL_CLASS(klass) \
+ OBJECT_CLASS_CHECK(S390IPLState, (klass), TYPE_S390_IPL)
+#define S390_IPL_GET_CLASS(obj) \
+ OBJECT_GET_CLASS(S390IPLState, (obj), TYPE_S390_IPL)
+#endif
+
+typedef struct S390IPLClass {
+ /*< private >*/
+ SysBusDeviceClass parent_class;
+ /*< public >*/
+
+ void (*parent_reset) (SysBusDevice *dev);
+} S390IPLClass;
+
+typedef struct S390IPLState {
+ /*< private >*/
+ SysBusDevice parent_obj;
+ /*< public >*/
+
+ char *kernel;
+ char *initrd;
+ char *cmdline;
+} S390IPLState;
+
+
+static void s390_ipl_cpu(uint64_t pswaddr)
+{
+ CPUS390XState *env = qemu_get_cpu(0);
+ env->psw.addr = pswaddr;
+ env->psw.mask = IPL_PSW_MASK;
+ s390_add_running_cpu(env);
+}
+
+static int s390_ipl_init(SysBusDevice *dev)
+{
+ S390IPLState *ipl = S390_IPL(dev);
+ ram_addr_t kernel_size = 0;
+
+ if (!ipl->kernel) {
+ ram_addr_t bios_size = 0;
+ char *bios_filename;
+
+ /* Load zipl bootloader */
+ if (bios_name == NULL) {
+ bios_name = ZIPL_FILENAME;
+ }
+
+ bios_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
+ bios_size = load_image_targphys(bios_filename, ZIPL_IMAGE_START, 4096);
+ g_free(bios_filename);
+
+ if ((long)bios_size < 0) {
+ hw_error("could not load bootloader '%s'\n", bios_name);
+ }
+
+ if (bios_size > 4096) {
+ hw_error("stage1 bootloader is > 4k\n");
+ }
+ return 0;
+ } else {
+ kernel_size = load_elf(ipl->kernel, NULL, NULL, NULL, NULL,
+ NULL, 1, ELF_MACHINE, 0);
+ if (kernel_size == -1UL) {
+ kernel_size = load_image_targphys(ipl->kernel, 0, ram_size);
+ }
+ if (kernel_size == -1UL) {
+ fprintf(stderr, "could not load kernel '%s'\n", ipl->kernel);
+ return -1;
+ }
+ /* we have to overwrite values in the kernel image, which are "rom" */
+ strcpy(rom_ptr(KERN_PARM_AREA), ipl->cmdline);
+ }
+ if (ipl->initrd) {
+ ram_addr_t initrd_offset, initrd_size;
+
+ initrd_offset = INITRD_START;
+ while (kernel_size + 0x100000 > initrd_offset) {
+ initrd_offset += 0x100000;
+ }
+ initrd_size = load_image_targphys(ipl->initrd, initrd_offset,
+ ram_size - initrd_offset);
+ if (initrd_size == -1UL) {
+ fprintf(stderr, "qemu: could not load initrd '%s'\n", ipl->initrd);
+ exit(1);
+ }
+
+ /* we have to overwrite values in the kernel image, which are "rom" */
+ stq_p(rom_ptr(INITRD_PARM_START), initrd_offset);
+ stq_p(rom_ptr(INITRD_PARM_SIZE), initrd_size);
+ }
+
+ return 0;
+}
+
+static Property s390_ipl_properties[] = {
+ DEFINE_PROP_STRING("kernel", S390IPLState, kernel),
+ DEFINE_PROP_STRING("initrd", S390IPLState, initrd),
+ DEFINE_PROP_STRING("cmdline", S390IPLState, cmdline),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
+static void s390_ipl_reset(DeviceState *dev)
+{
+ S390IPLState *ipl = S390_IPL(dev);
+
+ if (ipl->kernel) {
+ /*
+ * we can not rely on the ELF entry point, since up to 3.2 this
+ * value was 0x800 (the SALIPL loader) and it wont work. For
+ * all (Linux) cases 0x10000 (KERN_IMAGE_START) should be fine.
+ */
+ return s390_ipl_cpu(KERN_IMAGE_START);
+ } else {
+ return s390_ipl_cpu(ZIPL_IMAGE_START);
+ }
+}
+
+static void s390_ipl_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
+
+ k->init = s390_ipl_init;
+ dc->props = s390_ipl_properties;
+ dc->reset = s390_ipl_reset;
+ dc->no_user = 1;
+}
+
+static TypeInfo s390_ipl_info = {
+ .class_init = s390_ipl_class_init,
+ .parent = TYPE_SYS_BUS_DEVICE,
+ .name = "s390-ipl",
+ .instance_size = sizeof(S390IPLState),
+};
+
+static void s390_ipl_register_types(void)
+{
+ type_register_static(&s390_ipl_info);
+}
+
+type_init(s390_ipl_register_types)
--
1.7.12.4
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler
2012-12-18 17:50 [Qemu-devel] [PATCH 0/3] s390: ipl device, cpu reset handler and cpu model support Jens Freimann
2012-12-18 17:50 ` [Qemu-devel] [PATCH 1/3] s390: Move IPL code into a separate device Jens Freimann
@ 2012-12-18 17:50 ` Jens Freimann
2013-01-03 12:50 ` Alexander Graf
2013-01-03 12:55 ` Alexander Graf
2012-12-18 17:50 ` [Qemu-devel] [PATCH 3/3] S390: Enable -cpu help and QMP query-cpu-definitions Jens Freimann
2 siblings, 2 replies; 26+ messages in thread
From: Jens Freimann @ 2012-12-18 17:50 UTC (permalink / raw)
To: Alexander Graf
Cc: Heinz Graalfs, qemu-devel, Andreas Faerber, Christian Borntraeger,
Jens Freimann, Cornelia Huck, Einar Lueck
Add a CPU reset handler to have all CPUs in a PoP compliant
state.
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
---
v2 -> v3:
* remove FIXME
* separate parent reset from local reset by adding a while line
* use defines for register reset values
v1 -> v2:
* move setting of control registers and psa to s390_cpu_reset
and call it from the new s390_machine_cpu_reset_cb()
This makes it more similar to how it is done on x86
* in s390_cpu_reset() set env->halted state of cpu after
the memset. This is needed to keep our s390_cpu_running
counter in sync when s390_cpu_reset is called via the
qemu_devices_reset path
* set env->halted state in s390_cpu_initfn to 1 to avoid
decrementing the cpu counter during first reset
---
target-s390x/cpu.c | 35 +++++++++++++++++++++++++++++++++--
target-s390x/kvm.c | 9 ++++++++-
2 files changed, 41 insertions(+), 3 deletions(-)
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 619b202..58e412a 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -4,6 +4,7 @@
* Copyright (c) 2009 Ulrich Hecht
* Copyright (c) 2011 Alexander Graf
* Copyright (c) 2012 SUSE LINUX Products GmbH
+ * Copyright (c) 2012 IBM Corp.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -18,12 +19,19 @@
* 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/lgpl-2.1.html>
+ * Contributions after 2012-12-11 are licensed under the terms of the
+ * GNU GPL, version 2 or (at your option) any later version.
+ *
*/
#include "cpu.h"
+#include "hw/hw.h"
#include "qemu-common.h"
#include "qemu-timer.h"
+#define IPL_PSW_MASK 0x0000000180000000ULL
+#define CR0_RESET 0xE0UL
+#define CR14_RESET 0xC2000000UL;
/* CPUClass::reset() */
static void s390_cpu_reset(CPUState *s)
@@ -37,12 +45,30 @@ static void s390_cpu_reset(CPUState *s)
log_cpu_state(env, 0);
}
+ s390_del_running_cpu(env);
+
scc->parent_reset(s);
memset(env, 0, offsetof(CPUS390XState, breakpoints));
- /* FIXME: reset vector? */
+
+ /* architectured initial values for CR 0 and 14 */
+ env->cregs[0] = CR0_RESET;
+ env->cregs[14] = CR14_RESET;
+ /* set to z/Architecture mode */
+ env->psw.mask = IPL_PSW_MASK;
+ env->psa = 0;
+ /* set halted to 1 to make sure we can add the cpu in
+ * s390_ipl_cpu code, where env->halted is set back to 0
+ * after incrementing the cpu counter */
+ env->halted = 1;
tlb_flush(env, 1);
- s390_add_running_cpu(env);
+}
+
+static void s390_cpu_machine_reset_cb(void *opaque)
+{
+ S390CPU *cpu = opaque;
+
+ cpu_reset(CPU(cpu));
}
static void s390_cpu_initfn(Object *obj)
@@ -66,7 +92,12 @@ static void s390_cpu_initfn(Object *obj)
env->cpu_num = cpu_num++;
env->ext_index = -1;
+ /* set env->halted state to 1 to avoid decrementing the running
+ * cpu counter in s390_cpu_reset to a negative number at
+ * initial ipl */
+ env->halted = 1;
cpu_reset(CPU(cpu));
+ qemu_register_reset(s390_cpu_machine_reset_cb, cpu);
}
static void s390_cpu_class_init(ObjectClass *oc, void *data)
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 94de764..fda9f1f 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -85,7 +85,14 @@ int kvm_arch_init_vcpu(CPUS390XState *env)
void kvm_arch_reset_vcpu(CPUS390XState *env)
{
- /* FIXME: add code to reset vcpu. */
+ /* The initial reset call is needed here to reset in-kernel
+ * vcpu data that we can't access directly from QEMU
+ * (i.e. with older kernels which don't support sync_regs/ONE_REG).
+ * Before this ioctl cpu_synchronize_state() is called in common kvm
+ * code (kvm-all) */
+ if (kvm_vcpu_ioctl(env, KVM_S390_INITIAL_RESET, NULL)) {
+ perror("Can't reset vcpu\n");
+ }
}
int kvm_arch_put_registers(CPUS390XState *env, int level)
--
1.7.12.4
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH 3/3] S390: Enable -cpu help and QMP query-cpu-definitions
2012-12-18 17:50 [Qemu-devel] [PATCH 0/3] s390: ipl device, cpu reset handler and cpu model support Jens Freimann
2012-12-18 17:50 ` [Qemu-devel] [PATCH 1/3] s390: Move IPL code into a separate device Jens Freimann
2012-12-18 17:50 ` [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler Jens Freimann
@ 2012-12-18 17:50 ` Jens Freimann
2013-01-03 13:01 ` Alexander Graf
2 siblings, 1 reply; 26+ messages in thread
From: Jens Freimann @ 2012-12-18 17:50 UTC (permalink / raw)
To: Alexander Graf
Cc: Heinz Graalfs, qemu-devel, Andreas Faerber, Christian Borntraeger,
Jens Freimann, Cornelia Huck, Viktor Mihajlovski, Einar Lueck
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>
---
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 58e412a..1cddf41 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -28,11 +28,34 @@
#include "hw/hw.h"
#include "qemu-common.h"
#include "qemu-timer.h"
+#include "arch_init.h"
#define IPL_PSW_MASK 0x0000000180000000ULL
#define CR0_RESET 0xE0UL
#define CR14_RESET 0xC2000000UL;
+/* 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
--
1.7.12.4
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] s390: Move IPL code into a separate device
2012-12-18 17:50 ` [Qemu-devel] [PATCH 1/3] s390: Move IPL code into a separate device Jens Freimann
@ 2013-01-03 12:47 ` Alexander Graf
0 siblings, 0 replies; 26+ messages in thread
From: Alexander Graf @ 2013-01-03 12:47 UTC (permalink / raw)
To: Jens Freimann
Cc: Heinz Graalfs, qemu-devel, Andreas Faerber, Christian Borntraeger,
Cornelia Huck, Einar Lueck
On 18.12.2012, at 18:50, Jens Freimann wrote:
> From: Christian Borntraeger <borntraeger@de.ibm.com>
>
> Lets move the code to setup IPL for external kernel
> or via the zipl rom into a separate file. This allows to
>
> - define a reboot handler, setting up the PSW appropriately
> - enhance the boot code to IPL disks that contain a bootmap that
> was created with zipl under LPAR or z/VM (future patch)
> - reuse that code for several machines (e.g. virtio-ccw and virtio-s390)
> - allow different machines to provide different defaults
>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
>
> ---
> v2 -> v3:
> * changed include <sysemu.h> -> "sysemu.h"
> * make S390IPLState non-anonymous struct
> * add QOM cast macro S390_IPL(dev)
> * remove trailing whitespace
>
> v1 -> v2:
> * get rid of ipl.h
> * move defines to ipl.c and make s390_ipl_cpu static
> ---
> ---
> hw/s390-virtio.c | 98 +++-------------------------
> hw/s390x/Makefile.objs | 1 +
> hw/s390x/ipl.c | 174 +++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 185 insertions(+), 88 deletions(-)
> create mode 100644 hw/s390x/ipl.c
>
> diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
> index ca1bb09..a350430 100644
> --- a/hw/s390-virtio.c
> +++ b/hw/s390-virtio.c
> @@ -25,7 +25,6 @@
> #include "boards.h"
> #include "monitor.h"
> #include "loader.h"
> -#include "elf.h"
> #include "hw/virtio.h"
> #include "hw/sysbus.h"
> #include "kvm.h"
> @@ -48,17 +47,6 @@
> #define KVM_S390_VIRTIO_RESET 1
> #define KVM_S390_VIRTIO_SET_STATUS 2
>
> -#define KERN_IMAGE_START 0x010000UL
> -#define KERN_PARM_AREA 0x010480UL
> -#define INITRD_START 0x800000UL
> -#define INITRD_PARM_START 0x010408UL
> -#define INITRD_PARM_SIZE 0x010410UL
> -#define PARMFILE_START 0x001000UL
> -
> -#define ZIPL_START 0x009000UL
> -#define ZIPL_LOAD_ADDR 0x009000UL
> -#define ZIPL_FILENAME "s390-zipl.rom"
> -
> #define MAX_BLK_DEVS 10
>
> static VirtIOS390Bus *s390_bus;
> @@ -156,15 +144,10 @@ static void s390_init(QEMUMachineInitArgs *args)
> {
> ram_addr_t my_ram_size = args->ram_size;
> const char *cpu_model = args->cpu_model;
> - const char *kernel_filename = args->kernel_filename;
> - const char *kernel_cmdline = args->kernel_cmdline;
> - const char *initrd_filename = args->initrd_filename;
> CPUS390XState *env = NULL;
> + DeviceState *dev;
> MemoryRegion *sysmem = get_system_memory();
> MemoryRegion *ram = g_new(MemoryRegion, 1);
> - ram_addr_t kernel_size = 0;
> - ram_addr_t initrd_offset;
> - ram_addr_t initrd_size = 0;
> int shift = 0;
> uint8_t *storage_keys;
> void *virtio_region;
> @@ -185,6 +168,15 @@ static void s390_init(QEMUMachineInitArgs *args)
> /* get a BUS */
> s390_bus = s390_virtio_bus_init(&my_ram_size);
> s390_sclp_init();
> + dev = qdev_create(NULL, "s390-ipl");
> + if (args->kernel_filename) {
> + qdev_prop_set_string(dev, "kernel", args->kernel_filename);
> + }
> + if (args->initrd_filename) {
> + qdev_prop_set_string(dev, "initrd", args->initrd_filename);
> + }
> + qdev_prop_set_string(dev, "cmdline", args->kernel_cmdline);
> + qdev_init_nofail(dev);
>
> /* allocate RAM */
> memory_region_init_ram(ram, "s390.ram", my_ram_size);
> @@ -225,76 +217,6 @@ static void s390_init(QEMUMachineInitArgs *args)
> tmp_env->storage_keys = storage_keys;
> }
>
> - /* One CPU has to run */
> - s390_add_running_cpu(env);
> -
> - if (kernel_filename) {
> -
> - kernel_size = load_elf(kernel_filename, NULL, NULL, NULL, NULL,
> - NULL, 1, ELF_MACHINE, 0);
> - if (kernel_size == -1UL) {
> - kernel_size = load_image_targphys(kernel_filename, 0, ram_size);
> - }
> - if (kernel_size == -1UL) {
> - fprintf(stderr, "qemu: could not load kernel '%s'\n",
> - kernel_filename);
> - exit(1);
> - }
> - /*
> - * we can not rely on the ELF entry point, since up to 3.2 this
> - * value was 0x800 (the SALIPL loader) and it wont work. For
> - * all (Linux) cases 0x10000 (KERN_IMAGE_START) should be fine.
> - */
> - env->psw.addr = KERN_IMAGE_START;
> - env->psw.mask = 0x0000000180000000ULL;
> - } else {
> - ram_addr_t bios_size = 0;
> - char *bios_filename;
> -
> - /* Load zipl bootloader */
> - if (bios_name == NULL) {
> - bios_name = ZIPL_FILENAME;
> - }
> -
> - bios_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
> - bios_size = load_image_targphys(bios_filename, ZIPL_LOAD_ADDR, 4096);
> - g_free(bios_filename);
> -
> - if ((long)bios_size < 0) {
> - hw_error("could not load bootloader '%s'\n", bios_name);
> - }
> -
> - if (bios_size > 4096) {
> - hw_error("stage1 bootloader is > 4k\n");
> - }
> -
> - env->psw.addr = ZIPL_START;
> - env->psw.mask = 0x0000000180000000ULL;
> - }
> -
> - if (initrd_filename) {
> - initrd_offset = INITRD_START;
> - while (kernel_size + 0x100000 > initrd_offset) {
> - initrd_offset += 0x100000;
> - }
> - initrd_size = load_image_targphys(initrd_filename, initrd_offset,
> - ram_size - initrd_offset);
> - if (initrd_size == -1UL) {
> - fprintf(stderr, "qemu: could not load initrd '%s'\n",
> - initrd_filename);
> - exit(1);
> - }
> -
> - /* we have to overwrite values in the kernel image, which are "rom" */
> - stq_p(rom_ptr(INITRD_PARM_START), initrd_offset);
> - stq_p(rom_ptr(INITRD_PARM_SIZE), initrd_size);
> - }
> -
> - if (rom_ptr(KERN_PARM_AREA)) {
> - /* we have to overwrite values in the kernel image, which are "rom" */
> - memcpy(rom_ptr(KERN_PARM_AREA), kernel_cmdline,
> - strlen(kernel_cmdline) + 1);
> - }
>
> /* Create VirtIO network adapters */
> for(i = 0; i < nb_nics; i++) {
> diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
> index 096dfcd..4a5a5d8 100644
> --- a/hw/s390x/Makefile.objs
> +++ b/hw/s390x/Makefile.objs
> @@ -4,3 +4,4 @@ obj-y := $(addprefix ../,$(obj-y))
> obj-y += sclp.o
> obj-y += event-facility.o
> obj-y += sclpquiesce.o sclpconsole.o
> +obj-y += ipl.o
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> new file mode 100644
> index 0000000..7670079
> --- /dev/null
> +++ b/hw/s390x/ipl.c
> @@ -0,0 +1,174 @@
> +/*
> + * bootloader support
> + *
> + * Copyright IBM, Corp. 2012
> + *
> + * Authors:
> + * Christian Borntraeger <borntraeger@de.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or (at your
> + * option) any later version. See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "sysemu.h"
> +#include "cpu.h"
> +#include "elf.h"
> +#include "hw/loader.h"
> +#include "hw/sysbus.h"
> +
> +#define KERN_IMAGE_START 0x010000UL
> +#define KERN_PARM_AREA 0x010480UL
> +#define INITRD_START 0x800000UL
> +#define INITRD_PARM_START 0x010408UL
> +#define INITRD_PARM_SIZE 0x010410UL
> +#define PARMFILE_START 0x001000UL
> +#define ZIPL_FILENAME "s390-zipl.rom"
> +#define ZIPL_IMAGE_START 0x009000UL
> +#define IPL_PSW_MASK 0x0000000180000000ULL
I actually meant something along the lines of
#define IPL_PSW_MASK (PSW_MASK_32 | PSW_MASK_64)
but I'll just change it accordingly while applying the patch :).
> +
> +#define TYPE_S390_IPL "s390-ipl"
> +#define S390_IPL(obj) \
> + OBJECT_CHECK(S390IPLState, (obj), TYPE_S390_IPL)
> +#if 0
> +#define S390_IPL_CLASS(klass) \
> + OBJECT_CLASS_CHECK(S390IPLState, (klass), TYPE_S390_IPL)
> +#define S390_IPL_GET_CLASS(obj) \
> + OBJECT_GET_CLASS(S390IPLState, (obj), TYPE_S390_IPL)
> +#endif
> +
> +typedef struct S390IPLClass {
> + /*< private >*/
> + SysBusDeviceClass parent_class;
> + /*< public >*/
> +
> + void (*parent_reset) (SysBusDevice *dev);
> +} S390IPLClass;
> +
> +typedef struct S390IPLState {
> + /*< private >*/
> + SysBusDevice parent_obj;
> + /*< public >*/
> +
> + char *kernel;
> + char *initrd;
> + char *cmdline;
> +} S390IPLState;
> +
> +
> +static void s390_ipl_cpu(uint64_t pswaddr)
> +{
> + CPUS390XState *env = qemu_get_cpu(0);
> + env->psw.addr = pswaddr;
> + env->psw.mask = IPL_PSW_MASK;
> + s390_add_running_cpu(env);
> +}
> +
> +static int s390_ipl_init(SysBusDevice *dev)
> +{
> + S390IPLState *ipl = S390_IPL(dev);
> + ram_addr_t kernel_size = 0;
> +
> + if (!ipl->kernel) {
> + ram_addr_t bios_size = 0;
> + char *bios_filename;
> +
> + /* Load zipl bootloader */
> + if (bios_name == NULL) {
> + bios_name = ZIPL_FILENAME;
> + }
> +
> + bios_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
> + bios_size = load_image_targphys(bios_filename, ZIPL_IMAGE_START, 4096);
> + g_free(bios_filename);
> +
> + if ((long)bios_size < 0) {
> + hw_error("could not load bootloader '%s'\n", bios_name);
> + }
> +
> + if (bios_size > 4096) {
> + hw_error("stage1 bootloader is > 4k\n");
> + }
> + return 0;
> + } else {
> + kernel_size = load_elf(ipl->kernel, NULL, NULL, NULL, NULL,
> + NULL, 1, ELF_MACHINE, 0);
> + if (kernel_size == -1UL) {
> + kernel_size = load_image_targphys(ipl->kernel, 0, ram_size);
> + }
> + if (kernel_size == -1UL) {
> + fprintf(stderr, "could not load kernel '%s'\n", ipl->kernel);
> + return -1;
> + }
> + /* we have to overwrite values in the kernel image, which are "rom" */
> + strcpy(rom_ptr(KERN_PARM_AREA), ipl->cmdline);
> + }
> + if (ipl->initrd) {
> + ram_addr_t initrd_offset, initrd_size;
> +
> + initrd_offset = INITRD_START;
> + while (kernel_size + 0x100000 > initrd_offset) {
> + initrd_offset += 0x100000;
> + }
> + initrd_size = load_image_targphys(ipl->initrd, initrd_offset,
> + ram_size - initrd_offset);
> + if (initrd_size == -1UL) {
> + fprintf(stderr, "qemu: could not load initrd '%s'\n", ipl->initrd);
> + exit(1);
> + }
> +
> + /* we have to overwrite values in the kernel image, which are "rom" */
> + stq_p(rom_ptr(INITRD_PARM_START), initrd_offset);
> + stq_p(rom_ptr(INITRD_PARM_SIZE), initrd_size);
In the long run, these should be overwritten in RAM manually on reset, so that we can change load_image_targphys to reload the kernel from a file on reset.
Thanks, applied to s390-next.
Alex
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler
2012-12-18 17:50 ` [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler Jens Freimann
@ 2013-01-03 12:50 ` Alexander Graf
2013-01-04 13:55 ` Jens Freimann
2013-01-03 12:55 ` Alexander Graf
1 sibling, 1 reply; 26+ messages in thread
From: Alexander Graf @ 2013-01-03 12:50 UTC (permalink / raw)
To: Jens Freimann
Cc: Heinz Graalfs, qemu-devel, Andreas Faerber, Christian Borntraeger,
Cornelia Huck, Einar Lueck
On 18.12.2012, at 18:50, Jens Freimann wrote:
> Add a CPU reset handler to have all CPUs in a PoP compliant
> state.
>
> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
>
> ---
> v2 -> v3:
> * remove FIXME
> * separate parent reset from local reset by adding a while line
> * use defines for register reset values
>
> v1 -> v2:
> * move setting of control registers and psa to s390_cpu_reset
> and call it from the new s390_machine_cpu_reset_cb()
> This makes it more similar to how it is done on x86
> * in s390_cpu_reset() set env->halted state of cpu after
> the memset. This is needed to keep our s390_cpu_running
> counter in sync when s390_cpu_reset is called via the
> qemu_devices_reset path
> * set env->halted state in s390_cpu_initfn to 1 to avoid
> decrementing the cpu counter during first reset
> ---
> target-s390x/cpu.c | 35 +++++++++++++++++++++++++++++++++--
> target-s390x/kvm.c | 9 ++++++++-
> 2 files changed, 41 insertions(+), 3 deletions(-)
>
> diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> index 619b202..58e412a 100644
> --- a/target-s390x/cpu.c
> +++ b/target-s390x/cpu.c
> @@ -4,6 +4,7 @@
> * Copyright (c) 2009 Ulrich Hecht
> * Copyright (c) 2011 Alexander Graf
> * Copyright (c) 2012 SUSE LINUX Products GmbH
> + * Copyright (c) 2012 IBM Corp.
> *
> * This library is free software; you can redistribute it and/or
> * modify it under the terms of the GNU Lesser General Public
> @@ -18,12 +19,19 @@
> * 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/lgpl-2.1.html>
> + * Contributions after 2012-12-11 are licensed under the terms of the
> + * GNU GPL, version 2 or (at your option) any later version.
> + *
> */
>
> #include "cpu.h"
> +#include "hw/hw.h"
> #include "qemu-common.h"
> #include "qemu-timer.h"
>
> +#define IPL_PSW_MASK 0x0000000180000000ULL
> +#define CR0_RESET 0xE0UL
> +#define CR14_RESET 0xC2000000UL;
>
> /* CPUClass::reset() */
> static void s390_cpu_reset(CPUState *s)
> @@ -37,12 +45,30 @@ static void s390_cpu_reset(CPUState *s)
> log_cpu_state(env, 0);
> }
>
> + s390_del_running_cpu(env);
> +
> scc->parent_reset(s);
>
> memset(env, 0, offsetof(CPUS390XState, breakpoints));
> - /* FIXME: reset vector? */
> +
> + /* architectured initial values for CR 0 and 14 */
> + env->cregs[0] = CR0_RESET;
> + env->cregs[14] = CR14_RESET;
> + /* set to z/Architecture mode */
> + env->psw.mask = IPL_PSW_MASK;
Why would we set psw.mask, but not psw.addr? In fact, why are we setting psw at all here? Shouldn't we just leave it at 0 from the memset and simply override it in the ipl device?
Alex
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler
2012-12-18 17:50 ` [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler Jens Freimann
2013-01-03 12:50 ` Alexander Graf
@ 2013-01-03 12:55 ` Alexander Graf
2013-01-04 14:09 ` Jens Freimann
1 sibling, 1 reply; 26+ messages in thread
From: Alexander Graf @ 2013-01-03 12:55 UTC (permalink / raw)
To: Jens Freimann
Cc: Heinz Graalfs, qemu-devel, Andreas Faerber, Christian Borntraeger,
Cornelia Huck, Einar Lueck
On 18.12.2012, at 18:50, Jens Freimann wrote:
> Add a CPU reset handler to have all CPUs in a PoP compliant
> state.
>
> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
>
> ---
> v2 -> v3:
> * remove FIXME
> * separate parent reset from local reset by adding a while line
> * use defines for register reset values
>
> v1 -> v2:
> * move setting of control registers and psa to s390_cpu_reset
> and call it from the new s390_machine_cpu_reset_cb()
> This makes it more similar to how it is done on x86
> * in s390_cpu_reset() set env->halted state of cpu after
> the memset. This is needed to keep our s390_cpu_running
> counter in sync when s390_cpu_reset is called via the
> qemu_devices_reset path
> * set env->halted state in s390_cpu_initfn to 1 to avoid
> decrementing the cpu counter during first reset
> ---
> target-s390x/cpu.c | 35 +++++++++++++++++++++++++++++++++--
> target-s390x/kvm.c | 9 ++++++++-
> 2 files changed, 41 insertions(+), 3 deletions(-)
>
> diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> index 619b202..58e412a 100644
> --- a/target-s390x/cpu.c
> +++ b/target-s390x/cpu.c
> @@ -4,6 +4,7 @@
> * Copyright (c) 2009 Ulrich Hecht
> * Copyright (c) 2011 Alexander Graf
> * Copyright (c) 2012 SUSE LINUX Products GmbH
> + * Copyright (c) 2012 IBM Corp.
> *
> * This library is free software; you can redistribute it and/or
> * modify it under the terms of the GNU Lesser General Public
> @@ -18,12 +19,19 @@
> * 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/lgpl-2.1.html>
> + * Contributions after 2012-12-11 are licensed under the terms of the
> + * GNU GPL, version 2 or (at your option) any later version.
> + *
> */
>
> #include "cpu.h"
> +#include "hw/hw.h"
Also, have you verified that this doesn't break s390x-linux-user?
> #include "qemu-common.h"
> #include "qemu-timer.h"
>
> +#define IPL_PSW_MASK 0x0000000180000000ULL
> +#define CR0_RESET 0xE0UL
> +#define CR14_RESET 0xC2000000UL;
>
> /* CPUClass::reset() */
> static void s390_cpu_reset(CPUState *s)
> @@ -37,12 +45,30 @@ static void s390_cpu_reset(CPUState *s)
> log_cpu_state(env, 0);
> }
>
> + s390_del_running_cpu(env);
> +
> scc->parent_reset(s);
>
> memset(env, 0, offsetof(CPUS390XState, breakpoints));
> - /* FIXME: reset vector? */
> +
> + /* architectured initial values for CR 0 and 14 */
> + env->cregs[0] = CR0_RESET;
> + env->cregs[14] = CR14_RESET;
> + /* set to z/Architecture mode */
> + env->psw.mask = IPL_PSW_MASK;
In fact this one is correct for CONFIG_USER_ONLY.
> + env->psa = 0;
> + /* set halted to 1 to make sure we can add the cpu in
> + * s390_ipl_cpu code, where env->halted is set back to 0
> + * after incrementing the cpu counter */
> + env->halted = 1;
While this again probably breaks s390x-linux-user, no?
Alex
> tlb_flush(env, 1);
> - s390_add_running_cpu(env);
> +}
> +
> +static void s390_cpu_machine_reset_cb(void *opaque)
> +{
> + S390CPU *cpu = opaque;
> +
> + cpu_reset(CPU(cpu));
> }
>
> static void s390_cpu_initfn(Object *obj)
> @@ -66,7 +92,12 @@ static void s390_cpu_initfn(Object *obj)
> env->cpu_num = cpu_num++;
> env->ext_index = -1;
>
> + /* set env->halted state to 1 to avoid decrementing the running
> + * cpu counter in s390_cpu_reset to a negative number at
> + * initial ipl */
> + env->halted = 1;
> cpu_reset(CPU(cpu));
> + qemu_register_reset(s390_cpu_machine_reset_cb, cpu);
> }
>
> static void s390_cpu_class_init(ObjectClass *oc, void *data)
> diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
> index 94de764..fda9f1f 100644
> --- a/target-s390x/kvm.c
> +++ b/target-s390x/kvm.c
> @@ -85,7 +85,14 @@ int kvm_arch_init_vcpu(CPUS390XState *env)
>
> void kvm_arch_reset_vcpu(CPUS390XState *env)
> {
> - /* FIXME: add code to reset vcpu. */
> + /* The initial reset call is needed here to reset in-kernel
> + * vcpu data that we can't access directly from QEMU
> + * (i.e. with older kernels which don't support sync_regs/ONE_REG).
> + * Before this ioctl cpu_synchronize_state() is called in common kvm
> + * code (kvm-all) */
> + if (kvm_vcpu_ioctl(env, KVM_S390_INITIAL_RESET, NULL)) {
> + perror("Can't reset vcpu\n");
> + }
> }
>
> int kvm_arch_put_registers(CPUS390XState *env, int level)
> --
> 1.7.12.4
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] S390: Enable -cpu help and QMP query-cpu-definitions
2012-12-18 17:50 ` [Qemu-devel] [PATCH 3/3] S390: Enable -cpu help and QMP query-cpu-definitions Jens Freimann
@ 2013-01-03 13:01 ` Alexander Graf
0 siblings, 0 replies; 26+ messages in thread
From: Alexander Graf @ 2013-01-03 13:01 UTC (permalink / raw)
To: Jens Freimann
Cc: Heinz Graalfs, qemu-devel, Andreas Faerber, Christian Borntraeger,
Cornelia Huck, Viktor Mihajlovski, Einar Lueck
On 18.12.2012, at 18:50, 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>
Thanks, applied to s390-next.
Alex
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler
2013-01-03 12:50 ` Alexander Graf
@ 2013-01-04 13:55 ` Jens Freimann
0 siblings, 0 replies; 26+ messages in thread
From: Jens Freimann @ 2013-01-04 13:55 UTC (permalink / raw)
To: Alexander Graf
Cc: Heinz Graalfs, qemu-devel, Andreas Faerber, Christian Borntraeger,
Cornelia Huck, Einar Lueck
On Thu, Jan 03, 2013 at 01:50:22PM +0100, Alexander Graf wrote:
>
> On 18.12.2012, at 18:50, Jens Freimann wrote:
>
> > Add a CPU reset handler to have all CPUs in a PoP compliant
> > state.
> >
> > Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
> >
> > ---
> > v2 -> v3:
> > * remove FIXME
> > * separate parent reset from local reset by adding a while line
> > * use defines for register reset values
> >
> > v1 -> v2:
> > * move setting of control registers and psa to s390_cpu_reset
> > and call it from the new s390_machine_cpu_reset_cb()
> > This makes it more similar to how it is done on x86
> > * in s390_cpu_reset() set env->halted state of cpu after
> > the memset. This is needed to keep our s390_cpu_running
> > counter in sync when s390_cpu_reset is called via the
> > qemu_devices_reset path
> > * set env->halted state in s390_cpu_initfn to 1 to avoid
> > decrementing the cpu counter during first reset
> > ---
> > target-s390x/cpu.c | 35 +++++++++++++++++++++++++++++++++--
> > target-s390x/kvm.c | 9 ++++++++-
> > 2 files changed, 41 insertions(+), 3 deletions(-)
> >
> > diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> > index 619b202..58e412a 100644
> > --- a/target-s390x/cpu.c
> > +++ b/target-s390x/cpu.c
> > @@ -4,6 +4,7 @@
> > * Copyright (c) 2009 Ulrich Hecht
> > * Copyright (c) 2011 Alexander Graf
> > * Copyright (c) 2012 SUSE LINUX Products GmbH
> > + * Copyright (c) 2012 IBM Corp.
> > *
> > * This library is free software; you can redistribute it and/or
> > * modify it under the terms of the GNU Lesser General Public
> > @@ -18,12 +19,19 @@
> > * 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/lgpl-2.1.html>
> > + * Contributions after 2012-12-11 are licensed under the terms of the
> > + * GNU GPL, version 2 or (at your option) any later version.
> > + *
> > */
> >
> > #include "cpu.h"
> > +#include "hw/hw.h"
> > #include "qemu-common.h"
> > #include "qemu-timer.h"
> >
> > +#define IPL_PSW_MASK 0x0000000180000000ULL
> > +#define CR0_RESET 0xE0UL
> > +#define CR14_RESET 0xC2000000UL;
> >
> > /* CPUClass::reset() */
> > static void s390_cpu_reset(CPUState *s)
> > @@ -37,12 +45,30 @@ static void s390_cpu_reset(CPUState *s)
> > log_cpu_state(env, 0);
> > }
> >
> > + s390_del_running_cpu(env);
> > +
> > scc->parent_reset(s);
> >
> > memset(env, 0, offsetof(CPUS390XState, breakpoints));
> > - /* FIXME: reset vector? */
> > +
> > + /* architectured initial values for CR 0 and 14 */
> > + env->cregs[0] = CR0_RESET;
> > + env->cregs[14] = CR14_RESET;
> > + /* set to z/Architecture mode */
> > + env->psw.mask = IPL_PSW_MASK;
>
> Why would we set psw.mask, but not psw.addr? In fact, why are we setting psw at all here? Shouldn't we just leave it at 0 from the memset and simply override it in the ipl device?
I agree that it's redundand as we set it in the ipl device code and that it would work without setting
the mask here. I put it in here because I consider it part of the CPU reset code since we always want to start
in z/Architecture mode regardless of what code runs after the reset.
Jens
>
> Alex
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler
2013-01-03 12:55 ` Alexander Graf
@ 2013-01-04 14:09 ` Jens Freimann
2013-01-04 14:11 ` Alexander Graf
0 siblings, 1 reply; 26+ messages in thread
From: Jens Freimann @ 2013-01-04 14:09 UTC (permalink / raw)
To: Alexander Graf
Cc: Heinz Graalfs, qemu-devel, Andreas Faerber, Christian Borntraeger,
Cornelia Huck, Einar Lueck
On Thu, Jan 03, 2013 at 01:55:19PM +0100, Alexander Graf wrote:
>
> On 18.12.2012, at 18:50, Jens Freimann wrote:
>
> > Add a CPU reset handler to have all CPUs in a PoP compliant
> > state.
> >
> > Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
> >
> > ---
> > v2 -> v3:
> > * remove FIXME
> > * separate parent reset from local reset by adding a while line
> > * use defines for register reset values
> >
> > v1 -> v2:
> > * move setting of control registers and psa to s390_cpu_reset
> > and call it from the new s390_machine_cpu_reset_cb()
> > This makes it more similar to how it is done on x86
> > * in s390_cpu_reset() set env->halted state of cpu after
> > the memset. This is needed to keep our s390_cpu_running
> > counter in sync when s390_cpu_reset is called via the
> > qemu_devices_reset path
> > * set env->halted state in s390_cpu_initfn to 1 to avoid
> > decrementing the cpu counter during first reset
> > ---
> > target-s390x/cpu.c | 35 +++++++++++++++++++++++++++++++++--
> > target-s390x/kvm.c | 9 ++++++++-
> > 2 files changed, 41 insertions(+), 3 deletions(-)
> >
> > diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> > index 619b202..58e412a 100644
> > --- a/target-s390x/cpu.c
> > +++ b/target-s390x/cpu.c
> > @@ -4,6 +4,7 @@
> > * Copyright (c) 2009 Ulrich Hecht
> > * Copyright (c) 2011 Alexander Graf
> > * Copyright (c) 2012 SUSE LINUX Products GmbH
> > + * Copyright (c) 2012 IBM Corp.
> > *
> > * This library is free software; you can redistribute it and/or
> > * modify it under the terms of the GNU Lesser General Public
> > @@ -18,12 +19,19 @@
> > * 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/lgpl-2.1.html>
> > + * Contributions after 2012-12-11 are licensed under the terms of the
> > + * GNU GPL, version 2 or (at your option) any later version.
> > + *
> > */
> >
> > #include "cpu.h"
> > +#include "hw/hw.h"
>
> Also, have you verified that this doesn't break s390x-linux-user?
I verified s390x-linux-user still builds.
> > #include "qemu-common.h"
> > #include "qemu-timer.h"
> >
> > +#define IPL_PSW_MASK 0x0000000180000000ULL
> > +#define CR0_RESET 0xE0UL
> > +#define CR14_RESET 0xC2000000UL;
> >
> > /* CPUClass::reset() */
> > static void s390_cpu_reset(CPUState *s)
> > @@ -37,12 +45,30 @@ static void s390_cpu_reset(CPUState *s)
> > log_cpu_state(env, 0);
> > }
> >
> > + s390_del_running_cpu(env);
> > +
> > scc->parent_reset(s);
> >
> > memset(env, 0, offsetof(CPUS390XState, breakpoints));
> > - /* FIXME: reset vector? */
> > +
> > + /* architectured initial values for CR 0 and 14 */
> > + env->cregs[0] = CR0_RESET;
> > + env->cregs[14] = CR14_RESET;
> > + /* set to z/Architecture mode */
> > + env->psw.mask = IPL_PSW_MASK;
>
> In fact this one is correct for CONFIG_USER_ONLY.
>
> > + env->psa = 0;
> > + /* set halted to 1 to make sure we can add the cpu in
> > + * s390_ipl_cpu code, where env->halted is set back to 0
> > + * after incrementing the cpu counter */
> > + env->halted = 1;
>
> While this again probably breaks s390x-linux-user, no?
It still builds fine, if that's what you mean? env->halted is
not within an #ifdef !CONFIG_USER_ONLY clause.
Jens
>
> Alex
>
> > tlb_flush(env, 1);
> > - s390_add_running_cpu(env);
> > +}
> > +
> > +static void s390_cpu_machine_reset_cb(void *opaque)
> > +{
> > + S390CPU *cpu = opaque;
> > +
> > + cpu_reset(CPU(cpu));
> > }
> >
> > static void s390_cpu_initfn(Object *obj)
> > @@ -66,7 +92,12 @@ static void s390_cpu_initfn(Object *obj)
> > env->cpu_num = cpu_num++;
> > env->ext_index = -1;
> >
> > + /* set env->halted state to 1 to avoid decrementing the running
> > + * cpu counter in s390_cpu_reset to a negative number at
> > + * initial ipl */
> > + env->halted = 1;
> > cpu_reset(CPU(cpu));
> > + qemu_register_reset(s390_cpu_machine_reset_cb, cpu);
> > }
> >
> > static void s390_cpu_class_init(ObjectClass *oc, void *data)
> > diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
> > index 94de764..fda9f1f 100644
> > --- a/target-s390x/kvm.c
> > +++ b/target-s390x/kvm.c
> > @@ -85,7 +85,14 @@ int kvm_arch_init_vcpu(CPUS390XState *env)
> >
> > void kvm_arch_reset_vcpu(CPUS390XState *env)
> > {
> > - /* FIXME: add code to reset vcpu. */
> > + /* The initial reset call is needed here to reset in-kernel
> > + * vcpu data that we can't access directly from QEMU
> > + * (i.e. with older kernels which don't support sync_regs/ONE_REG).
> > + * Before this ioctl cpu_synchronize_state() is called in common kvm
> > + * code (kvm-all) */
> > + if (kvm_vcpu_ioctl(env, KVM_S390_INITIAL_RESET, NULL)) {
> > + perror("Can't reset vcpu\n");
> > + }
> > }
> >
> > int kvm_arch_put_registers(CPUS390XState *env, int level)
> > --
> > 1.7.12.4
> >
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler
2013-01-04 14:09 ` Jens Freimann
@ 2013-01-04 14:11 ` Alexander Graf
0 siblings, 0 replies; 26+ messages in thread
From: Alexander Graf @ 2013-01-04 14:11 UTC (permalink / raw)
To: Jens Freimann
Cc: Heinz Graalfs, qemu-devel, Andreas Faerber, Christian Borntraeger,
Cornelia Huck, Einar Lueck
On 04.01.2013, at 15:09, Jens Freimann wrote:
> On Thu, Jan 03, 2013 at 01:55:19PM +0100, Alexander Graf wrote:
>>
>> On 18.12.2012, at 18:50, Jens Freimann wrote:
>>
>>> Add a CPU reset handler to have all CPUs in a PoP compliant
>>> state.
>>>
>>> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
>>>
>>> ---
>>> v2 -> v3:
>>> * remove FIXME
>>> * separate parent reset from local reset by adding a while line
>>> * use defines for register reset values
>>>
>>> v1 -> v2:
>>> * move setting of control registers and psa to s390_cpu_reset
>>> and call it from the new s390_machine_cpu_reset_cb()
>>> This makes it more similar to how it is done on x86
>>> * in s390_cpu_reset() set env->halted state of cpu after
>>> the memset. This is needed to keep our s390_cpu_running
>>> counter in sync when s390_cpu_reset is called via the
>>> qemu_devices_reset path
>>> * set env->halted state in s390_cpu_initfn to 1 to avoid
>>> decrementing the cpu counter during first reset
>>> ---
>>> target-s390x/cpu.c | 35 +++++++++++++++++++++++++++++++++--
>>> target-s390x/kvm.c | 9 ++++++++-
>>> 2 files changed, 41 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
>>> index 619b202..58e412a 100644
>>> --- a/target-s390x/cpu.c
>>> +++ b/target-s390x/cpu.c
>>> @@ -4,6 +4,7 @@
>>> * Copyright (c) 2009 Ulrich Hecht
>>> * Copyright (c) 2011 Alexander Graf
>>> * Copyright (c) 2012 SUSE LINUX Products GmbH
>>> + * Copyright (c) 2012 IBM Corp.
>>> *
>>> * This library is free software; you can redistribute it and/or
>>> * modify it under the terms of the GNU Lesser General Public
>>> @@ -18,12 +19,19 @@
>>> * 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/lgpl-2.1.html>
>>> + * Contributions after 2012-12-11 are licensed under the terms of the
>>> + * GNU GPL, version 2 or (at your option) any later version.
>>> + *
>>> */
>>>
>>> #include "cpu.h"
>>> +#include "hw/hw.h"
>>
>> Also, have you verified that this doesn't break s390x-linux-user?
>
> I verified s390x-linux-user still builds.
>
>>> #include "qemu-common.h"
>>> #include "qemu-timer.h"
>>>
>>> +#define IPL_PSW_MASK 0x0000000180000000ULL
>>> +#define CR0_RESET 0xE0UL
>>> +#define CR14_RESET 0xC2000000UL;
>>>
>>> /* CPUClass::reset() */
>>> static void s390_cpu_reset(CPUState *s)
>>> @@ -37,12 +45,30 @@ static void s390_cpu_reset(CPUState *s)
>>> log_cpu_state(env, 0);
>>> }
>>>
>>> + s390_del_running_cpu(env);
>>> +
>>> scc->parent_reset(s);
>>>
>>> memset(env, 0, offsetof(CPUS390XState, breakpoints));
>>> - /* FIXME: reset vector? */
>>> +
>>> + /* architectured initial values for CR 0 and 14 */
>>> + env->cregs[0] = CR0_RESET;
>>> + env->cregs[14] = CR14_RESET;
>>> + /* set to z/Architecture mode */
>>> + env->psw.mask = IPL_PSW_MASK;
>>
>> In fact this one is correct for CONFIG_USER_ONLY.
>>
>>> + env->psa = 0;
>>> + /* set halted to 1 to make sure we can add the cpu in
>>> + * s390_ipl_cpu code, where env->halted is set back to 0
>>> + * after incrementing the cpu counter */
>>> + env->halted = 1;
>>
>> While this again probably breaks s390x-linux-user, no?
>
> It still builds fine, if that's what you mean? env->halted is
> not within an #ifdef !CONFIG_USER_ONLY clause.
Does it run? Or does the vcpu come up halted and never gets around to execute code?
Alex
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2013-01-04 14:11 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-18 17:50 [Qemu-devel] [PATCH 0/3] s390: ipl device, cpu reset handler and cpu model support Jens Freimann
2012-12-18 17:50 ` [Qemu-devel] [PATCH 1/3] s390: Move IPL code into a separate device Jens Freimann
2013-01-03 12:47 ` Alexander Graf
2012-12-18 17:50 ` [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler Jens Freimann
2013-01-03 12:50 ` Alexander Graf
2013-01-04 13:55 ` Jens Freimann
2013-01-03 12:55 ` Alexander Graf
2013-01-04 14:09 ` Jens Freimann
2013-01-04 14:11 ` Alexander Graf
2012-12-18 17:50 ` [Qemu-devel] [PATCH 3/3] S390: Enable -cpu help and QMP query-cpu-definitions Jens Freimann
2013-01-03 13:01 ` Alexander Graf
-- strict thread matches above, loose matches on Subject: below --
2012-12-14 16:46 [Qemu-devel] [PATCH 0/3] s390: ipl device, cpu reset handler and cpu model support Jens Freimann
2012-12-14 16:46 ` [Qemu-devel] [PATCH 3/3] S390: Enable -cpu help and QMP query-cpu-definitions Jens Freimann
2012-12-16 15:42 ` Andreas Färber
2012-12-17 14:47 ` Alexander Graf
2012-12-17 17:32 ` Andreas Färber
2012-12-12 13:08 [Qemu-devel] [PATCH 0/3] s390: ipl device, cpu reset handler and cpu model support Jens Freimann
2012-12-12 13:08 ` [Qemu-devel] [PATCH 3/3] S390: Enable -cpu help and QMP query-cpu-definitions Jens Freimann
2012-12-12 13:40 ` Alexander Graf
2012-12-12 13:51 ` Andreas Färber
2012-12-12 14:03 ` Alexander Graf
2012-12-12 14:57 ` Viktor Mihajlovski
2012-12-12 17:52 ` Richard Henderson
2012-12-12 18:25 ` Alexander Graf
2012-12-12 15:05 ` Viktor Mihajlovski
2012-12-12 15:50 ` Alexander Graf
2012-12-12 16:28 ` Andreas Färber
2012-12-12 18:23 ` Alexander Graf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).