Re: [Qemu-devel] [PATCH 4/4] s390x/kvm: hw debugging support via guest PER facility
On 30/05/14 11:01, Alexander Graf wrote:
On 30.05.14 10:57, Christian Borntraeger wrote:
On 30/05/14 10:32, Alexander Graf wrote:
+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;
What happens to the diag 501 now? Are we safe to just drop it?
There can only be a small number of HW breakpoints (basically only one
from-to range on s390).
So gdb can (and will) use both (hbreak vs. break)
Ah, let me explain what I'm referring to here. On x86 (and PPC, though the
patches are still missing), we use a generic breakpoint instruction for
sw breakpoints. The specific breakpoint interrupt generated by that
instruction traps into KVM which forwards it to QEMU.
If QEMU now detects that it didn't put the breakpoint into place, it
assumes that it's the guest that wanted the breakpoint to happen, so it
deflects a breakpoint interrupt into the guest.
My question here is whether we need something similar on s390x. With DIAG,
I think we're safe, as the guest can't expect that one to do anything
useful, but if we want to switch to a 2-byte breakpoint instruction
instead, it might make sense to implement the deflection mechanism.
Oh, I though What happens to the diag 501 now? Are we safe to just drop it?
was a question if we can get rid of the code.
Regarding deflection, yes if guest and host hardware breakpoints (PER) we
need to handle that (The host kernel is doing that in filter_guest_per_event)
With software breakpoints: yes diag501 is safe to use. When we change the
instruction later on then we have to see if we need deflection (could be).
Christian
Hi Alex,
I am already working on a solution for 2 byte software breakpoints.
The solution will most likely look like what we have on x86: A generic
breakpoint instruction (e.g. invalid opcode 0x0001) that is filtered in
QEMU. We'll need kernel support to allow invalid instructions to be
intercepted and handled in QEMU. I already have a prototype running.
David
Re: [Qemu-devel] [PATCH 4/4] s390x/kvm: hw debugging support via guest PER facility
On 12.05.14 14:15, Jens Freimann wrote:
From: David Hildenbrand d...@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 d...@linux.vnet.ibm.com
Signed-off-by: Jens Freimann jf...@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);
+}
Phew. Is there really nothing like a dynamically growing array helper in
glib?
+
+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
Re: [Qemu-devel] [PATCH 4/4] s390x/kvm: hw debugging support via guest PER facility
On 30/05/14 10:32, Alexander Graf wrote:
+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;
What happens to the diag 501 now? Are we safe to just drop it?
There can only be a small number of HW breakpoints (basically only one from-to
range on s390).
So gdb can (and will) use both (hbreak vs. break)
Re: [Qemu-devel] [PATCH 4/4] s390x/kvm: hw debugging support via guest PER facility
On 30.05.14 10:57, Christian Borntraeger wrote:
On 30/05/14 10:32, Alexander Graf wrote:
+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;
What happens to the diag 501 now? Are we safe to just drop it?
There can only be a small number of HW breakpoints (basically only one from-to
range on s390).
So gdb can (and will) use both (hbreak vs. break)
Ah, let me explain what I'm referring to here. On x86 (and PPC, though
the patches are still missing), we use a generic breakpoint
instruction for sw breakpoints. The specific breakpoint interrupt
generated by that instruction traps into KVM which forwards it to QEMU.
If QEMU now detects that it didn't put the breakpoint into place, it
assumes that it's the guest that wanted the breakpoint to happen, so it
deflects a breakpoint interrupt into the guest.
My question here is whether we need something similar on s390x. With
DIAG, I think we're safe, as the guest can't expect that one to do
anything useful, but if we want to switch to a 2-byte breakpoint
instruction instead, it might make sense to implement the deflection
mechanism.
Alex
Re: [Qemu-devel] [PATCH 4/4] s390x/kvm: hw debugging support via guest PER facility
On 30/05/14 11:01, Alexander Graf wrote:
On 30.05.14 10:57, Christian Borntraeger wrote:
On 30/05/14 10:32, Alexander Graf wrote:
+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;
What happens to the diag 501 now? Are we safe to just drop it?
There can only be a small number of HW breakpoints (basically only one
from-to range on s390).
So gdb can (and will) use both (hbreak vs. break)
Ah, let me explain what I'm referring to here. On x86 (and PPC, though the
patches are still missing), we use a generic breakpoint instruction for sw
breakpoints. The specific breakpoint interrupt generated by that instruction
traps into KVM which forwards it to QEMU.
If QEMU now detects that it didn't put the breakpoint into place, it assumes
that it's the guest that wanted the breakpoint to happen, so it deflects a
breakpoint interrupt into the guest.
My question here is whether we need something similar on s390x. With DIAG, I
think we're safe, as the guest can't expect that one to do anything useful,
but if we want to switch to a 2-byte breakpoint instruction instead, it might
make sense to implement the deflection mechanism.
Oh, I though What happens to the diag 501 now? Are we safe to just drop it?
was a question if we can get rid of the code.
Regarding deflection, yes if guest and host hardware breakpoints (PER) we need
to handle that (The host kernel is doing that in filter_guest_per_event)
With software breakpoints: yes diag501 is safe to use. When we change the
instruction later on then we have to see if we need deflection (could be).
Christian
[Qemu-devel] [PATCH 4/4] s390x/kvm: hw debugging support via guest PER facility
From: David Hildenbrand d...@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 d...@linux.vnet.ibm.com
Signed-off-by: Jens Freimann jf...@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]) {
+
[Qemu-devel] [PATCH 4/4] s390x/kvm: hw debugging support via guest PER facility
From: David Hildenbrand d...@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 d...@linux.vnet.ibm.com
Signed-off-by: Jens Freimann jf...@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]) {
+
