[Qemu-devel] [PATCH 0/4] s390x/kvm: guest debugging support

[Qemu-devel] [PATCH 0/4] s390x/kvm: guest debugging support

[Jens Freimann]

[fixed subject]

Cornelia, Alex, Christian,

this series has four patches implementing GDB guest debugging
support for s390 KVM guests. 

- Patch 1 is a header sync
- Patch 2 is a bugfix 
- Patches 3-4 implement soft- and hardware breakpoint support

David Hildenbrand (3):
  s390x: remove duplicate definitions of DIAG 501
  s390x/kvm: software breakpoint support
  s390x/kvm: hw debugging support via guest PER facility

Jens Freimann (1):
  linux-headers: update

 linux-headers/asm-s390/kvm.h |  28 +++++++
 linux-headers/linux/kvm.h    |   7 +-
 target-s390x/cpu-qom.h       |   1 +
 target-s390x/helper.c        |  12 +++
 target-s390x/kvm.c           | 191 +++++++++++++++++++++++++++++++++++++++----
 5 files changed, 224 insertions(+), 15 deletions(-)

-- 
1.8.5.5

[Qemu-devel] [PATCH 1/4] linux-headers: update

[Jens Freimann]

Sync linux-headers with kvm/next (87c00572ba05aa8c9db118da75c608f47eb10b9e)

Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
---
 linux-headers/asm-s390/kvm.h | 28 ++++++++++++++++++++++++++++
 linux-headers/linux/kvm.h    |  7 ++++++-
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/linux-headers/asm-s390/kvm.h b/linux-headers/asm-s390/kvm.h
index c003c6a..98bedf3 100644
--- a/linux-headers/asm-s390/kvm.h
+++ b/linux-headers/asm-s390/kvm.h
@@ -15,6 +15,7 @@
 #include <linux/types.h>
 
 #define __KVM_S390
+#define __KVM_HAVE_GUEST_DEBUG
 
 /* Device control API: s390-specific devices */
 #define KVM_DEV_FLIC_GET_ALL_IRQS	1
@@ -54,6 +55,13 @@ struct kvm_s390_io_adapter_req {
 	__u64 addr;
 };
 
+/* kvm attr_group  on vm fd */
+#define KVM_S390_VM_MEM_CTRL		0
+
+/* kvm attributes for mem_ctrl */
+#define KVM_S390_VM_MEM_ENABLE_CMMA	0
+#define KVM_S390_VM_MEM_CLR_CMMA	1
+
 /* for KVM_GET_REGS and KVM_SET_REGS */
 struct kvm_regs {
 	/* general purpose regs for s390 */
@@ -72,11 +80,31 @@ struct kvm_fpu {
 	__u64 fprs[16];
 };
 
+#define KVM_GUESTDBG_USE_HW_BP		0x00010000
+
+#define KVM_HW_BP			1
+#define KVM_HW_WP_WRITE			2
+#define KVM_SINGLESTEP			4
+
 struct kvm_debug_exit_arch {
+	__u64 addr;
+	__u8 type;
+	__u8 pad[7]; /* Should be set to 0 */
+};
+
+struct kvm_hw_breakpoint {
+	__u64 addr;
+	__u64 phys_addr;
+	__u64 len;
+	__u8 type;
+	__u8 pad[7]; /* Should be set to 0 */
 };
 
 /* for KVM_SET_GUEST_DEBUG */
 struct kvm_guest_debug_arch {
+	__u32 nr_hw_bp;
+	__u32 pad; /* Should be set to 0 */
+	struct kvm_hw_breakpoint *hw_bp;
 };
 
 #define KVM_SYNC_PREFIX (1UL << 0)
diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index b278ab3..42ddc2c 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -416,6 +416,8 @@ struct kvm_s390_psw {
 #define KVM_S390_INT_PFAULT_INIT	0xfffe0004u
 #define KVM_S390_INT_PFAULT_DONE	0xfffe0005u
 #define KVM_S390_MCHK			0xfffe1000u
+#define KVM_S390_INT_CLOCK_COMP		0xffff1004u
+#define KVM_S390_INT_CPU_TIMER		0xffff1005u
 #define KVM_S390_INT_VIRTIO		0xffff2603u
 #define KVM_S390_INT_SERVICE		0xffff2401u
 #define KVM_S390_INT_EMERGENCY		0xffff1201u
@@ -515,6 +517,7 @@ enum {
 	kvm_ioeventfd_flag_nr_pio,
 	kvm_ioeventfd_flag_nr_deassign,
 	kvm_ioeventfd_flag_nr_virtio_ccw_notify,
+	kvm_ioeventfd_flag_nr_fast_mmio,
 	kvm_ioeventfd_flag_nr_max,
 };
 
@@ -529,7 +532,7 @@ enum {
 struct kvm_ioeventfd {
 	__u64 datamatch;
 	__u64 addr;        /* legal pio/mmio address */
-	__u32 len;         /* 1, 2, 4, or 8 bytes    */
+	__u32 len;         /* 1, 2, 4, or 8 bytes; or 0 to ignore length */
 	__s32 fd;
 	__u32 flags;
 	__u8  pad[36];
@@ -743,6 +746,8 @@ struct kvm_ppc_smmu_info {
 #define KVM_CAP_IOAPIC_POLARITY_IGNORED 97
 #define KVM_CAP_ENABLE_CAP_VM 98
 #define KVM_CAP_S390_IRQCHIP 99
+#define KVM_CAP_IOEVENTFD_NO_LENGTH 100
+#define KVM_CAP_VM_ATTRIBUTES 101
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
-- 
1.8.5.5

[Qemu-devel] [PATCH 2/4] s390x: remove duplicate definitions of DIAG 501

[Jens Freimann]

From: David Hildenbrand <dahi@linux.vnet.ibm.com>

When restoring the previously saved instruction in 
kvm_arch_remove_sw_breakpoint(), we only restored one byte. Let's use
the sizeof() operator to make sure we restore the entire instruction.

While we are at it, let's remove the duplicate definitions of DIAG 501
and replace its size (used when reading/writing the instruction) with
a sizeof() operator to make the code self explaining and less error-prone.


Signed-off-by: David Hildenbrand <dahi@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 | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index b7b0edc..70cfe74 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -347,12 +347,16 @@ static void *legacy_s390_alloc(size_t size)
     return mem == MAP_FAILED ? NULL : mem;
 }
 
+/* DIAG 501 is used for sw breakpoints */
+static const uint8_t diag_501[] = {0x83, 0x24, 0x05, 0x01};
+
 int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
 {
-    static const uint8_t diag_501[] = {0x83, 0x24, 0x05, 0x01};
 
-    if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 4, 0) ||
-        cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)diag_501, 4, 1)) {
+    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)) {
         return -EINVAL;
     }
     return 0;
@@ -360,14 +364,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[4];
-    static const uint8_t diag_501[] = {0x83, 0x24, 0x05, 0x01};
+    uint8_t t[sizeof(diag_501)];
 
-    if (cpu_memory_rw_debug(cs, bp->pc, t, 4, 0)) {
+    if (cpu_memory_rw_debug(cs, bp->pc, t, sizeof(diag_501), 0)) {
         return -EINVAL;
-    } else if (memcmp(t, diag_501, 4)) {
+    } else if (memcmp(t, diag_501, sizeof(diag_501))) {
         return -EINVAL;
-    } else if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 1, 1)) {
+    } else if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn,
+                                   sizeof(diag_501), 1)) {
         return -EINVAL;
     }
 
-- 
1.8.5.5

[Qemu-devel] [PATCH 3/4] s390x/kvm: software breakpoint support

[Jens Freimann]

From: David Hildenbrand <dahi@linux.vnet.ibm.com>

This patch allows to insert and remove sw breakpoints using the QEMU gdbserver
on s390 as well as to interrupt execution on a breakpoint hit when running
with KVM enabled.

Whenever a software breakpoint is inserted, common code calls kvm ioctl
KVM_UPDATE_GUEST_DEBUG. As this method's default on s390 is to return an error
if not implement, the insertion will fail. Therefore, KVM also has to be
updated in order to make use of software breakpoints.

Signed-off-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 target-s390x/kvm.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 70cfe74..bf62c95 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -610,6 +610,22 @@ static void kvm_handle_diag_308(S390CPU *cpu, struct kvm_run *run)
     handle_diag_308(&cpu->env, r1, r3);
 }
 
+static int handle_sw_breakpoint(S390CPU *cpu, struct kvm_run *run)
+{
+    CPUS390XState *env = &cpu->env;
+    unsigned long pc;
+
+    cpu_synchronize_state(CPU(cpu));
+
+    pc = env->psw.addr - 4;
+    if (kvm_find_sw_breakpoint(CPU(cpu), pc)) {
+        env->psw.addr = pc;
+        return EXCP_DEBUG;
+    }
+
+    return -ENOENT;
+}
+
 #define DIAG_KVM_CODE_MASK 0x000000000000ffff
 
 static int handle_diag(S390CPU *cpu, struct kvm_run *run, uint32_t ipb)
@@ -630,7 +646,7 @@ static int handle_diag(S390CPU *cpu, struct kvm_run *run, uint32_t ipb)
         r = handle_hypercall(cpu, run);
         break;
     case DIAG_KVM_BREAKPOINT:
-        sleep(10);
+        r = handle_sw_breakpoint(cpu, run);
         break;
     default:
         DPRINTF("KVM: unknown DIAG: 0x%x\n", func_code);
@@ -732,7 +748,7 @@ out:
     return 0;
 }
 
-static void handle_instruction(S390CPU *cpu, struct kvm_run *run)
+static int handle_instruction(S390CPU *cpu, struct kvm_run *run)
 {
     unsigned int ipa0 = (run->s390_sieic.ipa & 0xff00);
     uint8_t ipa1 = run->s390_sieic.ipa & 0x00ff;
@@ -759,8 +775,11 @@ static void handle_instruction(S390CPU *cpu, struct kvm_run *run)
     }
 
     if (r < 0) {
+        r = 0;
         enter_pgmcheck(cpu, 0x0001);
     }
+
+    return r;
 }
 
 static bool is_special_wait_psw(CPUState *cs)
@@ -780,7 +799,7 @@ static int handle_intercept(S390CPU *cpu)
             (long)cs->kvm_run->psw_addr);
     switch (icpt_code) {
         case ICPT_INSTRUCTION:
-            handle_instruction(cpu, run);
+            r = handle_instruction(cpu, run);
             break;
         case ICPT_WAITPSW:
             /* disabled wait, since enabled wait is handled in kernel */
-- 
1.8.5.5

[Qemu-devel] [PATCH 4/4] s390x/kvm: hw debugging support via guest PER facility

[Jens Freimann]

From: David Hildenbrand <dahi@linux.vnet.ibm.com>

This patch makes use of the hw debugging support in kvm (provided by the guest's
PER facility) on s390. It enables the following features, available using the
gdbserver:
- single-stepping
- hw breakpoints
- hw watchpoints

Signed-off-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
---
 target-s390x/cpu-qom.h |   1 +
 target-s390x/helper.c  |  12 ++++
 target-s390x/kvm.c     | 146 ++++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 156 insertions(+), 3 deletions(-)

diff --git a/target-s390x/cpu-qom.h b/target-s390x/cpu-qom.h
index ac0460e..f9c96d1 100644
--- a/target-s390x/cpu-qom.h
+++ b/target-s390x/cpu-qom.h
@@ -86,6 +86,7 @@ int s390_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cs,
 int s390_cpu_write_elf64_qemunote(WriteCoreDumpFunction f,
                                   CPUState *cpu, void *opaque);
 hwaddr s390_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
+hwaddr s390_cpu_get_phys_addr_debug(CPUState *cpu, vaddr addr);
 int s390_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
 int s390_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
 
diff --git a/target-s390x/helper.c b/target-s390x/helper.c
index 7c76fc1..3d756ca 100644
--- a/target-s390x/helper.c
+++ b/target-s390x/helper.c
@@ -489,6 +489,18 @@ hwaddr s390_cpu_get_phys_page_debug(CPUState *cs, vaddr vaddr)
     return raddr;
 }
 
+hwaddr s390_cpu_get_phys_addr_debug(CPUState *cs, vaddr vaddr)
+{
+    hwaddr phys_addr;
+    target_ulong page;
+
+    page = vaddr & TARGET_PAGE_MASK;
+    phys_addr = cpu_get_phys_page_debug(cs, page);
+    phys_addr += (vaddr & ~TARGET_PAGE_MASK);
+
+    return phys_addr;
+}
+
 void load_psw(CPUS390XState *env, uint64_t mask, uint64_t addr)
 {
     if (mask & PSW_MASK_WAIT) {
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index bf62c95..50af3e6 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -36,6 +36,7 @@
 #include "sysemu/device_tree.h"
 #include "qapi/qmp/qjson.h"
 #include "monitor/monitor.h"
+#include "exec/gdbstub.h"
 #include "trace.h"
 
 /* #define DEBUG_KVM */
@@ -86,6 +87,14 @@
 #define ICPT_CPU_STOP                   0x28
 #define ICPT_IO                         0x40
 
+static CPUWatchpoint hw_watchpoint;
+/*
+ * We don't use a list because this structure is also used to transmit the
+ * hardware breakpoints to the kernel.
+ */
+static struct kvm_hw_breakpoint *hw_breakpoints;
+static int nb_hw_breakpoints;
+
 const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
     KVM_CAP_LAST_INFO
 };
@@ -378,24 +387,126 @@ int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
     return 0;
 }
 
+static struct kvm_hw_breakpoint *find_hw_breakpoint(target_ulong addr,
+                                                    int len, int type)
+{
+    int n;
+
+    for (n = 0; n < nb_hw_breakpoints; n++) {
+        if (hw_breakpoints[n].addr == addr && hw_breakpoints[n].type == type &&
+            (hw_breakpoints[n].len == len || len == -1)) {
+            return &hw_breakpoints[n];
+        }
+    }
+
+    return NULL;
+}
+
+static int insert_hw_breakpoint(target_ulong addr, int len, int type)
+{
+    int size;
+
+    if (find_hw_breakpoint(addr, len, type)) {
+        return -EEXIST;
+    }
+
+    size = (nb_hw_breakpoints + 1) * sizeof(struct kvm_hw_breakpoint);
+
+    if (!hw_breakpoints) {
+        nb_hw_breakpoints = 0;
+        hw_breakpoints = (struct kvm_hw_breakpoint *)g_try_malloc(size);
+    } else {
+        hw_breakpoints =
+            (struct kvm_hw_breakpoint *)g_try_realloc(hw_breakpoints, size);
+    }
+
+    if (!hw_breakpoints) {
+        nb_hw_breakpoints = 0;
+        return -ENOMEM;
+    }
+
+    hw_breakpoints[nb_hw_breakpoints].addr = addr;
+    hw_breakpoints[nb_hw_breakpoints].len = len;
+    hw_breakpoints[nb_hw_breakpoints].type = type;
+
+    nb_hw_breakpoints++;
+
+    return 0;
+}
+
 int kvm_arch_insert_hw_breakpoint(target_ulong addr,
                                   target_ulong len, int type)
 {
-    return -ENOSYS;
+    switch (type) {
+    case GDB_BREAKPOINT_HW:
+        type = KVM_HW_BP;
+        break;
+    case GDB_WATCHPOINT_WRITE:
+        if (len < 1) {
+            return -EINVAL;
+        }
+        type = KVM_HW_WP_WRITE;
+        break;
+    default:
+        return -ENOSYS;
+    }
+    return insert_hw_breakpoint(addr, len, type);
 }
 
 int kvm_arch_remove_hw_breakpoint(target_ulong addr,
                                   target_ulong len, int type)
 {
-    return -ENOSYS;
+    int size;
+    struct kvm_hw_breakpoint *bp = find_hw_breakpoint(addr, len, type);
+
+    if (bp == NULL) {
+        return -ENOENT;
+    }
+
+    nb_hw_breakpoints--;
+    if (nb_hw_breakpoints > 0) {
+        /*
+         * In order to trim the array, move the last element to the position to
+         * be removed - if necessary.
+         */
+        if (bp != &hw_breakpoints[nb_hw_breakpoints]) {
+            *bp = hw_breakpoints[nb_hw_breakpoints];
+        }
+        size = nb_hw_breakpoints * sizeof(struct kvm_hw_breakpoint);
+        hw_breakpoints =
+             (struct kvm_hw_breakpoint *)g_realloc(hw_breakpoints, size);
+    } else {
+        g_free(hw_breakpoints);
+        hw_breakpoints = NULL;
+    }
+
+    return 0;
 }
 
 void kvm_arch_remove_all_hw_breakpoints(void)
 {
+    nb_hw_breakpoints = 0;
+    g_free(hw_breakpoints);
+    hw_breakpoints = NULL;
 }
 
 void kvm_arch_update_guest_debug(CPUState *cpu, struct kvm_guest_debug *dbg)
 {
+    int i;
+
+    if (nb_hw_breakpoints > 0) {
+        dbg->arch.nr_hw_bp = nb_hw_breakpoints;
+        dbg->arch.hw_bp = hw_breakpoints;
+
+        for (i = 0; i < nb_hw_breakpoints; ++i) {
+            hw_breakpoints[i].phys_addr = s390_cpu_get_phys_addr_debug(cpu,
+                                                       hw_breakpoints[i].addr);
+        }
+        dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_HW_BP;
+    } else {
+        dbg->arch.nr_hw_bp = 0;
+        dbg->arch.hw_bp = NULL;
+    }
 }
 
 void kvm_arch_pre_run(CPUState *cpu, struct kvm_run *run)
@@ -880,7 +991,36 @@ static int handle_tsch(S390CPU *cpu)
 
 static int kvm_arch_handle_debug_exit(S390CPU *cpu)
 {
-    return -ENOSYS;
+    CPUState *cs = CPU(cpu);
+    struct kvm_run *run = cs->kvm_run;
+
+    int ret = 0;
+    struct kvm_debug_exit_arch *arch_info = &run->debug.arch;
+
+    switch (arch_info->type) {
+    case KVM_HW_WP_WRITE:
+        if (find_hw_breakpoint(arch_info->addr, -1, arch_info->type)) {
+            cs->watchpoint_hit = &hw_watchpoint;
+            hw_watchpoint.vaddr = arch_info->addr;
+            hw_watchpoint.flags = BP_MEM_WRITE;
+            ret = EXCP_DEBUG;
+        }
+        break;
+    case KVM_HW_BP:
+        if (find_hw_breakpoint(arch_info->addr, -1, arch_info->type)) {
+            ret = EXCP_DEBUG;
+        }
+        break;
+    case KVM_SINGLESTEP:
+        if (cs->singlestep_enabled) {
+            ret = EXCP_DEBUG;
+        }
+        break;
+    default:
+        ret = -ENOSYS;
+    }
+
+    return ret;
 }
 
 int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
-- 
1.8.5.5

Re: [Qemu-devel] [PATCH 0/4] s390x/kvm: guest debugging support

[Cornelia Huck]

On Mon, 12 May 2014 14:58:48 +0200
Jens Freimann <jfrei@linux.vnet.ibm.com> wrote:

> [fixed subject]
> 
> Cornelia, Alex, Christian,
> 
> this series has four patches implementing GDB guest debugging
> support for s390 KVM guests. 
> 
> - Patch 1 is a header sync
> - Patch 2 is a bugfix 
> - Patches 3-4 implement soft- and hardware breakpoint support
> 
> David Hildenbrand (3):
>   s390x: remove duplicate definitions of DIAG 501
>   s390x/kvm: software breakpoint support
>   s390x/kvm: hw debugging support via guest PER facility
> 
> Jens Freimann (1):
>   linux-headers: update
> 
>  linux-headers/asm-s390/kvm.h |  28 +++++++
>  linux-headers/linux/kvm.h    |   7 +-
>  target-s390x/cpu-qom.h       |   1 +
>  target-s390x/helper.c        |  12 +++
>  target-s390x/kvm.c           | 191 +++++++++++++++++++++++++++++++++++++++----
>  5 files changed, 224 insertions(+), 15 deletions(-)
> 

Applied to my s390-next branch, thanks.