* [PATCH v3 01/12] KVM: add comments for kvm_debug_exit_arch struct
[not found] <1430929407-3487-1-git-send-email-alex.bennee@linaro.org>
@ 2015-05-06 16:23 ` Alex Bennée
2015-05-08 9:19 ` Christoffer Dall
2015-05-06 16:23 ` [PATCH v3 02/12] KVM: define common __KVM_GUESTDBG_USE_SW/HW_BP values Alex Bennée
` (10 subsequent siblings)
11 siblings, 1 reply; 24+ messages in thread
From: Alex Bennée @ 2015-05-06 16:23 UTC (permalink / raw)
To: kvm, linux-arm-kernel, kvmarm, christoffer.dall, marc.zyngier,
peter.maydell, agraf, drjones, pbonzini, zhichao.huang
Cc: jan.kiszka, dahi, r65777, bp, Alex Bennée, Gleb Natapov,
Jonathan Corbet, open list:DOCUMENTATION, open list,
open list:ABI/API
Bring into line with the comments for the other structures and their
KVM_EXIT_* cases. Also update api.txt to reflect use in kvm_run
documentation.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
---
v2
- add comments for other exit types
v3
- s/commentary/comments/
- add rb tags
- update api.txt kvm_run to include KVM_EXIT_DEBUG desc
diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 9fa2bf8..cb90025 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -3070,11 +3070,13 @@ data_offset describes where the data is located (KVM_EXIT_IO_OUT) or
where kvm expects application code to place the data for the next
KVM_RUN invocation (KVM_EXIT_IO_IN). Data format is a packed array.
+ /* KVM_EXIT_DEBUG */
struct {
struct kvm_debug_exit_arch arch;
} debug;
-Unused.
+If the exit_reason in KVM_EXIT_DEBUG, then a vcpu is processing a debug event
+for which architecture dependant information is returned.
/* KVM_EXIT_MMIO */
struct {
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 4b60056..70ac641 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -237,6 +237,7 @@ struct kvm_run {
__u32 count;
__u64 data_offset; /* relative to kvm_run start */
} io;
+ /* KVM_EXIT_DEBUG */
struct {
struct kvm_debug_exit_arch arch;
} debug;
@@ -285,6 +286,7 @@ struct kvm_run {
__u32 data;
__u8 is_write;
} dcr;
+ /* KVM_EXIT_INTERNAL_ERROR */
struct {
__u32 suberror;
/* Available with KVM_CAP_INTERNAL_ERROR_DATA: */
@@ -295,6 +297,7 @@ struct kvm_run {
struct {
__u64 gprs[32];
} osi;
+ /* KVM_EXIT_PAPR_HCALL */
struct {
__u64 nr;
__u64 ret;
--
2.3.5
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 02/12] KVM: define common __KVM_GUESTDBG_USE_SW/HW_BP values
[not found] <1430929407-3487-1-git-send-email-alex.bennee@linaro.org>
2015-05-06 16:23 ` [PATCH v3 01/12] KVM: add comments for kvm_debug_exit_arch struct Alex Bennée
@ 2015-05-06 16:23 ` Alex Bennée
2015-05-08 9:23 ` Christoffer Dall
2015-05-06 16:23 ` [PATCH v3 03/12] KVM: arm64: guest debug, define API headers Alex Bennée
` (9 subsequent siblings)
11 siblings, 1 reply; 24+ messages in thread
From: Alex Bennée @ 2015-05-06 16:23 UTC (permalink / raw)
To: kvm, linux-arm-kernel, kvmarm, christoffer.dall, marc.zyngier,
peter.maydell, agraf, drjones, pbonzini, zhichao.huang
Cc: jan.kiszka, dahi, r65777, bp, Alex Bennée,
Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
maintainer:X86 ARCHITECTURE...,
Gleb Natapov, Bharat Bhushan, Mihai Caraman,
Alexey Kardashevskiy, Nadav Amit, open list:LINUX FOR POWERPC...,
open list, open list:ABI/API
Currently x86, powerpc and soon arm64 use the same two architecture
specific bits for guest debug support for software and hardware
breakpoints. This makes the shared values explicit while leaving the
gate open for another architecture to use some other value if they
really really want to.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Andrew Jones <drjones@redhat.com>
diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
index ab4d473..1731569 100644
--- a/arch/powerpc/include/uapi/asm/kvm.h
+++ b/arch/powerpc/include/uapi/asm/kvm.h
@@ -310,8 +310,8 @@ struct kvm_guest_debug_arch {
* and upper 16 bits are architecture specific. Architecture specific defines
* that ioctl is for setting hardware breakpoint or software breakpoint.
*/
-#define KVM_GUESTDBG_USE_SW_BP 0x00010000
-#define KVM_GUESTDBG_USE_HW_BP 0x00020000
+#define KVM_GUESTDBG_USE_SW_BP __KVM_GUESTDBG_USE_SW_BP
+#define KVM_GUESTDBG_USE_HW_BP __KVM_GUESTDBG_USE_HW_BP
/* definition of registers in kvm_run */
struct kvm_sync_regs {
diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
index d7dcef5..1438202 100644
--- a/arch/x86/include/uapi/asm/kvm.h
+++ b/arch/x86/include/uapi/asm/kvm.h
@@ -250,8 +250,8 @@ struct kvm_debug_exit_arch {
__u64 dr7;
};
-#define KVM_GUESTDBG_USE_SW_BP 0x00010000
-#define KVM_GUESTDBG_USE_HW_BP 0x00020000
+#define KVM_GUESTDBG_USE_SW_BP __KVM_GUESTDBG_USE_SW_BP
+#define KVM_GUESTDBG_USE_HW_BP __KVM_GUESTDBG_USE_HW_BP
#define KVM_GUESTDBG_INJECT_DB 0x00040000
#define KVM_GUESTDBG_INJECT_BP 0x00080000
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 70ac641..3b6252e 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -570,8 +570,16 @@ struct kvm_s390_irq_state {
/* for KVM_SET_GUEST_DEBUG */
-#define KVM_GUESTDBG_ENABLE 0x00000001
-#define KVM_GUESTDBG_SINGLESTEP 0x00000002
+#define KVM_GUESTDBG_ENABLE (1 << 0)
+#define KVM_GUESTDBG_SINGLESTEP (1 << 1)
+
+/*
+ * Architecture specific stuff uses the top 16 bits of the field,
+ * however there is some shared commonality for the common cases
+ */
+#define __KVM_GUESTDBG_USE_SW_BP (1 << 16)
+#define __KVM_GUESTDBG_USE_HW_BP (1 << 17)
+
struct kvm_guest_debug {
__u32 control;
--
2.3.5
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 03/12] KVM: arm64: guest debug, define API headers
[not found] <1430929407-3487-1-git-send-email-alex.bennee@linaro.org>
2015-05-06 16:23 ` [PATCH v3 01/12] KVM: add comments for kvm_debug_exit_arch struct Alex Bennée
2015-05-06 16:23 ` [PATCH v3 02/12] KVM: define common __KVM_GUESTDBG_USE_SW/HW_BP values Alex Bennée
@ 2015-05-06 16:23 ` Alex Bennée
2015-05-08 9:28 ` Christoffer Dall
2015-05-06 16:23 ` [PATCH v3 04/12] KVM: arm: guest debug, add stub KVM_SET_GUEST_DEBUG ioctl Alex Bennée
` (8 subsequent siblings)
11 siblings, 1 reply; 24+ messages in thread
From: Alex Bennée @ 2015-05-06 16:23 UTC (permalink / raw)
To: kvm, linux-arm-kernel, kvmarm, christoffer.dall, marc.zyngier,
peter.maydell, agraf, drjones, pbonzini, zhichao.huang
Cc: jan.kiszka, dahi, r65777, bp, Alex Bennée, Catalin Marinas,
Will Deacon, open list
This commit defines the API headers for guest debugging. There are two
architecture specific debug structures:
- kvm_guest_debug_arch, allows us to pass in HW debug registers
- kvm_debug_exit_arch, signals exception and possible faulting address
The type of debugging being used is controlled by the architecture
specific control bits of the kvm_guest_debug->control flags in the ioctl
structure.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
---
v2
- expose hsr and pc directly to user-space
v3
- s/control/controlled/ in commit message
- add v8 to ARM ARM comment (ARM Architecture Reference Manual)
- add rb tag
- rm pc, add far
- re-word comments on alignment
- rename KVM_ARM_NDBG_REGS -> KVM_ARM_MAX_DBG_REGS
diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index d268320..04957d7 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -100,10 +100,28 @@ struct kvm_sregs {
struct kvm_fpu {
};
+/*
+ * See v8 ARM ARM D7.3: Debug Registers
+ *
+ * The control registers are architecturally defined as 32 bits but are
+ * stored as 64 bit values alongside the value registers. This is done
+ * to keep the copying of these values into the vcpu context simple as
+ * everything is 64 bit aligned (see DBGBCR0_EL1 onwards in kvm_asm.h).
+ *
+ * The architectural limit is 16 debug registers of each type although
+ * in practice there are usually less (see ID_AA64DFR0_EL1).
+ */
+#define KVM_ARM_MAX_DBG_REGS 16
struct kvm_guest_debug_arch {
+ __u64 dbg_bcr[KVM_ARM_MAX_DBG_REGS];
+ __u64 dbg_bvr[KVM_ARM_MAX_DBG_REGS];
+ __u64 dbg_wcr[KVM_ARM_MAX_DBG_REGS];
+ __u64 dbg_wvr[KVM_ARM_MAX_DBG_REGS];
};
struct kvm_debug_exit_arch {
+ __u32 hsr;
+ __u64 far;
};
struct kvm_sync_regs {
@@ -216,4 +234,11 @@ struct kvm_arch_memory_slot {
#endif
+/*
+ * Architecture related debug defines - upper 16 bits of
+ * kvm_guest_debug->control
+ */
+#define KVM_GUESTDBG_USE_SW_BP __KVM_GUESTDBG_USE_SW_BP
+#define KVM_GUESTDBG_USE_HW_BP __KVM_GUESTDBG_USE_HW_BP
+
#endif /* __ARM_KVM_H__ */
--
2.3.5
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 04/12] KVM: arm: guest debug, add stub KVM_SET_GUEST_DEBUG ioctl
[not found] <1430929407-3487-1-git-send-email-alex.bennee@linaro.org>
` (2 preceding siblings ...)
2015-05-06 16:23 ` [PATCH v3 03/12] KVM: arm64: guest debug, define API headers Alex Bennée
@ 2015-05-06 16:23 ` Alex Bennée
2015-05-08 11:52 ` Christoffer Dall
2015-05-06 16:23 ` [PATCH v3 05/12] KVM: arm: introduce kvm_arm_init/setup/clear_debug Alex Bennée
` (7 subsequent siblings)
11 siblings, 1 reply; 24+ messages in thread
From: Alex Bennée @ 2015-05-06 16:23 UTC (permalink / raw)
To: kvm, linux-arm-kernel, kvmarm, christoffer.dall, marc.zyngier,
peter.maydell, agraf, drjones, pbonzini, zhichao.huang
Cc: jan.kiszka, dahi, r65777, bp, Alex Bennée, Gleb Natapov,
Jonathan Corbet, Russell King, open list:DOCUMENTATION,
open list
This commit adds a stub function to support the KVM_SET_GUEST_DEBUG
ioctl. Any unsupported flag will return -EINVAL. For now, only
KVM_GUESTDBG_ENABLE is supported, although it won't have any effects.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>.
---
v2
- simplified form of the ioctl (stuff will go into setup_debug)
v3
- KVM_GUESTDBG_VALID->KVM_GUESTDBG_VALID_MASK
- move mask check to the top of function
- add ioctl doc header
- split capability into separate patch
- tweaked commit wording w.r.t return of -EINVAL
diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index cb90025..4b0132f 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -2645,7 +2645,7 @@ handled.
4.87 KVM_SET_GUEST_DEBUG
Capability: KVM_CAP_SET_GUEST_DEBUG
-Architectures: x86, s390, ppc
+Architectures: x86, s390, ppc, arm64
Type: vcpu ioctl
Parameters: struct kvm_guest_debug (in)
Returns: 0 on success; -1 on error
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index d9631ec..52a1d4d38 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -302,10 +302,31 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
kvm_arm_set_running_vcpu(NULL);
}
+#define KVM_GUESTDBG_VALID_MASK (KVM_GUESTDBG_ENABLE)
+
+/**
+ * kvm_arch_vcpu_ioctl_set_guest_debug - set up guest debugging
+ * @kvm: pointer to the KVM struct
+ * @kvm_guest_debug: the ioctl data buffer
+ *
+ * This sets up and enables the VM for guest debugging. Userspace
+ * passes in a control flag to enable different debug types and
+ * potentially other architecture specific information in the rest of
+ * the structure.
+ */
int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
struct kvm_guest_debug *dbg)
{
- return -EINVAL;
+ if (dbg->control & ~KVM_GUESTDBG_VALID_MASK)
+ return -EINVAL;
+
+ if (dbg->control & KVM_GUESTDBG_ENABLE) {
+ vcpu->guest_debug = dbg->control;
+ } else {
+ /* If not enabled clear all flags */
+ vcpu->guest_debug = 0;
+ }
+ return 0;
}
--
2.3.5
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 05/12] KVM: arm: introduce kvm_arm_init/setup/clear_debug
[not found] <1430929407-3487-1-git-send-email-alex.bennee@linaro.org>
` (3 preceding siblings ...)
2015-05-06 16:23 ` [PATCH v3 04/12] KVM: arm: guest debug, add stub KVM_SET_GUEST_DEBUG ioctl Alex Bennée
@ 2015-05-06 16:23 ` Alex Bennée
2015-05-08 11:52 ` Christoffer Dall
2015-05-06 16:23 ` [PATCH v3 06/12] KVM: arm64: guest debug, add SW break point support Alex Bennée
` (6 subsequent siblings)
11 siblings, 1 reply; 24+ messages in thread
From: Alex Bennée @ 2015-05-06 16:23 UTC (permalink / raw)
To: kvm, linux-arm-kernel, kvmarm, christoffer.dall, marc.zyngier,
peter.maydell, agraf, drjones, pbonzini, zhichao.huang
Cc: jan.kiszka, dahi, r65777, bp, Alex Bennée, Gleb Natapov,
Russell King, Catalin Marinas, Will Deacon, Ard Biesheuvel,
Richard Weinberger, Andre Przywara, Lorenzo Pieralisi, open list
This is a precursor for later patches which will need to do more to
setup debug state before entering the hyp.S switch code. The existing
functionality for setting mdcr_el2 has been moved out of hyp.S and now
uses the value kept in vcpu->arch.mdcr_el2.
As the assembler used to previously mask and preserve MDCR_EL2.HPMN I've
had to add a mechanism to save the value of mdcr_el2 as a per-cpu
variable during the initialisation code. The kernel never sets this
number so we are assuming the bootcode has set up the correct value
here.
This also moves the conditional setting of the TDA bit from the hyp code
into the C code which is currently used for the lazy debug register
context switch code.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
v3
- rename fns from arch->arm
- preserve MDCR_EL2.HPMN setting
- re-word some of the comments
- fix some minor grammar nits
- merge setting of mdcr_el2
- introduce trap_debug flag
- move setup/clear within the irq lock section
create mode 100644 arch/arm64/kvm/debug.c
diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index d71607c..746c0c69 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -236,4 +236,8 @@ static inline void kvm_arch_sync_events(struct kvm *kvm) {}
static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
static inline void kvm_arch_sched_in(struct kvm_vcpu *vcpu, int cpu) {}
+static inline void kvm_arm_init_debug(void) {}
+static inline void kvm_arm_setup_debug(struct kvm_vcpu *vcpu) {}
+static inline void kvm_arm_clear_debug(struct kvm_vcpu *vcpu) {}
+
#endif /* __ARM_KVM_HOST_H__ */
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 52a1d4d38..4a274e1 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -570,6 +570,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
continue;
}
+ kvm_arm_setup_debug(vcpu);
+
/**************************************************************
* Enter the guest
*/
@@ -582,7 +584,10 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
vcpu->mode = OUTSIDE_GUEST_MODE;
kvm_guest_exit();
trace_kvm_exit(kvm_vcpu_trap_get_class(vcpu), *vcpu_pc(vcpu));
- /*
+
+ kvm_arm_clear_debug(vcpu);
+
+ /*
* We may have taken a host interrupt in HYP mode (ie
* while executing the guest). This interrupt is still
* pending, as we haven't serviced it yet!
@@ -930,6 +935,8 @@ static void cpu_init_hyp_mode(void *dummy)
vector_ptr = (unsigned long)__kvm_hyp_vector;
__cpu_init_hyp_mode(boot_pgd_ptr, pgd_ptr, hyp_stack_ptr, vector_ptr);
+
+ kvm_arm_init_debug();
}
static int hyp_init_cpu_notify(struct notifier_block *self,
diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
index 4f7310f..d6b507e 100644
--- a/arch/arm64/include/asm/kvm_asm.h
+++ b/arch/arm64/include/asm/kvm_asm.h
@@ -137,6 +137,8 @@ extern char __restore_vgic_v2_state[];
extern char __save_vgic_v3_state[];
extern char __restore_vgic_v3_state[];
+extern u32 __kvm_get_mdcr_el2(void);
+
#endif
#endif /* __ARM_KVM_ASM_H__ */
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index f0f58c9..7cb99b5 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -103,6 +103,7 @@ struct kvm_vcpu_arch {
/* HYP configuration */
u64 hcr_el2;
+ u32 mdcr_el2;
/* Exception Information */
struct kvm_vcpu_fault_info fault;
@@ -250,4 +251,8 @@ static inline void kvm_arch_sync_events(struct kvm *kvm) {}
static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
static inline void kvm_arch_sched_in(struct kvm_vcpu *vcpu, int cpu) {}
+void kvm_arm_init_debug(void);
+void kvm_arm_setup_debug(struct kvm_vcpu *vcpu);
+void kvm_arm_clear_debug(struct kvm_vcpu *vcpu);
+
#endif /* __ARM64_KVM_HOST_H__ */
diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index da675cc..dfb25a2 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -117,6 +117,7 @@ int main(void)
DEFINE(VCPU_HPFAR_EL2, offsetof(struct kvm_vcpu, arch.fault.hpfar_el2));
DEFINE(VCPU_DEBUG_FLAGS, offsetof(struct kvm_vcpu, arch.debug_flags));
DEFINE(VCPU_HCR_EL2, offsetof(struct kvm_vcpu, arch.hcr_el2));
+ DEFINE(VCPU_MDCR_EL2, offsetof(struct kvm_vcpu, arch.mdcr_el2));
DEFINE(VCPU_IRQ_LINES, offsetof(struct kvm_vcpu, arch.irq_lines));
DEFINE(VCPU_HOST_CONTEXT, offsetof(struct kvm_vcpu, arch.host_cpu_context));
DEFINE(VCPU_TIMER_CNTV_CTL, offsetof(struct kvm_vcpu, arch.timer_cpu.cntv_ctl));
diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
index d5904f8..90e3f39 100644
--- a/arch/arm64/kvm/Makefile
+++ b/arch/arm64/kvm/Makefile
@@ -17,7 +17,7 @@ kvm-$(CONFIG_KVM_ARM_HOST) += $(ARM)/psci.o $(ARM)/perf.o
kvm-$(CONFIG_KVM_ARM_HOST) += emulate.o inject_fault.o regmap.o
kvm-$(CONFIG_KVM_ARM_HOST) += hyp.o hyp-init.o handle_exit.o
-kvm-$(CONFIG_KVM_ARM_HOST) += guest.o reset.o sys_regs.o sys_regs_generic_v8.o
+kvm-$(CONFIG_KVM_ARM_HOST) += guest.o debug.o reset.o sys_regs.o sys_regs_generic_v8.o
kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic.o
kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic-v2.o
diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
new file mode 100644
index 0000000..b1f8731
--- /dev/null
+++ b/arch/arm64/kvm/debug.c
@@ -0,0 +1,83 @@
+/*
+ * Debug and Guest Debug support
+ *
+ * Copyright (C) 2015 - Linaro Ltd
+ * Author: Alex Bennée <alex.bennee@linaro.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/kvm_host.h>
+
+#include <asm/kvm_arm.h>
+
+static DEFINE_PER_CPU(u32, mdcr_el2);
+
+/**
+ * kvm_arm_init_debug - grab what we need for debug
+ *
+ * Currently the sole task of this function is to retrieve the initial
+ * value of mdcr_el2 so we can preserve MDCR_EL2.HPMN which has
+ * presumably been set-up by some knowledgeable bootcode.
+ *
+ * It is called once per-cpu during CPU hyp initialisation.
+ */
+
+void kvm_arm_init_debug(void)
+{
+ __this_cpu_write(mdcr_el2, kvm_call_hyp(__kvm_get_mdcr_el2));
+}
+
+
+/**
+ * kvm_arm_setup_debug - set up debug related stuff
+ *
+ * @vcpu: the vcpu pointer
+ *
+ * This is called before each entry into the hypervisor to setup any
+ * debug related registers. Currently this just ensures we will trap
+ * access to:
+ * - Performance monitors (MDCR_EL2_TPM/MDCR_EL2_TPMCR)
+ * - Debug ROM Address (MDCR_EL2_TDRA)
+ * - Power down debug registers (MDCR_EL2_TDOSA)
+ *
+ * Additionally, KVM only traps guest accesses to the debug registers if
+ * the guest is not actively using them (see the KVM_ARM64_DEBUG_DIRTY
+ * flag on vcpu->arch.debug_flags). Since the guest must not interfere
+ * with the hardware state when debugging the guest, we must ensure that
+ * trapping is enabled whenever we are debugging the guest using the
+ * debug registers.
+ */
+
+void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
+{
+ bool trap_debug = !(vcpu->arch.debug_flags & KVM_ARM64_DEBUG_DIRTY);
+
+ vcpu->arch.mdcr_el2 = __this_cpu_read(mdcr_el2) & MDCR_EL2_HPMN_MASK;
+ vcpu->arch.mdcr_el2 |= (MDCR_EL2_TPM |
+ MDCR_EL2_TPMCR |
+ MDCR_EL2_TDRA |
+ MDCR_EL2_TDOSA);
+
+ /* Trap on access to debug registers? */
+ if (trap_debug)
+ vcpu->arch.mdcr_el2 |= MDCR_EL2_TDA;
+ else
+ vcpu->arch.mdcr_el2 &= ~MDCR_EL2_TDA;
+
+}
+
+void kvm_arm_clear_debug(struct kvm_vcpu *vcpu)
+{
+ /* Nothing to do yet */
+}
diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
index 5befd01..15159aa 100644
--- a/arch/arm64/kvm/hyp.S
+++ b/arch/arm64/kvm/hyp.S
@@ -768,17 +768,8 @@
mov x2, #(1 << 15) // Trap CP15 Cr=15
msr hstr_el2, x2
- mrs x2, mdcr_el2
- and x2, x2, #MDCR_EL2_HPMN_MASK
- orr x2, x2, #(MDCR_EL2_TPM | MDCR_EL2_TPMCR)
- orr x2, x2, #(MDCR_EL2_TDRA | MDCR_EL2_TDOSA)
-
- // Check for KVM_ARM64_DEBUG_DIRTY, and set debug to trap
- // if not dirty.
- ldr x3, [x0, #VCPU_DEBUG_FLAGS]
- tbnz x3, #KVM_ARM64_DEBUG_DIRTY_SHIFT, 1f
- orr x2, x2, #MDCR_EL2_TDA
-1:
+ // Monitor Debug Config - see kvm_arch_setup_debug()
+ ldr x2, [x0, #VCPU_MDCR_EL2]
msr mdcr_el2, x2
.endm
@@ -1295,4 +1286,10 @@ ENTRY(__kvm_hyp_vector)
ventry el1_error_invalid // Error 32-bit EL1
ENDPROC(__kvm_hyp_vector)
+
+ENTRY(__kvm_get_mdcr_el2)
+ mrs x0, mdcr_el2
+ ret
+ENDPROC(__kvm_get_mdcr_el2)
+
.popsection
--
2.3.5
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 06/12] KVM: arm64: guest debug, add SW break point support
[not found] <1430929407-3487-1-git-send-email-alex.bennee@linaro.org>
` (4 preceding siblings ...)
2015-05-06 16:23 ` [PATCH v3 05/12] KVM: arm: introduce kvm_arm_init/setup/clear_debug Alex Bennée
@ 2015-05-06 16:23 ` Alex Bennée
2015-05-08 11:52 ` Christoffer Dall
2015-05-06 16:23 ` [PATCH v3 07/12] KVM: arm64: guest debug, add support for single-step Alex Bennée
` (5 subsequent siblings)
11 siblings, 1 reply; 24+ messages in thread
From: Alex Bennée @ 2015-05-06 16:23 UTC (permalink / raw)
To: kvm, linux-arm-kernel, kvmarm, christoffer.dall, marc.zyngier,
peter.maydell, agraf, drjones, pbonzini, zhichao.huang
Cc: jan.kiszka, dahi, r65777, bp, Alex Bennée, Gleb Natapov,
Jonathan Corbet, Russell King, Catalin Marinas, Will Deacon,
open list:DOCUMENTATION, open list
This adds support for SW breakpoints inserted by userspace.
We do this by trapping all guest software debug exceptions to the
hypervisor (MDCR_EL2.TDE). The exit handler sets an exit reason of
KVM_EXIT_DEBUG with the kvm_debug_exit_arch structure holding the
exception syndrome information.
It will be up to userspace to extract the PC (via GET_ONE_REG) and
determine if the debug event was for a breakpoint it inserted. If not
userspace will need to re-inject the correct exception restart the
hypervisor to deliver the debug exception to the guest.
Any other guest software debug exception (e.g. single step or HW
assisted breakpoints) will cause an error and the VM to be killed. This
is addressed by later patches which add support for the other debug
types.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
v2
- update to use new exit struct
- tweak for C setup
- do our setup in debug_setup/clear code
- fixed up comments
v3:
- fix spacing in KVM_GUESTDBG_VALID_MASK
- fix and clarify wording on kvm_handle_guest_debug
- handle error case in kvm_handle_guest_debug
- re-word the commit message
diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 4b0132f..5ef937c 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -2667,7 +2667,7 @@ when running. Common control bits are:
The top 16 bits of the control field are architecture specific control
flags which can include the following:
- - KVM_GUESTDBG_USE_SW_BP: using software breakpoints [x86]
+ - KVM_GUESTDBG_USE_SW_BP: using software breakpoints [x86, arm64]
- KVM_GUESTDBG_USE_HW_BP: using hardware breakpoints [x86, s390]
- KVM_GUESTDBG_INJECT_DB: inject DB type exception [x86]
- KVM_GUESTDBG_INJECT_BP: inject BP type exception [x86]
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 4a274e1..064c105 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -302,7 +302,7 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
kvm_arm_set_running_vcpu(NULL);
}
-#define KVM_GUESTDBG_VALID_MASK (KVM_GUESTDBG_ENABLE)
+#define KVM_GUESTDBG_VALID_MASK (KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP)
/**
* kvm_arch_vcpu_ioctl_set_guest_debug - set up guest debugging
diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
index b1f8731..5bee676 100644
--- a/arch/arm64/kvm/debug.c
+++ b/arch/arm64/kvm/debug.c
@@ -75,6 +75,12 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
else
vcpu->arch.mdcr_el2 &= ~MDCR_EL2_TDA;
+ /* Trap breakpoints? */
+ if (vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP)
+ vcpu->arch.mdcr_el2 |= MDCR_EL2_TDE;
+ else
+ vcpu->arch.mdcr_el2 &= ~MDCR_EL2_TDE;
+
}
void kvm_arm_clear_debug(struct kvm_vcpu *vcpu)
diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index 524fa25..27f38a9 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -82,6 +82,40 @@ static int kvm_handle_wfx(struct kvm_vcpu *vcpu, struct kvm_run *run)
return 1;
}
+/**
+ * kvm_handle_guest_debug - handle a debug exception instruction
+ *
+ * @vcpu: the vcpu pointer
+ * @run: access to the kvm_run structure for results
+ *
+ * We route all debug exceptions through the same handler. If both the
+ * guest and host are using the same debug facilities it will be up to
+ * userspace to re-inject the correct exception for guest delivery.
+ *
+ * @return: 0 (while setting run->exit_reason), -1 for error
+ */
+static int kvm_handle_guest_debug(struct kvm_vcpu *vcpu, struct kvm_run *run)
+{
+ u32 hsr = kvm_vcpu_get_hsr(vcpu);
+ int ret = 0;
+
+ run->exit_reason = KVM_EXIT_DEBUG;
+ run->debug.arch.hsr = hsr;
+
+ switch (hsr >> ESR_ELx_EC_SHIFT) {
+ case ESR_ELx_EC_BKPT32:
+ case ESR_ELx_EC_BRK64:
+ break;
+ default:
+ kvm_err("%s: un-handled case hsr: %#08x\n",
+ __func__, (unsigned int) hsr);
+ ret = -1;
+ break;
+ }
+
+ return ret;
+}
+
static exit_handle_fn arm_exit_handlers[] = {
[ESR_ELx_EC_WFx] = kvm_handle_wfx,
[ESR_ELx_EC_CP15_32] = kvm_handle_cp15_32,
@@ -96,6 +130,8 @@ static exit_handle_fn arm_exit_handlers[] = {
[ESR_ELx_EC_SYS64] = kvm_handle_sys_reg,
[ESR_ELx_EC_IABT_LOW] = kvm_handle_guest_abort,
[ESR_ELx_EC_DABT_LOW] = kvm_handle_guest_abort,
+ [ESR_ELx_EC_BKPT32] = kvm_handle_guest_debug,
+ [ESR_ELx_EC_BRK64] = kvm_handle_guest_debug,
};
static exit_handle_fn kvm_get_exit_handler(struct kvm_vcpu *vcpu)
--
2.3.5
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 07/12] KVM: arm64: guest debug, add support for single-step
[not found] <1430929407-3487-1-git-send-email-alex.bennee@linaro.org>
` (5 preceding siblings ...)
2015-05-06 16:23 ` [PATCH v3 06/12] KVM: arm64: guest debug, add SW break point support Alex Bennée
@ 2015-05-06 16:23 ` Alex Bennée
2015-05-08 11:52 ` Christoffer Dall
2015-05-07 9:07 ` [PATCH v3 08/12] KVM: arm64: re-factor hyp.S debug register code Alex Bennée
` (4 subsequent siblings)
11 siblings, 1 reply; 24+ messages in thread
From: Alex Bennée @ 2015-05-06 16:23 UTC (permalink / raw)
To: kvm, linux-arm-kernel, kvmarm, christoffer.dall, marc.zyngier,
peter.maydell, agraf, drjones, pbonzini, zhichao.huang
Cc: jan.kiszka, dahi, r65777, bp, Alex Bennée, Gleb Natapov,
Russell King, Catalin Marinas, Will Deacon, open list
This adds support for single-stepping the guest. To do this we need to
manipulate the guests PSTATE.SS and MDSCR_EL1.SS bits which we do in the
kvm_arm_setup/clear_debug() so we don't affect the apparent state of the
guest. Additionally while the host is debugging the guest we suppress
the ability of the guest to single-step itself.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
v2
- Move pstate/mdscr manipulation into C
- don't export guest_debug to assembly
- add accessor for saved_debug regs
- tweak save/restore of mdscr_el1
v3
- don't save PC in debug information struct
- rename debug_saved_regs->guest_debug_state
- save whole value, only use bits in restore
- add save/restore_guest-debug_regs helper functions
- simplify commit message for clarity
- rm vcpu_debug_saved_reg access fn
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 064c105..9b3ed6d 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -302,7 +302,9 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
kvm_arm_set_running_vcpu(NULL);
}
-#define KVM_GUESTDBG_VALID_MASK (KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP)
+#define KVM_GUESTDBG_VALID_MASK (KVM_GUESTDBG_ENABLE | \
+ KVM_GUESTDBG_USE_SW_BP | \
+ KVM_GUESTDBG_SINGLESTEP)
/**
* kvm_arch_vcpu_ioctl_set_guest_debug - set up guest debugging
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 7cb99b5..b60fa7a 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -123,6 +123,12 @@ struct kvm_vcpu_arch {
* here.
*/
+ /* Guest registers we preserve during guest debugging */
+ struct {
+ u32 pstate;
+ u32 mdscr_el1;
+ } guest_debug_state;
+
/* Don't run the guest */
bool pause;
diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
index 5bee676..19346e8 100644
--- a/arch/arm64/kvm/debug.c
+++ b/arch/arm64/kvm/debug.c
@@ -19,11 +19,42 @@
#include <linux/kvm_host.h>
+#include <asm/debug-monitors.h>
+#include <asm/kvm_asm.h>
#include <asm/kvm_arm.h>
+#include <asm/kvm_emulate.h>
+
+/* These are the bits of MDSCR_EL1 we may manipulate */
+#define MDSCR_EL1_DEBUG_MASK (DBG_MDSCR_SS | \
+ DBG_MDSCR_KDE | \
+ DBG_MDSCR_MDE)
+
+#define SPSR_DEBUG_MASK DBG_SPSR_SS
static DEFINE_PER_CPU(u32, mdcr_el2);
/**
+ * save/restore_guest_debug_regs
+ *
+ * For some debug operations we need to tweak some guest registers. As
+ * a result we need to save the state of those registers before we
+ * make those modifications.
+ */
+static void save_guest_debug_regs(struct kvm_vcpu *vcpu)
+{
+ vcpu->arch.guest_debug_state.pstate = *vcpu_cpsr(vcpu);
+ vcpu->arch.guest_debug_state.mdscr_el1 = vcpu_sys_reg(vcpu, MDSCR_EL1);
+}
+
+static void restore_guest_debug_regs(struct kvm_vcpu *vcpu)
+{
+ *vcpu_cpsr(vcpu) |=
+ (vcpu->arch.guest_debug_state.pstate & SPSR_DEBUG_MASK);
+ vcpu_sys_reg(vcpu, MDSCR_EL1) |=
+ (vcpu->arch.guest_debug_state.mdscr_el1 & MDSCR_EL1_DEBUG_MASK);
+}
+
+/**
* kvm_arm_init_debug - grab what we need for debug
*
* Currently the sole task of this function is to retrieve the initial
@@ -38,7 +69,6 @@ void kvm_arm_init_debug(void)
__this_cpu_write(mdcr_el2, kvm_call_hyp(__kvm_get_mdcr_el2));
}
-
/**
* kvm_arm_setup_debug - set up debug related stuff
*
@@ -75,15 +105,37 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
else
vcpu->arch.mdcr_el2 &= ~MDCR_EL2_TDA;
- /* Trap breakpoints? */
- if (vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP)
+ /* Is Guest debugging in effect? */
+ if (vcpu->guest_debug) {
vcpu->arch.mdcr_el2 |= MDCR_EL2_TDE;
- else
- vcpu->arch.mdcr_el2 &= ~MDCR_EL2_TDE;
+ /* Save guest debug state */
+ save_guest_debug_regs(vcpu);
+
+ /*
+ * Single Step (ARM ARM D2.12.3 The software step state
+ * machine)
+ *
+ * If we are doing Single Step we need to manipulate
+ * MDSCR_EL1.SS and PSTATE.SS. If not we need to
+ * suppress the guests ability to trigger single step itself.
+ */
+ if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) {
+ *vcpu_cpsr(vcpu) |= DBG_SPSR_SS;
+ vcpu_sys_reg(vcpu, MDSCR_EL1) |= DBG_MDSCR_SS;
+ } else {
+ *vcpu_cpsr(vcpu) &= ~DBG_SPSR_SS;
+ vcpu_sys_reg(vcpu, MDSCR_EL1) &= ~DBG_MDSCR_SS;
+ }
+
+ } else {
+ /* Debug operations can go straight to the guest */
+ vcpu->arch.mdcr_el2 &= ~MDCR_EL2_TDE;
+ }
}
void kvm_arm_clear_debug(struct kvm_vcpu *vcpu)
{
- /* Nothing to do yet */
+ if (vcpu->guest_debug)
+ restore_guest_debug_regs(vcpu);
}
diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index 27f38a9..e9de13e 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -103,6 +103,7 @@ static int kvm_handle_guest_debug(struct kvm_vcpu *vcpu, struct kvm_run *run)
run->debug.arch.hsr = hsr;
switch (hsr >> ESR_ELx_EC_SHIFT) {
+ case ESR_ELx_EC_SOFTSTP_LOW:
case ESR_ELx_EC_BKPT32:
case ESR_ELx_EC_BRK64:
break;
@@ -130,6 +131,7 @@ static exit_handle_fn arm_exit_handlers[] = {
[ESR_ELx_EC_SYS64] = kvm_handle_sys_reg,
[ESR_ELx_EC_IABT_LOW] = kvm_handle_guest_abort,
[ESR_ELx_EC_DABT_LOW] = kvm_handle_guest_abort,
+ [ESR_ELx_EC_SOFTSTP_LOW]= kvm_handle_guest_debug,
[ESR_ELx_EC_BKPT32] = kvm_handle_guest_debug,
[ESR_ELx_EC_BRK64] = kvm_handle_guest_debug,
};
--
2.3.5
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 08/12] KVM: arm64: re-factor hyp.S debug register code
[not found] <1430929407-3487-1-git-send-email-alex.bennee@linaro.org>
` (6 preceding siblings ...)
2015-05-06 16:23 ` [PATCH v3 07/12] KVM: arm64: guest debug, add support for single-step Alex Bennée
@ 2015-05-07 9:07 ` Alex Bennée
2015-05-08 14:12 ` Christoffer Dall
2015-05-07 9:07 ` [PATCH v3 09/12] KVM: arm64: guest debug, HW assisted debug support Alex Bennée
` (3 subsequent siblings)
11 siblings, 1 reply; 24+ messages in thread
From: Alex Bennée @ 2015-05-07 9:07 UTC (permalink / raw)
To: kvm, linux-arm-kernel, kvmarm, christoffer.dall, marc.zyngier,
peter.maydell, agraf, drjones, pbonzini, zhichao.huang
Cc: jan.kiszka, dahi, r65777, bp, Alex Bennée, Catalin Marinas,
Will Deacon, Gleb Natapov, Ard Biesheuvel, Andre Przywara,
Richard Weinberger, Lorenzo Pieralisi, open list
This is a pre-cursor to sharing the code with the guest debug support.
This replaces the big macro that fishes data out of a fixed location
with a more general helper macro to restore a set of debug registers. It
uses macro substitution so it can be re-used for debug control and value
registers. It does however rely on the debug registers being 64 bit
aligned (as they happen to be in the hyp ABI).
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
v3:
- return to the patch series
- add save and restore targets
- change register use and document
diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index dfb25a2..ce7b7dd 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -116,6 +116,10 @@ int main(void)
DEFINE(VCPU_FAR_EL2, offsetof(struct kvm_vcpu, arch.fault.far_el2));
DEFINE(VCPU_HPFAR_EL2, offsetof(struct kvm_vcpu, arch.fault.hpfar_el2));
DEFINE(VCPU_DEBUG_FLAGS, offsetof(struct kvm_vcpu, arch.debug_flags));
+ DEFINE(DEBUG_BCR, offsetof(struct kvm_guest_debug_arch, dbg_bcr));
+ DEFINE(DEBUG_BVR, offsetof(struct kvm_guest_debug_arch, dbg_bvr));
+ DEFINE(DEBUG_WCR, offsetof(struct kvm_guest_debug_arch, dbg_wcr));
+ DEFINE(DEBUG_WVR, offsetof(struct kvm_guest_debug_arch, dbg_wvr));
DEFINE(VCPU_HCR_EL2, offsetof(struct kvm_vcpu, arch.hcr_el2));
DEFINE(VCPU_MDCR_EL2, offsetof(struct kvm_vcpu, arch.mdcr_el2));
DEFINE(VCPU_IRQ_LINES, offsetof(struct kvm_vcpu, arch.irq_lines));
diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
index 15159aa..dd51fb1 100644
--- a/arch/arm64/kvm/hyp.S
+++ b/arch/arm64/kvm/hyp.S
@@ -228,199 +228,52 @@
stp x24, x25, [x3, #160]
.endm
-.macro save_debug
- // x2: base address for cpu context
- // x3: tmp register
-
- mrs x26, id_aa64dfr0_el1
- ubfx x24, x26, #12, #4 // Extract BRPs
- ubfx x25, x26, #20, #4 // Extract WRPs
- mov w26, #15
- sub w24, w26, w24 // How many BPs to skip
- sub w25, w26, w25 // How many WPs to skip
-
- add x3, x2, #CPU_SYSREG_OFFSET(DBGBCR0_EL1)
-
- adr x26, 1f
- add x26, x26, x24, lsl #2
- br x26
-1:
- mrs x20, dbgbcr15_el1
- mrs x19, dbgbcr14_el1
- mrs x18, dbgbcr13_el1
- mrs x17, dbgbcr12_el1
- mrs x16, dbgbcr11_el1
- mrs x15, dbgbcr10_el1
- mrs x14, dbgbcr9_el1
- mrs x13, dbgbcr8_el1
- mrs x12, dbgbcr7_el1
- mrs x11, dbgbcr6_el1
- mrs x10, dbgbcr5_el1
- mrs x9, dbgbcr4_el1
- mrs x8, dbgbcr3_el1
- mrs x7, dbgbcr2_el1
- mrs x6, dbgbcr1_el1
- mrs x5, dbgbcr0_el1
-
- adr x26, 1f
- add x26, x26, x24, lsl #2
- br x26
-
-1:
- str x20, [x3, #(15 * 8)]
- str x19, [x3, #(14 * 8)]
- str x18, [x3, #(13 * 8)]
- str x17, [x3, #(12 * 8)]
- str x16, [x3, #(11 * 8)]
- str x15, [x3, #(10 * 8)]
- str x14, [x3, #(9 * 8)]
- str x13, [x3, #(8 * 8)]
- str x12, [x3, #(7 * 8)]
- str x11, [x3, #(6 * 8)]
- str x10, [x3, #(5 * 8)]
- str x9, [x3, #(4 * 8)]
- str x8, [x3, #(3 * 8)]
- str x7, [x3, #(2 * 8)]
- str x6, [x3, #(1 * 8)]
- str x5, [x3, #(0 * 8)]
-
- add x3, x2, #CPU_SYSREG_OFFSET(DBGBVR0_EL1)
-
- adr x26, 1f
- add x26, x26, x24, lsl #2
- br x26
+.macro save_debug_registers type
+ // x4: pointer to register set
+ // x5: number of registers to copy
+ // x6..x22 trashed
+
+ adr x22, 1f
+ add x22, x22, x5, lsl #2
+ br x22
1:
- mrs x20, dbgbvr15_el1
- mrs x19, dbgbvr14_el1
- mrs x18, dbgbvr13_el1
- mrs x17, dbgbvr12_el1
- mrs x16, dbgbvr11_el1
- mrs x15, dbgbvr10_el1
- mrs x14, dbgbvr9_el1
- mrs x13, dbgbvr8_el1
- mrs x12, dbgbvr7_el1
- mrs x11, dbgbvr6_el1
- mrs x10, dbgbvr5_el1
- mrs x9, dbgbvr4_el1
- mrs x8, dbgbvr3_el1
- mrs x7, dbgbvr2_el1
- mrs x6, dbgbvr1_el1
- mrs x5, dbgbvr0_el1
-
- adr x26, 1f
- add x26, x26, x24, lsl #2
- br x26
-
-1:
- str x20, [x3, #(15 * 8)]
- str x19, [x3, #(14 * 8)]
- str x18, [x3, #(13 * 8)]
- str x17, [x3, #(12 * 8)]
- str x16, [x3, #(11 * 8)]
- str x15, [x3, #(10 * 8)]
- str x14, [x3, #(9 * 8)]
- str x13, [x3, #(8 * 8)]
- str x12, [x3, #(7 * 8)]
- str x11, [x3, #(6 * 8)]
- str x10, [x3, #(5 * 8)]
- str x9, [x3, #(4 * 8)]
- str x8, [x3, #(3 * 8)]
- str x7, [x3, #(2 * 8)]
- str x6, [x3, #(1 * 8)]
- str x5, [x3, #(0 * 8)]
-
- add x3, x2, #CPU_SYSREG_OFFSET(DBGWCR0_EL1)
-
- adr x26, 1f
- add x26, x26, x25, lsl #2
- br x26
-1:
- mrs x20, dbgwcr15_el1
- mrs x19, dbgwcr14_el1
- mrs x18, dbgwcr13_el1
- mrs x17, dbgwcr12_el1
- mrs x16, dbgwcr11_el1
- mrs x15, dbgwcr10_el1
- mrs x14, dbgwcr9_el1
- mrs x13, dbgwcr8_el1
- mrs x12, dbgwcr7_el1
- mrs x11, dbgwcr6_el1
- mrs x10, dbgwcr5_el1
- mrs x9, dbgwcr4_el1
- mrs x8, dbgwcr3_el1
- mrs x7, dbgwcr2_el1
- mrs x6, dbgwcr1_el1
- mrs x5, dbgwcr0_el1
-
- adr x26, 1f
- add x26, x26, x25, lsl #2
- br x26
-
+ mrs x21, \type\()15_el1
+ mrs x20, \type\()14_el1
+ mrs x19, \type\()13_el1
+ mrs x18, \type\()12_el1
+ mrs x17, \type\()11_el1
+ mrs x16, \type\()10_el1
+ mrs x15, \type\()9_el1
+ mrs x14, \type\()8_el1
+ mrs x13, \type\()7_el1
+ mrs x12, \type\()6_el1
+ mrs x11, \type\()5_el1
+ mrs x10, \type\()4_el1
+ mrs x9, \type\()3_el1
+ mrs x8, \type\()2_el1
+ mrs x7, \type\()1_el1
+ mrs x6, \type\()0_el1
+
+ adr x22, 1f
+ add x22, x22, x5, lsl #2
+ br x22
1:
- str x20, [x3, #(15 * 8)]
- str x19, [x3, #(14 * 8)]
- str x18, [x3, #(13 * 8)]
- str x17, [x3, #(12 * 8)]
- str x16, [x3, #(11 * 8)]
- str x15, [x3, #(10 * 8)]
- str x14, [x3, #(9 * 8)]
- str x13, [x3, #(8 * 8)]
- str x12, [x3, #(7 * 8)]
- str x11, [x3, #(6 * 8)]
- str x10, [x3, #(5 * 8)]
- str x9, [x3, #(4 * 8)]
- str x8, [x3, #(3 * 8)]
- str x7, [x3, #(2 * 8)]
- str x6, [x3, #(1 * 8)]
- str x5, [x3, #(0 * 8)]
-
- add x3, x2, #CPU_SYSREG_OFFSET(DBGWVR0_EL1)
-
- adr x26, 1f
- add x26, x26, x25, lsl #2
- br x26
-1:
- mrs x20, dbgwvr15_el1
- mrs x19, dbgwvr14_el1
- mrs x18, dbgwvr13_el1
- mrs x17, dbgwvr12_el1
- mrs x16, dbgwvr11_el1
- mrs x15, dbgwvr10_el1
- mrs x14, dbgwvr9_el1
- mrs x13, dbgwvr8_el1
- mrs x12, dbgwvr7_el1
- mrs x11, dbgwvr6_el1
- mrs x10, dbgwvr5_el1
- mrs x9, dbgwvr4_el1
- mrs x8, dbgwvr3_el1
- mrs x7, dbgwvr2_el1
- mrs x6, dbgwvr1_el1
- mrs x5, dbgwvr0_el1
-
- adr x26, 1f
- add x26, x26, x25, lsl #2
- br x26
-
-1:
- str x20, [x3, #(15 * 8)]
- str x19, [x3, #(14 * 8)]
- str x18, [x3, #(13 * 8)]
- str x17, [x3, #(12 * 8)]
- str x16, [x3, #(11 * 8)]
- str x15, [x3, #(10 * 8)]
- str x14, [x3, #(9 * 8)]
- str x13, [x3, #(8 * 8)]
- str x12, [x3, #(7 * 8)]
- str x11, [x3, #(6 * 8)]
- str x10, [x3, #(5 * 8)]
- str x9, [x3, #(4 * 8)]
- str x8, [x3, #(3 * 8)]
- str x7, [x3, #(2 * 8)]
- str x6, [x3, #(1 * 8)]
- str x5, [x3, #(0 * 8)]
-
- mrs x21, mdccint_el1
- str x21, [x2, #CPU_SYSREG_OFFSET(MDCCINT_EL1)]
+ str x21, [x4, #(15 * 8)]
+ str x20, [x4, #(14 * 8)]
+ str x19, [x4, #(13 * 8)]
+ str x18, [x4, #(12 * 8)]
+ str x17, [x4, #(11 * 8)]
+ str x16, [x4, #(10 * 8)]
+ str x15, [x4, #(9 * 8)]
+ str x14, [x4, #(8 * 8)]
+ str x13, [x4, #(7 * 8)]
+ str x12, [x4, #(6 * 8)]
+ str x11, [x4, #(5 * 8)]
+ str x10, [x4, #(4 * 8)]
+ str x9, [x4, #(3 * 8)]
+ str x8, [x4, #(2 * 8)]
+ str x7, [x4, #(1 * 8)]
+ str x6, [x4, #(0 * 8)]
.endm
.macro restore_sysregs
@@ -465,195 +318,52 @@
msr mdscr_el1, x25
.endm
-.macro restore_debug
- // x2: base address for cpu context
- // x3: tmp register
-
- mrs x26, id_aa64dfr0_el1
- ubfx x24, x26, #12, #4 // Extract BRPs
- ubfx x25, x26, #20, #4 // Extract WRPs
- mov w26, #15
- sub w24, w26, w24 // How many BPs to skip
- sub w25, w26, w25 // How many WPs to skip
-
- add x3, x2, #CPU_SYSREG_OFFSET(DBGBCR0_EL1)
+.macro setup_debug_registers type
+ // x4: pointer to register set
+ // x5: number of registers to copy
+ // x6..x22 trashed
- adr x26, 1f
- add x26, x26, x24, lsl #2
- br x26
-1:
- ldr x20, [x3, #(15 * 8)]
- ldr x19, [x3, #(14 * 8)]
- ldr x18, [x3, #(13 * 8)]
- ldr x17, [x3, #(12 * 8)]
- ldr x16, [x3, #(11 * 8)]
- ldr x15, [x3, #(10 * 8)]
- ldr x14, [x3, #(9 * 8)]
- ldr x13, [x3, #(8 * 8)]
- ldr x12, [x3, #(7 * 8)]
- ldr x11, [x3, #(6 * 8)]
- ldr x10, [x3, #(5 * 8)]
- ldr x9, [x3, #(4 * 8)]
- ldr x8, [x3, #(3 * 8)]
- ldr x7, [x3, #(2 * 8)]
- ldr x6, [x3, #(1 * 8)]
- ldr x5, [x3, #(0 * 8)]
-
- adr x26, 1f
- add x26, x26, x24, lsl #2
- br x26
+ adr x22, 1f
+ add x22, x22, x5, lsl #2
+ br x22
1:
- msr dbgbcr15_el1, x20
- msr dbgbcr14_el1, x19
- msr dbgbcr13_el1, x18
- msr dbgbcr12_el1, x17
- msr dbgbcr11_el1, x16
- msr dbgbcr10_el1, x15
- msr dbgbcr9_el1, x14
- msr dbgbcr8_el1, x13
- msr dbgbcr7_el1, x12
- msr dbgbcr6_el1, x11
- msr dbgbcr5_el1, x10
- msr dbgbcr4_el1, x9
- msr dbgbcr3_el1, x8
- msr dbgbcr2_el1, x7
- msr dbgbcr1_el1, x6
- msr dbgbcr0_el1, x5
-
- add x3, x2, #CPU_SYSREG_OFFSET(DBGBVR0_EL1)
-
- adr x26, 1f
- add x26, x26, x24, lsl #2
- br x26
+ ldr x21, [x4, #(15 * 8)]
+ ldr x20, [x4, #(14 * 8)]
+ ldr x19, [x4, #(13 * 8)]
+ ldr x18, [x4, #(12 * 8)]
+ ldr x17, [x4, #(11 * 8)]
+ ldr x16, [x4, #(10 * 8)]
+ ldr x15, [x4, #(9 * 8)]
+ ldr x14, [x4, #(8 * 8)]
+ ldr x13, [x4, #(7 * 8)]
+ ldr x12, [x4, #(6 * 8)]
+ ldr x11, [x4, #(5 * 8)]
+ ldr x10, [x4, #(4 * 8)]
+ ldr x9, [x4, #(3 * 8)]
+ ldr x8, [x4, #(2 * 8)]
+ ldr x7, [x4, #(1 * 8)]
+ ldr x6, [x4, #(0 * 8)]
+
+ adr x22, 1f
+ add x22, x22, x5, lsl #2
+ br x22
1:
- ldr x20, [x3, #(15 * 8)]
- ldr x19, [x3, #(14 * 8)]
- ldr x18, [x3, #(13 * 8)]
- ldr x17, [x3, #(12 * 8)]
- ldr x16, [x3, #(11 * 8)]
- ldr x15, [x3, #(10 * 8)]
- ldr x14, [x3, #(9 * 8)]
- ldr x13, [x3, #(8 * 8)]
- ldr x12, [x3, #(7 * 8)]
- ldr x11, [x3, #(6 * 8)]
- ldr x10, [x3, #(5 * 8)]
- ldr x9, [x3, #(4 * 8)]
- ldr x8, [x3, #(3 * 8)]
- ldr x7, [x3, #(2 * 8)]
- ldr x6, [x3, #(1 * 8)]
- ldr x5, [x3, #(0 * 8)]
-
- adr x26, 1f
- add x26, x26, x24, lsl #2
- br x26
-1:
- msr dbgbvr15_el1, x20
- msr dbgbvr14_el1, x19
- msr dbgbvr13_el1, x18
- msr dbgbvr12_el1, x17
- msr dbgbvr11_el1, x16
- msr dbgbvr10_el1, x15
- msr dbgbvr9_el1, x14
- msr dbgbvr8_el1, x13
- msr dbgbvr7_el1, x12
- msr dbgbvr6_el1, x11
- msr dbgbvr5_el1, x10
- msr dbgbvr4_el1, x9
- msr dbgbvr3_el1, x8
- msr dbgbvr2_el1, x7
- msr dbgbvr1_el1, x6
- msr dbgbvr0_el1, x5
-
- add x3, x2, #CPU_SYSREG_OFFSET(DBGWCR0_EL1)
-
- adr x26, 1f
- add x26, x26, x25, lsl #2
- br x26
-1:
- ldr x20, [x3, #(15 * 8)]
- ldr x19, [x3, #(14 * 8)]
- ldr x18, [x3, #(13 * 8)]
- ldr x17, [x3, #(12 * 8)]
- ldr x16, [x3, #(11 * 8)]
- ldr x15, [x3, #(10 * 8)]
- ldr x14, [x3, #(9 * 8)]
- ldr x13, [x3, #(8 * 8)]
- ldr x12, [x3, #(7 * 8)]
- ldr x11, [x3, #(6 * 8)]
- ldr x10, [x3, #(5 * 8)]
- ldr x9, [x3, #(4 * 8)]
- ldr x8, [x3, #(3 * 8)]
- ldr x7, [x3, #(2 * 8)]
- ldr x6, [x3, #(1 * 8)]
- ldr x5, [x3, #(0 * 8)]
-
- adr x26, 1f
- add x26, x26, x25, lsl #2
- br x26
-1:
- msr dbgwcr15_el1, x20
- msr dbgwcr14_el1, x19
- msr dbgwcr13_el1, x18
- msr dbgwcr12_el1, x17
- msr dbgwcr11_el1, x16
- msr dbgwcr10_el1, x15
- msr dbgwcr9_el1, x14
- msr dbgwcr8_el1, x13
- msr dbgwcr7_el1, x12
- msr dbgwcr6_el1, x11
- msr dbgwcr5_el1, x10
- msr dbgwcr4_el1, x9
- msr dbgwcr3_el1, x8
- msr dbgwcr2_el1, x7
- msr dbgwcr1_el1, x6
- msr dbgwcr0_el1, x5
-
- add x3, x2, #CPU_SYSREG_OFFSET(DBGWVR0_EL1)
-
- adr x26, 1f
- add x26, x26, x25, lsl #2
- br x26
-1:
- ldr x20, [x3, #(15 * 8)]
- ldr x19, [x3, #(14 * 8)]
- ldr x18, [x3, #(13 * 8)]
- ldr x17, [x3, #(12 * 8)]
- ldr x16, [x3, #(11 * 8)]
- ldr x15, [x3, #(10 * 8)]
- ldr x14, [x3, #(9 * 8)]
- ldr x13, [x3, #(8 * 8)]
- ldr x12, [x3, #(7 * 8)]
- ldr x11, [x3, #(6 * 8)]
- ldr x10, [x3, #(5 * 8)]
- ldr x9, [x3, #(4 * 8)]
- ldr x8, [x3, #(3 * 8)]
- ldr x7, [x3, #(2 * 8)]
- ldr x6, [x3, #(1 * 8)]
- ldr x5, [x3, #(0 * 8)]
-
- adr x26, 1f
- add x26, x26, x25, lsl #2
- br x26
-1:
- msr dbgwvr15_el1, x20
- msr dbgwvr14_el1, x19
- msr dbgwvr13_el1, x18
- msr dbgwvr12_el1, x17
- msr dbgwvr11_el1, x16
- msr dbgwvr10_el1, x15
- msr dbgwvr9_el1, x14
- msr dbgwvr8_el1, x13
- msr dbgwvr7_el1, x12
- msr dbgwvr6_el1, x11
- msr dbgwvr5_el1, x10
- msr dbgwvr4_el1, x9
- msr dbgwvr3_el1, x8
- msr dbgwvr2_el1, x7
- msr dbgwvr1_el1, x6
- msr dbgwvr0_el1, x5
-
- ldr x21, [x2, #CPU_SYSREG_OFFSET(MDCCINT_EL1)]
- msr mdccint_el1, x21
+ msr \type\()15_el1, x21
+ msr \type\()14_el1, x20
+ msr \type\()13_el1, x19
+ msr \type\()12_el1, x18
+ msr \type\()11_el1, x17
+ msr \type\()10_el1, x16
+ msr \type\()9_el1, x15
+ msr \type\()8_el1, x14
+ msr \type\()7_el1, x13
+ msr \type\()6_el1, x12
+ msr \type\()5_el1, x11
+ msr \type\()4_el1, x10
+ msr \type\()3_el1, x9
+ msr \type\()2_el1, x8
+ msr \type\()1_el1, x7
+ msr \type\()0_el1, x6
.endm
.macro skip_32bit_state tmp, target
@@ -887,12 +597,65 @@ __restore_sysregs:
restore_sysregs
ret
+/* Save debug state */
__save_debug:
- save_debug
+ // x0: base address for vcpu context
+ // x2: ptr to current CPU context
+ // x3: ptr to debug registers
+ // x4/x5: trashed
+
+ mrs x26, id_aa64dfr0_el1
+ ubfx x24, x26, #12, #4 // Extract BRPs
+ ubfx x25, x26, #20, #4 // Extract WRPs
+ mov w26, #15
+ sub w24, w26, w24 // How many BPs to skip
+ sub w25, w26, w25 // How many WPs to skip
+
+ mov x5, x24
+ add x4, x3, #DEBUG_BCR
+ save_debug_registers dbgbcr
+ add x4, x3, #DEBUG_BVR
+ save_debug_registers dbgbvr
+
+ mov x5, x25
+ add x4, x3, #DEBUG_WCR
+ save_debug_registers dbgwcr
+ add x4, x3, #DEBUG_WVR
+ save_debug_registers dbgwvr
+
+ mrs x21, mdccint_el1
+ str x21, [x2, #CPU_SYSREG_OFFSET(MDCCINT_EL1)]
ret
+/* Restore debug state */
__restore_debug:
- restore_debug
+ // x0: base address for cpu context
+ // x2: ptr to current CPU context
+ // x3: ptr to debug registers
+ // x4/x5: trashed
+
+ mrs x26, id_aa64dfr0_el1
+ ubfx x24, x26, #12, #4 // Extract BRPs
+ ubfx x25, x26, #20, #4 // Extract WRPs
+ mov w26, #15
+ sub w24, w26, w24 // How many BPs to skip
+ sub w25, w26, w25 // How many WPs to skip
+
+ mov x5, x24
+ add x4, x3, #DEBUG_BCR
+ setup_debug_registers dbgbcr
+ add x4, x3, #DEBUG_BVR
+ setup_debug_registers dbgbvr
+
+ mov x5, x25
+ add x4, x3, #DEBUG_WCR
+ setup_debug_registers dbgwcr
+ add x4, x3, #DEBUG_WVR
+ setup_debug_registers dbgwvr
+
+ ldr x21, [x2, #CPU_SYSREG_OFFSET(MDCCINT_EL1)]
+ msr mdccint_el1, x21
+
ret
__save_fpsimd:
@@ -927,6 +690,7 @@ ENTRY(__kvm_vcpu_run)
bl __save_sysregs
compute_debug_state 1f
+ add x3, x2, #CPU_SYSREG_OFFSET(DBGBCR0_EL1)
bl __save_debug
1:
activate_traps
@@ -942,6 +706,7 @@ ENTRY(__kvm_vcpu_run)
bl __restore_fpsimd
skip_debug_state x3, 1f
+ add x3, x2, #CPU_SYSREG_OFFSET(DBGBCR0_EL1)
bl __restore_debug
1:
restore_guest_32bit_state
@@ -962,6 +727,7 @@ __kvm_vcpu_return:
bl __save_sysregs
skip_debug_state x3, 1f
+ add x3, x2, #CPU_SYSREG_OFFSET(DBGBCR0_EL1)
bl __save_debug
1:
save_guest_32bit_state
@@ -984,6 +750,7 @@ __kvm_vcpu_return:
// already been saved. Note that we nuke the whole 64bit word.
// If we ever add more flags, we'll have to be more careful...
str xzr, [x0, #VCPU_DEBUG_FLAGS]
+ add x3, x2, #CPU_SYSREG_OFFSET(DBGBCR0_EL1)
bl __restore_debug
1:
restore_host_regs
--
2.3.5
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 09/12] KVM: arm64: guest debug, HW assisted debug support
[not found] <1430929407-3487-1-git-send-email-alex.bennee@linaro.org>
` (7 preceding siblings ...)
2015-05-07 9:07 ` [PATCH v3 08/12] KVM: arm64: re-factor hyp.S debug register code Alex Bennée
@ 2015-05-07 9:07 ` Alex Bennée
2015-05-08 16:32 ` Christoffer Dall
2015-05-07 9:07 ` [PATCH v3 10/12] KVM: arm64: trap nested debug register access Alex Bennée
` (2 subsequent siblings)
11 siblings, 1 reply; 24+ messages in thread
From: Alex Bennée @ 2015-05-07 9:07 UTC (permalink / raw)
To: kvm, linux-arm-kernel, kvmarm, christoffer.dall, marc.zyngier,
peter.maydell, agraf, drjones, pbonzini, zhichao.huang
Cc: jan.kiszka, dahi, r65777, bp, Alex Bennée, Gleb Natapov,
Jonathan Corbet, Russell King, Catalin Marinas, Will Deacon,
Ard Biesheuvel, Lorenzo Pieralisi, Andre Przywara,
Richard Weinberger, Peter Zijlstra, Ingo Molnar, AKASHI Takahiro,
open list:DOCUMENTATION, open list, open list:ABI/API
This adds support for userspace to control the HW debug registers for
guest debug. In the debug ioctl we copy the IMPDEF defined number of
registers into a new register set called host_debug_state. There is now
a new vcpu parameter called debug_ptr which selects which register set
is to copied into the real registers when world switch occurs.
I've moved some helper functions into the hw_breakpoint.h header for
re-use.
As with single step we need to tweak the guest registers to enable the
exceptions so we need to save and restore those bits.
Two new capabilities have been added to the KVM_EXTENSION ioctl to allow
userspace to query the number of hardware break and watch points
available on the host hardware.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
v2
- switched to C setup
- replace host debug registers directly into context
- minor tweak to api docs
- setup right register for debug
- add FAR_EL2 to debug exit structure
- add support for trapping debug register access
v3
- remove stray trace statement
- fix spacing around operators (various)
- clean-up usage of trap_debug
- introduce debug_ptr, replace excessive memcpy stuff
- don't use memcpy in ioctl, just assign
- update cap ioctl documentation
- reword a number comments
- rename host_debug_state->external_debug_state
diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 5ef937c..419f7a8 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -2668,7 +2668,7 @@ The top 16 bits of the control field are architecture specific control
flags which can include the following:
- KVM_GUESTDBG_USE_SW_BP: using software breakpoints [x86, arm64]
- - KVM_GUESTDBG_USE_HW_BP: using hardware breakpoints [x86, s390]
+ - KVM_GUESTDBG_USE_HW_BP: using hardware breakpoints [x86, s390, arm64]
- KVM_GUESTDBG_INJECT_DB: inject DB type exception [x86]
- KVM_GUESTDBG_INJECT_BP: inject BP type exception [x86]
- KVM_GUESTDBG_EXIT_PENDING: trigger an immediate guest exit [s390]
@@ -2683,6 +2683,11 @@ updated to the correct (supplied) values.
The second part of the structure is architecture specific and
typically contains a set of debug registers.
+For arm64 the number of debug registers is implementation defined and
+can be determined by querying the KVM_CAP_GUEST_DEBUG_HW_BPS and
+KVM_CAP_GUEST_DEBUG_HW_WPS capabilities which returns a +ve number
+indicating the number of supported registers.
+
When debug events exit the main run loop with the reason
KVM_EXIT_DEBUG with the kvm_debug_exit_arch part of the kvm_run
structure containing architecture specific debug information.
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 9b3ed6d..2920185 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -279,6 +279,10 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
/* Set up the timer */
kvm_timer_vcpu_init(vcpu);
+ /* Set the debug registers to be the guests */
+ vcpu->arch.debug_ptr = (struct kvm_guest_debug_arch *)
+ &vcpu_sys_reg(vcpu, DBGBCR0_EL1);
+
return 0;
}
@@ -304,6 +308,7 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
#define KVM_GUESTDBG_VALID_MASK (KVM_GUESTDBG_ENABLE | \
KVM_GUESTDBG_USE_SW_BP | \
+ KVM_GUESTDBG_USE_HW_BP | \
KVM_GUESTDBG_SINGLESTEP)
/**
@@ -324,6 +329,12 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
if (dbg->control & KVM_GUESTDBG_ENABLE) {
vcpu->guest_debug = dbg->control;
+
+ /* Hardware assisted Break and Watch points */
+ if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP) {
+ vcpu->arch.external_debug_state = dbg->arch;
+ }
+
} else {
/* If not enabled clear all flags */
vcpu->guest_debug = 0;
diff --git a/arch/arm64/include/asm/hw_breakpoint.h b/arch/arm64/include/asm/hw_breakpoint.h
index 52b484b..c450552 100644
--- a/arch/arm64/include/asm/hw_breakpoint.h
+++ b/arch/arm64/include/asm/hw_breakpoint.h
@@ -130,6 +130,18 @@ static inline void ptrace_hw_copy_thread(struct task_struct *task)
}
#endif
+/* Determine number of BRP registers available. */
+static inline int get_num_brps(void)
+{
+ return ((read_cpuid(ID_AA64DFR0_EL1) >> 12) & 0xf) + 1;
+}
+
+/* Determine number of WRP registers available. */
+static inline int get_num_wrps(void)
+{
+ return ((read_cpuid(ID_AA64DFR0_EL1) >> 20) & 0xf) + 1;
+}
+
extern struct pmu perf_ops_bp;
#endif /* __KERNEL__ */
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index b60fa7a..a44fb32 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -108,9 +108,18 @@ struct kvm_vcpu_arch {
/* Exception Information */
struct kvm_vcpu_fault_info fault;
- /* Debug state */
+ /* Guest debug state */
u64 debug_flags;
+ /*
+ * For debugging the guest we need to keep a set of debug
+ * registers which can override the guests own debug state
+ * while being used. These are set via the KVM_SET_GUEST_DEBUG
+ * ioctl.
+ */
+ struct kvm_guest_debug_arch *debug_ptr;
+ struct kvm_guest_debug_arch external_debug_state;
+
/* Pointer to host CPU context */
kvm_cpu_context_t *host_cpu_context;
diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index 04957d7..98e82ef 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -121,7 +121,7 @@ struct kvm_guest_debug_arch {
struct kvm_debug_exit_arch {
__u32 hsr;
- __u64 far;
+ __u64 far; /* used for watchpoints */
};
struct kvm_sync_regs {
diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index ce7b7dd..671ab13 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -116,6 +116,7 @@ int main(void)
DEFINE(VCPU_FAR_EL2, offsetof(struct kvm_vcpu, arch.fault.far_el2));
DEFINE(VCPU_HPFAR_EL2, offsetof(struct kvm_vcpu, arch.fault.hpfar_el2));
DEFINE(VCPU_DEBUG_FLAGS, offsetof(struct kvm_vcpu, arch.debug_flags));
+ DEFINE(VCPU_DEBUG_PTR, offsetof(struct kvm_vcpu, arch.debug_ptr));
DEFINE(DEBUG_BCR, offsetof(struct kvm_guest_debug_arch, dbg_bcr));
DEFINE(DEBUG_BVR, offsetof(struct kvm_guest_debug_arch, dbg_bvr));
DEFINE(DEBUG_WCR, offsetof(struct kvm_guest_debug_arch, dbg_wcr));
diff --git a/arch/arm64/kernel/hw_breakpoint.c b/arch/arm64/kernel/hw_breakpoint.c
index e7d934d..3a41bbf 100644
--- a/arch/arm64/kernel/hw_breakpoint.c
+++ b/arch/arm64/kernel/hw_breakpoint.c
@@ -49,18 +49,6 @@ static DEFINE_PER_CPU(int, stepping_kernel_bp);
static int core_num_brps;
static int core_num_wrps;
-/* Determine number of BRP registers available. */
-static int get_num_brps(void)
-{
- return ((read_cpuid(ID_AA64DFR0_EL1) >> 12) & 0xf) + 1;
-}
-
-/* Determine number of WRP registers available. */
-static int get_num_wrps(void)
-{
- return ((read_cpuid(ID_AA64DFR0_EL1) >> 20) & 0xf) + 1;
-}
-
int hw_breakpoint_slots(int type)
{
/*
diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
index 19346e8..1ab63dd 100644
--- a/arch/arm64/kvm/debug.c
+++ b/arch/arm64/kvm/debug.c
@@ -99,12 +99,6 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
MDCR_EL2_TDRA |
MDCR_EL2_TDOSA);
- /* Trap on access to debug registers? */
- if (trap_debug)
- vcpu->arch.mdcr_el2 |= MDCR_EL2_TDA;
- else
- vcpu->arch.mdcr_el2 &= ~MDCR_EL2_TDA;
-
/* Is Guest debugging in effect? */
if (vcpu->guest_debug) {
vcpu->arch.mdcr_el2 |= MDCR_EL2_TDE;
@@ -128,14 +122,54 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
vcpu_sys_reg(vcpu, MDSCR_EL1) &= ~DBG_MDSCR_SS;
}
+ /*
+ * HW Break/Watch points
+ *
+ * We simply switch the debug_ptr to point to our new
+ * external_debug_state which has been populated by the
+ * debug ioctl. The existing KVM_ARM64_DEBUG_DIRTY
+ * mechanism ensures the registers are updated on the
+ * world switch.
+ */
+ if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP) {
+
+ vcpu_sys_reg(vcpu, MDSCR_EL1) |=
+ (DBG_MDSCR_KDE | DBG_MDSCR_MDE);
+
+ vcpu->arch.debug_ptr = &vcpu->arch.external_debug_state;
+ vcpu->arch.debug_flags |= KVM_ARM64_DEBUG_DIRTY;
+ trap_debug = true;
+ }
+
} else {
/* Debug operations can go straight to the guest */
vcpu->arch.mdcr_el2 &= ~MDCR_EL2_TDE;
}
+
+ /*
+ * If the guest debug register state is dirty (the guest is
+ * actively accessing them), then we context-switch the
+ * registers in EL2. Otherwise, we trap-and-emulate all guest
+ * accesses to them.
+ */
+ if (trap_debug)
+ vcpu->arch.mdcr_el2 |= MDCR_EL2_TDA;
+ else
+ vcpu->arch.mdcr_el2 &= ~MDCR_EL2_TDA;
}
void kvm_arm_clear_debug(struct kvm_vcpu *vcpu)
{
- if (vcpu->guest_debug)
+ if (vcpu->guest_debug) {
restore_guest_debug_regs(vcpu);
+
+ /*
+ * If we were using HW debug we need to restore the
+ * debug_ptr to the guest debug state.
+ */
+ if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP) {
+ vcpu->arch.debug_ptr = (struct kvm_guest_debug_arch *)
+ &vcpu_sys_reg(vcpu, DBGBCR0_EL1);
+ }
+ }
}
diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index e9de13e..68a0759 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -103,7 +103,11 @@ static int kvm_handle_guest_debug(struct kvm_vcpu *vcpu, struct kvm_run *run)
run->debug.arch.hsr = hsr;
switch (hsr >> ESR_ELx_EC_SHIFT) {
+ case ESR_ELx_EC_WATCHPT_LOW:
+ run->debug.arch.far = vcpu->arch.fault.far_el2;
+ /* fall through */
case ESR_ELx_EC_SOFTSTP_LOW:
+ case ESR_ELx_EC_BREAKPT_LOW:
case ESR_ELx_EC_BKPT32:
case ESR_ELx_EC_BRK64:
break;
@@ -132,6 +136,8 @@ static exit_handle_fn arm_exit_handlers[] = {
[ESR_ELx_EC_IABT_LOW] = kvm_handle_guest_abort,
[ESR_ELx_EC_DABT_LOW] = kvm_handle_guest_abort,
[ESR_ELx_EC_SOFTSTP_LOW]= kvm_handle_guest_debug,
+ [ESR_ELx_EC_WATCHPT_LOW]= kvm_handle_guest_debug,
+ [ESR_ELx_EC_BREAKPT_LOW]= kvm_handle_guest_debug,
[ESR_ELx_EC_BKPT32] = kvm_handle_guest_debug,
[ESR_ELx_EC_BRK64] = kvm_handle_guest_debug,
};
diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
index dd51fb1..921d248 100644
--- a/arch/arm64/kvm/hyp.S
+++ b/arch/arm64/kvm/hyp.S
@@ -706,7 +706,8 @@ ENTRY(__kvm_vcpu_run)
bl __restore_fpsimd
skip_debug_state x3, 1f
- add x3, x2, #CPU_SYSREG_OFFSET(DBGBCR0_EL1)
+ ldr x3, [x0, #VCPU_DEBUG_PTR]
+ kern_hyp_va x3
bl __restore_debug
1:
restore_guest_32bit_state
@@ -727,7 +728,8 @@ __kvm_vcpu_return:
bl __save_sysregs
skip_debug_state x3, 1f
- add x3, x2, #CPU_SYSREG_OFFSET(DBGBCR0_EL1)
+ ldr x3, [x0, #VCPU_DEBUG_PTR]
+ kern_hyp_va x3
bl __save_debug
1:
save_guest_32bit_state
diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
index 0b43265..21d5a62 100644
--- a/arch/arm64/kvm/reset.c
+++ b/arch/arm64/kvm/reset.c
@@ -56,6 +56,12 @@ static bool cpu_has_32bit_el1(void)
return !!(pfr0 & 0x20);
}
+/**
+ * kvm_arch_dev_ioctl_check_extension
+ *
+ * We currently assume that the number of HW registers is uniform
+ * across all CPUs (see cpuinfo_sanity_check).
+ */
int kvm_arch_dev_ioctl_check_extension(long ext)
{
int r;
@@ -64,6 +70,12 @@ int kvm_arch_dev_ioctl_check_extension(long ext)
case KVM_CAP_ARM_EL1_32BIT:
r = cpu_has_32bit_el1();
break;
+ case KVM_CAP_GUEST_DEBUG_HW_BPS:
+ r = get_num_brps();
+ break;
+ case KVM_CAP_GUEST_DEBUG_HW_WPS:
+ r = get_num_wrps();
+ break;
default:
r = 0;
}
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 3b6252e..923c2aa 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -825,6 +825,8 @@ struct kvm_ppc_smmu_info {
#define KVM_CAP_S390_INJECT_IRQ 113
#define KVM_CAP_S390_IRQ_STATE 114
#define KVM_CAP_PPC_HWRNG 115
+#define KVM_CAP_GUEST_DEBUG_HW_BPS 116
+#define KVM_CAP_GUEST_DEBUG_HW_WPS 117
#ifdef KVM_CAP_IRQ_ROUTING
--
2.3.5
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 10/12] KVM: arm64: trap nested debug register access
[not found] <1430929407-3487-1-git-send-email-alex.bennee@linaro.org>
` (8 preceding siblings ...)
2015-05-07 9:07 ` [PATCH v3 09/12] KVM: arm64: guest debug, HW assisted debug support Alex Bennée
@ 2015-05-07 9:07 ` Alex Bennée
2015-05-08 16:46 ` Christoffer Dall
2015-05-07 9:07 ` [PATCH v3 11/12] KVM: arm64: enable KVM_CAP_SET_GUEST_DEBUG Alex Bennée
2015-05-07 9:07 ` [PATCH v3 12/12] KVM: arm64: add trace points for guest_debug debug Alex Bennée
11 siblings, 1 reply; 24+ messages in thread
From: Alex Bennée @ 2015-05-07 9:07 UTC (permalink / raw)
To: kvm, linux-arm-kernel, kvmarm, christoffer.dall, marc.zyngier,
peter.maydell, agraf, drjones, pbonzini, zhichao.huang
Cc: jan.kiszka, dahi, r65777, bp, Alex Bennée, Gleb Natapov,
Catalin Marinas, Will Deacon, open list
When we are using the hardware registers for guest debug we need to deal
with the guests access to them. There is already a mechanism for dealing
with these accesses so we build on top of that.
- any access to mdscr_el1 is now stored in the mirror location
- access to DBG[WB][CV]R continues to go to guest's context
There is one register (MDCCINT_EL1) which guest debug doesn't care about
so this behaves as before.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
v3
- re-factor for better flow and fall through.
- much simpler with debug_ptr (use the guest area as before)
- tweak shadow fn to avoid multi-line if
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index a44fb32..7aa3b3a 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -132,7 +132,13 @@ struct kvm_vcpu_arch {
* here.
*/
- /* Guest registers we preserve during guest debugging */
+ /*
+ * Guest registers we preserve during guest debugging.
+ *
+ * These shadow registers are updated by the kvm_handle_sys_reg
+ * trap handler if the guest accesses or updates them while we
+ * are using guest debug.
+ */
struct {
u32 pstate;
u32 mdscr_el1;
diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
index 1ab63dd..dc8bca8 100644
--- a/arch/arm64/kvm/debug.c
+++ b/arch/arm64/kvm/debug.c
@@ -50,8 +50,7 @@ static void restore_guest_debug_regs(struct kvm_vcpu *vcpu)
{
*vcpu_cpsr(vcpu) |=
(vcpu->arch.guest_debug_state.pstate & SPSR_DEBUG_MASK);
- vcpu_sys_reg(vcpu, MDSCR_EL1) |=
- (vcpu->arch.guest_debug_state.mdscr_el1 & MDSCR_EL1_DEBUG_MASK);
+ vcpu_sys_reg(vcpu, MDSCR_EL1) = vcpu->arch.guest_debug_state.mdscr_el1;
}
/**
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index c370b40..95f422f 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -196,11 +196,40 @@ static bool trap_dbgauthstatus_el1(struct kvm_vcpu *vcpu,
* - If the dirty bit is set, save guest registers, restore host
* registers and clear the dirty bit. This ensure that the host can
* now use the debug registers.
+ *
+ * We also use this mechanism to set-up the debug registers for guest
+ * debugging. If this is the case we want to ensure the guest sees
+ * the right versions of the registers - even if they are not going to
+ * be effective while guest debug is using HW debug.
+ *
*/
+
+static bool shadow_debug_reg(struct kvm_vcpu *vcpu,
+ const struct sys_reg_params *p,
+ const struct sys_reg_desc *r)
+{
+ /* MDSCR_EL1 */
+ if (r->reg == MDSCR_EL1) {
+ u32 *shadow_mdscr_el1 = &vcpu->arch.guest_debug_state.mdscr_el1;
+
+ if (p->is_write)
+ *shadow_mdscr_el1 = *vcpu_reg(vcpu, p->Rt);
+ else
+ *vcpu_reg(vcpu, p->Rt) = *shadow_mdscr_el1;
+
+ return true;
+ }
+
+ return false;
+}
+
static bool trap_debug_regs(struct kvm_vcpu *vcpu,
const struct sys_reg_params *p,
const struct sys_reg_desc *r)
{
+ if (vcpu->guest_debug && shadow_debug_reg(vcpu, p, r))
+ return true;
+
if (p->is_write) {
vcpu_sys_reg(vcpu, r->reg) = *vcpu_reg(vcpu, p->Rt);
vcpu->arch.debug_flags |= KVM_ARM64_DEBUG_DIRTY;
--
2.3.5
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 11/12] KVM: arm64: enable KVM_CAP_SET_GUEST_DEBUG
[not found] <1430929407-3487-1-git-send-email-alex.bennee@linaro.org>
` (9 preceding siblings ...)
2015-05-07 9:07 ` [PATCH v3 10/12] KVM: arm64: trap nested debug register access Alex Bennée
@ 2015-05-07 9:07 ` Alex Bennée
2015-05-08 17:21 ` Christoffer Dall
2015-05-07 9:07 ` [PATCH v3 12/12] KVM: arm64: add trace points for guest_debug debug Alex Bennée
11 siblings, 1 reply; 24+ messages in thread
From: Alex Bennée @ 2015-05-07 9:07 UTC (permalink / raw)
To: kvm, linux-arm-kernel, kvmarm, christoffer.dall, marc.zyngier,
peter.maydell, agraf, drjones, pbonzini, zhichao.huang
Cc: jan.kiszka, dahi, r65777, bp, Alex Bennée, Gleb Natapov,
Catalin Marinas, Will Deacon, open list
Finally advertise the KVM capability for SET_GUEST_DEBUG. Once arm
support is added this check can be moved to the common
kvm_vm_ioctl_check_extension() code.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
v3:
- separated capability check from previous patches
- moved into arm64 specific ioctl handler.
diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
index 21d5a62..88e5331 100644
--- a/arch/arm64/kvm/reset.c
+++ b/arch/arm64/kvm/reset.c
@@ -76,6 +76,9 @@ int kvm_arch_dev_ioctl_check_extension(long ext)
case KVM_CAP_GUEST_DEBUG_HW_WPS:
r = get_num_wrps();
break;
+ case KVM_CAP_SET_GUEST_DEBUG:
+ r = 1;
+ break;
default:
r = 0;
}
--
2.3.5
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 12/12] KVM: arm64: add trace points for guest_debug debug
[not found] <1430929407-3487-1-git-send-email-alex.bennee@linaro.org>
` (10 preceding siblings ...)
2015-05-07 9:07 ` [PATCH v3 11/12] KVM: arm64: enable KVM_CAP_SET_GUEST_DEBUG Alex Bennée
@ 2015-05-07 9:07 ` Alex Bennée
2015-05-08 17:25 ` Christoffer Dall
11 siblings, 1 reply; 24+ messages in thread
From: Alex Bennée @ 2015-05-07 9:07 UTC (permalink / raw)
To: kvm, linux-arm-kernel, kvmarm, christoffer.dall, marc.zyngier,
peter.maydell, agraf, drjones, pbonzini, zhichao.huang
Cc: jan.kiszka, dahi, r65777, bp, Alex Bennée, Gleb Natapov,
Catalin Marinas, Will Deacon, open list
This includes trace points for:
kvm_arch_setup_guest_debug
kvm_arch_clear_guest_debug
kvm_handle_guest_debug
I've also added some generic register setting trace events and also a
trace point to dump the array of hardware registers.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
v3
- add trace event for debug access.
- remove short trace #define, rename trace events
- use __print_array with fixed array instead of own func
- rationalise trace points (only one per register changed)
- add vcpu ptr to the debug_setup trace
- remove :: in prints
diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
index dc8bca8..08e1b83 100644
--- a/arch/arm64/kvm/debug.c
+++ b/arch/arm64/kvm/debug.c
@@ -24,6 +24,8 @@
#include <asm/kvm_arm.h>
#include <asm/kvm_emulate.h>
+#include "trace.h"
+
/* These are the bits of MDSCR_EL1 we may manipulate */
#define MDSCR_EL1_DEBUG_MASK (DBG_MDSCR_SS | \
DBG_MDSCR_KDE | \
@@ -44,6 +46,11 @@ static void save_guest_debug_regs(struct kvm_vcpu *vcpu)
{
vcpu->arch.guest_debug_state.pstate = *vcpu_cpsr(vcpu);
vcpu->arch.guest_debug_state.mdscr_el1 = vcpu_sys_reg(vcpu, MDSCR_EL1);
+
+ trace_kvm_arm_set_dreg32("Saved PSTATE",
+ vcpu->arch.guest_debug_state.pstate);
+ trace_kvm_arm_set_dreg32("Saved MDSCR_EL1",
+ vcpu->arch.guest_debug_state.mdscr_el1);
}
static void restore_guest_debug_regs(struct kvm_vcpu *vcpu)
@@ -51,6 +58,10 @@ static void restore_guest_debug_regs(struct kvm_vcpu *vcpu)
*vcpu_cpsr(vcpu) |=
(vcpu->arch.guest_debug_state.pstate & SPSR_DEBUG_MASK);
vcpu_sys_reg(vcpu, MDSCR_EL1) = vcpu->arch.guest_debug_state.mdscr_el1;
+
+ trace_kvm_arm_set_dreg32("Restored PSTATE", *vcpu_cpsr(vcpu));
+ trace_kvm_arm_set_dreg32("Restored MDSCR_EL1",
+ vcpu_sys_reg(vcpu, MDSCR_EL1));
}
/**
@@ -92,6 +103,8 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
{
bool trap_debug = !(vcpu->arch.debug_flags & KVM_ARM64_DEBUG_DIRTY);
+ trace_kvm_arm_setup_debug(vcpu, vcpu->guest_debug);
+
vcpu->arch.mdcr_el2 = __this_cpu_read(mdcr_el2) & MDCR_EL2_HPMN_MASK;
vcpu->arch.mdcr_el2 |= (MDCR_EL2_TPM |
MDCR_EL2_TPMCR |
@@ -121,6 +134,8 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
vcpu_sys_reg(vcpu, MDSCR_EL1) &= ~DBG_MDSCR_SS;
}
+ trace_kvm_arm_set_dreg32("SPSR_EL2", *vcpu_cpsr(vcpu));
+
/*
* HW Break/Watch points
*
@@ -138,6 +153,14 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
vcpu->arch.debug_ptr = &vcpu->arch.external_debug_state;
vcpu->arch.debug_flags |= KVM_ARM64_DEBUG_DIRTY;
trap_debug = true;
+
+ trace_kvm_arm_set_regset("BKPTS", get_num_brps(),
+ &vcpu->arch.debug_ptr->dbg_bcr[0],
+ &vcpu->arch.debug_ptr->dbg_bvr[0]);
+
+ trace_kvm_arm_set_regset("WAPTS", get_num_wrps(),
+ &vcpu->arch.debug_ptr->dbg_wcr[0],
+ &vcpu->arch.debug_ptr->dbg_wvr[0]);
}
} else {
@@ -155,10 +178,15 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
vcpu->arch.mdcr_el2 |= MDCR_EL2_TDA;
else
vcpu->arch.mdcr_el2 &= ~MDCR_EL2_TDA;
+
+ trace_kvm_arm_set_dreg32("MDCR_EL2", vcpu->arch.mdcr_el2);
+ trace_kvm_arm_set_dreg32("MDSCR_EL1", vcpu_sys_reg(vcpu, MDSCR_EL1));
}
void kvm_arm_clear_debug(struct kvm_vcpu *vcpu)
{
+ trace_kvm_arm_clear_debug(vcpu->guest_debug);
+
if (vcpu->guest_debug) {
restore_guest_debug_regs(vcpu);
@@ -169,6 +197,14 @@ void kvm_arm_clear_debug(struct kvm_vcpu *vcpu)
if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP) {
vcpu->arch.debug_ptr = (struct kvm_guest_debug_arch *)
&vcpu_sys_reg(vcpu, DBGBCR0_EL1);
+
+ trace_kvm_arm_set_regset("BKPTS", get_num_brps(),
+ &vcpu->arch.debug_ptr->dbg_bcr[0],
+ &vcpu->arch.debug_ptr->dbg_bvr[0]);
+
+ trace_kvm_arm_set_regset("WAPTS", get_num_wrps(),
+ &vcpu->arch.debug_ptr->dbg_wcr[0],
+ &vcpu->arch.debug_ptr->dbg_wvr[0]);
}
}
}
diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index 68a0759..c93b95e 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -99,6 +99,8 @@ static int kvm_handle_guest_debug(struct kvm_vcpu *vcpu, struct kvm_run *run)
u32 hsr = kvm_vcpu_get_hsr(vcpu);
int ret = 0;
+ trace_kvm_handle_guest_debug(*vcpu_pc(vcpu), hsr);
+
run->exit_reason = KVM_EXIT_DEBUG;
run->debug.arch.hsr = hsr;
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 95f422f..ec803ad 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -38,6 +38,8 @@
#include "sys_regs.h"
+#include "trace.h"
+
/*
* All of this file is extremly similar to the ARM coproc.c, but the
* types are different. My gut feeling is that it should be pretty
@@ -227,6 +229,9 @@ static bool trap_debug_regs(struct kvm_vcpu *vcpu,
const struct sys_reg_params *p,
const struct sys_reg_desc *r)
{
+ trace_trap_debug_regs(r->reg, p->is_write,
+ p->is_write?*vcpu_reg(vcpu, p->Rt):0);
+
if (vcpu->guest_debug && shadow_debug_reg(vcpu, p, r))
return true;
diff --git a/arch/arm64/kvm/trace.h b/arch/arm64/kvm/trace.h
index 157416e9..62e6b76 100644
--- a/arch/arm64/kvm/trace.h
+++ b/arch/arm64/kvm/trace.h
@@ -44,6 +44,113 @@ TRACE_EVENT(kvm_hvc_arm64,
__entry->vcpu_pc, __entry->r0, __entry->imm)
);
+TRACE_EVENT(kvm_handle_guest_debug,
+ TP_PROTO(unsigned long vcpu_pc, u32 hsr),
+ TP_ARGS(vcpu_pc, hsr),
+
+ TP_STRUCT__entry(
+ __field(unsigned long, vcpu_pc)
+ __field(u32, hsr)
+ ),
+
+ TP_fast_assign(
+ __entry->vcpu_pc = vcpu_pc;
+ __entry->hsr = hsr;
+ ),
+
+ TP_printk("debug exception at 0x%08lx (HSR: 0x%08x)",
+ __entry->vcpu_pc, __entry->hsr)
+);
+
+
+TRACE_EVENT(kvm_arm_setup_debug,
+ TP_PROTO(struct kvm_vcpu *vcpu, __u32 guest_debug),
+ TP_ARGS(vcpu, guest_debug),
+
+ TP_STRUCT__entry(
+ __field(struct kvm_vcpu *, vcpu)
+ __field(__u32, guest_debug)
+ ),
+
+ TP_fast_assign(
+ __entry->vcpu = vcpu;
+ __entry->guest_debug = guest_debug;
+ ),
+
+ TP_printk("vcpu: %p, flags: 0x%08x", __entry->vcpu, __entry->guest_debug)
+);
+
+TRACE_EVENT(kvm_arm_clear_debug,
+ TP_PROTO(__u32 guest_debug),
+ TP_ARGS(guest_debug),
+
+ TP_STRUCT__entry(
+ __field(__u32, guest_debug)
+ ),
+
+ TP_fast_assign(
+ __entry->guest_debug = guest_debug;
+ ),
+
+ TP_printk("flags: 0x%08x", __entry->guest_debug)
+);
+
+TRACE_EVENT(kvm_arm_set_dreg32,
+ TP_PROTO(const char *name, __u32 value),
+ TP_ARGS(name, value),
+
+ TP_STRUCT__entry(
+ __field(const char *, name)
+ __field(__u32, value)
+ ),
+
+ TP_fast_assign(
+ __entry->name = name;
+ __entry->value = value;
+ ),
+
+ TP_printk("%s: 0x%08x", __entry->name, __entry->value)
+);
+
+TRACE_EVENT(kvm_arm_set_regset,
+ TP_PROTO(const char *type, int len, __u64 *control, __u64 *value),
+ TP_ARGS(type, len, control, value),
+ TP_STRUCT__entry(
+ __field(const char *, name)
+ __field(int, len)
+ __array(u64, ctrls, 16)
+ __array(u64, values, 16)
+ ),
+ TP_fast_assign(
+ __entry->name = type;
+ __entry->len = len;
+ memcpy(__entry->ctrls, control, len << 3);
+ memcpy(__entry->values, value, len << 3);
+ ),
+ TP_printk("%d %s CTRL:%s VALUE:%s", __entry->len, __entry->name,
+ __print_array(__entry->ctrls, __entry->len, sizeof(__u64)),
+ __print_array(__entry->values, __entry->len, sizeof(__u64)))
+);
+
+TRACE_EVENT(trap_debug_regs,
+ TP_PROTO(int reg, bool is_write, u64 write_value),
+ TP_ARGS(reg, is_write, write_value),
+
+ TP_STRUCT__entry(
+ __field(int, reg)
+ __field(bool, is_write)
+ __field(u64, write_value)
+ ),
+
+ TP_fast_assign(
+ __entry->reg = reg;
+ __entry->is_write = is_write;
+ __entry->write_value = write_value;
+ ),
+
+ TP_printk("%s reg %d (0x%08llx)", __entry->is_write?"write to":"read from", __entry->reg, __entry->write_value)
+);
+
#endif /* _TRACE_ARM64_KVM_H */
#undef TRACE_INCLUDE_PATH
--
2.3.5
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v3 01/12] KVM: add comments for kvm_debug_exit_arch struct
2015-05-06 16:23 ` [PATCH v3 01/12] KVM: add comments for kvm_debug_exit_arch struct Alex Bennée
@ 2015-05-08 9:19 ` Christoffer Dall
0 siblings, 0 replies; 24+ messages in thread
From: Christoffer Dall @ 2015-05-08 9:19 UTC (permalink / raw)
To: Alex Bennée
Cc: kvm, linux-arm-kernel, kvmarm, marc.zyngier, peter.maydell,
agraf, drjones, pbonzini, zhichao.huang, jan.kiszka, dahi,
r65777, bp, Gleb Natapov, Jonathan Corbet,
open list:DOCUMENTATION, open list, open list:ABI/API
On Wed, May 06, 2015 at 05:23:16PM +0100, Alex Bennée wrote:
> Bring into line with the comments for the other structures and their
> KVM_EXIT_* cases. Also update api.txt to reflect use in kvm_run
> documentation.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
> Reviewed-by: Andrew Jones <drjones@redhat.com>
>
> ---
>
> v2
> - add comments for other exit types
> v3
> - s/commentary/comments/
> - add rb tags
> - update api.txt kvm_run to include KVM_EXIT_DEBUG desc
>
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index 9fa2bf8..cb90025 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -3070,11 +3070,13 @@ data_offset describes where the data is located (KVM_EXIT_IO_OUT) or
> where kvm expects application code to place the data for the next
> KVM_RUN invocation (KVM_EXIT_IO_IN). Data format is a packed array.
>
> + /* KVM_EXIT_DEBUG */
> struct {
> struct kvm_debug_exit_arch arch;
> } debug;
>
> -Unused.
> +If the exit_reason in KVM_EXIT_DEBUG, then a vcpu is processing a debug event
s/in/is/
> +for which architecture dependant information is returned.
s/dependant/dependent/ (but maybe architecture specific is better)
>
> /* KVM_EXIT_MMIO */
> struct {
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 4b60056..70ac641 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -237,6 +237,7 @@ struct kvm_run {
> __u32 count;
> __u64 data_offset; /* relative to kvm_run start */
> } io;
> + /* KVM_EXIT_DEBUG */
> struct {
> struct kvm_debug_exit_arch arch;
> } debug;
> @@ -285,6 +286,7 @@ struct kvm_run {
> __u32 data;
> __u8 is_write;
> } dcr;
> + /* KVM_EXIT_INTERNAL_ERROR */
> struct {
> __u32 suberror;
> /* Available with KVM_CAP_INTERNAL_ERROR_DATA: */
> @@ -295,6 +297,7 @@ struct kvm_run {
> struct {
> __u64 gprs[32];
> } osi;
> + /* KVM_EXIT_PAPR_HCALL */
> struct {
> __u64 nr;
> __u64 ret;
> --
> 2.3.5
>
otherwise:
Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 02/12] KVM: define common __KVM_GUESTDBG_USE_SW/HW_BP values
2015-05-06 16:23 ` [PATCH v3 02/12] KVM: define common __KVM_GUESTDBG_USE_SW/HW_BP values Alex Bennée
@ 2015-05-08 9:23 ` Christoffer Dall
0 siblings, 0 replies; 24+ messages in thread
From: Christoffer Dall @ 2015-05-08 9:23 UTC (permalink / raw)
To: Alex Bennée
Cc: kvm, linux-arm-kernel, kvmarm, marc.zyngier, peter.maydell,
agraf, drjones, pbonzini, zhichao.huang, jan.kiszka, dahi,
r65777, bp, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
maintainer:X86 ARCHITECTURE...,
Gleb Natapov, Bharat Bhushan, Mihai Caraman,
Alexey Kardashevskiy, Nadav Amit, open list:LINUX FOR POWERPC...,
open list, open list:ABI/API
On Wed, May 06, 2015 at 05:23:17PM +0100, Alex Bennée wrote:
> Currently x86, powerpc and soon arm64 use the same two architecture
> specific bits for guest debug support for software and hardware
> breakpoints. This makes the shared values explicit while leaving the
> gate open for another architecture to use some other value if they
> really really want to.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Reviewed-by: Andrew Jones <drjones@redhat.com>
>
> diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
> index ab4d473..1731569 100644
> --- a/arch/powerpc/include/uapi/asm/kvm.h
> +++ b/arch/powerpc/include/uapi/asm/kvm.h
> @@ -310,8 +310,8 @@ struct kvm_guest_debug_arch {
> * and upper 16 bits are architecture specific. Architecture specific defines
> * that ioctl is for setting hardware breakpoint or software breakpoint.
> */
> -#define KVM_GUESTDBG_USE_SW_BP 0x00010000
> -#define KVM_GUESTDBG_USE_HW_BP 0x00020000
> +#define KVM_GUESTDBG_USE_SW_BP __KVM_GUESTDBG_USE_SW_BP
> +#define KVM_GUESTDBG_USE_HW_BP __KVM_GUESTDBG_USE_HW_BP
>
> /* definition of registers in kvm_run */
> struct kvm_sync_regs {
> diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
> index d7dcef5..1438202 100644
> --- a/arch/x86/include/uapi/asm/kvm.h
> +++ b/arch/x86/include/uapi/asm/kvm.h
> @@ -250,8 +250,8 @@ struct kvm_debug_exit_arch {
> __u64 dr7;
> };
>
> -#define KVM_GUESTDBG_USE_SW_BP 0x00010000
> -#define KVM_GUESTDBG_USE_HW_BP 0x00020000
> +#define KVM_GUESTDBG_USE_SW_BP __KVM_GUESTDBG_USE_SW_BP
> +#define KVM_GUESTDBG_USE_HW_BP __KVM_GUESTDBG_USE_HW_BP
> #define KVM_GUESTDBG_INJECT_DB 0x00040000
> #define KVM_GUESTDBG_INJECT_BP 0x00080000
>
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 70ac641..3b6252e 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -570,8 +570,16 @@ struct kvm_s390_irq_state {
>
> /* for KVM_SET_GUEST_DEBUG */
>
> -#define KVM_GUESTDBG_ENABLE 0x00000001
> -#define KVM_GUESTDBG_SINGLESTEP 0x00000002
> +#define KVM_GUESTDBG_ENABLE (1 << 0)
> +#define KVM_GUESTDBG_SINGLESTEP (1 << 1)
> +
> +/*
> + * Architecture specific stuff uses the top 16 bits of the field,
s/stuff/<something more specific>/
> + * however there is some shared commonality for the common cases
> + */
> +#define __KVM_GUESTDBG_USE_SW_BP (1 << 16)
> +#define __KVM_GUESTDBG_USE_HW_BP (1 << 17)
> +
>
> struct kvm_guest_debug {
> __u32 control;
We sort of left this discussion hanging with me expressing slight
concern about the usefulness about these defines.
Paolo, what are your thoughts?
-Christoffer
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 03/12] KVM: arm64: guest debug, define API headers
2015-05-06 16:23 ` [PATCH v3 03/12] KVM: arm64: guest debug, define API headers Alex Bennée
@ 2015-05-08 9:28 ` Christoffer Dall
0 siblings, 0 replies; 24+ messages in thread
From: Christoffer Dall @ 2015-05-08 9:28 UTC (permalink / raw)
To: Alex Bennée
Cc: kvm, linux-arm-kernel, kvmarm, marc.zyngier, peter.maydell,
agraf, drjones, pbonzini, zhichao.huang, jan.kiszka, dahi,
r65777, bp, Catalin Marinas, Will Deacon, open list
On Wed, May 06, 2015 at 05:23:18PM +0100, Alex Bennée wrote:
> This commit defines the API headers for guest debugging. There are two
> architecture specific debug structures:
>
> - kvm_guest_debug_arch, allows us to pass in HW debug registers
> - kvm_debug_exit_arch, signals exception and possible faulting address
>
> The type of debugging being used is controlled by the architecture
> specific control bits of the kvm_guest_debug->control flags in the ioctl
> structure.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
> Reviewed-by: Andrew Jones <drjones@redhat.com>
>
> ---
> v2
> - expose hsr and pc directly to user-space
> v3
> - s/control/controlled/ in commit message
> - add v8 to ARM ARM comment (ARM Architecture Reference Manual)
> - add rb tag
> - rm pc, add far
> - re-word comments on alignment
> - rename KVM_ARM_NDBG_REGS -> KVM_ARM_MAX_DBG_REGS
>
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index d268320..04957d7 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -100,10 +100,28 @@ struct kvm_sregs {
> struct kvm_fpu {
> };
>
> +/*
> + * See v8 ARM ARM D7.3: Debug Registers
> + *
> + * The control registers are architecturally defined as 32 bits but are
> + * stored as 64 bit values alongside the value registers. This is done
> + * to keep the copying of these values into the vcpu context simple as
> + * everything is 64 bit aligned (see DBGBCR0_EL1 onwards in kvm_asm.h).
> + *
> + * The architectural limit is 16 debug registers of each type although
> + * in practice there are usually less (see ID_AA64DFR0_EL1).
> + */
> +#define KVM_ARM_MAX_DBG_REGS 16
> struct kvm_guest_debug_arch {
> + __u64 dbg_bcr[KVM_ARM_MAX_DBG_REGS];
> + __u64 dbg_bvr[KVM_ARM_MAX_DBG_REGS];
> + __u64 dbg_wcr[KVM_ARM_MAX_DBG_REGS];
> + __u64 dbg_wvr[KVM_ARM_MAX_DBG_REGS];
> };
>
> struct kvm_debug_exit_arch {
> + __u32 hsr;
> + __u64 far;
> };
>
> struct kvm_sync_regs {
> @@ -216,4 +234,11 @@ struct kvm_arch_memory_slot {
>
> #endif
>
> +/*
> + * Architecture related debug defines - upper 16 bits of
"Architecture specific debug control flags" seems more accurate.
> + * kvm_guest_debug->control
> + */
> +#define KVM_GUESTDBG_USE_SW_BP __KVM_GUESTDBG_USE_SW_BP
> +#define KVM_GUESTDBG_USE_HW_BP __KVM_GUESTDBG_USE_HW_BP
> +
> #endif /* __ARM_KVM_H__ */
> --
> 2.3.5
>
Otherwise:
Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
Thanks,
-Christoffer
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 04/12] KVM: arm: guest debug, add stub KVM_SET_GUEST_DEBUG ioctl
2015-05-06 16:23 ` [PATCH v3 04/12] KVM: arm: guest debug, add stub KVM_SET_GUEST_DEBUG ioctl Alex Bennée
@ 2015-05-08 11:52 ` Christoffer Dall
0 siblings, 0 replies; 24+ messages in thread
From: Christoffer Dall @ 2015-05-08 11:52 UTC (permalink / raw)
To: Alex Bennée
Cc: kvm, linux-arm-kernel, kvmarm, marc.zyngier, peter.maydell,
agraf, drjones, pbonzini, zhichao.huang, jan.kiszka, dahi,
r65777, bp, Gleb Natapov, Jonathan Corbet, Russell King,
open list:DOCUMENTATION, open list
On Wed, May 06, 2015 at 05:23:19PM +0100, Alex Bennée wrote:
> This commit adds a stub function to support the KVM_SET_GUEST_DEBUG
> ioctl. Any unsupported flag will return -EINVAL. For now, only
> KVM_GUESTDBG_ENABLE is supported, although it won't have any effects.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>.
>
> ---
> v2
> - simplified form of the ioctl (stuff will go into setup_debug)
> v3
> - KVM_GUESTDBG_VALID->KVM_GUESTDBG_VALID_MASK
> - move mask check to the top of function
> - add ioctl doc header
> - split capability into separate patch
> - tweaked commit wording w.r.t return of -EINVAL
>
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index cb90025..4b0132f 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -2645,7 +2645,7 @@ handled.
> 4.87 KVM_SET_GUEST_DEBUG
>
> Capability: KVM_CAP_SET_GUEST_DEBUG
> -Architectures: x86, s390, ppc
> +Architectures: x86, s390, ppc, arm64
> Type: vcpu ioctl
> Parameters: struct kvm_guest_debug (in)
> Returns: 0 on success; -1 on error
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index d9631ec..52a1d4d38 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -302,10 +302,31 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
> kvm_arm_set_running_vcpu(NULL);
> }
>
> +#define KVM_GUESTDBG_VALID_MASK (KVM_GUESTDBG_ENABLE)
> +
> +/**
> + * kvm_arch_vcpu_ioctl_set_guest_debug - set up guest debugging
> + * @kvm: pointer to the KVM struct
> + * @kvm_guest_debug: the ioctl data buffer
> + *
> + * This sets up and enables the VM for guest debugging. Userspace
> + * passes in a control flag to enable different debug types and
> + * potentially other architecture specific information in the rest of
> + * the structure.
> + */
> int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
> struct kvm_guest_debug *dbg)
> {
> - return -EINVAL;
> + if (dbg->control & ~KVM_GUESTDBG_VALID_MASK)
> + return -EINVAL;
> +
> + if (dbg->control & KVM_GUESTDBG_ENABLE) {
> + vcpu->guest_debug = dbg->control;
> + } else {
> + /* If not enabled clear all flags */
> + vcpu->guest_debug = 0;
> + }
> + return 0;
> }
>
Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 05/12] KVM: arm: introduce kvm_arm_init/setup/clear_debug
2015-05-06 16:23 ` [PATCH v3 05/12] KVM: arm: introduce kvm_arm_init/setup/clear_debug Alex Bennée
@ 2015-05-08 11:52 ` Christoffer Dall
0 siblings, 0 replies; 24+ messages in thread
From: Christoffer Dall @ 2015-05-08 11:52 UTC (permalink / raw)
To: Alex Bennée
Cc: kvm, linux-arm-kernel, kvmarm, marc.zyngier, peter.maydell,
agraf, drjones, pbonzini, zhichao.huang, jan.kiszka, dahi,
r65777, bp, Gleb Natapov, Russell King, Catalin Marinas,
Will Deacon, Ard Biesheuvel, Richard Weinberger, Andre Przywara,
Lorenzo Pieralisi, open list
On Wed, May 06, 2015 at 05:23:20PM +0100, Alex Bennée wrote:
> This is a precursor for later patches which will need to do more to
> setup debug state before entering the hyp.S switch code. The existing
> functionality for setting mdcr_el2 has been moved out of hyp.S and now
> uses the value kept in vcpu->arch.mdcr_el2.
>
> As the assembler used to previously mask and preserve MDCR_EL2.HPMN I've
> had to add a mechanism to save the value of mdcr_el2 as a per-cpu
> variable during the initialisation code. The kernel never sets this
> number so we are assuming the bootcode has set up the correct value
> here.
>
> This also moves the conditional setting of the TDA bit from the hyp code
> into the C code which is currently used for the lazy debug register
> context switch code.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> ---
> v3
> - rename fns from arch->arm
> - preserve MDCR_EL2.HPMN setting
> - re-word some of the comments
> - fix some minor grammar nits
> - merge setting of mdcr_el2
> - introduce trap_debug flag
> - move setup/clear within the irq lock section
>
> create mode 100644 arch/arm64/kvm/debug.c
>
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index d71607c..746c0c69 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -236,4 +236,8 @@ static inline void kvm_arch_sync_events(struct kvm *kvm) {}
> static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
> static inline void kvm_arch_sched_in(struct kvm_vcpu *vcpu, int cpu) {}
>
> +static inline void kvm_arm_init_debug(void) {}
> +static inline void kvm_arm_setup_debug(struct kvm_vcpu *vcpu) {}
> +static inline void kvm_arm_clear_debug(struct kvm_vcpu *vcpu) {}
> +
> #endif /* __ARM_KVM_HOST_H__ */
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index 52a1d4d38..4a274e1 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -570,6 +570,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
> continue;
> }
>
> + kvm_arm_setup_debug(vcpu);
> +
> /**************************************************************
> * Enter the guest
> */
> @@ -582,7 +584,10 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
> vcpu->mode = OUTSIDE_GUEST_MODE;
> kvm_guest_exit();
> trace_kvm_exit(kvm_vcpu_trap_get_class(vcpu), *vcpu_pc(vcpu));
> - /*
> +
> + kvm_arm_clear_debug(vcpu);
> +
> + /*
> * We may have taken a host interrupt in HYP mode (ie
> * while executing the guest). This interrupt is still
> * pending, as we haven't serviced it yet!
> @@ -930,6 +935,8 @@ static void cpu_init_hyp_mode(void *dummy)
> vector_ptr = (unsigned long)__kvm_hyp_vector;
>
> __cpu_init_hyp_mode(boot_pgd_ptr, pgd_ptr, hyp_stack_ptr, vector_ptr);
> +
> + kvm_arm_init_debug();
> }
>
> static int hyp_init_cpu_notify(struct notifier_block *self,
> diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
> index 4f7310f..d6b507e 100644
> --- a/arch/arm64/include/asm/kvm_asm.h
> +++ b/arch/arm64/include/asm/kvm_asm.h
> @@ -137,6 +137,8 @@ extern char __restore_vgic_v2_state[];
> extern char __save_vgic_v3_state[];
> extern char __restore_vgic_v3_state[];
>
> +extern u32 __kvm_get_mdcr_el2(void);
> +
> #endif
>
> #endif /* __ARM_KVM_ASM_H__ */
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index f0f58c9..7cb99b5 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -103,6 +103,7 @@ struct kvm_vcpu_arch {
>
> /* HYP configuration */
> u64 hcr_el2;
> + u32 mdcr_el2;
>
> /* Exception Information */
> struct kvm_vcpu_fault_info fault;
> @@ -250,4 +251,8 @@ static inline void kvm_arch_sync_events(struct kvm *kvm) {}
> static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
> static inline void kvm_arch_sched_in(struct kvm_vcpu *vcpu, int cpu) {}
>
> +void kvm_arm_init_debug(void);
> +void kvm_arm_setup_debug(struct kvm_vcpu *vcpu);
> +void kvm_arm_clear_debug(struct kvm_vcpu *vcpu);
> +
> #endif /* __ARM64_KVM_HOST_H__ */
> diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
> index da675cc..dfb25a2 100644
> --- a/arch/arm64/kernel/asm-offsets.c
> +++ b/arch/arm64/kernel/asm-offsets.c
> @@ -117,6 +117,7 @@ int main(void)
> DEFINE(VCPU_HPFAR_EL2, offsetof(struct kvm_vcpu, arch.fault.hpfar_el2));
> DEFINE(VCPU_DEBUG_FLAGS, offsetof(struct kvm_vcpu, arch.debug_flags));
> DEFINE(VCPU_HCR_EL2, offsetof(struct kvm_vcpu, arch.hcr_el2));
> + DEFINE(VCPU_MDCR_EL2, offsetof(struct kvm_vcpu, arch.mdcr_el2));
> DEFINE(VCPU_IRQ_LINES, offsetof(struct kvm_vcpu, arch.irq_lines));
> DEFINE(VCPU_HOST_CONTEXT, offsetof(struct kvm_vcpu, arch.host_cpu_context));
> DEFINE(VCPU_TIMER_CNTV_CTL, offsetof(struct kvm_vcpu, arch.timer_cpu.cntv_ctl));
> diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
> index d5904f8..90e3f39 100644
> --- a/arch/arm64/kvm/Makefile
> +++ b/arch/arm64/kvm/Makefile
> @@ -17,7 +17,7 @@ kvm-$(CONFIG_KVM_ARM_HOST) += $(ARM)/psci.o $(ARM)/perf.o
>
> kvm-$(CONFIG_KVM_ARM_HOST) += emulate.o inject_fault.o regmap.o
> kvm-$(CONFIG_KVM_ARM_HOST) += hyp.o hyp-init.o handle_exit.o
> -kvm-$(CONFIG_KVM_ARM_HOST) += guest.o reset.o sys_regs.o sys_regs_generic_v8.o
> +kvm-$(CONFIG_KVM_ARM_HOST) += guest.o debug.o reset.o sys_regs.o sys_regs_generic_v8.o
>
> kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic.o
> kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic-v2.o
> diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
> new file mode 100644
> index 0000000..b1f8731
> --- /dev/null
> +++ b/arch/arm64/kvm/debug.c
> @@ -0,0 +1,83 @@
> +/*
> + * Debug and Guest Debug support
> + *
> + * Copyright (C) 2015 - Linaro Ltd
> + * Author: Alex Bennée <alex.bennee@linaro.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/kvm_host.h>
> +
> +#include <asm/kvm_arm.h>
> +
> +static DEFINE_PER_CPU(u32, mdcr_el2);
> +
> +/**
> + * kvm_arm_init_debug - grab what we need for debug
> + *
> + * Currently the sole task of this function is to retrieve the initial
> + * value of mdcr_el2 so we can preserve MDCR_EL2.HPMN which has
> + * presumably been set-up by some knowledgeable bootcode.
> + *
> + * It is called once per-cpu during CPU hyp initialisation.
> + */
> +
> +void kvm_arm_init_debug(void)
> +{
> + __this_cpu_write(mdcr_el2, kvm_call_hyp(__kvm_get_mdcr_el2));
> +}
> +
> +
> +/**
> + * kvm_arm_setup_debug - set up debug related stuff
> + *
> + * @vcpu: the vcpu pointer
> + *
> + * This is called before each entry into the hypervisor to setup any
> + * debug related registers. Currently this just ensures we will trap
> + * access to:
> + * - Performance monitors (MDCR_EL2_TPM/MDCR_EL2_TPMCR)
> + * - Debug ROM Address (MDCR_EL2_TDRA)
> + * - Power down debug registers (MDCR_EL2_TDOSA)
TDOSA traps more than the DBGPRCR_EL1 register, so "OS-related
registers" is probably what you want here.
> + *
> + * Additionally, KVM only traps guest accesses to the debug registers if
> + * the guest is not actively using them (see the KVM_ARM64_DEBUG_DIRTY
> + * flag on vcpu->arch.debug_flags). Since the guest must not interfere
> + * with the hardware state when debugging the guest, we must ensure that
> + * trapping is enabled whenever we are debugging the guest using the
> + * debug registers.
> + */
> +
> +void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
> +{
> + bool trap_debug = !(vcpu->arch.debug_flags & KVM_ARM64_DEBUG_DIRTY);
> +
> + vcpu->arch.mdcr_el2 = __this_cpu_read(mdcr_el2) & MDCR_EL2_HPMN_MASK;
> + vcpu->arch.mdcr_el2 |= (MDCR_EL2_TPM |
> + MDCR_EL2_TPMCR |
> + MDCR_EL2_TDRA |
> + MDCR_EL2_TDOSA);
> +
> + /* Trap on access to debug registers? */
> + if (trap_debug)
> + vcpu->arch.mdcr_el2 |= MDCR_EL2_TDA;
> + else
> + vcpu->arch.mdcr_el2 &= ~MDCR_EL2_TDA;
the else-clause shouldn't be necessary as you've just initialized the
register with the HPMN_MASK.
> +
> +}
> +
> +void kvm_arm_clear_debug(struct kvm_vcpu *vcpu)
> +{
> + /* Nothing to do yet */
> +}
> diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
> index 5befd01..15159aa 100644
> --- a/arch/arm64/kvm/hyp.S
> +++ b/arch/arm64/kvm/hyp.S
> @@ -768,17 +768,8 @@
> mov x2, #(1 << 15) // Trap CP15 Cr=15
> msr hstr_el2, x2
>
> - mrs x2, mdcr_el2
> - and x2, x2, #MDCR_EL2_HPMN_MASK
> - orr x2, x2, #(MDCR_EL2_TPM | MDCR_EL2_TPMCR)
> - orr x2, x2, #(MDCR_EL2_TDRA | MDCR_EL2_TDOSA)
> -
> - // Check for KVM_ARM64_DEBUG_DIRTY, and set debug to trap
> - // if not dirty.
> - ldr x3, [x0, #VCPU_DEBUG_FLAGS]
> - tbnz x3, #KVM_ARM64_DEBUG_DIRTY_SHIFT, 1f
> - orr x2, x2, #MDCR_EL2_TDA
> -1:
> + // Monitor Debug Config - see kvm_arch_setup_debug()
kvm_arm_setup_debug() ?
> + ldr x2, [x0, #VCPU_MDCR_EL2]
> msr mdcr_el2, x2
> .endm
>
> @@ -1295,4 +1286,10 @@ ENTRY(__kvm_hyp_vector)
> ventry el1_error_invalid // Error 32-bit EL1
> ENDPROC(__kvm_hyp_vector)
>
> +
> +ENTRY(__kvm_get_mdcr_el2)
> + mrs x0, mdcr_el2
> + ret
> +ENDPROC(__kvm_get_mdcr_el2)
> +
> .popsection
> --
> 2.3.5
>
Thanks,
-Christoffer
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 06/12] KVM: arm64: guest debug, add SW break point support
2015-05-06 16:23 ` [PATCH v3 06/12] KVM: arm64: guest debug, add SW break point support Alex Bennée
@ 2015-05-08 11:52 ` Christoffer Dall
0 siblings, 0 replies; 24+ messages in thread
From: Christoffer Dall @ 2015-05-08 11:52 UTC (permalink / raw)
To: Alex Bennée
Cc: kvm, linux-arm-kernel, kvmarm, marc.zyngier, peter.maydell,
agraf, drjones, pbonzini, zhichao.huang, jan.kiszka, dahi,
r65777, bp, Gleb Natapov, Jonathan Corbet, Russell King,
Catalin Marinas, Will Deacon, open list:DOCUMENTATION, open list
On Wed, May 06, 2015 at 05:23:21PM +0100, Alex Bennée wrote:
> This adds support for SW breakpoints inserted by userspace.
>
> We do this by trapping all guest software debug exceptions to the
> hypervisor (MDCR_EL2.TDE). The exit handler sets an exit reason of
> KVM_EXIT_DEBUG with the kvm_debug_exit_arch structure holding the
> exception syndrome information.
>
> It will be up to userspace to extract the PC (via GET_ONE_REG) and
> determine if the debug event was for a breakpoint it inserted. If not
> userspace will need to re-inject the correct exception restart the
> hypervisor to deliver the debug exception to the guest.
>
> Any other guest software debug exception (e.g. single step or HW
> assisted breakpoints) will cause an error and the VM to be killed. This
> is addressed by later patches which add support for the other debug
> types.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> ---
> v2
> - update to use new exit struct
> - tweak for C setup
> - do our setup in debug_setup/clear code
> - fixed up comments
> v3:
> - fix spacing in KVM_GUESTDBG_VALID_MASK
> - fix and clarify wording on kvm_handle_guest_debug
> - handle error case in kvm_handle_guest_debug
> - re-word the commit message
>
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index 4b0132f..5ef937c 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -2667,7 +2667,7 @@ when running. Common control bits are:
> The top 16 bits of the control field are architecture specific control
> flags which can include the following:
>
> - - KVM_GUESTDBG_USE_SW_BP: using software breakpoints [x86]
> + - KVM_GUESTDBG_USE_SW_BP: using software breakpoints [x86, arm64]
> - KVM_GUESTDBG_USE_HW_BP: using hardware breakpoints [x86, s390]
> - KVM_GUESTDBG_INJECT_DB: inject DB type exception [x86]
> - KVM_GUESTDBG_INJECT_BP: inject BP type exception [x86]
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index 4a274e1..064c105 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -302,7 +302,7 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
> kvm_arm_set_running_vcpu(NULL);
> }
>
> -#define KVM_GUESTDBG_VALID_MASK (KVM_GUESTDBG_ENABLE)
> +#define KVM_GUESTDBG_VALID_MASK (KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP)
>
> /**
> * kvm_arch_vcpu_ioctl_set_guest_debug - set up guest debugging
> diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
> index b1f8731..5bee676 100644
> --- a/arch/arm64/kvm/debug.c
> +++ b/arch/arm64/kvm/debug.c
> @@ -75,6 +75,12 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
> else
> vcpu->arch.mdcr_el2 &= ~MDCR_EL2_TDA;
>
> + /* Trap breakpoints? */
> + if (vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP)
> + vcpu->arch.mdcr_el2 |= MDCR_EL2_TDE;
> + else
> + vcpu->arch.mdcr_el2 &= ~MDCR_EL2_TDE;
> +
I also don't think you need the else-clause here.
> }
>
> void kvm_arm_clear_debug(struct kvm_vcpu *vcpu)
> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
> index 524fa25..27f38a9 100644
> --- a/arch/arm64/kvm/handle_exit.c
> +++ b/arch/arm64/kvm/handle_exit.c
> @@ -82,6 +82,40 @@ static int kvm_handle_wfx(struct kvm_vcpu *vcpu, struct kvm_run *run)
> return 1;
> }
>
> +/**
> + * kvm_handle_guest_debug - handle a debug exception instruction
> + *
> + * @vcpu: the vcpu pointer
> + * @run: access to the kvm_run structure for results
> + *
> + * We route all debug exceptions through the same handler. If both the
> + * guest and host are using the same debug facilities it will be up to
> + * userspace to re-inject the correct exception for guest delivery.
> + *
> + * @return: 0 (while setting run->exit_reason), -1 for error
> + */
> +static int kvm_handle_guest_debug(struct kvm_vcpu *vcpu, struct kvm_run *run)
> +{
> + u32 hsr = kvm_vcpu_get_hsr(vcpu);
> + int ret = 0;
> +
> + run->exit_reason = KVM_EXIT_DEBUG;
> + run->debug.arch.hsr = hsr;
> +
> + switch (hsr >> ESR_ELx_EC_SHIFT) {
> + case ESR_ELx_EC_BKPT32:
> + case ESR_ELx_EC_BRK64:
> + break;
> + default:
> + kvm_err("%s: un-handled case hsr: %#08x\n",
> + __func__, (unsigned int) hsr);
> + ret = -1;
> + break;
> + }
> +
> + return ret;
> +}
> +
> static exit_handle_fn arm_exit_handlers[] = {
> [ESR_ELx_EC_WFx] = kvm_handle_wfx,
> [ESR_ELx_EC_CP15_32] = kvm_handle_cp15_32,
> @@ -96,6 +130,8 @@ static exit_handle_fn arm_exit_handlers[] = {
> [ESR_ELx_EC_SYS64] = kvm_handle_sys_reg,
> [ESR_ELx_EC_IABT_LOW] = kvm_handle_guest_abort,
> [ESR_ELx_EC_DABT_LOW] = kvm_handle_guest_abort,
> + [ESR_ELx_EC_BKPT32] = kvm_handle_guest_debug,
> + [ESR_ELx_EC_BRK64] = kvm_handle_guest_debug,
> };
>
> static exit_handle_fn kvm_get_exit_handler(struct kvm_vcpu *vcpu)
> --
> 2.3.5
>
Besides the nit:
Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 07/12] KVM: arm64: guest debug, add support for single-step
2015-05-06 16:23 ` [PATCH v3 07/12] KVM: arm64: guest debug, add support for single-step Alex Bennée
@ 2015-05-08 11:52 ` Christoffer Dall
0 siblings, 0 replies; 24+ messages in thread
From: Christoffer Dall @ 2015-05-08 11:52 UTC (permalink / raw)
To: Alex Bennée
Cc: kvm, linux-arm-kernel, kvmarm, marc.zyngier, peter.maydell,
agraf, drjones, pbonzini, zhichao.huang, jan.kiszka, dahi,
r65777, bp, Gleb Natapov, Russell King, Catalin Marinas,
Will Deacon, open list
On Wed, May 06, 2015 at 05:23:22PM +0100, Alex Bennée wrote:
> This adds support for single-stepping the guest. To do this we need to
> manipulate the guests PSTATE.SS and MDSCR_EL1.SS bits which we do in the
> kvm_arm_setup/clear_debug() so we don't affect the apparent state of the
> guest. Additionally while the host is debugging the guest we suppress
> the ability of the guest to single-step itself.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> ---
> v2
> - Move pstate/mdscr manipulation into C
> - don't export guest_debug to assembly
> - add accessor for saved_debug regs
> - tweak save/restore of mdscr_el1
> v3
> - don't save PC in debug information struct
> - rename debug_saved_regs->guest_debug_state
> - save whole value, only use bits in restore
> - add save/restore_guest-debug_regs helper functions
> - simplify commit message for clarity
> - rm vcpu_debug_saved_reg access fn
>
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index 064c105..9b3ed6d 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -302,7 +302,9 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
> kvm_arm_set_running_vcpu(NULL);
> }
>
> -#define KVM_GUESTDBG_VALID_MASK (KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP)
> +#define KVM_GUESTDBG_VALID_MASK (KVM_GUESTDBG_ENABLE | \
> + KVM_GUESTDBG_USE_SW_BP | \
> + KVM_GUESTDBG_SINGLESTEP)
>
> /**
> * kvm_arch_vcpu_ioctl_set_guest_debug - set up guest debugging
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 7cb99b5..b60fa7a 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -123,6 +123,12 @@ struct kvm_vcpu_arch {
> * here.
> */
>
> + /* Guest registers we preserve during guest debugging */
> + struct {
> + u32 pstate;
This could do a with a comment: /* preserve SPSR_DEBUG_MASK bits */
> + u32 mdscr_el1;
> + } guest_debug_state;
> +
> /* Don't run the guest */
> bool pause;
>
> diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
> index 5bee676..19346e8 100644
> --- a/arch/arm64/kvm/debug.c
> +++ b/arch/arm64/kvm/debug.c
> @@ -19,11 +19,42 @@
>
> #include <linux/kvm_host.h>
>
> +#include <asm/debug-monitors.h>
> +#include <asm/kvm_asm.h>
> #include <asm/kvm_arm.h>
> +#include <asm/kvm_emulate.h>
> +
> +/* These are the bits of MDSCR_EL1 we may manipulate */
> +#define MDSCR_EL1_DEBUG_MASK (DBG_MDSCR_SS | \
> + DBG_MDSCR_KDE | \
> + DBG_MDSCR_MDE)
> +
> +#define SPSR_DEBUG_MASK DBG_SPSR_SS
>
> static DEFINE_PER_CPU(u32, mdcr_el2);
>
> /**
> + * save/restore_guest_debug_regs
> + *
> + * For some debug operations we need to tweak some guest registers. As
> + * a result we need to save the state of those registers before we
> + * make those modifications.
> + */
> +static void save_guest_debug_regs(struct kvm_vcpu *vcpu)
> +{
> + vcpu->arch.guest_debug_state.pstate = *vcpu_cpsr(vcpu);
> + vcpu->arch.guest_debug_state.mdscr_el1 = vcpu_sys_reg(vcpu, MDSCR_EL1);
> +}
> +
> +static void restore_guest_debug_regs(struct kvm_vcpu *vcpu)
> +{
> + *vcpu_cpsr(vcpu) |=
> + (vcpu->arch.guest_debug_state.pstate & SPSR_DEBUG_MASK);
> + vcpu_sys_reg(vcpu, MDSCR_EL1) |=
> + (vcpu->arch.guest_debug_state.mdscr_el1 & MDSCR_EL1_DEBUG_MASK);
This doesn't look right. Don't you need to also clear the values if
they were set by us for single-stepping the guest? At least for the
MDSCR_EL1.SS bit.
What if we're single-stepping through guest code that modifies the SS bits
of these register for the guest state? Is that possible and do we capture
this somehow?
> +}
> +
> +/**
> * kvm_arm_init_debug - grab what we need for debug
> *
> * Currently the sole task of this function is to retrieve the initial
> @@ -38,7 +69,6 @@ void kvm_arm_init_debug(void)
> __this_cpu_write(mdcr_el2, kvm_call_hyp(__kvm_get_mdcr_el2));
> }
>
> -
> /**
> * kvm_arm_setup_debug - set up debug related stuff
> *
> @@ -75,15 +105,37 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
> else
> vcpu->arch.mdcr_el2 &= ~MDCR_EL2_TDA;
>
> - /* Trap breakpoints? */
> - if (vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP)
> + /* Is Guest debugging in effect? */
> + if (vcpu->guest_debug) {
you could have just checked the field like this in the original patch,
but ok.
> vcpu->arch.mdcr_el2 |= MDCR_EL2_TDE;
> - else
> - vcpu->arch.mdcr_el2 &= ~MDCR_EL2_TDE;
>
> + /* Save guest debug state */
> + save_guest_debug_regs(vcpu);
> +
> + /*
> + * Single Step (ARM ARM D2.12.3 The software step state
> + * machine)
> + *
> + * If we are doing Single Step we need to manipulate
> + * MDSCR_EL1.SS and PSTATE.SS. If not we need to
> + * suppress the guests ability to trigger single step itself.
> + */
> + if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) {
> + *vcpu_cpsr(vcpu) |= DBG_SPSR_SS;
> + vcpu_sys_reg(vcpu, MDSCR_EL1) |= DBG_MDSCR_SS;
> + } else {
> + *vcpu_cpsr(vcpu) &= ~DBG_SPSR_SS;
why must we clear PSTATE.SS when we have MDSCR_EL1.SS == 0 ?
> + vcpu_sys_reg(vcpu, MDSCR_EL1) &= ~DBG_MDSCR_SS;
> + }
> +
> + } else {
> + /* Debug operations can go straight to the guest */
> + vcpu->arch.mdcr_el2 &= ~MDCR_EL2_TDE;
still don't think you need this.
> + }
> }
>
> void kvm_arm_clear_debug(struct kvm_vcpu *vcpu)
> {
> - /* Nothing to do yet */
> + if (vcpu->guest_debug)
> + restore_guest_debug_regs(vcpu);
> }
> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
> index 27f38a9..e9de13e 100644
> --- a/arch/arm64/kvm/handle_exit.c
> +++ b/arch/arm64/kvm/handle_exit.c
> @@ -103,6 +103,7 @@ static int kvm_handle_guest_debug(struct kvm_vcpu *vcpu, struct kvm_run *run)
> run->debug.arch.hsr = hsr;
>
> switch (hsr >> ESR_ELx_EC_SHIFT) {
> + case ESR_ELx_EC_SOFTSTP_LOW:
> case ESR_ELx_EC_BKPT32:
> case ESR_ELx_EC_BRK64:
> break;
> @@ -130,6 +131,7 @@ static exit_handle_fn arm_exit_handlers[] = {
> [ESR_ELx_EC_SYS64] = kvm_handle_sys_reg,
> [ESR_ELx_EC_IABT_LOW] = kvm_handle_guest_abort,
> [ESR_ELx_EC_DABT_LOW] = kvm_handle_guest_abort,
> + [ESR_ELx_EC_SOFTSTP_LOW]= kvm_handle_guest_debug,
> [ESR_ELx_EC_BKPT32] = kvm_handle_guest_debug,
> [ESR_ELx_EC_BRK64] = kvm_handle_guest_debug,
> };
> --
> 2.3.5
>
Thanks,
-Christoffer
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 08/12] KVM: arm64: re-factor hyp.S debug register code
2015-05-07 9:07 ` [PATCH v3 08/12] KVM: arm64: re-factor hyp.S debug register code Alex Bennée
@ 2015-05-08 14:12 ` Christoffer Dall
0 siblings, 0 replies; 24+ messages in thread
From: Christoffer Dall @ 2015-05-08 14:12 UTC (permalink / raw)
To: Alex Bennée
Cc: kvm, linux-arm-kernel, kvmarm, marc.zyngier, peter.maydell,
agraf, drjones, pbonzini, zhichao.huang, jan.kiszka, dahi,
r65777, bp, Catalin Marinas, Will Deacon, Gleb Natapov,
Ard Biesheuvel, Andre Przywara, Richard Weinberger,
Lorenzo Pieralisi, open list
On Thu, May 07, 2015 at 10:07:11AM +0100, Alex Bennée wrote:
> This is a pre-cursor to sharing the code with the guest debug support.
> This replaces the big macro that fishes data out of a fixed location
> with a more general helper macro to restore a set of debug registers. It
> uses macro substitution so it can be re-used for debug control and value
> registers. It does however rely on the debug registers being 64 bit
> aligned (as they happen to be in the hyp ABI).
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> ---
> v3:
> - return to the patch series
> - add save and restore targets
> - change register use and document
>
> diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
> index dfb25a2..ce7b7dd 100644
> --- a/arch/arm64/kernel/asm-offsets.c
> +++ b/arch/arm64/kernel/asm-offsets.c
> @@ -116,6 +116,10 @@ int main(void)
> DEFINE(VCPU_FAR_EL2, offsetof(struct kvm_vcpu, arch.fault.far_el2));
> DEFINE(VCPU_HPFAR_EL2, offsetof(struct kvm_vcpu, arch.fault.hpfar_el2));
> DEFINE(VCPU_DEBUG_FLAGS, offsetof(struct kvm_vcpu, arch.debug_flags));
> + DEFINE(DEBUG_BCR, offsetof(struct kvm_guest_debug_arch, dbg_bcr));
> + DEFINE(DEBUG_BVR, offsetof(struct kvm_guest_debug_arch, dbg_bvr));
> + DEFINE(DEBUG_WCR, offsetof(struct kvm_guest_debug_arch, dbg_wcr));
> + DEFINE(DEBUG_WVR, offsetof(struct kvm_guest_debug_arch, dbg_wvr));
> DEFINE(VCPU_HCR_EL2, offsetof(struct kvm_vcpu, arch.hcr_el2));
> DEFINE(VCPU_MDCR_EL2, offsetof(struct kvm_vcpu, arch.mdcr_el2));
> DEFINE(VCPU_IRQ_LINES, offsetof(struct kvm_vcpu, arch.irq_lines));
> diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
> index 15159aa..dd51fb1 100644
> --- a/arch/arm64/kvm/hyp.S
> +++ b/arch/arm64/kvm/hyp.S
> @@ -228,199 +228,52 @@
> stp x24, x25, [x3, #160]
> .endm
>
> -.macro save_debug
> - // x2: base address for cpu context
> - // x3: tmp register
> -
> - mrs x26, id_aa64dfr0_el1
> - ubfx x24, x26, #12, #4 // Extract BRPs
> - ubfx x25, x26, #20, #4 // Extract WRPs
> - mov w26, #15
> - sub w24, w26, w24 // How many BPs to skip
> - sub w25, w26, w25 // How many WPs to skip
> -
> - add x3, x2, #CPU_SYSREG_OFFSET(DBGBCR0_EL1)
> -
> - adr x26, 1f
> - add x26, x26, x24, lsl #2
> - br x26
> -1:
> - mrs x20, dbgbcr15_el1
> - mrs x19, dbgbcr14_el1
> - mrs x18, dbgbcr13_el1
> - mrs x17, dbgbcr12_el1
> - mrs x16, dbgbcr11_el1
> - mrs x15, dbgbcr10_el1
> - mrs x14, dbgbcr9_el1
> - mrs x13, dbgbcr8_el1
> - mrs x12, dbgbcr7_el1
> - mrs x11, dbgbcr6_el1
> - mrs x10, dbgbcr5_el1
> - mrs x9, dbgbcr4_el1
> - mrs x8, dbgbcr3_el1
> - mrs x7, dbgbcr2_el1
> - mrs x6, dbgbcr1_el1
> - mrs x5, dbgbcr0_el1
> -
> - adr x26, 1f
> - add x26, x26, x24, lsl #2
> - br x26
> -
> -1:
> - str x20, [x3, #(15 * 8)]
> - str x19, [x3, #(14 * 8)]
> - str x18, [x3, #(13 * 8)]
> - str x17, [x3, #(12 * 8)]
> - str x16, [x3, #(11 * 8)]
> - str x15, [x3, #(10 * 8)]
> - str x14, [x3, #(9 * 8)]
> - str x13, [x3, #(8 * 8)]
> - str x12, [x3, #(7 * 8)]
> - str x11, [x3, #(6 * 8)]
> - str x10, [x3, #(5 * 8)]
> - str x9, [x3, #(4 * 8)]
> - str x8, [x3, #(3 * 8)]
> - str x7, [x3, #(2 * 8)]
> - str x6, [x3, #(1 * 8)]
> - str x5, [x3, #(0 * 8)]
> -
> - add x3, x2, #CPU_SYSREG_OFFSET(DBGBVR0_EL1)
> -
> - adr x26, 1f
> - add x26, x26, x24, lsl #2
> - br x26
> +.macro save_debug_registers type
> + // x4: pointer to register set
> + // x5: number of registers to copy
looking at the caller, you're actually passing the number of registers
to skip?
> + // x6..x22 trashed
> +
> + adr x22, 1f
> + add x22, x22, x5, lsl #2
> + br x22
> 1:
> - mrs x20, dbgbvr15_el1
> - mrs x19, dbgbvr14_el1
> - mrs x18, dbgbvr13_el1
> - mrs x17, dbgbvr12_el1
> - mrs x16, dbgbvr11_el1
> - mrs x15, dbgbvr10_el1
> - mrs x14, dbgbvr9_el1
> - mrs x13, dbgbvr8_el1
> - mrs x12, dbgbvr7_el1
> - mrs x11, dbgbvr6_el1
> - mrs x10, dbgbvr5_el1
> - mrs x9, dbgbvr4_el1
> - mrs x8, dbgbvr3_el1
> - mrs x7, dbgbvr2_el1
> - mrs x6, dbgbvr1_el1
> - mrs x5, dbgbvr0_el1
> -
> - adr x26, 1f
> - add x26, x26, x24, lsl #2
> - br x26
> -
> -1:
> - str x20, [x3, #(15 * 8)]
> - str x19, [x3, #(14 * 8)]
> - str x18, [x3, #(13 * 8)]
> - str x17, [x3, #(12 * 8)]
> - str x16, [x3, #(11 * 8)]
> - str x15, [x3, #(10 * 8)]
> - str x14, [x3, #(9 * 8)]
> - str x13, [x3, #(8 * 8)]
> - str x12, [x3, #(7 * 8)]
> - str x11, [x3, #(6 * 8)]
> - str x10, [x3, #(5 * 8)]
> - str x9, [x3, #(4 * 8)]
> - str x8, [x3, #(3 * 8)]
> - str x7, [x3, #(2 * 8)]
> - str x6, [x3, #(1 * 8)]
> - str x5, [x3, #(0 * 8)]
> -
> - add x3, x2, #CPU_SYSREG_OFFSET(DBGWCR0_EL1)
> -
> - adr x26, 1f
> - add x26, x26, x25, lsl #2
> - br x26
> -1:
> - mrs x20, dbgwcr15_el1
> - mrs x19, dbgwcr14_el1
> - mrs x18, dbgwcr13_el1
> - mrs x17, dbgwcr12_el1
> - mrs x16, dbgwcr11_el1
> - mrs x15, dbgwcr10_el1
> - mrs x14, dbgwcr9_el1
> - mrs x13, dbgwcr8_el1
> - mrs x12, dbgwcr7_el1
> - mrs x11, dbgwcr6_el1
> - mrs x10, dbgwcr5_el1
> - mrs x9, dbgwcr4_el1
> - mrs x8, dbgwcr3_el1
> - mrs x7, dbgwcr2_el1
> - mrs x6, dbgwcr1_el1
> - mrs x5, dbgwcr0_el1
> -
> - adr x26, 1f
> - add x26, x26, x25, lsl #2
> - br x26
> -
> + mrs x21, \type\()15_el1
> + mrs x20, \type\()14_el1
> + mrs x19, \type\()13_el1
> + mrs x18, \type\()12_el1
> + mrs x17, \type\()11_el1
> + mrs x16, \type\()10_el1
> + mrs x15, \type\()9_el1
> + mrs x14, \type\()8_el1
> + mrs x13, \type\()7_el1
> + mrs x12, \type\()6_el1
> + mrs x11, \type\()5_el1
> + mrs x10, \type\()4_el1
> + mrs x9, \type\()3_el1
> + mrs x8, \type\()2_el1
> + mrs x7, \type\()1_el1
> + mrs x6, \type\()0_el1
> +
> + adr x22, 1f
> + add x22, x22, x5, lsl #2
> + br x22
> 1:
> - str x20, [x3, #(15 * 8)]
> - str x19, [x3, #(14 * 8)]
> - str x18, [x3, #(13 * 8)]
> - str x17, [x3, #(12 * 8)]
> - str x16, [x3, #(11 * 8)]
> - str x15, [x3, #(10 * 8)]
> - str x14, [x3, #(9 * 8)]
> - str x13, [x3, #(8 * 8)]
> - str x12, [x3, #(7 * 8)]
> - str x11, [x3, #(6 * 8)]
> - str x10, [x3, #(5 * 8)]
> - str x9, [x3, #(4 * 8)]
> - str x8, [x3, #(3 * 8)]
> - str x7, [x3, #(2 * 8)]
> - str x6, [x3, #(1 * 8)]
> - str x5, [x3, #(0 * 8)]
> -
> - add x3, x2, #CPU_SYSREG_OFFSET(DBGWVR0_EL1)
> -
> - adr x26, 1f
> - add x26, x26, x25, lsl #2
> - br x26
> -1:
> - mrs x20, dbgwvr15_el1
> - mrs x19, dbgwvr14_el1
> - mrs x18, dbgwvr13_el1
> - mrs x17, dbgwvr12_el1
> - mrs x16, dbgwvr11_el1
> - mrs x15, dbgwvr10_el1
> - mrs x14, dbgwvr9_el1
> - mrs x13, dbgwvr8_el1
> - mrs x12, dbgwvr7_el1
> - mrs x11, dbgwvr6_el1
> - mrs x10, dbgwvr5_el1
> - mrs x9, dbgwvr4_el1
> - mrs x8, dbgwvr3_el1
> - mrs x7, dbgwvr2_el1
> - mrs x6, dbgwvr1_el1
> - mrs x5, dbgwvr0_el1
> -
> - adr x26, 1f
> - add x26, x26, x25, lsl #2
> - br x26
> -
> -1:
> - str x20, [x3, #(15 * 8)]
> - str x19, [x3, #(14 * 8)]
> - str x18, [x3, #(13 * 8)]
> - str x17, [x3, #(12 * 8)]
> - str x16, [x3, #(11 * 8)]
> - str x15, [x3, #(10 * 8)]
> - str x14, [x3, #(9 * 8)]
> - str x13, [x3, #(8 * 8)]
> - str x12, [x3, #(7 * 8)]
> - str x11, [x3, #(6 * 8)]
> - str x10, [x3, #(5 * 8)]
> - str x9, [x3, #(4 * 8)]
> - str x8, [x3, #(3 * 8)]
> - str x7, [x3, #(2 * 8)]
> - str x6, [x3, #(1 * 8)]
> - str x5, [x3, #(0 * 8)]
> -
> - mrs x21, mdccint_el1
> - str x21, [x2, #CPU_SYSREG_OFFSET(MDCCINT_EL1)]
> + str x21, [x4, #(15 * 8)]
> + str x20, [x4, #(14 * 8)]
> + str x19, [x4, #(13 * 8)]
> + str x18, [x4, #(12 * 8)]
> + str x17, [x4, #(11 * 8)]
> + str x16, [x4, #(10 * 8)]
> + str x15, [x4, #(9 * 8)]
> + str x14, [x4, #(8 * 8)]
> + str x13, [x4, #(7 * 8)]
> + str x12, [x4, #(6 * 8)]
> + str x11, [x4, #(5 * 8)]
> + str x10, [x4, #(4 * 8)]
> + str x9, [x4, #(3 * 8)]
> + str x8, [x4, #(2 * 8)]
> + str x7, [x4, #(1 * 8)]
> + str x6, [x4, #(0 * 8)]
> .endm
>
> .macro restore_sysregs
> @@ -465,195 +318,52 @@
> msr mdscr_el1, x25
> .endm
>
> -.macro restore_debug
> - // x2: base address for cpu context
> - // x3: tmp register
> -
> - mrs x26, id_aa64dfr0_el1
> - ubfx x24, x26, #12, #4 // Extract BRPs
> - ubfx x25, x26, #20, #4 // Extract WRPs
> - mov w26, #15
> - sub w24, w26, w24 // How many BPs to skip
> - sub w25, w26, w25 // How many WPs to skip
> -
> - add x3, x2, #CPU_SYSREG_OFFSET(DBGBCR0_EL1)
> +.macro setup_debug_registers type
why introduce a new naming scheme and not just stick to
restore_debug_registers?
the way all this code is written it's:
save: read stuff from HW and *save* it in memory
restore: *restore* stuff from memory and write it to HW
> + // x4: pointer to register set
> + // x5: number of registers to copy
ditto: skip/copy
> + // x6..x22 trashed
>
> - adr x26, 1f
> - add x26, x26, x24, lsl #2
> - br x26
> -1:
> - ldr x20, [x3, #(15 * 8)]
> - ldr x19, [x3, #(14 * 8)]
> - ldr x18, [x3, #(13 * 8)]
> - ldr x17, [x3, #(12 * 8)]
> - ldr x16, [x3, #(11 * 8)]
> - ldr x15, [x3, #(10 * 8)]
> - ldr x14, [x3, #(9 * 8)]
> - ldr x13, [x3, #(8 * 8)]
> - ldr x12, [x3, #(7 * 8)]
> - ldr x11, [x3, #(6 * 8)]
> - ldr x10, [x3, #(5 * 8)]
> - ldr x9, [x3, #(4 * 8)]
> - ldr x8, [x3, #(3 * 8)]
> - ldr x7, [x3, #(2 * 8)]
> - ldr x6, [x3, #(1 * 8)]
> - ldr x5, [x3, #(0 * 8)]
> -
> - adr x26, 1f
> - add x26, x26, x24, lsl #2
> - br x26
> + adr x22, 1f
> + add x22, x22, x5, lsl #2
> + br x22
> 1:
> - msr dbgbcr15_el1, x20
> - msr dbgbcr14_el1, x19
> - msr dbgbcr13_el1, x18
> - msr dbgbcr12_el1, x17
> - msr dbgbcr11_el1, x16
> - msr dbgbcr10_el1, x15
> - msr dbgbcr9_el1, x14
> - msr dbgbcr8_el1, x13
> - msr dbgbcr7_el1, x12
> - msr dbgbcr6_el1, x11
> - msr dbgbcr5_el1, x10
> - msr dbgbcr4_el1, x9
> - msr dbgbcr3_el1, x8
> - msr dbgbcr2_el1, x7
> - msr dbgbcr1_el1, x6
> - msr dbgbcr0_el1, x5
> -
> - add x3, x2, #CPU_SYSREG_OFFSET(DBGBVR0_EL1)
> -
> - adr x26, 1f
> - add x26, x26, x24, lsl #2
> - br x26
> + ldr x21, [x4, #(15 * 8)]
> + ldr x20, [x4, #(14 * 8)]
> + ldr x19, [x4, #(13 * 8)]
> + ldr x18, [x4, #(12 * 8)]
> + ldr x17, [x4, #(11 * 8)]
> + ldr x16, [x4, #(10 * 8)]
> + ldr x15, [x4, #(9 * 8)]
> + ldr x14, [x4, #(8 * 8)]
> + ldr x13, [x4, #(7 * 8)]
> + ldr x12, [x4, #(6 * 8)]
> + ldr x11, [x4, #(5 * 8)]
> + ldr x10, [x4, #(4 * 8)]
> + ldr x9, [x4, #(3 * 8)]
> + ldr x8, [x4, #(2 * 8)]
> + ldr x7, [x4, #(1 * 8)]
> + ldr x6, [x4, #(0 * 8)]
> +
> + adr x22, 1f
> + add x22, x22, x5, lsl #2
> + br x22
> 1:
> - ldr x20, [x3, #(15 * 8)]
> - ldr x19, [x3, #(14 * 8)]
> - ldr x18, [x3, #(13 * 8)]
> - ldr x17, [x3, #(12 * 8)]
> - ldr x16, [x3, #(11 * 8)]
> - ldr x15, [x3, #(10 * 8)]
> - ldr x14, [x3, #(9 * 8)]
> - ldr x13, [x3, #(8 * 8)]
> - ldr x12, [x3, #(7 * 8)]
> - ldr x11, [x3, #(6 * 8)]
> - ldr x10, [x3, #(5 * 8)]
> - ldr x9, [x3, #(4 * 8)]
> - ldr x8, [x3, #(3 * 8)]
> - ldr x7, [x3, #(2 * 8)]
> - ldr x6, [x3, #(1 * 8)]
> - ldr x5, [x3, #(0 * 8)]
> -
> - adr x26, 1f
> - add x26, x26, x24, lsl #2
> - br x26
> -1:
> - msr dbgbvr15_el1, x20
> - msr dbgbvr14_el1, x19
> - msr dbgbvr13_el1, x18
> - msr dbgbvr12_el1, x17
> - msr dbgbvr11_el1, x16
> - msr dbgbvr10_el1, x15
> - msr dbgbvr9_el1, x14
> - msr dbgbvr8_el1, x13
> - msr dbgbvr7_el1, x12
> - msr dbgbvr6_el1, x11
> - msr dbgbvr5_el1, x10
> - msr dbgbvr4_el1, x9
> - msr dbgbvr3_el1, x8
> - msr dbgbvr2_el1, x7
> - msr dbgbvr1_el1, x6
> - msr dbgbvr0_el1, x5
> -
> - add x3, x2, #CPU_SYSREG_OFFSET(DBGWCR0_EL1)
> -
> - adr x26, 1f
> - add x26, x26, x25, lsl #2
> - br x26
> -1:
> - ldr x20, [x3, #(15 * 8)]
> - ldr x19, [x3, #(14 * 8)]
> - ldr x18, [x3, #(13 * 8)]
> - ldr x17, [x3, #(12 * 8)]
> - ldr x16, [x3, #(11 * 8)]
> - ldr x15, [x3, #(10 * 8)]
> - ldr x14, [x3, #(9 * 8)]
> - ldr x13, [x3, #(8 * 8)]
> - ldr x12, [x3, #(7 * 8)]
> - ldr x11, [x3, #(6 * 8)]
> - ldr x10, [x3, #(5 * 8)]
> - ldr x9, [x3, #(4 * 8)]
> - ldr x8, [x3, #(3 * 8)]
> - ldr x7, [x3, #(2 * 8)]
> - ldr x6, [x3, #(1 * 8)]
> - ldr x5, [x3, #(0 * 8)]
> -
> - adr x26, 1f
> - add x26, x26, x25, lsl #2
> - br x26
> -1:
> - msr dbgwcr15_el1, x20
> - msr dbgwcr14_el1, x19
> - msr dbgwcr13_el1, x18
> - msr dbgwcr12_el1, x17
> - msr dbgwcr11_el1, x16
> - msr dbgwcr10_el1, x15
> - msr dbgwcr9_el1, x14
> - msr dbgwcr8_el1, x13
> - msr dbgwcr7_el1, x12
> - msr dbgwcr6_el1, x11
> - msr dbgwcr5_el1, x10
> - msr dbgwcr4_el1, x9
> - msr dbgwcr3_el1, x8
> - msr dbgwcr2_el1, x7
> - msr dbgwcr1_el1, x6
> - msr dbgwcr0_el1, x5
> -
> - add x3, x2, #CPU_SYSREG_OFFSET(DBGWVR0_EL1)
> -
> - adr x26, 1f
> - add x26, x26, x25, lsl #2
> - br x26
> -1:
> - ldr x20, [x3, #(15 * 8)]
> - ldr x19, [x3, #(14 * 8)]
> - ldr x18, [x3, #(13 * 8)]
> - ldr x17, [x3, #(12 * 8)]
> - ldr x16, [x3, #(11 * 8)]
> - ldr x15, [x3, #(10 * 8)]
> - ldr x14, [x3, #(9 * 8)]
> - ldr x13, [x3, #(8 * 8)]
> - ldr x12, [x3, #(7 * 8)]
> - ldr x11, [x3, #(6 * 8)]
> - ldr x10, [x3, #(5 * 8)]
> - ldr x9, [x3, #(4 * 8)]
> - ldr x8, [x3, #(3 * 8)]
> - ldr x7, [x3, #(2 * 8)]
> - ldr x6, [x3, #(1 * 8)]
> - ldr x5, [x3, #(0 * 8)]
> -
> - adr x26, 1f
> - add x26, x26, x25, lsl #2
> - br x26
> -1:
> - msr dbgwvr15_el1, x20
> - msr dbgwvr14_el1, x19
> - msr dbgwvr13_el1, x18
> - msr dbgwvr12_el1, x17
> - msr dbgwvr11_el1, x16
> - msr dbgwvr10_el1, x15
> - msr dbgwvr9_el1, x14
> - msr dbgwvr8_el1, x13
> - msr dbgwvr7_el1, x12
> - msr dbgwvr6_el1, x11
> - msr dbgwvr5_el1, x10
> - msr dbgwvr4_el1, x9
> - msr dbgwvr3_el1, x8
> - msr dbgwvr2_el1, x7
> - msr dbgwvr1_el1, x6
> - msr dbgwvr0_el1, x5
> -
> - ldr x21, [x2, #CPU_SYSREG_OFFSET(MDCCINT_EL1)]
> - msr mdccint_el1, x21
> + msr \type\()15_el1, x21
> + msr \type\()14_el1, x20
> + msr \type\()13_el1, x19
> + msr \type\()12_el1, x18
> + msr \type\()11_el1, x17
> + msr \type\()10_el1, x16
> + msr \type\()9_el1, x15
> + msr \type\()8_el1, x14
> + msr \type\()7_el1, x13
> + msr \type\()6_el1, x12
> + msr \type\()5_el1, x11
> + msr \type\()4_el1, x10
> + msr \type\()3_el1, x9
> + msr \type\()2_el1, x8
> + msr \type\()1_el1, x7
> + msr \type\()0_el1, x6
> .endm
>
> .macro skip_32bit_state tmp, target
> @@ -887,12 +597,65 @@ __restore_sysregs:
> restore_sysregs
> ret
>
> +/* Save debug state */
> __save_debug:
> - save_debug
> + // x0: base address for vcpu context
why do we need the vcpu context here?
> + // x2: ptr to current CPU context
what does 'current' mean here?
> + // x3: ptr to debug registers
> + // x4/x5: trashed
you're also trashing everything that save_debug_registers is...
> +
> + mrs x26, id_aa64dfr0_el1
> + ubfx x24, x26, #12, #4 // Extract BRPs
> + ubfx x25, x26, #20, #4 // Extract WRPs
> + mov w26, #15
> + sub w24, w26, w24 // How many BPs to skip
> + sub w25, w26, w25 // How many WPs to skip
> +
> + mov x5, x24
> + add x4, x3, #DEBUG_BCR
> + save_debug_registers dbgbcr
> + add x4, x3, #DEBUG_BVR
> + save_debug_registers dbgbvr
> +
> + mov x5, x25
> + add x4, x3, #DEBUG_WCR
> + save_debug_registers dbgwcr
> + add x4, x3, #DEBUG_WVR
> + save_debug_registers dbgwvr
> +
> + mrs x21, mdccint_el1
> + str x21, [x2, #CPU_SYSREG_OFFSET(MDCCINT_EL1)]
> ret
>
> +/* Restore debug state */
> __restore_debug:
> - restore_debug
> + // x0: base address for cpu context
same as above (also not here you say cpu context, above you say vcpu
context, this is confusing)
> + // x2: ptr to current CPU context
same question with 'current' ?
> + // x3: ptr to debug registers
> + // x4/x5: trashed
same trash, you probably want to specify x4..x22 (note that you're
explicitly also touching x21 below).
> +
> + mrs x26, id_aa64dfr0_el1
> + ubfx x24, x26, #12, #4 // Extract BRPs
> + ubfx x25, x26, #20, #4 // Extract WRPs
> + mov w26, #15
> + sub w24, w26, w24 // How many BPs to skip
> + sub w25, w26, w25 // How many WPs to skip
> +
> + mov x5, x24
> + add x4, x3, #DEBUG_BCR
> + setup_debug_registers dbgbcr
> + add x4, x3, #DEBUG_BVR
> + setup_debug_registers dbgbvr
> +
> + mov x5, x25
> + add x4, x3, #DEBUG_WCR
> + setup_debug_registers dbgwcr
> + add x4, x3, #DEBUG_WVR
> + setup_debug_registers dbgwvr
> +
> + ldr x21, [x2, #CPU_SYSREG_OFFSET(MDCCINT_EL1)]
> + msr mdccint_el1, x21
> +
> ret
>
> __save_fpsimd:
> @@ -927,6 +690,7 @@ ENTRY(__kvm_vcpu_run)
> bl __save_sysregs
>
> compute_debug_state 1f
> + add x3, x2, #CPU_SYSREG_OFFSET(DBGBCR0_EL1)
> bl __save_debug
> 1:
> activate_traps
> @@ -942,6 +706,7 @@ ENTRY(__kvm_vcpu_run)
> bl __restore_fpsimd
>
> skip_debug_state x3, 1f
> + add x3, x2, #CPU_SYSREG_OFFSET(DBGBCR0_EL1)
why is this sysreg cpu offset the debug registers that you can apply
offsets of kvm_guest_debug_arch to?
> bl __restore_debug
> 1:
> restore_guest_32bit_state
> @@ -962,6 +727,7 @@ __kvm_vcpu_return:
> bl __save_sysregs
>
> skip_debug_state x3, 1f
> + add x3, x2, #CPU_SYSREG_OFFSET(DBGBCR0_EL1)
> bl __save_debug
> 1:
> save_guest_32bit_state
> @@ -984,6 +750,7 @@ __kvm_vcpu_return:
> // already been saved. Note that we nuke the whole 64bit word.
> // If we ever add more flags, we'll have to be more careful...
> str xzr, [x0, #VCPU_DEBUG_FLAGS]
> + add x3, x2, #CPU_SYSREG_OFFSET(DBGBCR0_EL1)
> bl __restore_debug
> 1:
> restore_host_regs
> --
> 2.3.5
>
Thanks,
-Christoffer
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 09/12] KVM: arm64: guest debug, HW assisted debug support
2015-05-07 9:07 ` [PATCH v3 09/12] KVM: arm64: guest debug, HW assisted debug support Alex Bennée
@ 2015-05-08 16:32 ` Christoffer Dall
0 siblings, 0 replies; 24+ messages in thread
From: Christoffer Dall @ 2015-05-08 16:32 UTC (permalink / raw)
To: Alex Bennée
Cc: kvm, linux-arm-kernel, kvmarm, marc.zyngier, peter.maydell,
agraf, drjones, pbonzini, zhichao.huang, jan.kiszka, dahi,
r65777, bp, Gleb Natapov, Jonathan Corbet, Russell King,
Catalin Marinas, Will Deacon, Ard Biesheuvel, Lorenzo Pieralisi,
Andre Przywara, Richard Weinberger, Peter Zijlstra, Ingo Molnar,
AKASHI Takahiro, open list:DOCUMENTATION, open list,
open list:ABI/API
On Thu, May 07, 2015 at 10:07:12AM +0100, Alex Bennée wrote:
> This adds support for userspace to control the HW debug registers for
> guest debug. In the debug ioctl we copy the IMPDEF defined number of
> registers into a new register set called host_debug_state. There is now
> a new vcpu parameter called debug_ptr which selects which register set
> is to copied into the real registers when world switch occurs.
>
> I've moved some helper functions into the hw_breakpoint.h header for
> re-use.
>
> As with single step we need to tweak the guest registers to enable the
> exceptions so we need to save and restore those bits.
>
> Two new capabilities have been added to the KVM_EXTENSION ioctl to allow
> userspace to query the number of hardware break and watch points
> available on the host hardware.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> ---
> v2
> - switched to C setup
> - replace host debug registers directly into context
> - minor tweak to api docs
> - setup right register for debug
> - add FAR_EL2 to debug exit structure
> - add support for trapping debug register access
> v3
> - remove stray trace statement
> - fix spacing around operators (various)
> - clean-up usage of trap_debug
> - introduce debug_ptr, replace excessive memcpy stuff
> - don't use memcpy in ioctl, just assign
> - update cap ioctl documentation
> - reword a number comments
> - rename host_debug_state->external_debug_state
>
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index 5ef937c..419f7a8 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -2668,7 +2668,7 @@ The top 16 bits of the control field are architecture specific control
> flags which can include the following:
>
> - KVM_GUESTDBG_USE_SW_BP: using software breakpoints [x86, arm64]
> - - KVM_GUESTDBG_USE_HW_BP: using hardware breakpoints [x86, s390]
> + - KVM_GUESTDBG_USE_HW_BP: using hardware breakpoints [x86, s390, arm64]
> - KVM_GUESTDBG_INJECT_DB: inject DB type exception [x86]
> - KVM_GUESTDBG_INJECT_BP: inject BP type exception [x86]
> - KVM_GUESTDBG_EXIT_PENDING: trigger an immediate guest exit [s390]
> @@ -2683,6 +2683,11 @@ updated to the correct (supplied) values.
> The second part of the structure is architecture specific and
> typically contains a set of debug registers.
>
> +For arm64 the number of debug registers is implementation defined and
> +can be determined by querying the KVM_CAP_GUEST_DEBUG_HW_BPS and
> +KVM_CAP_GUEST_DEBUG_HW_WPS capabilities which returns a +ve number
s/returns/return/
s/+ve/positive/
> +indicating the number of supported registers.
> +
> When debug events exit the main run loop with the reason
> KVM_EXIT_DEBUG with the kvm_debug_exit_arch part of the kvm_run
> structure containing architecture specific debug information.
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index 9b3ed6d..2920185 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -279,6 +279,10 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
> /* Set up the timer */
> kvm_timer_vcpu_init(vcpu);
>
> + /* Set the debug registers to be the guests */
> + vcpu->arch.debug_ptr = (struct kvm_guest_debug_arch *)
> + &vcpu_sys_reg(vcpu, DBGBCR0_EL1);
> +
yikes, I don't like this cast, how bad is it to get rid of the debug
registers in the sys_regs array ?
Also, pretty sure this is part of the breakage for the 32-bit build...
> return 0;
> }
>
> @@ -304,6 +308,7 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
>
> #define KVM_GUESTDBG_VALID_MASK (KVM_GUESTDBG_ENABLE | \
> KVM_GUESTDBG_USE_SW_BP | \
> + KVM_GUESTDBG_USE_HW_BP | \
> KVM_GUESTDBG_SINGLESTEP)
>
> /**
> @@ -324,6 +329,12 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
>
> if (dbg->control & KVM_GUESTDBG_ENABLE) {
> vcpu->guest_debug = dbg->control;
> +
> + /* Hardware assisted Break and Watch points */
> + if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP) {
is this only breakpoints or breakpoints and watch points?
> + vcpu->arch.external_debug_state = dbg->arch;
> + }
> +
> } else {
> /* If not enabled clear all flags */
> vcpu->guest_debug = 0;
> diff --git a/arch/arm64/include/asm/hw_breakpoint.h b/arch/arm64/include/asm/hw_breakpoint.h
> index 52b484b..c450552 100644
> --- a/arch/arm64/include/asm/hw_breakpoint.h
> +++ b/arch/arm64/include/asm/hw_breakpoint.h
> @@ -130,6 +130,18 @@ static inline void ptrace_hw_copy_thread(struct task_struct *task)
> }
> #endif
>
> +/* Determine number of BRP registers available. */
> +static inline int get_num_brps(void)
> +{
> + return ((read_cpuid(ID_AA64DFR0_EL1) >> 12) & 0xf) + 1;
> +}
> +
> +/* Determine number of WRP registers available. */
> +static inline int get_num_wrps(void)
> +{
> + return ((read_cpuid(ID_AA64DFR0_EL1) >> 20) & 0xf) + 1;
> +}
> +
I will need an ack from Catalin/Will to merge this. It may be better to
move these functions in a separate patch.
> extern struct pmu perf_ops_bp;
>
> #endif /* __KERNEL__ */
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index b60fa7a..a44fb32 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -108,9 +108,18 @@ struct kvm_vcpu_arch {
> /* Exception Information */
> struct kvm_vcpu_fault_info fault;
>
> - /* Debug state */
> + /* Guest debug state */
> u64 debug_flags;
>
> + /*
> + * For debugging the guest we need to keep a set of debug
> + * registers which can override the guests own debug state
s/guests/guest's/
> + * while being used. These are set via the KVM_SET_GUEST_DEBUG
> + * ioctl.
> + */
> + struct kvm_guest_debug_arch *debug_ptr;
> + struct kvm_guest_debug_arch external_debug_state;
> +
> /* Pointer to host CPU context */
> kvm_cpu_context_t *host_cpu_context;
>
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index 04957d7..98e82ef 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -121,7 +121,7 @@ struct kvm_guest_debug_arch {
>
> struct kvm_debug_exit_arch {
> __u32 hsr;
> - __u64 far;
> + __u64 far; /* used for watchpoints */
seems strange to amend this now?
> };
>
> struct kvm_sync_regs {
> diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
> index ce7b7dd..671ab13 100644
> --- a/arch/arm64/kernel/asm-offsets.c
> +++ b/arch/arm64/kernel/asm-offsets.c
> @@ -116,6 +116,7 @@ int main(void)
> DEFINE(VCPU_FAR_EL2, offsetof(struct kvm_vcpu, arch.fault.far_el2));
> DEFINE(VCPU_HPFAR_EL2, offsetof(struct kvm_vcpu, arch.fault.hpfar_el2));
> DEFINE(VCPU_DEBUG_FLAGS, offsetof(struct kvm_vcpu, arch.debug_flags));
> + DEFINE(VCPU_DEBUG_PTR, offsetof(struct kvm_vcpu, arch.debug_ptr));
> DEFINE(DEBUG_BCR, offsetof(struct kvm_guest_debug_arch, dbg_bcr));
> DEFINE(DEBUG_BVR, offsetof(struct kvm_guest_debug_arch, dbg_bvr));
> DEFINE(DEBUG_WCR, offsetof(struct kvm_guest_debug_arch, dbg_wcr));
> diff --git a/arch/arm64/kernel/hw_breakpoint.c b/arch/arm64/kernel/hw_breakpoint.c
> index e7d934d..3a41bbf 100644
> --- a/arch/arm64/kernel/hw_breakpoint.c
> +++ b/arch/arm64/kernel/hw_breakpoint.c
> @@ -49,18 +49,6 @@ static DEFINE_PER_CPU(int, stepping_kernel_bp);
> static int core_num_brps;
> static int core_num_wrps;
>
> -/* Determine number of BRP registers available. */
> -static int get_num_brps(void)
> -{
> - return ((read_cpuid(ID_AA64DFR0_EL1) >> 12) & 0xf) + 1;
> -}
> -
> -/* Determine number of WRP registers available. */
> -static int get_num_wrps(void)
> -{
> - return ((read_cpuid(ID_AA64DFR0_EL1) >> 20) & 0xf) + 1;
> -}
> -
> int hw_breakpoint_slots(int type)
> {
> /*
> diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
> index 19346e8..1ab63dd 100644
> --- a/arch/arm64/kvm/debug.c
> +++ b/arch/arm64/kvm/debug.c
> @@ -99,12 +99,6 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
> MDCR_EL2_TDRA |
> MDCR_EL2_TDOSA);
>
> - /* Trap on access to debug registers? */
> - if (trap_debug)
> - vcpu->arch.mdcr_el2 |= MDCR_EL2_TDA;
> - else
> - vcpu->arch.mdcr_el2 &= ~MDCR_EL2_TDA;
> -
> /* Is Guest debugging in effect? */
> if (vcpu->guest_debug) {
> vcpu->arch.mdcr_el2 |= MDCR_EL2_TDE;
> @@ -128,14 +122,54 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
> vcpu_sys_reg(vcpu, MDSCR_EL1) &= ~DBG_MDSCR_SS;
> }
>
> + /*
> + * HW Break/Watch points
> + *
> + * We simply switch the debug_ptr to point to our new
> + * external_debug_state which has been populated by the
> + * debug ioctl. The existing KVM_ARM64_DEBUG_DIRTY
> + * mechanism ensures the registers are updated on the
> + * world switch.
> + */
> + if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP) {
> +
> + vcpu_sys_reg(vcpu, MDSCR_EL1) |=
> + (DBG_MDSCR_KDE | DBG_MDSCR_MDE);
Why do we need to set these two bits? Is it obvious or does it deserve
a comment?
> +
> + vcpu->arch.debug_ptr = &vcpu->arch.external_debug_state;
> + vcpu->arch.debug_flags |= KVM_ARM64_DEBUG_DIRTY;
> + trap_debug = true;
> + }
> +
> } else {
> /* Debug operations can go straight to the guest */
> vcpu->arch.mdcr_el2 &= ~MDCR_EL2_TDE;
> }
> +
> + /*
> + * If the guest debug register state is dirty (the guest is
> + * actively accessing them), then we context-switch the
> + * registers in EL2. Otherwise, we trap-and-emulate all guest
> + * accesses to them.
> + */
I think this comment now feels strange, because it was explaining why we
would set the trap_debug variable when the dirty flag was set, but the
code just sets TDA when trap_debug is set. So you should either move
this comment to the top of the function and have it above a separate
line that sets trap_debug based on KVM_ARM64_DEBUG_DIRTY (instead of
initializing at declaration), or you should explain which conditions set
trap_debug (guest is using the regs or we are debugging the guest), or
just get rid of the comment.
> + if (trap_debug)
> + vcpu->arch.mdcr_el2 |= MDCR_EL2_TDA;
> + else
> + vcpu->arch.mdcr_el2 &= ~MDCR_EL2_TDA;
still don't need the else.
> }
>
> void kvm_arm_clear_debug(struct kvm_vcpu *vcpu)
> {
> - if (vcpu->guest_debug)
> + if (vcpu->guest_debug) {
> restore_guest_debug_regs(vcpu);
> +
> + /*
> + * If we were using HW debug we need to restore the
> + * debug_ptr to the guest debug state.
> + */
> + if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP) {
> + vcpu->arch.debug_ptr = (struct kvm_guest_debug_arch *)
> + &vcpu_sys_reg(vcpu, DBGBCR0_EL1);
> + }
I would find it easier to follow the code if you only configure the
debug_ptr in kvm_arm_setup_debug() because it feels like you're setting
up state here which will not be used before in a very long time (after
handle_exit, exit to userspace etc.).
> + }
> }
> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
> index e9de13e..68a0759 100644
> --- a/arch/arm64/kvm/handle_exit.c
> +++ b/arch/arm64/kvm/handle_exit.c
> @@ -103,7 +103,11 @@ static int kvm_handle_guest_debug(struct kvm_vcpu *vcpu, struct kvm_run *run)
> run->debug.arch.hsr = hsr;
>
> switch (hsr >> ESR_ELx_EC_SHIFT) {
> + case ESR_ELx_EC_WATCHPT_LOW:
> + run->debug.arch.far = vcpu->arch.fault.far_el2;
> + /* fall through */
> case ESR_ELx_EC_SOFTSTP_LOW:
> + case ESR_ELx_EC_BREAKPT_LOW:
> case ESR_ELx_EC_BKPT32:
> case ESR_ELx_EC_BRK64:
> break;
> @@ -132,6 +136,8 @@ static exit_handle_fn arm_exit_handlers[] = {
> [ESR_ELx_EC_IABT_LOW] = kvm_handle_guest_abort,
> [ESR_ELx_EC_DABT_LOW] = kvm_handle_guest_abort,
> [ESR_ELx_EC_SOFTSTP_LOW]= kvm_handle_guest_debug,
> + [ESR_ELx_EC_WATCHPT_LOW]= kvm_handle_guest_debug,
> + [ESR_ELx_EC_BREAKPT_LOW]= kvm_handle_guest_debug,
> [ESR_ELx_EC_BKPT32] = kvm_handle_guest_debug,
> [ESR_ELx_EC_BRK64] = kvm_handle_guest_debug,
> };
> diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
> index dd51fb1..921d248 100644
> --- a/arch/arm64/kvm/hyp.S
> +++ b/arch/arm64/kvm/hyp.S
> @@ -706,7 +706,8 @@ ENTRY(__kvm_vcpu_run)
> bl __restore_fpsimd
>
> skip_debug_state x3, 1f
> - add x3, x2, #CPU_SYSREG_OFFSET(DBGBCR0_EL1)
> + ldr x3, [x0, #VCPU_DEBUG_PTR]
> + kern_hyp_va x3
> bl __restore_debug
> 1:
> restore_guest_32bit_state
> @@ -727,7 +728,8 @@ __kvm_vcpu_return:
> bl __save_sysregs
>
> skip_debug_state x3, 1f
> - add x3, x2, #CPU_SYSREG_OFFSET(DBGBCR0_EL1)
> + ldr x3, [x0, #VCPU_DEBUG_PTR]
> + kern_hyp_va x3
> bl __save_debug
> 1:
> save_guest_32bit_state
> diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
> index 0b43265..21d5a62 100644
> --- a/arch/arm64/kvm/reset.c
> +++ b/arch/arm64/kvm/reset.c
> @@ -56,6 +56,12 @@ static bool cpu_has_32bit_el1(void)
> return !!(pfr0 & 0x20);
> }
>
> +/**
> + * kvm_arch_dev_ioctl_check_extension
> + *
> + * We currently assume that the number of HW registers is uniform
> + * across all CPUs (see cpuinfo_sanity_check).
> + */
> int kvm_arch_dev_ioctl_check_extension(long ext)
> {
> int r;
> @@ -64,6 +70,12 @@ int kvm_arch_dev_ioctl_check_extension(long ext)
> case KVM_CAP_ARM_EL1_32BIT:
> r = cpu_has_32bit_el1();
> break;
> + case KVM_CAP_GUEST_DEBUG_HW_BPS:
> + r = get_num_brps();
> + break;
> + case KVM_CAP_GUEST_DEBUG_HW_WPS:
> + r = get_num_wrps();
> + break;
> default:
> r = 0;
> }
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 3b6252e..923c2aa 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -825,6 +825,8 @@ struct kvm_ppc_smmu_info {
> #define KVM_CAP_S390_INJECT_IRQ 113
> #define KVM_CAP_S390_IRQ_STATE 114
> #define KVM_CAP_PPC_HWRNG 115
> +#define KVM_CAP_GUEST_DEBUG_HW_BPS 116
> +#define KVM_CAP_GUEST_DEBUG_HW_WPS 117
>
> #ifdef KVM_CAP_IRQ_ROUTING
>
> --
> 2.3.5
>
Thanks,
-Christoffer
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 10/12] KVM: arm64: trap nested debug register access
2015-05-07 9:07 ` [PATCH v3 10/12] KVM: arm64: trap nested debug register access Alex Bennée
@ 2015-05-08 16:46 ` Christoffer Dall
0 siblings, 0 replies; 24+ messages in thread
From: Christoffer Dall @ 2015-05-08 16:46 UTC (permalink / raw)
To: Alex Bennée
Cc: kvm, linux-arm-kernel, kvmarm, marc.zyngier, peter.maydell,
agraf, drjones, pbonzini, zhichao.huang, jan.kiszka, dahi,
r65777, bp, Gleb Natapov, Catalin Marinas, Will Deacon,
open list
On Thu, May 07, 2015 at 10:07:13AM +0100, Alex Bennée wrote:
> When we are using the hardware registers for guest debug we need to deal
> with the guests access to them. There is already a mechanism for dealing
> with these accesses so we build on top of that.
>
> - any access to mdscr_el1 is now stored in the mirror location
> - access to DBG[WB][CV]R continues to go to guest's context
>
> There is one register (MDCCINT_EL1) which guest debug doesn't care about
> so this behaves as before.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> ---
> v3
> - re-factor for better flow and fall through.
> - much simpler with debug_ptr (use the guest area as before)
> - tweak shadow fn to avoid multi-line if
>
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index a44fb32..7aa3b3a 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -132,7 +132,13 @@ struct kvm_vcpu_arch {
> * here.
> */
>
> - /* Guest registers we preserve during guest debugging */
> + /*
> + * Guest registers we preserve during guest debugging.
> + *
> + * These shadow registers are updated by the kvm_handle_sys_reg
> + * trap handler if the guest accesses or updates them while we
> + * are using guest debug.
> + */
> struct {
> u32 pstate;
> u32 mdscr_el1;
> diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
> index 1ab63dd..dc8bca8 100644
> --- a/arch/arm64/kvm/debug.c
> +++ b/arch/arm64/kvm/debug.c
> @@ -50,8 +50,7 @@ static void restore_guest_debug_regs(struct kvm_vcpu *vcpu)
> {
> *vcpu_cpsr(vcpu) |=
> (vcpu->arch.guest_debug_state.pstate & SPSR_DEBUG_MASK);
> - vcpu_sys_reg(vcpu, MDSCR_EL1) |=
> - (vcpu->arch.guest_debug_state.mdscr_el1 & MDSCR_EL1_DEBUG_MASK);
> + vcpu_sys_reg(vcpu, MDSCR_EL1) = vcpu->arch.guest_debug_state.mdscr_el1;
> }
>
> /**
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index c370b40..95f422f 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -196,11 +196,40 @@ static bool trap_dbgauthstatus_el1(struct kvm_vcpu *vcpu,
> * - If the dirty bit is set, save guest registers, restore host
> * registers and clear the dirty bit. This ensure that the host can
> * now use the debug registers.
> + *
> + * We also use this mechanism to set-up the debug registers for guest
s/set-up/set up/
> + * debugging. If this is the case we want to ensure the guest sees
If this is the case, (comma)
> + * the right versions of the registers - even if they are not going to
> + * be effective while guest debug is using HW debug.
> + *
> */
> +
> +static bool shadow_debug_reg(struct kvm_vcpu *vcpu,
> + const struct sys_reg_params *p,
> + const struct sys_reg_desc *r)
> +{
> + /* MDSCR_EL1 */
> + if (r->reg == MDSCR_EL1) {
> + u32 *shadow_mdscr_el1 = &vcpu->arch.guest_debug_state.mdscr_el1;
> +
> + if (p->is_write)
> + *shadow_mdscr_el1 = *vcpu_reg(vcpu, p->Rt);
> + else
> + *vcpu_reg(vcpu, p->Rt) = *shadow_mdscr_el1;
> +
> + return true;
> + }
> +
> + return false;
> +}
> +
> static bool trap_debug_regs(struct kvm_vcpu *vcpu,
> const struct sys_reg_params *p,
> const struct sys_reg_desc *r)
> {
> + if (vcpu->guest_debug && shadow_debug_reg(vcpu, p, r))
> + return true;
> +
so do we also have a MDSCR_EL1 in sys_regs and one in guest_debug_state
now?
If yes, what are the differences between the two?
> if (p->is_write) {
> vcpu_sys_reg(vcpu, r->reg) = *vcpu_reg(vcpu, p->Rt);
> vcpu->arch.debug_flags |= KVM_ARM64_DEBUG_DIRTY;
> --
> 2.3.5
>
Thanks,
-Christoffer
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 11/12] KVM: arm64: enable KVM_CAP_SET_GUEST_DEBUG
2015-05-07 9:07 ` [PATCH v3 11/12] KVM: arm64: enable KVM_CAP_SET_GUEST_DEBUG Alex Bennée
@ 2015-05-08 17:21 ` Christoffer Dall
0 siblings, 0 replies; 24+ messages in thread
From: Christoffer Dall @ 2015-05-08 17:21 UTC (permalink / raw)
To: Alex Bennée
Cc: kvm, linux-arm-kernel, kvmarm, marc.zyngier, peter.maydell,
agraf, drjones, pbonzini, zhichao.huang, jan.kiszka, dahi,
r65777, bp, Gleb Natapov, Catalin Marinas, Will Deacon,
open list
On Thu, May 07, 2015 at 10:07:14AM +0100, Alex Bennée wrote:
> Finally advertise the KVM capability for SET_GUEST_DEBUG. Once arm
> support is added this check can be moved to the common
> kvm_vm_ioctl_check_extension() code.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 12/12] KVM: arm64: add trace points for guest_debug debug
2015-05-07 9:07 ` [PATCH v3 12/12] KVM: arm64: add trace points for guest_debug debug Alex Bennée
@ 2015-05-08 17:25 ` Christoffer Dall
0 siblings, 0 replies; 24+ messages in thread
From: Christoffer Dall @ 2015-05-08 17:25 UTC (permalink / raw)
To: Alex Bennée
Cc: kvm, linux-arm-kernel, kvmarm, marc.zyngier, peter.maydell,
agraf, drjones, pbonzini, zhichao.huang, jan.kiszka, dahi,
r65777, bp, Gleb Natapov, Catalin Marinas, Will Deacon,
open list
On Thu, May 07, 2015 at 10:07:15AM +0100, Alex Bennée wrote:
> This includes trace points for:
> kvm_arch_setup_guest_debug
> kvm_arch_clear_guest_debug
> kvm_handle_guest_debug
>
> I've also added some generic register setting trace events and also a
> trace point to dump the array of hardware registers.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> ---
> v3
> - add trace event for debug access.
> - remove short trace #define, rename trace events
> - use __print_array with fixed array instead of own func
> - rationalise trace points (only one per register changed)
> - add vcpu ptr to the debug_setup trace
> - remove :: in prints
>
> diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
> index dc8bca8..08e1b83 100644
> --- a/arch/arm64/kvm/debug.c
> +++ b/arch/arm64/kvm/debug.c
> @@ -24,6 +24,8 @@
> #include <asm/kvm_arm.h>
> #include <asm/kvm_emulate.h>
>
> +#include "trace.h"
> +
> /* These are the bits of MDSCR_EL1 we may manipulate */
> #define MDSCR_EL1_DEBUG_MASK (DBG_MDSCR_SS | \
> DBG_MDSCR_KDE | \
> @@ -44,6 +46,11 @@ static void save_guest_debug_regs(struct kvm_vcpu *vcpu)
> {
> vcpu->arch.guest_debug_state.pstate = *vcpu_cpsr(vcpu);
> vcpu->arch.guest_debug_state.mdscr_el1 = vcpu_sys_reg(vcpu, MDSCR_EL1);
> +
> + trace_kvm_arm_set_dreg32("Saved PSTATE",
> + vcpu->arch.guest_debug_state.pstate);
> + trace_kvm_arm_set_dreg32("Saved MDSCR_EL1",
> + vcpu->arch.guest_debug_state.mdscr_el1);
wouldn't it make sense to turn these into a single tracepoint with two
parameters?
> }
>
> static void restore_guest_debug_regs(struct kvm_vcpu *vcpu)
> @@ -51,6 +58,10 @@ static void restore_guest_debug_regs(struct kvm_vcpu *vcpu)
> *vcpu_cpsr(vcpu) |=
> (vcpu->arch.guest_debug_state.pstate & SPSR_DEBUG_MASK);
> vcpu_sys_reg(vcpu, MDSCR_EL1) = vcpu->arch.guest_debug_state.mdscr_el1;
> +
> + trace_kvm_arm_set_dreg32("Restored PSTATE", *vcpu_cpsr(vcpu));
> + trace_kvm_arm_set_dreg32("Restored MDSCR_EL1",
> + vcpu_sys_reg(vcpu, MDSCR_EL1));
ditto
> }
>
> /**
> @@ -92,6 +103,8 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
> {
> bool trap_debug = !(vcpu->arch.debug_flags & KVM_ARM64_DEBUG_DIRTY);
>
> + trace_kvm_arm_setup_debug(vcpu, vcpu->guest_debug);
> +
> vcpu->arch.mdcr_el2 = __this_cpu_read(mdcr_el2) & MDCR_EL2_HPMN_MASK;
> vcpu->arch.mdcr_el2 |= (MDCR_EL2_TPM |
> MDCR_EL2_TPMCR |
> @@ -121,6 +134,8 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
> vcpu_sys_reg(vcpu, MDSCR_EL1) &= ~DBG_MDSCR_SS;
> }
>
> + trace_kvm_arm_set_dreg32("SPSR_EL2", *vcpu_cpsr(vcpu));
> +
> /*
> * HW Break/Watch points
> *
> @@ -138,6 +153,14 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
> vcpu->arch.debug_ptr = &vcpu->arch.external_debug_state;
> vcpu->arch.debug_flags |= KVM_ARM64_DEBUG_DIRTY;
> trap_debug = true;
> +
> + trace_kvm_arm_set_regset("BKPTS", get_num_brps(),
> + &vcpu->arch.debug_ptr->dbg_bcr[0],
> + &vcpu->arch.debug_ptr->dbg_bvr[0]);
> +
> + trace_kvm_arm_set_regset("WAPTS", get_num_wrps(),
> + &vcpu->arch.debug_ptr->dbg_wcr[0],
> + &vcpu->arch.debug_ptr->dbg_wvr[0]);
feels like this should also be a single tracepoint
> }
>
> } else {
> @@ -155,10 +178,15 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
> vcpu->arch.mdcr_el2 |= MDCR_EL2_TDA;
> else
> vcpu->arch.mdcr_el2 &= ~MDCR_EL2_TDA;
> +
> + trace_kvm_arm_set_dreg32("MDCR_EL2", vcpu->arch.mdcr_el2);
> + trace_kvm_arm_set_dreg32("MDSCR_EL1", vcpu_sys_reg(vcpu, MDSCR_EL1));
> }
>
> void kvm_arm_clear_debug(struct kvm_vcpu *vcpu)
> {
> + trace_kvm_arm_clear_debug(vcpu->guest_debug);
> +
> if (vcpu->guest_debug) {
> restore_guest_debug_regs(vcpu);
>
> @@ -169,6 +197,14 @@ void kvm_arm_clear_debug(struct kvm_vcpu *vcpu)
> if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP) {
> vcpu->arch.debug_ptr = (struct kvm_guest_debug_arch *)
> &vcpu_sys_reg(vcpu, DBGBCR0_EL1);
> +
> + trace_kvm_arm_set_regset("BKPTS", get_num_brps(),
> + &vcpu->arch.debug_ptr->dbg_bcr[0],
> + &vcpu->arch.debug_ptr->dbg_bvr[0]);
> +
> + trace_kvm_arm_set_regset("WAPTS", get_num_wrps(),
> + &vcpu->arch.debug_ptr->dbg_wcr[0],
> + &vcpu->arch.debug_ptr->dbg_wvr[0]);
ditto
> }
> }
> }
> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
> index 68a0759..c93b95e 100644
> --- a/arch/arm64/kvm/handle_exit.c
> +++ b/arch/arm64/kvm/handle_exit.c
> @@ -99,6 +99,8 @@ static int kvm_handle_guest_debug(struct kvm_vcpu *vcpu, struct kvm_run *run)
> u32 hsr = kvm_vcpu_get_hsr(vcpu);
> int ret = 0;
>
> + trace_kvm_handle_guest_debug(*vcpu_pc(vcpu), hsr);
> +
does this provide anything beyond the generic handle_exit tracepoint?
> run->exit_reason = KVM_EXIT_DEBUG;
> run->debug.arch.hsr = hsr;
>
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 95f422f..ec803ad 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -38,6 +38,8 @@
>
> #include "sys_regs.h"
>
> +#include "trace.h"
> +
> /*
> * All of this file is extremly similar to the ARM coproc.c, but the
> * types are different. My gut feeling is that it should be pretty
> @@ -227,6 +229,9 @@ static bool trap_debug_regs(struct kvm_vcpu *vcpu,
> const struct sys_reg_params *p,
> const struct sys_reg_desc *r)
> {
> + trace_trap_debug_regs(r->reg, p->is_write,
> + p->is_write?*vcpu_reg(vcpu, p->Rt):0);
> +
> if (vcpu->guest_debug && shadow_debug_reg(vcpu, p, r))
> return true;
>
> diff --git a/arch/arm64/kvm/trace.h b/arch/arm64/kvm/trace.h
> index 157416e9..62e6b76 100644
> --- a/arch/arm64/kvm/trace.h
> +++ b/arch/arm64/kvm/trace.h
> @@ -44,6 +44,113 @@ TRACE_EVENT(kvm_hvc_arm64,
> __entry->vcpu_pc, __entry->r0, __entry->imm)
> );
>
> +TRACE_EVENT(kvm_handle_guest_debug,
> + TP_PROTO(unsigned long vcpu_pc, u32 hsr),
> + TP_ARGS(vcpu_pc, hsr),
> +
> + TP_STRUCT__entry(
> + __field(unsigned long, vcpu_pc)
> + __field(u32, hsr)
> + ),
> +
> + TP_fast_assign(
> + __entry->vcpu_pc = vcpu_pc;
> + __entry->hsr = hsr;
> + ),
> +
> + TP_printk("debug exception at 0x%08lx (HSR: 0x%08x)",
> + __entry->vcpu_pc, __entry->hsr)
> +);
> +
> +
> +TRACE_EVENT(kvm_arm_setup_debug,
> + TP_PROTO(struct kvm_vcpu *vcpu, __u32 guest_debug),
> + TP_ARGS(vcpu, guest_debug),
> +
> + TP_STRUCT__entry(
> + __field(struct kvm_vcpu *, vcpu)
> + __field(__u32, guest_debug)
> + ),
> +
> + TP_fast_assign(
> + __entry->vcpu = vcpu;
> + __entry->guest_debug = guest_debug;
> + ),
> +
> + TP_printk("vcpu: %p, flags: 0x%08x", __entry->vcpu, __entry->guest_debug)
> +);
> +
> +TRACE_EVENT(kvm_arm_clear_debug,
> + TP_PROTO(__u32 guest_debug),
> + TP_ARGS(guest_debug),
> +
> + TP_STRUCT__entry(
> + __field(__u32, guest_debug)
> + ),
> +
> + TP_fast_assign(
> + __entry->guest_debug = guest_debug;
> + ),
> +
> + TP_printk("flags: 0x%08x", __entry->guest_debug)
> +);
> +
> +TRACE_EVENT(kvm_arm_set_dreg32,
> + TP_PROTO(const char *name, __u32 value),
> + TP_ARGS(name, value),
> +
> + TP_STRUCT__entry(
> + __field(const char *, name)
> + __field(__u32, value)
> + ),
> +
> + TP_fast_assign(
> + __entry->name = name;
> + __entry->value = value;
> + ),
> +
> + TP_printk("%s: 0x%08x", __entry->name, __entry->value)
> +);
> +
> +TRACE_EVENT(kvm_arm_set_regset,
> + TP_PROTO(const char *type, int len, __u64 *control, __u64 *value),
> + TP_ARGS(type, len, control, value),
> + TP_STRUCT__entry(
> + __field(const char *, name)
> + __field(int, len)
> + __array(u64, ctrls, 16)
> + __array(u64, values, 16)
> + ),
> + TP_fast_assign(
> + __entry->name = type;
> + __entry->len = len;
> + memcpy(__entry->ctrls, control, len << 3);
> + memcpy(__entry->values, value, len << 3);
> + ),
> + TP_printk("%d %s CTRL:%s VALUE:%s", __entry->len, __entry->name,
> + __print_array(__entry->ctrls, __entry->len, sizeof(__u64)),
> + __print_array(__entry->values, __entry->len, sizeof(__u64)))
> +);
> +
> +TRACE_EVENT(trap_debug_regs,
> + TP_PROTO(int reg, bool is_write, u64 write_value),
> + TP_ARGS(reg, is_write, write_value),
> +
> + TP_STRUCT__entry(
> + __field(int, reg)
> + __field(bool, is_write)
> + __field(u64, write_value)
> + ),
> +
> + TP_fast_assign(
> + __entry->reg = reg;
> + __entry->is_write = is_write;
> + __entry->write_value = write_value;
> + ),
> +
> + TP_printk("%s reg %d (0x%08llx)", __entry->is_write?"write to":"read from", __entry->reg, __entry->write_value)
> +);
> +
> #endif /* _TRACE_ARM64_KVM_H */
>
> #undef TRACE_INCLUDE_PATH
> --
> 2.3.5
>
Thanks,
-Christoffer
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2015-05-08 17:25 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <1430929407-3487-1-git-send-email-alex.bennee@linaro.org>
2015-05-06 16:23 ` [PATCH v3 01/12] KVM: add comments for kvm_debug_exit_arch struct Alex Bennée
2015-05-08 9:19 ` Christoffer Dall
2015-05-06 16:23 ` [PATCH v3 02/12] KVM: define common __KVM_GUESTDBG_USE_SW/HW_BP values Alex Bennée
2015-05-08 9:23 ` Christoffer Dall
2015-05-06 16:23 ` [PATCH v3 03/12] KVM: arm64: guest debug, define API headers Alex Bennée
2015-05-08 9:28 ` Christoffer Dall
2015-05-06 16:23 ` [PATCH v3 04/12] KVM: arm: guest debug, add stub KVM_SET_GUEST_DEBUG ioctl Alex Bennée
2015-05-08 11:52 ` Christoffer Dall
2015-05-06 16:23 ` [PATCH v3 05/12] KVM: arm: introduce kvm_arm_init/setup/clear_debug Alex Bennée
2015-05-08 11:52 ` Christoffer Dall
2015-05-06 16:23 ` [PATCH v3 06/12] KVM: arm64: guest debug, add SW break point support Alex Bennée
2015-05-08 11:52 ` Christoffer Dall
2015-05-06 16:23 ` [PATCH v3 07/12] KVM: arm64: guest debug, add support for single-step Alex Bennée
2015-05-08 11:52 ` Christoffer Dall
2015-05-07 9:07 ` [PATCH v3 08/12] KVM: arm64: re-factor hyp.S debug register code Alex Bennée
2015-05-08 14:12 ` Christoffer Dall
2015-05-07 9:07 ` [PATCH v3 09/12] KVM: arm64: guest debug, HW assisted debug support Alex Bennée
2015-05-08 16:32 ` Christoffer Dall
2015-05-07 9:07 ` [PATCH v3 10/12] KVM: arm64: trap nested debug register access Alex Bennée
2015-05-08 16:46 ` Christoffer Dall
2015-05-07 9:07 ` [PATCH v3 11/12] KVM: arm64: enable KVM_CAP_SET_GUEST_DEBUG Alex Bennée
2015-05-08 17:21 ` Christoffer Dall
2015-05-07 9:07 ` [PATCH v3 12/12] KVM: arm64: add trace points for guest_debug debug Alex Bennée
2015-05-08 17:25 ` Christoffer Dall
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).