Skip to contentSkip to conversation

s390x: SMP for TCG

PATCH Applied David Hildenbrand 4 participants 65 messages Sep 28, 2017 – Oct 16, 2017

lore
Share & tools
lore
#0 lore raw

v2 cover s390x: SMP for TCG

This series is based on a couple of other patches floating around on the mailing list (e.g. cleanups and LAP). The fist patches of v1 have been sent as a separate series.

As I am working on kvm-unit-tests for SIGP, I discovered some bugs that have been fixed in this series.

======================================
I'll be on vacation until 15. October.
======================================

This series contains:
- properly implement local external interrupts for TCG. Take care of interrupt masks. Cleanup service interrupt handling.
- 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.

CPU hotplug does still not work correctly.

v1 -> v2:
- "turn INTERRUPT_EXT into a mask"
-- fix one check (& vs |)
-- rename EXT_FLOATING to EXT_SERVICE
-- drop duplicate storing of interrupt parameters for queded ext irqa
- "s390x/tcg: cleanup service interrupt injectio"
-- added, as we don't have virtio irq anymore (for legacy transport)
- "s390x/tcg: rework checking for deliverable interrupts"
-- added, we actually check if interrupts are deliverable
- "s390x/tcg: take care of external interrupt subclasses"
-- properly check for the subclasses we implement
- "target/s390x: factor out handling of WAIT PSW into s390_handle_wait()" -- renamed handle_wait() -> s390_handle_wait()
- "s390x/tcg: handle WAIT PSWs during interrupt injection"
-- any feedback if this is the right way to do it? needed for stop interrupts
- "target/s390x: interpret PSW_MASK_WAIT only for TCG"
-- identified while testing SIGP
- "s390x/kvm: pass ipb directly into handle_sigp()"
-- order parameters alphabetically
- "s390x/kvm: factor out storing of CPU status"
-- the rework/conversion has been integrated as a fix (Connys queue)
- "MAINTAINERS: use s390 KVM maintainers for target/s390x/sigp.c" -- added as requested by Christian
- "s390x/tcg: implement STOP and RESET interrupts for TCG"
-- some change due to the way interrupts are handled after the rework
for deliverable interrupts
- "s390x/cpumodel: allow to enable SENSE RUNNING STATUS for qemu" -- added, because we implement SENSE RUNNING STATUS
- "target/s390x: special handling when starting a CPU with WAIT PSW" -- identified while testing SIGP code

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

David Hildenbrand (30):
s390x/tcg: turn INTERRUPT_EXT into a mask
s390x/tcg: cleanup service interrupt injection
s390x/tcg: injection of emergency signals and external calls s390x/tcg: rework checking for deliverable interrupts s390x/tcg: take care of external interrupt subclasses 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 s390_handle_wait() s390x/tcg: handle WAIT PSWs during interrupt injection target/s390x: interpret PSW_MASK_WAIT only for TCG 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
s390x/kvm: factor out storing of adtl CPU status s390x/kvm: drop two debug prints
s390x/kvm: factor out SIGP code into sigp.c
MAINTAINERS: use s390 KVM maintainers for target/s390x/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/cpumodel: allow to enable SENSE RUNNING STATUS for qemu s390x/tcg: unlock NMI
s390x/tcg: refactor stfl(e) to use s390_get_feat_block() target/s390x: special handling when starting a CPU with WAIT PSW

 MAINTAINERS                |   1 +
 hw/s390x/s390-virtio-ccw.c |  11 +-
 target/s390x/Makefile.objs |   1 +
 target/s390x/cpu.c         |  35 ++--
 target/s390x/cpu.h         |  42 ++--
 target/s390x/cpu_models.c  |   1 +
 target/s390x/excp_helper.c | 116 +++++++----
 target/s390x/helper.c      | 132 ++++++++++--
 target/s390x/helper.h      |   2 +-
 target/s390x/internal.h    |  23 +-
 target/s390x/interrupt.c   | 172 ++++++++++++++-
 target/s390x/kvm-stub.c    |  13 +-
 target/s390x/kvm.c         | 491 +++----------------------------------------
 target/s390x/kvm_s390x.h   |   3 +-
 target/s390x/misc_helper.c | 109 ++++------
 target/s390x/sigp.c        | 508 +++++++++++++++++++++++++++++++++++++++++++++
 target/s390x/trace-events  |   4 +-
 target/s390x/translate.c   |  12 +-
 18 files changed, 1013 insertions(+), 663 deletions(-)
 create mode 100644 target/s390x/sigp.c
Signature · 1 line
2.13.5
#1 lore raw

PATCH v2 1/30 s390x/tcg: turn INTERRUPT_EXT into a mask

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.

The only floating external interrupts used right now are SERVICE interrupts, so let's use that name. Following patches will clean up SERVICE interrupt injection.

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.

---
 target/s390x/cpu.h         | 13 ++++++----
 target/s390x/excp_helper.c | 63 +++++++++++++++++++++++-----------------------
 target/s390x/helper.c      | 12 ++-------
 target/s390x/internal.h    |  2 ++
 target/s390x/interrupt.c   | 18 ++++++++++++-
 5 files changed, 61 insertions(+), 47 deletions(-)
target/s390x/cpu.h +8 -5
index 7e864c8478..e32f87a2f1 100644
@@ -404,11 +404,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_SERVICE            (1 << 2)
+#define INTERRUPT_EXT_CPU_TIMER          (1 << 3)
+#define INTERRUPT_EXT_CLOCK_COMPARATOR   (1 << 4)
+#define INTERRUPT_EXT                    (INTERRUPT_EXT_SERVICE | \
+                                          INTERRUPT_EXT_CPU_TIMER | \
+                                          INTERRUPT_EXT_CLOCK_COMPARATOR)
 
 /* Program Status Word.  */
 #define S390_PSWM_REGNUM 0
target/s390x/excp_helper.c +32 -31

82 lines not on this page · raw message

target/s390x/helper.c +2 -10
index 97adbcc86d..e22b93258b 100644
@@ -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
 
target/s390x/internal.h +2 -0
index 14bf3ea5e2..4dda5cf2f1 100644
@@ -362,6 +362,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 */
target/s390x/interrupt.c +17 -1
index 058e219fe5..b9c30f86d7 100644
@@ -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_SERVICE;
+    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);
 }
 
Signature · 1 line
2.13.5
#2 lore raw

Re: [Qemu-devel] [PATCH v2 01/30] s390x/tcg: turn INTERRUPT_EXT into a mask

Quoted 28 lines · 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.

The only floating external interrupts used right now are SERVICE interrupts, so let's use that name. Following patches will clean up SERVICE interrupt injection.

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.

---
 target/s390x/cpu.h         | 13 ++++++----
 target/s390x/excp_helper.c | 63 +++++++++++++++++++++++-----------------------
 target/s390x/helper.c      | 12 ++-------
 target/s390x/internal.h    |  2 ++
 target/s390x/interrupt.c   | 18 ++++++++++++-
 5 files changed, 61 insertions(+), 47 deletions(-)

[...]

Quoted 18 lines
target/s390x/interrupt.c +9 -1
index 058e219fe5..b9c30f86d7 100644
@@ -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_SERVICE;
+    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;

target/s390x/interrupt.c void cpu_inject_ext(S390CPU *cpu, uint32_t code, uint32_t param, · line 92

Quoted 10 lines
target/s390x/interrupt.c +8 -0
+    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);
 }

The last two function look similar enough that you could merge the functions, e.g.:

void cpu_inject_ext_pending_bit(S390CPU *cpu, int bit)

{
    CPUS390XState *env = &cpu->env;

    env->pending_int |= bit;
    cpu_interrupt(CPU(cpu), CPU_INTERRUPT_HARD);
}

?

Apart from that, the patch looks fine to me.

Thomas

#3 lore raw

Re: [Qemu-devel] [PATCH v2 01/30] s390x/tcg: turn INTERRUPT_EXT into a mask

Quoted 70 lines · On Fri, 6 Oct 2017 09:08:45 +0200 Thomas Huth <[email protected]> wrote:
Quoted 28 lines · 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.

The only floating external interrupts used right now are SERVICE interrupts, so let's use that name. Following patches will clean up SERVICE interrupt injection.

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.

---
 target/s390x/cpu.h         | 13 ++++++----
 target/s390x/excp_helper.c | 63 +++++++++++++++++++++++-----------------------
 target/s390x/helper.c      | 12 ++-------
 target/s390x/internal.h    |  2 ++
 target/s390x/interrupt.c   | 18 ++++++++++++-
 5 files changed, 61 insertions(+), 47 deletions(-)  

[...]

Quoted 18 lines
target/s390x/interrupt.c +9 -1
index 058e219fe5..b9c30f86d7 100644
@@ -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_SERVICE;
+    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;

target/s390x/interrupt.c void cpu_inject_ext(S390CPU *cpu, uint32_t code, uint32_t param, · line 92

Quoted 10 lines
target/s390x/interrupt.c +8 -0
+    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);
 }  

The last two function look similar enough that you could merge the functions, e.g.:

void cpu_inject_ext_pending_bit(S390CPU *cpu, int bit)

{
    CPUS390XState *env = &cpu->env;

    env->pending_int |= bit;
    cpu_interrupt(CPU(cpu), CPU_INTERRUPT_HARD);
}
Quoted 5 lines

?

Apart from that, the patch looks fine to me.

Thomas

FWIW, I'd prefer to keep these as separate functions.

#4 lore raw

Re: [Qemu-devel] [PATCH v2 01/30] s390x/tcg: turn INTERRUPT_EXT into a mask

Quoted 11 lines · Cornelia Huck
Quoted 11 lines

The last two function look similar enough that you could merge the functions, e.g.:

void cpu_inject_ext_pending_bit(S390CPU *cpu, int bit)

{
    CPUS390XState *env = &cpu->env;

    env->pending_int |= bit;
    cpu_interrupt(CPU(cpu), CPU_INTERRUPT_HARD);
}
Quoted 6 lines
Quoted 5 lines

?

Apart from that, the patch looks fine to me.

Thomas

FWIW, I'd prefer to keep these as separate functions.

Certainly. In a later patch they diverge much more than would be easily separated as Thomas suggests.

r~

#5 lore raw

PATCH v2 2/30 s390x/tcg: cleanup service interrupt injection

In reply to David Hildenbrand

There are still some leftovers from old virtio interrupts in there. Most importantly, we don't have to queue service interrupts anymore. Just like KVM, we can simply multiplex the SCLP service interrupts and avoid the queue.

Also, now only valid parametes/cpu_addr will be stored on service interrupts.

---
 target/s390x/cpu.c         |  2 --
 target/s390x/cpu.h         | 10 +---------
 target/s390x/excp_helper.c | 16 +++++-----------
 target/s390x/internal.h    |  2 --
 target/s390x/interrupt.c   | 18 ++++--------------
 5 files changed, 10 insertions(+), 38 deletions(-)
target/s390x/cpu.c +0 -2
index f42e28ea25..e240d99fe6 100644
@@ -107,7 +107,6 @@ static void s390_cpu_initial_reset(CPUState *s)
     env->gbea = 1;
 
     env->pfault_token = -1UL;
-    env->ext_index = -1;
     for (i = 0; i < ARRAY_SIZE(env->io_index); i++) {
         env->io_index[i] = -1;
     }
@@ -144,7 +143,6 @@ static void s390_cpu_full_reset(CPUState *s)
     env->gbea = 1;
 
     env->pfault_token = -1UL;
-    env->ext_index = -1;
     for (i = 0; i < ARRAY_SIZE(env->io_index); i++) {
         env->io_index[i] = -1;
     }
target/s390x/cpu.h +1 -9
index e32f87a2f1..7bea97a2d7 100644
@@ -53,7 +53,6 @@
 
 #define MMU_USER_IDX 0
 
-#define MAX_EXT_QUEUE 16
 #define MAX_IO_QUEUE 16
 #define MAX_MCHK_QUEUE 16
 
@@ -67,12 +66,6 @@ typedef struct PSW {
     uint64_t addr;
 } PSW;
 
-typedef struct ExtQueue {
-    uint32_t code;
-    uint32_t param;
-    uint32_t param64;
-} ExtQueue;
-
 typedef struct IOIntQueue {
     uint16_t id;
     uint16_t nr;
@@ -128,12 +121,11 @@ struct CPUS390XState {
 
     uint64_t cregs[16]; /* control registers */
 
-    ExtQueue ext_queue[MAX_EXT_QUEUE];
     IOIntQueue io_queue[MAX_IO_QUEUE][8];
     MchkQueue mchk_queue[MAX_MCHK_QUEUE];
 
     int pending_int;
-    int ext_index;
+    uint32_t service_param;
     int io_index[8];
     int mchk_index;
 
target/s390x/excp_helper.c +5 -11
index 8521043bf2..8228104a49 100644
@@ -240,7 +240,6 @@ static void do_ext_interrupt(CPUS390XState *env)
     S390CPU *cpu = s390_env_get_cpu(env);
     uint64_t mask, addr;
     LowCore *lowcore;
-    ExtQueue *q;
 
     if (!(env->psw.mask & PSW_MASK_EXT)) {
         cpu_abort(CPU(cpu), "Ext int w/o ext mask\n");
@@ -257,20 +256,15 @@ static void do_ext_interrupt(CPUS390XState *env)
         lowcore->cpu_addr = 0;
         env->pending_int &= ~INTERRUPT_EXT_CPU_TIMER;
     } else if (env->pending_int & INTERRUPT_EXT_SERVICE) {
-        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->cpu_addr = cpu_to_be16(env->core_id | VIRTIO_SUBCODE_64);
-        env->ext_index--;
-        if (env->ext_index == -1) {
-            env->pending_int &= ~INTERRUPT_EXT_SERVICE;
-        }
+        lowcore->ext_int_code = cpu_to_be16(EXT_SERVICE);
+        lowcore->ext_params = cpu_to_be32(env->service_param);
+        lowcore->cpu_addr = 0;
+        env->service_param = 0;
+        env->pending_int &= ~INTERRUPT_EXT_SERVICE;
     } else {
         g_assert_not_reached();
     }
target/s390x/internal.h +0 -2
index 4dda5cf2f1..eaa071a183 100644
@@ -360,8 +360,6 @@ void cpu_unmap_lowcore(LowCore *lowcore);
 
 /* interrupt.c */
 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);
 
target/s390x/interrupt.c +4 -14
index b9c30f86d7..cbc7b7696d 100644
@@ -54,22 +54,12 @@ void program_interrupt(CPUS390XState *env, uint32_t code, int ilen)
 }
 
 #if !defined(CONFIG_USER_ONLY)
-void cpu_inject_ext(S390CPU *cpu, uint32_t code, uint32_t param,
-                    uint64_t param64)
+static void cpu_inject_service(S390CPU *cpu, uint32_t param)
 {
     CPUS390XState *env = &cpu->env;
 
-    if (env->ext_index == MAX_EXT_QUEUE - 1) {
-        /* ugh - can't queue anymore. Let's drop. */
-        return;
-    }
-
-    env->ext_index++;
-    assert(env->ext_index < MAX_EXT_QUEUE);
-
-    env->ext_queue[env->ext_index].code = code;
-    env->ext_queue[env->ext_index].param = param;
-    env->ext_queue[env->ext_index].param64 = param64;
+    /* multiplexing is good enough for sclp - also kvm does that internally */
+    env->service_param |= param;
 
     env->pending_int |= INTERRUPT_EXT_SERVICE;
     cpu_interrupt(CPU(cpu), CPU_INTERRUPT_HARD);
@@ -145,7 +135,7 @@ void s390_sclp_extint(uint32_t parm)
     } else {
         S390CPU *dummy_cpu = s390_cpu_addr2state(0);
 
-        cpu_inject_ext(dummy_cpu, EXT_SERVICE, parm, 0);
+        cpu_inject_service(dummy_cpu, parm);
     }
 }
 
Signature · 1 line
2.13.5
Reviewed-by on v2 2/30
#6 lore raw

Re: [Qemu-devel] [PATCH v2 02/30] s390x/tcg: cleanup service interrupt injection

Quoted 8 lines · David Hildenbrand

There are still some leftovers from old virtio interrupts in there. Most importantly, we don't have to queue service interrupts anymore. Just like KVM, we can simply multiplex the SCLP service interrupts and avoid the queue.

Also, now only valid parametes/cpu_addr will be stored on service interrupts.

Quoted 8 lines
---
 target/s390x/cpu.c         |  2 --
 target/s390x/cpu.h         | 10 +---------
 target/s390x/excp_helper.c | 16 +++++-----------
 target/s390x/internal.h    |  2 --
 target/s390x/interrupt.c   | 18 ++++--------------
 5 files changed, 10 insertions(+), 38 deletions(-)

r~

#7 lore raw

Re: [Qemu-devel] [PATCH v2 02/30] s390x/tcg: cleanup service interrupt injection

In reply to David Hildenbrand
Quoted 6 lines · On Thu, 28 Sep 2017 22:36:40 +0200 David Hildenbrand <[email protected]> wrote:

There are still some leftovers from old virtio interrupts in there. Most importantly, we don't have to queue service interrupts anymore. Just like KVM, we can simply multiplex the SCLP service interrupts and avoid the queue.

Also, now only valid parametes/cpu_addr will be stored on service

s/parametes/parameters/

Quoted 28 lines

interrupts.

---
 target/s390x/cpu.c         |  2 --
 target/s390x/cpu.h         | 10 +---------
 target/s390x/excp_helper.c | 16 +++++-----------
 target/s390x/internal.h    |  2 --
 target/s390x/interrupt.c   | 18 ++++--------------
 5 files changed, 10 insertions(+), 38 deletions(-)
target/s390x/interrupt.c +1 -4
index b9c30f86d7..cbc7b7696d 100644
@@ -54,22 +54,12 @@ void program_interrupt(CPUS390XState *env, uint32_t code, int ilen)
 }
 
 #if !defined(CONFIG_USER_ONLY)
-void cpu_inject_ext(S390CPU *cpu, uint32_t code, uint32_t param,
-                    uint64_t param64)
+static void cpu_inject_service(S390CPU *cpu, uint32_t param)
 {
     CPUS390XState *env = &cpu->env;
 
-    if (env->ext_index == MAX_EXT_QUEUE - 1) {
-        /* ugh - can't queue anymore. Let's drop. */

target/s390x/interrupt.c void program_interrupt(CPUS390XState *env, uint32_t code, int ilen) · line 61

Quoted 10 lines
target/s390x/interrupt.c +1 -9
-        return;
-    }
-
-    env->ext_index++;
-    assert(env->ext_index < MAX_EXT_QUEUE);
-
-    env->ext_queue[env->ext_index].code = code;
-    env->ext_queue[env->ext_index].param = param;
-    env->ext_queue[env->ext_index].param64 = param64;
+    /* multiplexing is good enough for sclp - also kvm does that internally */

"kvm does that internally as well"?

(Can fix while applying.)

Quoted 13 lines
target/s390x/interrupt.c +2 -1
+    env->service_param |= param;
 
     env->pending_int |= INTERRUPT_EXT_SERVICE;
     cpu_interrupt(CPU(cpu), CPU_INTERRUPT_HARD);
@@ -145,7 +135,7 @@ void s390_sclp_extint(uint32_t parm)
     } else {
         S390CPU *dummy_cpu = s390_cpu_addr2state(0);
 
-        cpu_inject_ext(dummy_cpu, EXT_SERVICE, parm, 0);
+        cpu_inject_service(dummy_cpu, parm);
     }
 }
 
#8 lore raw

PATCH v2 3/30 s390x/tcg: injection of emergency signals and external calls

In reply to David Hildenbrand

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

Take care of interrupt priority.

---
 target/s390x/cpu.h         |  8 +++++++-
 target/s390x/excp_helper.c | 16 +++++++++++++++-
 target/s390x/internal.h    |  2 ++
 target/s390x/interrupt.c   | 26 ++++++++++++++++++++++++++
 4 files changed, 50 insertions(+), 2 deletions(-)
target/s390x/cpu.h +7 -1
index 7bea97a2d7..f0f5ff0359 100644
@@ -126,6 +126,8 @@ struct CPUS390XState {
 
     int pending_int;
     uint32_t service_param;
+    uint16_t external_call_addr;
+    DECLARE_BITMAP(emergency_signals, S390_MAX_CPUS);
     int io_index[8];
     int mchk_index;
 
@@ -401,9 +403,13 @@ static inline void cpu_get_tb_cpu_state(CPUS390XState* env, target_ulong *pc,
 #define INTERRUPT_EXT_SERVICE            (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_SERVICE | \
                                           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
target/s390x/excp_helper.c +15 -1
index 8228104a49..bffcf7f39e 100644
@@ -239,6 +239,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;
 
     if (!(env->psw.mask & PSW_MASK_EXT)) {
@@ -247,7 +248,20 @@ static void do_ext_interrupt(CPUS390XState *env)
 
     lowcore = cpu_map_lowcore(env);
 
-    if (env->pending_int & INTERRUPT_EXT_CLOCK_COMPARATOR) {
+    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_CLOCK_COMPARATOR) {
         lowcore->ext_int_code = cpu_to_be16(EXT_CLOCK_COMP);
         lowcore->cpu_addr = 0;
         env->pending_int &= ~INTERRUPT_EXT_CLOCK_COMPARATOR;
target/s390x/internal.h +2 -0
index eaa071a183..f67c2a1785 100644
@@ -362,6 +362,8 @@ void cpu_unmap_lowcore(LowCore *lowcore);
 void trigger_pgm_exception(CPUS390XState *env, uint32_t code, uint32_t ilen);
 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 */
target/s390x/interrupt.c +26 -0
index cbc7b7696d..afd311c1e1 100644
@@ -81,6 +81,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)
Signature · 1 line
2.13.5
Reviewed-by on v2 3/30
#9 lore raw

Re: [Qemu-devel] [PATCH v2 03/30] s390x/tcg: injection of emergency signals and external calls

Quoted 5 lines · David Hildenbrand

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

Take care of interrupt priority.

Quoted 7 lines
---
 target/s390x/cpu.h         |  8 +++++++-
 target/s390x/excp_helper.c | 16 +++++++++++++++-
 target/s390x/internal.h    |  2 ++
 target/s390x/interrupt.c   | 26 ++++++++++++++++++++++++++
 4 files changed, 50 insertions(+), 2 deletions(-)

r~

#10 lore raw

PATCH v2 4/30 s390x/tcg: rework checking for deliverable interrupts

In reply to David Hildenbrand

Currently, enabling/disabling of interrupts is not really supported.

Let's improve interrupt handling code by explicitly checking for deliverable interrupts only. This is the first step. Checking for external interrupt subclasses will be done next.

---
 target/s390x/cpu.c         |  8 +++++---
 target/s390x/excp_helper.c | 21 +++++++--------------
 target/s390x/internal.h    |  4 ++++
 target/s390x/interrupt.c   | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 62 insertions(+), 17 deletions(-)
target/s390x/cpu.c +5 -3
index e240d99fe6..9c2a1e6ac8 100644
@@ -56,10 +56,12 @@ static void s390_cpu_set_pc(CPUState *cs, vaddr value)
 static bool s390_cpu_has_work(CPUState *cs)
 {
     S390CPU *cpu = S390_CPU(cs);
-    CPUS390XState *env = &cpu->env;
 
-    return (cs->interrupt_request & CPU_INTERRUPT_HARD) &&
-           (env->psw.mask & PSW_MASK_EXT);
+    if (!(cs->interrupt_request & CPU_INTERRUPT_HARD)) {
+        return false;
+    }
+
+    return s390_cpu_has_int(cpu);
 }
 
 #if !defined(CONFIG_USER_ONLY)
target/s390x/excp_helper.c +7 -14
index bffcf7f39e..02849e41e6 100644
@@ -434,24 +434,16 @@ void s390_cpu_do_interrupt(CPUState *cs)
 
     s390_cpu_set_state(CPU_STATE_OPERATING, cpu);
     /* handle machine checks */
-    if ((env->psw.mask & PSW_MASK_MCHECK) &&
-        (cs->exception_index == -1)) {
-        if (env->pending_int & INTERRUPT_MCHK) {
-            cs->exception_index = EXCP_MCHK;
-        }
+    if (cs->exception_index == -1 && s390_cpu_has_mcck_int(cpu)) {
+        cs->exception_index = EXCP_MCHK;
     }
     /* handle external interrupts */
-    if ((env->psw.mask & PSW_MASK_EXT) &&
-        cs->exception_index == -1 &&
-        (env->pending_int & INTERRUPT_EXT)) {
+    if (cs->exception_index == -1 && s390_cpu_has_ext_int(cpu)) {
         cs->exception_index = EXCP_EXT;
     }
     /* handle I/O interrupts */
-    if ((env->psw.mask & PSW_MASK_IO) &&
-        (cs->exception_index == -1)) {
-        if (env->pending_int & INTERRUPT_IO) {
-            cs->exception_index = EXCP_IO;
-        }
+    if (cs->exception_index == -1 && s390_cpu_has_io_int(cpu)) {
+        cs->exception_index = EXCP_IO;
     }
 
     switch (cs->exception_index) {
@@ -473,6 +465,7 @@ void s390_cpu_do_interrupt(CPUState *cs)
     }
     cs->exception_index = -1;
 
+    /* we might still have pending interrupts, but not deliverable */
     if (!env->pending_int) {
         cs->interrupt_request &= ~CPU_INTERRUPT_HARD;
     }
@@ -489,7 +482,7 @@ bool s390_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
                the parent EXECUTE insn.  */
             return false;
         }
-        if (env->psw.mask & PSW_MASK_EXT) {
+        if (s390_cpu_has_int(cpu)) {
             s390_cpu_do_interrupt(cs);
             return true;
         }
target/s390x/internal.h +4 -0
index f67c2a1785..e41fb2e38e 100644
@@ -364,6 +364,10 @@ 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);
+bool s390_cpu_has_io_int(S390CPU *cpu);
+bool s390_cpu_has_ext_int(S390CPU *cpu);
+bool s390_cpu_has_mcck_int(S390CPU *cpu);
+bool s390_cpu_has_int(S390CPU *cpu);
 
 
 /* ioinst.c */
target/s390x/interrupt.c +46 -0

50 lines not on this page · raw message

Signature · 1 line
2.13.5
Reviewed-by on v2 4/30
#11 lore raw

Re: [Qemu-devel] [PATCH v2 04/30] s390x/tcg: rework checking for deliverable interrupts

Quoted 6 lines · David Hildenbrand

Currently, enabling/disabling of interrupts is not really supported.

Let's improve interrupt handling code by explicitly checking for deliverable interrupts only. This is the first step. Checking for external interrupt subclasses will be done next.

Quoted 7 lines
---
 target/s390x/cpu.c         |  8 +++++---
 target/s390x/excp_helper.c | 21 +++++++--------------
 target/s390x/internal.h    |  4 ++++
 target/s390x/interrupt.c   | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 62 insertions(+), 17 deletions(-)

r~

#12 lore raw

PATCH v2 5/30 s390x/tcg: take care of external interrupt subclasses

In reply to David Hildenbrand

We can now let go of INTERRUPT_EXT. When cr0 changes, we have to revalidate if we now have a pending external interrupt, just like when the PSW (or SYSTEM MASK only) changes.

---
 target/s390x/cpu.h         | 10 +++++-----
 target/s390x/excp_helper.c | 15 ++++++++++-----
 target/s390x/interrupt.c   | 32 +++++++++++++++++++++++++++++++-
 target/s390x/translate.c   |  6 ++++--
 4 files changed, 50 insertions(+), 13 deletions(-)
target/s390x/cpu.h +5 -5
index f0f5ff0359..6ae0c7dfc8 100644
@@ -345,6 +345,11 @@ extern const struct VMStateDescription vmstate_s390_cpu;
 #define CR0_LOWPROT             0x0000000010000000ULL
 #define CR0_SECONDARY           0x0000000004000000ULL
 #define CR0_EDAT                0x0000000000800000ULL
+#define CR0_EMERGENCY_SIGNAL_SC 0x0000000000004000ULL
+#define CR0_EXTERNAL_CALL_SC    0x0000000000002000ULL
+#define CR0_CKC_SC              0x0000000000000800ULL
+#define CR0_CPU_TIMER_SC        0x0000000000000400ULL
+#define CR0_SERVICE_SC          0x0000000000000200ULL
 
 /* MMU */
 #define MMU_PRIMARY_IDX         0
@@ -405,11 +410,6 @@ 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_EXT                    (INTERRUPT_EXT_SERVICE | \
-                                          INTERRUPT_EXT_CPU_TIMER | \
-                                          INTERRUPT_EXT_CLOCK_COMPARATOR | \
-                                          INTERRUPT_EXTERNAL_CALL | \
-                                          INTERRUPT_EMERGENCY_SIGNAL)
 
 /* Program Status Word.  */
 #define S390_PSWM_REGNUM 0
target/s390x/excp_helper.c +10 -5
index 02849e41e6..fe68de5a77 100644
@@ -248,7 +248,8 @@ static void do_ext_interrupt(CPUS390XState *env)
 
     lowcore = cpu_map_lowcore(env);
 
-    if (env->pending_int & INTERRUPT_EMERGENCY_SIGNAL) {
+    if ((env->pending_int & INTERRUPT_EMERGENCY_SIGNAL) &&
+        (env->cregs[0] & CR0_EMERGENCY_SIGNAL_SC)) {
         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);
@@ -257,19 +258,23 @@ static void do_ext_interrupt(CPUS390XState *env)
         if (bitmap_empty(env->emergency_signals, max_cpus)) {
             env->pending_int &= ~INTERRUPT_EMERGENCY_SIGNAL;
         }
-    } else if (env->pending_int & INTERRUPT_EXTERNAL_CALL) {
+    } else if ((env->pending_int & INTERRUPT_EXTERNAL_CALL) &&
+               (env->cregs[0] & CR0_EXTERNAL_CALL_SC)) {
         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_CLOCK_COMPARATOR) {
+    } else if ((env->pending_int & INTERRUPT_EXT_CLOCK_COMPARATOR) &&
+               (env->cregs[0] & CR0_CKC_SC)) {
         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) {
+    } else if ((env->pending_int & INTERRUPT_EXT_CPU_TIMER) &&
+               (env->cregs[0] & CR0_CPU_TIMER_SC)) {
         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_SERVICE) {
+    } else if ((env->pending_int & INTERRUPT_EXT_SERVICE) &&
+               (env->cregs[0] & CR0_SERVICE_SC)) {
         /*
          * FIXME: floating IRQs should be considered by all CPUs and
          *        shuld not get cleared by CPU reset.
target/s390x/interrupt.c +31 -1
index 54eb400b0c..c1eaaea98b 100644
@@ -209,7 +209,37 @@ bool s390_cpu_has_ext_int(S390CPU *cpu)
         return false;
     }
 
-    return env->pending_int & INTERRUPT_EXT;
+    if ((env->pending_int & INTERRUPT_EMERGENCY_SIGNAL) &&
+        (env->cregs[0] & CR0_EMERGENCY_SIGNAL_SC)) {
+        return true;
+    }
+
+    if ((env->pending_int & INTERRUPT_EXTERNAL_CALL) &&
+        (env->cregs[0] & CR0_EXTERNAL_CALL_SC)) {
+        return true;
+    }
+
+    if ((env->pending_int & INTERRUPT_EXTERNAL_CALL) &&
+        (env->cregs[0] & CR0_EXTERNAL_CALL_SC)) {
+        return true;
+    }
+
+    if ((env->pending_int & INTERRUPT_EXT_CLOCK_COMPARATOR) &&
+        (env->cregs[0] & CR0_CKC_SC)) {
+        return true;
+    }
+
+    if ((env->pending_int & INTERRUPT_EXT_CPU_TIMER) &&
+        (env->cregs[0] & CR0_CPU_TIMER_SC)) {
+        return true;
+    }
+
+    if ((env->pending_int & INTERRUPT_EXT_SERVICE) &&
+        (env->cregs[0] & CR0_SERVICE_SC)) {
+        return true;
+    }
+
+    return false;
 }
 
 bool s390_cpu_has_io_int(S390CPU *cpu)
target/s390x/translate.c +4 -2
index 9ef95141f9..405d94b87d 100644
@@ -2719,7 +2719,8 @@ static ExitStatus op_lctl(DisasContext *s, DisasOps *o)
     gen_helper_lctl(cpu_env, r1, o->in2, r3);
     tcg_temp_free_i32(r1);
     tcg_temp_free_i32(r3);
-    return NO_EXIT;
+    /* Exit to main loop to reevaluate s390_cpu_exec_interrupt.  */
+    return EXIT_PC_STALE_NOCHAIN;
 }
 
 static ExitStatus op_lctlg(DisasContext *s, DisasOps *o)
@@ -2730,7 +2731,8 @@ static ExitStatus op_lctlg(DisasContext *s, DisasOps *o)
     gen_helper_lctlg(cpu_env, r1, o->in2, r3);
     tcg_temp_free_i32(r1);
     tcg_temp_free_i32(r3);
-    return NO_EXIT;
+    /* Exit to main loop to reevaluate s390_cpu_exec_interrupt.  */
+    return EXIT_PC_STALE_NOCHAIN;
 }
 
 static ExitStatus op_lra(DisasContext *s, DisasOps *o)
Signature · 1 line
2.13.5
Reviewed-by on v2 5/30
#13 lore raw

Re: [Qemu-devel] [PATCH v2 05/30] s390x/tcg: take care of external interrupt subclasses

Quoted 11 lines · David Hildenbrand

We can now let go of INTERRUPT_EXT. When cr0 changes, we have to revalidate if we now have a pending external interrupt, just like when the PSW (or SYSTEM MASK only) changes.

---
 target/s390x/cpu.h         | 10 +++++-----
 target/s390x/excp_helper.c | 15 ++++++++++-----
 target/s390x/interrupt.c   | 32 +++++++++++++++++++++++++++++++-
 target/s390x/translate.c   |  6 ++++--
 4 files changed, 50 insertions(+), 13 deletions(-)

r~

#14 lore raw

PATCH v2 6/30 s390x/tcg: STOPPED cpus can never wake up

In reply to David Hildenbrand

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

---
 target/s390x/cpu.c | 6 ++++++
 1 file changed, 6 insertions(+)
target/s390x/cpu.c +6 -0
index 9c2a1e6ac8..a0b139056f 100644
@@ -57,6 +57,12 @@ static bool s390_cpu_has_work(CPUState *cs)
 {
     S390CPU *cpu = S390_CPU(cs);
 
+    /* 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;
+    }
+
     if (!(cs->interrupt_request & CPU_INTERRUPT_HARD)) {
         return false;
     }
Signature · 1 line
2.13.5
#15 lore raw

PATCH v2 7/30 s390x/tcg: a CPU cannot switch state due to an interrupt

In reply to 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.

---
 target/s390x/excp_helper.c | 1 -
 1 file changed, 1 deletion(-)
target/s390x/excp_helper.c +0 -1
index fe68de5a77..f6747f3877 100644
@@ -437,7 +437,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 (cs->exception_index == -1 && s390_cpu_has_mcck_int(cpu)) {
         cs->exception_index = EXCP_MCHK;
Signature · 1 line
2.13.5
#16 lore raw

PATCH v2 8/30 target/s390x: factor out handling of WAIT PSW into s390_handle_wait()

In reply to 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().

---
 target/s390x/helper.c   | 27 +++++++++++++++++++++------
 target/s390x/internal.h |  1 +
 target/s390x/kvm.c      | 14 +-------------
 3 files changed, 23 insertions(+), 19 deletions(-)
target/s390x/helper.c +21 -6
index e22b93258b..2d7df83c59 100644
@@ -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,25 @@ 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 s390_handle_wait(S390CPU *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 +148,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
-        }
+        s390_handle_wait(s390_env_get_cpu(env));
     }
 }
 
target/s390x/internal.h +1 -0
index e41fb2e38e..6e500d6bb7 100644
@@ -352,6 +352,7 @@ 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);
+void s390_handle_wait(S390CPU *cpu);
 #ifndef CONFIG_USER_ONLY
 LowCore *cpu_map_lowcore(CPUS390XState *env);
 void cpu_unmap_lowcore(LowCore *lowcore);
target/s390x/kvm.c +1 -13
index 9b91eeb7a9..8dc82fbb59 100644
@@ -1960,12 +1960,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);
@@ -2037,13 +2031,7 @@ static int handle_intercept(S390CPU *cpu)
         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);
-                }
-            }
+            s390_handle_wait(cpu);
             r = EXCP_HALTED;
             break;
         case ICPT_CPU_STOP:
Signature · 1 line
2.13.5
#17 lore raw

PATCH v2 9/30 s390x/tcg: handle WAIT PSWs during interrupt injection

In reply to David Hildenbrand

If we encounter a WAIT PSW, we have to halt immediately. Using cpu_loop_exit() at this point feels wrong. Simply leaving cs->exception_index set doesn't result in an immediate stop.

This is also necessary to properly handle SIGP STOP interrupts later.

The CPU_INTERRUPT_HALT will be processed immediately and properly set the CPU to halted (also resetting cs->exception_index to EXCP_HLT)

---
 target/s390x/excp_helper.c | 6 ++++++
 1 file changed, 6 insertions(+)
target/s390x/excp_helper.c +6 -0
index f6747f3877..1a0d0770a9 100644
@@ -467,6 +467,12 @@ void s390_cpu_do_interrupt(CPUState *cs)
         do_mchk_interrupt(env);
         break;
     }
+
+    /* WAIT PSW during interrupt injection */
+    if (cs->exception_index == EXCP_HLT) {
+        /* don't trigger a cpu_loop_exit(), use an interrupt instead */
+        cpu_interrupt(CPU(cpu), CPU_INTERRUPT_HALT);
+    }
     cs->exception_index = -1;
 
     /* we might still have pending interrupts, but not deliverable */
Signature · 1 line
2.13.5
Reviewed-by on v2 9/30
#18 lore raw

Re: [Qemu-devel] [PATCH v2 09/30] s390x/tcg: handle WAIT PSWs during interrupt injection

Quoted 6 lines · David Hildenbrand

If we encounter a WAIT PSW, we have to halt immediately. Using cpu_loop_exit() at this point feels wrong. Simply leaving cs->exception_index set doesn't result in an immediate stop.

This is also necessary to properly handle SIGP STOP interrupts later.

Quoted 7 lines

The CPU_INTERRUPT_HALT will be processed immediately and properly set the CPU to halted (also resetting cs->exception_index to EXCP_HLT)

---
 target/s390x/excp_helper.c | 6 ++++++
 1 file changed, 6 insertions(+)

r~

#19 lore raw

PATCH v2 10/30 target/s390x: interpret PSW_MASK_WAIT only for TCG

In reply to David Hildenbrand

KVM handles the wait PSW itself and triggers a WAIT ICPT in case it really want to sleep (disabled wait).

This will later allow us to change the order of loading a restart interrupt and setting a CPU to OPERATING on SIGP RESTART without changing KVM behavior.

---
 target/s390x/helper.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
target/s390x/helper.c +2 -1
index 2d7df83c59..baa18777f1 100644
@@ -147,7 +147,8 @@ void load_psw(CPUS390XState *env, uint64_t mask, uint64_t addr)
         s390_cpu_recompute_watchpoints(CPU(s390_env_get_cpu(env)));
     }
 
-    if (mask & PSW_MASK_WAIT) {
+    /* KVM will handle all WAITs and trigger a WAIT exit on disabled_wait */
+    if (tcg_enabled() && (mask & PSW_MASK_WAIT)) {
         s390_handle_wait(s390_env_get_cpu(env));
     }
 }
Signature · 1 line
2.13.5
Reviewed-by on v2 10/30
#20 lore raw

Re: [Qemu-devel] [PATCH v2 10/30] target/s390x: interpret PSW_MASK_WAIT only for TCG

Quoted 11 lines · David Hildenbrand

KVM handles the wait PSW itself and triggers a WAIT ICPT in case it really want to sleep (disabled wait).

This will later allow us to change the order of loading a restart interrupt and setting a CPU to OPERATING on SIGP RESTART without changing KVM behavior.

---
 target/s390x/helper.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

r~

#21 lore raw

Re: [Qemu-devel] [PATCH v2 10/30] target/s390x: interpret PSW_MASK_WAIT only for TCG

In reply to David Hildenbrand
Quoted 2 lines · On Thu, 28 Sep 2017 22:36:48 +0200 David Hildenbrand <[email protected]> wrote:

KVM handles the wait PSW itself and triggers a WAIT ICPT in case it really want to sleep (disabled wait).

s/want/wants/

(fixing up while applying)

Quoted 24 lines

This will later allow us to change the order of loading a restart interrupt and setting a CPU to OPERATING on SIGP RESTART without changing KVM behavior.

---
 target/s390x/helper.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
target/s390x/helper.c +2 -1
index 2d7df83c59..baa18777f1 100644
@@ -147,7 +147,8 @@ void load_psw(CPUS390XState *env, uint64_t mask, uint64_t addr)
         s390_cpu_recompute_watchpoints(CPU(s390_env_get_cpu(env)));
     }
 
-    if (mask & PSW_MASK_WAIT) {
+    /* KVM will handle all WAITs and trigger a WAIT exit on disabled_wait */
+    if (tcg_enabled() && (mask & PSW_MASK_WAIT)) {
         s390_handle_wait(s390_env_get_cpu(env));
     }
 }
#22 lore raw

PATCH v2 11/30 s390x/kvm: pass ipb directly into handle_sigp()

In reply to David Hildenbrand

No need to pass kvm_run. Pass parameters alphabetically ordered.

---
 target/s390x/kvm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
target/s390x/kvm.c +3 -3
index 8dc82fbb59..43f8f7331a 100644
@@ -1876,7 +1876,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, uint8_t ipa1, uint32_t ipb)
 {
     CPUS390XState *env = &cpu->env;
     const uint8_t r1 = ipa1 >> 4;
@@ -1890,7 +1890,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];
@@ -1948,7 +1948,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, ipa1, run->s390_sieic.ipb);
         break;
     }
 
Signature · 1 line
2.13.5
#23 lore raw

PATCH v2 12/30 s390x/kvm: generalize SIGP stop and restart interrupt injection

In reply to David Hildenbrand

Preparation for factoring it out into !kvm code.

---
 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(-)
target/s390x/internal.h +2 -0
index 6e500d6bb7..0ac026d30f 100644
@@ -369,6 +369,8 @@ bool s390_cpu_has_io_int(S390CPU *cpu);
 bool s390_cpu_has_ext_int(S390CPU *cpu);
 bool s390_cpu_has_mcck_int(S390CPU *cpu);
 bool s390_cpu_has_int(S390CPU *cpu);
+void cpu_inject_restart(S390CPU *cpu);
+void cpu_inject_stop(S390CPU *cpu);
 
 
 /* ioinst.c */
target/s390x/interrupt.c +20 -0
index c1eaaea98b..64b3f519ef 100644
@@ -107,6 +107,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)
target/s390x/kvm-stub.c +8 -0
index 261e1cdc44..18b53b2ad6 100644
@@ -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)
+{
+}
target/s390x/kvm.c +21 -12
index 43f8f7331a..7f2623f679 100644
@@ -1502,9 +1502,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;
@@ -1517,7 +1514,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;
 }
@@ -1616,9 +1613,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) {
@@ -1628,7 +1622,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:
@@ -1718,9 +1712,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:
@@ -1730,7 +1721,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;
@@ -2781,3 +2772,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);
+}
target/s390x/kvm_s390x.h +2 -0
index 2d594bd4ee..fd03cd662a 100644
@@ -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);
Signature · 1 line
2.13.5
#24 lore raw

PATCH v2 13/30 s390x/kvm: factor out storing of CPU status

In reply to David Hildenbrand

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

---
 target/s390x/helper.c   | 60 ++++++++++++++++++++++++++++++++++++++++++
 target/s390x/internal.h |  2 ++
 target/s390x/kvm.c      | 69 +++----------------------------------------------
 3 files changed, 65 insertions(+), 66 deletions(-)
target/s390x/helper.c +60 -0
index baa18777f1..4c11822074 100644
@@ -245,6 +245,66 @@ void s390_cpu_recompute_watchpoints(CPUState *cs)
     }
 }
 
+struct sigp_save_area {
+    uint64_t    fprs[16];                       /* 0x0000 */
+    uint64_t    grs[16];                        /* 0x0080 */
+    PSW         psw;                            /* 0x0100 */
+    uint8_t     pad_0x0110[0x0118 - 0x0110];    /* 0x0110 */
+    uint32_t    prefix;                         /* 0x0118 */
+    uint32_t    fpc;                            /* 0x011c */
+    uint8_t     pad_0x0120[0x0124 - 0x0120];    /* 0x0120 */
+    uint32_t    todpr;                          /* 0x0124 */
+    uint64_t    cputm;                          /* 0x0128 */
+    uint64_t    ckc;                            /* 0x0130 */
+    uint8_t     pad_0x0138[0x0140 - 0x0138];    /* 0x0138 */
+    uint32_t    ars[16];                        /* 0x0140 */
+    uint64_t    crs[16];                        /* 0x0384 */
+};
+QEMU_BUILD_BUG_ON(sizeof(struct sigp_save_area) != 512);
+
+int s390_store_status(S390CPU *cpu, hwaddr addr, bool store_arch)
+{
+    static const uint8_t ar_id = 1;
+    struct sigp_save_area *sa;
+    hwaddr len = sizeof(*sa);
+    int i;
+
+    sa = cpu_physical_memory_map(addr, &len, 1);
+    if (!sa) {
+        return -EFAULT;
+    }
+    if (len != sizeof(*sa)) {
+        cpu_physical_memory_unmap(sa, 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) {
+        sa->fprs[i] = cpu_to_be64(get_freg(&cpu->env, i)->ll);
+    }
+    for (i = 0; i < 16; ++i) {
+        sa->grs[i] = cpu_to_be64(cpu->env.regs[i]);
+    }
+    sa->psw.addr = cpu_to_be64(cpu->env.psw.addr);
+    sa->psw.mask = cpu_to_be64(get_psw_mask(&cpu->env));
+    sa->prefix = cpu_to_be32(cpu->env.psa);
+    sa->fpc = cpu_to_be32(cpu->env.fpc);
+    sa->todpr = cpu_to_be32(cpu->env.todpr);
+    sa->cputm = cpu_to_be64(cpu->env.cputm);
+    sa->ckc = cpu_to_be64(cpu->env.ckc >> 8);
+    for (i = 0; i < 16; ++i) {
+        sa->ars[i] = cpu_to_be32(cpu->env.aregs[i]);
+    }
+    for (i = 0; i < 16; ++i) {
+        sa->ars[i] = cpu_to_be64(cpu->env.cregs[i]);
+    }
+
+    cpu_physical_memory_unmap(sa, len, 1, len);
+
+    return 0;
+}
 #endif /* CONFIG_USER_ONLY */
 
 void s390_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
target/s390x/internal.h +2 -0
index 0ac026d30f..a301237c17 100644
@@ -353,6 +353,8 @@ void s390x_tod_timer(void *opaque);
 void s390x_cpu_timer(void *opaque);
 void do_restart_interrupt(CPUS390XState *env);
 void s390_handle_wait(S390CPU *cpu);
+#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);
target/s390x/kvm.c +3 -66

93 lines not on this page · raw message

Signature · 1 line
2.13.5
Reviewed-by on v2 13/30
#25 lore raw

Re: [Qemu-devel] [PATCH v2 13/30] s390x/kvm: factor out storing of CPU status

Quoted 8 lines · David Hildenbrand

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

---
 target/s390x/helper.c   | 60 ++++++++++++++++++++++++++++++++++++++++++
 target/s390x/internal.h |  2 ++
 target/s390x/kvm.c      | 69 +++----------------------------------------------
 3 files changed, 65 insertions(+), 66 deletions(-)

r~

#26 lore raw

PATCH v2 14/30 s390x/kvm: factor out storing of adtl CPU status

In reply to David Hildenbrand

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

---
 target/s390x/helper.c   | 29 +++++++++++++++++++++++++++++
 target/s390x/internal.h |  1 +
 target/s390x/kvm.c      | 30 +-----------------------------
 3 files changed, 31 insertions(+), 29 deletions(-)
target/s390x/helper.c +29 -0
index 4c11822074..2505f3aec0 100644
@@ -305,6 +305,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,
target/s390x/internal.h +1 -0
index a301237c17..fb8ff6b078 100644
@@ -355,6 +355,7 @@ void do_restart_interrupt(CPUS390XState *env);
 void s390_handle_wait(S390CPU *cpu);
 #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);
target/s390x/kvm.c +1 -29
index c7d0145fa9..1aec1e26d4 100644
@@ -1519,34 +1519,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);
@@ -1639,7 +1611,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;
     }
Signature · 1 line
2.13.5
#27 lore raw

PATCH v2 15/30 s390x/kvm: drop two debug prints

In reply to David Hildenbrand

Preparation for moving it out of kvm.c.

---
 target/s390x/kvm.c | 2 --
 1 file changed, 2 deletions(-)
target/s390x/kvm.c +0 -2
index 1aec1e26d4..fbbdca23b2 100644
@@ -1642,7 +1642,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;
 }
 
@@ -1744,7 +1743,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);
     }
 
Signature · 1 line
2.13.5
#28 lore raw

PATCH v2 16/30 s390x/kvm: factor out SIGP code into sigp.c

In reply to 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.

---
 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
hw/s390x/s390-virtio-ccw.c +3 -0
index c747b22d66..d188ce17ba 100644
@@ -265,6 +265,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();
+
     /* get a BUS */
     css_bus = virtual_css_bus_init();
     s390_init_ipl_dev(machine->kernel_filename, machine->kernel_cmdline,
target/s390x/Makefile.objs +1 -0
index c88ac81e84..31932de9cf 100644
@@ -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 crypto_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
 
target/s390x/cpu.c +0 -8
index a0b139056f..e27827afb3 100644
@@ -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()) {
target/s390x/cpu.h +5 -1
index 6ae0c7dfc8..f94d7f96e0 100644
@@ -676,7 +676,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);
@@ -730,6 +729,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);
 
target/s390x/internal.h +4 -0
index fb8ff6b078..d6ab45add4 100644
@@ -415,4 +415,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 */
target/s390x/kvm-stub.c +0 -5
index 18b53b2ad6..861b4dedf6 100644
@@ -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)
 {
 }
target/s390x/kvm.c +6 -343

384 lines not on this page · raw message

target/s390x/kvm_s390x.h +0 -1
index fd03cd662a..33906a893b 100644
@@ -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);
target/s390x/sigp.c new file +366 -0

366 lines not on this page · raw message

target/s390x/trace-events +3 -1
index 4d871f5087..a84e316e49 100644
@@ -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"
Signature · 1 line
2.13.5
#29 lore raw

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

Quoted 9 lines · On Thu, 28 Sep 2017 22:36:54 +0200 David Hildenbrand <[email protected]> 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.

Quoted 13 lines
---
 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

This had gained some conflicts in the meanwhile, but they were easy to fix.

#30 lore raw

PATCH v2 17/30 MAINTAINERS: use s390 KVM maintainers for target/s390x/sigp.c

In reply to David Hildenbrand

All code in target/s390x/sigp.c was factored out from target/s390x/kvm.c, so let's also copy the entry in the MAINTAINERS file.

---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)
MAINTAINERS +1 -0
index 772ac209e1..d34a72320c 100644
@@ -303,6 +303,7 @@ F: target/s390x/kvm_s390x.h
 F: target/s390x/kvm-stub.c
 F: target/s390x/ioinst.[ch]
 F: target/s390x/machine.c
+F: target/s390x/sigp.c
 F: hw/intc/s390_flic.c
 F: hw/intc/s390_flic_kvm.c
 F: include/hw/s390x/s390_flic.h
Signature · 1 line
2.13.5
Reviewed-by on v2 17/30
#31 lore raw

Re: [Qemu-devel] [PATCH v2 17/30] MAINTAINERS: use s390 KVM maintainers for target/s390x/sigp.c

Quoted 8 lines · David Hildenbrand

All code in target/s390x/sigp.c was factored out from target/s390x/kvm.c, so let's also copy the entry in the MAINTAINERS file.

---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

Probably should squash with previous, but...

r~

#32 lore raw

Re: [Qemu-devel] [PATCH v2 17/30] MAINTAINERS: use s390 KVM maintainers for target/s390x/sigp.c

Quoted 10 lines · On Fri, 6 Oct 2017 09:40:49 -0400 Richard Henderson…
Quoted 8 lines · David Hildenbrand

All code in target/s390x/sigp.c was factored out from target/s390x/kvm.c, so let's also copy the entry in the MAINTAINERS file.

---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)  
Quoted 7 lines

Probably should squash with previous, but...

r~

Agreed, that makes more sense than keeping it as a standalone patch.

#33 lore raw

PATCH v2 18/30 s390x/kvm: factor out actual handling of STOP interrupts

In reply to 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.

---
 target/s390x/internal.h |  1 +
 target/s390x/kvm.c      |  8 +-------
 target/s390x/sigp.c     | 15 ++++++++++++++-
 3 files changed, 16 insertions(+), 8 deletions(-)
target/s390x/internal.h +1 -0
index d6ab45add4..2c3fc3fce0 100644
@@ -418,5 +418,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 */
target/s390x/kvm.c +1 -7
index 1174e249ab..72008e436f 100644
@@ -1597,13 +1597,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:
target/s390x/sigp.c +14 -1
index 4813123aad..9587c3d319 100644
@@ -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);
Signature · 1 line
2.13.5
Reviewed-by on v2 18/30
#34 lore raw

Re: [Qemu-devel] [PATCH v2 18/30] s390x/kvm: factor out actual handling of STOP interrupts

Quoted 10 lines · 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.

---
 target/s390x/internal.h |  1 +
 target/s390x/kvm.c      |  8 +-------
 target/s390x/sigp.c     | 15 ++++++++++++++-
 3 files changed, 16 insertions(+), 8 deletions(-)

r~

#35 lore raw

PATCH v2 19/30 s390x/tcg: implement SIGP SENSE RUNNING STATUS

In reply to David Hildenbrand

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

---
 target/s390x/cpu.h  |  2 ++
 target/s390x/sigp.c | 25 +++++++++++++++++++++++++
 2 files changed, 27 insertions(+)
target/s390x/cpu.h +2 -0
index f94d7f96e0..6b92b0751a 100644
@@ -590,6 +590,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 */
@@ -600,6 +601,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
target/s390x/sigp.c +25 -0
index 9587c3d319..c57312b743 100644
@@ -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);
     }
Signature · 1 line
2.13.5
#37 lore raw

PATCH v2 20/30 s390x/tcg: implement SIGP SENSE

In reply to David Hildenbrand

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

---
 target/s390x/sigp.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)
target/s390x/sigp.c +29 -0
index c57312b743..063a34ccc7 100644
@@ -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;
Signature · 1 line
2.13.5
#40 lore raw

PATCH v2 21/30 s390x/tcg: implement SIGP EXTERNAL CALL

In reply to David Hildenbrand

As preparation for TCG.

---
 target/s390x/sigp.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)
target/s390x/sigp.c +23 -2
index 063a34ccc7..080bce3fba 100644
@@ -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);
 
Signature · 1 line
2.13.5
#42 lore raw

PATCH v2 22/30 s390x/tcg: implement SIGP EMERGENCY SIGNAL

In reply to David Hildenbrand

As preparation for TCG.

---
 target/s390x/sigp.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
target/s390x/sigp.c +15 -0
index 080bce3fba..d492885787 100644
@@ -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;
Signature · 1 line
2.13.5
#44 lore raw

PATCH v2 23/30 s390x/tcg: implement SIGP CONDITIONAL EMERGENCY SIGNAL

In reply to David Hildenbrand

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

---
 target/s390x/cpu.h  |  1 +
 target/s390x/sigp.c | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+)
target/s390x/cpu.h +1 -0
index 6b92b0751a..7f4f03f410 100644
@@ -590,6 +590,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
 
target/s390x/sigp.c +37 -0
index d492885787..ce8fda9d01 100644
@@ -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;
Signature · 1 line
2.13.5
Reviewed-by on v2 23/30
#45 lore raw

Re: [Qemu-devel] [PATCH v2 23/30] s390x/tcg: implement SIGP CONDITIONAL EMERGENCY SIGNAL

Quoted 8 lines · David Hildenbrand

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

---
 target/s390x/cpu.h  |  1 +
 target/s390x/sigp.c | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+)

r~

#46 lore raw

PATCH v2 24/30 s390x/tcg: implement STOP and RESET interrupts for TCG

In reply to 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()).

---
 target/s390x/cpu.h         |  4 ++++
 target/s390x/excp_helper.c | 16 +++++++++++++++-
 target/s390x/helper.c      |  1 +
 target/s390x/internal.h    |  2 ++
 target/s390x/interrupt.c   | 32 +++++++++++++++++++++++++++-----
 target/s390x/sigp.c        |  2 ++
 6 files changed, 51 insertions(+), 6 deletions(-)
target/s390x/cpu.h +4 -0
index 7f4f03f410..b684502900 100644
@@ -400,6 +400,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 */
 
@@ -410,6 +412,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)
 
 /* Program Status Word.  */
 #define S390_PSWM_REGNUM 0
target/s390x/excp_helper.c +15 -1
index 1a0d0770a9..e04b670663 100644
@@ -449,6 +449,14 @@ void s390_cpu_do_interrupt(CPUState *cs)
     if (cs->exception_index == -1 && s390_cpu_has_io_int(cpu)) {
         cs->exception_index = EXCP_IO;
     }
+    /* RESTART interrupt */
+    if (cs->exception_index == -1 && s390_cpu_has_restart_int(cpu)) {
+        cs->exception_index = EXCP_RESTART;
+    }
+    /* STOP interrupt has least priority */
+    if (cs->exception_index == -1 && s390_cpu_has_stop_int(cpu)) {
+        cs->exception_index = EXCP_STOP;
+    }
 
     switch (cs->exception_index) {
     case EXCP_PGM:
@@ -466,9 +474,15 @@ 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;
     }
 
-    /* WAIT PSW during interrupt injection */
+    /* WAIT PSW during interrupt injection or STOP interrupt */
     if (cs->exception_index == EXCP_HLT) {
         /* don't trigger a cpu_loop_exit(), use an interrupt instead */
         cpu_interrupt(CPU(cpu), CPU_INTERRUPT_HALT);
target/s390x/helper.c +1 -0
index 2505f3aec0..c41aa4c4ff 100644
@@ -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);
 }
target/s390x/internal.h +2 -0
index 2c3fc3fce0..3aff54ada4 100644
@@ -372,6 +372,8 @@ bool s390_cpu_has_io_int(S390CPU *cpu);
 bool s390_cpu_has_ext_int(S390CPU *cpu);
 bool s390_cpu_has_mcck_int(S390CPU *cpu);
 bool s390_cpu_has_int(S390CPU *cpu);
+bool s390_cpu_has_restart_int(S390CPU *cpu);
+bool s390_cpu_has_stop_int(S390CPU *cpu);
 void cpu_inject_restart(S390CPU *cpu);
 void cpu_inject_stop(S390CPU *cpu);
 
target/s390x/interrupt.c +27 -5
index 64b3f519ef..f2ca881bed 100644
@@ -109,22 +109,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,
@@ -272,6 +278,20 @@ bool s390_cpu_has_io_int(S390CPU *cpu)
 
     return env->pending_int & INTERRUPT_IO;
 }
+
+bool s390_cpu_has_restart_int(S390CPU *cpu)
+{
+    CPUS390XState *env = &cpu->env;
+
+    return env->pending_int & INTERRUPT_RESTART;
+}
+
+bool s390_cpu_has_stop_int(S390CPU *cpu)
+{
+    CPUS390XState *env = &cpu->env;
+
+    return env->pending_int & INTERRUPT_STOP;
+}
 #endif
 
 bool s390_cpu_has_int(S390CPU *cpu)
@@ -282,7 +302,9 @@ bool s390_cpu_has_int(S390CPU *cpu)
     }
     return s390_cpu_has_mcck_int(cpu) ||
            s390_cpu_has_ext_int(cpu) ||
-           s390_cpu_has_io_int(cpu);
+           s390_cpu_has_io_int(cpu) ||
+           s390_cpu_has_restart_int(cpu) ||
+           s390_cpu_has_stop_int(cpu);
 #else
     return false;
 #endif
target/s390x/sigp.c +2 -0
index ce8fda9d01..d70f5cb0ba 100644
@@ -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"
 
@@ -498,6 +499,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)
Signature · 1 line
2.13.5
Reviewed-by on v2 24/30
#47 lore raw

Re: [Qemu-devel] [PATCH v2 24/30] s390x/tcg: implement STOP and RESET interrupts for TCG

Quoted 13 lines · 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()).

Quoted 8 lines
---
 target/s390x/cpu.h         |  4 ++++
 target/s390x/excp_helper.c | 16 +++++++++++++++-
 target/s390x/helper.c      |  1 +
 target/s390x/internal.h    |  2 ++
 target/s390x/interrupt.c   | 32 +++++++++++++++++++++++++++-----
 target/s390x/sigp.c        |  2 ++
 6 files changed, 51 insertions(+), 6 deletions(-)

r~

#48 lore raw

Re: [Qemu-devel] [PATCH v2 24/30] s390x/tcg: implement STOP and RESET interrupts for TCG

In reply to David Hildenbrand
Quoted 2 lines · On Thu, 28 Sep 2017 22:37:02 +0200 David Hildenbrand <[email protected]> wrote:

Implement them like KVM implements/handles them. Both can only be triggered via SIGP instructions. RESET has (almos)the lowest priority if

s/(almos)/(almost) /

Quoted 6 lines

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.

How good are we with interrupt priorities in tcg generally? We might want to care about machine check priority, but that's currently only partially implemented anyway.

Just making sure we do not run out of things to do in the tcg interrupt code :)

Quoted 13 lines

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

---
 target/s390x/cpu.h         |  4 ++++
 target/s390x/excp_helper.c | 16 +++++++++++++++-
 target/s390x/helper.c      |  1 +
 target/s390x/internal.h    |  2 ++
 target/s390x/interrupt.c   | 32 +++++++++++++++++++++++++++-----
 target/s390x/sigp.c        |  2 ++
 6 files changed, 51 insertions(+), 6 deletions(-)
#49 lore raw

PATCH v2 25/30 s390x/tcg: flush the tlb on SIGP SET PREFIX

In reply to David Hildenbrand

Thanks to Aurelien Jarno for doing this in his prototype.

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

---
 target/s390x/sigp.c | 1 +
 1 file changed, 1 insertion(+)
target/s390x/sigp.c +1 -0
index d70f5cb0ba..c5a5dac911 100644
@@ -287,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;
 }
Signature · 1 line
2.13.5
#51 lore raw

PATCH v2 26/30 s390x/tcg: switch to new SIGP handling code

In reply to 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).

---
 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(-)
hw/s390x/s390-virtio-ccw.c +1 -3
index d188ce17ba..3a23fd3f27 100644
@@ -58,9 +58,7 @@ static void s390_init_cpus(MachineState *machine)
     int i;
 
     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 */
target/s390x/helper.h +1 -1
index 52c2963baa..81c5727168 100644
@@ -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)
target/s390x/misc_helper.c +6 -36

50 lines not on this page · raw message

target/s390x/translate.c +3 -2
index 405d94b87d..6a2e084f87 100644
@@ -3710,11 +3710,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
Signature · 1 line
2.13.5
Reviewed-by on v2 26/30
#52 lore raw

Re: [Qemu-devel] [PATCH v2 26/30] s390x/tcg: switch to new SIGP handling code

target/s390x/translate.c static ExitStatus op_servc(DisasContext *s, DisasOps *o) · line 3720

Quoted 13 lines · David Hildenbrand

static ExitStatus op_sigp(DisasContext *s, DisasOps *o)

 {
     TCGv_i32 r1 = tcg_const_i32(get_field(s->fields, r1));
target/s390x/translate.c +3 -2
+    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;
 }

You want to change insn-data.def as well.

-    C(0xae00, SIGP,    RS_a,  Z,   r3_o, a2, 0, 0, sigp, 0)
+    C(0xae00, SIGP,    RS_a,  Z,   0, a2, 0, 0, sigp, 0)

Otherwise,

r~

#53 lore raw

Re: [Qemu-devel] [PATCH v2 26/30] s390x/tcg: switch to new SIGP handling code

Quoted 17 lines · On Fri, 6 Oct 2017 10:02:43 -0400 Richard Henderson…

target/s390x/translate.c static ExitStatus op_servc(DisasContext *s, DisasOps *o) · line 3720

Quoted 13 lines · David Hildenbrand

static ExitStatus op_sigp(DisasContext *s, DisasOps *o)

 {
     TCGv_i32 r1 = tcg_const_i32(get_field(s->fields, r1));
target/s390x/translate.c +3 -2
+    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;
 }  

You want to change insn-data.def as well.

Review

Quoted 2 lines
(quoted diff) +1 -1
-    C(0xae00, SIGP,    RS_a,  Z,   r3_o, a2, 0, 0, sigp, 0)
+    C(0xae00, SIGP,    RS_a,  Z,   0, a2, 0, 0, sigp, 0)

I'll do that while applying. Let's avoid another patchbomb :)

Quoted 6 lines

Otherwise,

r~

#54 lore raw

PATCH v2 27/30 s390x/cpumodel: allow to enable SENSE RUNNING STATUS for qemu

In reply to David Hildenbrand

As we properly implement it, allow to enable it.

---
 target/s390x/cpu_models.c | 1 +
 1 file changed, 1 insertion(+)
target/s390x/cpu_models.c +1 -0
index 07ef8a3b6e..095a11cbca 100644
@@ -823,6 +823,7 @@ static void add_qemu_cpu_model_features(S390FeatBitmap fbm)
         S390_FEAT_DAT_ENH,
         S390_FEAT_IDTE_SEGMENT,
         S390_FEAT_STFLE,
+        S390_FEAT_SENSE_RUNNING_STATUS,
         S390_FEAT_EXTENDED_IMMEDIATE,
         S390_FEAT_EXTENDED_TRANSLATION_2,
         S390_FEAT_MSA,
Signature · 1 line
2.13.5
#56 lore raw

PATCH v2 28/30 s390x/tcg: unlock NMI

In reply to David Hildenbrand

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

---
 hw/s390x/s390-virtio-ccw.c | 4 +---
 target/s390x/sigp.c        | 5 -----
 2 files changed, 1 insertion(+), 8 deletions(-)
hw/s390x/s390-virtio-ccw.c +1 -3
index 3a23fd3f27..7f7e4908b1 100644
@@ -402,9 +402,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)
target/s390x/sigp.c +0 -5
index c5a5dac911..964c75a736 100644
@@ -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;
 }
Signature · 1 line
2.13.5
#58 lore raw

Re: [Qemu-devel] [PATCH v2 28/30] s390x/tcg: unlock NMI

In reply to David Hildenbrand
Quoted 30 lines · On Thu, 28 Sep 2017 22:37:06 +0200 David Hildenbrand <[email protected]> wrote:

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

---
 hw/s390x/s390-virtio-ccw.c | 4 +---
 target/s390x/sigp.c        | 5 -----
 2 files changed, 1 insertion(+), 8 deletions(-)
hw/s390x/s390-virtio-ccw.c +1 -3
index 3a23fd3f27..7f7e4908b1 100644
@@ -402,9 +402,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)
target/s390x/sigp.c +0 -0
index c5a5dac911..964c75a736 100644
@@ -480,11 +480,6 @@ int s390_cpu_restart(S390CPU *cpu)
 {

target/s390x/sigp.c int s390_cpu_restart(S390CPU *cpu) · line 485

Quoted 10 lines
target/s390x/sigp.c +0 -5
     SigpInfo si = {};
 
-    if (tcg_enabled()) {
-        /* FIXME TCG */
-        return -ENOSYS;
-    }
-
     run_on_cpu(CPU(cpu), sigp_restart, RUN_ON_CPU_HOST_PTR(&si));
     return 0;
 }

We might want to make this void. But I'll apply as-is for now.

#59 lore raw

PATCH v2 29/30 s390x/tcg: refactor stfl(e) to use s390_get_feat_block()

In reply to 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.

---
 target/s390x/misc_helper.c | 67 ++++++++++++++++++++--------------------------
 target/s390x/translate.c   |  1 -
 2 files changed, 29 insertions(+), 39 deletions(-)
target/s390x/misc_helper.c +29 -38
index 0b3613ea5f..4afd90b969 100644
@@ -475,66 +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;
 
     lowcore = cpu_map_lowcore(env);
-    do_stfle(env, words);
-    lowcore->stfl_fac_list = cpu_to_be32(words[0] >> 32);
+    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;
 }
target/s390x/translate.c +0 -1
index 6a2e084f87..6fad20f75d 100644
@@ -4143,7 +4143,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;
Signature · 1 line
2.13.5
Reviewed-by on v2 29/30
#60 lore raw

Re: [Qemu-devel] [PATCH v2 29/30] s390x/tcg: refactor stfl(e) to use s390_get_feat_block()

Quoted 10 lines · 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.

Quoted 5 lines
---
 target/s390x/misc_helper.c | 67 ++++++++++++++++++++--------------------------
 target/s390x/translate.c   |  1 -
 2 files changed, 29 insertions(+), 39 deletions(-)

r~

#61 lore raw

PATCH v2 30/30 target/s390x: special handling when starting a CPU with WAIT PSW

In reply to David Hildenbrand

When we try to start a CPU with a WAIT PSW, we have to take care that TCG will actually try to continue executing instructions.

We must therefore really only unhalt the CPU if we don't have a WAIT PSW. Also document the special order for restart interrupts, which load a new PSW and change the state to operating.

To keep KVM working, simply don't have a look at the WAIT bit when loading the PSW. Otherwise the behavior of a restart interrupt when a CPU stopped would be changed.

---
 target/s390x/cpu.c  | 11 +++++++++--
 target/s390x/sigp.c |  6 +++++-
 2 files changed, 14 insertions(+), 3 deletions(-)
target/s390x/cpu.c +9 -2
index e27827afb3..981f5d4f39 100644
@@ -335,8 +335,15 @@ unsigned int s390_cpu_set_state(uint8_t cpu_state, S390CPU *cpu)
         break;
     case CPU_STATE_OPERATING:
     case CPU_STATE_LOAD:
-        /* unhalt the cpu for common infrastructure */
-        s390_cpu_unhalt(cpu);
+        /*
+         * Starting a CPU with a PSW WAIT bit set:
+         * KVM: handles this internally and triggers another WAIT exit.
+         * TCG: will actually try to continue to run. Don't unhalt, will
+         *      be done when the CPU actually has work (an interrupt).
+         */
+        if (!tcg_enabled() || !(cpu->env.psw.mask & PSW_MASK_WAIT)) {
+            s390_cpu_unhalt(cpu);
+        }
         break;
     default:
         error_report("Requested CPU state is not a valid S390 CPU state: %u",
target/s390x/sigp.c +5 -1
index 964c75a736..ac3f8e7dc2 100644
@@ -232,8 +232,12 @@ static void sigp_restart(CPUState *cs, run_on_cpu_data arg)
     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);
+        /*
+         * Set OPERATING (and unhalting) before loading the restart PSW.
+         * load_psw() will then properly halt the CPU again if necessary (TCG).
+         */
         s390_cpu_set_state(CPU_STATE_OPERATING, cpu);
+        do_restart_interrupt(&cpu->env);
         break;
     case CPU_STATE_OPERATING:
         cpu_inject_restart(cpu);
Signature · 1 line
2.13.5
Reviewed-by on v2 30/30
#62 lore raw

Re: [Qemu-devel] [PATCH v2 30/30] target/s390x: special handling when starting a CPU with WAIT PSW

Quoted 11 lines · David Hildenbrand

When we try to start a CPU with a WAIT PSW, we have to take care that TCG will actually try to continue executing instructions.

We must therefore really only unhalt the CPU if we don't have a WAIT PSW. Also document the special order for restart interrupts, which load a new PSW and change the state to operating.

To keep KVM working, simply don't have a look at the WAIT bit when loading the PSW. Otherwise the behavior of a restart interrupt when a CPU stopped would be changed.

Quoted 5 lines
---
 target/s390x/cpu.c  | 11 +++++++++--
 target/s390x/sigp.c |  6 +++++-
 2 files changed, 14 insertions(+), 3 deletions(-)

r~

#63 lore raw
In reply to David Hildenbrand
Quoted 21 lines · On Thu, 28 Sep 2017 22:36:38 +0200 David Hildenbrand <[email protected]> wrote:

This series is based on a couple of other patches floating around on the mailing list (e.g. cleanups and LAP). The fist patches of v1 have been sent as a separate series.

As I am working on kvm-unit-tests for SIGP, I discovered some bugs that have been fixed in this series.

======================================
I'll be on vacation until 15. October.
======================================

This series contains:
- properly implement local external interrupts for TCG. Take care of interrupt masks. Cleanup service interrupt handling.
- 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

Quoted 5 lines

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.

Thanks, applied (with some minor changes, see the individual patches).

Quoted 1 line

CPU hotplug does still not work correctly.

Do you have an idea what still needs to be done?

#64 lore raw
Quoted 24 lines · Cornelia Huck
Quoted 21 lines · On Thu, 28 Sep 2017 22:36:38 +0200 David Hildenbrand <[email protected]> wrote:

This series is based on a couple of other patches floating around on the mailing list (e.g. cleanups and LAP). The fist patches of v1 have been sent as a separate series.

As I am working on kvm-unit-tests for SIGP, I discovered some bugs that have been fixed in this series.

======================================
I'll be on vacation until 15. October.
======================================

This series contains:
- properly implement local external interrupts for TCG. Take care of interrupt masks. Cleanup service interrupt handling.
- 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

Quoted 8 lines
Quoted 5 lines

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.

Thanks, applied (with some minor changes, see the individual patches).

Thanks, all looks sane.

Quoted 4 lines
Quoted 1 line

CPU hotplug does still not work correctly.

Do you have an idea what still needs to be done?

Nope, no idea yet. As the hotplugged CPU starts running but somehow produces a deadlock, however ordinary onlining/offlining of CPUs works, this could be a common code problem.

If I find further problems using my kvm-unit-tests/testing, I'll send fixes.

Signature · 3 lines
Thanks,

David