* [PATCH v7 01/11] KVM: add comments for kvm_debug_exit_arch struct
[not found] <1435775343-20034-1-git-send-email-alex.bennee@linaro.org>
@ 2015-07-01 18:28 ` Alex Bennée
2015-07-01 18:28 ` [PATCH v7 02/11] KVM: arm64: guest debug, define API headers Alex Bennée
` (9 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Alex Bennée @ 2015-07-01 18:28 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>
Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
---
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
v4
- sp fixes
- add a-b
---
Documentation/virtual/kvm/api.txt | 4 +++-
include/uapi/linux/kvm.h | 3 +++
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 9fa2bf8..c34c32d 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 is KVM_EXIT_DEBUG, then a vcpu is processing a debug event
+for which architecture specific 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.4.5
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v7 02/11] KVM: arm64: guest debug, define API headers
[not found] <1435775343-20034-1-git-send-email-alex.bennee@linaro.org>
2015-07-01 18:28 ` [PATCH v7 01/11] KVM: add comments for kvm_debug_exit_arch struct Alex Bennée
@ 2015-07-01 18:28 ` Alex Bennée
2015-07-01 18:28 ` [PATCH v7 03/11] KVM: arm: guest debug, add stub KVM_SET_GUEST_DEBUG ioctl Alex Bennée
` (8 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Alex Bennée @ 2015-07-01 18:28 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>
Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
---
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
v4
- now uses common HW/SW BP define
- add a-b-tag
- use u32 for control regs
v5
- revert to have arch specific KVM_GUESTDBG_USE_SW/HW_BP
- rm stale comments dbgctrl was stored as u64
v6
- mv far comment from later patch
- KVM_GUESTDBG_USE_HW_BP -> KVM_GUESTDBG_USE_HW
- revert control regs to u64 (parity with GET/SET_ONE_REG)
---
arch/arm64/include/uapi/asm/kvm.h | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index d268320..d82f3f3 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -100,12 +100,39 @@ struct kvm_sregs {
struct kvm_fpu {
};
+/*
+ * See v8 ARM ARM D7.3: Debug Registers
+ *
+ * The architectural limit is 16 debug registers of each type although
+ * in practice there are usually less (see ID_AA64DFR0_EL1).
+ *
+ * Although the control registers are architecturally defined as 32
+ * bits wide we use a 64 bit structure here to keep parity with
+ * KVM_GET/SET_ONE_REG behaviour which treats all system registers as
+ * 64 bit values. It also allows for the possibility of the
+ * architecture expanding the control registers without having to
+ * change the userspace ABI.
+ */
+#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; /* used for watchpoints */
};
+/*
+ * Architecture specific defines for kvm_guest_debug->control
+ */
+
+#define KVM_GUESTDBG_USE_SW_BP (1 << 16)
+#define KVM_GUESTDBG_USE_HW (1 << 17)
+
struct kvm_sync_regs {
};
--
2.4.5
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v7 03/11] KVM: arm: guest debug, add stub KVM_SET_GUEST_DEBUG ioctl
[not found] <1435775343-20034-1-git-send-email-alex.bennee@linaro.org>
2015-07-01 18:28 ` [PATCH v7 01/11] KVM: add comments for kvm_debug_exit_arch struct Alex Bennée
2015-07-01 18:28 ` [PATCH v7 02/11] KVM: arm64: guest debug, define API headers Alex Bennée
@ 2015-07-01 18:28 ` Alex Bennée
2015-07-01 18:28 ` [PATCH v7 04/11] KVM: arm: introduce kvm_arm_init/setup/clear_debug Alex Bennée
` (7 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Alex Bennée @ 2015-07-01 18:28 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 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>.
Reviewed-by: Christoffer Dall <christoffer.dall@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
v4
- add r-b-tag
v7
- moved ioctl to arm64/kvm/guest.c, stubbed arm/kvm/guest.c
---
Documentation/virtual/kvm/api.txt | 2 +-
arch/arm/kvm/arm.c | 7 -------
arch/arm/kvm/guest.c | 6 ++++++
arch/arm64/kvm/guest.c | 27 +++++++++++++++++++++++++++
4 files changed, 34 insertions(+), 8 deletions(-)
diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index c34c32d..ba635c7 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..92b80bc 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -302,13 +302,6 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
kvm_arm_set_running_vcpu(NULL);
}
-int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
- struct kvm_guest_debug *dbg)
-{
- return -EINVAL;
-}
-
-
int kvm_arch_vcpu_ioctl_get_mpstate(struct kvm_vcpu *vcpu,
struct kvm_mp_state *mp_state)
{
diff --git a/arch/arm/kvm/guest.c b/arch/arm/kvm/guest.c
index d503fbb..96e935b 100644
--- a/arch/arm/kvm/guest.c
+++ b/arch/arm/kvm/guest.c
@@ -290,3 +290,9 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *vcpu,
{
return -EINVAL;
}
+
+int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
+ struct kvm_guest_debug *dbg)
+{
+ return -EINVAL;
+}
diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index 9535bd5..0ba8677 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -331,3 +331,30 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *vcpu,
{
return -EINVAL;
}
+
+#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)
+{
+ 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.4.5
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v7 04/11] KVM: arm: introduce kvm_arm_init/setup/clear_debug
[not found] <1435775343-20034-1-git-send-email-alex.bennee@linaro.org>
` (2 preceding siblings ...)
2015-07-01 18:28 ` [PATCH v7 03/11] KVM: arm: guest debug, add stub KVM_SET_GUEST_DEBUG ioctl Alex Bennée
@ 2015-07-01 18:28 ` Alex Bennée
2015-07-01 18:28 ` [PATCH v7 05/11] KVM: arm64: guest debug, add SW break point support Alex Bennée
` (6 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Alex Bennée @ 2015-07-01 18:28 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,
Andre Przywara, Richard Weinberger, 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>
Reviewed-by: Christoffer Dall <christoffer.dall@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
v4
- fix TDOSA desc
- rm un-needed else leg
- s/arch/arm/
v6
- add s-o-b tag
---
arch/arm/include/asm/kvm_host.h | 4 ++
arch/arm/kvm/arm.c | 9 ++++-
arch/arm64/include/asm/kvm_asm.h | 2 +
arch/arm64/include/asm/kvm_host.h | 5 +++
arch/arm64/kernel/asm-offsets.c | 1 +
arch/arm64/kvm/Makefile | 2 +-
arch/arm64/kvm/debug.c | 81 +++++++++++++++++++++++++++++++++++++++
arch/arm64/kvm/hyp.S | 19 ++++-----
8 files changed, 110 insertions(+), 13 deletions(-)
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 92b80bc..af60e6f 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -542,6 +542,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
continue;
}
+ kvm_arm_setup_debug(vcpu);
+
/**************************************************************
* Enter the guest
*/
@@ -554,7 +556,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!
@@ -902,6 +907,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..faf0e1f
--- /dev/null
+++ b/arch/arm64/kvm/debug.c
@@ -0,0 +1,81 @@
+/*
+ * 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)
+ * - OS related 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;
+
+}
+
+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..2c67a14 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_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.4.5
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v7 05/11] KVM: arm64: guest debug, add SW break point support
[not found] <1435775343-20034-1-git-send-email-alex.bennee@linaro.org>
` (3 preceding siblings ...)
2015-07-01 18:28 ` [PATCH v7 04/11] KVM: arm: introduce kvm_arm_init/setup/clear_debug Alex Bennée
@ 2015-07-01 18:28 ` Alex Bennée
2015-07-01 18:28 ` [PATCH v7 06/11] KVM: arm64: guest debug, add support for single-step Alex Bennée
` (5 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Alex Bennée @ 2015-07-01 18:28 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, 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>
Reviewed-by: Christoffer Dall <christoffer.dall@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
v4
- rm else leg
- add r-b-tag
v7
- moved ioctl to guest
---
Documentation/virtual/kvm/api.txt | 2 +-
arch/arm64/kvm/debug.c | 3 +++
arch/arm64/kvm/guest.c | 2 +-
arch/arm64/kvm/handle_exit.c | 36 ++++++++++++++++++++++++++++++++++++
4 files changed, 41 insertions(+), 2 deletions(-)
diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index ba635c7..33c8143 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/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
index faf0e1f..8d1bfa4 100644
--- a/arch/arm64/kvm/debug.c
+++ b/arch/arm64/kvm/debug.c
@@ -73,6 +73,9 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
if (trap_debug)
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;
}
void kvm_arm_clear_debug(struct kvm_vcpu *vcpu)
diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index 0ba8677..22d22c5 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -332,7 +332,7 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *vcpu,
return -EINVAL;
}
-#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/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.4.5
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v7 06/11] KVM: arm64: guest debug, add support for single-step
[not found] <1435775343-20034-1-git-send-email-alex.bennee@linaro.org>
` (4 preceding siblings ...)
2015-07-01 18:28 ` [PATCH v7 05/11] KVM: arm64: guest debug, add SW break point support Alex Bennée
@ 2015-07-01 18:28 ` Alex Bennée
2015-07-01 18:28 ` [PATCH v7 07/11] KVM: arm64: re-factor hyp.S debug register code Alex Bennée
` (4 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Alex Bennée @ 2015-07-01 18:28 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 adds support for single-stepping the guest. To do this we need to
manipulate the guests PSTATE.SS and MDSCR_EL1.SS bits to trigger
stepping. We take care to preserve MDSCR_EL1 and trap access to it to
ensure we don't affect the apparent state of the guest.
As we have to enable trapping of all software debug exceptions we
suppress the ability of the guest to single-step itself. If we didn't we
would have to deal with the exception arriving while the guest was in
kernelspace when the guest is expecting to single-step userspace. This
is something we don't want to unwind in the kernel. Once the host is no
longer debugging the guest its ability to single-step userspace is
restored.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Christoffer Dall <christoffer.dall@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
v4
- added more comments based on suggestions
- guest_debug_state->guest_debug_preserved
- no point masking restore, we will trap out
v5
- more comments
- don't bother preserving pstate.ss (guest never sees change)
v6
- reword comments on guest SS suppression
- simplify comment for save regs, SS explained in detail later on
- add r-b-t (code)
- expanded commit description
v7
- merge fix for ioctl move to guest.c
---
arch/arm64/include/asm/kvm_host.h | 11 +++++++
arch/arm64/kvm/debug.c | 68 ++++++++++++++++++++++++++++++++++++---
arch/arm64/kvm/guest.c | 4 ++-
arch/arm64/kvm/handle_exit.c | 2 ++
4 files changed, 80 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 7cb99b5..e2db6a6 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -123,6 +123,17 @@ struct kvm_vcpu_arch {
* here.
*/
+ /*
+ * 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 mdscr_el1;
+ } guest_debug_preserved;
+
/* Don't run the guest */
bool pause;
diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
index 8d1bfa4..d439eb8 100644
--- a/arch/arm64/kvm/debug.c
+++ b/arch/arm64/kvm/debug.c
@@ -19,11 +19,39 @@
#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)
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.
+ *
+ * Guest access to MDSCR_EL1 is trapped by the hypervisor and handled
+ * after we have restored the preserved value to the main context.
+ */
+static void save_guest_debug_regs(struct kvm_vcpu *vcpu)
+{
+ vcpu->arch.guest_debug_preserved.mdscr_el1 = vcpu_sys_reg(vcpu, MDSCR_EL1);
+}
+
+static void restore_guest_debug_regs(struct kvm_vcpu *vcpu)
+{
+ vcpu_sys_reg(vcpu, MDSCR_EL1) = vcpu->arch.guest_debug_preserved.mdscr_el1;
+}
+
+/**
* kvm_arm_init_debug - grab what we need for debug
*
* Currently the sole task of this function is to retrieve the initial
@@ -38,7 +66,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
*
@@ -73,12 +100,45 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
if (trap_debug)
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) {
+ /* Route all software debug exceptions to EL2 */
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
+ * the guest's MDSCR_EL1.SS and PSTATE.SS. Once the
+ * step has occurred the hypervisor will trap the
+ * debug exception and we return to userspace.
+ *
+ * If the guest attempts to single step its userspace
+ * we would have to deal with a trapped exception
+ * while in the guest kernel. Because this would be
+ * hard to unwind we suppress the guest's ability to
+ * do so by masking MDSCR_EL.SS.
+ *
+ * This confuses guest debuggers which use
+ * single-step behind the scenes but everything
+ * returns to normal once the host is no longer
+ * debugging the system.
+ */
+ if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) {
+ *vcpu_cpsr(vcpu) |= DBG_SPSR_SS;
+ vcpu_sys_reg(vcpu, MDSCR_EL1) |= DBG_MDSCR_SS;
+ } else {
+ vcpu_sys_reg(vcpu, MDSCR_EL1) &= ~DBG_MDSCR_SS;
+ }
+ }
}
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/guest.c b/arch/arm64/kvm/guest.c
index 22d22c5..48de4f4 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -332,7 +332,9 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *vcpu,
return -EINVAL;
}
-#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/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.4.5
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v7 07/11] KVM: arm64: re-factor hyp.S debug register code
[not found] <1435775343-20034-1-git-send-email-alex.bennee@linaro.org>
` (5 preceding siblings ...)
2015-07-01 18:28 ` [PATCH v7 06/11] KVM: arm64: guest debug, add support for single-step Alex Bennée
@ 2015-07-01 18:28 ` Alex Bennée
2015-07-01 18:29 ` [PATCH v7 08/11] KVM: arm64: introduce vcpu->arch.debug_ptr Alex Bennée
` (3 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Alex Bennée @ 2015-07-01 18:28 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 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>
Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
---
v3:
- return to the patch series
- add save and restore targets
- change register use and document
v4:
- keep original setup/restore names
- don't use split u32/u64 structure yet
v6:
- fix ws and clobber info in hyp.S
v7:
- fix whitespace
- add r-b-tag
---
arch/arm64/kvm/hyp.S | 517 ++++++++++++++-------------------------------------
1 file changed, 138 insertions(+), 379 deletions(-)
diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
index 2c67a14..77c08df 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
-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
+.macro save_debug type
+ // x4: pointer to register set
+ // x5: number of registers to skip
+ // x6..x22 trashed
+
+ adr x22, 1f
+ add x22, x22, x5, lsl #2
+ br x22
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
-
-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
+ 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:
- 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 restore_debug type
+ // x4: pointer to register set
+ // x5: number of registers to skip
+ // x6..x22 trashed
- adr x26, 1f
- add x26, x26, x24, lsl #2
- br x26
+ 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
+ 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:
- 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
-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,61 @@ __restore_sysregs:
restore_sysregs
ret
+/* Save debug state */
__save_debug:
- save_debug
+ // x2: ptr to CPU context
+ // x4/x5/x6-22/x24-26: 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, x2, #CPU_SYSREG_OFFSET(DBGBCR0_EL1)
+ save_debug dbgbcr
+ add x4, x2, #CPU_SYSREG_OFFSET(DBGBVR0_EL1)
+ save_debug dbgbvr
+
+ mov x5, x25
+ add x4, x2, #CPU_SYSREG_OFFSET(DBGWCR0_EL1)
+ save_debug dbgwcr
+ add x4, x2, #CPU_SYSREG_OFFSET(DBGWVR0_EL1)
+ save_debug dbgwvr
+
+ mrs x21, mdccint_el1
+ str x21, [x2, #CPU_SYSREG_OFFSET(MDCCINT_EL1)]
ret
+/* Restore debug state */
__restore_debug:
- restore_debug
+ // x2: ptr to CPU context
+ // x4/x5/x6-22/x24-26: 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, x2, #CPU_SYSREG_OFFSET(DBGBCR0_EL1)
+ restore_debug dbgbcr
+ add x4, x2, #CPU_SYSREG_OFFSET(DBGBVR0_EL1)
+ restore_debug dbgbvr
+
+ mov x5, x25
+ add x4, x2, #CPU_SYSREG_OFFSET(DBGWCR0_EL1)
+ restore_debug dbgwcr
+ add x4, x2, #CPU_SYSREG_OFFSET(DBGWVR0_EL1)
+ restore_debug dbgwvr
+
+ ldr x21, [x2, #CPU_SYSREG_OFFSET(MDCCINT_EL1)]
+ msr mdccint_el1, x21
+
ret
__save_fpsimd:
--
2.4.5
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v7 08/11] KVM: arm64: introduce vcpu->arch.debug_ptr
[not found] <1435775343-20034-1-git-send-email-alex.bennee@linaro.org>
` (6 preceding siblings ...)
2015-07-01 18:28 ` [PATCH v7 07/11] KVM: arm64: re-factor hyp.S debug register code Alex Bennée
@ 2015-07-01 18:29 ` Alex Bennée
2015-07-02 18:34 ` Christoffer Dall
2015-07-03 21:43 ` Christoffer Dall
2015-07-01 18:29 ` [PATCH v7 09/11] KVM: arm64: guest debug, HW assisted debug support Alex Bennée
` (2 subsequent siblings)
10 siblings, 2 replies; 22+ messages in thread
From: Alex Bennée @ 2015-07-01 18:29 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,
Lorenzo Pieralisi, Richard Weinberger, Andre Przywara, open list
This introduces a level of indirection for the debug registers. Instead
of using the sys_regs[] directly we store registers in a structure in
the vcpu. The new kvm_arm_reset_debug_ptr() sets the debug ptr to the
guest context.
This also entails updating the sys_regs code to access this new
structure. New access function have been added for each set of debug
registers. The generic functions are still used for the few registers
stored in the main context.
New access function pointers have been added to the sys_reg_desc
structure to support the GET/SET_ONE_REG ioctl operations.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
v6:
- fix up some ws issues
- correct clobber info
- re-word commentary in kvm_host.h
- fix endian access issues for aarch32 fields
- revert all KVM_GET/SET_ONE_REG to 64bit (also see ABI update)
v7
- new fn kvm_arm_reset_debug_ptr(), stubbed for arm
- split trap fns into bcr,bvr,bcr,wvr and wxvr
- add set/get fns to sys_regs_desc
- reg_to_dbg/dbg_to_reg helpers for 32bit support
---
arch/arm/include/asm/kvm_host.h | 2 +-
arch/arm/kvm/arm.c | 2 +
arch/arm64/include/asm/kvm_asm.h | 24 ++--
arch/arm64/include/asm/kvm_host.h | 17 ++-
arch/arm64/kernel/asm-offsets.c | 6 +
arch/arm64/kvm/debug.c | 9 ++
arch/arm64/kvm/hyp.S | 24 ++--
arch/arm64/kvm/sys_regs.c | 281 ++++++++++++++++++++++++++++++++++----
arch/arm64/kvm/sys_regs.h | 6 +
9 files changed, 321 insertions(+), 50 deletions(-)
diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index 746c0c69..f42759b 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -239,5 +239,5 @@ 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) {}
-
+static inline void kvm_arm_reset_debug_ptr(struct kvm_vcpu *vcpu) {}
#endif /* __ARM_KVM_HOST_H__ */
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index af60e6f..525473f 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -279,6 +279,8 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
/* Set up the timer */
kvm_timer_vcpu_init(vcpu);
+ kvm_arm_reset_debug_ptr(vcpu);
+
return 0;
}
diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
index d6b507e..e997404 100644
--- a/arch/arm64/include/asm/kvm_asm.h
+++ b/arch/arm64/include/asm/kvm_asm.h
@@ -46,24 +46,16 @@
#define CNTKCTL_EL1 20 /* Timer Control Register (EL1) */
#define PAR_EL1 21 /* Physical Address Register */
#define MDSCR_EL1 22 /* Monitor Debug System Control Register */
-#define DBGBCR0_EL1 23 /* Debug Breakpoint Control Registers (0-15) */
-#define DBGBCR15_EL1 38
-#define DBGBVR0_EL1 39 /* Debug Breakpoint Value Registers (0-15) */
-#define DBGBVR15_EL1 54
-#define DBGWCR0_EL1 55 /* Debug Watchpoint Control Registers (0-15) */
-#define DBGWCR15_EL1 70
-#define DBGWVR0_EL1 71 /* Debug Watchpoint Value Registers (0-15) */
-#define DBGWVR15_EL1 86
-#define MDCCINT_EL1 87 /* Monitor Debug Comms Channel Interrupt Enable Reg */
+#define MDCCINT_EL1 23 /* Monitor Debug Comms Channel Interrupt Enable Reg */
/* 32bit specific registers. Keep them at the end of the range */
-#define DACR32_EL2 88 /* Domain Access Control Register */
-#define IFSR32_EL2 89 /* Instruction Fault Status Register */
-#define FPEXC32_EL2 90 /* Floating-Point Exception Control Register */
-#define DBGVCR32_EL2 91 /* Debug Vector Catch Register */
-#define TEECR32_EL1 92 /* ThumbEE Configuration Register */
-#define TEEHBR32_EL1 93 /* ThumbEE Handler Base Register */
-#define NR_SYS_REGS 94
+#define DACR32_EL2 24 /* Domain Access Control Register */
+#define IFSR32_EL2 25 /* Instruction Fault Status Register */
+#define FPEXC32_EL2 26 /* Floating-Point Exception Control Register */
+#define DBGVCR32_EL2 27 /* Debug Vector Catch Register */
+#define TEECR32_EL1 28 /* ThumbEE Configuration Register */
+#define TEEHBR32_EL1 29 /* ThumbEE Handler Base Register */
+#define NR_SYS_REGS 30
/* 32bit mapping */
#define c0_MPIDR (MPIDR_EL1 * 2) /* MultiProcessor ID Register */
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index e2db6a6..461d288 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -108,11 +108,25 @@ struct kvm_vcpu_arch {
/* Exception Information */
struct kvm_vcpu_fault_info fault;
- /* Debug state */
+ /* Guest debug state */
u64 debug_flags;
+ /*
+ * We maintain more than a single set of debug registers to support
+ * debugging the guest from the host and to maintain separate host and
+ * guest state during world switches. vcpu_debug_state are the debug
+ * registers of the vcpu as the guest sees them. host_debug_state are
+ * the host registers which are saved and restored during world switches.
+ *
+ * debug_ptr points to the set of debug registers that should be loaded
+ * onto the hardware when running the guest.
+ */
+ struct kvm_guest_debug_arch *debug_ptr;
+ struct kvm_guest_debug_arch vcpu_debug_state;
+
/* Pointer to host CPU context */
kvm_cpu_context_t *host_cpu_context;
+ struct kvm_guest_debug_arch host_debug_state;
/* VGIC state */
struct vgic_cpu vgic_cpu;
@@ -265,5 +279,6 @@ 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);
+void kvm_arm_reset_debug_ptr(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 dfb25a2..1a8e97c 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -116,10 +116,16 @@ 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));
+ 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));
DEFINE(VCPU_HOST_CONTEXT, offsetof(struct kvm_vcpu, arch.host_cpu_context));
+ DEFINE(VCPU_HOST_DEBUG_STATE, offsetof(struct kvm_vcpu, arch.host_debug_state));
DEFINE(VCPU_TIMER_CNTV_CTL, offsetof(struct kvm_vcpu, arch.timer_cpu.cntv_ctl));
DEFINE(VCPU_TIMER_CNTV_CVAL, offsetof(struct kvm_vcpu, arch.timer_cpu.cntv_cval));
DEFINE(KVM_TIMER_CNTVOFF, offsetof(struct kvm, arch.timer.cntvoff));
diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
index d439eb8..ffefb97 100644
--- a/arch/arm64/kvm/debug.c
+++ b/arch/arm64/kvm/debug.c
@@ -67,6 +67,15 @@ void kvm_arm_init_debug(void)
}
/**
+ * kvm_arm_reset_debug_ptr - set the debug registers to be the guests
+ */
+
+void kvm_arm_reset_debug_ptr(struct kvm_vcpu *vcpu)
+{
+ vcpu->arch.debug_ptr = &vcpu->arch.vcpu_debug_state;
+}
+
+/**
* kvm_arm_setup_debug - set up debug related stuff
*
* @vcpu: the vcpu pointer
diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
index 77c08df..f56e93f 100644
--- a/arch/arm64/kvm/hyp.S
+++ b/arch/arm64/kvm/hyp.S
@@ -600,6 +600,7 @@ __restore_sysregs:
/* Save debug state */
__save_debug:
// x2: ptr to CPU context
+ // x3: ptr to debug reg struct
// x4/x5/x6-22/x24-26: trashed
mrs x26, id_aa64dfr0_el1
@@ -610,15 +611,15 @@ __save_debug:
sub w25, w26, w25 // How many WPs to skip
mov x5, x24
- add x4, x2, #CPU_SYSREG_OFFSET(DBGBCR0_EL1)
+ add x4, x3, #DEBUG_BCR
save_debug dbgbcr
- add x4, x2, #CPU_SYSREG_OFFSET(DBGBVR0_EL1)
+ add x4, x3, #DEBUG_BVR
save_debug dbgbvr
mov x5, x25
- add x4, x2, #CPU_SYSREG_OFFSET(DBGWCR0_EL1)
+ add x4, x3, #DEBUG_WCR
save_debug dbgwcr
- add x4, x2, #CPU_SYSREG_OFFSET(DBGWVR0_EL1)
+ add x4, x3, #DEBUG_WVR
save_debug dbgwvr
mrs x21, mdccint_el1
@@ -628,6 +629,7 @@ __save_debug:
/* Restore debug state */
__restore_debug:
// x2: ptr to CPU context
+ // x3: ptr to debug reg struct
// x4/x5/x6-22/x24-26: trashed
mrs x26, id_aa64dfr0_el1
@@ -638,15 +640,15 @@ __restore_debug:
sub w25, w26, w25 // How many WPs to skip
mov x5, x24
- add x4, x2, #CPU_SYSREG_OFFSET(DBGBCR0_EL1)
+ add x4, x3, #DEBUG_BCR
restore_debug dbgbcr
- add x4, x2, #CPU_SYSREG_OFFSET(DBGBVR0_EL1)
+ add x4, x3, #DEBUG_BVR
restore_debug dbgbvr
mov x5, x25
- add x4, x2, #CPU_SYSREG_OFFSET(DBGWCR0_EL1)
+ add x4, x3, #DEBUG_WCR
restore_debug dbgwcr
- add x4, x2, #CPU_SYSREG_OFFSET(DBGWVR0_EL1)
+ add x4, x3, #DEBUG_WVR
restore_debug dbgwvr
ldr x21, [x2, #CPU_SYSREG_OFFSET(MDCCINT_EL1)]
@@ -686,6 +688,7 @@ ENTRY(__kvm_vcpu_run)
bl __save_sysregs
compute_debug_state 1f
+ add x3, x0, #VCPU_HOST_DEBUG_STATE
bl __save_debug
1:
activate_traps
@@ -701,6 +704,8 @@ ENTRY(__kvm_vcpu_run)
bl __restore_fpsimd
skip_debug_state x3, 1f
+ ldr x3, [x0, #VCPU_DEBUG_PTR]
+ kern_hyp_va x3
bl __restore_debug
1:
restore_guest_32bit_state
@@ -721,6 +726,8 @@ __kvm_vcpu_return:
bl __save_sysregs
skip_debug_state x3, 1f
+ ldr x3, [x0, #VCPU_DEBUG_PTR]
+ kern_hyp_va x3
bl __save_debug
1:
save_guest_32bit_state
@@ -743,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, x0, #VCPU_HOST_DEBUG_STATE
bl __restore_debug
1:
restore_host_regs
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index c370b40..3267b96 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -173,14 +173,14 @@ static bool trap_dbgauthstatus_el1(struct kvm_vcpu *vcpu,
/*
* We want to avoid world-switching all the DBG registers all the
* time:
- *
+ *
* - If we've touched any debug register, it is likely that we're
* going to touch more of them. It then makes sense to disable the
* traps and start doing the save/restore dance
* - If debug is active (DBG_MDSCR_KDE or DBG_MDSCR_MDE set), it is
* then mandatory to save/restore the registers, as the guest
* depends on them.
- *
+ *
* For this, we use a DIRTY bit, indicating the guest has modified the
* debug registers, used as follow:
*
@@ -211,6 +211,187 @@ static bool trap_debug_regs(struct kvm_vcpu *vcpu,
return true;
}
+/*
+ * reg_to_dbg/dbg_to_reg - simple masking for 32bit access
+ */
+static inline unsigned long reg_to_dbg(struct kvm_vcpu *vcpu,
+ const struct sys_reg_params *p)
+{
+ unsigned long val = *vcpu_reg(vcpu, p->Rt);
+ vcpu->arch.debug_flags |= KVM_ARM64_DEBUG_DIRTY;
+ if (p->is_32bit)
+ val &= 0xffffffffUL;
+
+ return val;
+}
+
+static inline void dbg_to_reg(struct kvm_vcpu *vcpu,
+ const struct sys_reg_params *p, unsigned long val)
+{
+ if (p->is_32bit)
+ val &= 0xffffffffUL;
+ *vcpu_reg(vcpu, p->Rt) = val;
+}
+
+static inline bool trap_bvr(struct kvm_vcpu *vcpu,
+ const struct sys_reg_params *p,
+ const struct sys_reg_desc *rd)
+{
+ if (p->is_write) {
+ vcpu->arch.vcpu_debug_state.dbg_bvr[rd->reg] =
+ reg_to_dbg(vcpu, p);
+ } else {
+ dbg_to_reg(vcpu, p,
+ vcpu->arch.vcpu_debug_state.dbg_bvr[rd->reg]);
+ }
+
+ return true;
+}
+
+static int set_bvr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
+ const struct kvm_one_reg *reg, void __user *uaddr)
+{
+ __u64 *r = &vcpu->arch.vcpu_debug_state.dbg_bvr[rd->reg];
+ if (copy_from_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
+ return -EFAULT;
+ return 0;
+}
+
+static int get_bvr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
+ const struct kvm_one_reg *reg, void __user *uaddr)
+{
+ __u64 *r = &vcpu->arch.vcpu_debug_state.dbg_bvr[rd->reg];
+ if (copy_to_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
+ return -EFAULT;
+ return 0;
+}
+
+static inline void reset_bvr(struct kvm_vcpu *vcpu,
+ const struct sys_reg_desc *rd)
+{
+ vcpu->arch.vcpu_debug_state.dbg_bvr[rd->reg] = rd->val;
+}
+
+static inline bool trap_bcr(struct kvm_vcpu *vcpu,
+ const struct sys_reg_params *p,
+ const struct sys_reg_desc *rd)
+{
+ if (p->is_write) {
+ vcpu->arch.vcpu_debug_state.dbg_bcr[rd->reg] =
+ reg_to_dbg(vcpu, p);
+ } else {
+ dbg_to_reg(vcpu, p,
+ vcpu->arch.vcpu_debug_state.dbg_bcr[rd->reg]);
+ }
+
+ return true;
+}
+
+static int set_bcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
+ const struct kvm_one_reg *reg, void __user *uaddr)
+{
+ __u64 *r = &vcpu->arch.vcpu_debug_state.dbg_bcr[rd->reg];
+ if (copy_from_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
+ return -EFAULT;
+ return 0;
+}
+
+static int get_bcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
+ const struct kvm_one_reg *reg, void __user *uaddr)
+{
+ __u64 *r = &vcpu->arch.vcpu_debug_state.dbg_bcr[rd->reg];
+ if (copy_to_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
+ return -EFAULT;
+ return 0;
+}
+
+static inline void reset_bcr(struct kvm_vcpu *vcpu,
+ const struct sys_reg_desc *rd)
+{
+ vcpu->arch.vcpu_debug_state.dbg_bcr[rd->reg] = rd->val;
+}
+
+static inline bool trap_wvr(struct kvm_vcpu *vcpu,
+ const struct sys_reg_params *p,
+ const struct sys_reg_desc *rd)
+{
+ if (p->is_write) {
+ vcpu->arch.vcpu_debug_state.dbg_wvr[rd->reg] =
+ reg_to_dbg(vcpu, p);
+ } else {
+ dbg_to_reg(vcpu, p,
+ vcpu->arch.vcpu_debug_state.dbg_wvr[rd->reg]);
+ }
+
+ return true;
+}
+
+static int set_wvr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
+ const struct kvm_one_reg *reg, void __user *uaddr)
+{
+ __u64 *r = &vcpu->arch.vcpu_debug_state.dbg_wvr[rd->reg];
+ if (copy_from_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
+ return -EFAULT;
+ return 0;
+}
+
+static int get_wvr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
+ const struct kvm_one_reg *reg, void __user *uaddr)
+{
+ __u64 *r = &vcpu->arch.vcpu_debug_state.dbg_wvr[rd->reg];
+ if (copy_to_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
+ return -EFAULT;
+ return 0;
+}
+
+static inline void reset_wvr(struct kvm_vcpu *vcpu,
+ const struct sys_reg_desc *rd)
+{
+ vcpu->arch.vcpu_debug_state.dbg_wvr[rd->reg] = rd->val;
+}
+
+static inline bool trap_wcr(struct kvm_vcpu *vcpu,
+ const struct sys_reg_params *p,
+ const struct sys_reg_desc *rd)
+{
+ if (p->is_write) {
+ vcpu->arch.vcpu_debug_state.dbg_wcr[rd->reg] =
+ reg_to_dbg(vcpu, p);
+ } else {
+ dbg_to_reg(vcpu, p,
+ vcpu->arch.vcpu_debug_state.dbg_wcr[rd->reg]);
+ }
+
+ trace_trap_reg(__func__, rd->reg, p->is_write,
+ vcpu->arch.vcpu_debug_state.dbg_wcr[rd->reg]);
+
+ return true;
+}
+
+static int set_wcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
+ const struct kvm_one_reg *reg, void __user *uaddr)
+{
+ __u64 *r = &vcpu->arch.vcpu_debug_state.dbg_wcr[rd->reg];
+ if (copy_from_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
+ return -EFAULT;
+ return 0;
+}
+
+static int get_wcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
+ const struct kvm_one_reg *reg, void __user *uaddr)
+{
+ __u64 *r = &vcpu->arch.vcpu_debug_state.dbg_wcr[rd->reg];
+ if (copy_to_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
+ return -EFAULT;
+ return 0;
+}
+
+static inline void reset_wcr(struct kvm_vcpu *vcpu,
+ const struct sys_reg_desc *rd)
+{
+ vcpu->arch.vcpu_debug_state.dbg_wcr[rd->reg] = rd->val;
+}
+
static void reset_amair_el1(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
{
u64 amair;
@@ -240,16 +421,16 @@ static void reset_mpidr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
#define DBG_BCR_BVR_WCR_WVR_EL1(n) \
/* DBGBVRn_EL1 */ \
{ Op0(0b10), Op1(0b000), CRn(0b0000), CRm((n)), Op2(0b100), \
- trap_debug_regs, reset_val, (DBGBVR0_EL1 + (n)), 0 }, \
+ trap_bvr, reset_bvr, n, 0, get_bvr, set_bvr }, \
/* DBGBCRn_EL1 */ \
{ Op0(0b10), Op1(0b000), CRn(0b0000), CRm((n)), Op2(0b101), \
- trap_debug_regs, reset_val, (DBGBCR0_EL1 + (n)), 0 }, \
+ trap_bcr, reset_bcr, n, 0, get_bcr, set_bcr }, \
/* DBGWVRn_EL1 */ \
{ Op0(0b10), Op1(0b000), CRn(0b0000), CRm((n)), Op2(0b110), \
- trap_debug_regs, reset_val, (DBGWVR0_EL1 + (n)), 0 }, \
+ trap_wvr, reset_wvr, n, 0, get_wvr, set_wvr }, \
/* DBGWCRn_EL1 */ \
{ Op0(0b10), Op1(0b000), CRn(0b0000), CRm((n)), Op2(0b111), \
- trap_debug_regs, reset_val, (DBGWCR0_EL1 + (n)), 0 }
+ trap_wcr, reset_wcr, n, 0, get_wcr, set_wcr }
/*
* Architected system registers.
@@ -516,28 +697,51 @@ static bool trap_debug32(struct kvm_vcpu *vcpu,
return true;
}
-#define DBG_BCR_BVR_WCR_WVR(n) \
- /* DBGBVRn */ \
- { Op1( 0), CRn( 0), CRm((n)), Op2( 4), trap_debug32, \
- NULL, (cp14_DBGBVR0 + (n) * 2) }, \
- /* DBGBCRn */ \
- { Op1( 0), CRn( 0), CRm((n)), Op2( 5), trap_debug32, \
- NULL, (cp14_DBGBCR0 + (n) * 2) }, \
- /* DBGWVRn */ \
- { Op1( 0), CRn( 0), CRm((n)), Op2( 6), trap_debug32, \
- NULL, (cp14_DBGWVR0 + (n) * 2) }, \
- /* DBGWCRn */ \
- { Op1( 0), CRn( 0), CRm((n)), Op2( 7), trap_debug32, \
- NULL, (cp14_DBGWCR0 + (n) * 2) }
-
-#define DBGBXVR(n) \
- { Op1( 0), CRn( 1), CRm((n)), Op2( 1), trap_debug32, \
- NULL, cp14_DBGBXVR0 + n * 2 }
+/* AArch32 debug register mappings
+ *
+ * AArch32 DBGBVRn is mapped to DBGBVRn_EL1[31:0]
+ * AArch32 DBGBXVRn is mapped to DBGBVRn_EL1[63:32]
+ *
+ * All control registers and watchpoint value registers are mapped to
+ * the lower 32 bits of their AArch64 equivalents. We share the trap
+ * handlers with the above AArch64 code which checks what mode the
+ * system is in.
+ */
+
+static inline bool trap_xvr(struct kvm_vcpu *vcpu,
+ const struct sys_reg_params *p,
+ const struct sys_reg_desc *rd)
+{
+ if (p->is_write) {
+ u64 val = vcpu->arch.vcpu_debug_state.dbg_bvr[rd->reg];
+ val &= 0xffffffffUL;
+ val |= reg_to_dbg(vcpu, p) << 32;
+ vcpu->arch.vcpu_debug_state.dbg_bvr[rd->reg] = val;
+ } else {
+ dbg_to_reg(vcpu, p,
+ vcpu->arch.vcpu_debug_state.dbg_bvr[rd->reg] >> 32);
+ }
+
+ return true;
+}
+
+#define DBG_BCR_BVR_WCR_WVR(n) \
+ /* DBGBVRn */ \
+ { Op1( 0), CRn( 0), CRm((n)), Op2( 4), trap_bvr, NULL, n }, \
+ /* DBGBCRn */ \
+ { Op1( 0), CRn( 0), CRm((n)), Op2( 5), trap_bcr, NULL, n }, \
+ /* DBGWVRn */ \
+ { Op1( 0), CRn( 0), CRm((n)), Op2( 6), trap_wvr, NULL, n }, \
+ /* DBGWCRn */ \
+ { Op1( 0), CRn( 0), CRm((n)), Op2( 7), trap_wcr, NULL, n }
+
+#define DBGBXVR(n) \
+ { Op1( 0), CRn( 1), CRm((n)), Op2( 1), trap_xvr, NULL, n }
/*
* Trapped cp14 registers. We generally ignore most of the external
* debug, on the principle that they don't really make sense to a
- * guest. Revisit this one day, whould this principle change.
+ * guest. Revisit this one day, would this principle change.
*/
static const struct sys_reg_desc cp14_regs[] = {
/* DBGIDR */
@@ -1288,6 +1492,29 @@ static int demux_c15_set(u64 id, void __user *uaddr)
}
}
+/*
+ * Access functions for vcpu_debug_state.
+ */
+#if 0
+static int debug_set64(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
+ const struct kvm_one_reg *reg, void __user *uaddr)
+{
+ __u64 *r = (__u64 *) ((void * )&vcpu->arch.vcpu_debug_state + rd->reg);
+ if (copy_from_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
+ return -EFAULT;
+ return 0;
+}
+
+static int debug_get64(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
+ const struct kvm_one_reg *reg, void __user *uaddr)
+{
+ __u64 *r = (__u64 *) ((void * )&vcpu->arch.vcpu_debug_state + rd->reg);
+ if (copy_to_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
+ return -EFAULT;
+ return 0;
+}
+#endif
+
int kvm_arm_sys_reg_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
{
const struct sys_reg_desc *r;
@@ -1303,6 +1530,9 @@ int kvm_arm_sys_reg_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg
if (!r)
return get_invariant_sys_reg(reg->id, uaddr);
+ if (r->get)
+ return (r->get)(vcpu, r, reg, uaddr);
+
return reg_to_user(uaddr, &vcpu_sys_reg(vcpu, r->reg), reg->id);
}
@@ -1321,6 +1551,9 @@ int kvm_arm_sys_reg_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg
if (!r)
return set_invariant_sys_reg(reg->id, uaddr);
+ if (r->set)
+ return (r->set)(vcpu, r, reg, uaddr);
+
return reg_from_user(&vcpu_sys_reg(vcpu, r->reg), uaddr, reg->id);
}
diff --git a/arch/arm64/kvm/sys_regs.h b/arch/arm64/kvm/sys_regs.h
index d411e25..9265e7d 100644
--- a/arch/arm64/kvm/sys_regs.h
+++ b/arch/arm64/kvm/sys_regs.h
@@ -55,6 +55,12 @@ struct sys_reg_desc {
/* Value (usually reset value) */
u64 val;
+
+ /* Get/Set functions, fallback if NULL */
+ int (*get)(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
+ const struct kvm_one_reg *reg, void __user *uaddr);
+ int (*set)(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
+ const struct kvm_one_reg *reg, void __user *uaddr);
};
static inline void print_sys_reg_instr(const struct sys_reg_params *p)
--
2.4.5
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v7 08/11] KVM: arm64: introduce vcpu->arch.debug_ptr
2015-07-01 18:29 ` [PATCH v7 08/11] KVM: arm64: introduce vcpu->arch.debug_ptr Alex Bennée
@ 2015-07-02 18:34 ` Christoffer Dall
2015-07-03 7:14 ` Alex Bennée
2015-07-03 21:43 ` Christoffer Dall
1 sibling, 1 reply; 22+ messages in thread
From: Christoffer Dall @ 2015-07-02 18:34 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, Lorenzo Pieralisi, Richard Weinberger,
Andre Przywara, open list
On Wed, Jul 01, 2015 at 07:29:00PM +0100, Alex Bennée wrote:
> This introduces a level of indirection for the debug registers. Instead
> of using the sys_regs[] directly we store registers in a structure in
> the vcpu. The new kvm_arm_reset_debug_ptr() sets the debug ptr to the
> guest context.
>
> This also entails updating the sys_regs code to access this new
> structure. New access function have been added for each set of debug
> registers. The generic functions are still used for the few registers
> stored in the main context.
>
> New access function pointers have been added to the sys_reg_desc
> structure to support the GET/SET_ONE_REG ioctl operations.
Why is this needed?
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> ---
> v6:
> - fix up some ws issues
> - correct clobber info
> - re-word commentary in kvm_host.h
> - fix endian access issues for aarch32 fields
> - revert all KVM_GET/SET_ONE_REG to 64bit (also see ABI update)
> v7
> - new fn kvm_arm_reset_debug_ptr(), stubbed for arm
> - split trap fns into bcr,bvr,bcr,wvr and wxvr
> - add set/get fns to sys_regs_desc
> - reg_to_dbg/dbg_to_reg helpers for 32bit support
> ---
> arch/arm/include/asm/kvm_host.h | 2 +-
> arch/arm/kvm/arm.c | 2 +
> arch/arm64/include/asm/kvm_asm.h | 24 ++--
> arch/arm64/include/asm/kvm_host.h | 17 ++-
> arch/arm64/kernel/asm-offsets.c | 6 +
> arch/arm64/kvm/debug.c | 9 ++
> arch/arm64/kvm/hyp.S | 24 ++--
> arch/arm64/kvm/sys_regs.c | 281 ++++++++++++++++++++++++++++++++++----
> arch/arm64/kvm/sys_regs.h | 6 +
> 9 files changed, 321 insertions(+), 50 deletions(-)
>
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index 746c0c69..f42759b 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -239,5 +239,5 @@ 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) {}
> -
> +static inline void kvm_arm_reset_debug_ptr(struct kvm_vcpu *vcpu) {}
seems like a strange decision to nuke the whitespace for adding this
function ;)
> #endif /* __ARM_KVM_HOST_H__ */
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index af60e6f..525473f 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -279,6 +279,8 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
> /* Set up the timer */
> kvm_timer_vcpu_init(vcpu);
>
> + kvm_arm_reset_debug_ptr(vcpu);
> +
> return 0;
> }
>
> diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
> index d6b507e..e997404 100644
> --- a/arch/arm64/include/asm/kvm_asm.h
> +++ b/arch/arm64/include/asm/kvm_asm.h
> @@ -46,24 +46,16 @@
> #define CNTKCTL_EL1 20 /* Timer Control Register (EL1) */
> #define PAR_EL1 21 /* Physical Address Register */
> #define MDSCR_EL1 22 /* Monitor Debug System Control Register */
> -#define DBGBCR0_EL1 23 /* Debug Breakpoint Control Registers (0-15) */
> -#define DBGBCR15_EL1 38
> -#define DBGBVR0_EL1 39 /* Debug Breakpoint Value Registers (0-15) */
> -#define DBGBVR15_EL1 54
> -#define DBGWCR0_EL1 55 /* Debug Watchpoint Control Registers (0-15) */
> -#define DBGWCR15_EL1 70
> -#define DBGWVR0_EL1 71 /* Debug Watchpoint Value Registers (0-15) */
> -#define DBGWVR15_EL1 86
> -#define MDCCINT_EL1 87 /* Monitor Debug Comms Channel Interrupt Enable Reg */
> +#define MDCCINT_EL1 23 /* Monitor Debug Comms Channel Interrupt Enable Reg */
>
> /* 32bit specific registers. Keep them at the end of the range */
> -#define DACR32_EL2 88 /* Domain Access Control Register */
> -#define IFSR32_EL2 89 /* Instruction Fault Status Register */
> -#define FPEXC32_EL2 90 /* Floating-Point Exception Control Register */
> -#define DBGVCR32_EL2 91 /* Debug Vector Catch Register */
> -#define TEECR32_EL1 92 /* ThumbEE Configuration Register */
> -#define TEEHBR32_EL1 93 /* ThumbEE Handler Base Register */
> -#define NR_SYS_REGS 94
> +#define DACR32_EL2 24 /* Domain Access Control Register */
> +#define IFSR32_EL2 25 /* Instruction Fault Status Register */
> +#define FPEXC32_EL2 26 /* Floating-Point Exception Control Register */
> +#define DBGVCR32_EL2 27 /* Debug Vector Catch Register */
> +#define TEECR32_EL1 28 /* ThumbEE Configuration Register */
> +#define TEEHBR32_EL1 29 /* ThumbEE Handler Base Register */
> +#define NR_SYS_REGS 30
>
> /* 32bit mapping */
> #define c0_MPIDR (MPIDR_EL1 * 2) /* MultiProcessor ID Register */
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index e2db6a6..461d288 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -108,11 +108,25 @@ struct kvm_vcpu_arch {
> /* Exception Information */
> struct kvm_vcpu_fault_info fault;
>
> - /* Debug state */
> + /* Guest debug state */
> u64 debug_flags;
>
> + /*
> + * We maintain more than a single set of debug registers to support
> + * debugging the guest from the host and to maintain separate host and
> + * guest state during world switches. vcpu_debug_state are the debug
> + * registers of the vcpu as the guest sees them. host_debug_state are
> + * the host registers which are saved and restored during world switches.
> + *
> + * debug_ptr points to the set of debug registers that should be loaded
> + * onto the hardware when running the guest.
> + */
> + struct kvm_guest_debug_arch *debug_ptr;
> + struct kvm_guest_debug_arch vcpu_debug_state;
> +
> /* Pointer to host CPU context */
> kvm_cpu_context_t *host_cpu_context;
> + struct kvm_guest_debug_arch host_debug_state;
>
> /* VGIC state */
> struct vgic_cpu vgic_cpu;
> @@ -265,5 +279,6 @@ 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);
> +void kvm_arm_reset_debug_ptr(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 dfb25a2..1a8e97c 100644
> --- a/arch/arm64/kernel/asm-offsets.c
> +++ b/arch/arm64/kernel/asm-offsets.c
> @@ -116,10 +116,16 @@ 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));
> + 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));
> DEFINE(VCPU_HOST_CONTEXT, offsetof(struct kvm_vcpu, arch.host_cpu_context));
> + DEFINE(VCPU_HOST_DEBUG_STATE, offsetof(struct kvm_vcpu, arch.host_debug_state));
> DEFINE(VCPU_TIMER_CNTV_CTL, offsetof(struct kvm_vcpu, arch.timer_cpu.cntv_ctl));
> DEFINE(VCPU_TIMER_CNTV_CVAL, offsetof(struct kvm_vcpu, arch.timer_cpu.cntv_cval));
> DEFINE(KVM_TIMER_CNTVOFF, offsetof(struct kvm, arch.timer.cntvoff));
> diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
> index d439eb8..ffefb97 100644
> --- a/arch/arm64/kvm/debug.c
> +++ b/arch/arm64/kvm/debug.c
> @@ -67,6 +67,15 @@ void kvm_arm_init_debug(void)
> }
>
> /**
> + * kvm_arm_reset_debug_ptr - set the debug registers to be the guests
nit: guest's, but I actually prefer:
kvm_arm_reset_debug_ptr - reset the debug ptr to point to the vcpu state
> + */
> +
> +void kvm_arm_reset_debug_ptr(struct kvm_vcpu *vcpu)
> +{
> + vcpu->arch.debug_ptr = &vcpu->arch.vcpu_debug_state;
> +}
> +
> +/**
> * kvm_arm_setup_debug - set up debug related stuff
> *
> * @vcpu: the vcpu pointer
> diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
> index 77c08df..f56e93f 100644
> --- a/arch/arm64/kvm/hyp.S
> +++ b/arch/arm64/kvm/hyp.S
> @@ -600,6 +600,7 @@ __restore_sysregs:
> /* Save debug state */
> __save_debug:
> // x2: ptr to CPU context
> + // x3: ptr to debug reg struct
> // x4/x5/x6-22/x24-26: trashed
>
> mrs x26, id_aa64dfr0_el1
> @@ -610,15 +611,15 @@ __save_debug:
> sub w25, w26, w25 // How many WPs to skip
>
> mov x5, x24
> - add x4, x2, #CPU_SYSREG_OFFSET(DBGBCR0_EL1)
> + add x4, x3, #DEBUG_BCR
> save_debug dbgbcr
> - add x4, x2, #CPU_SYSREG_OFFSET(DBGBVR0_EL1)
> + add x4, x3, #DEBUG_BVR
> save_debug dbgbvr
>
> mov x5, x25
> - add x4, x2, #CPU_SYSREG_OFFSET(DBGWCR0_EL1)
> + add x4, x3, #DEBUG_WCR
> save_debug dbgwcr
> - add x4, x2, #CPU_SYSREG_OFFSET(DBGWVR0_EL1)
> + add x4, x3, #DEBUG_WVR
> save_debug dbgwvr
>
> mrs x21, mdccint_el1
> @@ -628,6 +629,7 @@ __save_debug:
> /* Restore debug state */
> __restore_debug:
> // x2: ptr to CPU context
> + // x3: ptr to debug reg struct
> // x4/x5/x6-22/x24-26: trashed
>
> mrs x26, id_aa64dfr0_el1
> @@ -638,15 +640,15 @@ __restore_debug:
> sub w25, w26, w25 // How many WPs to skip
>
> mov x5, x24
> - add x4, x2, #CPU_SYSREG_OFFSET(DBGBCR0_EL1)
> + add x4, x3, #DEBUG_BCR
> restore_debug dbgbcr
> - add x4, x2, #CPU_SYSREG_OFFSET(DBGBVR0_EL1)
> + add x4, x3, #DEBUG_BVR
> restore_debug dbgbvr
>
> mov x5, x25
> - add x4, x2, #CPU_SYSREG_OFFSET(DBGWCR0_EL1)
> + add x4, x3, #DEBUG_WCR
> restore_debug dbgwcr
> - add x4, x2, #CPU_SYSREG_OFFSET(DBGWVR0_EL1)
> + add x4, x3, #DEBUG_WVR
> restore_debug dbgwvr
>
> ldr x21, [x2, #CPU_SYSREG_OFFSET(MDCCINT_EL1)]
> @@ -686,6 +688,7 @@ ENTRY(__kvm_vcpu_run)
> bl __save_sysregs
>
> compute_debug_state 1f
> + add x3, x0, #VCPU_HOST_DEBUG_STATE
> bl __save_debug
> 1:
> activate_traps
> @@ -701,6 +704,8 @@ ENTRY(__kvm_vcpu_run)
> bl __restore_fpsimd
>
> skip_debug_state x3, 1f
> + ldr x3, [x0, #VCPU_DEBUG_PTR]
> + kern_hyp_va x3
> bl __restore_debug
> 1:
> restore_guest_32bit_state
> @@ -721,6 +726,8 @@ __kvm_vcpu_return:
> bl __save_sysregs
>
> skip_debug_state x3, 1f
> + ldr x3, [x0, #VCPU_DEBUG_PTR]
> + kern_hyp_va x3
> bl __save_debug
> 1:
> save_guest_32bit_state
> @@ -743,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, x0, #VCPU_HOST_DEBUG_STATE
> bl __restore_debug
> 1:
> restore_host_regs
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index c370b40..3267b96 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -173,14 +173,14 @@ static bool trap_dbgauthstatus_el1(struct kvm_vcpu *vcpu,
> /*
> * We want to avoid world-switching all the DBG registers all the
> * time:
> - *
> + *
> * - If we've touched any debug register, it is likely that we're
> * going to touch more of them. It then makes sense to disable the
> * traps and start doing the save/restore dance
> * - If debug is active (DBG_MDSCR_KDE or DBG_MDSCR_MDE set), it is
> * then mandatory to save/restore the registers, as the guest
> * depends on them.
> - *
> + *
stray whitespace changes?
> * For this, we use a DIRTY bit, indicating the guest has modified the
> * debug registers, used as follow:
> *
> @@ -211,6 +211,187 @@ static bool trap_debug_regs(struct kvm_vcpu *vcpu,
> return true;
> }
>
> +/*
> + * reg_to_dbg/dbg_to_reg - simple masking for 32bit access
> + */
> +static inline unsigned long reg_to_dbg(struct kvm_vcpu *vcpu,
> + const struct sys_reg_params *p)
> +{
> + unsigned long val = *vcpu_reg(vcpu, p->Rt);
> + vcpu->arch.debug_flags |= KVM_ARM64_DEBUG_DIRTY;
> + if (p->is_32bit)
> + val &= 0xffffffffUL;
> +
> + return val;
> +}
> +
> +static inline void dbg_to_reg(struct kvm_vcpu *vcpu,
> + const struct sys_reg_params *p, unsigned long val)
> +{
> + if (p->is_32bit)
> + val &= 0xffffffffUL;
> + *vcpu_reg(vcpu, p->Rt) = val;
> +}
> +
> +static inline bool trap_bvr(struct kvm_vcpu *vcpu,
> + const struct sys_reg_params *p,
> + const struct sys_reg_desc *rd)
> +{
> + if (p->is_write) {
> + vcpu->arch.vcpu_debug_state.dbg_bvr[rd->reg] =
> + reg_to_dbg(vcpu, p);
> + } else {
> + dbg_to_reg(vcpu, p,
> + vcpu->arch.vcpu_debug_state.dbg_bvr[rd->reg]);
> + }
> +
> + return true;
> +}
> +
> +static int set_bvr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> + const struct kvm_one_reg *reg, void __user *uaddr)
> +{
> + __u64 *r = &vcpu->arch.vcpu_debug_state.dbg_bvr[rd->reg];
> + if (copy_from_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
> + return -EFAULT;
> + return 0;
> +}
> +
> +static int get_bvr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> + const struct kvm_one_reg *reg, void __user *uaddr)
> +{
> + __u64 *r = &vcpu->arch.vcpu_debug_state.dbg_bvr[rd->reg];
> + if (copy_to_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
> + return -EFAULT;
> + return 0;
> +}
> +
> +static inline void reset_bvr(struct kvm_vcpu *vcpu,
> + const struct sys_reg_desc *rd)
> +{
> + vcpu->arch.vcpu_debug_state.dbg_bvr[rd->reg] = rd->val;
> +}
> +
> +static inline bool trap_bcr(struct kvm_vcpu *vcpu,
> + const struct sys_reg_params *p,
> + const struct sys_reg_desc *rd)
> +{
> + if (p->is_write) {
> + vcpu->arch.vcpu_debug_state.dbg_bcr[rd->reg] =
> + reg_to_dbg(vcpu, p);
> + } else {
> + dbg_to_reg(vcpu, p,
> + vcpu->arch.vcpu_debug_state.dbg_bcr[rd->reg]);
> + }
> +
> + return true;
> +}
> +
> +static int set_bcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> + const struct kvm_one_reg *reg, void __user *uaddr)
> +{
> + __u64 *r = &vcpu->arch.vcpu_debug_state.dbg_bcr[rd->reg];
> + if (copy_from_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
> + return -EFAULT;
> + return 0;
> +}
> +
> +static int get_bcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> + const struct kvm_one_reg *reg, void __user *uaddr)
> +{
> + __u64 *r = &vcpu->arch.vcpu_debug_state.dbg_bcr[rd->reg];
> + if (copy_to_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
> + return -EFAULT;
> + return 0;
> +}
> +
> +static inline void reset_bcr(struct kvm_vcpu *vcpu,
> + const struct sys_reg_desc *rd)
> +{
> + vcpu->arch.vcpu_debug_state.dbg_bcr[rd->reg] = rd->val;
> +}
> +
> +static inline bool trap_wvr(struct kvm_vcpu *vcpu,
> + const struct sys_reg_params *p,
> + const struct sys_reg_desc *rd)
> +{
> + if (p->is_write) {
> + vcpu->arch.vcpu_debug_state.dbg_wvr[rd->reg] =
> + reg_to_dbg(vcpu, p);
> + } else {
> + dbg_to_reg(vcpu, p,
> + vcpu->arch.vcpu_debug_state.dbg_wvr[rd->reg]);
> + }
> +
> + return true;
> +}
> +
> +static int set_wvr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> + const struct kvm_one_reg *reg, void __user *uaddr)
> +{
> + __u64 *r = &vcpu->arch.vcpu_debug_state.dbg_wvr[rd->reg];
> + if (copy_from_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
> + return -EFAULT;
> + return 0;
> +}
> +
> +static int get_wvr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> + const struct kvm_one_reg *reg, void __user *uaddr)
> +{
> + __u64 *r = &vcpu->arch.vcpu_debug_state.dbg_wvr[rd->reg];
> + if (copy_to_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
> + return -EFAULT;
> + return 0;
> +}
> +
> +static inline void reset_wvr(struct kvm_vcpu *vcpu,
> + const struct sys_reg_desc *rd)
> +{
> + vcpu->arch.vcpu_debug_state.dbg_wvr[rd->reg] = rd->val;
> +}
> +
> +static inline bool trap_wcr(struct kvm_vcpu *vcpu,
> + const struct sys_reg_params *p,
> + const struct sys_reg_desc *rd)
> +{
> + if (p->is_write) {
> + vcpu->arch.vcpu_debug_state.dbg_wcr[rd->reg] =
> + reg_to_dbg(vcpu, p);
> + } else {
> + dbg_to_reg(vcpu, p,
> + vcpu->arch.vcpu_debug_state.dbg_wcr[rd->reg]);
> + }
> +
> + trace_trap_reg(__func__, rd->reg, p->is_write,
> + vcpu->arch.vcpu_debug_state.dbg_wcr[rd->reg]);
> +
> + return true;
> +}
> +
> +static int set_wcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> + const struct kvm_one_reg *reg, void __user *uaddr)
> +{
> + __u64 *r = &vcpu->arch.vcpu_debug_state.dbg_wcr[rd->reg];
> + if (copy_from_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
> + return -EFAULT;
> + return 0;
> +}
> +
> +static int get_wcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> + const struct kvm_one_reg *reg, void __user *uaddr)
> +{
> + __u64 *r = &vcpu->arch.vcpu_debug_state.dbg_wcr[rd->reg];
> + if (copy_to_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
> + return -EFAULT;
> + return 0;
> +}
> +
> +static inline void reset_wcr(struct kvm_vcpu *vcpu,
> + const struct sys_reg_desc *rd)
> +{
> + vcpu->arch.vcpu_debug_state.dbg_wcr[rd->reg] = rd->val;
> +}
> +
> static void reset_amair_el1(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
> {
> u64 amair;
> @@ -240,16 +421,16 @@ static void reset_mpidr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
> #define DBG_BCR_BVR_WCR_WVR_EL1(n) \
> /* DBGBVRn_EL1 */ \
> { Op0(0b10), Op1(0b000), CRn(0b0000), CRm((n)), Op2(0b100), \
> - trap_debug_regs, reset_val, (DBGBVR0_EL1 + (n)), 0 }, \
> + trap_bvr, reset_bvr, n, 0, get_bvr, set_bvr }, \
> /* DBGBCRn_EL1 */ \
> { Op0(0b10), Op1(0b000), CRn(0b0000), CRm((n)), Op2(0b101), \
> - trap_debug_regs, reset_val, (DBGBCR0_EL1 + (n)), 0 }, \
> + trap_bcr, reset_bcr, n, 0, get_bcr, set_bcr }, \
> /* DBGWVRn_EL1 */ \
> { Op0(0b10), Op1(0b000), CRn(0b0000), CRm((n)), Op2(0b110), \
> - trap_debug_regs, reset_val, (DBGWVR0_EL1 + (n)), 0 }, \
> + trap_wvr, reset_wvr, n, 0, get_wvr, set_wvr }, \
> /* DBGWCRn_EL1 */ \
> { Op0(0b10), Op1(0b000), CRn(0b0000), CRm((n)), Op2(0b111), \
> - trap_debug_regs, reset_val, (DBGWCR0_EL1 + (n)), 0 }
> + trap_wcr, reset_wcr, n, 0, get_wcr, set_wcr }
>
> /*
> * Architected system registers.
> @@ -516,28 +697,51 @@ static bool trap_debug32(struct kvm_vcpu *vcpu,
> return true;
> }
>
> -#define DBG_BCR_BVR_WCR_WVR(n) \
> - /* DBGBVRn */ \
> - { Op1( 0), CRn( 0), CRm((n)), Op2( 4), trap_debug32, \
> - NULL, (cp14_DBGBVR0 + (n) * 2) }, \
> - /* DBGBCRn */ \
> - { Op1( 0), CRn( 0), CRm((n)), Op2( 5), trap_debug32, \
> - NULL, (cp14_DBGBCR0 + (n) * 2) }, \
> - /* DBGWVRn */ \
> - { Op1( 0), CRn( 0), CRm((n)), Op2( 6), trap_debug32, \
> - NULL, (cp14_DBGWVR0 + (n) * 2) }, \
> - /* DBGWCRn */ \
> - { Op1( 0), CRn( 0), CRm((n)), Op2( 7), trap_debug32, \
> - NULL, (cp14_DBGWCR0 + (n) * 2) }
> -
> -#define DBGBXVR(n) \
> - { Op1( 0), CRn( 1), CRm((n)), Op2( 1), trap_debug32, \
> - NULL, cp14_DBGBXVR0 + n * 2 }
> +/* AArch32 debug register mappings
> + *
> + * AArch32 DBGBVRn is mapped to DBGBVRn_EL1[31:0]
> + * AArch32 DBGBXVRn is mapped to DBGBVRn_EL1[63:32]
> + *
> + * All control registers and watchpoint value registers are mapped to
> + * the lower 32 bits of their AArch64 equivalents. We share the trap
> + * handlers with the above AArch64 code which checks what mode the
> + * system is in.
> + */
> +
> +static inline bool trap_xvr(struct kvm_vcpu *vcpu,
> + const struct sys_reg_params *p,
> + const struct sys_reg_desc *rd)
> +{
> + if (p->is_write) {
> + u64 val = vcpu->arch.vcpu_debug_state.dbg_bvr[rd->reg];
> + val &= 0xffffffffUL;
> + val |= reg_to_dbg(vcpu, p) << 32;
> + vcpu->arch.vcpu_debug_state.dbg_bvr[rd->reg] = val;
> + } else {
> + dbg_to_reg(vcpu, p,
> + vcpu->arch.vcpu_debug_state.dbg_bvr[rd->reg] >> 32);
> + }
> +
> + return true;
> +}
> +
> +#define DBG_BCR_BVR_WCR_WVR(n) \
> + /* DBGBVRn */ \
> + { Op1( 0), CRn( 0), CRm((n)), Op2( 4), trap_bvr, NULL, n }, \
> + /* DBGBCRn */ \
> + { Op1( 0), CRn( 0), CRm((n)), Op2( 5), trap_bcr, NULL, n }, \
> + /* DBGWVRn */ \
> + { Op1( 0), CRn( 0), CRm((n)), Op2( 6), trap_wvr, NULL, n }, \
> + /* DBGWCRn */ \
> + { Op1( 0), CRn( 0), CRm((n)), Op2( 7), trap_wcr, NULL, n }
> +
> +#define DBGBXVR(n) \
> + { Op1( 0), CRn( 1), CRm((n)), Op2( 1), trap_xvr, NULL, n }
>
> /*
> * Trapped cp14 registers. We generally ignore most of the external
> * debug, on the principle that they don't really make sense to a
> - * guest. Revisit this one day, whould this principle change.
> + * guest. Revisit this one day, would this principle change.
> */
> static const struct sys_reg_desc cp14_regs[] = {
> /* DBGIDR */
> @@ -1288,6 +1492,29 @@ static int demux_c15_set(u64 id, void __user *uaddr)
> }
> }
>
> +/*
> + * Access functions for vcpu_debug_state.
> + */
> +#if 0
> +static int debug_set64(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> + const struct kvm_one_reg *reg, void __user *uaddr)
> +{
> + __u64 *r = (__u64 *) ((void * )&vcpu->arch.vcpu_debug_state + rd->reg);
> + if (copy_from_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
> + return -EFAULT;
> + return 0;
> +}
> +
> +static int debug_get64(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> + const struct kvm_one_reg *reg, void __user *uaddr)
> +{
> + __u64 *r = (__u64 *) ((void * )&vcpu->arch.vcpu_debug_state + rd->reg);
> + if (copy_to_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
> + return -EFAULT;
> + return 0;
> +}
> +#endif
> +
what is this ifdef'ed block of code doing here?
> int kvm_arm_sys_reg_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
> {
> const struct sys_reg_desc *r;
> @@ -1303,6 +1530,9 @@ int kvm_arm_sys_reg_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg
> if (!r)
> return get_invariant_sys_reg(reg->id, uaddr);
>
> + if (r->get)
> + return (r->get)(vcpu, r, reg, uaddr);
> +
> return reg_to_user(uaddr, &vcpu_sys_reg(vcpu, r->reg), reg->id);
> }
>
> @@ -1321,6 +1551,9 @@ int kvm_arm_sys_reg_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg
> if (!r)
> return set_invariant_sys_reg(reg->id, uaddr);
>
> + if (r->set)
> + return (r->set)(vcpu, r, reg, uaddr);
> +
> return reg_from_user(&vcpu_sys_reg(vcpu, r->reg), uaddr, reg->id);
> }
>
> diff --git a/arch/arm64/kvm/sys_regs.h b/arch/arm64/kvm/sys_regs.h
> index d411e25..9265e7d 100644
> --- a/arch/arm64/kvm/sys_regs.h
> +++ b/arch/arm64/kvm/sys_regs.h
> @@ -55,6 +55,12 @@ struct sys_reg_desc {
>
> /* Value (usually reset value) */
> u64 val;
> +
> + /* Get/Set functions, fallback if NULL */
Is this only meant for usersapce access or when should one use these?
> + int (*get)(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> + const struct kvm_one_reg *reg, void __user *uaddr);
> + int (*set)(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> + const struct kvm_one_reg *reg, void __user *uaddr);
> };
>
> static inline void print_sys_reg_instr(const struct sys_reg_params *p)
> --
> 2.4.5
>
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v7 08/11] KVM: arm64: introduce vcpu->arch.debug_ptr
2015-07-02 18:34 ` Christoffer Dall
@ 2015-07-03 7:14 ` Alex Bennée
2015-07-03 21:46 ` Christoffer Dall
0 siblings, 1 reply; 22+ messages in thread
From: Alex Bennée @ 2015-07-03 7:14 UTC (permalink / raw)
To: Christoffer Dall
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, Lorenzo Pieralisi, Richard Weinberger,
Andre Przywara, open list
Christoffer Dall <christoffer.dall@linaro.org> writes:
> On Wed, Jul 01, 2015 at 07:29:00PM +0100, Alex Bennée wrote:
>> This introduces a level of indirection for the debug registers. Instead
>> of using the sys_regs[] directly we store registers in a structure in
>> the vcpu. The new kvm_arm_reset_debug_ptr() sets the debug ptr to the
>> guest context.
>>
>> This also entails updating the sys_regs code to access this new
>> structure. New access function have been added for each set of debug
>> registers. The generic functions are still used for the few registers
>> stored in the main context.
>>
>> New access function pointers have been added to the sys_reg_desc
>> structure to support the GET/SET_ONE_REG ioctl operations.
>
> Why is this needed?
Previously I had a hacky:
if (r->access == trap_debug64)
return debug_get64(vcpu, r, reg, uaddr);
Which used the same offset information. Now we have a cleaner:
if (r->set)
return (r->set)(vcpu, r, reg, uaddr);
Which accesses the structure directly, as the trap functions do:
__u64 *r = &vcpu->arch.vcpu_debug_state.dbg_bvr[rd->reg];
if (copy_from_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
return -EFAULT;
return 0;
<snip>
>> +#if 0
>> +static int debug_set64(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
>> + const struct kvm_one_reg *reg, void __user *uaddr)
>> +{
>> + __u64 *r = (__u64 *) ((void * )&vcpu->arch.vcpu_debug_state + rd->reg);
>> + if (copy_from_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
>> + return -EFAULT;
>> + return 0;
>> +}
>> +
>> +static int debug_get64(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
>> + const struct kvm_one_reg *reg, void __user *uaddr)
>> +{
>> + __u64 *r = (__u64 *) ((void * )&vcpu->arch.vcpu_debug_state + rd->reg);
>> + if (copy_to_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
>> + return -EFAULT;
>> + return 0;
>> +}
>> +#endif
>> +
>
> what is this ifdef'ed block of code doing here?
Oops. Yeah looks like I missed removing that after I finished the
re-factor. These where the old get/set functions I used.
>
>> int kvm_arm_sys_reg_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
>> {
>> const struct sys_reg_desc *r;
>> @@ -1303,6 +1530,9 @@ int kvm_arm_sys_reg_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg
>> if (!r)
>> return get_invariant_sys_reg(reg->id, uaddr);
>>
>> + if (r->get)
>> + return (r->get)(vcpu, r, reg, uaddr);
>> +
>> return reg_to_user(uaddr, &vcpu_sys_reg(vcpu, r->reg), reg->id);
>> }
>>
>> @@ -1321,6 +1551,9 @@ int kvm_arm_sys_reg_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg
>> if (!r)
>> return set_invariant_sys_reg(reg->id, uaddr);
>>
>> + if (r->set)
>> + return (r->set)(vcpu, r, reg, uaddr);
>> +
>> return reg_from_user(&vcpu_sys_reg(vcpu, r->reg), uaddr, reg->id);
>> }
>>
>> diff --git a/arch/arm64/kvm/sys_regs.h b/arch/arm64/kvm/sys_regs.h
>> index d411e25..9265e7d 100644
>> --- a/arch/arm64/kvm/sys_regs.h
>> +++ b/arch/arm64/kvm/sys_regs.h
>> @@ -55,6 +55,12 @@ struct sys_reg_desc {
>>
>> /* Value (usually reset value) */
>> u64 val;
>> +
>> + /* Get/Set functions, fallback if NULL */
>
> Is this only meant for usersapce access or when should one use these?
Yes for GET/SET
>
>> + int (*get)(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
>> + const struct kvm_one_reg *reg, void __user *uaddr);
>> + int (*set)(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
>> + const struct kvm_one_reg *reg, void __user *uaddr);
>> };
>>
>> static inline void print_sys_reg_instr(const struct sys_reg_params *p)
>> --
>> 2.4.5
>>
--
Alex Bennée
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v7 08/11] KVM: arm64: introduce vcpu->arch.debug_ptr
2015-07-03 7:14 ` Alex Bennée
@ 2015-07-03 21:46 ` Christoffer Dall
0 siblings, 0 replies; 22+ messages in thread
From: Christoffer Dall @ 2015-07-03 21: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, Russell King, Catalin Marinas, Will Deacon,
Ard Biesheuvel, Lorenzo Pieralisi, Richard Weinberger,
Andre Przywara, open list
On Fri, Jul 03, 2015 at 08:14:52AM +0100, Alex Bennée wrote:
>
> Christoffer Dall <christoffer.dall@linaro.org> writes:
>
> > On Wed, Jul 01, 2015 at 07:29:00PM +0100, Alex Bennée wrote:
> >> This introduces a level of indirection for the debug registers. Instead
> >> of using the sys_regs[] directly we store registers in a structure in
> >> the vcpu. The new kvm_arm_reset_debug_ptr() sets the debug ptr to the
> >> guest context.
> >>
> >> This also entails updating the sys_regs code to access this new
> >> structure. New access function have been added for each set of debug
> >> registers. The generic functions are still used for the few registers
> >> stored in the main context.
> >>
> >> New access function pointers have been added to the sys_reg_desc
> >> structure to support the GET/SET_ONE_REG ioctl operations.
> >
> > Why is this needed?
>
> Previously I had a hacky:
>
> if (r->access == trap_debug64)
> return debug_get64(vcpu, r, reg, uaddr);
>
> Which used the same offset information. Now we have a cleaner:
>
> if (r->set)
> return (r->set)(vcpu, r, reg, uaddr);
>
> Which accesses the structure directly, as the trap functions do:
>
> __u64 *r = &vcpu->arch.vcpu_debug_state.dbg_bvr[rd->reg];
> if (copy_from_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
> return -EFAULT;
> return 0;
I get it now, had to take another look at sys_regs.c.
I think this commit message needs a littel tweaking saying "Because we
no longer give the sys_regs offset for the sys_reg_desc->reg field, but
instead the index into a debug-specific struct, the generic user-space
access code no longer works, and we are therefore forced to add specific
user_set/get functions for these registers."
At least it was hard for me to understand the rationale without this,
and I think it would be good to have for someone doing git blame later.
>
> <snip>
> >> +#if 0
> >> +static int debug_set64(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> >> + const struct kvm_one_reg *reg, void __user *uaddr)
> >> +{
> >> + __u64 *r = (__u64 *) ((void * )&vcpu->arch.vcpu_debug_state + rd->reg);
> >> + if (copy_from_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
> >> + return -EFAULT;
> >> + return 0;
> >> +}
> >> +
> >> +static int debug_get64(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> >> + const struct kvm_one_reg *reg, void __user *uaddr)
> >> +{
> >> + __u64 *r = (__u64 *) ((void * )&vcpu->arch.vcpu_debug_state + rd->reg);
> >> + if (copy_to_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
> >> + return -EFAULT;
> >> + return 0;
> >> +}
> >> +#endif
> >> +
> >
> > what is this ifdef'ed block of code doing here?
>
> Oops. Yeah looks like I missed removing that after I finished the
> re-factor. These where the old get/set functions I used.
>
> >
> >> int kvm_arm_sys_reg_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
> >> {
> >> const struct sys_reg_desc *r;
> >> @@ -1303,6 +1530,9 @@ int kvm_arm_sys_reg_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg
> >> if (!r)
> >> return get_invariant_sys_reg(reg->id, uaddr);
> >>
> >> + if (r->get)
> >> + return (r->get)(vcpu, r, reg, uaddr);
> >> +
> >> return reg_to_user(uaddr, &vcpu_sys_reg(vcpu, r->reg), reg->id);
> >> }
> >>
> >> @@ -1321,6 +1551,9 @@ int kvm_arm_sys_reg_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg
> >> if (!r)
> >> return set_invariant_sys_reg(reg->id, uaddr);
> >>
> >> + if (r->set)
> >> + return (r->set)(vcpu, r, reg, uaddr);
> >> +
> >> return reg_from_user(&vcpu_sys_reg(vcpu, r->reg), uaddr, reg->id);
> >> }
> >>
> >> diff --git a/arch/arm64/kvm/sys_regs.h b/arch/arm64/kvm/sys_regs.h
> >> index d411e25..9265e7d 100644
> >> --- a/arch/arm64/kvm/sys_regs.h
> >> +++ b/arch/arm64/kvm/sys_regs.h
> >> @@ -55,6 +55,12 @@ struct sys_reg_desc {
> >>
> >> /* Value (usually reset value) */
> >> u64 val;
> >> +
> >> + /* Get/Set functions, fallback if NULL */
> >
> > Is this only meant for usersapce access or when should one use these?
>
> Yes for GET/SET
>
ok, see other mail for potential rename to get_user / set_user if you
like the idea.
Thanks,
-Christoffer
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v7 08/11] KVM: arm64: introduce vcpu->arch.debug_ptr
2015-07-01 18:29 ` [PATCH v7 08/11] KVM: arm64: introduce vcpu->arch.debug_ptr Alex Bennée
2015-07-02 18:34 ` Christoffer Dall
@ 2015-07-03 21:43 ` Christoffer Dall
1 sibling, 0 replies; 22+ messages in thread
From: Christoffer Dall @ 2015-07-03 21:43 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, Lorenzo Pieralisi, Richard Weinberger,
Andre Przywara, open list
On Wed, Jul 01, 2015 at 07:29:00PM +0100, Alex Bennée wrote:
> This introduces a level of indirection for the debug registers. Instead
> of using the sys_regs[] directly we store registers in a structure in
> the vcpu. The new kvm_arm_reset_debug_ptr() sets the debug ptr to the
> guest context.
>
> This also entails updating the sys_regs code to access this new
> structure. New access function have been added for each set of debug
> registers. The generic functions are still used for the few registers
> stored in the main context.
>
> New access function pointers have been added to the sys_reg_desc
> structure to support the GET/SET_ONE_REG ioctl operations.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> ---
> v6:
> - fix up some ws issues
> - correct clobber info
> - re-word commentary in kvm_host.h
> - fix endian access issues for aarch32 fields
> - revert all KVM_GET/SET_ONE_REG to 64bit (also see ABI update)
> v7
> - new fn kvm_arm_reset_debug_ptr(), stubbed for arm
> - split trap fns into bcr,bvr,bcr,wvr and wxvr
> - add set/get fns to sys_regs_desc
> - reg_to_dbg/dbg_to_reg helpers for 32bit support
> ---
> arch/arm/include/asm/kvm_host.h | 2 +-
> arch/arm/kvm/arm.c | 2 +
> arch/arm64/include/asm/kvm_asm.h | 24 ++--
> arch/arm64/include/asm/kvm_host.h | 17 ++-
> arch/arm64/kernel/asm-offsets.c | 6 +
> arch/arm64/kvm/debug.c | 9 ++
> arch/arm64/kvm/hyp.S | 24 ++--
> arch/arm64/kvm/sys_regs.c | 281 ++++++++++++++++++++++++++++++++++----
> arch/arm64/kvm/sys_regs.h | 6 +
> 9 files changed, 321 insertions(+), 50 deletions(-)
>
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index 746c0c69..f42759b 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -239,5 +239,5 @@ 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) {}
> -
> +static inline void kvm_arm_reset_debug_ptr(struct kvm_vcpu *vcpu) {}
> #endif /* __ARM_KVM_HOST_H__ */
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index af60e6f..525473f 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -279,6 +279,8 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
> /* Set up the timer */
> kvm_timer_vcpu_init(vcpu);
>
> + kvm_arm_reset_debug_ptr(vcpu);
> +
> return 0;
> }
>
> diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
> index d6b507e..e997404 100644
> --- a/arch/arm64/include/asm/kvm_asm.h
> +++ b/arch/arm64/include/asm/kvm_asm.h
> @@ -46,24 +46,16 @@
> #define CNTKCTL_EL1 20 /* Timer Control Register (EL1) */
> #define PAR_EL1 21 /* Physical Address Register */
> #define MDSCR_EL1 22 /* Monitor Debug System Control Register */
> -#define DBGBCR0_EL1 23 /* Debug Breakpoint Control Registers (0-15) */
> -#define DBGBCR15_EL1 38
> -#define DBGBVR0_EL1 39 /* Debug Breakpoint Value Registers (0-15) */
> -#define DBGBVR15_EL1 54
> -#define DBGWCR0_EL1 55 /* Debug Watchpoint Control Registers (0-15) */
> -#define DBGWCR15_EL1 70
> -#define DBGWVR0_EL1 71 /* Debug Watchpoint Value Registers (0-15) */
> -#define DBGWVR15_EL1 86
> -#define MDCCINT_EL1 87 /* Monitor Debug Comms Channel Interrupt Enable Reg */
> +#define MDCCINT_EL1 23 /* Monitor Debug Comms Channel Interrupt Enable Reg */
>
> /* 32bit specific registers. Keep them at the end of the range */
> -#define DACR32_EL2 88 /* Domain Access Control Register */
> -#define IFSR32_EL2 89 /* Instruction Fault Status Register */
> -#define FPEXC32_EL2 90 /* Floating-Point Exception Control Register */
> -#define DBGVCR32_EL2 91 /* Debug Vector Catch Register */
> -#define TEECR32_EL1 92 /* ThumbEE Configuration Register */
> -#define TEEHBR32_EL1 93 /* ThumbEE Handler Base Register */
> -#define NR_SYS_REGS 94
> +#define DACR32_EL2 24 /* Domain Access Control Register */
> +#define IFSR32_EL2 25 /* Instruction Fault Status Register */
> +#define FPEXC32_EL2 26 /* Floating-Point Exception Control Register */
> +#define DBGVCR32_EL2 27 /* Debug Vector Catch Register */
> +#define TEECR32_EL1 28 /* ThumbEE Configuration Register */
> +#define TEEHBR32_EL1 29 /* ThumbEE Handler Base Register */
> +#define NR_SYS_REGS 30
>
> /* 32bit mapping */
> #define c0_MPIDR (MPIDR_EL1 * 2) /* MultiProcessor ID Register */
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index e2db6a6..461d288 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -108,11 +108,25 @@ struct kvm_vcpu_arch {
> /* Exception Information */
> struct kvm_vcpu_fault_info fault;
>
> - /* Debug state */
> + /* Guest debug state */
> u64 debug_flags;
>
> + /*
> + * We maintain more than a single set of debug registers to support
> + * debugging the guest from the host and to maintain separate host and
> + * guest state during world switches. vcpu_debug_state are the debug
> + * registers of the vcpu as the guest sees them. host_debug_state are
> + * the host registers which are saved and restored during world switches.
> + *
> + * debug_ptr points to the set of debug registers that should be loaded
> + * onto the hardware when running the guest.
> + */
> + struct kvm_guest_debug_arch *debug_ptr;
> + struct kvm_guest_debug_arch vcpu_debug_state;
> +
> /* Pointer to host CPU context */
> kvm_cpu_context_t *host_cpu_context;
> + struct kvm_guest_debug_arch host_debug_state;
>
> /* VGIC state */
> struct vgic_cpu vgic_cpu;
> @@ -265,5 +279,6 @@ 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);
> +void kvm_arm_reset_debug_ptr(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 dfb25a2..1a8e97c 100644
> --- a/arch/arm64/kernel/asm-offsets.c
> +++ b/arch/arm64/kernel/asm-offsets.c
> @@ -116,10 +116,16 @@ 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));
> + 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));
> DEFINE(VCPU_HOST_CONTEXT, offsetof(struct kvm_vcpu, arch.host_cpu_context));
> + DEFINE(VCPU_HOST_DEBUG_STATE, offsetof(struct kvm_vcpu, arch.host_debug_state));
> DEFINE(VCPU_TIMER_CNTV_CTL, offsetof(struct kvm_vcpu, arch.timer_cpu.cntv_ctl));
> DEFINE(VCPU_TIMER_CNTV_CVAL, offsetof(struct kvm_vcpu, arch.timer_cpu.cntv_cval));
> DEFINE(KVM_TIMER_CNTVOFF, offsetof(struct kvm, arch.timer.cntvoff));
> diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
> index d439eb8..ffefb97 100644
> --- a/arch/arm64/kvm/debug.c
> +++ b/arch/arm64/kvm/debug.c
> @@ -67,6 +67,15 @@ void kvm_arm_init_debug(void)
> }
>
> /**
> + * kvm_arm_reset_debug_ptr - set the debug registers to be the guests
> + */
> +
> +void kvm_arm_reset_debug_ptr(struct kvm_vcpu *vcpu)
> +{
> + vcpu->arch.debug_ptr = &vcpu->arch.vcpu_debug_state;
> +}
> +
> +/**
> * kvm_arm_setup_debug - set up debug related stuff
> *
> * @vcpu: the vcpu pointer
> diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
> index 77c08df..f56e93f 100644
> --- a/arch/arm64/kvm/hyp.S
> +++ b/arch/arm64/kvm/hyp.S
> @@ -600,6 +600,7 @@ __restore_sysregs:
> /* Save debug state */
> __save_debug:
> // x2: ptr to CPU context
> + // x3: ptr to debug reg struct
> // x4/x5/x6-22/x24-26: trashed
>
> mrs x26, id_aa64dfr0_el1
> @@ -610,15 +611,15 @@ __save_debug:
> sub w25, w26, w25 // How many WPs to skip
>
> mov x5, x24
> - add x4, x2, #CPU_SYSREG_OFFSET(DBGBCR0_EL1)
> + add x4, x3, #DEBUG_BCR
> save_debug dbgbcr
> - add x4, x2, #CPU_SYSREG_OFFSET(DBGBVR0_EL1)
> + add x4, x3, #DEBUG_BVR
> save_debug dbgbvr
>
> mov x5, x25
> - add x4, x2, #CPU_SYSREG_OFFSET(DBGWCR0_EL1)
> + add x4, x3, #DEBUG_WCR
> save_debug dbgwcr
> - add x4, x2, #CPU_SYSREG_OFFSET(DBGWVR0_EL1)
> + add x4, x3, #DEBUG_WVR
> save_debug dbgwvr
>
> mrs x21, mdccint_el1
> @@ -628,6 +629,7 @@ __save_debug:
> /* Restore debug state */
> __restore_debug:
> // x2: ptr to CPU context
> + // x3: ptr to debug reg struct
> // x4/x5/x6-22/x24-26: trashed
>
> mrs x26, id_aa64dfr0_el1
> @@ -638,15 +640,15 @@ __restore_debug:
> sub w25, w26, w25 // How many WPs to skip
>
> mov x5, x24
> - add x4, x2, #CPU_SYSREG_OFFSET(DBGBCR0_EL1)
> + add x4, x3, #DEBUG_BCR
> restore_debug dbgbcr
> - add x4, x2, #CPU_SYSREG_OFFSET(DBGBVR0_EL1)
> + add x4, x3, #DEBUG_BVR
> restore_debug dbgbvr
>
> mov x5, x25
> - add x4, x2, #CPU_SYSREG_OFFSET(DBGWCR0_EL1)
> + add x4, x3, #DEBUG_WCR
> restore_debug dbgwcr
> - add x4, x2, #CPU_SYSREG_OFFSET(DBGWVR0_EL1)
> + add x4, x3, #DEBUG_WVR
> restore_debug dbgwvr
>
> ldr x21, [x2, #CPU_SYSREG_OFFSET(MDCCINT_EL1)]
> @@ -686,6 +688,7 @@ ENTRY(__kvm_vcpu_run)
> bl __save_sysregs
>
> compute_debug_state 1f
> + add x3, x0, #VCPU_HOST_DEBUG_STATE
> bl __save_debug
> 1:
> activate_traps
> @@ -701,6 +704,8 @@ ENTRY(__kvm_vcpu_run)
> bl __restore_fpsimd
>
> skip_debug_state x3, 1f
> + ldr x3, [x0, #VCPU_DEBUG_PTR]
> + kern_hyp_va x3
> bl __restore_debug
> 1:
> restore_guest_32bit_state
> @@ -721,6 +726,8 @@ __kvm_vcpu_return:
> bl __save_sysregs
>
> skip_debug_state x3, 1f
> + ldr x3, [x0, #VCPU_DEBUG_PTR]
> + kern_hyp_va x3
> bl __save_debug
> 1:
> save_guest_32bit_state
> @@ -743,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, x0, #VCPU_HOST_DEBUG_STATE
> bl __restore_debug
> 1:
> restore_host_regs
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index c370b40..3267b96 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -173,14 +173,14 @@ static bool trap_dbgauthstatus_el1(struct kvm_vcpu *vcpu,
> /*
> * We want to avoid world-switching all the DBG registers all the
> * time:
> - *
> + *
> * - If we've touched any debug register, it is likely that we're
> * going to touch more of them. It then makes sense to disable the
> * traps and start doing the save/restore dance
> * - If debug is active (DBG_MDSCR_KDE or DBG_MDSCR_MDE set), it is
> * then mandatory to save/restore the registers, as the guest
> * depends on them.
> - *
> + *
> * For this, we use a DIRTY bit, indicating the guest has modified the
> * debug registers, used as follow:
> *
> @@ -211,6 +211,187 @@ static bool trap_debug_regs(struct kvm_vcpu *vcpu,
> return true;
> }
>
> +/*
> + * reg_to_dbg/dbg_to_reg - simple masking for 32bit access
important side effect of setting debug dirty flag, note that here.
> + */
> +static inline unsigned long reg_to_dbg(struct kvm_vcpu *vcpu,
> + const struct sys_reg_params *p)
> +{
> + unsigned long val = *vcpu_reg(vcpu, p->Rt);
> + vcpu->arch.debug_flags |= KVM_ARM64_DEBUG_DIRTY;
> + if (p->is_32bit)
> + val &= 0xffffffffUL;
> +
> + return val;
> +}
> +
> +static inline void dbg_to_reg(struct kvm_vcpu *vcpu,
> + const struct sys_reg_params *p, unsigned long val)
> +{
> + if (p->is_32bit)
> + val &= 0xffffffffUL;
doesn't this potentially zero out the most-significant bits of a 64-bit
value if the access is done in 32-bit mode?
Is this really what you want?
> + *vcpu_reg(vcpu, p->Rt) = val;
> +}
> +
> +static inline bool trap_bvr(struct kvm_vcpu *vcpu,
> + const struct sys_reg_params *p,
> + const struct sys_reg_desc *rd)
> +{
> + if (p->is_write) {
> + vcpu->arch.vcpu_debug_state.dbg_bvr[rd->reg] =
> + reg_to_dbg(vcpu, p);
> + } else {
> + dbg_to_reg(vcpu, p,
> + vcpu->arch.vcpu_debug_state.dbg_bvr[rd->reg]);
> + }
> +
> + return true;
> +}
> +
> +static int set_bvr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> + const struct kvm_one_reg *reg, void __user *uaddr)
> +{
> + __u64 *r = &vcpu->arch.vcpu_debug_state.dbg_bvr[rd->reg];
> + if (copy_from_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
> + return -EFAULT;
> + return 0;
> +}
> +
> +static int get_bvr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> + const struct kvm_one_reg *reg, void __user *uaddr)
> +{
> + __u64 *r = &vcpu->arch.vcpu_debug_state.dbg_bvr[rd->reg];
> + if (copy_to_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
> + return -EFAULT;
> + return 0;
> +}
> +
> +static inline void reset_bvr(struct kvm_vcpu *vcpu,
> + const struct sys_reg_desc *rd)
> +{
> + vcpu->arch.vcpu_debug_state.dbg_bvr[rd->reg] = rd->val;
> +}
> +
> +static inline bool trap_bcr(struct kvm_vcpu *vcpu,
> + const struct sys_reg_params *p,
> + const struct sys_reg_desc *rd)
> +{
> + if (p->is_write) {
> + vcpu->arch.vcpu_debug_state.dbg_bcr[rd->reg] =
> + reg_to_dbg(vcpu, p);
> + } else {
> + dbg_to_reg(vcpu, p,
> + vcpu->arch.vcpu_debug_state.dbg_bcr[rd->reg]);
> + }
> +
> + return true;
> +}
> +
> +static int set_bcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> + const struct kvm_one_reg *reg, void __user *uaddr)
> +{
> + __u64 *r = &vcpu->arch.vcpu_debug_state.dbg_bcr[rd->reg];
> + if (copy_from_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
> + return -EFAULT;
> + return 0;
> +}
> +
> +static int get_bcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> + const struct kvm_one_reg *reg, void __user *uaddr)
> +{
> + __u64 *r = &vcpu->arch.vcpu_debug_state.dbg_bcr[rd->reg];
> + if (copy_to_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
> + return -EFAULT;
> + return 0;
> +}
> +
> +static inline void reset_bcr(struct kvm_vcpu *vcpu,
> + const struct sys_reg_desc *rd)
> +{
> + vcpu->arch.vcpu_debug_state.dbg_bcr[rd->reg] = rd->val;
> +}
> +
> +static inline bool trap_wvr(struct kvm_vcpu *vcpu,
> + const struct sys_reg_params *p,
> + const struct sys_reg_desc *rd)
> +{
> + if (p->is_write) {
> + vcpu->arch.vcpu_debug_state.dbg_wvr[rd->reg] =
> + reg_to_dbg(vcpu, p);
> + } else {
> + dbg_to_reg(vcpu, p,
> + vcpu->arch.vcpu_debug_state.dbg_wvr[rd->reg]);
> + }
> +
> + return true;
> +}
> +
> +static int set_wvr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> + const struct kvm_one_reg *reg, void __user *uaddr)
> +{
> + __u64 *r = &vcpu->arch.vcpu_debug_state.dbg_wvr[rd->reg];
> + if (copy_from_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
> + return -EFAULT;
> + return 0;
> +}
> +
> +static int get_wvr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> + const struct kvm_one_reg *reg, void __user *uaddr)
> +{
> + __u64 *r = &vcpu->arch.vcpu_debug_state.dbg_wvr[rd->reg];
> + if (copy_to_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
> + return -EFAULT;
> + return 0;
> +}
> +
> +static inline void reset_wvr(struct kvm_vcpu *vcpu,
> + const struct sys_reg_desc *rd)
> +{
> + vcpu->arch.vcpu_debug_state.dbg_wvr[rd->reg] = rd->val;
> +}
> +
> +static inline bool trap_wcr(struct kvm_vcpu *vcpu,
> + const struct sys_reg_params *p,
> + const struct sys_reg_desc *rd)
> +{
> + if (p->is_write) {
> + vcpu->arch.vcpu_debug_state.dbg_wcr[rd->reg] =
> + reg_to_dbg(vcpu, p);
> + } else {
> + dbg_to_reg(vcpu, p,
> + vcpu->arch.vcpu_debug_state.dbg_wcr[rd->reg]);
> + }
> +
> + trace_trap_reg(__func__, rd->reg, p->is_write,
> + vcpu->arch.vcpu_debug_state.dbg_wcr[rd->reg]);
> +
> + return true;
> +}
> +
> +static int set_wcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> + const struct kvm_one_reg *reg, void __user *uaddr)
> +{
> + __u64 *r = &vcpu->arch.vcpu_debug_state.dbg_wcr[rd->reg];
> + if (copy_from_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
> + return -EFAULT;
> + return 0;
> +}
> +
> +static int get_wcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> + const struct kvm_one_reg *reg, void __user *uaddr)
> +{
> + __u64 *r = &vcpu->arch.vcpu_debug_state.dbg_wcr[rd->reg];
> + if (copy_to_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
> + return -EFAULT;
> + return 0;
> +}
> +
> +static inline void reset_wcr(struct kvm_vcpu *vcpu,
> + const struct sys_reg_desc *rd)
> +{
> + vcpu->arch.vcpu_debug_state.dbg_wcr[rd->reg] = rd->val;
> +}
> +
> static void reset_amair_el1(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
> {
> u64 amair;
> @@ -240,16 +421,16 @@ static void reset_mpidr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
> #define DBG_BCR_BVR_WCR_WVR_EL1(n) \
> /* DBGBVRn_EL1 */ \
> { Op0(0b10), Op1(0b000), CRn(0b0000), CRm((n)), Op2(0b100), \
> - trap_debug_regs, reset_val, (DBGBVR0_EL1 + (n)), 0 }, \
> + trap_bvr, reset_bvr, n, 0, get_bvr, set_bvr }, \
> /* DBGBCRn_EL1 */ \
> { Op0(0b10), Op1(0b000), CRn(0b0000), CRm((n)), Op2(0b101), \
> - trap_debug_regs, reset_val, (DBGBCR0_EL1 + (n)), 0 }, \
> + trap_bcr, reset_bcr, n, 0, get_bcr, set_bcr }, \
> /* DBGWVRn_EL1 */ \
> { Op0(0b10), Op1(0b000), CRn(0b0000), CRm((n)), Op2(0b110), \
> - trap_debug_regs, reset_val, (DBGWVR0_EL1 + (n)), 0 }, \
> + trap_wvr, reset_wvr, n, 0, get_wvr, set_wvr }, \
> /* DBGWCRn_EL1 */ \
> { Op0(0b10), Op1(0b000), CRn(0b0000), CRm((n)), Op2(0b111), \
> - trap_debug_regs, reset_val, (DBGWCR0_EL1 + (n)), 0 }
> + trap_wcr, reset_wcr, n, 0, get_wcr, set_wcr }
>
> /*
> * Architected system registers.
> @@ -516,28 +697,51 @@ static bool trap_debug32(struct kvm_vcpu *vcpu,
> return true;
> }
>
> -#define DBG_BCR_BVR_WCR_WVR(n) \
> - /* DBGBVRn */ \
> - { Op1( 0), CRn( 0), CRm((n)), Op2( 4), trap_debug32, \
> - NULL, (cp14_DBGBVR0 + (n) * 2) }, \
> - /* DBGBCRn */ \
> - { Op1( 0), CRn( 0), CRm((n)), Op2( 5), trap_debug32, \
> - NULL, (cp14_DBGBCR0 + (n) * 2) }, \
> - /* DBGWVRn */ \
> - { Op1( 0), CRn( 0), CRm((n)), Op2( 6), trap_debug32, \
> - NULL, (cp14_DBGWVR0 + (n) * 2) }, \
> - /* DBGWCRn */ \
> - { Op1( 0), CRn( 0), CRm((n)), Op2( 7), trap_debug32, \
> - NULL, (cp14_DBGWCR0 + (n) * 2) }
> -
> -#define DBGBXVR(n) \
> - { Op1( 0), CRn( 1), CRm((n)), Op2( 1), trap_debug32, \
> - NULL, cp14_DBGBXVR0 + n * 2 }
> +/* AArch32 debug register mappings
> + *
> + * AArch32 DBGBVRn is mapped to DBGBVRn_EL1[31:0]
> + * AArch32 DBGBXVRn is mapped to DBGBVRn_EL1[63:32]
> + *
> + * All control registers and watchpoint value registers are mapped to
> + * the lower 32 bits of their AArch64 equivalents. We share the trap
> + * handlers with the above AArch64 code which checks what mode the
> + * system is in.
> + */
> +
> +static inline bool trap_xvr(struct kvm_vcpu *vcpu,
> + const struct sys_reg_params *p,
> + const struct sys_reg_desc *rd)
> +{
> + if (p->is_write) {
> + u64 val = vcpu->arch.vcpu_debug_state.dbg_bvr[rd->reg];
> + val &= 0xffffffffUL;
> + val |= reg_to_dbg(vcpu, p) << 32;
> + vcpu->arch.vcpu_debug_state.dbg_bvr[rd->reg] = val;
> + } else {
> + dbg_to_reg(vcpu, p,
> + vcpu->arch.vcpu_debug_state.dbg_bvr[rd->reg] >> 32);
> + }
> +
> + return true;
> +}
> +
> +#define DBG_BCR_BVR_WCR_WVR(n) \
> + /* DBGBVRn */ \
> + { Op1( 0), CRn( 0), CRm((n)), Op2( 4), trap_bvr, NULL, n }, \
> + /* DBGBCRn */ \
> + { Op1( 0), CRn( 0), CRm((n)), Op2( 5), trap_bcr, NULL, n }, \
> + /* DBGWVRn */ \
> + { Op1( 0), CRn( 0), CRm((n)), Op2( 6), trap_wvr, NULL, n }, \
> + /* DBGWCRn */ \
> + { Op1( 0), CRn( 0), CRm((n)), Op2( 7), trap_wcr, NULL, n }
> +
> +#define DBGBXVR(n) \
> + { Op1( 0), CRn( 1), CRm((n)), Op2( 1), trap_xvr, NULL, n }
>
> /*
> * Trapped cp14 registers. We generally ignore most of the external
> * debug, on the principle that they don't really make sense to a
> - * guest. Revisit this one day, whould this principle change.
> + * guest. Revisit this one day, would this principle change.
> */
> static const struct sys_reg_desc cp14_regs[] = {
> /* DBGIDR */
> @@ -1288,6 +1492,29 @@ static int demux_c15_set(u64 id, void __user *uaddr)
> }
> }
>
> +/*
> + * Access functions for vcpu_debug_state.
> + */
> +#if 0
> +static int debug_set64(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> + const struct kvm_one_reg *reg, void __user *uaddr)
> +{
> + __u64 *r = (__u64 *) ((void * )&vcpu->arch.vcpu_debug_state + rd->reg);
> + if (copy_from_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
> + return -EFAULT;
> + return 0;
> +}
> +
> +static int debug_get64(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> + const struct kvm_one_reg *reg, void __user *uaddr)
> +{
> + __u64 *r = (__u64 *) ((void * )&vcpu->arch.vcpu_debug_state + rd->reg);
> + if (copy_to_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
> + return -EFAULT;
> + return 0;
> +}
> +#endif
> +
> int kvm_arm_sys_reg_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
> {
> const struct sys_reg_desc *r;
> @@ -1303,6 +1530,9 @@ int kvm_arm_sys_reg_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg
> if (!r)
> return get_invariant_sys_reg(reg->id, uaddr);
>
> + if (r->get)
> + return (r->get)(vcpu, r, reg, uaddr);
> +
> return reg_to_user(uaddr, &vcpu_sys_reg(vcpu, r->reg), reg->id);
> }
>
> @@ -1321,6 +1551,9 @@ int kvm_arm_sys_reg_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg
> if (!r)
> return set_invariant_sys_reg(reg->id, uaddr);
>
> + if (r->set)
> + return (r->set)(vcpu, r, reg, uaddr);
> +
> return reg_from_user(&vcpu_sys_reg(vcpu, r->reg), uaddr, reg->id);
> }
>
> diff --git a/arch/arm64/kvm/sys_regs.h b/arch/arm64/kvm/sys_regs.h
> index d411e25..9265e7d 100644
> --- a/arch/arm64/kvm/sys_regs.h
> +++ b/arch/arm64/kvm/sys_regs.h
> @@ -55,6 +55,12 @@ struct sys_reg_desc {
>
> /* Value (usually reset value) */
> u64 val;
> +
> + /* Get/Set functions, fallback if NULL */
> + int (*get)(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> + const struct kvm_one_reg *reg, void __user *uaddr);
> + int (*set)(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> + const struct kvm_one_reg *reg, void __user *uaddr);
can you call these get_user and set_user instead?
> };
>
> static inline void print_sys_reg_instr(const struct sys_reg_params *p)
> --
> 2.4.5
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v7 09/11] KVM: arm64: guest debug, HW assisted debug support
[not found] <1435775343-20034-1-git-send-email-alex.bennee@linaro.org>
` (7 preceding siblings ...)
2015-07-01 18:29 ` [PATCH v7 08/11] KVM: arm64: introduce vcpu->arch.debug_ptr Alex Bennée
@ 2015-07-01 18:29 ` Alex Bennée
2015-07-02 8:48 ` Will Deacon
2015-07-01 18:29 ` [PATCH v7 10/11] KVM: arm64: enable KVM_CAP_SET_GUEST_DEBUG Alex Bennée
2015-07-01 18:29 ` [PATCH v7 11/11] KVM: arm64: add trace points for guest_debug debug Alex Bennée
10 siblings, 1 reply; 22+ messages in thread
From: Alex Bennée @ 2015-07-01 18:29 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, Catalin Marinas, Will Deacon, Lorenzo Pieralisi,
Ingo Molnar, Peter Zijlstra, 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 an IMPDEF registers into a new
register set called host_debug_state.
We use the recently introduced vcpu parameter debug_ptr to select which
register set is copied into the real registers when world switch occurs.
I've made some helper functions from hw_breakpoint.c more widely
available 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>
Reviewed-by: Christoffer Dall <christoffer.dall@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
v4
- use the new u32/u64 split debug_ptr approach
- fix some wording/comments
v5
- don't set MDSCR_EL1.KDE (not needed)
v6
- update wording given change in commentary
- KVM_GUESTDBG_USE_HW_BP->KVM_GUESTDBG_USE_HW
v7
- fix merge conflicts from ioctl move to guest.c
- use kvm_arm_reset_debug_ptr to reset ptr
- a BUG_ON() test has been added to trap failure to reset debug_ptr
- debugging->debug in kvm_host.h comment
- s/defined// s/to// in commit msg
- rm ref to introducing debug_ptr in commit msg
- add r-b tag
---
Documentation/virtual/kvm/api.txt | 7 +++++-
arch/arm64/include/asm/hw_breakpoint.h | 4 ++++
arch/arm64/include/asm/kvm_host.h | 6 ++++-
arch/arm64/kernel/hw_breakpoint.c | 4 ++--
arch/arm64/kvm/debug.c | 40 +++++++++++++++++++++++++++++-----
arch/arm64/kvm/guest.c | 7 ++++++
arch/arm64/kvm/handle_exit.c | 6 +++++
arch/arm64/kvm/reset.c | 12 ++++++++++
arch/arm64/kvm/sys_regs.c | 3 ---
include/uapi/linux/kvm.h | 2 ++
10 files changed, 79 insertions(+), 12 deletions(-)
diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 33c8143..ada57df 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 return a positive 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/arm64/include/asm/hw_breakpoint.h b/arch/arm64/include/asm/hw_breakpoint.h
index 52b484b..9da2824 100644
--- a/arch/arm64/include/asm/hw_breakpoint.h
+++ b/arch/arm64/include/asm/hw_breakpoint.h
@@ -130,6 +130,10 @@ static inline void ptrace_hw_copy_thread(struct task_struct *task)
}
#endif
+/* Determine number of BRP/WRP registers available. */
+extern int get_num_brps(void);
+extern int get_num_wrps(void);
+
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 461d288..6c745e0 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -116,13 +116,17 @@ struct kvm_vcpu_arch {
* debugging the guest from the host and to maintain separate host and
* guest state during world switches. vcpu_debug_state are the debug
* registers of the vcpu as the guest sees them. host_debug_state are
- * the host registers which are saved and restored during world switches.
+ * the host registers which are saved and restored during
+ * world switches. external_debug_state contains the debug
+ * values we want to debug the guest. This is set via the
+ * KVM_SET_GUEST_DEBUG ioctl.
*
* debug_ptr points to the set of debug registers that should be loaded
* onto the hardware when running the guest.
*/
struct kvm_guest_debug_arch *debug_ptr;
struct kvm_guest_debug_arch vcpu_debug_state;
+ 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/kernel/hw_breakpoint.c b/arch/arm64/kernel/hw_breakpoint.c
index e7d934d..ac07f2a 100644
--- a/arch/arm64/kernel/hw_breakpoint.c
+++ b/arch/arm64/kernel/hw_breakpoint.c
@@ -50,13 +50,13 @@ static int core_num_brps;
static int core_num_wrps;
/* Determine number of BRP registers available. */
-static int get_num_brps(void)
+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)
+int get_num_wrps(void)
{
return ((read_cpuid(ID_AA64DFR0_EL1) >> 20) & 0xf) + 1;
}
diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
index ffefb97..46b73d7 100644
--- a/arch/arm64/kvm/debug.c
+++ b/arch/arm64/kvm/debug.c
@@ -105,10 +105,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;
-
/* Is Guest debugging in effect? */
if (vcpu->guest_debug) {
/* Route all software debug exceptions to EL2 */
@@ -143,11 +139,45 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
} else {
vcpu_sys_reg(vcpu, MDSCR_EL1) &= ~DBG_MDSCR_SS;
}
+
+ /*
+ * HW Breakpoints and watchpoints
+ *
+ * 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) {
+ /* Enable breakpoints/watchpoints */
+ vcpu_sys_reg(vcpu, MDSCR_EL1) |= DBG_MDSCR_MDE;
+
+ vcpu->arch.debug_ptr = &vcpu->arch.external_debug_state;
+ vcpu->arch.debug_flags |= KVM_ARM64_DEBUG_DIRTY;
+ trap_debug = true;
+ }
}
+
+ BUG_ON(!vcpu->guest_debug &&
+ vcpu->arch.debug_ptr != &vcpu->arch.vcpu_debug_state);
+
+ /* Trap debug register access */
+ if (trap_debug)
+ 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)
+ kvm_arm_reset_debug_ptr(vcpu);
+
+ }
}
diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index 48de4f4..6f1b249 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -334,6 +334,7 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *vcpu,
#define KVM_GUESTDBG_VALID_MASK (KVM_GUESTDBG_ENABLE | \
KVM_GUESTDBG_USE_SW_BP | \
+ KVM_GUESTDBG_USE_HW | \
KVM_GUESTDBG_SINGLESTEP)
/**
@@ -354,6 +355,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) {
+ vcpu->arch.external_debug_state = dbg->arch;
+ }
+
} else {
/* If not enabled clear all flags */
vcpu->guest_debug = 0;
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/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/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 3267b96..f753a67 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -362,9 +362,6 @@ static inline bool trap_wcr(struct kvm_vcpu *vcpu,
vcpu->arch.vcpu_debug_state.dbg_wcr[rd->reg]);
}
- trace_trap_reg(__func__, rd->reg, p->is_write,
- vcpu->arch.vcpu_debug_state.dbg_wcr[rd->reg]);
-
return true;
}
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 70ac641..f020dd0 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -817,6 +817,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.4.5
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v7 09/11] KVM: arm64: guest debug, HW assisted debug support
2015-07-01 18:29 ` [PATCH v7 09/11] KVM: arm64: guest debug, HW assisted debug support Alex Bennée
@ 2015-07-02 8:48 ` Will Deacon
2015-07-02 13:50 ` Alex Bennée
0 siblings, 1 reply; 22+ messages in thread
From: Will Deacon @ 2015-07-02 8:48 UTC (permalink / raw)
To: Alex Bennée
Cc: kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
kvmarm@lists.cs.columbia.edu, christoffer.dall@linaro.org,
Marc Zyngier, peter.maydell@linaro.org, agraf@suse.de,
drjones@redhat.com, pbonzini@redhat.com, zhichao.huang@linaro.org,
jan.kiszka@siemens.com, dahi@linux.vnet.ibm.com,
r65777@freescale.com, bp@suse.de, Gleb Natapov, Jonathan Corbet,
Catalin Marinas, Lorenzo Pieralisi, Ingo Molnar, Peter Zijlstra,
open list:DOCUMENTATION, open list, open list:ABI/API
Hi Alex,
On Wed, Jul 01, 2015 at 07:29:01PM +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 an IMPDEF registers into a new
> register set called host_debug_state.
>
> We use the recently introduced vcpu parameter debug_ptr to select which
> register set is copied into the real registers when world switch occurs.
>
> I've made some helper functions from hw_breakpoint.c more widely
> available 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>
> Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
[...]
> diff --git a/arch/arm64/kernel/hw_breakpoint.c b/arch/arm64/kernel/hw_breakpoint.c
> index e7d934d..ac07f2a 100644
> --- a/arch/arm64/kernel/hw_breakpoint.c
> +++ b/arch/arm64/kernel/hw_breakpoint.c
> @@ -50,13 +50,13 @@ static int core_num_brps;
> static int core_num_wrps;
>
> /* Determine number of BRP registers available. */
> -static int get_num_brps(void)
> +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)
> +int get_num_wrps(void)
> {
> return ((read_cpuid(ID_AA64DFR0_EL1) >> 20) & 0xf) + 1;
> }
Sorry, just noticed this, but we already have a public interface for
figuring these numbers out as required by perf. Can't you use
hw_breakpoint_slots(...) instead of exposing these internal helpers?
Will
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v7 09/11] KVM: arm64: guest debug, HW assisted debug support
2015-07-02 8:48 ` Will Deacon
@ 2015-07-02 13:50 ` Alex Bennée
2015-07-03 9:23 ` Will Deacon
0 siblings, 1 reply; 22+ messages in thread
From: Alex Bennée @ 2015-07-02 13:50 UTC (permalink / raw)
To: Will Deacon
Cc: kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
kvmarm@lists.cs.columbia.edu, christoffer.dall@linaro.org,
Marc Zyngier, peter.maydell@linaro.org, agraf@suse.de,
drjones@redhat.com, pbonzini@redhat.com, zhichao.huang@linaro.org,
jan.kiszka@siemens.com, dahi@linux.vnet.ibm.com,
r65777@freescale.com, bp@suse.de, Gleb Natapov, Jonathan Corbet,
Catalin Marinas, Lorenzo Pieralisi, Ingo Molnar, Peter Zijlstra,
open list:DOCUMENTATION, open list, open list:ABI/API
Will Deacon <will.deacon@arm.com> writes:
Are you happy with this?:
Subject: [PATCH v8 09/11] KVM: arm64: guest debug, HW assisted debug support
This adds support for userspace to control the HW debug registers for
guest debug. In the debug ioctl we copy an IMPDEF registers into a new
register set called host_debug_state.
We use the recently introduced vcpu parameter debug_ptr to select which
register set is copied into the real registers when world switch occurs.
I've made some helper functions from hw_breakpoint.c more widely
available 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>
Reviewed-by: Christoffer Dall <christoffer.dall@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
v4
- use the new u32/u64 split debug_ptr approach
- fix some wording/comments
v5
- don't set MDSCR_EL1.KDE (not needed)
v6
- update wording given change in commentary
- KVM_GUESTDBG_USE_HW_BP->KVM_GUESTDBG_USE_HW
v7
- fix merge conflicts from ioctl move to guest.c
- use kvm_arm_reset_debug_ptr to reset ptr
- a BUG_ON() test has been added to trap failure to reset debug_ptr
- debugging->debug in kvm_host.h comment
- s/defined// s/to// in commit msg
- rm ref to introducing debug_ptr in commit msg
- add r-b tag
v8
- use hw_breakpoint_slots() instead
---
Documentation/virtual/kvm/api.txt | 7 ++++++-
arch/arm64/include/asm/kvm_host.h | 6 +++++-
arch/arm64/kvm/debug.c | 40 ++++++++++++++++++++++++++++++++++-----
arch/arm64/kvm/guest.c | 7 +++++++
arch/arm64/kvm/handle_exit.c | 6 ++++++
arch/arm64/kvm/reset.c | 13 +++++++++++++
arch/arm64/kvm/sys_regs.c | 3 ---
include/uapi/linux/kvm.h | 2 ++
8 files changed, 74 insertions(+), 10 deletions(-)
diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 33c8143..ada57df 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 return a positive 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/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 461d288..6c745e0 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -116,13 +116,17 @@ struct kvm_vcpu_arch {
* debugging the guest from the host and to maintain separate host and
* guest state during world switches. vcpu_debug_state are the debug
* registers of the vcpu as the guest sees them. host_debug_state are
- * the host registers which are saved and restored during world switches.
+ * the host registers which are saved and restored during
+ * world switches. external_debug_state contains the debug
+ * values we want to debug the guest. This is set via the
+ * KVM_SET_GUEST_DEBUG ioctl.
*
* debug_ptr points to the set of debug registers that should be loaded
* onto the hardware when running the guest.
*/
struct kvm_guest_debug_arch *debug_ptr;
struct kvm_guest_debug_arch vcpu_debug_state;
+ 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/kvm/debug.c b/arch/arm64/kvm/debug.c
index ffefb97..46b73d7 100644
--- a/arch/arm64/kvm/debug.c
+++ b/arch/arm64/kvm/debug.c
@@ -105,10 +105,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;
-
/* Is Guest debugging in effect? */
if (vcpu->guest_debug) {
/* Route all software debug exceptions to EL2 */
@@ -143,11 +139,45 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
} else {
vcpu_sys_reg(vcpu, MDSCR_EL1) &= ~DBG_MDSCR_SS;
}
+
+ /*
+ * HW Breakpoints and watchpoints
+ *
+ * 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) {
+ /* Enable breakpoints/watchpoints */
+ vcpu_sys_reg(vcpu, MDSCR_EL1) |= DBG_MDSCR_MDE;
+
+ vcpu->arch.debug_ptr = &vcpu->arch.external_debug_state;
+ vcpu->arch.debug_flags |= KVM_ARM64_DEBUG_DIRTY;
+ trap_debug = true;
+ }
}
+
+ BUG_ON(!vcpu->guest_debug &&
+ vcpu->arch.debug_ptr != &vcpu->arch.vcpu_debug_state);
+
+ /* Trap debug register access */
+ if (trap_debug)
+ 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)
+ kvm_arm_reset_debug_ptr(vcpu);
+
+ }
}
diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index 48de4f4..6f1b249 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -334,6 +334,7 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *vcpu,
#define KVM_GUESTDBG_VALID_MASK (KVM_GUESTDBG_ENABLE | \
KVM_GUESTDBG_USE_SW_BP | \
+ KVM_GUESTDBG_USE_HW | \
KVM_GUESTDBG_SINGLESTEP)
/**
@@ -354,6 +355,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) {
+ vcpu->arch.external_debug_state = dbg->arch;
+ }
+
} else {
/* If not enabled clear all flags */
vcpu->guest_debug = 0;
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/reset.c b/arch/arm64/kvm/reset.c
index 0b43265..5a954c4 100644
--- a/arch/arm64/kvm/reset.c
+++ b/arch/arm64/kvm/reset.c
@@ -22,6 +22,7 @@
#include <linux/errno.h>
#include <linux/kvm_host.h>
#include <linux/kvm.h>
+#include <linux/hw_breakpoint.h>
#include <kvm/arm_arch_timer.h>
@@ -56,6 +57,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 +71,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 = hw_breakpoint_slots(TYPE_INST);
+ break;
+ case KVM_CAP_GUEST_DEBUG_HW_WPS:
+ r = hw_breakpoint_slots(TYPE_DATA);
+ break;
default:
r = 0;
}
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 3267b96..f753a67 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -362,9 +362,6 @@ static inline bool trap_wcr(struct kvm_vcpu *vcpu,
vcpu->arch.vcpu_debug_state.dbg_wcr[rd->reg]);
}
- trace_trap_reg(__func__, rd->reg, p->is_write,
- vcpu->arch.vcpu_debug_state.dbg_wcr[rd->reg]);
-
return true;
}
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 70ac641..f020dd0 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -817,6 +817,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.4.5
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v7 09/11] KVM: arm64: guest debug, HW assisted debug support
2015-07-02 13:50 ` Alex Bennée
@ 2015-07-03 9:23 ` Will Deacon
2015-07-03 16:07 ` Alex Bennée
0 siblings, 1 reply; 22+ messages in thread
From: Will Deacon @ 2015-07-03 9:23 UTC (permalink / raw)
To: Alex Bennée
Cc: kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
kvmarm@lists.cs.columbia.edu, christoffer.dall@linaro.org,
Marc Zyngier, peter.maydell@linaro.org, agraf@suse.de,
drjones@redhat.com, pbonzini@redhat.com, zhichao.huang@linaro.org,
jan.kiszka@siemens.com, dahi@linux.vnet.ibm.com,
r65777@freescale.com, bp@suse.de, Gleb Natapov, Jonathan Corbet,
Catalin Marinas, Lorenzo Pieralisi, Ingo Molnar, Peter Zijlstra,
open list:DOCUMENTATION, open list, open list:ABI/API
Hi Alex,
On Thu, Jul 02, 2015 at 02:50:33PM +0100, Alex Bennée wrote:
> Are you happy with this?:
[...]
> +/**
> + * 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 +71,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 = hw_breakpoint_slots(TYPE_INST);
> + break;
> + case KVM_CAP_GUEST_DEBUG_HW_WPS:
> + r = hw_breakpoint_slots(TYPE_DATA);
> + break;
Whilst I much prefer this code, it actually adds an unwanted dependency
on PERF_EVENTS that I didn't think about to start with. Sorry to keep
messing you about -- I guess your original patch is the best thing after
all.
Will
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v7 09/11] KVM: arm64: guest debug, HW assisted debug support
2015-07-03 9:23 ` Will Deacon
@ 2015-07-03 16:07 ` Alex Bennée
2015-07-06 8:51 ` Will Deacon
0 siblings, 1 reply; 22+ messages in thread
From: Alex Bennée @ 2015-07-03 16:07 UTC (permalink / raw)
To: Will Deacon
Cc: kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
kvmarm@lists.cs.columbia.edu, christoffer.dall@linaro.org,
Marc Zyngier, peter.maydell@linaro.org, agraf@suse.de,
drjones@redhat.com, pbonzini@redhat.com, zhichao.huang@linaro.org,
jan.kiszka@siemens.com, dahi@linux.vnet.ibm.com,
r65777@freescale.com, bp@suse.de, Gleb Natapov, Jonathan Corbet,
Catalin Marinas, Lorenzo Pieralisi, Ingo Molnar, Peter Zijlstra,
open list:DOCUMENTATION, open list, open list:ABI/API
Will Deacon <will.deacon@arm.com> writes:
> Hi Alex,
>
> On Thu, Jul 02, 2015 at 02:50:33PM +0100, Alex Bennée wrote:
>> Are you happy with this?:
>
> [...]
>
>> +/**
>> + * 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 +71,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 = hw_breakpoint_slots(TYPE_INST);
>> + break;
>> + case KVM_CAP_GUEST_DEBUG_HW_WPS:
>> + r = hw_breakpoint_slots(TYPE_DATA);
>> + break;
>
> Whilst I much prefer this code, it actually adds an unwanted dependency
> on PERF_EVENTS that I didn't think about to start with. Sorry to keep
> messing you about -- I guess your original patch is the best thing after
> all.
Everything looks to be in hw_breakpoint.[ch] which does depend on
CONFIG_HAVE_HW_BREAKPOINT which depends on PERF_EVENTS to be built.
However the previous code depended on this behaviour as well.
It would seem weird to enable guest debug using HW debug registers to
debug the guest yet not allowing the host kernel to use them? Of course
this is the only code they would share as all the magic of guest
debugging is already mostly there for dirty guest handling.
I'm not familiar with Kconfig but it looks like this is all part of
arm64 defconfig. Are people really going to want to disable PERF_EVENTS
but still debug their guests with HW support?
>
> Will
--
Alex Bennée
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v7 09/11] KVM: arm64: guest debug, HW assisted debug support
2015-07-03 16:07 ` Alex Bennée
@ 2015-07-06 8:51 ` Will Deacon
2015-07-06 9:02 ` Alex Bennée
2015-07-06 9:31 ` Christoffer Dall
0 siblings, 2 replies; 22+ messages in thread
From: Will Deacon @ 2015-07-06 8:51 UTC (permalink / raw)
To: Alex Bennée
Cc: kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
kvmarm@lists.cs.columbia.edu, christoffer.dall@linaro.org,
Marc Zyngier, peter.maydell@linaro.org, agraf@suse.de,
drjones@redhat.com, pbonzini@redhat.com, zhichao.huang@linaro.org,
jan.kiszka@siemens.com, dahi@linux.vnet.ibm.com,
r65777@freescale.com, bp@suse.de, Gleb Natapov, Jonathan Corbet,
Catalin Marinas, Lorenzo Pieralisi, Ingo Molnar, Peter Zijlstra,
open list:DOCUMENTATION, open list, open list:ABI/API
On Fri, Jul 03, 2015 at 05:07:41PM +0100, Alex Bennée wrote:
> Will Deacon <will.deacon@arm.com> writes:
> > On Thu, Jul 02, 2015 at 02:50:33PM +0100, Alex Bennée wrote:
> >> Are you happy with this?:
> >
> > [...]
> >
> >> +/**
> >> + * 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 +71,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 = hw_breakpoint_slots(TYPE_INST);
> >> + break;
> >> + case KVM_CAP_GUEST_DEBUG_HW_WPS:
> >> + r = hw_breakpoint_slots(TYPE_DATA);
> >> + break;
> >
> > Whilst I much prefer this code, it actually adds an unwanted dependency
> > on PERF_EVENTS that I didn't think about to start with. Sorry to keep
> > messing you about -- I guess your original patch is the best thing after
> > all.
>
> Everything looks to be in hw_breakpoint.[ch] which does depend on
> CONFIG_HAVE_HW_BREAKPOINT which depends on PERF_EVENTS to be built.
> However the previous code depended on this behaviour as well.
I think your original approach (of sticking stuff in the header file) works
regardless of the CONFIG option, no?
> It would seem weird to enable guest debug using HW debug registers to
> debug the guest yet not allowing the host kernel to use them? Of course
> this is the only code they would share as all the magic of guest
> debugging is already mostly there for dirty guest handling.
>
> I'm not familiar with Kconfig but it looks like this is all part of
> arm64 defconfig. Are people really going to want to disable PERF_EVENTS
> but still debug their guests with HW support?
Then it's your call. I just find the host dependency on perf a bit weird
to get guest debug working (especially as the dependency is completely
"fake" because we don't use any perf infrastructure at all).
Will
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v7 09/11] KVM: arm64: guest debug, HW assisted debug support
2015-07-06 8:51 ` Will Deacon
@ 2015-07-06 9:02 ` Alex Bennée
2015-07-06 9:31 ` Christoffer Dall
1 sibling, 0 replies; 22+ messages in thread
From: Alex Bennée @ 2015-07-06 9:02 UTC (permalink / raw)
To: Will Deacon
Cc: kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
kvmarm@lists.cs.columbia.edu, christoffer.dall@linaro.org,
Marc Zyngier, peter.maydell@linaro.org, agraf@suse.de,
drjones@redhat.com, pbonzini@redhat.com, zhichao.huang@linaro.org,
jan.kiszka@siemens.com, dahi@linux.vnet.ibm.com,
r65777@freescale.com, bp@suse.de, Gleb Natapov, Jonathan Corbet,
Catalin Marinas, Lorenzo Pieralisi, Ingo Molnar, Peter Zijlstra,
open list:DOCUMENTATION, open list, open list:ABI/API
Will Deacon <will.deacon@arm.com> writes:
> On Fri, Jul 03, 2015 at 05:07:41PM +0100, Alex Bennée wrote:
>> Will Deacon <will.deacon@arm.com> writes:
>> > On Thu, Jul 02, 2015 at 02:50:33PM +0100, Alex Bennée wrote:
>> >> Are you happy with this?:
>> >
>> > [...]
>> >
>> >> +/**
>> >> + * 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 +71,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 = hw_breakpoint_slots(TYPE_INST);
>> >> + break;
>> >> + case KVM_CAP_GUEST_DEBUG_HW_WPS:
>> >> + r = hw_breakpoint_slots(TYPE_DATA);
>> >> + break;
>> >
>> > Whilst I much prefer this code, it actually adds an unwanted dependency
>> > on PERF_EVENTS that I didn't think about to start with. Sorry to keep
>> > messing you about -- I guess your original patch is the best thing after
>> > all.
>>
>> Everything looks to be in hw_breakpoint.[ch] which does depend on
>> CONFIG_HAVE_HW_BREAKPOINT which depends on PERF_EVENTS to be built.
>> However the previous code depended on this behaviour as well.
>
> I think your original approach (of sticking stuff in the header file) works
> regardless of the CONFIG option, no?
Ahh yeah I reverted that to an extern due to random compile breakage:
http://storage.kernelci.org/alex/v4.1-12-gd38574dba3ec/arm64-allmodconfig/build.log
I'll see if I can fix that up.
>> It would seem weird to enable guest debug using HW debug registers to
>> debug the guest yet not allowing the host kernel to use them? Of course
>> this is the only code they would share as all the magic of guest
>> debugging is already mostly there for dirty guest handling.
>>
>> I'm not familiar with Kconfig but it looks like this is all part of
>> arm64 defconfig. Are people really going to want to disable PERF_EVENTS
>> but still debug their guests with HW support?
>
> Then it's your call. I just find the host dependency on perf a bit weird
> to get guest debug working (especially as the dependency is completely
> "fake" because we don't use any perf infrastructure at all).
>
> Will
--
Alex Bennée
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v7 09/11] KVM: arm64: guest debug, HW assisted debug support
2015-07-06 8:51 ` Will Deacon
2015-07-06 9:02 ` Alex Bennée
@ 2015-07-06 9:31 ` Christoffer Dall
1 sibling, 0 replies; 22+ messages in thread
From: Christoffer Dall @ 2015-07-06 9:31 UTC (permalink / raw)
To: Will Deacon
Cc: Alex Bennée, kvm@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
kvmarm@lists.cs.columbia.edu, Marc Zyngier,
peter.maydell@linaro.org, agraf@suse.de, drjones@redhat.com,
pbonzini@redhat.com, zhichao.huang@linaro.org,
jan.kiszka@siemens.com, dahi@linux.vnet.ibm.com,
r65777@freescale.com, bp@suse.de, Gleb Natapov, Jonathan Corbet,
Catalin Marinas, Lorenzo Pieralisi, Ingo Molnar, Peter Zijlstra,
open list:DOCUMENTATION, open list, open list:ABI/API
On Mon, Jul 06, 2015 at 09:51:40AM +0100, Will Deacon wrote:
> On Fri, Jul 03, 2015 at 05:07:41PM +0100, Alex Bennée wrote:
> > Will Deacon <will.deacon@arm.com> writes:
> > > On Thu, Jul 02, 2015 at 02:50:33PM +0100, Alex Bennée wrote:
> > >> Are you happy with this?:
> > >
> > > [...]
> > >
> > >> +/**
> > >> + * 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 +71,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 = hw_breakpoint_slots(TYPE_INST);
> > >> + break;
> > >> + case KVM_CAP_GUEST_DEBUG_HW_WPS:
> > >> + r = hw_breakpoint_slots(TYPE_DATA);
> > >> + break;
> > >
> > > Whilst I much prefer this code, it actually adds an unwanted dependency
> > > on PERF_EVENTS that I didn't think about to start with. Sorry to keep
> > > messing you about -- I guess your original patch is the best thing after
> > > all.
> >
> > Everything looks to be in hw_breakpoint.[ch] which does depend on
> > CONFIG_HAVE_HW_BREAKPOINT which depends on PERF_EVENTS to be built.
> > However the previous code depended on this behaviour as well.
>
> I think your original approach (of sticking stuff in the header file) works
> regardless of the CONFIG option, no?
>
> > It would seem weird to enable guest debug using HW debug registers to
> > debug the guest yet not allowing the host kernel to use them? Of course
> > this is the only code they would share as all the magic of guest
> > debugging is already mostly there for dirty guest handling.
> >
> > I'm not familiar with Kconfig but it looks like this is all part of
> > arm64 defconfig. Are people really going to want to disable PERF_EVENTS
> > but still debug their guests with HW support?
>
> Then it's your call. I just find the host dependency on perf a bit weird
> to get guest debug working (especially as the dependency is completely
> "fake" because we don't use any perf infrastructure at all).
>
Agreed, let's see if we can avoid this.
Thanks,
-Christoffer
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v7 10/11] KVM: arm64: enable KVM_CAP_SET_GUEST_DEBUG
[not found] <1435775343-20034-1-git-send-email-alex.bennee@linaro.org>
` (8 preceding siblings ...)
2015-07-01 18:29 ` [PATCH v7 09/11] KVM: arm64: guest debug, HW assisted debug support Alex Bennée
@ 2015-07-01 18:29 ` Alex Bennée
2015-07-01 18:29 ` [PATCH v7 11/11] KVM: arm64: add trace points for guest_debug debug Alex Bennée
10 siblings, 0 replies; 22+ messages in thread
From: Alex Bennée @ 2015-07-01 18:29 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>
Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
---
v3:
- separated capability check from previous patches
- moved into arm64 specific ioctl handler.
v4:
- add a-b-tag
---
arch/arm64/kvm/reset.c | 3 +++
1 file changed, 3 insertions(+)
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.4.5
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v7 11/11] KVM: arm64: add trace points for guest_debug debug
[not found] <1435775343-20034-1-git-send-email-alex.bennee@linaro.org>
` (9 preceding siblings ...)
2015-07-01 18:29 ` [PATCH v7 10/11] KVM: arm64: enable KVM_CAP_SET_GUEST_DEBUG Alex Bennée
@ 2015-07-01 18:29 ` Alex Bennée
10 siblings, 0 replies; 22+ messages in thread
From: Alex Bennée @ 2015-07-01 18:29 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
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
v4
- u32/u64 split on debug registers
- fix for renames
- add tracing of traps/set_guest_debug
- remove handle_guest_debug trace
v5
- minor print fmt fix
- rm pstate traces
v6
- fix merge conflicts
- update control reg tracking to u64 (abi change)
v7
- fix merge conflicts from ioctl move
- fix other minor merge conflicts
- fixes for the re-factored sys_regs code
---
arch/arm64/kvm/debug.c | 35 ++++++++++++-
arch/arm64/kvm/guest.c | 4 ++
arch/arm64/kvm/sys_regs.c | 21 ++++++++
arch/arm64/kvm/trace.h | 123 ++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 182 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
index 46b73d7..119107f 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,11 +46,17 @@ static DEFINE_PER_CPU(u32, mdcr_el2);
static void save_guest_debug_regs(struct kvm_vcpu *vcpu)
{
vcpu->arch.guest_debug_preserved.mdscr_el1 = vcpu_sys_reg(vcpu, MDSCR_EL1);
+
+ trace_kvm_arm_set_dreg32("Saved MDSCR_EL1",
+ vcpu->arch.guest_debug_preserved.mdscr_el1);
}
static void restore_guest_debug_regs(struct kvm_vcpu *vcpu)
{
vcpu_sys_reg(vcpu, MDSCR_EL1) = vcpu->arch.guest_debug_preserved.mdscr_el1;
+
+ trace_kvm_arm_set_dreg32("Restored MDSCR_EL1",
+ vcpu_sys_reg(vcpu, MDSCR_EL1));
}
/**
@@ -99,6 +107,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 |
@@ -140,6 +150,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 Breakpoints and watchpoints
*
@@ -156,6 +168,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]);
}
}
@@ -165,10 +185,15 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
/* Trap debug register access */
if (trap_debug)
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);
@@ -176,8 +201,16 @@ void kvm_arm_clear_debug(struct kvm_vcpu *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)
+ if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW) {
kvm_arm_reset_debug_ptr(vcpu);
+ 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/guest.c b/arch/arm64/kvm/guest.c
index 6f1b249..48868d8 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -32,6 +32,8 @@
#include <asm/kvm_emulate.h>
#include <asm/kvm_coproc.h>
+#include "trace.h"
+
struct kvm_stats_debugfs_item debugfs_entries[] = {
{ NULL }
};
@@ -350,6 +352,8 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *vcpu,
int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
struct kvm_guest_debug *dbg)
{
+ trace_kvm_set_guest_debug(vcpu, dbg->control);
+
if (dbg->control & ~KVM_GUESTDBG_VALID_MASK)
return -EINVAL;
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index f753a67..2c9aa68 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
@@ -208,6 +210,8 @@ static bool trap_debug_regs(struct kvm_vcpu *vcpu,
*vcpu_reg(vcpu, p->Rt) = vcpu_sys_reg(vcpu, r->reg);
}
+ trace_trap_reg(__func__, r->reg, p->is_write, *vcpu_reg(vcpu, p->Rt));
+
return true;
}
@@ -245,6 +249,9 @@ static inline bool trap_bvr(struct kvm_vcpu *vcpu,
vcpu->arch.vcpu_debug_state.dbg_bvr[rd->reg]);
}
+ trace_trap_reg(__func__, rd->reg, p->is_write,
+ vcpu->arch.vcpu_debug_state.dbg_bvr[rd->reg]);
+
return true;
}
@@ -284,6 +291,9 @@ static inline bool trap_bcr(struct kvm_vcpu *vcpu,
vcpu->arch.vcpu_debug_state.dbg_bcr[rd->reg]);
}
+ trace_trap_reg(__func__, rd->reg, p->is_write,
+ vcpu->arch.vcpu_debug_state.dbg_bcr[rd->reg]);
+
return true;
}
@@ -323,6 +333,9 @@ static inline bool trap_wvr(struct kvm_vcpu *vcpu,
vcpu->arch.vcpu_debug_state.dbg_wvr[rd->reg]);
}
+ trace_trap_reg(__func__, rd->reg, p->is_write,
+ vcpu->arch.vcpu_debug_state.dbg_wvr[rd->reg]);
+
return true;
}
@@ -362,6 +375,9 @@ static inline bool trap_wcr(struct kvm_vcpu *vcpu,
vcpu->arch.vcpu_debug_state.dbg_wcr[rd->reg]);
}
+ trace_trap_reg(__func__, rd->reg, p->is_write,
+ vcpu->arch.vcpu_debug_state.dbg_wcr[rd->reg]);
+
return true;
}
@@ -719,6 +735,9 @@ static inline bool trap_xvr(struct kvm_vcpu *vcpu,
vcpu->arch.vcpu_debug_state.dbg_bvr[rd->reg] >> 32);
}
+ trace_trap_reg(__func__, rd->reg, p->is_write,
+ vcpu->arch.vcpu_debug_state.dbg_bvr[rd->reg]);
+
return true;
}
@@ -1200,6 +1219,8 @@ int kvm_handle_sys_reg(struct kvm_vcpu *vcpu, struct kvm_run *run)
struct sys_reg_params params;
unsigned long esr = kvm_vcpu_get_hsr(vcpu);
+ trace_kvm_handle_sys_reg(esr);
+
params.is_aarch32 = false;
params.is_32bit = false;
params.Op0 = (esr >> 20) & 3;
diff --git a/arch/arm64/kvm/trace.h b/arch/arm64/kvm/trace.h
index 157416e9..7fb0008 100644
--- a/arch/arm64/kvm/trace.h
+++ b/arch/arm64/kvm/trace.h
@@ -44,6 +44,129 @@ TRACE_EVENT(kvm_hvc_arm64,
__entry->vcpu_pc, __entry->r0, __entry->imm)
);
+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_reg,
+ TP_PROTO(const char *fn, int reg, bool is_write, u64 write_value),
+ TP_ARGS(fn, reg, is_write, write_value),
+
+ TP_STRUCT__entry(
+ __field(const char *, fn)
+ __field(int, reg)
+ __field(bool, is_write)
+ __field(u64, write_value)
+ ),
+
+ TP_fast_assign(
+ __entry->fn = fn;
+ __entry->reg = reg;
+ __entry->is_write = is_write;
+ __entry->write_value = write_value;
+ ),
+
+ TP_printk("%s %s reg %d (0x%08llx)", __entry->fn, __entry->is_write?"write to":"read from", __entry->reg, __entry->write_value)
+);
+
+TRACE_EVENT(kvm_handle_sys_reg,
+ TP_PROTO(unsigned long hsr),
+ TP_ARGS(hsr),
+
+ TP_STRUCT__entry(
+ __field(unsigned long, hsr)
+ ),
+
+ TP_fast_assign(
+ __entry->hsr = hsr;
+ ),
+
+ TP_printk("HSR 0x%08lx", __entry->hsr)
+);
+
+TRACE_EVENT(kvm_set_guest_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)
+);
+
+
#endif /* _TRACE_ARM64_KVM_H */
#undef TRACE_INCLUDE_PATH
--
2.4.5
^ permalink raw reply related [flat|nested] 22+ messages in thread