[Qemu-devel] [PATCH 0/5] s390x/kvm: additional fixes and cleanup

[Qemu-devel] [PATCH 0/5] s390x/kvm: additional fixes and cleanup

[Christian Borntraeger]

Folks,

here are 5 additional patches for s390x/kvm that I want to push for 2.0.

Frank Blaschka (1):
  s390x/kvm: Rework priv instruction handlers

Thomas Huth (4):
  s390x/cpu: Use ioctl to reset state in the kernel
  s390x/kvm: Rework SIGP INITIAL CPU RESET handler
  s390x/kvm: Add missing SIGP CPU RESET order
  s390x/ipl: Fix crash of ELF images with arbitrary entry points

 hw/s390x/ipl.c     |  21 ++++---
 target-s390x/cpu.c |   9 +++
 target-s390x/kvm.c | 173 ++++++++++++++++++++++++++++++-----------------------
 3 files changed, 120 insertions(+), 83 deletions(-)

-- 
1.8.4.2

[Qemu-devel] [PATCH 1/5] s390x/cpu: Use ioctl to reset state in the kernel

[Christian Borntraeger]

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

Some of the state in the kernel can not be reset from QEMU yet.
For this we've got to use the KVM_S390_INITIAL_RESET ioctl to make
sure that the state in the kernel is set to the right values during
initial CPU reset, too.

Signed-off-by: Thomas Huth <thuth@linux.vnet.ibm.com>
Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 target-s390x/cpu.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index f1319e5..1a8c1cc 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -108,6 +108,15 @@ static void s390_cpu_initial_reset(CPUState *s)
     env->cregs[14] = CR14_RESET;
 
     env->pfault_token = -1UL;
+
+#if defined(CONFIG_KVM)
+    /* Reset state inside the kernel that we cannot access yet from QEMU. */
+    if (kvm_enabled()) {
+        if (kvm_vcpu_ioctl(s, KVM_S390_INITIAL_RESET, NULL)) {
+            perror("Initial CPU reset failed");
+        }
+    }
+#endif
 }
 
 /* CPUClass:reset() */
-- 
1.8.4.2

[Qemu-devel] [PATCH 2/5] s390x/kvm: Rework SIGP INITIAL CPU RESET handler

[Christian Borntraeger]

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

The s390_cpu_initial_reset() function had two deficiencies: First, it
used an ioctl for the destination CPU, and this ioctl could block
nearly forever, as long as the destination CPU was running in the SIE
loop. Second, it also cleared the general purpose registers - something
it should not do according to the Principles of Operations.
Since we've already got another function for the initial CPU reset in
cpu.c, we can also use that function instead. And by using run_on_cpu()
for executing this code, we make sure that the destination CPU is
correctly kicked out of kernel mode now.

Suggested-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Thomas Huth <thuth@linux.vnet.ibm.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 target-s390x/kvm.c | 25 +++++++------------------
 1 file changed, 7 insertions(+), 18 deletions(-)

diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index d3f0d4a..75e8822 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -626,25 +626,13 @@ int kvm_s390_cpu_restart(S390CPU *cpu)
     return 0;
 }
 
-static int s390_cpu_initial_reset(S390CPU *cpu)
+static void sigp_initial_cpu_reset(void *arg)
 {
-    CPUState *cs = CPU(cpu);
-    CPUS390XState *env = &cpu->env;
-    int i;
-
-    s390_del_running_cpu(cpu);
-    if (kvm_vcpu_ioctl(cs, KVM_S390_INITIAL_RESET, NULL) < 0) {
-        perror("cannot init reset vcpu");
-    }
+    CPUState *cpu = arg;
+    S390CPUClass *scc = S390_CPU_GET_CLASS(cpu);
 
-    /* Manually zero out all registers */
-    cpu_synchronize_state(cs);
-    for (i = 0; i < 16; i++) {
-        env->regs[i] = 0;
-    }
-
-    DPRINTF("DONE: SIGP initial reset: %p\n", env);
-    return 0;
+    cpu_synchronize_state(cpu);
+    scc->initial_cpu_reset(cpu);
 }
 
 #define SIGP_ORDER_MASK 0x000000ff
@@ -683,7 +671,8 @@ static int handle_sigp(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1)
         cc = 1;   /* status stored */
         break;
     case SIGP_INITIAL_CPU_RESET:
-        cc = s390_cpu_initial_reset(target_cpu);
+        run_on_cpu(CPU(target_cpu), sigp_initial_cpu_reset, CPU(target_cpu));
+        cc = 0;
         break;
     default:
         DPRINTF("KVM: unknown SIGP: 0x%x\n", order_code);
-- 
1.8.4.2

[Qemu-devel] [PATCH 3/5] s390x/kvm: Add missing SIGP CPU RESET order

[Christian Borntraeger]

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

The SIGP order CPU RESET was still missing in the list of our
supported handler. This patch now adds a simple implementation,
by using the cpu_reset() function that is already available in
target-s390x/cpu.c.

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

diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 75e8822..20c711f 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -635,6 +635,15 @@ static void sigp_initial_cpu_reset(void *arg)
     scc->initial_cpu_reset(cpu);
 }
 
+static void sigp_cpu_reset(void *arg)
+{
+    CPUState *cpu = arg;
+    S390CPUClass *scc = S390_CPU_GET_CLASS(cpu);
+
+    cpu_synchronize_state(cpu);
+    scc->cpu_reset(cpu);
+}
+
 #define SIGP_ORDER_MASK 0x000000ff
 
 static int handle_sigp(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1)
@@ -674,6 +683,10 @@ static int handle_sigp(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1)
         run_on_cpu(CPU(target_cpu), sigp_initial_cpu_reset, CPU(target_cpu));
         cc = 0;
         break;
+    case SIGP_CPU_RESET:
+        run_on_cpu(CPU(target_cpu), sigp_cpu_reset, CPU(target_cpu));
+        cc = 0;
+        break;
     default:
         DPRINTF("KVM: unknown SIGP: 0x%x\n", order_code);
         *statusreg &= 0xffffffff00000000UL;
-- 
1.8.4.2

[Qemu-devel] [PATCH 4/5] s390x/kvm: Rework priv instruction handlers

[Christian Borntraeger]

From: Frank Blaschka <blaschka@linux.vnet.ibm.com>

The current implementation uses the second byte of the instruction
to identify the instruction handler. This is not sufficient to
support instructions not starting with 0xb2. This patch
adds separate handlers for 0xb2, 0xb9 and 0xeb to be able to
support the full instruction set.

Signed-off-by: Frank Blaschka <blaschka@linux.vnet.ibm.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 target-s390x/kvm.c | 139 ++++++++++++++++++++++++++++++-----------------------
 1 file changed, 80 insertions(+), 59 deletions(-)

diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 20c711f..11feda9 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -53,25 +53,28 @@
 #define IPA0_B9                         0xb900
 #define IPA0_EB                         0xeb00
 
-#define PRIV_SCLP_CALL                  0x20
-#define PRIV_CSCH                       0x30
-#define PRIV_HSCH                       0x31
-#define PRIV_MSCH                       0x32
-#define PRIV_SSCH                       0x33
-#define PRIV_STSCH                      0x34
-#define PRIV_TSCH                       0x35
-#define PRIV_TPI                        0x36
-#define PRIV_SAL                        0x37
-#define PRIV_RSCH                       0x38
-#define PRIV_STCRW                      0x39
-#define PRIV_STCPS                      0x3a
-#define PRIV_RCHP                       0x3b
-#define PRIV_SCHM                       0x3c
-#define PRIV_CHSC                       0x5f
-#define PRIV_SIGA                       0x74
-#define PRIV_XSCH                       0x76
-#define PRIV_SQBS                       0x8a
-#define PRIV_EQBS                       0x9c
+#define PRIV_B2_SCLP_CALL               0x20
+#define PRIV_B2_CSCH                    0x30
+#define PRIV_B2_HSCH                    0x31
+#define PRIV_B2_MSCH                    0x32
+#define PRIV_B2_SSCH                    0x33
+#define PRIV_B2_STSCH                   0x34
+#define PRIV_B2_TSCH                    0x35
+#define PRIV_B2_TPI                     0x36
+#define PRIV_B2_SAL                     0x37
+#define PRIV_B2_RSCH                    0x38
+#define PRIV_B2_STCRW                   0x39
+#define PRIV_B2_STCPS                   0x3a
+#define PRIV_B2_RCHP                    0x3b
+#define PRIV_B2_SCHM                    0x3c
+#define PRIV_B2_CHSC                    0x5f
+#define PRIV_B2_SIGA                    0x74
+#define PRIV_B2_XSCH                    0x76
+
+#define PRIV_EB_SQBS                    0x8a
+
+#define PRIV_B9_EQBS                    0x9c
+
 #define DIAG_IPL                        0x308
 #define DIAG_KVM_HYPERCALL              0x500
 #define DIAG_KVM_BREAKPOINT             0x501
@@ -458,96 +461,110 @@ static int kvm_sclp_service_call(S390CPU *cpu, struct kvm_run *run,
     return 0;
 }
 
-static int kvm_handle_css_inst(S390CPU *cpu, struct kvm_run *run,
-                               uint8_t ipa0, uint8_t ipa1, uint8_t ipb)
+static int handle_b2(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1)
 {
     CPUS390XState *env = &cpu->env;
-
-    if (ipa0 != 0xb2) {
-        /* Not handled for now. */
-        return -1;
-    }
+    int rc = 0;
+    uint16_t ipbh0 = (run->s390_sieic.ipb & 0xffff0000) >> 16;
 
     cpu_synchronize_state(CPU(cpu));
 
     switch (ipa1) {
-    case PRIV_XSCH:
+    case PRIV_B2_XSCH:
         ioinst_handle_xsch(cpu, env->regs[1]);
         break;
-    case PRIV_CSCH:
+    case PRIV_B2_CSCH:
         ioinst_handle_csch(cpu, env->regs[1]);
         break;
-    case PRIV_HSCH:
+    case PRIV_B2_HSCH:
         ioinst_handle_hsch(cpu, env->regs[1]);
         break;
-    case PRIV_MSCH:
+    case PRIV_B2_MSCH:
         ioinst_handle_msch(cpu, env->regs[1], run->s390_sieic.ipb);
         break;
-    case PRIV_SSCH:
+    case PRIV_B2_SSCH:
         ioinst_handle_ssch(cpu, env->regs[1], run->s390_sieic.ipb);
         break;
-    case PRIV_STCRW:
+    case PRIV_B2_STCRW:
         ioinst_handle_stcrw(cpu, run->s390_sieic.ipb);
         break;
-    case PRIV_STSCH:
+    case PRIV_B2_STSCH:
         ioinst_handle_stsch(cpu, env->regs[1], run->s390_sieic.ipb);
         break;
-    case PRIV_TSCH:
+    case PRIV_B2_TSCH:
         /* We should only get tsch via KVM_EXIT_S390_TSCH. */
         fprintf(stderr, "Spurious tsch intercept\n");
         break;
-    case PRIV_CHSC:
+    case PRIV_B2_CHSC:
         ioinst_handle_chsc(cpu, run->s390_sieic.ipb);
         break;
-    case PRIV_TPI:
+    case PRIV_B2_TPI:
         /* This should have been handled by kvm already. */
         fprintf(stderr, "Spurious tpi intercept\n");
         break;
-    case PRIV_SCHM:
+    case PRIV_B2_SCHM:
         ioinst_handle_schm(cpu, env->regs[1], env->regs[2],
                            run->s390_sieic.ipb);
         break;
-    case PRIV_RSCH:
+    case PRIV_B2_RSCH:
         ioinst_handle_rsch(cpu, env->regs[1]);
         break;
-    case PRIV_RCHP:
+    case PRIV_B2_RCHP:
         ioinst_handle_rchp(cpu, env->regs[1]);
         break;
-    case PRIV_STCPS:
+    case PRIV_B2_STCPS:
         /* We do not provide this instruction, it is suppressed. */
         break;
-    case PRIV_SAL:
+    case PRIV_B2_SAL:
         ioinst_handle_sal(cpu, env->regs[1]);
         break;
-    case PRIV_SIGA:
+    case PRIV_B2_SIGA:
         /* Not provided, set CC = 3 for subchannel not operational */
         setcc(cpu, 3);
         break;
+    case PRIV_B2_SCLP_CALL:
+        rc = kvm_sclp_service_call(cpu, run, ipbh0);
+        break;
     default:
-        return -1;
+        rc = -1;
+        DPRINTF("KVM: unhandled PRIV: 0xb2%x\n", ipa1);
+        break;
     }
 
-    return 0;
+    return rc;
 }
 
-static int handle_priv(S390CPU *cpu, struct kvm_run *run,
-                       uint8_t ipa0, uint8_t ipa1)
+static int handle_b9(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1)
 {
     int r = 0;
-    uint16_t ipbh0 = (run->s390_sieic.ipb & 0xffff0000) >> 16;
-    uint8_t ipb = run->s390_sieic.ipb & 0xff;
 
-    DPRINTF("KVM: PRIV: %d\n", ipa1);
     switch (ipa1) {
-        case PRIV_SCLP_CALL:
-            r = kvm_sclp_service_call(cpu, run, ipbh0);
-            break;
-        default:
-            r = kvm_handle_css_inst(cpu, run, ipa0, ipa1, ipb);
-            if (r == -1) {
-                DPRINTF("KVM: unhandled PRIV: 0x%x\n", ipa1);
-            }
-            break;
+    case PRIV_B9_EQBS:
+        /* just inject exception */
+        r = -1;
+        break;
+    default:
+        r = -1;
+        DPRINTF("KVM: unhandled PRIV: 0xb9%x\n", ipa1);
+        break;
+    }
+
+    return r;
+}
+
+static int handle_eb(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1)
+{
+    int r = 0;
+
+    switch (ipa1) {
+    case PRIV_EB_SQBS:
+        /* just inject exception */
+        r = -1;
+        break;
+    default:
+        r = -1;
+        DPRINTF("KVM: unhandled PRIV: 0xeb%x\n", ipa1);
+        break;
     }
 
     return r;
@@ -710,9 +727,13 @@ static void handle_instruction(S390CPU *cpu, struct kvm_run *run)
             run->s390_sieic.ipa, run->s390_sieic.ipb);
     switch (ipa0) {
     case IPA0_B2:
+        r = handle_b2(cpu, run, ipa1);
+        break;
     case IPA0_B9:
+        r = handle_b9(cpu, run, ipa1);
+        break;
     case IPA0_EB:
-        r = handle_priv(cpu, run, ipa0 >> 8, ipa1);
+        r = handle_eb(cpu, run, ipa1);
         break;
     case IPA0_DIAG:
         r = handle_diag(cpu, run, run->s390_sieic.ipb);
-- 
1.8.4.2

[Qemu-devel] [PATCH 5/5] s390x/ipl: Fix crash of ELF images with arbitrary entry points

[Christian Borntraeger]

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

When loading S390 kernels, the current code expects an ELF file with the
start address 0x10000. Other ELF files cause a segmentation fault. To avoid
these crashes, we should get the start address from the ELF file instead
of always using a hard-coded address.

Signed-off-by: Thomas Huth <thuth@linux.vnet.ibm.com>
Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 hw/s390x/ipl.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 1a6397b..04fb1a8 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -95,7 +95,8 @@ static int s390_ipl_init(SysBusDevice *dev)
         }
         return 0;
     } else {
-        kernel_size = load_elf(ipl->kernel, NULL, NULL, NULL, NULL,
+        uint64_t pentry = KERN_IMAGE_START;
+        kernel_size = load_elf(ipl->kernel, NULL, NULL, &pentry, NULL,
                                NULL, 1, ELF_MACHINE, 0);
         if (kernel_size == -1) {
             kernel_size = load_image_targphys(ipl->kernel, 0, ram_size);
@@ -104,15 +105,19 @@ static int s390_ipl_init(SysBusDevice *dev)
             fprintf(stderr, "could not load kernel '%s'\n", ipl->kernel);
             return -1;
         }
-        /* we have to overwrite values in the kernel image, which are "rom" */
-        strcpy(rom_ptr(KERN_PARM_AREA), ipl->cmdline);
-
         /*
-         * we can not rely on the ELF entry point, since up to 3.2 this
-         * value was 0x800 (the SALIPL loader) and it wont work. For
-         * all (Linux) cases 0x10000 (KERN_IMAGE_START) should be fine.
+         * Is it a Linux kernel (starting at 0x10000)? If yes, we fill in the
+         * kernel parameters here as well. Note: For old kernels (up to 3.2)
+         * we can not rely on the ELF entry point - it was 0x800 (the SALIPL
+         * loader) and it won't work. For this case we force it to 0x10000, too.
          */
-        ipl->start_addr = KERN_IMAGE_START;
+        if (pentry == KERN_IMAGE_START || pentry == 0x800) {
+            ipl->start_addr = KERN_IMAGE_START;
+            /* Overwrite parameters in the kernel image, which are "rom" */
+            strcpy(rom_ptr(KERN_PARM_AREA), ipl->cmdline);
+        } else {
+            ipl->start_addr = pentry;
+        }
     }
     if (ipl->initrd) {
         ram_addr_t initrd_offset;
-- 
1.8.4.2