[Qemu-devel] [PATCH for-2.8 09/38] s390x/kvm: 2 byte software breakpoint support

[Cornelia Huck]

From: David Hildenbrand <address@hidden>

Diag 501 (4 bytes) was used until now for software breakpoints on s390.
As instructions on s390 might be 2 bytes long, temporarily overwriting them
with 4 bytes is evil and can result in very strange guest behaviour.

We make use of invalid instruction 0x0000 as new sw breakpoint instruction.
We have to enable interception of that instruction in KVM using a
capability.

If no software breakpoint has been inserted at the reported position, an
operation exception has to be injected into the guest. Otherwise a
breakpoint has been hit and the pc has to be rewound.

If KVM doesn't yet support interception of instruction 0x0000 the
existing mechanism exploiting diag 501 is used. To keep overhead low,
interception of instruction 0x0000 will only be enabled if sw breakpoints
are really used.

Reviewed-by: Christian Borntraeger <address@hidden>
Signed-off-by: David Hildenbrand <address@hidden>
Signed-off-by: Cornelia Huck <address@hidden>
---
 target-s390x/cpu.h |  2 ++
 target-s390x/kvm.c | 50 ++++++++++++++++++++++++++++++++++++++++----------
 2 files changed, 42 insertions(+), 10 deletions(-)

diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index c216bda..eeff1b0 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -394,6 +394,8 @@ static inline void cpu_get_tb_cpu_state(CPUS390XState* env, 
target_ulong *pc,
              ((env->psw.mask & PSW_MASK_32) ? FLAG_MASK_32 : 0);
 }
 
+#define MAX_ILEN 6
+
 /* While the PoO talks about ILC (a number between 1-3) what is actually
    stored in LowCore is shifted left one bit (an even between 2-6).  As
    this is the actual length of the insn and therefore more useful, that
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 80ac621..4341d54 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -109,6 +109,7 @@
 #define ICPT_WAITPSW                    0x1c
 #define ICPT_SOFT_INTERCEPT             0x24
 #define ICPT_CPU_STOP                   0x28
+#define ICPT_OPEREXC                    0x2c
 #define ICPT_IO                         0x40
 
 #define NR_LOCAL_IRQS 32
@@ -665,16 +666,37 @@ static void *legacy_s390_alloc(size_t size, uint64_t 
*align)
     return mem == MAP_FAILED ? NULL : mem;
 }
 
-/* DIAG 501 is used for sw breakpoints */
-static const uint8_t diag_501[] = {0x83, 0x24, 0x05, 0x01};
+static uint8_t const *sw_bp_inst;
+static uint8_t sw_bp_ilen;
+
+static void determine_sw_breakpoint_instr(void)
+{
+        /* DIAG 501 is used for sw breakpoints with old kernels */
+        static const uint8_t diag_501[] = {0x83, 0x24, 0x05, 0x01};
+        /* Instruction 0x0000 is used for sw breakpoints with recent kernels */
+        static const uint8_t instr_0x0000[] = {0x00, 0x00};
+
+        if (sw_bp_inst) {
+            return;
+        }
+        if (kvm_vm_enable_cap(kvm_state, KVM_CAP_S390_USER_INSTR0, 0)) {
+            sw_bp_inst = diag_501;
+            sw_bp_ilen = sizeof(diag_501);
+            DPRINTF("KVM: will use 4-byte sw breakpoints.\n");
+        } else {
+            sw_bp_inst = instr_0x0000;
+            sw_bp_ilen = sizeof(instr_0x0000);
+            DPRINTF("KVM: will use 2-byte sw breakpoints.\n");
+        }
+}
 
 int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
 {
+    determine_sw_breakpoint_instr();
 
     if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn,
-                            sizeof(diag_501), 0) ||
-        cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)diag_501,
-                            sizeof(diag_501), 1)) {
+                            sw_bp_ilen, 0) ||
+        cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)sw_bp_inst, sw_bp_ilen, 1)) 
{
         return -EINVAL;
     }
     return 0;
@@ -682,14 +704,14 @@ int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct 
kvm_sw_breakpoint *bp)
 
 int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
 {
-    uint8_t t[sizeof(diag_501)];
+    uint8_t t[MAX_ILEN];
 
-    if (cpu_memory_rw_debug(cs, bp->pc, t, sizeof(diag_501), 0)) {
+    if (cpu_memory_rw_debug(cs, bp->pc, t, sw_bp_ilen, 0)) {
         return -EINVAL;
-    } else if (memcmp(t, diag_501, sizeof(diag_501))) {
+    } else if (memcmp(t, sw_bp_inst, sw_bp_ilen)) {
         return -EINVAL;
     } else if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn,
-                                   sizeof(diag_501), 1)) {
+                                   sw_bp_ilen, 1)) {
         return -EINVAL;
     }
 
@@ -1310,7 +1332,7 @@ static int handle_sw_breakpoint(S390CPU *cpu, struct 
kvm_run *run)
 
     cpu_synchronize_state(CPU(cpu));
 
-    pc = env->psw.addr - 4;
+    pc = env->psw.addr - sw_bp_ilen;
     if (kvm_find_sw_breakpoint(CPU(cpu), pc)) {
         env->psw.addr = pc;
         return EXCP_DEBUG;
@@ -1864,6 +1886,14 @@ static int handle_intercept(S390CPU *cpu)
             cpu->env.sigp_order = 0;
             r = EXCP_HALTED;
             break;
+        case ICPT_OPEREXC:
+            /* currently only instr 0x0000 after enabled via capability */
+            r = handle_sw_breakpoint(cpu, run);
+            if (r == -ENOENT) {
+                enter_pgmcheck(cpu, PGM_OPERATION);
+                r = 0;
+            }
+            break;
         case ICPT_SOFT_INTERCEPT:
             fprintf(stderr, "KVM unimplemented icpt SOFT\n");
             exit(1);
-- 
2.9.3