[Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler

[Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler

[Jens Freimann]

Add a CPU reset handler to have all CPUs in a PoP compliant
state.

Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>

---
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 | 31 +++++++++++++++++++++++++++++--
 target-s390x/kvm.c |  9 ++++++++-
 2 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 619b202..75f60b3 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,9 +19,13 @@
  * 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"
 
@@ -37,12 +42,29 @@ static void s390_cpu_reset(CPUState *s)
         log_cpu_state(env, 0);
     }
 
-    scc->parent_reset(s);
+    s390_del_running_cpu(env);
 
+    scc->parent_reset(s);
     memset(env, 0, offsetof(CPUS390XState, breakpoints));
+
+    /* architectured initial values for CR 0 and 14 */
+    env->cregs[0] = 0xE0UL;
+    env->cregs[14] = 0xC2000000UL;
+    /* set to z/Architecture mode */
+    env->psw.mask = 0x0000000180000000ULL;
+    env->psa = 0;
+    /* set halted to 1 to make sure we can add the cpu in
+     * s390_ipl_cpu code */
+    env->halted = 1;
     /* FIXME: reset vector? */
     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 +88,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

Re: [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler

[Alexander Graf]

On 12.12.2012, at 14:08, 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>
> 
> ---
> 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 | 31 +++++++++++++++++++++++++++++--
> target-s390x/kvm.c |  9 ++++++++-
> 2 files changed, 37 insertions(+), 3 deletions(-)
> 
> diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> index 619b202..75f60b3 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,9 +19,13 @@
>  * 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"
> 
> @@ -37,12 +42,29 @@ static void s390_cpu_reset(CPUState *s)
>         log_cpu_state(env, 0);
>     }
> 
> -    scc->parent_reset(s);
> +    s390_del_running_cpu(env);
> 
> +    scc->parent_reset(s);
>     memset(env, 0, offsetof(CPUS390XState, breakpoints));
> +
> +    /* architectured initial values for CR 0 and 14 */
> +    env->cregs[0] = 0xE0UL;
> +    env->cregs[14] = 0xC2000000UL;
> +    /* set to z/Architecture mode */
> +    env->psw.mask = 0x0000000180000000ULL;
> +    env->psa = 0;
> +    /* set halted to 1 to make sure we can add the cpu in
> +     * s390_ipl_cpu code */
> +    env->halted = 1;

So who sets cpu0 to halted=0 again? Please add that as a comment here.


Alex

>     /* FIXME: reset vector? */
>     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 +88,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
> 

Re: [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler

[Jens Freimann]

On Wed, Dec 12, 2012 at 02:38:40PM +0100, Alexander Graf wrote:
> 
> On 12.12.2012, at 14:08, 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>
> > 
> > ---
> > 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 | 31 +++++++++++++++++++++++++++++--
> > target-s390x/kvm.c |  9 ++++++++-
> > 2 files changed, 37 insertions(+), 3 deletions(-)
> > 
> > diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> > index 619b202..75f60b3 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,9 +19,13 @@
> >  * 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"
> > 
> > @@ -37,12 +42,29 @@ static void s390_cpu_reset(CPUState *s)
> >         log_cpu_state(env, 0);
> >     }
> > 
> > -    scc->parent_reset(s);
> > +    s390_del_running_cpu(env);
> > 
> > +    scc->parent_reset(s);
> >     memset(env, 0, offsetof(CPUS390XState, breakpoints));
> > +
> > +    /* architectured initial values for CR 0 and 14 */
> > +    env->cregs[0] = 0xE0UL;
> > +    env->cregs[14] = 0xC2000000UL;
> > +    /* set to z/Architecture mode */
> > +    env->psw.mask = 0x0000000180000000ULL;
> > +    env->psa = 0;
> > +    /* set halted to 1 to make sure we can add the cpu in
> > +     * s390_ipl_cpu code */
> > +    env->halted = 1;
> 
> So who sets cpu0 to halted=0 again? Please add that as a comment here.

It is set to halted=0 by s390_add_running_cpu() in s390_ipl_cpu() for
cpu 0 and via the sigp restart path for all other cpus.
I will add a comment and send a new version. 
 
 Jens

> Alex
> 
> >     /* FIXME: reset vector? */
> >     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 +88,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
> > 
> 

-- 
Mit freundlichen Grüßen / Kind regards 

Jens Freimann 

-- 
IBM Linux Technology Center / Boeblingen lab
IBM Systems &Technology Group, Systems Software Development
-------------------------------------------------------------
IBM Deutschland
Schoenaicher Str 220
71032 Boeblingen
Phone: +49-7031-16 x1122
E-Mail: jfrei@de.ibm.com
-------------------------------------------------------------
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

[Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler

[Jens Freimann]

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: 
* explain in comment which code sets cpu 0 to running during IPL

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 | 32 ++++++++++++++++++++++++++++++--
 target-s390x/kvm.c |  9 ++++++++-
 2 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 619b202..75d4036 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,9 +19,13 @@
  * 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"
 
@@ -37,12 +42,30 @@ static void s390_cpu_reset(CPUState *s)
         log_cpu_state(env, 0);
     }
 
-    scc->parent_reset(s);
+    s390_del_running_cpu(env);
 
+    scc->parent_reset(s);
     memset(env, 0, offsetof(CPUS390XState, breakpoints));
+
+    /* architectured initial values for CR 0 and 14 */
+    env->cregs[0] = 0xE0UL;
+    env->cregs[14] = 0xC2000000UL;
+    /* set to z/Architecture mode */
+    env->psw.mask = 0x0000000180000000ULL;
+    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;
     /* FIXME: reset vector? */
     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 +89,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

Re: [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler

[Andreas Färber]

Am 14.12.2012 17:46, schrieb Jens Freimann:
> Add a CPU reset handler to have all CPUs in a PoP compliant
> state.
> 
> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>

The logic looks okay now. Some comments inline.

> ---
> v2 -> v3: 
> * explain in comment which code sets cpu 0 to running during IPL
> 
> 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 | 32 ++++++++++++++++++++++++++++++--
>  target-s390x/kvm.c |  9 ++++++++-
>  2 files changed, 38 insertions(+), 3 deletions(-)
> 
> diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> index 619b202..75d4036 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,9 +19,13 @@
>   * 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"
>  
> @@ -37,12 +42,30 @@ static void s390_cpu_reset(CPUState *s)
>          log_cpu_state(env, 0);
>      }
>  
> -    scc->parent_reset(s);
> +    s390_del_running_cpu(env);
>  
> +    scc->parent_reset(s);

If this gets respun, a white line separating the parent reset from the
local reset would be nice. :)

>      memset(env, 0, offsetof(CPUS390XState, breakpoints));
> +
> +    /* architectured initial values for CR 0 and 14 */
> +    env->cregs[0] = 0xE0UL;
> +    env->cregs[14] = 0xC2000000UL;
> +    /* set to z/Architecture mode */
> +    env->psw.mask = 0x0000000180000000ULL;
> +    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;
>      /* FIXME: reset vector? */

Do the above added cregs/psw/psa reset values resolve this FIXME? Or
does that refer to something different?

>      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 +89,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);

Since we register the reset handler in instance_init, we should
unregister it in a instance_finalize callback (uninitfn?). Since we do
not hot-unplug s390 CPUs yet to my knowledge, that could be done in a
follow-up.

(For x86 it it registered in the provisional realize function and not
unregistered lacking a matching unrealization mechanism today; elsewhere
reset registration is done in the machine.)

>  }
>  
>  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)
>  {

Note to Alex: In my upcoming KVM CPUState series, this argument type
changes to CPUState ...

> -    /* 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)) {

... but so does the argument for kvm_vcpu_ioctl(), so merging becomes a
trivial env -> cpu change.

> +        perror("Can't reset vcpu\n");
> +    }
>  }
>  
>  int kvm_arch_put_registers(CPUS390XState *env, int level)

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler

[Jens Freimann]

On Sun, Dec 16, 2012 at 04:30:21PM +0100, Andreas Färber wrote:
> Am 14.12.2012 17:46, schrieb Jens Freimann:
> > Add a CPU reset handler to have all CPUs in a PoP compliant
> > state.
> > 
> > Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
> 
> The logic looks okay now. Some comments inline.
> 
> > ---
> > v2 -> v3: 
> > * explain in comment which code sets cpu 0 to running during IPL
> > 
> > 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 | 32 ++++++++++++++++++++++++++++++--
> >  target-s390x/kvm.c |  9 ++++++++-
> >  2 files changed, 38 insertions(+), 3 deletions(-)
> > 
> > diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> > index 619b202..75d4036 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,9 +19,13 @@
> >   * 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"
> >  
> > @@ -37,12 +42,30 @@ static void s390_cpu_reset(CPUState *s)
> >          log_cpu_state(env, 0);
> >      }
> >  
> > -    scc->parent_reset(s);
> > +    s390_del_running_cpu(env);
> >  
> > +    scc->parent_reset(s);
> 
> If this gets respun, a white line separating the parent reset from the
> local reset would be nice. :)

Ok

> >      memset(env, 0, offsetof(CPUS390XState, breakpoints));
> > +
> > +    /* architectured initial values for CR 0 and 14 */
> > +    env->cregs[0] = 0xE0UL;
> > +    env->cregs[14] = 0xC2000000UL;
> > +    /* set to z/Architecture mode */
> > +    env->psw.mask = 0x0000000180000000ULL;
> > +    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;
> >      /* FIXME: reset vector? */
> 
> Do the above added cregs/psw/psa reset values resolve this FIXME? Or
> does that refer to something different?

Yes, together with the ipl device this fixme is resolved.

> >      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 +89,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);
> 
> Since we register the reset handler in instance_init, we should
> unregister it in a instance_finalize callback (uninitfn?). Since we do
> not hot-unplug s390 CPUs yet to my knowledge, that could be done in a
> follow-up.
> 
> (For x86 it it registered in the provisional realize function and not
> unregistered lacking a matching unrealization mechanism today; elsewhere
> reset registration is done in the machine.)

Ok, I'll keep that in mind and send a followup patch.

Thanks!
Jens

> 
> >  }
> >  
> >  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)
> >  {
> 
> Note to Alex: In my upcoming KVM CPUState series, this argument type
> changes to CPUState ...
> 
> > -    /* 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)) {
> 
> ... but so does the argument for kvm_vcpu_ioctl(), so merging becomes a
> trivial env -> cpu change.
> 
> > +        perror("Can't reset vcpu\n");
> > +    }
> >  }
> >  
> >  int kvm_arch_put_registers(CPUS390XState *env, int level)
> 
> Andreas
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
> 

-- 
Mit freundlichen Grüßen / Kind regards 

Jens Freimann 

-- 
IBM Linux Technology Center / Boeblingen lab
IBM Systems &Technology Group, Systems Software Development
-------------------------------------------------------------
IBM Deutschland
Schoenaicher Str 220
71032 Boeblingen
Phone: +49-7031-16 x1122
E-Mail: jfrei@de.ibm.com
-------------------------------------------------------------
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

Re: [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler

[Alexander Graf]

On 14.12.2012, at 17:46, 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: 
> * explain in comment which code sets cpu 0 to running during IPL
> 
> 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 | 32 ++++++++++++++++++++++++++++++--
> target-s390x/kvm.c |  9 ++++++++-
> 2 files changed, 38 insertions(+), 3 deletions(-)
> 
> diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> index 619b202..75d4036 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,9 +19,13 @@
>  * 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"
> 
> @@ -37,12 +42,30 @@ static void s390_cpu_reset(CPUState *s)
>         log_cpu_state(env, 0);
>     }
> 
> -    scc->parent_reset(s);
> +    s390_del_running_cpu(env);
> 
> +    scc->parent_reset(s);
>     memset(env, 0, offsetof(CPUS390XState, breakpoints));

Shouldn't parent_reset already do the memset?

> +
> +    /* architectured initial values for CR 0 and 14 */
> +    env->cregs[0] = 0xE0UL;
> +    env->cregs[14] = 0xC2000000UL;
> +    /* set to z/Architecture mode */
> +    env->psw.mask = 0x0000000180000000ULL;

While at it, please convert this into something that uses #define's to make things readable.


Alex

> +    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;
>     /* FIXME: reset vector? */
>     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 +89,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
> 

Re: [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler

[Jens Freimann]

On Mon, Dec 17, 2012 at 03:49:41PM +0100, Alexander Graf wrote:
> 
> On 14.12.2012, at 17:46, 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: 
> > * explain in comment which code sets cpu 0 to running during IPL
> > 
> > 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 | 32 ++++++++++++++++++++++++++++++--
> > target-s390x/kvm.c |  9 ++++++++-
> > 2 files changed, 38 insertions(+), 3 deletions(-)
> > 
> > diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> > index 619b202..75d4036 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,9 +19,13 @@
> >  * 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"
> > 
> > @@ -37,12 +42,30 @@ static void s390_cpu_reset(CPUState *s)
> >         log_cpu_state(env, 0);
> >     }
> > 
> > -    scc->parent_reset(s);
> > +    s390_del_running_cpu(env);
> > 
> > +    scc->parent_reset(s);
> >     memset(env, 0, offsetof(CPUS390XState, breakpoints));
> 
> Shouldn't parent_reset already do the memset?

parent_reset is cpu_common_reset() in qom/cpu.c and that
is an empty function as of now. It's what ARM and x86 do as well.

> > +
> > +    /* architectured initial values for CR 0 and 14 */
> > +    env->cregs[0] = 0xE0UL;
> > +    env->cregs[14] = 0xC2000000UL;
> > +    /* set to z/Architecture mode */
> > +    env->psw.mask = 0x0000000180000000ULL;
> 
> While at it, please convert this into something that uses #define's to make things readable.

ok

Jens

> 
> Alex
> 
> > +    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;
> >     /* FIXME: reset vector? */
> >     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 +89,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
> > 
> 

-- 
Mit freundlichen Grüßen / Kind regards 

Jens Freimann 

-- 
IBM Linux Technology Center / Boeblingen lab
IBM Systems &Technology Group, Systems Software Development
-------------------------------------------------------------
IBM Deutschland
Schoenaicher Str 220
71032 Boeblingen
Phone: +49-7031-16 x1122
E-Mail: jfrei@de.ibm.com
-------------------------------------------------------------
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

Re: [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler

[Andreas Färber]

Am 17.12.2012 15:49, schrieb Alexander Graf:
> 
> On 14.12.2012, at 17:46, Jens Freimann wrote:
> 
>> @@ -37,12 +42,30 @@ static void s390_cpu_reset(CPUState *s)
>>         log_cpu_state(env, 0);
>>     }
>>
>> -    scc->parent_reset(s);
>> +    s390_del_running_cpu(env);
>>
>> +    scc->parent_reset(s);
>>     memset(env, 0, offsetof(CPUS390XState, breakpoints));
> 
> Shouldn't parent_reset already do the memset?

No, because "env" location and size are specific to S390CPU.

And yes, it is ugly boilerplate code, but it cannot be solved with my
CPU_COMMON field movements alone (which partially add explicit reset
code based on the field location), there's quite a large number of
per-target fields that get reset that way, some intentionally, some
accidentally. ;-)

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler

[Alexander Graf]

On 17.12.2012, at 18:21, Andreas Färber wrote:

> Am 17.12.2012 15:49, schrieb Alexander Graf:
>> 
>> On 14.12.2012, at 17:46, Jens Freimann wrote:
>> 
>>> @@ -37,12 +42,30 @@ static void s390_cpu_reset(CPUState *s)
>>>        log_cpu_state(env, 0);
>>>    }
>>> 
>>> -    scc->parent_reset(s);
>>> +    s390_del_running_cpu(env);
>>> 
>>> +    scc->parent_reset(s);
>>>    memset(env, 0, offsetof(CPUS390XState, breakpoints));
>> 
>> Shouldn't parent_reset already do the memset?
> 
> No, because "env" location and size are specific to S390CPU.
> 
> And yes, it is ugly boilerplate code, but it cannot be solved with my
> CPU_COMMON field movements alone (which partially add explicit reset
> code based on the field location), there's quite a large number of
> per-target fields that get reset that way, some intentionally, some
> accidentally. ;-)

I see :)


Alex

[Qemu-devel] [PATCH 0/3] s390: ipl device, cpu reset handler and cpu model support

[Jens Freimann]

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

[Qemu-devel] [PATCH 1/3] s390: Move IPL code into a separate device

[Jens Freimann]

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

[Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler

[Jens Freimann]

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

[Qemu-devel] [PATCH 3/3] S390: Enable -cpu help and QMP query-cpu-definitions

[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 | 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

Re: [Qemu-devel] [PATCH 1/3] s390: Move IPL code into a separate device

[Alexander Graf]

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

Re: [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler

[Alexander Graf]

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

Re: [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler

[Alexander Graf]

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
> 

Re: [Qemu-devel] [PATCH 3/3] S390: Enable -cpu help and QMP query-cpu-definitions

[Alexander Graf]

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

Re: [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler

[Jens Freimann]

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
> 

Re: [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler

[Jens Freimann]

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
> > 
> 

Re: [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler

[Alexander Graf]

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