Re: [PATCH v5 09/12] KVM: arm64: introduce vcpu->arch.debug_ptr
On Fri, May 29, 2015 at 10:30:25AM +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. As we are no longer tied to the layout of the sys_regs[] we
> can make the copies size appropriate for control and value registers.
>
> This also entails updating the sys_regs code to access this new
> structure. Instead of passing a register index we now pass an offset
> into the kvm_guest_debug_arch structure.
>
> We also need to ensure the GET/SET_ONE_REG ioctl operations store the
> registers in their correct location.
>
> Signed-off-by: Alex Bennée
> ---
> arch/arm/kvm/arm.c| 3 +
> arch/arm64/include/asm/kvm_asm.h | 24 +++-
> arch/arm64/include/asm/kvm_host.h | 12 +++-
> arch/arm64/kernel/asm-offsets.c | 6 ++
> arch/arm64/kvm/hyp.S | 107 +---
> arch/arm64/kvm/sys_regs.c | 126
> +++---
> 6 files changed, 188 insertions(+), 90 deletions(-)
>
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index 9b3ed6d..0d17c7b 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -279,6 +279,9 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
> /* Set up the timer */
> kvm_timer_vcpu_init(vcpu);
>
> + /* Set the debug registers to be the guests */
> + vcpu->arch.debug_ptr = &vcpu->arch.vcpu_debug_state;
> +
> 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_EL122 /* 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_EL291 /* Debug Vector Catch Register */
> -#define TEECR32_EL1 92 /* ThumbEE Configuration Register */
> -#define TEEHBR32_EL193 /* 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_EL227 /* Debug Vector Catch Register */
> +#define TEECR32_EL1 28 /* ThumbEE Configuration Register */
> +#define TEEHBR32_EL129 /* 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..e5040b6 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -108,11 +108,21 @@ struct kvm_vcpu_arch {
> /* Exception Information */
> struct kvm_vcpu_fault_info fault;
>
> - /* Debug state */
> + /* Guest debug state */
> u64 debug_flags;
>
> + /*
> + * For debugging the guest we need to keep a set of debug
> + * registers which can override the guests own debug state
the guest's
> + * while being used. These are set via the KVM_SET_GUEST_DEBUG
> + * ioctl.
Which ones are set via the KVM_SET_GUEST_DEBUG ioctl?
This comment doesn't feel like it's properly explaining the fields
below, because it feels like there's missing a set of registers for this
to add up. I would suggest rewording this comment to something like:
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 whi
[PATCH v5 09/12] KVM: arm64: introduce vcpu->arch.debug_ptr
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. As we are no longer tied to the layout of the sys_regs[] we
can make the copies size appropriate for control and value registers.
This also entails updating the sys_regs code to access this new
structure. Instead of passing a register index we now pass an offset
into the kvm_guest_debug_arch structure.
We also need to ensure the GET/SET_ONE_REG ioctl operations store the
registers in their correct location.
Signed-off-by: Alex Bennée
---
arch/arm/kvm/arm.c| 3 +
arch/arm64/include/asm/kvm_asm.h | 24 +++-
arch/arm64/include/asm/kvm_host.h | 12 +++-
arch/arm64/kernel/asm-offsets.c | 6 ++
arch/arm64/kvm/hyp.S | 107 +---
arch/arm64/kvm/sys_regs.c | 126 +++---
6 files changed, 188 insertions(+), 90 deletions(-)
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 9b3ed6d..0d17c7b 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -279,6 +279,9 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
/* Set up the timer */
kvm_timer_vcpu_init(vcpu);
+ /* Set the debug registers to be the guests */
+ vcpu->arch.debug_ptr = &vcpu->arch.vcpu_debug_state;
+
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 @@
#defineCNTKCTL_EL1 20 /* Timer Control Register (EL1) */
#definePAR_EL1 21 /* Physical Address Register */
#define MDSCR_EL1 22 /* Monitor Debug System Control Register */
-#define DBGBCR0_EL123 /* Debug Breakpoint Control Registers (0-15) */
-#define DBGBCR15_EL1 38
-#define DBGBVR0_EL139 /* Debug Breakpoint Value Registers (0-15) */
-#define DBGBVR15_EL1 54
-#define DBGWCR0_EL155 /* Debug Watchpoint Control Registers (0-15) */
-#define DBGWCR15_EL1 70
-#define DBGWVR0_EL171 /* Debug Watchpoint Value Registers (0-15) */
-#define DBGWVR15_EL1 86
-#define MDCCINT_EL187 /* Monitor Debug Comms Channel Interrupt Enable
Reg */
+#define MDCCINT_EL123 /* Monitor Debug Comms Channel Interrupt Enable
Reg */
/* 32bit specific registers. Keep them at the end of the range */
-#defineDACR32_EL2 88 /* Domain Access Control Register */
-#defineIFSR32_EL2 89 /* Instruction Fault Status Register */
-#defineFPEXC32_EL2 90 /* Floating-Point Exception Control
Register */
-#defineDBGVCR32_EL291 /* Debug Vector Catch Register */
-#defineTEECR32_EL1 92 /* ThumbEE Configuration Register */
-#defineTEEHBR32_EL193 /* ThumbEE Handler Base Register */
-#defineNR_SYS_REGS 94
+#defineDACR32_EL2 24 /* Domain Access Control Register */
+#defineIFSR32_EL2 25 /* Instruction Fault Status Register */
+#defineFPEXC32_EL2 26 /* Floating-Point Exception Control
Register */
+#defineDBGVCR32_EL227 /* Debug Vector Catch Register */
+#defineTEECR32_EL1 28 /* ThumbEE Configuration Register */
+#defineTEEHBR32_EL129 /* ThumbEE Handler Base Register */
+#defineNR_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..e5040b6 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -108,11 +108,21 @@ struct kvm_vcpu_arch {
/* Exception Information */
struct kvm_vcpu_fault_info fault;
- /* Debug state */
+ /* Guest debug state */
u64 debug_flags;
+ /*
+* For debugging the guest we need to keep a set of debug
+* registers which can override the guests own debug state
+* while being used. These are set via the KVM_SET_GUEST_DEBUG
+* ioctl.
+*/
+ struct kvm_guest_debug_arch *debug_ptr;
+ struct kvm_guest_debug_arch 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;
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 k
