[Qemu-devel] [PATCH v1 00/27] s390x: SMP for TCG (+ cleanups)

[Qemu-devel] [PATCH v1 00/27] s390x: SMP for TCG (+ cleanups)

[David Hildenbrand]

This series contains:
- properly implement local external interrupts for TCG
- factor out KVM SIGP handling code into common code
- implement missing SIGP orders for TCG handled by the kernel for KVM
  (including STOP and RESTART interrupts)
- make TCG use the new SIGP code - experimental SMP support for s390x TCG
- refactor STFL(E) implementation for TCG
- bunch of cleanups

Basically all SIGP instructions are fully supported.

Thanks to Aurelien Jarno for the initital prototype and showcasing that
supporting experimental SMP code can be implemented quite easily.

TCG SMP on s390x - what works?
- "-smp X,maxcpus=X" with both, single and multi threaded TCG
- "-smp ... -device qemu-s390-cpu,id=cpuX,core-id=X"
- system_powerdown, system_reset, shutdown, reboot, NMI
- online/offline of CPUs from inside the guest

TCG SMP on s390x - what does not work?
- Floating interrupts all target CPU 0. Don't offline it.
- CPU hotplug after the machine/main loop has been fully setup
-- the new CPU comes up, answers and sends emergency signals, but suddenly
   the VM gets stuck. This is strange, as ordinary online/offline works
   just fine.
-- can be triggered by "cpu-add 1" + "system_reset". The system will hang
   when trying to online CPUs. (note: in Linux code they are fully up and
   running and already executed code)
-- also if hotplugging with "-S", before anything has run. This is strange,
   as "-device qemu-s390-cpu" works just fine.
-- does not seem to be related to CPU setup/reset code, I checked that
-- seems to be related to some TCG internals (as broken for single and
   multi threaded TCG).
-- common code seems to be somehow broken, not sure if this is even
   expected to work (e.g. for single threaded TCG, hotplugged CPUs will
   never get set "cpu->created = true". But doesn't seem to be related to
   this)


Based on: https://github.com/cohuck/qemu.git s390-next
Available on: git@github.com:davidhildenbrand/qemu.git s390x-queue


David Hildenbrand (27):
  s390x: raise CPU hotplug irq after really hotplugged
  s390x/cpumodel: fix max STFL(E) bit number
  target/s390x: get rid of next_core_id
  s390x: introduce and use S390_MAX_CPUS
  s390/tcg: turn INTERRUPT_EXT into a mask
  s390x/tcg: injection of emergency signals and extarnal calls
  s390x/tcg: STOPPED cpus can never wake up
  s390x/tcg: a CPU cannot switch state due to an interrupt
  target/s390x: factor out handling of WAIT PSW into handle_wait()
  s390x/kvm: pass ipb directly into handle_sigp()
  s390x/kvm: generalize SIGP stop and restart interrupt injection
  s390x/kvm: factor out storing of CPU status
  target/s390x: proper cpu->be convertion in s390_store_status()
  s390x/kvm: factor out storing of adtl CPU status
  s390x/kvm: drop two debug prints
  s390x/kvm: factor out SIGP code into sigp.c
  s390x/kvm: factor out actual handling of STOP interrupts
  s390x/tcg: implement SIGP SENSE RUNNING STATUS
  s390x/tcg: implement SIGP SENSE
  s390x/tcg: implement SIGP EXTERNAL CALL
  s390x/tcg: implement SIGP EMERGENCY SIGNAL
  s390x/tcg: implement SIGP CONDITIONAL EMERGENCY SIGNAL
  s390x/tcg: implement STOP and RESET interrupts for TCG
  s390x/tcg: flush the tlb on SIGP SET PREFIX
  s390x/tcg: switch to new SIGP handling code
  s390x/tcg: unlock NMI
  s390x/tcg: refactor stfl(e) to use s390_get_feat_block()

 hw/s390x/s390-virtio-ccw.c  |  17 +-
 target/s390x/Makefile.objs  |   1 +
 target/s390x/cpu-qom.h      |   2 -
 target/s390x/cpu.c          |  40 ++--
 target/s390x/cpu.h          |  36 +++-
 target/s390x/cpu_features.c |   2 +-
 target/s390x/cpu_models.c   |   2 +
 target/s390x/excp_helper.c  |  98 ++++++---
 target/s390x/helper.c       | 115 ++++++++--
 target/s390x/helper.h       |   4 +-
 target/s390x/internal.h     |  15 ++
 target/s390x/interrupt.c    |  70 +++++-
 target/s390x/kvm-stub.c     |  13 +-
 target/s390x/kvm.c          | 470 +++--------------------------------------
 target/s390x/kvm_s390x.h    |   3 +-
 target/s390x/misc_helper.c  | 114 ++++------
 target/s390x/sigp.c         | 504 ++++++++++++++++++++++++++++++++++++++++++++
 target/s390x/trace-events   |   4 +-
 target/s390x/translate.c    |   6 +-
 19 files changed, 896 insertions(+), 620 deletions(-)
 create mode 100644 target/s390x/sigp.c

-- 
2.13.5

[Qemu-devel] [PATCH v1 01/27] s390x: raise CPU hotplug irq after really hotplugged

[David Hildenbrand]

Let's move it into the machine, so we trigger the IRQ after setting
ms->possible_cpus (which SCLP uses to construct the list of
online CPUs).

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 hw/s390x/s390-virtio-ccw.c | 4 ++++
 target/s390x/cpu.c         | 8 --------
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 0471407187..d5a12a2472 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -327,6 +327,10 @@ static void s390_cpu_plug(HotplugHandler *hotplug_dev,
 
     g_assert(!ms->possible_cpus->cpus[cpu->env.core_id].cpu);
     ms->possible_cpus->cpus[cpu->env.core_id].cpu = OBJECT(dev);
+
+    if (dev->hotplugged) {
+        raise_irq_cpu_hotplug();
+    }
 }
 
 static void s390_machine_reset(void)
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index 34538c3ab9..4920da3625 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -41,7 +41,6 @@
 #include "hw/hw.h"
 #include "sysemu/arch_init.h"
 #include "sysemu/sysemu.h"
-#include "hw/s390x/sclp.h"
 #endif
 
 #define CR0_RESET       0xE0UL
@@ -227,13 +226,6 @@ static void s390_cpu_realizefn(DeviceState *dev, Error **errp)
 #endif
 
     scc->parent_realize(dev, &err);
-
-#if !defined(CONFIG_USER_ONLY)
-    if (dev->hotplugged) {
-        raise_irq_cpu_hotplug();
-    }
-#endif
-
 out:
     error_propagate(errp, err);
 }
-- 
2.13.5

[Qemu-devel] [PATCH v1 02/27] s390x/cpumodel: fix max STFL(E) bit number

[David Hildenbrand]

Not that it would matter in the near future, but it is actually 2048
bytes, therefore 16384 possible bits.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/cpu_features.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
index 1d3a036393..31a4676f05 100644
--- a/target/s390x/cpu_features.c
+++ b/target/s390x/cpu_features.c
@@ -381,7 +381,7 @@ void s390_add_from_feat_block(S390FeatBitmap features, S390FeatType type,
 
     switch (type) {
     case S390_FEAT_TYPE_STFL:
-       nr_bits = 2048;
+       nr_bits = 16384;
        break;
     case S390_FEAT_TYPE_PLO:
        nr_bits = 256;
-- 
2.13.5

[Qemu-devel] [PATCH v1 03/27] target/s390x: get rid of next_core_id

[David Hildenbrand]

core_id is not needed by linux-user, as the core_id a.k.a. CPU address
is only accessible from kernel space.

Therefore, drop next_core_id and make cpu_index get autoassigned again
for linux-user.

While at it, shield core_id and cpuid completely from linux-user. cpuid
can also only be queried from kernel space.

Suggested-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/cpu-qom.h    |  2 --
 target/s390x/cpu.c        | 14 +++++++-------
 target/s390x/cpu.h        |  2 ++
 target/s390x/cpu_models.c |  2 ++
 4 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/target/s390x/cpu-qom.h b/target/s390x/cpu-qom.h
index 2e446fab51..0510c49e07 100644
--- a/target/s390x/cpu-qom.h
+++ b/target/s390x/cpu-qom.h
@@ -54,8 +54,6 @@ typedef struct S390CPUClass {
     bool is_migration_safe;
     const char *desc;
 
-    uint32_t next_core_id;
-
     DeviceRealize parent_realize;
     void (*parent_reset)(CPUState *cpu);
     void (*load_normal)(CPUState *cpu);
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index 4920da3625..f42e28ea25 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -178,8 +178,9 @@ static void s390_cpu_realizefn(DeviceState *dev, Error **errp)
 {
     CPUState *cs = CPU(dev);
     S390CPUClass *scc = S390_CPU_GET_CLASS(dev);
+#if !defined(CONFIG_USER_ONLY)
     S390CPU *cpu = S390_CPU(dev);
-    CPUS390XState *env = &cpu->env;
+#endif
     Error *err = NULL;
 
     /* the model has to be realized before qemu_init_vcpu() due to kvm */
@@ -195,11 +196,6 @@ static void s390_cpu_realizefn(DeviceState *dev, Error **errp)
                    max_cpus - 1);
         goto out;
     }
-#else
-    /* implicitly set for linux-user only */
-    cpu->env.core_id = scc->next_core_id;
-    scc->next_core_id++;
-#endif
 
     if (cpu_exists(cpu->env.core_id)) {
         error_setg(&err, "Unable to add CPU with core-id: %" PRIu32
@@ -208,7 +204,9 @@ static void s390_cpu_realizefn(DeviceState *dev, Error **errp)
     }
 
     /* sync cs->cpu_index and env->core_id. The latter is needed for TCG. */
-    cs->cpu_index = env->core_id;
+    cs->cpu_index = cpu->env.core_id;
+#endif
+
     cpu_exec_realizefn(cs, &err);
     if (err != NULL) {
         goto out;
@@ -440,7 +438,9 @@ static gchar *s390_gdb_arch_name(CPUState *cs)
 }
 
 static Property s390x_cpu_properties[] = {
+#if !defined(CONFIG_USER_ONLY)
     DEFINE_PROP_UINT32("core-id", S390CPU, env.core_id, 0),
+#endif
     DEFINE_PROP_END_OF_LIST()
 };
 
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 9b549dc491..f13aea3dd8 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -150,8 +150,10 @@ struct CPUS390XState {
 
     CPU_COMMON
 
+#if !defined(CONFIG_USER_ONLY)
     uint32_t core_id; /* PoP "CPU address", same as cpu_index */
     uint64_t cpuid;
+#endif
 
     uint64_t tod_offset;
     uint64_t tod_basetime;
diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 5169379db5..191aee6dd4 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -941,11 +941,13 @@ void s390_realize_cpu_model(CPUState *cs, Error **errp)
 
     apply_cpu_model(cpu->model, errp);
 
+#if !defined(CONFIG_USER_ONLY)
     cpu->env.cpuid = s390_cpuid_from_cpu_model(cpu->model);
     if (tcg_enabled()) {
         /* basic mode, write the cpu address into the first 4 bit of the ID */
         cpu->env.cpuid = deposit64(cpu->env.cpuid, 54, 4, cpu->env.core_id);
     }
+#endif
 }
 
 static void get_feature(Object *obj, Visitor *v, const char *name,
-- 
2.13.5

[Qemu-devel] [PATCH v1 04/27] s390x: introduce and use S390_MAX_CPUS

[David Hildenbrand]

Will be handy in the next patches.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 hw/s390x/s390-virtio-ccw.c | 2 +-
 target/s390x/cpu.h         | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index d5a12a2472..a57d435cbf 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -443,7 +443,7 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data)
     mc->no_parallel = 1;
     mc->no_sdcard = 1;
     mc->use_sclp = 1;
-    mc->max_cpus = 248;
+    mc->max_cpus = S390_MAX_CPUS;
     mc->has_hotpluggable_cpus = true;
     mc->get_hotplug_handler = s390_get_hotplug_handler;
     mc->cpu_index_to_instance_props = s390_cpu_index_to_props;
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index f13aea3dd8..5f539b1057 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -59,6 +59,8 @@
 #define PSW_MCHK_MASK 0x0004000000000000
 #define PSW_IO_MASK 0x0200000000000000
 
+#define S390_MAX_CPUS 248
+
 typedef struct PSW {
     uint64_t mask;
     uint64_t addr;
-- 
2.13.5

[Qemu-devel] [PATCH v1 05/27] s390/tcg: turn INTERRUPT_EXT into a mask

[David Hildenbrand]

External interrupts are currently all handled like floating external
interrupts, they are queued. Let's prepare for a split of floating
and local interrupts by turning INTERRUPT_EXT into a mask.

While we can have various floating external interrupts of one kind, there
is usually only one (or a fixed number) of the local external interrupts.

So turn INTERRUPT_EXT into a mask and properly indicate the kind of
external interrupt. Floating interrupts will have to moved out of
one CPU instance later once we have SMP support.

This get's rid of the ugly special handling for cpu timer and clock
comparator interrupts. And we really only store the parameters as
defined by the PoP.

We'll introduce external calls and emergency signals in the following
patches.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/cpu.h         | 13 ++++++----
 target/s390x/excp_helper.c | 65 ++++++++++++++++++++++++----------------------
 target/s390x/helper.c      | 12 ++-------
 target/s390x/internal.h    |  2 ++
 target/s390x/interrupt.c   | 18 ++++++++++++-
 5 files changed, 63 insertions(+), 47 deletions(-)

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 5f539b1057..d4e2aa2f24 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -400,11 +400,14 @@ static inline void cpu_get_tb_cpu_state(CPUS390XState* env, target_ulong *pc,
 #define EXCP_IO  7 /* I/O interrupt */
 #define EXCP_MCHK 8 /* machine check */
 
-#define INTERRUPT_EXT        (1 << 0)
-#define INTERRUPT_TOD        (1 << 1)
-#define INTERRUPT_CPUTIMER   (1 << 2)
-#define INTERRUPT_IO         (1 << 3)
-#define INTERRUPT_MCHK       (1 << 4)
+#define INTERRUPT_IO                     (1 << 0)
+#define INTERRUPT_MCHK                   (1 << 1)
+#define INTERRUPT_EXT_FLOATING           (1 << 2)
+#define INTERRUPT_EXT_CPU_TIMER          (1 << 3)
+#define INTERRUPT_EXT_CLOCK_COMPARATOR   (1 << 4)
+#define INTERRUPT_EXT                    (INTERRUPT_EXT_FLOATING | \
+                                          INTERRUPT_EXT_CPU_TIMER | \
+                                          INTERRUPT_EXT_CLOCK_COMPARATOR)
 
 /* Program Status Word.  */
 #define S390_PSWM_REGNUM 0
diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
index 470cf8f5bc..46d015f260 100644
--- a/target/s390x/excp_helper.c
+++ b/target/s390x/excp_helper.c
@@ -238,29 +238,44 @@ static void do_ext_interrupt(CPUS390XState *env)
         cpu_abort(CPU(cpu), "Ext int w/o ext mask\n");
     }
 
-    if (env->ext_index < 0 || env->ext_index >= MAX_EXT_QUEUE) {
-        cpu_abort(CPU(cpu), "Ext queue overrun: %d\n", env->ext_index);
-    }
-
-    q = &env->ext_queue[env->ext_index];
     lowcore = cpu_map_lowcore(env);
 
-    lowcore->ext_int_code = cpu_to_be16(q->code);
-    lowcore->ext_params = cpu_to_be32(q->param);
-    lowcore->ext_params2 = cpu_to_be64(q->param64);
-    lowcore->external_old_psw.mask = cpu_to_be64(get_psw_mask(env));
-    lowcore->external_old_psw.addr = cpu_to_be64(env->psw.addr);
-    lowcore->cpu_addr = cpu_to_be16(env->core_id | VIRTIO_SUBCODE_64);
+    if (env->pending_int & INTERRUPT_EXT_CLOCK_COMPARATOR) {
+        lowcore->ext_int_code = cpu_to_be16(EXT_CLOCK_COMP);
+        lowcore->cpu_addr = 0;
+        env->pending_int &= ~INTERRUPT_EXT_CLOCK_COMPARATOR;
+    } else if (env->pending_int & INTERRUPT_EXT_CPU_TIMER) {
+        lowcore->ext_int_code = cpu_to_be16(EXT_CPU_TIMER);
+        lowcore->cpu_addr = 0;
+        env->pending_int &= ~INTERRUPT_EXT_CPU_TIMER;
+    } else if (env->pending_int | INTERRUPT_EXT_FLOATING) {
+        g_assert(env->ext_index >= 0);
+        /*
+         * FIXME: floating IRQs should be considered by all CPUs and
+         *        shuld not get cleared by CPU reset.
+         */
+        q = &env->ext_queue[env->ext_index];
+        lowcore->ext_int_code = cpu_to_be16(q->code);
+        lowcore->ext_params = cpu_to_be32(q->param);
+        lowcore->ext_params2 = cpu_to_be64(q->param64);
+        lowcore->external_old_psw.mask = cpu_to_be64(get_psw_mask(env));
+        lowcore->external_old_psw.addr = cpu_to_be64(env->psw.addr);
+        lowcore->cpu_addr = cpu_to_be16(env->core_id | VIRTIO_SUBCODE_64);
+        env->ext_index--;
+        if (env->ext_index == -1) {
+            env->pending_int &= ~INTERRUPT_EXT_FLOATING;
+        }
+    } else {
+        g_assert_not_reached();
+    }
+
     mask = be64_to_cpu(lowcore->external_new_psw.mask);
     addr = be64_to_cpu(lowcore->external_new_psw.addr);
+    lowcore->external_old_psw.mask = cpu_to_be64(get_psw_mask(env));
+    lowcore->external_old_psw.addr = cpu_to_be64(env->psw.addr);
 
     cpu_unmap_lowcore(lowcore);
 
-    env->ext_index--;
-    if (env->ext_index == -1) {
-        env->pending_int &= ~INTERRUPT_EXT;
-    }
-
     DPRINTF("%s: %" PRIx64 " %" PRIx64 "\n", __func__,
             env->psw.mask, env->psw.addr);
 
@@ -413,21 +428,9 @@ void s390_cpu_do_interrupt(CPUState *cs)
     }
     /* handle external interrupts */
     if ((env->psw.mask & PSW_MASK_EXT) &&
-        cs->exception_index == -1) {
-        if (env->pending_int & INTERRUPT_EXT) {
-            /* code is already in env */
-            cs->exception_index = EXCP_EXT;
-        } else if (env->pending_int & INTERRUPT_TOD) {
-            cpu_inject_ext(cpu, 0x1004, 0, 0);
-            cs->exception_index = EXCP_EXT;
-            env->pending_int &= ~INTERRUPT_EXT;
-            env->pending_int &= ~INTERRUPT_TOD;
-        } else if (env->pending_int & INTERRUPT_CPUTIMER) {
-            cpu_inject_ext(cpu, 0x1005, 0, 0);
-            cs->exception_index = EXCP_EXT;
-            env->pending_int &= ~INTERRUPT_EXT;
-            env->pending_int &= ~INTERRUPT_TOD;
-        }
+        cs->exception_index == -1 &&
+        (env->pending_int & INTERRUPT_EXT)) {
+        cs->exception_index = EXCP_EXT;
     }
     /* handle I/O interrupts */
     if ((env->psw.mask & PSW_MASK_IO) &&
diff --git a/target/s390x/helper.c b/target/s390x/helper.c
index 97adbcc86d..e22b93258b 100644
--- a/target/s390x/helper.c
+++ b/target/s390x/helper.c
@@ -51,20 +51,12 @@
 #ifndef CONFIG_USER_ONLY
 void s390x_tod_timer(void *opaque)
 {
-    S390CPU *cpu = opaque;
-    CPUS390XState *env = &cpu->env;
-
-    env->pending_int |= INTERRUPT_TOD;
-    cpu_interrupt(CPU(cpu), CPU_INTERRUPT_HARD);
+    cpu_inject_clock_comparator((S390CPU *) opaque);
 }
 
 void s390x_cpu_timer(void *opaque)
 {
-    S390CPU *cpu = opaque;
-    CPUS390XState *env = &cpu->env;
-
-    env->pending_int |= INTERRUPT_CPUTIMER;
-    cpu_interrupt(CPU(cpu), CPU_INTERRUPT_HARD);
+    cpu_inject_cpu_timer((S390CPU *) opaque);
 }
 #endif
 
diff --git a/target/s390x/internal.h b/target/s390x/internal.h
index bc8f83129a..ae4b27a574 100644
--- a/target/s390x/internal.h
+++ b/target/s390x/internal.h
@@ -348,6 +348,8 @@ void cpu_unmap_lowcore(LowCore *lowcore);
 void trigger_pgm_exception(CPUS390XState *env, uint32_t code, uint32_t ilen);
 void cpu_inject_ext(S390CPU *cpu, uint32_t code, uint32_t param,
                     uint64_t param64);
+void cpu_inject_clock_comparator(S390CPU *cpu);
+void cpu_inject_cpu_timer(S390CPU *cpu);
 
 
 /* ioinst.c */
diff --git a/target/s390x/interrupt.c b/target/s390x/interrupt.c
index 058e219fe5..4ba33c49cb 100644
--- a/target/s390x/interrupt.c
+++ b/target/s390x/interrupt.c
@@ -71,7 +71,23 @@ void cpu_inject_ext(S390CPU *cpu, uint32_t code, uint32_t param,
     env->ext_queue[env->ext_index].param = param;
     env->ext_queue[env->ext_index].param64 = param64;
 
-    env->pending_int |= INTERRUPT_EXT;
+    env->pending_int |= INTERRUPT_EXT_FLOATING;
+    cpu_interrupt(CPU(cpu), CPU_INTERRUPT_HARD);
+}
+
+void cpu_inject_clock_comparator(S390CPU *cpu)
+{
+    CPUS390XState *env = &cpu->env;
+
+    env->pending_int |= INTERRUPT_EXT_CLOCK_COMPARATOR;
+    cpu_interrupt(CPU(cpu), CPU_INTERRUPT_HARD);
+}
+
+void cpu_inject_cpu_timer(S390CPU *cpu)
+{
+    CPUS390XState *env = &cpu->env;
+
+    env->pending_int |= INTERRUPT_EXT_CPU_TIMER;
     cpu_interrupt(CPU(cpu), CPU_INTERRUPT_HARD);
 }
 
-- 
2.13.5

[Qemu-devel] [PATCH v1 06/27] s390x/tcg: injection of emergency signals and extarnal calls

[David Hildenbrand]

Preparation for new TCG SIGP code. Especially also prepare for
indicating that another external call is already pending.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/cpu.h         |  8 +++++++-
 target/s390x/excp_helper.c | 14 ++++++++++++++
 target/s390x/internal.h    |  2 ++
 target/s390x/interrupt.c   | 26 ++++++++++++++++++++++++++
 4 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index d4e2aa2f24..857af21d06 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -132,6 +132,8 @@ struct CPUS390XState {
     MchkQueue mchk_queue[MAX_MCHK_QUEUE];
 
     int pending_int;
+    uint16_t external_call_addr;
+    DECLARE_BITMAP(emergency_signals, S390_MAX_CPUS);
     int ext_index;
     int io_index[8];
     int mchk_index;
@@ -405,9 +407,13 @@ static inline void cpu_get_tb_cpu_state(CPUS390XState* env, target_ulong *pc,
 #define INTERRUPT_EXT_FLOATING           (1 << 2)
 #define INTERRUPT_EXT_CPU_TIMER          (1 << 3)
 #define INTERRUPT_EXT_CLOCK_COMPARATOR   (1 << 4)
+#define INTERRUPT_EXTERNAL_CALL          (1 << 5)
+#define INTERRUPT_EMERGENCY_SIGNAL       (1 << 6)
 #define INTERRUPT_EXT                    (INTERRUPT_EXT_FLOATING | \
                                           INTERRUPT_EXT_CPU_TIMER | \
-                                          INTERRUPT_EXT_CLOCK_COMPARATOR)
+                                          INTERRUPT_EXT_CLOCK_COMPARATOR | \
+                                          INTERRUPT_EXTERNAL_CALL | \
+                                          INTERRUPT_EMERGENCY_SIGNAL)
 
 /* Program Status Word.  */
 #define S390_PSWM_REGNUM 0
diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
index 46d015f260..ff257ba72e 100644
--- a/target/s390x/excp_helper.c
+++ b/target/s390x/excp_helper.c
@@ -231,6 +231,7 @@ static void do_ext_interrupt(CPUS390XState *env)
 {
     S390CPU *cpu = s390_env_get_cpu(env);
     uint64_t mask, addr;
+    uint16_t cpu_addr;
     LowCore *lowcore;
     ExtQueue *q;
 
@@ -248,6 +249,19 @@ static void do_ext_interrupt(CPUS390XState *env)
         lowcore->ext_int_code = cpu_to_be16(EXT_CPU_TIMER);
         lowcore->cpu_addr = 0;
         env->pending_int &= ~INTERRUPT_EXT_CPU_TIMER;
+    } else if (env->pending_int & INTERRUPT_EMERGENCY_SIGNAL) {
+        lowcore->ext_int_code = cpu_to_be16(EXT_EMERGENCY);
+        cpu_addr = find_first_bit(env->emergency_signals, S390_MAX_CPUS);
+        g_assert(cpu_addr < S390_MAX_CPUS);
+        lowcore->cpu_addr = cpu_to_be16(cpu_addr);
+        clear_bit(cpu_addr, env->emergency_signals);
+        if (bitmap_empty(env->emergency_signals, max_cpus)) {
+            env->pending_int &= ~INTERRUPT_EMERGENCY_SIGNAL;
+        }
+    } else if (env->pending_int & INTERRUPT_EXTERNAL_CALL) {
+        lowcore->ext_int_code = cpu_to_be16(EXT_EXTERNAL_CALL);
+        lowcore->cpu_addr = cpu_to_be16(env->external_call_addr);
+        env->pending_int &= ~INTERRUPT_EXTERNAL_CALL;
     } else if (env->pending_int | INTERRUPT_EXT_FLOATING) {
         g_assert(env->ext_index >= 0);
         /*
diff --git a/target/s390x/internal.h b/target/s390x/internal.h
index ae4b27a574..15743ec40f 100644
--- a/target/s390x/internal.h
+++ b/target/s390x/internal.h
@@ -350,6 +350,8 @@ void cpu_inject_ext(S390CPU *cpu, uint32_t code, uint32_t param,
                     uint64_t param64);
 void cpu_inject_clock_comparator(S390CPU *cpu);
 void cpu_inject_cpu_timer(S390CPU *cpu);
+void cpu_inject_emergency_signal(S390CPU *cpu, uint16_t src_cpu_addr);
+int cpu_inject_external_call(S390CPU *cpu, uint16_t src_cpu_addr);
 
 
 /* ioinst.c */
diff --git a/target/s390x/interrupt.c b/target/s390x/interrupt.c
index 4ba33c49cb..462316be8e 100644
--- a/target/s390x/interrupt.c
+++ b/target/s390x/interrupt.c
@@ -91,6 +91,32 @@ void cpu_inject_cpu_timer(S390CPU *cpu)
     cpu_interrupt(CPU(cpu), CPU_INTERRUPT_HARD);
 }
 
+void cpu_inject_emergency_signal(S390CPU *cpu, uint16_t src_cpu_addr)
+{
+    CPUS390XState *env = &cpu->env;
+
+    g_assert(src_cpu_addr < S390_MAX_CPUS);
+    set_bit(src_cpu_addr, env->emergency_signals);
+
+    env->pending_int |= INTERRUPT_EMERGENCY_SIGNAL;
+    cpu_interrupt(CPU(cpu), CPU_INTERRUPT_HARD);
+}
+
+int cpu_inject_external_call(S390CPU *cpu, uint16_t src_cpu_addr)
+{
+    CPUS390XState *env = &cpu->env;
+
+    g_assert(src_cpu_addr < S390_MAX_CPUS);
+    if (env->pending_int & INTERRUPT_EXTERNAL_CALL) {
+        return -EBUSY;
+    }
+    env->external_call_addr = src_cpu_addr;
+
+    env->pending_int |= INTERRUPT_EXTERNAL_CALL;
+    cpu_interrupt(CPU(cpu), CPU_INTERRUPT_HARD);
+    return 0;
+}
+
 static void cpu_inject_io(S390CPU *cpu, uint16_t subchannel_id,
                           uint16_t subchannel_number,
                           uint32_t io_int_parm, uint32_t io_int_word)
-- 
2.13.5

[Qemu-devel] [PATCH v1 07/27] s390x/tcg: STOPPED cpus can never wake up

[David Hildenbrand]

Interrupts can't wake such CPUs up. SIGP from other CPUs has to be used
to toggle the state.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/cpu.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index f42e28ea25..64db086702 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -58,6 +58,12 @@ static bool s390_cpu_has_work(CPUState *cs)
     S390CPU *cpu = S390_CPU(cs);
     CPUS390XState *env = &cpu->env;
 
+    /* STOPPED cpus can never wake up */
+    if (s390_cpu_get_state(cpu) != CPU_STATE_LOAD &&
+        s390_cpu_get_state(cpu) != CPU_STATE_OPERATING) {
+        return false;
+    }
+
     return (cs->interrupt_request & CPU_INTERRUPT_HARD) &&
            (env->psw.mask & PSW_MASK_EXT);
 }
-- 
2.13.5

[Qemu-devel] [PATCH v1 08/27] s390x/tcg: a CPU cannot switch state due to an interrupt

[David Hildenbrand]

Going to OPERATING here looks wrong. A CPU should even never be
!OPERATING at this point. Unhalting will already be done in
cpu_handle_halt() if there is work, so we can drop this statement
completely.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/excp_helper.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
index ff257ba72e..4dc30e056a 100644
--- a/target/s390x/excp_helper.c
+++ b/target/s390x/excp_helper.c
@@ -432,7 +432,6 @@ void s390_cpu_do_interrupt(CPUState *cs)
     qemu_log_mask(CPU_LOG_INT, "%s: %d at pc=%" PRIx64 "\n",
                   __func__, cs->exception_index, env->psw.addr);
 
-    s390_cpu_set_state(CPU_STATE_OPERATING, cpu);
     /* handle machine checks */
     if ((env->psw.mask & PSW_MASK_MCHECK) &&
         (cs->exception_index == -1)) {
-- 
2.13.5

[Qemu-devel] [PATCH v1 09/27] target/s390x: factor out handling of WAIT PSW into handle_wait()

[David Hildenbrand]

This will now also detect crashes under TCG. We can directly use
cpu->env.psw.addr instead of kvm_run, as we do a
cpu_synchronize_state().

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/helper.c   | 28 ++++++++++++++++++++++------
 target/s390x/internal.h |  1 +
 target/s390x/kvm.c      | 15 +--------------
 3 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/target/s390x/helper.c b/target/s390x/helper.c
index e22b93258b..75ceb0bf2b 100644
--- a/target/s390x/helper.c
+++ b/target/s390x/helper.c
@@ -26,6 +26,7 @@
 #include "qemu/timer.h"
 #include "exec/exec-all.h"
 #include "hw/s390x/ioinst.h"
+#include "sysemu/hw_accel.h"
 #ifndef CONFIG_USER_ONLY
 #include "sysemu/sysemu.h"
 #endif
@@ -113,6 +114,26 @@ hwaddr s390_cpu_get_phys_addr_debug(CPUState *cs, vaddr vaddr)
     return phys_addr;
 }
 
+static inline bool is_special_wait_psw(uint64_t psw_addr)
+{
+    /* signal quiesce */
+    return psw_addr == 0xfffUL;
+}
+
+void handle_wait(S390CPU *cpu)
+{
+    cpu_synchronize_state(CPU(cpu));
+    if (s390_cpu_halt(cpu) == 0) {
+#ifndef CONFIG_USER_ONLY
+        if (is_special_wait_psw(cpu->env.psw.addr)) {
+            qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
+        } else {
+            qemu_system_guest_panicked(NULL);
+        }
+#endif
+    }
+}
+
 void load_psw(CPUS390XState *env, uint64_t mask, uint64_t addr)
 {
     uint64_t old_mask = env->psw.mask;
@@ -128,12 +149,7 @@ void load_psw(CPUS390XState *env, uint64_t mask, uint64_t addr)
     }
 
     if (mask & PSW_MASK_WAIT) {
-        S390CPU *cpu = s390_env_get_cpu(env);
-        if (s390_cpu_halt(cpu) == 0) {
-#ifndef CONFIG_USER_ONLY
-            qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
-#endif
-        }
+        handle_wait(s390_env_get_cpu(env));
     }
 }
 
diff --git a/target/s390x/internal.h b/target/s390x/internal.h
index 15743ec40f..cb331f35ea 100644
--- a/target/s390x/internal.h
+++ b/target/s390x/internal.h
@@ -280,6 +280,7 @@ const char *cc_name(enum cc_op cc_op);
 void load_psw(CPUS390XState *env, uint64_t mask, uint64_t addr);
 uint32_t calc_cc(CPUS390XState *env, uint32_t cc_op, uint64_t src, uint64_t dst,
                  uint64_t vr);
+void handle_wait(S390CPU *cpu);
 
 
 /* cpu.c */
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 3f9983154f..14f864697d 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -1936,12 +1936,6 @@ static int handle_instruction(S390CPU *cpu, struct kvm_run *run)
     return r;
 }
 
-static bool is_special_wait_psw(CPUState *cs)
-{
-    /* signal quiesce */
-    return cs->kvm_run->psw_addr == 0xfffUL;
-}
-
 static void unmanageable_intercept(S390CPU *cpu, const char *str, int pswoffset)
 {
     CPUState *cs = CPU(cpu);
@@ -2012,14 +2006,7 @@ static int handle_intercept(S390CPU *cpu)
             break;
         case ICPT_WAITPSW:
             /* disabled wait, since enabled wait is handled in kernel */
-            cpu_synchronize_state(cs);
-            if (s390_cpu_halt(cpu) == 0) {
-                if (is_special_wait_psw(cs)) {
-                    qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
-                } else {
-                    qemu_system_guest_panicked(NULL);
-                }
-            }
+            handle_wait(cpu);
             r = EXCP_HALTED;
             break;
         case ICPT_CPU_STOP:
-- 
2.13.5

[Qemu-devel] [PATCH v1 10/27] s390x/kvm: pass ipb directly into handle_sigp()

[David Hildenbrand]

No need to pass kvm_run.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/kvm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 14f864697d..da3f7e9895 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -1852,7 +1852,7 @@ static int sigp_set_architecture(S390CPU *cpu, uint32_t param,
     return SIGP_CC_STATUS_STORED;
 }
 
-static int handle_sigp(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1)
+static int handle_sigp(S390CPU *cpu, uint32_t ipb, uint8_t ipa1)
 {
     CPUS390XState *env = &cpu->env;
     const uint8_t r1 = ipa1 >> 4;
@@ -1866,7 +1866,7 @@ static int handle_sigp(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1)
     cpu_synchronize_state(CPU(cpu));
 
     /* get order code */
-    order = decode_basedisp_rs(env, run->s390_sieic.ipb, NULL)
+    order = decode_basedisp_rs(env, ipb, NULL)
         & SIGP_ORDER_MASK;
     status_reg = &env->regs[r1];
     param = (r1 % 2) ? env->regs[r1] : env->regs[r1 + 1];
@@ -1924,7 +1924,7 @@ static int handle_instruction(S390CPU *cpu, struct kvm_run *run)
         r = handle_diag(cpu, run, run->s390_sieic.ipb);
         break;
     case IPA0_SIGP:
-        r = handle_sigp(cpu, run, ipa1);
+        r = handle_sigp(cpu, run->s390_sieic.ipb, ipa1);
         break;
     }
 
-- 
2.13.5

[Qemu-devel] [PATCH v1 11/27] s390x/kvm: generalize SIGP stop and restart interrupt injection

[David Hildenbrand]

Preparation for factoring it out into !kvm code.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/internal.h  |  2 ++
 target/s390x/interrupt.c | 20 ++++++++++++++++++++
 target/s390x/kvm-stub.c  |  8 ++++++++
 target/s390x/kvm.c       | 33 +++++++++++++++++++++------------
 target/s390x/kvm_s390x.h |  2 ++
 5 files changed, 53 insertions(+), 12 deletions(-)

diff --git a/target/s390x/internal.h b/target/s390x/internal.h
index cb331f35ea..8fdeb867e1 100644
--- a/target/s390x/internal.h
+++ b/target/s390x/internal.h
@@ -353,6 +353,8 @@ void cpu_inject_clock_comparator(S390CPU *cpu);
 void cpu_inject_cpu_timer(S390CPU *cpu);
 void cpu_inject_emergency_signal(S390CPU *cpu, uint16_t src_cpu_addr);
 int cpu_inject_external_call(S390CPU *cpu, uint16_t src_cpu_addr);
+void cpu_inject_restart(S390CPU *cpu);
+void cpu_inject_stop(S390CPU *cpu);
 
 
 /* ioinst.c */
diff --git a/target/s390x/interrupt.c b/target/s390x/interrupt.c
index 462316be8e..b1dc28c8dd 100644
--- a/target/s390x/interrupt.c
+++ b/target/s390x/interrupt.c
@@ -117,6 +117,26 @@ int cpu_inject_external_call(S390CPU *cpu, uint16_t src_cpu_addr)
     return 0;
 }
 
+void cpu_inject_restart(S390CPU *cpu)
+{
+    if (kvm_enabled()) {
+        kvm_s390_restart_interrupt(cpu);
+        return;
+    }
+    /* FIXME TCG */
+    g_assert_not_reached();
+}
+
+void cpu_inject_stop(S390CPU *cpu)
+{
+    if (kvm_enabled()) {
+        kvm_s390_stop_interrupt(cpu);
+        return;
+    }
+    /* FIXME TCG */
+    g_assert_not_reached();
+}
+
 static void cpu_inject_io(S390CPU *cpu, uint16_t subchannel_id,
                           uint16_t subchannel_number,
                           uint32_t io_int_parm, uint32_t io_int_word)
diff --git a/target/s390x/kvm-stub.c b/target/s390x/kvm-stub.c
index 261e1cdc44..18b53b2ad6 100644
--- a/target/s390x/kvm-stub.c
+++ b/target/s390x/kvm-stub.c
@@ -109,3 +109,11 @@ int kvm_s390_set_mem_limit(uint64_t new_limit, uint64_t *hw_limit)
 void kvm_s390_crypto_reset(void)
 {
 }
+
+void kvm_s390_stop_interrupt(S390CPU *cpu)
+{
+}
+
+void kvm_s390_restart_interrupt(S390CPU *cpu)
+{
+}
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index da3f7e9895..66a56db175 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -1500,9 +1500,6 @@ static void sigp_stop(CPUState *cs, run_on_cpu_data arg)
 {
     S390CPU *cpu = S390_CPU(cs);
     SigpInfo *si = arg.host_ptr;
-    struct kvm_s390_irq irq = {
-        .type = KVM_S390_SIGP_STOP,
-    };
 
     if (s390_cpu_get_state(cpu) != CPU_STATE_OPERATING) {
         si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
@@ -1515,7 +1512,7 @@ static void sigp_stop(CPUState *cs, run_on_cpu_data arg)
     } else {
         /* execute the stop function */
         cpu->env.sigp_order = SIGP_STOP;
-        kvm_s390_vcpu_interrupt(cpu, &irq);
+        cpu_inject_stop(cpu);
     }
     si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
 }
@@ -1592,9 +1589,6 @@ static void sigp_stop_and_store_status(CPUState *cs, run_on_cpu_data arg)
 {
     S390CPU *cpu = S390_CPU(cs);
     SigpInfo *si = arg.host_ptr;
-    struct kvm_s390_irq irq = {
-        .type = KVM_S390_SIGP_STOP,
-    };
 
     /* disabled wait - sleeping in user space */
     if (s390_cpu_get_state(cpu) == CPU_STATE_OPERATING && cs->halted) {
@@ -1604,7 +1598,7 @@ static void sigp_stop_and_store_status(CPUState *cs, run_on_cpu_data arg)
     switch (s390_cpu_get_state(cpu)) {
     case CPU_STATE_OPERATING:
         cpu->env.sigp_order = SIGP_STOP_STORE_STATUS;
-        kvm_s390_vcpu_interrupt(cpu, &irq);
+        cpu_inject_stop(cpu);
         /* store will be performed when handling the stop intercept */
         break;
     case CPU_STATE_STOPPED:
@@ -1694,9 +1688,6 @@ static void sigp_restart(CPUState *cs, run_on_cpu_data arg)
 {
     S390CPU *cpu = S390_CPU(cs);
     SigpInfo *si = arg.host_ptr;
-    struct kvm_s390_irq irq = {
-        .type = KVM_S390_RESTART,
-    };
 
     switch (s390_cpu_get_state(cpu)) {
     case CPU_STATE_STOPPED:
@@ -1706,7 +1697,7 @@ static void sigp_restart(CPUState *cs, run_on_cpu_data arg)
         s390_cpu_set_state(CPU_STATE_OPERATING, cpu);
         break;
     case CPU_STATE_OPERATING:
-        kvm_s390_vcpu_interrupt(cpu, &irq);
+        cpu_inject_restart(cpu);
         break;
     }
     si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
@@ -2756,3 +2747,21 @@ void kvm_s390_apply_cpu_model(const S390CPUModel *model, Error **errp)
         kvm_s390_enable_cmma();
     }
 }
+
+void kvm_s390_restart_interrupt(S390CPU *cpu)
+{
+    struct kvm_s390_irq irq = {
+        .type = KVM_S390_RESTART,
+    };
+
+    kvm_s390_vcpu_interrupt(cpu, &irq);
+}
+
+void kvm_s390_stop_interrupt(S390CPU *cpu)
+{
+    struct kvm_s390_irq irq = {
+        .type = KVM_S390_SIGP_STOP,
+    };
+
+    kvm_s390_vcpu_interrupt(cpu, &irq);
+}
diff --git a/target/s390x/kvm_s390x.h b/target/s390x/kvm_s390x.h
index 2d594bd4ee..fd03cd662a 100644
--- a/target/s390x/kvm_s390x.h
+++ b/target/s390x/kvm_s390x.h
@@ -40,6 +40,8 @@ void kvm_s390_cmma_reset(void);
 void kvm_s390_reset_vcpu(S390CPU *cpu);
 int kvm_s390_set_mem_limit(uint64_t new_limit, uint64_t *hw_limit);
 void kvm_s390_crypto_reset(void);
+void kvm_s390_restart_interrupt(S390CPU *cpu);
+void kvm_s390_stop_interrupt(S390CPU *cpu);
 
 /* implemented outside of target/s390x/ */
 int kvm_s390_inject_flic(struct kvm_s390_irq *irq);
-- 
2.13.5

[Qemu-devel] [PATCH v1 12/27] s390x/kvm: factor out storing of CPU status

[David Hildenbrand]

Factor it out into s390_store_status(), to be used also by TCG later on.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/helper.c   | 38 ++++++++++++++++++++++++++++++++++++++
 target/s390x/internal.h |  2 ++
 target/s390x/kvm.c      | 47 +++--------------------------------------------
 3 files changed, 43 insertions(+), 44 deletions(-)

diff --git a/target/s390x/helper.c b/target/s390x/helper.c
index 75ceb0bf2b..09468f85fa 100644
--- a/target/s390x/helper.c
+++ b/target/s390x/helper.c
@@ -245,6 +245,44 @@ void s390_cpu_recompute_watchpoints(CPUState *cs)
     }
 }
 
+#define SAVE_AREA_SIZE 512
+int s390_store_status(S390CPU *cpu, hwaddr addr, bool store_arch)
+{
+    static const uint8_t ar_id = 1;
+    uint64_t ckc = cpu->env.ckc >> 8;
+    void *mem;
+    int i;
+    hwaddr len = SAVE_AREA_SIZE;
+
+    mem = cpu_physical_memory_map(addr, &len, 1);
+    if (!mem) {
+        return -EFAULT;
+    }
+    if (len != SAVE_AREA_SIZE) {
+        cpu_physical_memory_unmap(mem, len, 1, 0);
+        return -EFAULT;
+    }
+
+    if (store_arch) {
+        cpu_physical_memory_write(offsetof(LowCore, ar_access_id), &ar_id, 1);
+    }
+    for (i = 0; i < 16; ++i) {
+        *((uint64_t *)mem + i) = get_freg(&cpu->env, i)->ll;
+    }
+    memcpy(mem + 128, &cpu->env.regs, 128);
+    memcpy(mem + 256, &cpu->env.psw, 16);
+    memcpy(mem + 280, &cpu->env.psa, 4);
+    memcpy(mem + 284, &cpu->env.fpc, 4);
+    memcpy(mem + 292, &cpu->env.todpr, 4);
+    memcpy(mem + 296, &cpu->env.cputm, 8);
+    memcpy(mem + 304, &ckc, 8);
+    memcpy(mem + 320, &cpu->env.aregs, 64);
+    memcpy(mem + 384, &cpu->env.cregs, 128);
+
+    cpu_physical_memory_unmap(mem, len, 1, len);
+
+    return 0;
+}
 #endif /* CONFIG_USER_ONLY */
 
 void s390_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
diff --git a/target/s390x/internal.h b/target/s390x/internal.h
index 8fdeb867e1..30bcd7b662 100644
--- a/target/s390x/internal.h
+++ b/target/s390x/internal.h
@@ -339,6 +339,8 @@ void s390_cpu_recompute_watchpoints(CPUState *cs);
 void s390x_tod_timer(void *opaque);
 void s390x_cpu_timer(void *opaque);
 void do_restart_interrupt(CPUS390XState *env);
+#define S390_STORE_STATUS_DEF_ADDR offsetof(LowCore, floating_pt_save_area)
+int s390_store_status(S390CPU *cpu, hwaddr addr, bool store_arch);
 #ifndef CONFIG_USER_ONLY
 LowCore *cpu_map_lowcore(CPUS390XState *env);
 void cpu_unmap_lowcore(LowCore *lowcore);
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 66a56db175..a6bc27eb28 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -1545,46 +1545,6 @@ static int do_store_adtl_status(S390CPU *cpu, hwaddr addr, hwaddr len)
     return 0;
 }
 
-#define KVM_S390_STORE_STATUS_DEF_ADDR offsetof(LowCore, floating_pt_save_area)
-#define SAVE_AREA_SIZE 512
-static int kvm_s390_store_status(S390CPU *cpu, hwaddr addr, bool store_arch)
-{
-    static const uint8_t ar_id = 1;
-    uint64_t ckc = cpu->env.ckc >> 8;
-    void *mem;
-    int i;
-    hwaddr len = SAVE_AREA_SIZE;
-
-    mem = cpu_physical_memory_map(addr, &len, 1);
-    if (!mem) {
-        return -EFAULT;
-    }
-    if (len != SAVE_AREA_SIZE) {
-        cpu_physical_memory_unmap(mem, len, 1, 0);
-        return -EFAULT;
-    }
-
-    if (store_arch) {
-        cpu_physical_memory_write(offsetof(LowCore, ar_access_id), &ar_id, 1);
-    }
-    for (i = 0; i < 16; ++i) {
-        *((uint64_t *)mem + i) = get_freg(&cpu->env, i)->ll;
-    }
-    memcpy(mem + 128, &cpu->env.regs, 128);
-    memcpy(mem + 256, &cpu->env.psw, 16);
-    memcpy(mem + 280, &cpu->env.psa, 4);
-    memcpy(mem + 284, &cpu->env.fpc, 4);
-    memcpy(mem + 292, &cpu->env.todpr, 4);
-    memcpy(mem + 296, &cpu->env.cputm, 8);
-    memcpy(mem + 304, &ckc, 8);
-    memcpy(mem + 320, &cpu->env.aregs, 64);
-    memcpy(mem + 384, &cpu->env.cregs, 128);
-
-    cpu_physical_memory_unmap(mem, len, 1, len);
-
-    return 0;
-}
-
 static void sigp_stop_and_store_status(CPUState *cs, run_on_cpu_data arg)
 {
     S390CPU *cpu = S390_CPU(cs);
@@ -1604,7 +1564,7 @@ static void sigp_stop_and_store_status(CPUState *cs, run_on_cpu_data arg)
     case CPU_STATE_STOPPED:
         /* already stopped, just store the status */
         cpu_synchronize_state(cs);
-        kvm_s390_store_status(cpu, KVM_S390_STORE_STATUS_DEF_ADDR, true);
+        s390_store_status(cpu, S390_STORE_STATUS_DEF_ADDR, true);
         break;
     }
     si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
@@ -1624,7 +1584,7 @@ static void sigp_store_status_at_address(CPUState *cs, run_on_cpu_data arg)
 
     cpu_synchronize_state(cs);
 
-    if (kvm_s390_store_status(cpu, address, false)) {
+    if (s390_store_status(cpu, address, false)) {
         set_sigp_status(si, SIGP_STAT_INVALID_PARAMETER);
         return;
     }
@@ -2005,8 +1965,7 @@ static int handle_intercept(S390CPU *cpu)
                 qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
             }
             if (cpu->env.sigp_order == SIGP_STOP_STORE_STATUS) {
-                kvm_s390_store_status(cpu, KVM_S390_STORE_STATUS_DEF_ADDR,
-                                      true);
+                s390_store_status(cpu, S390_STORE_STATUS_DEF_ADDR, true);
             }
             cpu->env.sigp_order = 0;
             r = EXCP_HALTED;
-- 
2.13.5

[Qemu-devel] [PATCH v1 13/27] target/s390x: proper cpu->be convertion in s390_store_status()

[David Hildenbrand]

Necessary so TCG can also use it.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/helper.c | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/target/s390x/helper.c b/target/s390x/helper.c
index 09468f85fa..e44d54b932 100644
--- a/target/s390x/helper.c
+++ b/target/s390x/helper.c
@@ -267,17 +267,24 @@ int s390_store_status(S390CPU *cpu, hwaddr addr, bool store_arch)
         cpu_physical_memory_write(offsetof(LowCore, ar_access_id), &ar_id, 1);
     }
     for (i = 0; i < 16; ++i) {
-        *((uint64_t *)mem + i) = get_freg(&cpu->env, i)->ll;
+        *((uint64_t *)mem + i) = cpu_to_be64(get_freg(&cpu->env, i)->ll);
+    }
+    for (i = 0; i < 16; ++i) {
+        *((uint64_t *)(mem + 128) + i) = cpu_to_be64(cpu->env.regs[i]);
+    }
+    *((uint64_t *)(mem + 256)) = cpu_to_be64(get_psw_mask(&cpu->env));
+    *((uint64_t *)(mem + 264)) = cpu_to_be64(cpu->env.psw.addr);
+    *((uint32_t *)(mem + 280)) = cpu_to_be32(cpu->env.psa);
+    *((uint32_t *)(mem + 284)) = cpu_to_be32(cpu->env.fpc);
+    *((uint32_t *)(mem + 292)) = cpu_to_be32(cpu->env.todpr);
+    *((uint64_t *)(mem + 296)) = cpu_to_be64(cpu->env.cputm);
+    *((uint64_t *)(mem + 304)) = cpu_to_be64(ckc);
+    for (i = 0; i < 16; ++i) {
+        *((uint32_t *)(mem + 320) + i) = cpu_to_be32(cpu->env.aregs[i]);
+    }
+    for (i = 0; i < 16; ++i) {
+        *((uint64_t *)(mem + 384) + i) = cpu_to_be64(cpu->env.cregs[i]);
     }
-    memcpy(mem + 128, &cpu->env.regs, 128);
-    memcpy(mem + 256, &cpu->env.psw, 16);
-    memcpy(mem + 280, &cpu->env.psa, 4);
-    memcpy(mem + 284, &cpu->env.fpc, 4);
-    memcpy(mem + 292, &cpu->env.todpr, 4);
-    memcpy(mem + 296, &cpu->env.cputm, 8);
-    memcpy(mem + 304, &ckc, 8);
-    memcpy(mem + 320, &cpu->env.aregs, 64);
-    memcpy(mem + 384, &cpu->env.cregs, 128);
 
     cpu_physical_memory_unmap(mem, len, 1, len);
 
-- 
2.13.5

[Qemu-devel] [PATCH v1 14/27] s390x/kvm: factor out storing of adtl CPU status

[David Hildenbrand]

Called from SIGP code to be factored out, so let's move it. Add a
FIXME for TCG code in the future.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/helper.c   | 29 +++++++++++++++++++++++++++++
 target/s390x/internal.h |  1 +
 target/s390x/kvm.c      | 30 +-----------------------------
 3 files changed, 31 insertions(+), 29 deletions(-)

diff --git a/target/s390x/helper.c b/target/s390x/helper.c
index e44d54b932..4d5a92c2ca 100644
--- a/target/s390x/helper.c
+++ b/target/s390x/helper.c
@@ -290,6 +290,35 @@ int s390_store_status(S390CPU *cpu, hwaddr addr, bool store_arch)
 
     return 0;
 }
+
+#define ADTL_GS_OFFSET   1024 /* offset of GS data in adtl save area */
+#define ADTL_GS_MIN_SIZE 2048 /* minimal size of adtl save area for GS */
+int s390_store_adtl_status(S390CPU *cpu, hwaddr addr, hwaddr len)
+{
+    hwaddr save = len;
+    void *mem;
+
+    mem = cpu_physical_memory_map(addr, &save, 1);
+    if (!mem) {
+        return -EFAULT;
+    }
+    if (save != len) {
+        cpu_physical_memory_unmap(mem, len, 1, 0);
+        return -EFAULT;
+    }
+
+    /* FIXME: as soon as TCG supports these features, convert cpu->be */
+    if (s390_has_feat(S390_FEAT_VECTOR)) {
+        memcpy(mem, &cpu->env.vregs, 512);
+    }
+    if (s390_has_feat(S390_FEAT_GUARDED_STORAGE) && len >= ADTL_GS_MIN_SIZE) {
+        memcpy(mem + ADTL_GS_OFFSET, &cpu->env.gscb, 32);
+    }
+
+    cpu_physical_memory_unmap(mem, len, 1, len);
+
+    return 0;
+}
 #endif /* CONFIG_USER_ONLY */
 
 void s390_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
diff --git a/target/s390x/internal.h b/target/s390x/internal.h
index 30bcd7b662..1d86f48990 100644
--- a/target/s390x/internal.h
+++ b/target/s390x/internal.h
@@ -341,6 +341,7 @@ void s390x_cpu_timer(void *opaque);
 void do_restart_interrupt(CPUS390XState *env);
 #define S390_STORE_STATUS_DEF_ADDR offsetof(LowCore, floating_pt_save_area)
 int s390_store_status(S390CPU *cpu, hwaddr addr, bool store_arch);
+int s390_store_adtl_status(S390CPU *cpu, hwaddr addr, hwaddr len);
 #ifndef CONFIG_USER_ONLY
 LowCore *cpu_map_lowcore(CPUS390XState *env);
 void cpu_unmap_lowcore(LowCore *lowcore);
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index a6bc27eb28..01c4125d21 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -1517,34 +1517,6 @@ static void sigp_stop(CPUState *cs, run_on_cpu_data arg)
     si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
 }
 
-#define ADTL_GS_OFFSET   1024 /* offset of GS data in adtl save area */
-#define ADTL_GS_MIN_SIZE 2048 /* minimal size of adtl save area for GS */
-static int do_store_adtl_status(S390CPU *cpu, hwaddr addr, hwaddr len)
-{
-    hwaddr save = len;
-    void *mem;
-
-    mem = cpu_physical_memory_map(addr, &save, 1);
-    if (!mem) {
-        return -EFAULT;
-    }
-    if (save != len) {
-        cpu_physical_memory_unmap(mem, len, 1, 0);
-        return -EFAULT;
-    }
-
-    if (s390_has_feat(S390_FEAT_VECTOR)) {
-        memcpy(mem, &cpu->env.vregs, 512);
-    }
-    if (s390_has_feat(S390_FEAT_GUARDED_STORAGE) && len >= ADTL_GS_MIN_SIZE) {
-        memcpy(mem + ADTL_GS_OFFSET, &cpu->env.gscb, 32);
-    }
-
-    cpu_physical_memory_unmap(mem, len, 1, len);
-
-    return 0;
-}
-
 static void sigp_stop_and_store_status(CPUState *cs, run_on_cpu_data arg)
 {
     S390CPU *cpu = S390_CPU(cs);
@@ -1637,7 +1609,7 @@ static void sigp_store_adtl_status(CPUState *cs, run_on_cpu_data arg)
 
     cpu_synchronize_state(cs);
 
-    if (do_store_adtl_status(cpu, addr, len)) {
+    if (s390_store_adtl_status(cpu, addr, len)) {
         set_sigp_status(si, SIGP_STAT_INVALID_PARAMETER);
         return;
     }
-- 
2.13.5

[Qemu-devel] [PATCH v1 15/27] s390x/kvm: drop two debug prints

[David Hildenbrand]

Preparation for moving it out of kvm.c.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/kvm.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 01c4125d21..d759edf1c8 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -1640,7 +1640,6 @@ int kvm_s390_cpu_restart(S390CPU *cpu)
     SigpInfo si = {};
 
     run_on_cpu(CPU(cpu), sigp_restart, RUN_ON_CPU_HOST_PTR(&si));
-    DPRINTF("DONE: KVM cpu restart: %p\n", &cpu->env);
     return 0;
 }
 
@@ -1742,7 +1741,6 @@ static int handle_sigp_single_dst(S390CPU *dst_cpu, uint8_t order,
         run_on_cpu(CPU(dst_cpu), sigp_cpu_reset, RUN_ON_CPU_HOST_PTR(&si));
         break;
     default:
-        DPRINTF("KVM: unknown SIGP: 0x%x\n", order);
         set_sigp_status(&si, SIGP_STAT_INVALID_ORDER);
     }
 
-- 
2.13.5

[Qemu-devel] [PATCH v1 16/27] s390x/kvm: factor out SIGP code into sigp.c

[David Hildenbrand]

We want to use the same code base for TCG, so let's cleanly factor it
out.

The sigp mutex is currently not really needed, as everything is
protected by the iothread mutex. But this could change later, so leave
it in place and initialize it properly from common code.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 hw/s390x/s390-virtio-ccw.c |   3 +
 target/s390x/Makefile.objs |   1 +
 target/s390x/cpu.c         |   8 -
 target/s390x/cpu.h         |   6 +-
 target/s390x/internal.h    |   4 +
 target/s390x/kvm-stub.c    |   5 -
 target/s390x/kvm.c         | 349 +-----------------------------------------
 target/s390x/kvm_s390x.h   |   1 -
 target/s390x/sigp.c        | 366 +++++++++++++++++++++++++++++++++++++++++++++
 target/s390x/trace-events  |   4 +-
 10 files changed, 388 insertions(+), 359 deletions(-)
 create mode 100644 target/s390x/sigp.c

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index a57d435cbf..49c1ab29ed 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -280,6 +280,9 @@ static void ccw_init(MachineState *machine)
     /* init CPUs (incl. CPU model) early so s390_has_feature() works */
     s390_init_cpus(machine);
 
+    /* init the SIGP facility */
+    s390_init_sigp();
+
     s390_flic_init();
 
     /* get a BUS */
diff --git a/target/s390x/Makefile.objs b/target/s390x/Makefile.objs
index 9615256d81..5f8a96b1a6 100644
--- a/target/s390x/Makefile.objs
+++ b/target/s390x/Makefile.objs
@@ -2,6 +2,7 @@ obj-y += cpu.o cpu_models.o cpu_features.o gdbstub.o interrupt.o helper.o
 obj-$(CONFIG_TCG) += translate.o cc_helper.o excp_helper.o fpu_helper.o
 obj-$(CONFIG_TCG) += int_helper.o mem_helper.o misc_helper.o
 obj-$(CONFIG_SOFTMMU) += machine.o ioinst.o arch_dump.o mmu_helper.o diag.o
+obj-$(CONFIG_SOFTMMU) += sigp.o
 obj-$(CONFIG_KVM) += kvm.o
 obj-$(call lnot,$(CONFIG_KVM)) += kvm-stub.o
 
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index 64db086702..9a01f591b6 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -386,14 +386,6 @@ void s390_cmma_reset(void)
     }
 }
 
-int s390_cpu_restart(S390CPU *cpu)
-{
-    if (kvm_enabled()) {
-        return kvm_s390_cpu_restart(cpu);
-    }
-    return -ENOSYS;
-}
-
 int s390_get_memslot_count(void)
 {
     if (kvm_enabled()) {
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 857af21d06..5d03802c7d 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -680,7 +680,6 @@ bool s390_get_squash_mcss(void);
 int s390_get_memslot_count(void);
 int s390_set_memory_limit(uint64_t new_limit, uint64_t *hw_limit);
 void s390_cmma_reset(void);
-int s390_cpu_restart(S390CPU *cpu);
 void s390_enable_css_support(S390CPU *cpu);
 int s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch_id,
                                 int vq, bool assign);
@@ -732,6 +731,11 @@ int s390_cpu_virt_mem_rw(S390CPU *cpu, vaddr laddr, uint8_t ar, void *hostbuf,
         s390_cpu_virt_mem_rw(cpu, laddr, ar, NULL, len, true)
 
 
+/* sigp.c */
+int s390_cpu_restart(S390CPU *cpu);
+void s390_init_sigp(void);
+
+
 /* outside of target/s390x/ */
 S390CPU *s390_cpu_addr2state(uint16_t cpu_addr);
 
diff --git a/target/s390x/internal.h b/target/s390x/internal.h
index 1d86f48990..99413beeae 100644
--- a/target/s390x/internal.h
+++ b/target/s390x/internal.h
@@ -397,4 +397,8 @@ void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3);
 /* translate.c */
 void s390x_translate_init(void);
 
+
+/* sigp.c */
+int handle_sigp(CPUS390XState *env, uint8_t order, uint64_t r1, uint64_t r3);
+
 #endif /* S390X_INTERNAL_H */
diff --git a/target/s390x/kvm-stub.c b/target/s390x/kvm-stub.c
index 18b53b2ad6..861b4dedf6 100644
--- a/target/s390x/kvm-stub.c
+++ b/target/s390x/kvm-stub.c
@@ -83,11 +83,6 @@ int kvm_s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch,
     return -ENOSYS;
 }
 
-int kvm_s390_cpu_restart(S390CPU *cpu)
-{
-    return -ENOSYS;
-}
-
 void kvm_s390_cmma_reset(void)
 {
 }
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index d759edf1c8..142b63eef0 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -135,8 +135,6 @@ const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
     KVM_CAP_LAST_INFO
 };
 
-static QemuMutex qemu_sigp_mutex;
-
 static int cap_sync_regs;
 static int cap_async_pf;
 static int cap_mem_op;
@@ -314,8 +312,6 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
     /* Try to enable AIS facility */
     kvm_vm_enable_cap(s, KVM_CAP_S390_AIS, 0);
 
-    qemu_mutex_init(&qemu_sigp_mutex);
-
     return 0;
 }
 
@@ -1469,355 +1465,22 @@ static int handle_diag(S390CPU *cpu, struct kvm_run *run, uint32_t ipb)
     return r;
 }
 
-typedef struct SigpInfo {
-    uint64_t param;
-    int cc;
-    uint64_t *status_reg;
-} SigpInfo;
-
-static void set_sigp_status(SigpInfo *si, uint64_t status)
-{
-    *si->status_reg &= 0xffffffff00000000ULL;
-    *si->status_reg |= status;
-    si->cc = SIGP_CC_STATUS_STORED;
-}
-
-static void sigp_start(CPUState *cs, run_on_cpu_data arg)
-{
-    S390CPU *cpu = S390_CPU(cs);
-    SigpInfo *si = arg.host_ptr;
-
-    if (s390_cpu_get_state(cpu) != CPU_STATE_STOPPED) {
-        si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
-        return;
-    }
-
-    s390_cpu_set_state(CPU_STATE_OPERATING, cpu);
-    si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
-}
-
-static void sigp_stop(CPUState *cs, run_on_cpu_data arg)
-{
-    S390CPU *cpu = S390_CPU(cs);
-    SigpInfo *si = arg.host_ptr;
-
-    if (s390_cpu_get_state(cpu) != CPU_STATE_OPERATING) {
-        si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
-        return;
-    }
-
-    /* disabled wait - sleeping in user space */
-    if (cs->halted) {
-        s390_cpu_set_state(CPU_STATE_STOPPED, cpu);
-    } else {
-        /* execute the stop function */
-        cpu->env.sigp_order = SIGP_STOP;
-        cpu_inject_stop(cpu);
-    }
-    si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
-}
-
-static void sigp_stop_and_store_status(CPUState *cs, run_on_cpu_data arg)
-{
-    S390CPU *cpu = S390_CPU(cs);
-    SigpInfo *si = arg.host_ptr;
-
-    /* disabled wait - sleeping in user space */
-    if (s390_cpu_get_state(cpu) == CPU_STATE_OPERATING && cs->halted) {
-        s390_cpu_set_state(CPU_STATE_STOPPED, cpu);
-    }
-
-    switch (s390_cpu_get_state(cpu)) {
-    case CPU_STATE_OPERATING:
-        cpu->env.sigp_order = SIGP_STOP_STORE_STATUS;
-        cpu_inject_stop(cpu);
-        /* store will be performed when handling the stop intercept */
-        break;
-    case CPU_STATE_STOPPED:
-        /* already stopped, just store the status */
-        cpu_synchronize_state(cs);
-        s390_store_status(cpu, S390_STORE_STATUS_DEF_ADDR, true);
-        break;
-    }
-    si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
-}
-
-static void sigp_store_status_at_address(CPUState *cs, run_on_cpu_data arg)
-{
-    S390CPU *cpu = S390_CPU(cs);
-    SigpInfo *si = arg.host_ptr;
-    uint32_t address = si->param & 0x7ffffe00u;
-
-    /* cpu has to be stopped */
-    if (s390_cpu_get_state(cpu) != CPU_STATE_STOPPED) {
-        set_sigp_status(si, SIGP_STAT_INCORRECT_STATE);
-        return;
-    }
-
-    cpu_synchronize_state(cs);
-
-    if (s390_store_status(cpu, address, false)) {
-        set_sigp_status(si, SIGP_STAT_INVALID_PARAMETER);
-        return;
-    }
-    si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
-}
-
-#define ADTL_SAVE_LC_MASK  0xfUL
-static void sigp_store_adtl_status(CPUState *cs, run_on_cpu_data arg)
-{
-    S390CPU *cpu = S390_CPU(cs);
-    SigpInfo *si = arg.host_ptr;
-    uint8_t lc = si->param & ADTL_SAVE_LC_MASK;
-    hwaddr addr = si->param & ~ADTL_SAVE_LC_MASK;
-    hwaddr len = 1UL << (lc ? lc : 10);
-
-    if (!s390_has_feat(S390_FEAT_VECTOR) &&
-        !s390_has_feat(S390_FEAT_GUARDED_STORAGE)) {
-        set_sigp_status(si, SIGP_STAT_INVALID_ORDER);
-        return;
-    }
-
-    /* cpu has to be stopped */
-    if (s390_cpu_get_state(cpu) != CPU_STATE_STOPPED) {
-        set_sigp_status(si, SIGP_STAT_INCORRECT_STATE);
-        return;
-    }
-
-    /* address must be aligned to length */
-    if (addr & (len - 1)) {
-        set_sigp_status(si, SIGP_STAT_INVALID_PARAMETER);
-        return;
-    }
-
-    /* no GS: only lc == 0 is valid */
-    if (!s390_has_feat(S390_FEAT_GUARDED_STORAGE) &&
-        lc != 0) {
-        set_sigp_status(si, SIGP_STAT_INVALID_PARAMETER);
-        return;
-    }
-
-    /* GS: 0, 10, 11, 12 are valid */
-    if (s390_has_feat(S390_FEAT_GUARDED_STORAGE) &&
-        lc != 0 &&
-        lc != 10 &&
-        lc != 11 &&
-        lc != 12) {
-        set_sigp_status(si, SIGP_STAT_INVALID_PARAMETER);
-        return;
-    }
-
-    cpu_synchronize_state(cs);
-
-    if (s390_store_adtl_status(cpu, addr, len)) {
-        set_sigp_status(si, SIGP_STAT_INVALID_PARAMETER);
-        return;
-    }
-    si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
-}
-
-static void sigp_restart(CPUState *cs, run_on_cpu_data arg)
-{
-    S390CPU *cpu = S390_CPU(cs);
-    SigpInfo *si = arg.host_ptr;
-
-    switch (s390_cpu_get_state(cpu)) {
-    case CPU_STATE_STOPPED:
-        /* the restart irq has to be delivered prior to any other pending irq */
-        cpu_synchronize_state(cs);
-        do_restart_interrupt(&cpu->env);
-        s390_cpu_set_state(CPU_STATE_OPERATING, cpu);
-        break;
-    case CPU_STATE_OPERATING:
-        cpu_inject_restart(cpu);
-        break;
-    }
-    si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
-}
-
-int kvm_s390_cpu_restart(S390CPU *cpu)
-{
-    SigpInfo si = {};
-
-    run_on_cpu(CPU(cpu), sigp_restart, RUN_ON_CPU_HOST_PTR(&si));
-    return 0;
-}
-
-static void sigp_initial_cpu_reset(CPUState *cs, run_on_cpu_data arg)
-{
-    S390CPU *cpu = S390_CPU(cs);
-    S390CPUClass *scc = S390_CPU_GET_CLASS(cpu);
-    SigpInfo *si = arg.host_ptr;
-
-    cpu_synchronize_state(cs);
-    scc->initial_cpu_reset(cs);
-    cpu_synchronize_post_reset(cs);
-    si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
-}
-
-static void sigp_cpu_reset(CPUState *cs, run_on_cpu_data arg)
-{
-    S390CPU *cpu = S390_CPU(cs);
-    S390CPUClass *scc = S390_CPU_GET_CLASS(cpu);
-    SigpInfo *si = arg.host_ptr;
-
-    cpu_synchronize_state(cs);
-    scc->cpu_reset(cs);
-    cpu_synchronize_post_reset(cs);
-    si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
-}
-
-static void sigp_set_prefix(CPUState *cs, run_on_cpu_data arg)
-{
-    S390CPU *cpu = S390_CPU(cs);
-    SigpInfo *si = arg.host_ptr;
-    uint32_t addr = si->param & 0x7fffe000u;
-
-    cpu_synchronize_state(cs);
-
-    if (!address_space_access_valid(&address_space_memory, addr,
-                                    sizeof(struct LowCore), false)) {
-        set_sigp_status(si, SIGP_STAT_INVALID_PARAMETER);
-        return;
-    }
-
-    /* cpu has to be stopped */
-    if (s390_cpu_get_state(cpu) != CPU_STATE_STOPPED) {
-        set_sigp_status(si, SIGP_STAT_INCORRECT_STATE);
-        return;
-    }
-
-    cpu->env.psa = addr;
-    cpu_synchronize_post_init(cs);
-    si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
-}
-
-static int handle_sigp_single_dst(S390CPU *dst_cpu, uint8_t order,
-                                  uint64_t param, uint64_t *status_reg)
-{
-    SigpInfo si = {
-        .param = param,
-        .status_reg = status_reg,
-    };
-
-    /* cpu available? */
-    if (dst_cpu == NULL) {
-        return SIGP_CC_NOT_OPERATIONAL;
-    }
-
-    /* only resets can break pending orders */
-    if (dst_cpu->env.sigp_order != 0 &&
-        order != SIGP_CPU_RESET &&
-        order != SIGP_INITIAL_CPU_RESET) {
-        return SIGP_CC_BUSY;
-    }
-
-    switch (order) {
-    case SIGP_START:
-        run_on_cpu(CPU(dst_cpu), sigp_start, RUN_ON_CPU_HOST_PTR(&si));
-        break;
-    case SIGP_STOP:
-        run_on_cpu(CPU(dst_cpu), sigp_stop, RUN_ON_CPU_HOST_PTR(&si));
-        break;
-    case SIGP_RESTART:
-        run_on_cpu(CPU(dst_cpu), sigp_restart, RUN_ON_CPU_HOST_PTR(&si));
-        break;
-    case SIGP_STOP_STORE_STATUS:
-        run_on_cpu(CPU(dst_cpu), sigp_stop_and_store_status, RUN_ON_CPU_HOST_PTR(&si));
-        break;
-    case SIGP_STORE_STATUS_ADDR:
-        run_on_cpu(CPU(dst_cpu), sigp_store_status_at_address, RUN_ON_CPU_HOST_PTR(&si));
-        break;
-    case SIGP_STORE_ADTL_STATUS:
-        run_on_cpu(CPU(dst_cpu), sigp_store_adtl_status, RUN_ON_CPU_HOST_PTR(&si));
-        break;
-    case SIGP_SET_PREFIX:
-        run_on_cpu(CPU(dst_cpu), sigp_set_prefix, RUN_ON_CPU_HOST_PTR(&si));
-        break;
-    case SIGP_INITIAL_CPU_RESET:
-        run_on_cpu(CPU(dst_cpu), sigp_initial_cpu_reset, RUN_ON_CPU_HOST_PTR(&si));
-        break;
-    case SIGP_CPU_RESET:
-        run_on_cpu(CPU(dst_cpu), sigp_cpu_reset, RUN_ON_CPU_HOST_PTR(&si));
-        break;
-    default:
-        set_sigp_status(&si, SIGP_STAT_INVALID_ORDER);
-    }
-
-    return si.cc;
-}
-
-static int sigp_set_architecture(S390CPU *cpu, uint32_t param,
-                                 uint64_t *status_reg)
-{
-    CPUState *cur_cs;
-    S390CPU *cur_cpu;
-    bool all_stopped = true;
-
-    CPU_FOREACH(cur_cs) {
-        cur_cpu = S390_CPU(cur_cs);
-
-        if (cur_cpu == cpu) {
-            continue;
-        }
-        if (s390_cpu_get_state(cur_cpu) != CPU_STATE_STOPPED) {
-            all_stopped = false;
-        }
-    }
-
-    *status_reg &= 0xffffffff00000000ULL;
-
-    /* Reject set arch order, with czam we're always in z/Arch mode. */
-    *status_reg |= (all_stopped ? SIGP_STAT_INVALID_PARAMETER :
-                    SIGP_STAT_INCORRECT_STATE);
-    return SIGP_CC_STATUS_STORED;
-}
-
-static int handle_sigp(S390CPU *cpu, uint32_t ipb, uint8_t ipa1)
+static int kvm_s390_handle_sigp(S390CPU *cpu, uint32_t ipb, uint8_t ipa1)
 {
     CPUS390XState *env = &cpu->env;
     const uint8_t r1 = ipa1 >> 4;
     const uint8_t r3 = ipa1 & 0x0f;
     int ret;
     uint8_t order;
-    uint64_t *status_reg;
-    uint64_t param;
-    S390CPU *dst_cpu = NULL;
 
     cpu_synchronize_state(CPU(cpu));
 
     /* get order code */
-    order = decode_basedisp_rs(env, ipb, NULL)
-        & SIGP_ORDER_MASK;
-    status_reg = &env->regs[r1];
-    param = (r1 % 2) ? env->regs[r1] : env->regs[r1 + 1];
+    order = decode_basedisp_rs(env, ipb, NULL) & SIGP_ORDER_MASK;
 
-    if (qemu_mutex_trylock(&qemu_sigp_mutex)) {
-        ret = SIGP_CC_BUSY;
-        goto out;
-    }
-
-    switch (order) {
-    case SIGP_SET_ARCH:
-        ret = sigp_set_architecture(cpu, param, status_reg);
-        break;
-    default:
-        /* all other sigp orders target a single vcpu */
-        dst_cpu = s390_cpu_addr2state(env->regs[r3]);
-        ret = handle_sigp_single_dst(dst_cpu, order, param, status_reg);
-    }
-    qemu_mutex_unlock(&qemu_sigp_mutex);
-
-out:
-    trace_kvm_sigp_finished(order, CPU(cpu)->cpu_index,
-                            dst_cpu ? CPU(dst_cpu)->cpu_index : -1, ret);
-
-    if (ret >= 0) {
-        setcc(cpu, ret);
-        return 0;
-    }
-
-    return ret;
+    ret = handle_sigp(env, order, r1, r3);
+    setcc(cpu, ret);
+    return 0;
 }
 
 static int handle_instruction(S390CPU *cpu, struct kvm_run *run)
@@ -1845,7 +1508,7 @@ static int handle_instruction(S390CPU *cpu, struct kvm_run *run)
         r = handle_diag(cpu, run, run->s390_sieic.ipb);
         break;
     case IPA0_SIGP:
-        r = handle_sigp(cpu, run->s390_sieic.ipb, ipa1);
+        r = kvm_s390_handle_sigp(cpu, run->s390_sieic.ipb, ipa1);
         break;
     }
 
diff --git a/target/s390x/kvm_s390x.h b/target/s390x/kvm_s390x.h
index fd03cd662a..33906a893b 100644
--- a/target/s390x/kvm_s390x.h
+++ b/target/s390x/kvm_s390x.h
@@ -33,7 +33,6 @@ int kvm_s390_set_clock(uint8_t *tod_high, uint64_t *tod_clock);
 void kvm_s390_enable_css_support(S390CPU *cpu);
 int kvm_s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch,
                                     int vq, bool assign);
-int kvm_s390_cpu_restart(S390CPU *cpu);
 int kvm_s390_get_memslot_count(void);
 int kvm_s390_cmma_active(void);
 void kvm_s390_cmma_reset(void);
diff --git a/target/s390x/sigp.c b/target/s390x/sigp.c
new file mode 100644
index 0000000000..4813123aad
--- /dev/null
+++ b/target/s390x/sigp.c
@@ -0,0 +1,366 @@
+/*
+ * s390x SIGP instruction handling
+ *
+ * Copyright (c) 2009 Alexander Graf <agraf@suse.de>
+ * Copyright IBM Corp. 2012
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "cpu.h"
+#include "internal.h"
+#include "sysemu/hw_accel.h"
+#include "exec/address-spaces.h"
+#include "sysemu/sysemu.h"
+#include "trace.h"
+
+QemuMutex qemu_sigp_mutex;
+
+typedef struct SigpInfo {
+    uint64_t param;
+    int cc;
+    uint64_t *status_reg;
+} SigpInfo;
+
+static void set_sigp_status(SigpInfo *si, uint64_t status)
+{
+    *si->status_reg &= 0xffffffff00000000ULL;
+    *si->status_reg |= status;
+    si->cc = SIGP_CC_STATUS_STORED;
+}
+
+static void sigp_start(CPUState *cs, run_on_cpu_data arg)
+{
+    S390CPU *cpu = S390_CPU(cs);
+    SigpInfo *si = arg.host_ptr;
+
+    if (s390_cpu_get_state(cpu) != CPU_STATE_STOPPED) {
+        si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
+        return;
+    }
+
+    s390_cpu_set_state(CPU_STATE_OPERATING, cpu);
+    si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
+}
+
+static void sigp_stop(CPUState *cs, run_on_cpu_data arg)
+{
+    S390CPU *cpu = S390_CPU(cs);
+    SigpInfo *si = arg.host_ptr;
+
+    if (s390_cpu_get_state(cpu) != CPU_STATE_OPERATING) {
+        si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
+        return;
+    }
+
+    /* disabled wait - sleeping in user space */
+    if (cs->halted) {
+        s390_cpu_set_state(CPU_STATE_STOPPED, cpu);
+    } else {
+        /* execute the stop function */
+        cpu->env.sigp_order = SIGP_STOP;
+        cpu_inject_stop(cpu);
+    }
+    si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
+}
+
+static void sigp_stop_and_store_status(CPUState *cs, run_on_cpu_data arg)
+{
+    S390CPU *cpu = S390_CPU(cs);
+    SigpInfo *si = arg.host_ptr;
+
+    /* disabled wait - sleeping in user space */
+    if (s390_cpu_get_state(cpu) == CPU_STATE_OPERATING && cs->halted) {
+        s390_cpu_set_state(CPU_STATE_STOPPED, cpu);
+    }
+
+    switch (s390_cpu_get_state(cpu)) {
+    case CPU_STATE_OPERATING:
+        cpu->env.sigp_order = SIGP_STOP_STORE_STATUS;
+        cpu_inject_stop(cpu);
+        /* store will be performed when handling the stop intercept */
+        break;
+    case CPU_STATE_STOPPED:
+        /* already stopped, just store the status */
+        cpu_synchronize_state(cs);
+        s390_store_status(cpu, S390_STORE_STATUS_DEF_ADDR, true);
+        break;
+    }
+    si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
+}
+
+static void sigp_store_status_at_address(CPUState *cs, run_on_cpu_data arg)
+{
+    S390CPU *cpu = S390_CPU(cs);
+    SigpInfo *si = arg.host_ptr;
+    uint32_t address = si->param & 0x7ffffe00u;
+
+    /* cpu has to be stopped */
+    if (s390_cpu_get_state(cpu) != CPU_STATE_STOPPED) {
+        set_sigp_status(si, SIGP_STAT_INCORRECT_STATE);
+        return;
+    }
+
+    cpu_synchronize_state(cs);
+
+    if (s390_store_status(cpu, address, false)) {
+        set_sigp_status(si, SIGP_STAT_INVALID_PARAMETER);
+        return;
+    }
+    si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
+}
+
+#define ADTL_SAVE_LC_MASK  0xfUL
+static void sigp_store_adtl_status(CPUState *cs, run_on_cpu_data arg)
+{
+    S390CPU *cpu = S390_CPU(cs);
+    SigpInfo *si = arg.host_ptr;
+    uint8_t lc = si->param & ADTL_SAVE_LC_MASK;
+    hwaddr addr = si->param & ~ADTL_SAVE_LC_MASK;
+    hwaddr len = 1UL << (lc ? lc : 10);
+
+    if (!s390_has_feat(S390_FEAT_VECTOR) &&
+        !s390_has_feat(S390_FEAT_GUARDED_STORAGE)) {
+        set_sigp_status(si, SIGP_STAT_INVALID_ORDER);
+        return;
+    }
+
+    /* cpu has to be stopped */
+    if (s390_cpu_get_state(cpu) != CPU_STATE_STOPPED) {
+        set_sigp_status(si, SIGP_STAT_INCORRECT_STATE);
+        return;
+    }
+
+    /* address must be aligned to length */
+    if (addr & (len - 1)) {
+        set_sigp_status(si, SIGP_STAT_INVALID_PARAMETER);
+        return;
+    }
+
+    /* no GS: only lc == 0 is valid */
+    if (!s390_has_feat(S390_FEAT_GUARDED_STORAGE) &&
+        lc != 0) {
+        set_sigp_status(si, SIGP_STAT_INVALID_PARAMETER);
+        return;
+    }
+
+    /* GS: 0, 10, 11, 12 are valid */
+    if (s390_has_feat(S390_FEAT_GUARDED_STORAGE) &&
+        lc != 0 &&
+        lc != 10 &&
+        lc != 11 &&
+        lc != 12) {
+        set_sigp_status(si, SIGP_STAT_INVALID_PARAMETER);
+        return;
+    }
+
+    cpu_synchronize_state(cs);
+
+    if (s390_store_adtl_status(cpu, addr, len)) {
+        set_sigp_status(si, SIGP_STAT_INVALID_PARAMETER);
+        return;
+    }
+    si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
+}
+
+static void sigp_restart(CPUState *cs, run_on_cpu_data arg)
+{
+    S390CPU *cpu = S390_CPU(cs);
+    SigpInfo *si = arg.host_ptr;
+
+    switch (s390_cpu_get_state(cpu)) {
+    case CPU_STATE_STOPPED:
+        /* the restart irq has to be delivered prior to any other pending irq */
+        cpu_synchronize_state(cs);
+        do_restart_interrupt(&cpu->env);
+        s390_cpu_set_state(CPU_STATE_OPERATING, cpu);
+        break;
+    case CPU_STATE_OPERATING:
+        cpu_inject_restart(cpu);
+        break;
+    }
+    si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
+}
+
+static void sigp_initial_cpu_reset(CPUState *cs, run_on_cpu_data arg)
+{
+    S390CPU *cpu = S390_CPU(cs);
+    S390CPUClass *scc = S390_CPU_GET_CLASS(cpu);
+    SigpInfo *si = arg.host_ptr;
+
+    cpu_synchronize_state(cs);
+    scc->initial_cpu_reset(cs);
+    cpu_synchronize_post_reset(cs);
+    si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
+}
+
+static void sigp_cpu_reset(CPUState *cs, run_on_cpu_data arg)
+{
+    S390CPU *cpu = S390_CPU(cs);
+    S390CPUClass *scc = S390_CPU_GET_CLASS(cpu);
+    SigpInfo *si = arg.host_ptr;
+
+    cpu_synchronize_state(cs);
+    scc->cpu_reset(cs);
+    cpu_synchronize_post_reset(cs);
+    si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
+}
+
+static void sigp_set_prefix(CPUState *cs, run_on_cpu_data arg)
+{
+    S390CPU *cpu = S390_CPU(cs);
+    SigpInfo *si = arg.host_ptr;
+    uint32_t addr = si->param & 0x7fffe000u;
+
+    cpu_synchronize_state(cs);
+
+    if (!address_space_access_valid(&address_space_memory, addr,
+                                    sizeof(struct LowCore), false)) {
+        set_sigp_status(si, SIGP_STAT_INVALID_PARAMETER);
+        return;
+    }
+
+    /* cpu has to be stopped */
+    if (s390_cpu_get_state(cpu) != CPU_STATE_STOPPED) {
+        set_sigp_status(si, SIGP_STAT_INCORRECT_STATE);
+        return;
+    }
+
+    cpu->env.psa = addr;
+    cpu_synchronize_post_init(cs);
+    si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
+}
+
+static int handle_sigp_single_dst(S390CPU *dst_cpu, uint8_t order,
+                                  uint64_t param, uint64_t *status_reg)
+{
+    SigpInfo si = {
+        .param = param,
+        .status_reg = status_reg,
+    };
+
+    /* cpu available? */
+    if (dst_cpu == NULL) {
+        return SIGP_CC_NOT_OPERATIONAL;
+    }
+
+    /* only resets can break pending orders */
+    if (dst_cpu->env.sigp_order != 0 &&
+        order != SIGP_CPU_RESET &&
+        order != SIGP_INITIAL_CPU_RESET) {
+        return SIGP_CC_BUSY;
+    }
+
+    switch (order) {
+    case SIGP_START:
+        run_on_cpu(CPU(dst_cpu), sigp_start, RUN_ON_CPU_HOST_PTR(&si));
+        break;
+    case SIGP_STOP:
+        run_on_cpu(CPU(dst_cpu), sigp_stop, RUN_ON_CPU_HOST_PTR(&si));
+        break;
+    case SIGP_RESTART:
+        run_on_cpu(CPU(dst_cpu), sigp_restart, RUN_ON_CPU_HOST_PTR(&si));
+        break;
+    case SIGP_STOP_STORE_STATUS:
+        run_on_cpu(CPU(dst_cpu), sigp_stop_and_store_status, RUN_ON_CPU_HOST_PTR(&si));
+        break;
+    case SIGP_STORE_STATUS_ADDR:
+        run_on_cpu(CPU(dst_cpu), sigp_store_status_at_address, RUN_ON_CPU_HOST_PTR(&si));
+        break;
+    case SIGP_STORE_ADTL_STATUS:
+        run_on_cpu(CPU(dst_cpu), sigp_store_adtl_status, RUN_ON_CPU_HOST_PTR(&si));
+        break;
+    case SIGP_SET_PREFIX:
+        run_on_cpu(CPU(dst_cpu), sigp_set_prefix, RUN_ON_CPU_HOST_PTR(&si));
+        break;
+    case SIGP_INITIAL_CPU_RESET:
+        run_on_cpu(CPU(dst_cpu), sigp_initial_cpu_reset, RUN_ON_CPU_HOST_PTR(&si));
+        break;
+    case SIGP_CPU_RESET:
+        run_on_cpu(CPU(dst_cpu), sigp_cpu_reset, RUN_ON_CPU_HOST_PTR(&si));
+        break;
+    default:
+        set_sigp_status(&si, SIGP_STAT_INVALID_ORDER);
+    }
+
+    return si.cc;
+}
+
+static int sigp_set_architecture(S390CPU *cpu, uint32_t param,
+                                 uint64_t *status_reg)
+{
+    CPUState *cur_cs;
+    S390CPU *cur_cpu;
+    bool all_stopped = true;
+
+    CPU_FOREACH(cur_cs) {
+        cur_cpu = S390_CPU(cur_cs);
+
+        if (cur_cpu == cpu) {
+            continue;
+        }
+        if (s390_cpu_get_state(cur_cpu) != CPU_STATE_STOPPED) {
+            all_stopped = false;
+        }
+    }
+
+    *status_reg &= 0xffffffff00000000ULL;
+
+    /* Reject set arch order, with czam we're always in z/Arch mode. */
+    *status_reg |= (all_stopped ? SIGP_STAT_INVALID_PARAMETER :
+                    SIGP_STAT_INCORRECT_STATE);
+    return SIGP_CC_STATUS_STORED;
+}
+
+int handle_sigp(CPUS390XState *env, uint8_t order, uint64_t r1, uint64_t r3)
+{
+    uint64_t *status_reg = &env->regs[r1];
+    uint64_t param = (r1 % 2) ? env->regs[r1] : env->regs[r1 + 1];
+    S390CPU *cpu = s390_env_get_cpu(env);
+    S390CPU *dst_cpu = NULL;
+    int ret;
+
+    if (qemu_mutex_trylock(&qemu_sigp_mutex)) {
+        ret = SIGP_CC_BUSY;
+        goto out;
+    }
+
+    switch (order) {
+    case SIGP_SET_ARCH:
+        ret = sigp_set_architecture(cpu, param, status_reg);
+        break;
+    default:
+        /* all other sigp orders target a single vcpu */
+        dst_cpu = s390_cpu_addr2state(env->regs[r3]);
+        ret = handle_sigp_single_dst(dst_cpu, order, param, status_reg);
+    }
+    qemu_mutex_unlock(&qemu_sigp_mutex);
+
+out:
+    trace_sigp_finished(order, CPU(cpu)->cpu_index,
+                        dst_cpu ? CPU(dst_cpu)->cpu_index : -1, ret);
+    g_assert(ret >= 0);
+
+    return ret;
+}
+
+int s390_cpu_restart(S390CPU *cpu)
+{
+    SigpInfo si = {};
+
+    if (tcg_enabled()) {
+        /* FIXME TCG */
+        return -ENOSYS;
+    }
+
+    run_on_cpu(CPU(cpu), sigp_restart, RUN_ON_CPU_HOST_PTR(&si));
+    return 0;
+}
+
+void s390_init_sigp(void)
+{
+    qemu_mutex_init(&qemu_sigp_mutex);
+}
diff --git a/target/s390x/trace-events b/target/s390x/trace-events
index 4d871f5087..a84e316e49 100644
--- a/target/s390x/trace-events
+++ b/target/s390x/trace-events
@@ -14,9 +14,11 @@ ioinst_chsc_cmd(uint16_t cmd, uint16_t len) "IOINST: chsc command 0x%04x, len 0x
 kvm_enable_cmma(int rc) "CMMA: enabling with result code %d"
 kvm_clear_cmma(int rc) "CMMA: clearing with result code %d"
 kvm_failed_cpu_state_set(int cpu_index, uint8_t state, const char *msg) "Warning: Unable to set cpu %d state %" PRIu8 " to KVM: %s"
-kvm_sigp_finished(uint8_t order, int cpu_index, int dst_index, int cc) "SIGP: Finished order %u on cpu %d -> cpu %d with cc=%d"
 
 # target/s390x/cpu.c
 cpu_set_state(int cpu_index, uint8_t state) "setting cpu %d state to %" PRIu8
 cpu_halt(int cpu_index) "halting cpu %d"
 cpu_unhalt(int cpu_index) "unhalting cpu %d"
+
+# target/s390x/sigp.c
+sigp_finished(uint8_t order, int cpu_index, int dst_index, int cc) "SIGP: Finished order %u on cpu %d -> cpu %d with cc=%d"
-- 
2.13.5

[Qemu-devel] [PATCH v1 17/27] s390x/kvm: factor out actual handling of STOP interrupts

[David Hildenbrand]

For KVM, the KVM module decides when a STOP can be performed (when the
STOP interrupt can be processed). Factor it out so we can use it
later for TCG.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/internal.h |  1 +
 target/s390x/kvm.c      |  8 +-------
 target/s390x/sigp.c     | 15 ++++++++++++++-
 3 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/target/s390x/internal.h b/target/s390x/internal.h
index 99413beeae..2e738de6af 100644
--- a/target/s390x/internal.h
+++ b/target/s390x/internal.h
@@ -400,5 +400,6 @@ void s390x_translate_init(void);
 
 /* sigp.c */
 int handle_sigp(CPUS390XState *env, uint8_t order, uint64_t r1, uint64_t r3);
+void do_stop_interrupt(CPUS390XState *env);
 
 #endif /* S390X_INTERNAL_H */
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 142b63eef0..eb8b297a88 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -1594,13 +1594,7 @@ static int handle_intercept(S390CPU *cpu)
             r = EXCP_HALTED;
             break;
         case ICPT_CPU_STOP:
-            if (s390_cpu_set_state(CPU_STATE_STOPPED, cpu) == 0) {
-                qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
-            }
-            if (cpu->env.sigp_order == SIGP_STOP_STORE_STATUS) {
-                s390_store_status(cpu, S390_STORE_STATUS_DEF_ADDR, true);
-            }
-            cpu->env.sigp_order = 0;
+            do_stop_interrupt(&cpu->env);
             r = EXCP_HALTED;
             break;
         case ICPT_OPEREXC:
diff --git a/target/s390x/sigp.c b/target/s390x/sigp.c
index 4813123aad..9587c3d319 100644
--- a/target/s390x/sigp.c
+++ b/target/s390x/sigp.c
@@ -81,7 +81,7 @@ static void sigp_stop_and_store_status(CPUState *cs, run_on_cpu_data arg)
     case CPU_STATE_OPERATING:
         cpu->env.sigp_order = SIGP_STOP_STORE_STATUS;
         cpu_inject_stop(cpu);
-        /* store will be performed when handling the stop intercept */
+        /* store will be performed in do_stop_interrup() */
         break;
     case CPU_STATE_STOPPED:
         /* already stopped, just store the status */
@@ -360,6 +360,19 @@ int s390_cpu_restart(S390CPU *cpu)
     return 0;
 }
 
+void do_stop_interrupt(CPUS390XState *env)
+{
+    S390CPU *cpu = s390_env_get_cpu(env);
+
+    if (s390_cpu_set_state(CPU_STATE_STOPPED, cpu) == 0) {
+        qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
+    }
+    if (cpu->env.sigp_order == SIGP_STOP_STORE_STATUS) {
+        s390_store_status(cpu, S390_STORE_STATUS_DEF_ADDR, true);
+    }
+    env->sigp_order = 0;
+}
+
 void s390_init_sigp(void)
 {
     qemu_mutex_init(&qemu_sigp_mutex);
-- 
2.13.5

[Qemu-devel] [PATCH v1 18/27] s390x/tcg: implement SIGP SENSE RUNNING STATUS

[David Hildenbrand]

Preparation for TCG, for KVM is this is completely handled in the
kernel.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/cpu.h  |  2 ++
 target/s390x/sigp.c | 25 +++++++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 5d03802c7d..5aa755d7b5 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -594,6 +594,7 @@ struct sysib_322 {
 #define SIGP_SET_PREFIX        0x0d
 #define SIGP_STORE_STATUS_ADDR 0x0e
 #define SIGP_SET_ARCH          0x12
+#define SIGP_SENSE_RUNNING     0x15
 #define SIGP_STORE_ADTL_STATUS 0x17
 
 /* SIGP condition codes */
@@ -604,6 +605,7 @@ struct sysib_322 {
 
 /* SIGP status bits */
 #define SIGP_STAT_EQUIPMENT_CHECK   0x80000000UL
+#define SIGP_STAT_NOT_RUNNING       0x00000400UL
 #define SIGP_STAT_INCORRECT_STATE   0x00000200UL
 #define SIGP_STAT_INVALID_PARAMETER 0x00000100UL
 #define SIGP_STAT_EXT_CALL_PENDING  0x00000080UL
diff --git a/target/s390x/sigp.c b/target/s390x/sigp.c
index 9587c3d319..c57312b743 100644
--- a/target/s390x/sigp.c
+++ b/target/s390x/sigp.c
@@ -234,6 +234,28 @@ static void sigp_set_prefix(CPUState *cs, run_on_cpu_data arg)
     si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
 }
 
+static void sigp_sense_running(S390CPU *dst_cpu, SigpInfo *si)
+{
+    if (!tcg_enabled()) {
+        /* handled in KVM */
+        set_sigp_status(si, SIGP_STAT_INVALID_ORDER);
+        return;
+    }
+
+    /* sensing without locks is racy, but it's the same for real hw */
+    if (!s390_has_feat(S390_FEAT_SENSE_RUNNING_STATUS)) {
+        set_sigp_status(si, SIGP_STAT_INVALID_ORDER);
+        return;
+    }
+
+    /* If halted (which includes also STOPPED), it is not running */
+    if (CPU(dst_cpu)->halted) {
+        si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
+    } else {
+        set_sigp_status(si, SIGP_STAT_NOT_RUNNING);
+    }
+}
+
 static int handle_sigp_single_dst(S390CPU *dst_cpu, uint8_t order,
                                   uint64_t param, uint64_t *status_reg)
 {
@@ -282,6 +304,9 @@ static int handle_sigp_single_dst(S390CPU *dst_cpu, uint8_t order,
     case SIGP_CPU_RESET:
         run_on_cpu(CPU(dst_cpu), sigp_cpu_reset, RUN_ON_CPU_HOST_PTR(&si));
         break;
+    case SIGP_SENSE_RUNNING:
+        sigp_sense_running(dst_cpu, &si);
+        break;
     default:
         set_sigp_status(&si, SIGP_STAT_INVALID_ORDER);
     }
-- 
2.13.5

[Qemu-devel] [PATCH v1 19/27] s390x/tcg: implement SIGP SENSE

[David Hildenbrand]

Add it as preparation for TCG. Sensing coul later be done completely
lockless.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/sigp.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/target/s390x/sigp.c b/target/s390x/sigp.c
index c57312b743..063a34ccc7 100644
--- a/target/s390x/sigp.c
+++ b/target/s390x/sigp.c
@@ -32,6 +32,32 @@ static void set_sigp_status(SigpInfo *si, uint64_t status)
     si->cc = SIGP_CC_STATUS_STORED;
 }
 
+static void sigp_sense(S390CPU *dst_cpu, SigpInfo *si)
+{
+    uint8_t state = s390_cpu_get_state(dst_cpu);
+    bool ext_call = dst_cpu->env.pending_int & INTERRUPT_EXTERNAL_CALL;
+    uint64_t status = 0;
+
+    if (!tcg_enabled()) {
+        /* handled in KVM */
+        set_sigp_status(si, SIGP_STAT_INVALID_ORDER);
+        return;
+    }
+
+    /* sensing without locks is racy, but it's the same for real hw */
+    if (state != CPU_STATE_STOPPED && !ext_call) {
+        si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
+    } else {
+        if (ext_call) {
+            status |= SIGP_STAT_EXT_CALL_PENDING;
+        }
+        if (state == CPU_STATE_STOPPED) {
+            status |= SIGP_STAT_STOPPED;
+        }
+        set_sigp_status(si, status);
+    }
+}
+
 static void sigp_start(CPUState *cs, run_on_cpu_data arg)
 {
     S390CPU *cpu = S390_CPU(cs);
@@ -277,6 +303,9 @@ static int handle_sigp_single_dst(S390CPU *dst_cpu, uint8_t order,
     }
 
     switch (order) {
+    case SIGP_SENSE:
+        sigp_sense(dst_cpu, &si);
+        break;
     case SIGP_START:
         run_on_cpu(CPU(dst_cpu), sigp_start, RUN_ON_CPU_HOST_PTR(&si));
         break;
-- 
2.13.5

[Qemu-devel] [PATCH v1 20/27] s390x/tcg: implement SIGP EXTERNAL CALL

[David Hildenbrand]

As preparation for TCG.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/sigp.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/target/s390x/sigp.c b/target/s390x/sigp.c
index 063a34ccc7..080bce3fba 100644
--- a/target/s390x/sigp.c
+++ b/target/s390x/sigp.c
@@ -58,6 +58,24 @@ static void sigp_sense(S390CPU *dst_cpu, SigpInfo *si)
     }
 }
 
+static void sigp_external_call(S390CPU *src_cpu, S390CPU *dst_cpu, SigpInfo *si)
+{
+    int ret;
+
+    if (!tcg_enabled()) {
+        /* handled in KVM */
+        set_sigp_status(si, SIGP_STAT_INVALID_ORDER);
+        return;
+    }
+
+    ret = cpu_inject_external_call(dst_cpu, src_cpu->env.core_id);
+    if (!ret) {
+        si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
+    } else {
+        set_sigp_status(si, SIGP_STAT_EXT_CALL_PENDING);
+    }
+}
+
 static void sigp_start(CPUState *cs, run_on_cpu_data arg)
 {
     S390CPU *cpu = S390_CPU(cs);
@@ -282,7 +300,7 @@ static void sigp_sense_running(S390CPU *dst_cpu, SigpInfo *si)
     }
 }
 
-static int handle_sigp_single_dst(S390CPU *dst_cpu, uint8_t order,
+static int handle_sigp_single_dst(S390CPU *cpu, S390CPU *dst_cpu, uint8_t order,
                                   uint64_t param, uint64_t *status_reg)
 {
     SigpInfo si = {
@@ -306,6 +324,9 @@ static int handle_sigp_single_dst(S390CPU *dst_cpu, uint8_t order,
     case SIGP_SENSE:
         sigp_sense(dst_cpu, &si);
         break;
+    case SIGP_EXTERNAL_CALL:
+        sigp_external_call(cpu, dst_cpu, &si);
+        break;
     case SIGP_START:
         run_on_cpu(CPU(dst_cpu), sigp_start, RUN_ON_CPU_HOST_PTR(&si));
         break;
@@ -389,7 +410,7 @@ int handle_sigp(CPUS390XState *env, uint8_t order, uint64_t r1, uint64_t r3)
     default:
         /* all other sigp orders target a single vcpu */
         dst_cpu = s390_cpu_addr2state(env->regs[r3]);
-        ret = handle_sigp_single_dst(dst_cpu, order, param, status_reg);
+        ret = handle_sigp_single_dst(cpu, dst_cpu, order, param, status_reg);
     }
     qemu_mutex_unlock(&qemu_sigp_mutex);
 
-- 
2.13.5

[Qemu-devel] [PATCH v1 21/27] s390x/tcg: implement SIGP EMERGENCY SIGNAL

[David Hildenbrand]

As preparation for TCG.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/sigp.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/target/s390x/sigp.c b/target/s390x/sigp.c
index 080bce3fba..d492885787 100644
--- a/target/s390x/sigp.c
+++ b/target/s390x/sigp.c
@@ -76,6 +76,18 @@ static void sigp_external_call(S390CPU *src_cpu, S390CPU *dst_cpu, SigpInfo *si)
     }
 }
 
+static void sigp_emergency(S390CPU *src_cpu, S390CPU *dst_cpu, SigpInfo *si)
+{
+    if (!tcg_enabled()) {
+        /* handled in KVM */
+        set_sigp_status(si, SIGP_STAT_INVALID_ORDER);
+        return;
+    }
+
+    cpu_inject_emergency_signal(dst_cpu, src_cpu->env.core_id);
+    si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
+}
+
 static void sigp_start(CPUState *cs, run_on_cpu_data arg)
 {
     S390CPU *cpu = S390_CPU(cs);
@@ -327,6 +339,9 @@ static int handle_sigp_single_dst(S390CPU *cpu, S390CPU *dst_cpu, uint8_t order,
     case SIGP_EXTERNAL_CALL:
         sigp_external_call(cpu, dst_cpu, &si);
         break;
+    case SIGP_EMERGENCY:
+        sigp_emergency(cpu, dst_cpu, &si);
+        break;
     case SIGP_START:
         run_on_cpu(CPU(dst_cpu), sigp_start, RUN_ON_CPU_HOST_PTR(&si));
         break;
-- 
2.13.5

[Qemu-devel] [PATCH v1 22/27] s390x/tcg: implement SIGP CONDITIONAL EMERGENCY SIGNAL

[David Hildenbrand]

Mostly analogous to the kernel/KVM version (so I assume the checks are
correct :) ). As a preparation for TCG.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/cpu.h  |  1 +
 target/s390x/sigp.c | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+)

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 5aa755d7b5..97d4abb6c0 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -594,6 +594,7 @@ struct sysib_322 {
 #define SIGP_SET_PREFIX        0x0d
 #define SIGP_STORE_STATUS_ADDR 0x0e
 #define SIGP_SET_ARCH          0x12
+#define SIGP_COND_EMERGENCY    0x13
 #define SIGP_SENSE_RUNNING     0x15
 #define SIGP_STORE_ADTL_STATUS 0x17
 
diff --git a/target/s390x/sigp.c b/target/s390x/sigp.c
index d492885787..ce8fda9d01 100644
--- a/target/s390x/sigp.c
+++ b/target/s390x/sigp.c
@@ -290,6 +290,40 @@ static void sigp_set_prefix(CPUState *cs, run_on_cpu_data arg)
     si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
 }
 
+static void sigp_cond_emergency(S390CPU *src_cpu, S390CPU *dst_cpu,
+                                SigpInfo *si)
+{
+    const uint64_t psw_int_mask = PSW_MASK_IO | PSW_MASK_EXT;
+    uint16_t p_asn, s_asn, asn;
+    uint64_t psw_addr, psw_mask;
+    bool idle;
+
+    if (!tcg_enabled()) {
+        /* handled in KVM */
+        set_sigp_status(si, SIGP_STAT_INVALID_ORDER);
+        return;
+    }
+
+    /* this looks racy, but these values are only used when STOPPED */
+    idle = CPU(dst_cpu)->halted;
+    psw_addr = dst_cpu->env.psw.addr;
+    psw_mask = dst_cpu->env.psw.mask;
+    asn = si->param;
+    p_asn = dst_cpu->env.cregs[4] & 0xffff;  /* Primary ASN */
+    s_asn = dst_cpu->env.cregs[3] & 0xffff;  /* Secondary ASN */
+
+    if (s390_cpu_get_state(dst_cpu) != CPU_STATE_STOPPED ||
+        (psw_mask & psw_int_mask) != psw_int_mask ||
+        (idle && psw_addr != 0) ||
+        (!idle && (asn == p_asn || asn == s_asn))) {
+        cpu_inject_emergency_signal(dst_cpu, src_cpu->env.core_id);
+    } else {
+        set_sigp_status(si, SIGP_STAT_INCORRECT_STATE);
+    }
+
+    si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
+}
+
 static void sigp_sense_running(S390CPU *dst_cpu, SigpInfo *si)
 {
     if (!tcg_enabled()) {
@@ -369,6 +403,9 @@ static int handle_sigp_single_dst(S390CPU *cpu, S390CPU *dst_cpu, uint8_t order,
     case SIGP_CPU_RESET:
         run_on_cpu(CPU(dst_cpu), sigp_cpu_reset, RUN_ON_CPU_HOST_PTR(&si));
         break;
+    case SIGP_COND_EMERGENCY:
+        sigp_cond_emergency(cpu, dst_cpu, &si);
+        break;
     case SIGP_SENSE_RUNNING:
         sigp_sense_running(dst_cpu, &si);
         break;
-- 
2.13.5

[Qemu-devel] [PATCH v1 23/27] s390x/tcg: implement STOP and RESET interrupts for TCG

[David Hildenbrand]

Implement them like KVM implements/handles them. Both can only be
triggered via SIGP instructions. RESET has (almos)the lowest priority if
the CPU is running, and the highest if the CPU is STOPPED. This is handled
in SIGP code already. On delivery, we only have to care about the
"CPU running" scenario.

STOP is defined to be delivered after all other interrupts have been
delivered. Therefore it has the actual lowest priority.

As both can wake up a CPU if sleeping, indicate them correctly to
external code (e.g. cpu_has_work()).

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/cpu.c         |  4 +++-
 target/s390x/cpu.h         |  4 ++++
 target/s390x/excp_helper.c | 18 +++++++++++++++++-
 target/s390x/helper.c      |  1 +
 target/s390x/interrupt.c   | 14 ++++++++++----
 target/s390x/sigp.c        |  1 +
 6 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index 9a01f591b6..9d20434075 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -65,7 +65,9 @@ static bool s390_cpu_has_work(CPUState *cs)
     }
 
     return (cs->interrupt_request & CPU_INTERRUPT_HARD) &&
-           (env->psw.mask & PSW_MASK_EXT);
+            ((env->psw.mask & PSW_MASK_EXT) ||
+             env->pending_int & INTERRUPT_STOP ||
+             env->pending_int & INTERRUPT_RESTART);
 }
 
 #if !defined(CONFIG_USER_ONLY)
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 97d4abb6c0..fc1f622c52 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -399,6 +399,8 @@ static inline void cpu_get_tb_cpu_state(CPUS390XState* env, target_ulong *pc,
 #define EXCP_EXT 1 /* external interrupt */
 #define EXCP_SVC 2 /* supervisor call (syscall) */
 #define EXCP_PGM 3 /* program interruption */
+#define EXCP_RESTART 4 /* restart interrupt */
+#define EXCP_STOP 5 /* stop interrupt */
 #define EXCP_IO  7 /* I/O interrupt */
 #define EXCP_MCHK 8 /* machine check */
 
@@ -409,6 +411,8 @@ static inline void cpu_get_tb_cpu_state(CPUS390XState* env, target_ulong *pc,
 #define INTERRUPT_EXT_CLOCK_COMPARATOR   (1 << 4)
 #define INTERRUPT_EXTERNAL_CALL          (1 << 5)
 #define INTERRUPT_EMERGENCY_SIGNAL       (1 << 6)
+#define INTERRUPT_RESTART                (1 << 7)
+#define INTERRUPT_STOP                   (1 << 8)
 #define INTERRUPT_EXT                    (INTERRUPT_EXT_FLOATING | \
                                           INTERRUPT_EXT_CPU_TIMER | \
                                           INTERRUPT_EXT_CLOCK_COMPARATOR | \
diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
index 4dc30e056a..ce28708499 100644
--- a/target/s390x/excp_helper.c
+++ b/target/s390x/excp_helper.c
@@ -452,6 +452,14 @@ void s390_cpu_do_interrupt(CPUState *cs)
             cs->exception_index = EXCP_IO;
         }
     }
+    /* RESTART interrupt */
+    if (cs->exception_index == -1 && env->pending_int & INTERRUPT_RESTART) {
+        cs->exception_index = EXCP_RESTART;
+    }
+    /* STOP interrupt has least priority */
+    if (cs->exception_index == -1 && env->pending_int & INTERRUPT_STOP) {
+        cs->exception_index = EXCP_STOP;
+    }
 
     switch (cs->exception_index) {
     case EXCP_PGM:
@@ -469,6 +477,12 @@ void s390_cpu_do_interrupt(CPUState *cs)
     case EXCP_MCHK:
         do_mchk_interrupt(env);
         break;
+    case EXCP_RESTART:
+        do_restart_interrupt(env);
+        break;
+    case EXCP_STOP:
+        do_stop_interrupt(env);
+        break;
     }
     cs->exception_index = -1;
 
@@ -488,7 +502,9 @@ bool s390_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
                the parent EXECUTE insn.  */
             return false;
         }
-        if (env->psw.mask & PSW_MASK_EXT) {
+        if (env->psw.mask & PSW_MASK_EXT ||
+            env->pending_int & INTERRUPT_STOP ||
+            env->pending_int & INTERRUPT_RESTART) {
             s390_cpu_do_interrupt(cs);
             return true;
         }
diff --git a/target/s390x/helper.c b/target/s390x/helper.c
index 4d5a92c2ca..abda74d810 100644
--- a/target/s390x/helper.c
+++ b/target/s390x/helper.c
@@ -202,6 +202,7 @@ void do_restart_interrupt(CPUS390XState *env)
     addr = be64_to_cpu(lowcore->restart_new_psw.addr);
 
     cpu_unmap_lowcore(lowcore);
+    env->pending_int &= ~INTERRUPT_RESTART;
 
     load_psw(env, mask, addr);
 }
diff --git a/target/s390x/interrupt.c b/target/s390x/interrupt.c
index b1dc28c8dd..2f004ffe6d 100644
--- a/target/s390x/interrupt.c
+++ b/target/s390x/interrupt.c
@@ -119,22 +119,28 @@ int cpu_inject_external_call(S390CPU *cpu, uint16_t src_cpu_addr)
 
 void cpu_inject_restart(S390CPU *cpu)
 {
+    CPUS390XState *env = &cpu->env;
+
     if (kvm_enabled()) {
         kvm_s390_restart_interrupt(cpu);
         return;
     }
-    /* FIXME TCG */
-    g_assert_not_reached();
+
+    env->pending_int |= INTERRUPT_RESTART;
+    cpu_interrupt(CPU(cpu), CPU_INTERRUPT_HARD);
 }
 
 void cpu_inject_stop(S390CPU *cpu)
 {
+    CPUS390XState *env = &cpu->env;
+
     if (kvm_enabled()) {
         kvm_s390_stop_interrupt(cpu);
         return;
     }
-    /* FIXME TCG */
-    g_assert_not_reached();
+
+    env->pending_int |= INTERRUPT_STOP;
+    cpu_interrupt(CPU(cpu), CPU_INTERRUPT_HARD);
 }
 
 static void cpu_inject_io(S390CPU *cpu, uint16_t subchannel_id,
diff --git a/target/s390x/sigp.c b/target/s390x/sigp.c
index ce8fda9d01..521dcc75f3 100644
--- a/target/s390x/sigp.c
+++ b/target/s390x/sigp.c
@@ -498,6 +498,7 @@ void do_stop_interrupt(CPUS390XState *env)
         s390_store_status(cpu, S390_STORE_STATUS_DEF_ADDR, true);
     }
     env->sigp_order = 0;
+    env->pending_int &= ~INTERRUPT_STOP;
 }
 
 void s390_init_sigp(void)
-- 
2.13.5

[Qemu-devel] [PATCH v1 24/27] s390x/tcg: flush the tlb on SIGP SET PREFIX

[David Hildenbrand]

Thanks to Aurelien Jarno for doing this in his prototype.

We can flush the whole TLB as this should happen really rarely.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/sigp.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/s390x/sigp.c b/target/s390x/sigp.c
index 521dcc75f3..c5a5dac911 100644
--- a/target/s390x/sigp.c
+++ b/target/s390x/sigp.c
@@ -14,6 +14,7 @@
 #include "internal.h"
 #include "sysemu/hw_accel.h"
 #include "exec/address-spaces.h"
+#include "exec/exec-all.h"
 #include "sysemu/sysemu.h"
 #include "trace.h"
 
@@ -286,6 +287,7 @@ static void sigp_set_prefix(CPUState *cs, run_on_cpu_data arg)
     }
 
     cpu->env.psa = addr;
+    tlb_flush(cs);
     cpu_synchronize_post_init(cs);
     si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
 }
-- 
2.13.5

[Qemu-devel] [PATCH v1 25/27] s390x/tcg: switch to new SIGP handling code

[David Hildenbrand]

This effectively enables experimental SMP support. Floating interrupts are
still a mess, so allow it but print a big warning. There also seems
to be a problem with CPU hotplug (after the main loop started).

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 hw/s390x/s390-virtio-ccw.c |  4 +---
 target/s390x/helper.h      |  2 +-
 target/s390x/misc_helper.c | 42 ++++++------------------------------------
 target/s390x/translate.c   |  5 +++--
 4 files changed, 11 insertions(+), 42 deletions(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 49c1ab29ed..732feb9cff 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -65,9 +65,7 @@ static void s390_init_cpus(MachineState *machine)
         machine->cpu_model = s390_default_cpu_model_name();
     }
     if (tcg_enabled() && max_cpus > 1) {
-        error_report("Number of SMP CPUs requested (%d) exceeds max CPUs "
-                     "supported by TCG (1) on s390x", max_cpus);
-        exit(1);
+        error_report("WARNING: SMP support on s390x is experimental!");
     }
 
     /* initialize possible_cpus */
diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 4b0290774e..a0e4bfb850 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -138,7 +138,7 @@ DEF_HELPER_FLAGS_3(sske, TCG_CALL_NO_RWG, void, env, i64, i64)
 DEF_HELPER_FLAGS_2(rrbe, TCG_CALL_NO_RWG, i32, env, i64)
 DEF_HELPER_4(mvcs, i32, env, i64, i64, i64)
 DEF_HELPER_4(mvcp, i32, env, i64, i64, i64)
-DEF_HELPER_4(sigp, i32, env, i64, i32, i64)
+DEF_HELPER_4(sigp, i32, env, i64, i32, i32)
 DEF_HELPER_FLAGS_2(sacf, TCG_CALL_NO_WG, void, env, i64)
 DEF_HELPER_FLAGS_4(idte, TCG_CALL_NO_RWG, void, env, i64, i64, i32)
 DEF_HELPER_FLAGS_4(ipte, TCG_CALL_NO_RWG, void, env, i64, i64, i32)
diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
index 293fc8428a..1e4d3c8526 100644
--- a/target/s390x/misc_helper.c
+++ b/target/s390x/misc_helper.c
@@ -319,44 +319,14 @@ uint32_t HELPER(stsi)(CPUS390XState *env, uint64_t a0,
 }
 
 uint32_t HELPER(sigp)(CPUS390XState *env, uint64_t order_code, uint32_t r1,
-                      uint64_t cpu_addr)
+                      uint32_t r3)
 {
-    int cc = SIGP_CC_ORDER_CODE_ACCEPTED;
+    int cc;
 
-    HELPER_LOG("%s: %016" PRIx64 " %08x %016" PRIx64 "\n",
-               __func__, order_code, r1, cpu_addr);
-
-    /* Remember: Use "R1 or R1 + 1, whichever is the odd-numbered register"
-       as parameter (input). Status (output) is always R1. */
-
-    switch (order_code & SIGP_ORDER_MASK) {
-    case SIGP_SET_ARCH:
-        /* switch arch */
-        break;
-    case SIGP_SENSE:
-        /* enumerate CPU status */
-        if (cpu_addr) {
-            /* XXX implement when SMP comes */
-            return 3;
-        }
-        env->regs[r1] &= 0xffffffff00000000ULL;
-        cc = 1;
-        break;
-#if !defined(CONFIG_USER_ONLY)
-    case SIGP_RESTART:
-        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
-        cpu_loop_exit(CPU(s390_env_get_cpu(env)));
-        break;
-    case SIGP_STOP:
-        qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
-        cpu_loop_exit(CPU(s390_env_get_cpu(env)));
-        break;
-#endif
-    default:
-        /* unknown sigp */
-        fprintf(stderr, "XXX unknown sigp: 0x%" PRIx64 "\n", order_code);
-        cc = SIGP_CC_NOT_OPERATIONAL;
-    }
+    /* TODO: needed to inject interrupts  - push further down */
+    qemu_mutex_lock_iothread();
+    cc = handle_sigp(env, order_code & SIGP_ORDER_MASK, r1, r3);
+    qemu_mutex_unlock_iothread();
 
     return cc;
 }
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 5abd34fb34..abf2feb7aa 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -3658,11 +3658,12 @@ static ExitStatus op_servc(DisasContext *s, DisasOps *o)
 static ExitStatus op_sigp(DisasContext *s, DisasOps *o)
 {
     TCGv_i32 r1 = tcg_const_i32(get_field(s->fields, r1));
+    TCGv_i32 r3 = tcg_const_i32(get_field(s->fields, r3));
     check_privileged(s);
-    potential_page_fault(s);
-    gen_helper_sigp(cc_op, cpu_env, o->in2, r1, o->in1);
+    gen_helper_sigp(cc_op, cpu_env, o->in2, r1, r3);
     set_cc_static(s);
     tcg_temp_free_i32(r1);
+    tcg_temp_free_i32(r3);
     return NO_EXIT;
 }
 #endif
-- 
2.13.5

[Qemu-devel] [PATCH v1 26/27] s390x/tcg: unlock NMI

[David Hildenbrand]

Nothing hindering us anymore from unlocking the restart code (used for
NMI).

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 hw/s390x/s390-virtio-ccw.c | 4 +---
 target/s390x/sigp.c        | 5 -----
 2 files changed, 1 insertion(+), 8 deletions(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 732feb9cff..c5c989f673 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -418,9 +418,7 @@ static void s390_nmi(NMIState *n, int cpu_index, Error **errp)
 {
     CPUState *cs = qemu_get_cpu(cpu_index);
 
-    if (s390_cpu_restart(S390_CPU(cs))) {
-        error_setg(errp, QERR_UNSUPPORTED);
-    }
+    s390_cpu_restart(S390_CPU(cs));
 }
 
 static void ccw_machine_class_init(ObjectClass *oc, void *data)
diff --git a/target/s390x/sigp.c b/target/s390x/sigp.c
index c5a5dac911..964c75a736 100644
--- a/target/s390x/sigp.c
+++ b/target/s390x/sigp.c
@@ -480,11 +480,6 @@ int s390_cpu_restart(S390CPU *cpu)
 {
     SigpInfo si = {};
 
-    if (tcg_enabled()) {
-        /* FIXME TCG */
-        return -ENOSYS;
-    }
-
     run_on_cpu(CPU(cpu), sigp_restart, RUN_ON_CPU_HOST_PTR(&si));
     return 0;
 }
-- 
2.13.5

[Qemu-devel] [PATCH v1 27/27] s390x/tcg: refactor stfl(e) to use s390_get_feat_block()

[David Hildenbrand]

Refactor it to use s390_get_feat_block(). Directly write into the mapped
lowcore with stfl and make sure it is really only compiled if needed.

While at it, add an alignment check for STFLE and avoid
potential_page_fault() by properly restoring the CPU state.

Due to s390_get_feat_block(), we will now also indicate the
"Configuration-z-architectural-mode", which is with new SIGP code the
right thing to do.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/helper.h      |  2 +-
 target/s390x/misc_helper.c | 72 ++++++++++++++++++++++------------------------
 target/s390x/translate.c   |  1 -
 3 files changed, 35 insertions(+), 40 deletions(-)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index a0e4bfb850..a6d94d2d82 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -104,7 +104,6 @@ DEF_HELPER_FLAGS_5(calc_cc, TCG_CALL_NO_RWG_SE, i32, env, i32, i64, i64, i64)
 DEF_HELPER_FLAGS_2(sfpc, TCG_CALL_NO_RWG, void, env, i64)
 DEF_HELPER_FLAGS_2(sfas, TCG_CALL_NO_WG, void, env, i64)
 DEF_HELPER_FLAGS_1(popcnt, TCG_CALL_NO_RWG_SE, i64, i64)
-DEF_HELPER_FLAGS_1(stfl, TCG_CALL_NO_RWG, void, env)
 DEF_HELPER_2(stfle, i32, env, i64)
 DEF_HELPER_FLAGS_2(lpq, TCG_CALL_NO_WG, i64, env, i64)
 DEF_HELPER_FLAGS_4(stpq, TCG_CALL_NO_WG, void, env, i64, i64, i64)
@@ -152,6 +151,7 @@ DEF_HELPER_FLAGS_3(sturg, TCG_CALL_NO_WG, void, env, i64, i64)
 DEF_HELPER_1(per_check_exception, void, env)
 DEF_HELPER_FLAGS_3(per_branch, TCG_CALL_NO_RWG, void, env, i64, i64)
 DEF_HELPER_FLAGS_2(per_ifetch, TCG_CALL_NO_RWG, void, env, i64)
+DEF_HELPER_FLAGS_1(stfl, TCG_CALL_NO_RWG, void, env)
 
 DEF_HELPER_2(xsch, void, env, i64)
 DEF_HELPER_2(csch, void, env, i64)
diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
index 1e4d3c8526..4afd90b969 100644
--- a/target/s390x/misc_helper.c
+++ b/target/s390x/misc_helper.c
@@ -475,61 +475,57 @@ void HELPER(per_ifetch)(CPUS390XState *env, uint64_t addr)
 }
 #endif
 
-/* The maximum bit defined at the moment is 129.  */
-#define MAX_STFL_WORDS  3
+static uint8_t stfl_bytes[2048];
+static unsigned int used_stfl_bytes;
 
-/* Canonicalize the current cpu's features into the 64-bit words required
-   by STFLE.  Return the index-1 of the max word that is non-zero.  */
-static unsigned do_stfle(CPUS390XState *env, uint64_t words[MAX_STFL_WORDS])
+static void prepare_stfl(void)
 {
-    S390CPU *cpu = s390_env_get_cpu(env);
-    const unsigned long *features = cpu->model->features;
-    unsigned max_bit = 0;
-    S390Feat feat;
-
-    memset(words, 0, sizeof(uint64_t) * MAX_STFL_WORDS);
+    static bool initialized;
+    int i;
 
-    if (test_bit(S390_FEAT_ZARCH, features)) {
-        /* z/Architecture is always active if around */
-        words[0] = 1ull << (63 - 2);
+    /* racy, but we don't care, the same values are always written */
+    if (initialized) {
+        return;
     }
 
-    for (feat = find_first_bit(features, S390_FEAT_MAX);
-         feat < S390_FEAT_MAX;
-         feat = find_next_bit(features, S390_FEAT_MAX, feat + 1)) {
-        const S390FeatDef *def = s390_feat_def(feat);
-        if (def->type == S390_FEAT_TYPE_STFL) {
-            unsigned bit = def->bit;
-            if (bit > max_bit) {
-                max_bit = bit;
-            }
-            assert(bit / 64 < MAX_STFL_WORDS);
-            words[bit / 64] |= 1ULL << (63 - bit % 64);
+    s390_get_feat_block(S390_FEAT_TYPE_STFL, stfl_bytes);
+    for (i = 0; i < sizeof(stfl_bytes); i++) {
+        if (stfl_bytes[i]) {
+            used_stfl_bytes = i + 1;
         }
     }
-
-    return max_bit / 64;
+    initialized = true;
 }
 
+#ifndef CONFIG_USER_ONLY
 void HELPER(stfl)(CPUS390XState *env)
 {
-    uint64_t words[MAX_STFL_WORDS];
+    LowCore *lowcore;
 
-    do_stfle(env, words);
-    cpu_stl_data(env, 200, words[0] >> 32);
+    lowcore = cpu_map_lowcore(env);
+    prepare_stfl();
+    memcpy(&lowcore->stfl_fac_list, stfl_bytes, sizeof(lowcore->stfl_fac_list));
+    cpu_unmap_lowcore(lowcore);
 }
+#endif
 
 uint32_t HELPER(stfle)(CPUS390XState *env, uint64_t addr)
 {
-    uint64_t words[MAX_STFL_WORDS];
-    unsigned count_m1 = env->regs[0] & 0xff;
-    unsigned max_m1 = do_stfle(env, words);
-    unsigned i;
+    const uintptr_t ra = GETPC();
+    const int count_bytes = ((env->regs[0] & 0xff) + 1) * 8;
+    const int max_bytes = ROUND_UP(used_stfl_bytes, 8);
+    int i;
+
+    if (addr & 0x7) {
+        cpu_restore_state(ENV_GET_CPU(env), ra);
+        program_interrupt(env, PGM_SPECIFICATION, 4);
+    }
 
-    for (i = 0; i <= count_m1; ++i) {
-        cpu_stq_data(env, addr + 8 * i, words[i]);
+    prepare_stfl();
+    for (i = 0; i < count_bytes; ++i) {
+        cpu_stb_data_ra(env, addr + i, stfl_bytes[i], ra);
     }
 
-    env->regs[0] = deposit64(env->regs[0], 0, 8, max_m1);
-    return (count_m1 >= max_m1 ? 0 : 3);
+    env->regs[0] = deposit64(env->regs[0], 0, 8, (max_bytes / 8) - 1);
+    return count_bytes >= max_bytes ? 0 : 3;
 }
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index abf2feb7aa..0e05942fe9 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -4082,7 +4082,6 @@ static ExitStatus op_sturg(DisasContext *s, DisasOps *o)
 
 static ExitStatus op_stfle(DisasContext *s, DisasOps *o)
 {
-    potential_page_fault(s);
     gen_helper_stfle(cc_op, cpu_env, o->in2);
     set_cc_static(s);
     return NO_EXIT;
-- 
2.13.5

Re: [Qemu-devel] [PATCH v1 16/27] s390x/kvm: factor out SIGP code into sigp.c

[Christian Borntraeger]

On 09/18/2017 06:00 PM, David Hildenbrand wrote:
> We want to use the same code base for TCG, so let's cleanly factor it
> out.
> 
> The sigp mutex is currently not really needed, as everything is
> protected by the iothread mutex. But this could change later, so leave
> it in place and initialize it properly from common code.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  hw/s390x/s390-virtio-ccw.c |   3 +
>  target/s390x/Makefile.objs |   1 +
>  target/s390x/cpu.c         |   8 -
>  target/s390x/cpu.h         |   6 +-
>  target/s390x/internal.h    |   4 +
>  target/s390x/kvm-stub.c    |   5 -
>  target/s390x/kvm.c         | 349 +-----------------------------------------
>  target/s390x/kvm_s390x.h   |   1 -
>  target/s390x/sigp.c        | 366 +++++++++++++++++++++++++++++++++++++++++++++

You should probably do something in MAINTAINERS for sigp.c,
otherwise Conny and myself no longer maintain the sigp code.

Re: [Qemu-devel] [PATCH v1 00/27] s390x: SMP for TCG (+ cleanups)

[Christian Borntraeger]

David,

can you outline how much you tested KVM after these changes?  I assume we should
review/test the whole series properly?

On 09/18/2017 05:59 PM, David Hildenbrand wrote:
> This series contains:
> - properly implement local external interrupts for TCG
> - factor out KVM SIGP handling code into common code
> - implement missing SIGP orders for TCG handled by the kernel for KVM
>   (including STOP and RESTART interrupts)
> - make TCG use the new SIGP code - experimental SMP support for s390x TCG
> - refactor STFL(E) implementation for TCG
> - bunch of cleanups
> 
> Basically all SIGP instructions are fully supported.
> 
> Thanks to Aurelien Jarno for the initital prototype and showcasing that
> supporting experimental SMP code can be implemented quite easily.
> 
> TCG SMP on s390x - what works?
> - "-smp X,maxcpus=X" with both, single and multi threaded TCG
> - "-smp ... -device qemu-s390-cpu,id=cpuX,core-id=X"
> - system_powerdown, system_reset, shutdown, reboot, NMI
> - online/offline of CPUs from inside the guest
> 
> TCG SMP on s390x - what does not work?
> - Floating interrupts all target CPU 0. Don't offline it.
> - CPU hotplug after the machine/main loop has been fully setup
> -- the new CPU comes up, answers and sends emergency signals, but suddenly
>    the VM gets stuck. This is strange, as ordinary online/offline works
>    just fine.
> -- can be triggered by "cpu-add 1" + "system_reset". The system will hang
>    when trying to online CPUs. (note: in Linux code they are fully up and
>    running and already executed code)
> -- also if hotplugging with "-S", before anything has run. This is strange,
>    as "-device qemu-s390-cpu" works just fine.
> -- does not seem to be related to CPU setup/reset code, I checked that
> -- seems to be related to some TCG internals (as broken for single and
>    multi threaded TCG).
> -- common code seems to be somehow broken, not sure if this is even
>    expected to work (e.g. for single threaded TCG, hotplugged CPUs will
>    never get set "cpu->created = true". But doesn't seem to be related to
>    this)
> 
> 
> Based on: https://github.com/cohuck/qemu.git s390-next
> Available on: git@github.com:davidhildenbrand/qemu.git s390x-queue
> 
> 
> David Hildenbrand (27):
>   s390x: raise CPU hotplug irq after really hotplugged
>   s390x/cpumodel: fix max STFL(E) bit number
>   target/s390x: get rid of next_core_id
>   s390x: introduce and use S390_MAX_CPUS
>   s390/tcg: turn INTERRUPT_EXT into a mask
>   s390x/tcg: injection of emergency signals and extarnal calls
>   s390x/tcg: STOPPED cpus can never wake up
>   s390x/tcg: a CPU cannot switch state due to an interrupt
>   target/s390x: factor out handling of WAIT PSW into handle_wait()
>   s390x/kvm: pass ipb directly into handle_sigp()
>   s390x/kvm: generalize SIGP stop and restart interrupt injection
>   s390x/kvm: factor out storing of CPU status
>   target/s390x: proper cpu->be convertion in s390_store_status()
>   s390x/kvm: factor out storing of adtl CPU status
>   s390x/kvm: drop two debug prints
>   s390x/kvm: factor out SIGP code into sigp.c
>   s390x/kvm: factor out actual handling of STOP interrupts
>   s390x/tcg: implement SIGP SENSE RUNNING STATUS
>   s390x/tcg: implement SIGP SENSE
>   s390x/tcg: implement SIGP EXTERNAL CALL
>   s390x/tcg: implement SIGP EMERGENCY SIGNAL
>   s390x/tcg: implement SIGP CONDITIONAL EMERGENCY SIGNAL
>   s390x/tcg: implement STOP and RESET interrupts for TCG
>   s390x/tcg: flush the tlb on SIGP SET PREFIX
>   s390x/tcg: switch to new SIGP handling code
>   s390x/tcg: unlock NMI
>   s390x/tcg: refactor stfl(e) to use s390_get_feat_block()
> 
>  hw/s390x/s390-virtio-ccw.c  |  17 +-
>  target/s390x/Makefile.objs  |   1 +
>  target/s390x/cpu-qom.h      |   2 -
>  target/s390x/cpu.c          |  40 ++--
>  target/s390x/cpu.h          |  36 +++-
>  target/s390x/cpu_features.c |   2 +-
>  target/s390x/cpu_models.c   |   2 +
>  target/s390x/excp_helper.c  |  98 ++++++---
>  target/s390x/helper.c       | 115 ++++++++--
>  target/s390x/helper.h       |   4 +-
>  target/s390x/internal.h     |  15 ++
>  target/s390x/interrupt.c    |  70 +++++-
>  target/s390x/kvm-stub.c     |  13 +-
>  target/s390x/kvm.c          | 470 +++--------------------------------------
>  target/s390x/kvm_s390x.h    |   3 +-
>  target/s390x/misc_helper.c  | 114 ++++------
>  target/s390x/sigp.c         | 504 ++++++++++++++++++++++++++++++++++++++++++++
>  target/s390x/trace-events   |   4 +-
>  target/s390x/translate.c    |   6 +-
>  19 files changed, 896 insertions(+), 620 deletions(-)
>  create mode 100644 target/s390x/sigp.c
> 

Re: [Qemu-devel] [PATCH v1 16/27] s390x/kvm: factor out SIGP code into sigp.c

[David Hildenbrand]

On 18.09.2017 19:25, Christian Borntraeger wrote:
> 
> 
> On 09/18/2017 06:00 PM, David Hildenbrand wrote:
>> We want to use the same code base for TCG, so let's cleanly factor it
>> out.
>>
>> The sigp mutex is currently not really needed, as everything is
>> protected by the iothread mutex. But this could change later, so leave
>> it in place and initialize it properly from common code.
>>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>>  hw/s390x/s390-virtio-ccw.c |   3 +
>>  target/s390x/Makefile.objs |   1 +
>>  target/s390x/cpu.c         |   8 -
>>  target/s390x/cpu.h         |   6 +-
>>  target/s390x/internal.h    |   4 +
>>  target/s390x/kvm-stub.c    |   5 -
>>  target/s390x/kvm.c         | 349 +-----------------------------------------
>>  target/s390x/kvm_s390x.h   |   1 -
>>  target/s390x/sigp.c        | 366 +++++++++++++++++++++++++++++++++++++++++++++
> 
> You should probably do something in MAINTAINERS for sigp.c,
> otherwise Conny and myself no longer maintain the sigp code.
> 

Good point, I'll add a patch. (copying the maintainers from kvm.c -  you
and Conny  :) )

-- 

Thanks,

David

Re: [Qemu-devel] [PATCH v1 00/27] s390x: SMP for TCG (+ cleanups)

[David Hildenbrand]

On 18.09.2017 19:31, Christian Borntraeger wrote:
> David,
> 
> can you outline how much you tested KVM after these changes?  I assume we should
> review/test the whole series properly?

Related to my tests:

I did the usual shutdown/reboot/system_reset/online-offline test with
KVM. I remember also playing with CPU hotplug again. However, no
kvm-unit-tests so far (also not in my pipeline).


Related to the patches:

I tried to modify as little KVM code as possible. Most stuff really is
just moving code around. But still, subtle errors can happen.

E.g. for all new SIGP instructions, I keep existing behavior for KVM by
checking for tcg/kvm directly at the start at the handlers. So most
s390x/tcg patches should have no influence.

So reviewing all s390x/kvm patches and !tcg patches would be great. I
will certainly not complain if some IBM folks would also review the TCG
patches ;)

Special care should need:
- "target/s390x: proper cpu->be convertion in s390_store_status()"
-- to make sure I haven't done any stupid mistakes while converting
   (just realized s/convertion/conversion/ :) )
- "s390x/kvm: factor out SIGP code into sigp.c"
-- to make sure the new kvm_s390_handle_sigp() and handle_sigp() do the
   same thing as handle_sigp() did before.

"s390x/tcg: STOPPED cpus can never wake up" and
"s390x/tcg: implement STOP and RESET interrupts for TCG" modify a
function called also for kvm (s390_cpu_has_work()). It is expected to
return always "false" for kvm. This behavior should still be that way.

Thanks!

> 
> On 09/18/2017 05:59 PM, David Hildenbrand wrote:
>> This series contains:
>> - properly implement local external interrupts for TCG
>> - factor out KVM SIGP handling code into common code
>> - implement missing SIGP orders for TCG handled by the kernel for KVM
>>   (including STOP and RESTART interrupts)
>> - make TCG use the new SIGP code - experimental SMP support for s390x TCG
>> - refactor STFL(E) implementation for TCG
>> - bunch of cleanups
>>
>> Basically all SIGP instructions are fully supported.
>>
>> Thanks to Aurelien Jarno for the initital prototype and showcasing that
>> supporting experimental SMP code can be implemented quite easily.
>>
>> TCG SMP on s390x - what works?
>> - "-smp X,maxcpus=X" with both, single and multi threaded TCG
>> - "-smp ... -device qemu-s390-cpu,id=cpuX,core-id=X"
>> - system_powerdown, system_reset, shutdown, reboot, NMI
>> - online/offline of CPUs from inside the guest
>>
>> TCG SMP on s390x - what does not work?
>> - Floating interrupts all target CPU 0. Don't offline it.
>> - CPU hotplug after the machine/main loop has been fully setup
>> -- the new CPU comes up, answers and sends emergency signals, but suddenly
>>    the VM gets stuck. This is strange, as ordinary online/offline works
>>    just fine.
>> -- can be triggered by "cpu-add 1" + "system_reset". The system will hang
>>    when trying to online CPUs. (note: in Linux code they are fully up and
>>    running and already executed code)
>> -- also if hotplugging with "-S", before anything has run. This is strange,
>>    as "-device qemu-s390-cpu" works just fine.
>> -- does not seem to be related to CPU setup/reset code, I checked that
>> -- seems to be related to some TCG internals (as broken for single and
>>    multi threaded TCG).
>> -- common code seems to be somehow broken, not sure if this is even
>>    expected to work (e.g. for single threaded TCG, hotplugged CPUs will
>>    never get set "cpu->created = true". But doesn't seem to be related to
>>    this)
>>
>>
>> Based on: https://github.com/cohuck/qemu.git s390-next
>> Available on: git@github.com:davidhildenbrand/qemu.git s390x-queue
>>
>>
>> David Hildenbrand (27):
>>   s390x: raise CPU hotplug irq after really hotplugged
>>   s390x/cpumodel: fix max STFL(E) bit number
>>   target/s390x: get rid of next_core_id
>>   s390x: introduce and use S390_MAX_CPUS
>>   s390/tcg: turn INTERRUPT_EXT into a mask
>>   s390x/tcg: injection of emergency signals and extarnal calls
>>   s390x/tcg: STOPPED cpus can never wake up
>>   s390x/tcg: a CPU cannot switch state due to an interrupt
>>   target/s390x: factor out handling of WAIT PSW into handle_wait()
>>   s390x/kvm: pass ipb directly into handle_sigp()
>>   s390x/kvm: generalize SIGP stop and restart interrupt injection
>>   s390x/kvm: factor out storing of CPU status
>>   target/s390x: proper cpu->be convertion in s390_store_status()
>>   s390x/kvm: factor out storing of adtl CPU status
>>   s390x/kvm: drop two debug prints
>>   s390x/kvm: factor out SIGP code into sigp.c
>>   s390x/kvm: factor out actual handling of STOP interrupts
>>   s390x/tcg: implement SIGP SENSE RUNNING STATUS
>>   s390x/tcg: implement SIGP SENSE
>>   s390x/tcg: implement SIGP EXTERNAL CALL
>>   s390x/tcg: implement SIGP EMERGENCY SIGNAL
>>   s390x/tcg: implement SIGP CONDITIONAL EMERGENCY SIGNAL
>>   s390x/tcg: implement STOP and RESET interrupts for TCG
>>   s390x/tcg: flush the tlb on SIGP SET PREFIX
>>   s390x/tcg: switch to new SIGP handling code
>>   s390x/tcg: unlock NMI
>>   s390x/tcg: refactor stfl(e) to use s390_get_feat_block()
>>
>>  hw/s390x/s390-virtio-ccw.c  |  17 +-
>>  target/s390x/Makefile.objs  |   1 +
>>  target/s390x/cpu-qom.h      |   2 -
>>  target/s390x/cpu.c          |  40 ++--
>>  target/s390x/cpu.h          |  36 +++-
>>  target/s390x/cpu_features.c |   2 +-
>>  target/s390x/cpu_models.c   |   2 +
>>  target/s390x/excp_helper.c  |  98 ++++++---
>>  target/s390x/helper.c       | 115 ++++++++--
>>  target/s390x/helper.h       |   4 +-
>>  target/s390x/internal.h     |  15 ++
>>  target/s390x/interrupt.c    |  70 +++++-
>>  target/s390x/kvm-stub.c     |  13 +-
>>  target/s390x/kvm.c          | 470 +++--------------------------------------
>>  target/s390x/kvm_s390x.h    |   3 +-
>>  target/s390x/misc_helper.c  | 114 ++++------
>>  target/s390x/sigp.c         | 504 ++++++++++++++++++++++++++++++++++++++++++++
>>  target/s390x/trace-events   |   4 +-
>>  target/s390x/translate.c    |   6 +-
>>  19 files changed, 896 insertions(+), 620 deletions(-)
>>  create mode 100644 target/s390x/sigp.c
>>
> 


-- 

Thanks,

David

Re: [Qemu-devel] [PATCH v1 23/27] s390x/tcg: implement STOP and RESET interrupts for TCG

[David Hildenbrand]

>  static void cpu_inject_io(S390CPU *cpu, uint16_t subchannel_id,
> diff --git a/target/s390x/sigp.c b/target/s390x/sigp.c
> index ce8fda9d01..521dcc75f3 100644
> --- a/target/s390x/sigp.c
> +++ b/target/s390x/sigp.c
> @@ -498,6 +498,7 @@ void do_stop_interrupt(CPUS390XState *env)
>          s390_store_status(cpu, S390_STORE_STATUS_DEF_ADDR, true);
>      }
>      env->sigp_order = 0;
> +    env->pending_int &= ~INTERRUPT_STOP;

The *awesome* kvm-unit-tests (yes, the single sigp_stop() we do when
shutting down) just found a missing cpu_loop_exit()

diff --git a/target/s390x/sigp.c b/target/s390x/sigp.c
index ce8fda9d01..9aadc3ffdd 100644
--- a/target/s390x/sigp.c
+++ b/target/s390x/sigp.c
@@ -498,6 +498,11 @@ void do_stop_interrupt(CPUS390XState *env)
         s390_store_status(cpu, S390_STORE_STATUS_DEF_ADDR, true);
     }
     env->sigp_order = 0;
+    env->pending_int &= ~INTERRUPT_STOP;
+#ifdef CONFIG_TCG
+    /* the CPU has been stopped, we must immediately go out of the loop */
+    cpu_loop_exit(CPU(s390_env_get_cpu(env)));
+#endif
 }


>  }
>  
>  void s390_init_sigp(void)
> 


-- 

Thanks,

David

Re: [Qemu-devel] [PATCH v1 23/27] s390x/tcg: implement STOP and RESET interrupts for TCG

[David Hildenbrand]

On 18.09.2017 22:00, David Hildenbrand wrote:
> 
>>  static void cpu_inject_io(S390CPU *cpu, uint16_t subchannel_id,
>> diff --git a/target/s390x/sigp.c b/target/s390x/sigp.c
>> index ce8fda9d01..521dcc75f3 100644
>> --- a/target/s390x/sigp.c
>> +++ b/target/s390x/sigp.c
>> @@ -498,6 +498,7 @@ void do_stop_interrupt(CPUS390XState *env)
>>          s390_store_status(cpu, S390_STORE_STATUS_DEF_ADDR, true);
>>      }
>>      env->sigp_order = 0;
>> +    env->pending_int &= ~INTERRUPT_STOP;
> 
> The *awesome* kvm-unit-tests (yes, the single sigp_stop() we do when
> shutting down) just found a missing cpu_loop_exit()
> 
> diff --git a/target/s390x/sigp.c b/target/s390x/sigp.c
> index ce8fda9d01..9aadc3ffdd 100644
> --- a/target/s390x/sigp.c
> +++ b/target/s390x/sigp.c
> @@ -498,6 +498,11 @@ void do_stop_interrupt(CPUS390XState *env)
>          s390_store_status(cpu, S390_STORE_STATUS_DEF_ADDR, true);
>      }
>      env->sigp_order = 0;
> +    env->pending_int &= ~INTERRUPT_STOP;
> +#ifdef CONFIG_TCG
> +    /* the CPU has been stopped, we must immediately go out of the loop */
> +    cpu_loop_exit(CPU(s390_env_get_cpu(env)));
> +#endif
>  }
> 

if (tcg_enabled()) of course ...


-- 

Thanks,

David

Re: [Qemu-devel] [PATCH v1 00/27] s390x: SMP for TCG (+ cleanups)

[David Hildenbrand]

> David Hildenbrand (27):
>   s390x: raise CPU hotplug irq after really hotplugged
>   s390x/cpumodel: fix max STFL(E) bit number
>   target/s390x: get rid of next_core_id
>   s390x: introduce and use S390_MAX_CPUS

It probably makes sense to apply 1-3/4 before the SMP support.

Igor, can you take a look at Patch nr. 3?

>   s390/tcg: turn INTERRUPT_EXT into a mask
>   s390x/tcg: injection of emergency signals and extarnal calls
>   s390x/tcg: STOPPED cpus can never wake up
>   s390x/tcg: a CPU cannot switch state due to an interrupt
>   target/s390x: factor out handling of WAIT PSW into handle_wait()
>   s390x/kvm: pass ipb directly into handle_sigp()
>   s390x/kvm: generalize SIGP stop and restart interrupt injection
>   s390x/kvm: factor out storing of CPU status
>   target/s390x: proper cpu->be convertion in s390_store_status()
>   s390x/kvm: factor out storing of adtl CPU status
>   s390x/kvm: drop two debug prints
>   s390x/kvm: factor out SIGP code into sigp.c
>   s390x/kvm: factor out actual handling of STOP interrupts
>   s390x/tcg: implement SIGP SENSE RUNNING STATUS
>   s390x/tcg: implement SIGP SENSE
>   s390x/tcg: implement SIGP EXTERNAL CALL
>   s390x/tcg: implement SIGP EMERGENCY SIGNAL
>   s390x/tcg: implement SIGP CONDITIONAL EMERGENCY SIGNAL
>   s390x/tcg: implement STOP and RESET interrupts for TCG
>   s390x/tcg: flush the tlb on SIGP SET PREFIX
>   s390x/tcg: switch to new SIGP handling code
>   s390x/tcg: unlock NMI
>   s390x/tcg: refactor stfl(e) to use s390_get_feat_block()


-- 

Thanks,

David

Re: [Qemu-devel] [PATCH v1 03/27] target/s390x: get rid of next_core_id

[Igor Mammedov]

On Mon, 18 Sep 2017 17:59:48 +0200
David Hildenbrand <david@redhat.com> wrote:

> core_id is not needed by linux-user, as the core_id a.k.a. CPU address
> is only accessible from kernel space.
> 
> Therefore, drop next_core_id and make cpu_index get autoassigned again
> for linux-user.
> 
> While at it, shield core_id and cpuid completely from linux-user. cpuid
> can also only be queried from kernel space.
> 
> Suggested-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>

[...]

Re: [Qemu-devel] [PATCH v1 01/27] s390x: raise CPU hotplug irq after really hotplugged

[Christian Borntraeger]

On 09/18/2017 05:59 PM, David Hildenbrand wrote:
> Let's move it into the machine, so we trigger the IRQ after setting
> ms->possible_cpus (which SCLP uses to construct the list of
> online CPUs).
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>

Makes sense
Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>

> ---
>  hw/s390x/s390-virtio-ccw.c | 4 ++++
>  target/s390x/cpu.c         | 8 --------
>  2 files changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 0471407187..d5a12a2472 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -327,6 +327,10 @@ static void s390_cpu_plug(HotplugHandler *hotplug_dev,
> 
>      g_assert(!ms->possible_cpus->cpus[cpu->env.core_id].cpu);
>      ms->possible_cpus->cpus[cpu->env.core_id].cpu = OBJECT(dev);
> +
> +    if (dev->hotplugged) {
> +        raise_irq_cpu_hotplug();
> +    }
>  }
> 
>  static void s390_machine_reset(void)
> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
> index 34538c3ab9..4920da3625 100644
> --- a/target/s390x/cpu.c
> +++ b/target/s390x/cpu.c
> @@ -41,7 +41,6 @@
>  #include "hw/hw.h"
>  #include "sysemu/arch_init.h"
>  #include "sysemu/sysemu.h"
> -#include "hw/s390x/sclp.h"
>  #endif
> 
>  #define CR0_RESET       0xE0UL
> @@ -227,13 +226,6 @@ static void s390_cpu_realizefn(DeviceState *dev, Error **errp)
>  #endif
> 
>      scc->parent_realize(dev, &err);
> -
> -#if !defined(CONFIG_USER_ONLY)
> -    if (dev->hotplugged) {
> -        raise_irq_cpu_hotplug();
> -    }
> -#endif
> -
>  out:
>      error_propagate(errp, err);
>  }
> 

Re: [Qemu-devel] [PATCH v1 02/27] s390x/cpumodel: fix max STFL(E) bit number

[Christian Borntraeger]

Makes sense. I asked myself if there is code outside that does not zero out 2048-16k so
I looked into the kernel source and it looks like we always did the right thing
(and zeroed out the response buffer), so this should not uncover any bugs.

Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>

On 09/18/2017 05:59 PM, David Hildenbrand wrote:
> Not that it would matter in the near future, but it is actually 2048
> bytes, therefore 16384 possible bits.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/cpu_features.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
> index 1d3a036393..31a4676f05 100644
> --- a/target/s390x/cpu_features.c
> +++ b/target/s390x/cpu_features.c
> @@ -381,7 +381,7 @@ void s390_add_from_feat_block(S390FeatBitmap features, S390FeatType type,
> 
>      switch (type) {
>      case S390_FEAT_TYPE_STFL:
> -       nr_bits = 2048;
> +       nr_bits = 16384;
>         break;
>      case S390_FEAT_TYPE_PLO:
>         nr_bits = 256;
> 

Re: [Qemu-devel] [PATCH v1 02/27] s390x/cpumodel: fix max STFL(E) bit number

[Thomas Huth]

On 18.09.2017 17:59, David Hildenbrand wrote:
> Not that it would matter in the near future, but it is actually 2048
> bytes, therefore 16384 possible bits.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/cpu_features.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
> index 1d3a036393..31a4676f05 100644
> --- a/target/s390x/cpu_features.c
> +++ b/target/s390x/cpu_features.c
> @@ -381,7 +381,7 @@ void s390_add_from_feat_block(S390FeatBitmap features, S390FeatType type,
>  
>      switch (type) {
>      case S390_FEAT_TYPE_STFL:
> -       nr_bits = 2048;
> +       nr_bits = 16384;
>         break;
>      case S390_FEAT_TYPE_PLO:
>         nr_bits = 256;

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [Qemu-devel] [PATCH v1 04/27] s390x: introduce and use S390_MAX_CPUS

[Thomas Huth]

On 18.09.2017 17:59, David Hildenbrand wrote:
> Will be handy in the next patches.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  hw/s390x/s390-virtio-ccw.c | 2 +-
>  target/s390x/cpu.h         | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index d5a12a2472..a57d435cbf 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -443,7 +443,7 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data)
>      mc->no_parallel = 1;
>      mc->no_sdcard = 1;
>      mc->use_sclp = 1;
> -    mc->max_cpus = 248;
> +    mc->max_cpus = S390_MAX_CPUS;
>      mc->has_hotpluggable_cpus = true;
>      mc->get_hotplug_handler = s390_get_hotplug_handler;
>      mc->cpu_index_to_instance_props = s390_cpu_index_to_props;
> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> index f13aea3dd8..5f539b1057 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -59,6 +59,8 @@
>  #define PSW_MCHK_MASK 0x0004000000000000
>  #define PSW_IO_MASK 0x0200000000000000
>  
> +#define S390_MAX_CPUS 248
> +
>  typedef struct PSW {
>      uint64_t mask;
>      uint64_t addr;
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [Qemu-devel] [PATCH v1 09/27] target/s390x: factor out handling of WAIT PSW into handle_wait()

[Thomas Huth]

On 18.09.2017 17:59, David Hildenbrand wrote:
> This will now also detect crashes under TCG. We can directly use
> cpu->env.psw.addr instead of kvm_run, as we do a
> cpu_synchronize_state().
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/helper.c   | 28 ++++++++++++++++++++++------
>  target/s390x/internal.h |  1 +
>  target/s390x/kvm.c      | 15 +--------------
>  3 files changed, 24 insertions(+), 20 deletions(-)
> 
> diff --git a/target/s390x/helper.c b/target/s390x/helper.c
> index e22b93258b..75ceb0bf2b 100644
> --- a/target/s390x/helper.c
> +++ b/target/s390x/helper.c
> @@ -26,6 +26,7 @@
>  #include "qemu/timer.h"
>  #include "exec/exec-all.h"
>  #include "hw/s390x/ioinst.h"
> +#include "sysemu/hw_accel.h"
>  #ifndef CONFIG_USER_ONLY
>  #include "sysemu/sysemu.h"
>  #endif
> @@ -113,6 +114,26 @@ hwaddr s390_cpu_get_phys_addr_debug(CPUState *cs, vaddr vaddr)
>      return phys_addr;
>  }
>  
> +static inline bool is_special_wait_psw(uint64_t psw_addr)
> +{
> +    /* signal quiesce */
> +    return psw_addr == 0xfffUL;
> +}
> +
> +void handle_wait(S390CPU *cpu)

"handle_wait" is a very generic name ... could you maybe at least name
this "s390x_handle_wait" or so?

> +{
> +    cpu_synchronize_state(CPU(cpu));
> +    if (s390_cpu_halt(cpu) == 0) {
> +#ifndef CONFIG_USER_ONLY
> +        if (is_special_wait_psw(cpu->env.psw.addr)) {
> +            qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
> +        } else {
> +            qemu_system_guest_panicked(NULL);
> +        }
> +#endif
> +    }
> +}
> +
>  void load_psw(CPUS390XState *env, uint64_t mask, uint64_t addr)
>  {
>      uint64_t old_mask = env->psw.mask;
> @@ -128,12 +149,7 @@ void load_psw(CPUS390XState *env, uint64_t mask, uint64_t addr)
>      }
>  
>      if (mask & PSW_MASK_WAIT) {
> -        S390CPU *cpu = s390_env_get_cpu(env);
> -        if (s390_cpu_halt(cpu) == 0) {
> -#ifndef CONFIG_USER_ONLY
> -            qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
> -#endif
> -        }
> +        handle_wait(s390_env_get_cpu(env));
>      }
>  }
>  
> diff --git a/target/s390x/internal.h b/target/s390x/internal.h
> index 15743ec40f..cb331f35ea 100644
> --- a/target/s390x/internal.h
> +++ b/target/s390x/internal.h
> @@ -280,6 +280,7 @@ const char *cc_name(enum cc_op cc_op);
>  void load_psw(CPUS390XState *env, uint64_t mask, uint64_t addr);
>  uint32_t calc_cc(CPUS390XState *env, uint32_t cc_op, uint64_t src, uint64_t dst,
>                   uint64_t vr);
> +void handle_wait(S390CPU *cpu);
>  
>  
>  /* cpu.c */
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index 3f9983154f..14f864697d 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -1936,12 +1936,6 @@ static int handle_instruction(S390CPU *cpu, struct kvm_run *run)
>      return r;
>  }
>  
> -static bool is_special_wait_psw(CPUState *cs)
> -{
> -    /* signal quiesce */
> -    return cs->kvm_run->psw_addr == 0xfffUL;
> -}
> -
>  static void unmanageable_intercept(S390CPU *cpu, const char *str, int pswoffset)
>  {
>      CPUState *cs = CPU(cpu);
> @@ -2012,14 +2006,7 @@ static int handle_intercept(S390CPU *cpu)
>              break;
>          case ICPT_WAITPSW:
>              /* disabled wait, since enabled wait is handled in kernel */
> -            cpu_synchronize_state(cs);

Maybe we should rather keep the cpu_synchronize_state() here instead of
putting it in helper.c? No other function in helper.c seems to call that
on its own yet...

> -            if (s390_cpu_halt(cpu) == 0) {
> -                if (is_special_wait_psw(cs)) {
> -                    qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
> -                } else {
> -                    qemu_system_guest_panicked(NULL);
> -                }
> -            }
> +            handle_wait(cpu);
>              r = EXCP_HALTED;
>              break;
>          case ICPT_CPU_STOP:
> 

 Thomas

Re: [Qemu-devel] [PATCH v1 10/27] s390x/kvm: pass ipb directly into handle_sigp()

[Thomas Huth]

On 18.09.2017 17:59, David Hildenbrand wrote:
> No need to pass kvm_run.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/kvm.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index 14f864697d..da3f7e9895 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -1852,7 +1852,7 @@ static int sigp_set_architecture(S390CPU *cpu, uint32_t param,
>      return SIGP_CC_STATUS_STORED;
>  }
>  
> -static int handle_sigp(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1)
> +static int handle_sigp(S390CPU *cpu, uint32_t ipb, uint8_t ipa1)

Maybe order the parameters alphabetically now? I.e. first ipa, then ipb?

Anyway, feel free to add my:

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [Qemu-devel] [PATCH v1 15/27] s390x/kvm: drop two debug prints

[Thomas Huth]

On 18.09.2017 18:00, David Hildenbrand wrote:
> Preparation for moving it out of kvm.c.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/kvm.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index 01c4125d21..d759edf1c8 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -1640,7 +1640,6 @@ int kvm_s390_cpu_restart(S390CPU *cpu)
>      SigpInfo si = {};
>  
>      run_on_cpu(CPU(cpu), sigp_restart, RUN_ON_CPU_HOST_PTR(&si));
> -    DPRINTF("DONE: KVM cpu restart: %p\n", &cpu->env);
>      return 0;
>  }
>  
> @@ -1742,7 +1741,6 @@ static int handle_sigp_single_dst(S390CPU *dst_cpu, uint8_t order,
>          run_on_cpu(CPU(dst_cpu), sigp_cpu_reset, RUN_ON_CPU_HOST_PTR(&si));
>          break;
>      default:
> -        DPRINTF("KVM: unknown SIGP: 0x%x\n", order);
>          set_sigp_status(&si, SIGP_STAT_INVALID_ORDER);
>      }

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [Qemu-devel] [PATCH v1 18/27] s390x/tcg: implement SIGP SENSE RUNNING STATUS

[Thomas Huth]

On 18.09.2017 18:00, David Hildenbrand wrote:
> Preparation for TCG, for KVM is this is completely handled in the
> kernel.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/cpu.h  |  2 ++
>  target/s390x/sigp.c | 25 +++++++++++++++++++++++++
>  2 files changed, 27 insertions(+)
> 
> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> index 5d03802c7d..5aa755d7b5 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -594,6 +594,7 @@ struct sysib_322 {
>  #define SIGP_SET_PREFIX        0x0d
>  #define SIGP_STORE_STATUS_ADDR 0x0e
>  #define SIGP_SET_ARCH          0x12
> +#define SIGP_SENSE_RUNNING     0x15
>  #define SIGP_STORE_ADTL_STATUS 0x17
>  
>  /* SIGP condition codes */
> @@ -604,6 +605,7 @@ struct sysib_322 {
>  
>  /* SIGP status bits */
>  #define SIGP_STAT_EQUIPMENT_CHECK   0x80000000UL
> +#define SIGP_STAT_NOT_RUNNING       0x00000400UL
>  #define SIGP_STAT_INCORRECT_STATE   0x00000200UL
>  #define SIGP_STAT_INVALID_PARAMETER 0x00000100UL
>  #define SIGP_STAT_EXT_CALL_PENDING  0x00000080UL
> diff --git a/target/s390x/sigp.c b/target/s390x/sigp.c
> index 9587c3d319..c57312b743 100644
> --- a/target/s390x/sigp.c
> +++ b/target/s390x/sigp.c
> @@ -234,6 +234,28 @@ static void sigp_set_prefix(CPUState *cs, run_on_cpu_data arg)
>      si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
>  }
>  
> +static void sigp_sense_running(S390CPU *dst_cpu, SigpInfo *si)
> +{
> +    if (!tcg_enabled()) {
> +        /* handled in KVM */
> +        set_sigp_status(si, SIGP_STAT_INVALID_ORDER);
> +        return;
> +    }

If we're sure that this is always handled in the kernel, I think you
could simply do a "g_assert(tcg_enabled())" here instead?

 Thomas

Re: [Qemu-devel] [PATCH v1 18/27] s390x/tcg: implement SIGP SENSE RUNNING STATUS

[David Hildenbrand]

On 25.09.2017 14:47, Thomas Huth wrote:
> On 18.09.2017 18:00, David Hildenbrand wrote:
>> Preparation for TCG, for KVM is this is completely handled in the
>> kernel.
>>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>>  target/s390x/cpu.h  |  2 ++
>>  target/s390x/sigp.c | 25 +++++++++++++++++++++++++
>>  2 files changed, 27 insertions(+)
>>
>> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
>> index 5d03802c7d..5aa755d7b5 100644
>> --- a/target/s390x/cpu.h
>> +++ b/target/s390x/cpu.h
>> @@ -594,6 +594,7 @@ struct sysib_322 {
>>  #define SIGP_SET_PREFIX        0x0d
>>  #define SIGP_STORE_STATUS_ADDR 0x0e
>>  #define SIGP_SET_ARCH          0x12
>> +#define SIGP_SENSE_RUNNING     0x15
>>  #define SIGP_STORE_ADTL_STATUS 0x17
>>  
>>  /* SIGP condition codes */
>> @@ -604,6 +605,7 @@ struct sysib_322 {
>>  
>>  /* SIGP status bits */
>>  #define SIGP_STAT_EQUIPMENT_CHECK   0x80000000UL
>> +#define SIGP_STAT_NOT_RUNNING       0x00000400UL
>>  #define SIGP_STAT_INCORRECT_STATE   0x00000200UL
>>  #define SIGP_STAT_INVALID_PARAMETER 0x00000100UL
>>  #define SIGP_STAT_EXT_CALL_PENDING  0x00000080UL
>> diff --git a/target/s390x/sigp.c b/target/s390x/sigp.c
>> index 9587c3d319..c57312b743 100644
>> --- a/target/s390x/sigp.c
>> +++ b/target/s390x/sigp.c
>> @@ -234,6 +234,28 @@ static void sigp_set_prefix(CPUState *cs, run_on_cpu_data arg)
>>      si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
>>  }
>>  
>> +static void sigp_sense_running(S390CPU *dst_cpu, SigpInfo *si)
>> +{
>> +    if (!tcg_enabled()) {
>> +        /* handled in KVM */
>> +        set_sigp_status(si, SIGP_STAT_INVALID_ORDER);
>> +        return;
>> +    }
> 
> If we're sure that this is always handled in the kernel, I think you
> could simply do a "g_assert(tcg_enabled())" here instead?
> 
>  Thomas
> 

This keeps existing behavior and does not crash the guest. Therefore I
decided to not use a g_assert().

Especially, kernels throw every SIGP order to user space that they don't
understand. So e.g. a SIGP SENSE RUNNING could end up here for older
kernels.

-- 

Thanks,

David

Re: [Qemu-devel] [PATCH v1 06/27] s390x/tcg: injection of emergency signals and extarnal calls

[David Hildenbrand]

> @@ -248,6 +249,19 @@ static void do_ext_interrupt(CPUS390XState *env)
>          lowcore->ext_int_code = cpu_to_be16(EXT_CPU_TIMER);
>          lowcore->cpu_addr = 0;
>          env->pending_int &= ~INTERRUPT_EXT_CPU_TIMER;
> +    } else if (env->pending_int & INTERRUPT_EMERGENCY_SIGNAL) {
> +        lowcore->ext_int_code = cpu_to_be16(EXT_EMERGENCY);
> +        cpu_addr = find_first_bit(env->emergency_signals, S390_MAX_CPUS);
> +        g_assert(cpu_addr < S390_MAX_CPUS);
> +        lowcore->cpu_addr = cpu_to_be16(cpu_addr);
> +        clear_bit(cpu_addr, env->emergency_signals);
> +        if (bitmap_empty(env->emergency_signals, max_cpus)) {
> +            env->pending_int &= ~INTERRUPT_EMERGENCY_SIGNAL;
> +        }
> +    } else if (env->pending_int & INTERRUPT_EXTERNAL_CALL) {
> +        lowcore->ext_int_code = cpu_to_be16(EXT_EXTERNAL_CALL);
> +        lowcore->cpu_addr = cpu_to_be16(env->external_call_addr);
> +        env->pending_int &= ~INTERRUPT_EXTERNAL_CALL;

The priority of the interrupts is still wrong and we don't check for the
subclass-mask bits. Will fix this.

-- 

Thanks,

David

Re: [Qemu-devel] [PATCH v1 01/27] s390x: raise CPU hotplug irq after really hotplugged

[Richard Henderson]

On 09/18/2017 08:59 AM, David Hildenbrand wrote:
> Let's move it into the machine, so we trigger the IRQ after setting
> ms->possible_cpus (which SCLP uses to construct the list of
> online CPUs).
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  hw/s390x/s390-virtio-ccw.c | 4 ++++
>  target/s390x/cpu.c         | 8 --------
>  2 files changed, 4 insertions(+), 8 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH v1 03/27] target/s390x: get rid of next_core_id

[Richard Henderson]

On 09/18/2017 08:59 AM, David Hildenbrand wrote:
>      /* sync cs->cpu_index and env->core_id. The latter is needed for TCG. */
> -    cs->cpu_index = env->core_id;
> +    cs->cpu_index = cpu->env.core_id;
> +#endif

Any reason not to drop core_id entirely in favour of cpu_index?
(Since cpu_index itself is generic and can't be dropped.)


r~

Re: [Qemu-devel] [PATCH v1 04/27] s390x: introduce and use S390_MAX_CPUS

[Richard Henderson]

On 09/18/2017 08:59 AM, David Hildenbrand wrote:
> Will be handy in the next patches.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  hw/s390x/s390-virtio-ccw.c | 2 +-
>  target/s390x/cpu.h         | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH v1 05/27] s390/tcg: turn INTERRUPT_EXT into a mask

[Richard Henderson]

On 09/18/2017 08:59 AM, David Hildenbrand wrote:
> +    } else if (env->pending_int | INTERRUPT_EXT_FLOATING) {

Surely & here.

Otherwise,

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH v1 07/27] s390x/tcg: STOPPED cpus can never wake up

[Richard Henderson]

On 09/18/2017 08:59 AM, David Hildenbrand wrote:
> Interrupts can't wake such CPUs up. SIGP from other CPUs has to be used
> to toggle the state.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/cpu.c | 6 ++++++
>  1 file changed, 6 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH v1 08/27] s390x/tcg: a CPU cannot switch state due to an interrupt

[Richard Henderson]

On 09/18/2017 08:59 AM, David Hildenbrand wrote:
> Going to OPERATING here looks wrong. A CPU should even never be
> !OPERATING at this point. Unhalting will already be done in
> cpu_handle_halt() if there is work, so we can drop this statement
> completely.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/excp_helper.c | 1 -
>  1 file changed, 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH v1 09/27] target/s390x: factor out handling of WAIT PSW into handle_wait()

[Richard Henderson]

On 09/18/2017 08:59 AM, David Hildenbrand wrote:
> This will now also detect crashes under TCG. We can directly use
> cpu->env.psw.addr instead of kvm_run, as we do a
> cpu_synchronize_state().
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/helper.c   | 28 ++++++++++++++++++++++------
>  target/s390x/internal.h |  1 +
>  target/s390x/kvm.c      | 15 +--------------
>  3 files changed, 24 insertions(+), 20 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH v1 10/27] s390x/kvm: pass ipb directly into handle_sigp()

[Richard Henderson]

On 09/18/2017 08:59 AM, David Hildenbrand wrote:
> No need to pass kvm_run.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/kvm.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH v1 11/27] s390x/kvm: generalize SIGP stop and restart interrupt injection

[Richard Henderson]

On 09/18/2017 08:59 AM, David Hildenbrand wrote:
> Preparation for factoring it out into !kvm code.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/internal.h  |  2 ++
>  target/s390x/interrupt.c | 20 ++++++++++++++++++++
>  target/s390x/kvm-stub.c  |  8 ++++++++
>  target/s390x/kvm.c       | 33 +++++++++++++++++++++------------
>  target/s390x/kvm_s390x.h |  2 ++
>  5 files changed, 53 insertions(+), 12 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH v1 14/27] s390x/kvm: factor out storing of adtl CPU status

[Richard Henderson]

On 09/18/2017 08:59 AM, David Hildenbrand wrote:
> Called from SIGP code to be factored out, so let's move it. Add a
> FIXME for TCG code in the future.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/helper.c   | 29 +++++++++++++++++++++++++++++
>  target/s390x/internal.h |  1 +
>  target/s390x/kvm.c      | 30 +-----------------------------
>  3 files changed, 31 insertions(+), 29 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH v1 14/27] s390x/kvm: factor out storing of adtl CPU status

[Richard Henderson]

On 09/18/2017 08:59 AM, David Hildenbrand wrote:
> Called from SIGP code to be factored out, so let's move it. Add a
> FIXME for TCG code in the future.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/helper.c   | 29 +++++++++++++++++++++++++++++
>  target/s390x/internal.h |  1 +
>  target/s390x/kvm.c      | 30 +-----------------------------
>  3 files changed, 31 insertions(+), 29 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH v1 15/27] s390x/kvm: drop two debug prints

[Richard Henderson]

On 09/18/2017 09:00 AM, David Hildenbrand wrote:
> Preparation for moving it out of kvm.c.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/kvm.c | 2 --
>  1 file changed, 2 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH v1 16/27] s390x/kvm: factor out SIGP code into sigp.c

[Richard Henderson]

On 09/18/2017 09:00 AM, David Hildenbrand wrote:
> We want to use the same code base for TCG, so let's cleanly factor it
> out.
> 
> The sigp mutex is currently not really needed, as everything is
> protected by the iothread mutex. But this could change later, so leave
> it in place and initialize it properly from common code.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  hw/s390x/s390-virtio-ccw.c |   3 +
>  target/s390x/Makefile.objs |   1 +
>  target/s390x/cpu.c         |   8 -
>  target/s390x/cpu.h         |   6 +-
>  target/s390x/internal.h    |   4 +
>  target/s390x/kvm-stub.c    |   5 -
>  target/s390x/kvm.c         | 349 +-----------------------------------------
>  target/s390x/kvm_s390x.h   |   1 -
>  target/s390x/sigp.c        | 366 +++++++++++++++++++++++++++++++++++++++++++++
>  target/s390x/trace-events  |   4 +-
>  10 files changed, 388 insertions(+), 359 deletions(-)
>  create mode 100644 target/s390x/sigp.c


Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH v1 03/27] target/s390x: get rid of next_core_id

[Igor Mammedov]

On Mon, 25 Sep 2017 16:14:16 -0700
Richard Henderson <richard.henderson@linaro.org> wrote:

> On 09/18/2017 08:59 AM, David Hildenbrand wrote:
> >      /* sync cs->cpu_index and env->core_id. The latter is needed for TCG. */
> > -    cs->cpu_index = env->core_id;
> > +    cs->cpu_index = cpu->env.core_id;
> > +#endif  
> 
> Any reason not to drop core_id entirely in favour of cpu_index?
> (Since cpu_index itself is generic and can't be dropped.)
I prefer core_id vs cpu_index as the later is too generic
(and we were trying to decouple target specific attributes
/coreid/cpuid/whatnot from cpu_index as much as possible
so it would be more clear what they mean and if possible
make them in-depended of QEMU internal cpu_index which is
essentially not owned by target).

Re: [Qemu-devel] [PATCH v1 03/27] target/s390x: get rid of next_core_id

[David Hildenbrand]

On 26.09.2017 10:49, Igor Mammedov wrote:
> On Mon, 25 Sep 2017 16:14:16 -0700
> Richard Henderson <richard.henderson@linaro.org> wrote:
> 
>> On 09/18/2017 08:59 AM, David Hildenbrand wrote:
>>>      /* sync cs->cpu_index and env->core_id. The latter is needed for TCG. */
>>> -    cs->cpu_index = env->core_id;
>>> +    cs->cpu_index = cpu->env.core_id;
>>> +#endif  
>>
>> Any reason not to drop core_id entirely in favour of cpu_index?
>> (Since cpu_index itself is generic and can't be dropped.)
> I prefer core_id vs cpu_index as the later is too generic
> (and we were trying to decouple target specific attributes
> /coreid/cpuid/whatnot from cpu_index as much as possible
> so it would be more clear what they mean and if possible
> make them in-depended of QEMU internal cpu_index which is
> essentially not owned by target).
> 

As x86 also has this split I also prefer to keep it for now as it is.

cpu_index just has to match our core id, otherwise e.g. migration could
break (cpu_index is used as identifier for cpus in the migration stream).

If we would drop core_id, we would have to implement a handler for the
STAP instruction, as the CPU object is not accessible from translate.c.
(not a big problem, just another reason to simply leave it as is for
now), and also a new property handler, so only a uint16_t can be set
using "core-id" onto a CPU object.

-- 

Thanks,

David

Re: [Qemu-devel] [PATCH v1 09/27] target/s390x: factor out handling of WAIT PSW into handle_wait()

[David Hildenbrand]

>> +
>> +void handle_wait(S390CPU *cpu)
> 
> "handle_wait" is a very generic name ... could you maybe at least name
> this "s390x_handle_wait" or so?

sure, will do, thanks!

> 
>> +{
>> +    cpu_synchronize_state(CPU(cpu));
>> +    if (s390_cpu_halt(cpu) == 0) {
>> +#ifndef CONFIG_USER_ONLY
>> +        if (is_special_wait_psw(cpu->env.psw.addr)) {
>> +            qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
>> +        } else {
>> +            qemu_system_guest_panicked(NULL);
>> +        }
>> +#endif
>> +    }
>> +}
[...]

>> -            cpu_synchronize_state(cs);
> 
> Maybe we should rather keep the cpu_synchronize_state() here instead of
> putting it in helper.c? No other function in helper.c seems to call that
> on its own yet...

Don't have a strong opinion on this, as long as it works :)

> 
>> -            if (s390_cpu_halt(cpu) == 0) {
>> -                if (is_special_wait_psw(cs)) {
>> -                    qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
>> -                } else {
>> -                    qemu_system_guest_panicked(NULL);
>> -                }
>> -            }
>> +            handle_wait(cpu);
>>              r = EXCP_HALTED;
>>              break;
>>          case ICPT_CPU_STOP:
>>
> 
>  Thomas
> 


-- 

Thanks,

David

Re: [Qemu-devel] [PATCH v1 05/27] s390/tcg: turn INTERRUPT_EXT into a mask

[David Hildenbrand]

On 26.09.2017 01:18, Richard Henderson wrote:
> On 09/18/2017 08:59 AM, David Hildenbrand wrote:
>> +    } else if (env->pending_int | INTERRUPT_EXT_FLOATING) {
> 
> Surely & here.
> 

Yes, the code just worked because the next check is a
g_assert_not_reached() :)

Thanks!

> Otherwise,
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> 
> 
> r~
> 


-- 

Thanks,

David