[Qemu-devel] [PATCH 0/8] s390 sigp, chsc and diag bugfixes/cleanups

[Qemu-devel] [PATCH 0/8] s390 sigp, chsc and diag bugfixes/cleanups

[Jens Freimann]

Alex,

this is mostly bugfixes and cleanup patches, except for Patch 4/8 which
implements a SIGP order code for "cpu start".
  
regards
Jens

Cornelia Huck (1):
  s390x/kvm: Fix diagnose handling.

Thomas Huth (7):
  s390x/kvm: Removed duplicated SIGP defines
  s390x/kvm: Removed s390_store_status stub
  s390x/kvm: Fix coding style in handle_sigp()
  s390x/kvm: Implemented SIGP START
  s390x/kvm: Simplified the calculation of the SIGP order code
  s390x/kvm: Fixed condition code for unknown SIGP orders
  s390x/ioinst: CHSC has to set a condition code

 target-s390x/cpu.h    |  3 ++
 target-s390x/ioinst.c |  1 +
 target-s390x/kvm.c    | 98 ++++++++++++++++++++++++---------------------------
 3 files changed, 50 insertions(+), 52 deletions(-)

-- 
1.8.3.4

[Qemu-devel] [PATCH 1/8] s390x/kvm: Fix diagnose handling.

[Jens Freimann]

From: Cornelia Huck <cornelia.huck@de.ibm.com>

The instruction intercept handler for diagnose used only the displacement
when trying to calculate the function code. This is only correct for base
0, however; we need to perform a complete base/displacement address
calculation and use bits 48-63 as the function code.

Reviewed-by: Thomas Huth <thuth@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
---
 target-s390x/cpu.h |  3 +++
 target-s390x/kvm.c | 19 +++++++++++++------
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index a2c077b..68b5ab7 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -352,6 +352,9 @@ static inline hwaddr decode_basedisp_s(CPUS390XState *env, uint32_t ipb)
     return addr;
 }
 
+/* Base/displacement are at the same locations. */
+#define decode_basedisp_rs decode_basedisp_s
+
 void s390x_tod_timer(void *opaque);
 void s390x_cpu_timer(void *opaque);
 
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 02ac4ba..b00a661 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -562,11 +562,19 @@ static void kvm_handle_diag_308(S390CPU *cpu, struct kvm_run *run)
     handle_diag_308(&cpu->env, r1, r3);
 }
 
-static int handle_diag(S390CPU *cpu, struct kvm_run *run, int ipb_code)
+#define DIAG_KVM_CODE_MASK 0x000000000000ffff
+
+static int handle_diag(S390CPU *cpu, struct kvm_run *run, uint32_t ipb)
 {
     int r = 0;
-
-    switch (ipb_code) {
+    uint16_t func_code;
+
+    /*
+     * For any diagnose call we support, bits 48-63 of the resulting
+     * address specify the function code; the remainder is ignored.
+     */
+    func_code = decode_basedisp_rs(&cpu->env, ipb) & DIAG_KVM_CODE_MASK;
+    switch (func_code) {
     case DIAG_IPL:
         kvm_handle_diag_308(cpu, run);
         break;
@@ -577,7 +585,7 @@ static int handle_diag(S390CPU *cpu, struct kvm_run *run, int ipb_code)
         sleep(10);
         break;
     default:
-        DPRINTF("KVM: unknown DIAG: 0x%x\n", ipb_code);
+        DPRINTF("KVM: unknown DIAG: 0x%x\n", func_code);
         r = -1;
         break;
     }
@@ -684,7 +692,6 @@ static void handle_instruction(S390CPU *cpu, struct kvm_run *run)
 {
     unsigned int ipa0 = (run->s390_sieic.ipa & 0xff00);
     uint8_t ipa1 = run->s390_sieic.ipa & 0x00ff;
-    int ipb_code = (run->s390_sieic.ipb & 0x0fff0000) >> 16;
     int r = -1;
 
     DPRINTF("handle_instruction 0x%x 0x%x\n",
@@ -696,7 +703,7 @@ static void handle_instruction(S390CPU *cpu, struct kvm_run *run)
         r = handle_priv(cpu, run, ipa0 >> 8, ipa1);
         break;
     case IPA0_DIAG:
-        r = handle_diag(cpu, run, ipb_code);
+        r = handle_diag(cpu, run, run->s390_sieic.ipb);
         break;
     case IPA0_SIGP:
         r = handle_sigp(cpu, run, ipa1);
-- 
1.8.3.4

[Qemu-devel] [PATCH 2/8] s390x/kvm: Removed duplicated SIGP defines

[Jens Freimann]

From: Thomas Huth <thuth@linux.vnet.ibm.com>

The SIGP order defines are also available in cpu.h,
so there is no need to re-define them in kvm.c.

Signed-off-by: Thomas Huth <thuth@linux.vnet.ibm.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
---
 target-s390x/kvm.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index b00a661..52d93a7 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -82,11 +82,6 @@
 #define ICPT_CPU_STOP                   0x28
 #define ICPT_IO                         0x40
 
-#define SIGP_RESTART                    0x06
-#define SIGP_INITIAL_CPU_RESET          0x0b
-#define SIGP_STORE_STATUS_ADDR          0x0e
-#define SIGP_SET_ARCH                   0x12
-
 const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
     KVM_CAP_LAST_INFO
 };
-- 
1.8.3.4

[Qemu-devel] [PATCH 3/8] s390x/kvm: Removed s390_store_status stub

[Jens Freimann]

From: Thomas Huth <thuth@linux.vnet.ibm.com>

The SIGP order STORE STATUS AT ADDRESS will be handled in
kernel space, so we do not need the stub in QEMU anymore.

Signed-off-by: Thomas Huth <thuth@linux.vnet.ibm.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
---
 target-s390x/kvm.c | 22 ----------------------
 1 file changed, 22 deletions(-)

diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 52d93a7..5b243b4 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -597,13 +597,6 @@ int kvm_s390_cpu_restart(S390CPU *cpu)
     return 0;
 }
 
-static int s390_store_status(CPUS390XState *env, uint32_t parameter)
-{
-    /* XXX */
-    fprintf(stderr, "XXX SIGP store status\n");
-    return -1;
-}
-
 static int s390_cpu_initial_reset(S390CPU *cpu)
 {
     CPUState *cs = CPU(cpu);
@@ -629,12 +622,9 @@ static int handle_sigp(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1)
 {
     CPUS390XState *env = &cpu->env;
     uint8_t order_code;
-    uint32_t parameter;
     uint16_t cpu_addr;
-    uint8_t t;
     int r = -1;
     S390CPU *target_cpu;
-    CPUS390XState *target_env;
 
     cpu_synchronize_state(CPU(cpu));
 
@@ -645,28 +635,16 @@ static int handle_sigp(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1)
     }
     order_code += (run->s390_sieic.ipb & 0x0fff0000) >> 16;
 
-    /* get parameters */
-    t = (ipa1 & 0xf0) >> 4;
-    if (!(t % 2)) {
-        t++;
-    }
-
-    parameter = env->regs[t] & 0x7ffffe00;
     cpu_addr = env->regs[ipa1 & 0x0f];
-
     target_cpu = s390_cpu_addr2state(cpu_addr);
     if (target_cpu == NULL) {
         goto out;
     }
-    target_env = &target_cpu->env;
 
     switch (order_code) {
         case SIGP_RESTART:
             r = kvm_s390_cpu_restart(target_cpu);
             break;
-        case SIGP_STORE_STATUS_ADDR:
-            r = s390_store_status(target_env, parameter);
-            break;
         case SIGP_SET_ARCH:
             /* make the caller panic */
             return -1;
-- 
1.8.3.4

[Qemu-devel] [PATCH 4/8] s390x/kvm: Fix coding style in handle_sigp()

[Jens Freimann]

From: Thomas Huth <thuth@linux.vnet.ibm.com>

To make scripts/checkpatch.pl happy for the following patches,
the coding style in handle_sigp() has to be fixed first.

Signed-off-by: Thomas Huth <thuth@linux.vnet.ibm.com>
Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
---
 target-s390x/kvm.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 5b243b4..8c54134 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -642,18 +642,18 @@ static int handle_sigp(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1)
     }
 
     switch (order_code) {
-        case SIGP_RESTART:
-            r = kvm_s390_cpu_restart(target_cpu);
-            break;
-        case SIGP_SET_ARCH:
-            /* make the caller panic */
-            return -1;
-        case SIGP_INITIAL_CPU_RESET:
-            r = s390_cpu_initial_reset(target_cpu);
-            break;
-        default:
-            fprintf(stderr, "KVM: unknown SIGP: 0x%x\n", order_code);
-            break;
+    case SIGP_RESTART:
+        r = kvm_s390_cpu_restart(target_cpu);
+        break;
+    case SIGP_SET_ARCH:
+        /* make the caller panic */
+        return -1;
+    case SIGP_INITIAL_CPU_RESET:
+        r = s390_cpu_initial_reset(target_cpu);
+        break;
+    default:
+        fprintf(stderr, "KVM: unknown SIGP: 0x%x\n", order_code);
+        break;
     }
 
 out:
-- 
1.8.3.4

[Qemu-devel] [PATCH 5/8] s390x/kvm: Implemented SIGP START

[Jens Freimann]

From: Thomas Huth <thuth@linux.vnet.ibm.com>

This patch adds the missing START order to the SIGP instruction handler.

Signed-off-by: Thomas Huth <thuth@linux.vnet.ibm.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
---
 target-s390x/kvm.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 8c54134..fcc159f 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -588,6 +588,14 @@ static int handle_diag(S390CPU *cpu, struct kvm_run *run, uint32_t ipb)
     return r;
 }
 
+static int kvm_s390_cpu_start(S390CPU *cpu)
+{
+    s390_add_running_cpu(cpu);
+    qemu_cpu_kick(CPU(cpu));
+    DPRINTF("DONE: KVM cpu start: %p\n", &cpu->env);
+    return 0;
+}
+
 int kvm_s390_cpu_restart(S390CPU *cpu)
 {
     kvm_s390_interrupt(cpu, KVM_S390_RESTART, 0);
@@ -642,6 +650,9 @@ static int handle_sigp(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1)
     }
 
     switch (order_code) {
+    case SIGP_START:
+        r = kvm_s390_cpu_start(target_cpu);
+        break;
     case SIGP_RESTART:
         r = kvm_s390_cpu_restart(target_cpu);
         break;
-- 
1.8.3.4

[Qemu-devel] [PATCH 6/8] s390x/kvm: Simplified the calculation of the SIGP order code

[Jens Freimann]

From: Thomas Huth <thuth@linux.vnet.ibm.com>

We've already got a helper function for calculating the
base/displacement of RS formatted instructions, so we can
get rid of the manual calculation of the SIGP order code.

Signed-off-by: Thomas Huth <thuth@linux.vnet.ibm.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
---
 target-s390x/kvm.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index fcc159f..0bf3d1f 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -626,6 +626,8 @@ static int s390_cpu_initial_reset(S390CPU *cpu)
     return 0;
 }
 
+#define SIGP_ORDER_MASK 0x000000ff
+
 static int handle_sigp(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1)
 {
     CPUS390XState *env = &cpu->env;
@@ -637,11 +639,7 @@ static int handle_sigp(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1)
     cpu_synchronize_state(CPU(cpu));
 
     /* get order code */
-    order_code = run->s390_sieic.ipb >> 28;
-    if (order_code > 0) {
-        order_code = env->regs[order_code];
-    }
-    order_code += (run->s390_sieic.ipb & 0x0fff0000) >> 16;
+    order_code = decode_basedisp_rs(env, run->s390_sieic.ipb) & SIGP_ORDER_MASK;
 
     cpu_addr = env->regs[ipa1 & 0x0f];
     target_cpu = s390_cpu_addr2state(cpu_addr);
-- 
1.8.3.4

[Qemu-devel] [PATCH 7/8] s390x/kvm: Fixed condition code for unknown SIGP orders

[Jens Freimann]

From: Thomas Huth <thuth@linux.vnet.ibm.com>

If SIGP is called with an unknown order code, it has to return CC1
instead of CC3 and set the "invalid order" bit in the return status.

Signed-off-by: Thomas Huth <thuth@linux.vnet.ibm.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
---
 target-s390x/kvm.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 0bf3d1f..f7b7726 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -633,8 +633,9 @@ static int handle_sigp(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1)
     CPUS390XState *env = &cpu->env;
     uint8_t order_code;
     uint16_t cpu_addr;
-    int r = -1;
     S390CPU *target_cpu;
+    uint64_t *statusreg = &env->regs[ipa1 >> 4];
+    int cc;
 
     cpu_synchronize_state(CPU(cpu));
 
@@ -644,29 +645,33 @@ static int handle_sigp(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1)
     cpu_addr = env->regs[ipa1 & 0x0f];
     target_cpu = s390_cpu_addr2state(cpu_addr);
     if (target_cpu == NULL) {
+        cc = 3;    /* not operational */
         goto out;
     }
 
     switch (order_code) {
     case SIGP_START:
-        r = kvm_s390_cpu_start(target_cpu);
+        cc = kvm_s390_cpu_start(target_cpu);
         break;
     case SIGP_RESTART:
-        r = kvm_s390_cpu_restart(target_cpu);
+        cc = kvm_s390_cpu_restart(target_cpu);
         break;
     case SIGP_SET_ARCH:
         /* make the caller panic */
         return -1;
     case SIGP_INITIAL_CPU_RESET:
-        r = s390_cpu_initial_reset(target_cpu);
+        cc = s390_cpu_initial_reset(target_cpu);
         break;
     default:
-        fprintf(stderr, "KVM: unknown SIGP: 0x%x\n", order_code);
+        DPRINTF("KVM: unknown SIGP: 0x%x\n", order_code);
+        *statusreg &= 0xffffffff00000000UL;
+        *statusreg |= SIGP_STAT_INVALID_ORDER;
+        cc = 1;   /* status stored */
         break;
     }
 
 out:
-    setcc(cpu, r ? 3 : 0);
+    setcc(cpu, cc);
     return 0;
 }
 
-- 
1.8.3.4

[Qemu-devel] [PATCH 8/8] s390x/ioinst: CHSC has to set a condition code

[Jens Freimann]

From: Thomas Huth <thuth@linux.vnet.ibm.com>

I missed to set the CC in the CHSC instruction when I refactored
the CC setting in the IO instructions with the following commit:
	5d9bf1c07c1369ab3506fc82cc65a10f4415d867
	s390/ioinst: Moved the CC setting to the IO instruction handlers
This patch now restores the correct behaviour of CHSC by setting the
condition code 0 at the end of the instruction.

Signed-off-by: Thomas Huth <thuth@linux.vnet.ibm.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
---
 target-s390x/ioinst.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target-s390x/ioinst.c b/target-s390x/ioinst.c
index 8d6363d..b8a6486 100644
--- a/target-s390x/ioinst.c
+++ b/target-s390x/ioinst.c
@@ -622,6 +622,7 @@ void ioinst_handle_chsc(S390CPU *cpu, uint32_t ipb)
         break;
     }
 
+    setcc(cpu, 0);    /* Command execution complete */
 out:
     s390_cpu_physical_memory_unmap(env, req, map_size, 1);
 }
-- 
1.8.3.4

Re: [Qemu-devel] [PATCH 5/8] s390x/kvm: Implemented SIGP START

[Alexander Graf]

On 17.12.2013, at 14:22, Jens Freimann <jfrei@linux.vnet.ibm.com> wrote:

> From: Thomas Huth <thuth@linux.vnet.ibm.com>
> 
> This patch adds the missing START order to the SIGP instruction handler.

Does the spec define what happens on START when the CPU is already running? Does START modify any register state?


Alex

> 
> Signed-off-by: Thomas Huth <thuth@linux.vnet.ibm.com>
> Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
> ---
> target-s390x/kvm.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
> 
> diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
> index 8c54134..fcc159f 100644
> --- a/target-s390x/kvm.c
> +++ b/target-s390x/kvm.c
> @@ -588,6 +588,14 @@ static int handle_diag(S390CPU *cpu, struct kvm_run *run, uint32_t ipb)
>     return r;
> }
> 
> +static int kvm_s390_cpu_start(S390CPU *cpu)
> +{
> +    s390_add_running_cpu(cpu);
> +    qemu_cpu_kick(CPU(cpu));
> +    DPRINTF("DONE: KVM cpu start: %p\n", &cpu->env);
> +    return 0;
> +}
> +
> int kvm_s390_cpu_restart(S390CPU *cpu)
> {
>     kvm_s390_interrupt(cpu, KVM_S390_RESTART, 0);
> @@ -642,6 +650,9 @@ static int handle_sigp(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1)
>     }
> 
>     switch (order_code) {
> +    case SIGP_START:
> +        r = kvm_s390_cpu_start(target_cpu);
> +        break;
>     case SIGP_RESTART:
>         r = kvm_s390_cpu_restart(target_cpu);
>         break;
> -- 
> 1.8.3.4
> 

Re: [Qemu-devel] [PATCH 8/8] s390x/ioinst: CHSC has to set a condition code

[Alexander Graf]

On 17.12.2013, at 14:22, Jens Freimann <jfrei@linux.vnet.ibm.com> wrote:

> From: Thomas Huth <thuth@linux.vnet.ibm.com>
> 
> I missed to set the CC in the CHSC instruction when I refactored
> the CC setting in the IO instructions with the following commit:
> 	5d9bf1c07c1369ab3506fc82cc65a10f4415d867
> 	s390/ioinst: Moved the CC setting to the IO instruction handlers
> This patch now restores the correct behaviour of CHSC by setting the
> condition code 0 at the end of the instruction.
> 
> Signed-off-by: Thomas Huth <thuth@linux.vnet.ibm.com>
> Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>

I suppose this patch should be CC'ed to stable?


Alex

> ---
> target-s390x/ioinst.c | 1 +
> 1 file changed, 1 insertion(+)
> 
> diff --git a/target-s390x/ioinst.c b/target-s390x/ioinst.c
> index 8d6363d..b8a6486 100644
> --- a/target-s390x/ioinst.c
> +++ b/target-s390x/ioinst.c
> @@ -622,6 +622,7 @@ void ioinst_handle_chsc(S390CPU *cpu, uint32_t ipb)
>         break;
>     }
> 
> +    setcc(cpu, 0);    /* Command execution complete */
> out:
>     s390_cpu_physical_memory_unmap(env, req, map_size, 1);
> }
> -- 
> 1.8.3.4
> 

Re: [Qemu-devel] [PATCH 5/8] s390x/kvm: Implemented SIGP START

[Thomas Huth]

 Hi Alex,

On Tue, 17 Dec 2013 14:56:33 +0100
Alexander Graf <agraf@suse.de> wrote:

> 
> On 17.12.2013, at 14:22, Jens Freimann <jfrei@linux.vnet.ibm.com> wrote:
> 
> > From: Thomas Huth <thuth@linux.vnet.ibm.com>
> > 
> > This patch adds the missing START order to the SIGP instruction handler.
> 
> Does the spec define what happens on START when the CPU is already running?

As far as I can see, the spec does not explicitely defines the behavior
in that case, it says only: "The order is effective only when the
addressed CPU is in the stopped state". But according to my experiments
(I wrote a test program that I ran without additional hypervisor), the
START order is simply ignored when the CPU is already running and CC0 is
returned.

This is also what happens with the code below, since
s390_add_running_cpu() only does something if the "halted" flag is set,
and kicking a CPU that is already running does not hurt either, as far
as I can see.

> Does START modify any register state?

No, the CPU simply continues running with the state where it has been
stopped before.

 Thomas


> 
> > 
> > Signed-off-by: Thomas Huth <thuth@linux.vnet.ibm.com>
> > Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> > Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
> > ---
> > target-s390x/kvm.c | 11 +++++++++++
> > 1 file changed, 11 insertions(+)
> > 
> > diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
> > index 8c54134..fcc159f 100644
> > --- a/target-s390x/kvm.c
> > +++ b/target-s390x/kvm.c
> > @@ -588,6 +588,14 @@ static int handle_diag(S390CPU *cpu, struct kvm_run *run, uint32_t ipb)
> >     return r;
> > }
> > 
> > +static int kvm_s390_cpu_start(S390CPU *cpu)
> > +{
> > +    s390_add_running_cpu(cpu);
> > +    qemu_cpu_kick(CPU(cpu));
> > +    DPRINTF("DONE: KVM cpu start: %p\n", &cpu->env);
> > +    return 0;
> > +}
> > +
> > int kvm_s390_cpu_restart(S390CPU *cpu)
> > {
> >     kvm_s390_interrupt(cpu, KVM_S390_RESTART, 0);
> > @@ -642,6 +650,9 @@ static int handle_sigp(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1)
> >     }
> > 
> >     switch (order_code) {
> > +    case SIGP_START:
> > +        r = kvm_s390_cpu_start(target_cpu);
> > +        break;
> >     case SIGP_RESTART:
> >         r = kvm_s390_cpu_restart(target_cpu);
> >         break;
> > -- 
> > 1.8.3.4
> > 
> 

Re: [Qemu-devel] [PATCH 5/8] s390x/kvm: Implemented SIGP START

[Alexander Graf]

On 17.12.2013, at 15:26, Thomas Huth <thuth@linux.vnet.ibm.com> wrote:

> 
> Hi Alex,
> 
> On Tue, 17 Dec 2013 14:56:33 +0100
> Alexander Graf <agraf@suse.de> wrote:
> 
>> 
>> On 17.12.2013, at 14:22, Jens Freimann <jfrei@linux.vnet.ibm.com> wrote:
>> 
>>> From: Thomas Huth <thuth@linux.vnet.ibm.com>
>>> 
>>> This patch adds the missing START order to the SIGP instruction handler.
>> 
>> Does the spec define what happens on START when the CPU is already running?
> 
> As far as I can see, the spec does not explicitely defines the behavior
> in that case, it says only: "The order is effective only when the
> addressed CPU is in the stopped state". But according to my experiments
> (I wrote a test program that I ran without additional hypervisor), the
> START order is simply ignored when the CPU is already running and CC0 is
> returned.
> 
> This is also what happens with the code below, since
> s390_add_running_cpu() only does something if the "halted" flag is set,
> and kicking a CPU that is already running does not hurt either, as far
> as I can see.

Yup :)

> 
>> Does START modify any register state?
> 
> No, the CPU simply continues running with the state where it has been
> stopped before.

Ok, great. Then it really does the same thing :)


Alex

Re: [Qemu-devel] [PATCH 8/8] s390x/ioinst: CHSC has to set a condition code

[Jens Freimann]

On Tue, Dec 17, 2013 at 03:01:43PM +0100, Alexander Graf wrote:
> 
> On 17.12.2013, at 14:22, Jens Freimann <jfrei@linux.vnet.ibm.com> wrote:
> 
> > From: Thomas Huth <thuth@linux.vnet.ibm.com>
> > 
> > I missed to set the CC in the CHSC instruction when I refactored
> > the CC setting in the IO instructions with the following commit:
> > 	5d9bf1c07c1369ab3506fc82cc65a10f4415d867
> > 	s390/ioinst: Moved the CC setting to the IO instruction handlers
> > This patch now restores the correct behaviour of CHSC by setting the
> > condition code 0 at the end of the instruction.
> > 
> > Signed-off-by: Thomas Huth <thuth@linux.vnet.ibm.com>
> > Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> > Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
> 
> I suppose this patch should be CC'ed to stable?

Good point, I'll send it again with stable on CC.
Same thing for Patch 1/8. 

Thanks!

regards
Jens
 
> 
> Alex
> 
> > ---
> > target-s390x/ioinst.c | 1 +
> > 1 file changed, 1 insertion(+)
> > 
> > diff --git a/target-s390x/ioinst.c b/target-s390x/ioinst.c
> > index 8d6363d..b8a6486 100644
> > --- a/target-s390x/ioinst.c
> > +++ b/target-s390x/ioinst.c
> > @@ -622,6 +622,7 @@ void ioinst_handle_chsc(S390CPU *cpu, uint32_t ipb)
> >         break;
> >     }
> > 
> > +    setcc(cpu, 0);    /* Command execution complete */
> > out:
> >     s390_cpu_physical_memory_unmap(env, req, map_size, 1);
> > }
> > -- 
> > 1.8.3.4
> > 
> 
> 

[Qemu-devel] [PATCH 1/8] s390x/kvm: Fix diagnose handling.

[Jens Freimann]

From: Cornelia Huck <cornelia.huck@de.ibm.com>

The instruction intercept handler for diagnose used only the displacement
when trying to calculate the function code. This is only correct for base
0, however; we need to perform a complete base/displacement address
calculation and use bits 48-63 as the function code.

Reviewed-by: Thomas Huth <thuth@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
---
 target-s390x/cpu.h |  3 +++
 target-s390x/kvm.c | 19 +++++++++++++------
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index a2c077b..68b5ab7 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -352,6 +352,9 @@ static inline hwaddr decode_basedisp_s(CPUS390XState *env, uint32_t ipb)
     return addr;
 }
 
+/* Base/displacement are at the same locations. */
+#define decode_basedisp_rs decode_basedisp_s
+
 void s390x_tod_timer(void *opaque);
 void s390x_cpu_timer(void *opaque);
 
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 02ac4ba..b00a661 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -562,11 +562,19 @@ static void kvm_handle_diag_308(S390CPU *cpu, struct kvm_run *run)
     handle_diag_308(&cpu->env, r1, r3);
 }
 
-static int handle_diag(S390CPU *cpu, struct kvm_run *run, int ipb_code)
+#define DIAG_KVM_CODE_MASK 0x000000000000ffff
+
+static int handle_diag(S390CPU *cpu, struct kvm_run *run, uint32_t ipb)
 {
     int r = 0;
-
-    switch (ipb_code) {
+    uint16_t func_code;
+
+    /*
+     * For any diagnose call we support, bits 48-63 of the resulting
+     * address specify the function code; the remainder is ignored.
+     */
+    func_code = decode_basedisp_rs(&cpu->env, ipb) & DIAG_KVM_CODE_MASK;
+    switch (func_code) {
     case DIAG_IPL:
         kvm_handle_diag_308(cpu, run);
         break;
@@ -577,7 +585,7 @@ static int handle_diag(S390CPU *cpu, struct kvm_run *run, int ipb_code)
         sleep(10);
         break;
     default:
-        DPRINTF("KVM: unknown DIAG: 0x%x\n", ipb_code);
+        DPRINTF("KVM: unknown DIAG: 0x%x\n", func_code);
         r = -1;
         break;
     }
@@ -684,7 +692,6 @@ static void handle_instruction(S390CPU *cpu, struct kvm_run *run)
 {
     unsigned int ipa0 = (run->s390_sieic.ipa & 0xff00);
     uint8_t ipa1 = run->s390_sieic.ipa & 0x00ff;
-    int ipb_code = (run->s390_sieic.ipb & 0x0fff0000) >> 16;
     int r = -1;
 
     DPRINTF("handle_instruction 0x%x 0x%x\n",
@@ -696,7 +703,7 @@ static void handle_instruction(S390CPU *cpu, struct kvm_run *run)
         r = handle_priv(cpu, run, ipa0 >> 8, ipa1);
         break;
     case IPA0_DIAG:
-        r = handle_diag(cpu, run, ipb_code);
+        r = handle_diag(cpu, run, run->s390_sieic.ipb);
         break;
     case IPA0_SIGP:
         r = handle_sigp(cpu, run, ipa1);
-- 
1.8.3.4

[Qemu-devel] [PATCH 8/8] s390x/ioinst: CHSC has to set a condition code

[Jens Freimann]

From: Thomas Huth <thuth@linux.vnet.ibm.com>

I missed to set the CC in the CHSC instruction when I refactored
the CC setting in the IO instructions with the following commit:
	5d9bf1c07c1369ab3506fc82cc65a10f4415d867
	s390/ioinst: Moved the CC setting to the IO instruction handlers
This patch now restores the correct behaviour of CHSC by setting the
condition code 0 at the end of the instruction.

Signed-off-by: Thomas Huth <thuth@linux.vnet.ibm.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
---
 target-s390x/ioinst.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target-s390x/ioinst.c b/target-s390x/ioinst.c
index 8d6363d..b8a6486 100644
--- a/target-s390x/ioinst.c
+++ b/target-s390x/ioinst.c
@@ -622,6 +622,7 @@ void ioinst_handle_chsc(S390CPU *cpu, uint32_t ipb)
         break;
     }
 
+    setcc(cpu, 0);    /* Command execution complete */
 out:
     s390_cpu_physical_memory_unmap(env, req, map_size, 1);
 }
-- 
1.8.3.4

Re: [Qemu-devel] [PATCH 0/8] s390 sigp, chsc and diag bugfixes/cleanups

[Alexander Graf]

On 17.12.2013, at 14:22, Jens Freimann <jfrei@linux.vnet.ibm.com> wrote:

> Alex,
> 
> this is mostly bugfixes and cleanup patches, except for Patch 4/8 which
> implements a SIGP order code for "cpu start".

Thanks, applied all to s390-next.


Alex