Re: [PATCH 5/7] KVM: arm64: guest debug, add support for single-step
On Mon, Dec 01, 2014 at 11:50:14AM +, Alex Bennée wrote:
>
> Christoffer Dall writes:
>
> > On Tue, Nov 25, 2014 at 04:10:03PM +, Alex Bennée wrote:
> >> This adds support for single-stepping the guest. As userspace can and
> >> will manipulate guest registers before restarting any tweaking of the
> >> registers has to occur just before control is passed back to the guest.
> >> Furthermore while guest debugging is in effect we need to squash the
> >> ability of the guest to single-step itself as we have no easy way of
> >> re-entering the guest after the exception has been delivered to the
> >> hypervisor.
> >
> > Admittedly this is a corner case, but wouldn't the only really nasty bit
> > of this be to emulate the guest debug exception?
>
> Well yes - currently this is all squashed by ignoring the guest's wishes
> while we are debugging (save for SW breakpoints).
>
> >
> >>
> >> Signed-off-by: Alex Bennée
> >>
> >> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> >> index 48d26bb..a76daae 100644
> >> --- a/arch/arm/kvm/arm.c
> >> +++ b/arch/arm/kvm/arm.c
> >> @@ -38,6 +38,7 @@
> >> #include
> >> #include
> >> #include
> >> +#include
> >> #include
> >> #include
> >> #include
> >> @@ -300,6 +301,17 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
> >>kvm_arm_set_running_vcpu(NULL);
> >> }
> >>
> >> +/**
> >> + * kvm_arch_vcpu_ioctl_set_guest_debug - Setup guest debugging
> >> + * @kvm: pointer to the KVM struct
> >> + * @kvm_guest_debug: the ioctl data buffer
> >> + *
> >> + * This sets up the VM for guest debugging. Care has to be taken when
> >> + * manipulating guest registers as these will be set/cleared by the
> >> + * hyper-visor controller, typically before each kvm_run event. As a
> >
> > hypervisor
> >
> >> + * result modification of the guest registers needs to take place
> >> + * after they have been restored in the hyp.S trampoline code.
> >
> > I don't understand this??
>
> We can't use GET/SET one reg to manipulate the registers we want as
> these are the guest visible versions and subject to modification by
> userspace. This is why the debugging code makes it's changes after the
> guest state has been restored.
>
eh, once you're in the KVM_RUN ioctl, user space can't fiddle your VCPU
regs because you're holding the vcpu mutex, so doing stuff in some
callout from kvm_arch_vcpu_ioctl_run() seems every bid as valid for this
case as doing it in EL2. In fact, the only reason why we're doing
anything in EL2 is when you're accessing state only accessible in EL2,
when you need to write the whole thing in assembly (like the context
switch of GP registers) etc.
If it doesn't have huge performance costs, we should use C-code in EL1
to the furthest extent possible.
> >
> >> + */
> >> int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
> >>struct kvm_guest_debug *dbg)
> >> {
> >> @@ -317,8 +329,8 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct
> >> kvm_vcpu *vcpu,
> >>
> >>/* Single Step */
> >>if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) {
> >> - kvm_info("SS requested, not yet implemented\n");
> >> - return -EINVAL;
> >> + kvm_info("SS requested\n");
> >> + route_el2 = true;
> >>}
> >>
> >>/* Software Break Points */
> >> diff --git a/arch/arm64/kernel/asm-offsets.c
> >> b/arch/arm64/kernel/asm-offsets.c
> >> index 8da1043..78e5ae1 100644
> >> --- a/arch/arm64/kernel/asm-offsets.c
> >> +++ b/arch/arm64/kernel/asm-offsets.c
> >> @@ -121,6 +121,7 @@ int main(void)
> >>DEFINE(VCPU_FAR_EL2,offsetof(struct kvm_vcpu,
> >> arch.fault.far_el2));
> >>DEFINE(VCPU_HPFAR_EL2, offsetof(struct kvm_vcpu,
> >> arch.fault.hpfar_el2));
> >>DEFINE(VCPU_DEBUG_FLAGS,offsetof(struct kvm_vcpu,
> >> arch.debug_flags));
> >> + DEFINE(GUEST_DEBUG, offsetof(struct kvm_vcpu, guest_debug));
> >>DEFINE(VCPU_HCR_EL2,offsetof(struct kvm_vcpu,
> >> arch.hcr_el2));
> >>DEFINE(VCPU_MDCR_EL2, offsetof(struct kvm_vcpu, arch.mdcr_el2));
> >>DEFINE(VCPU_IRQ_LINES, offsetof(struct kvm_vcpu, arch.irq_lines));
> >> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
> >> index 28dc92b..6def054 100644
> >> --- a/arch/arm64/kvm/handle_exit.c
> >> +++ b/arch/arm64/kvm/handle_exit.c
> >> @@ -91,6 +91,25 @@ static int kvm_handle_bkpt(struct kvm_vcpu *vcpu,
> >> struct kvm_run *run)
> >>return 0;
> >> }
> >>
> >> +/**
> >> + * kvm_handle_ss - handle single step exceptions
> >> + *
> >> + * @vcpu: the vcpu pointer
> >> + *
> >> + * See: ARM ARM D2.12 for the details. While the host is routing debug
> >> + * exceptions to it's handlers we have to suppress the ability of the
> >
> > its handlers
> >
> >> + * guest to trigger exceptions.
> >
> > not really sure why this comment is here? Does it really help anyone
> > reading this specific function or does it just confuse people more?
> >
>
Re: [PATCH 5/7] KVM: arm64: guest debug, add support for single-step
On Mon, Dec 01, 2014 at 11:50:14AM +, Alex Bennée wrote:
Christoffer Dall christoffer.d...@linaro.org writes:
On Tue, Nov 25, 2014 at 04:10:03PM +, Alex Bennée wrote:
This adds support for single-stepping the guest. As userspace can and
will manipulate guest registers before restarting any tweaking of the
registers has to occur just before control is passed back to the guest.
Furthermore while guest debugging is in effect we need to squash the
ability of the guest to single-step itself as we have no easy way of
re-entering the guest after the exception has been delivered to the
hypervisor.
Admittedly this is a corner case, but wouldn't the only really nasty bit
of this be to emulate the guest debug exception?
Well yes - currently this is all squashed by ignoring the guest's wishes
while we are debugging (save for SW breakpoints).
Signed-off-by: Alex Bennée alex.ben...@linaro.org
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 48d26bb..a76daae 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -38,6 +38,7 @@
#include asm/tlbflush.h
#include asm/cacheflush.h
#include asm/virt.h
+#include asm/debug-monitors.h
#include asm/kvm_arm.h
#include asm/kvm_asm.h
#include asm/kvm_mmu.h
@@ -300,6 +301,17 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
kvm_arm_set_running_vcpu(NULL);
}
+/**
+ * kvm_arch_vcpu_ioctl_set_guest_debug - Setup guest debugging
+ * @kvm: pointer to the KVM struct
+ * @kvm_guest_debug: the ioctl data buffer
+ *
+ * This sets up the VM for guest debugging. Care has to be taken when
+ * manipulating guest registers as these will be set/cleared by the
+ * hyper-visor controller, typically before each kvm_run event. As a
hypervisor
+ * result modification of the guest registers needs to take place
+ * after they have been restored in the hyp.S trampoline code.
I don't understand this??
We can't use GET/SET one reg to manipulate the registers we want as
these are the guest visible versions and subject to modification by
userspace. This is why the debugging code makes it's changes after the
guest state has been restored.
eh, once you're in the KVM_RUN ioctl, user space can't fiddle your VCPU
regs because you're holding the vcpu mutex, so doing stuff in some
callout from kvm_arch_vcpu_ioctl_run() seems every bid as valid for this
case as doing it in EL2. In fact, the only reason why we're doing
anything in EL2 is when you're accessing state only accessible in EL2,
when you need to write the whole thing in assembly (like the context
switch of GP registers) etc.
If it doesn't have huge performance costs, we should use C-code in EL1
to the furthest extent possible.
+ */
int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
struct kvm_guest_debug *dbg)
{
@@ -317,8 +329,8 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct
kvm_vcpu *vcpu,
/* Single Step */
if (vcpu-guest_debug KVM_GUESTDBG_SINGLESTEP) {
- kvm_info(SS requested, not yet implemented\n);
- return -EINVAL;
+ kvm_info(SS requested\n);
+ route_el2 = true;
}
/* Software Break Points */
diff --git a/arch/arm64/kernel/asm-offsets.c
b/arch/arm64/kernel/asm-offsets.c
index 8da1043..78e5ae1 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -121,6 +121,7 @@ int main(void)
DEFINE(VCPU_FAR_EL2,offsetof(struct kvm_vcpu,
arch.fault.far_el2));
DEFINE(VCPU_HPFAR_EL2, offsetof(struct kvm_vcpu,
arch.fault.hpfar_el2));
DEFINE(VCPU_DEBUG_FLAGS,offsetof(struct kvm_vcpu,
arch.debug_flags));
+ DEFINE(GUEST_DEBUG, offsetof(struct kvm_vcpu, guest_debug));
DEFINE(VCPU_HCR_EL2,offsetof(struct kvm_vcpu,
arch.hcr_el2));
DEFINE(VCPU_MDCR_EL2, offsetof(struct kvm_vcpu, arch.mdcr_el2));
DEFINE(VCPU_IRQ_LINES, offsetof(struct kvm_vcpu, arch.irq_lines));
diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index 28dc92b..6def054 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -91,6 +91,25 @@ static int kvm_handle_bkpt(struct kvm_vcpu *vcpu,
struct kvm_run *run)
return 0;
}
+/**
+ * kvm_handle_ss - handle single step exceptions
+ *
+ * @vcpu: the vcpu pointer
+ *
+ * See: ARM ARM D2.12 for the details. While the host is routing debug
+ * exceptions to it's handlers we have to suppress the ability of the
its handlers
+ * guest to trigger exceptions.
not really sure why this comment is here? Does it really help anyone
reading this specific function or does it just confuse people more?
+ */
+static int kvm_handle_ss(struct kvm_vcpu *vcpu, struct kvm_run *run)
+{
+ WARN_ON(!(vcpu-guest_debug KVM_GUESTDBG_SINGLESTEP));
is this something
Re: [PATCH 5/7] KVM: arm64: guest debug, add support for single-step
Christoffer Dall writes:
> On Tue, Nov 25, 2014 at 04:10:03PM +, Alex Bennée wrote:
>> This adds support for single-stepping the guest. As userspace can and
>> will manipulate guest registers before restarting any tweaking of the
>> registers has to occur just before control is passed back to the guest.
>> Furthermore while guest debugging is in effect we need to squash the
>> ability of the guest to single-step itself as we have no easy way of
>> re-entering the guest after the exception has been delivered to the
>> hypervisor.
>
> Admittedly this is a corner case, but wouldn't the only really nasty bit
> of this be to emulate the guest debug exception?
Well yes - currently this is all squashed by ignoring the guest's wishes
while we are debugging (save for SW breakpoints).
>
>>
>> Signed-off-by: Alex Bennée
>>
>> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
>> index 48d26bb..a76daae 100644
>> --- a/arch/arm/kvm/arm.c
>> +++ b/arch/arm/kvm/arm.c
>> @@ -38,6 +38,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>> #include
>> #include
>> @@ -300,6 +301,17 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
>> kvm_arm_set_running_vcpu(NULL);
>> }
>>
>> +/**
>> + * kvm_arch_vcpu_ioctl_set_guest_debug - Setup guest debugging
>> + * @kvm:pointer to the KVM struct
>> + * @kvm_guest_debug: the ioctl data buffer
>> + *
>> + * This sets up the VM for guest debugging. Care has to be taken when
>> + * manipulating guest registers as these will be set/cleared by the
>> + * hyper-visor controller, typically before each kvm_run event. As a
>
> hypervisor
>
>> + * result modification of the guest registers needs to take place
>> + * after they have been restored in the hyp.S trampoline code.
>
> I don't understand this??
We can't use GET/SET one reg to manipulate the registers we want as
these are the guest visible versions and subject to modification by
userspace. This is why the debugging code makes it's changes after the
guest state has been restored.
>
>> + */
>> int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
>> struct kvm_guest_debug *dbg)
>> {
>> @@ -317,8 +329,8 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu
>> *vcpu,
>>
>> /* Single Step */
>> if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) {
>> -kvm_info("SS requested, not yet implemented\n");
>> -return -EINVAL;
>> +kvm_info("SS requested\n");
>> +route_el2 = true;
>> }
>>
>> /* Software Break Points */
>> diff --git a/arch/arm64/kernel/asm-offsets.c
>> b/arch/arm64/kernel/asm-offsets.c
>> index 8da1043..78e5ae1 100644
>> --- a/arch/arm64/kernel/asm-offsets.c
>> +++ b/arch/arm64/kernel/asm-offsets.c
>> @@ -121,6 +121,7 @@ int main(void)
>>DEFINE(VCPU_FAR_EL2, offsetof(struct kvm_vcpu,
>> arch.fault.far_el2));
>>DEFINE(VCPU_HPFAR_EL2,offsetof(struct kvm_vcpu,
>> arch.fault.hpfar_el2));
>>DEFINE(VCPU_DEBUG_FLAGS, offsetof(struct kvm_vcpu, arch.debug_flags));
>> + DEFINE(GUEST_DEBUG, offsetof(struct kvm_vcpu, guest_debug));
>>DEFINE(VCPU_HCR_EL2, offsetof(struct kvm_vcpu,
>> arch.hcr_el2));
>>DEFINE(VCPU_MDCR_EL2, offsetof(struct kvm_vcpu, arch.mdcr_el2));
>>DEFINE(VCPU_IRQ_LINES,offsetof(struct kvm_vcpu, arch.irq_lines));
>> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
>> index 28dc92b..6def054 100644
>> --- a/arch/arm64/kvm/handle_exit.c
>> +++ b/arch/arm64/kvm/handle_exit.c
>> @@ -91,6 +91,25 @@ static int kvm_handle_bkpt(struct kvm_vcpu *vcpu, struct
>> kvm_run *run)
>> return 0;
>> }
>>
>> +/**
>> + * kvm_handle_ss - handle single step exceptions
>> + *
>> + * @vcpu: the vcpu pointer
>> + *
>> + * See: ARM ARM D2.12 for the details. While the host is routing debug
>> + * exceptions to it's handlers we have to suppress the ability of the
>
> its handlers
>
>> + * guest to trigger exceptions.
>
> not really sure why this comment is here? Does it really help anyone
> reading this specific function or does it just confuse people more?
>
>> + */
>> +static int kvm_handle_ss(struct kvm_vcpu *vcpu, struct kvm_run *run)
>> +{
>> +WARN_ON(!(vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP));
>
> is this something that can actually happen or should it be a BUG_ON() -
> which may even go away once you're doing hacking on this?
It shouldn't happen. I was treating more like an assert, failure of
which would indicate something has gone wrong somewhere although
generally not worth bringing the kernel down for.
>
>> +
>> +run->exit_reason = KVM_EXIT_DEBUG;
>> +run->debug.arch.exit_type = KVM_DEBUG_EXIT_SINGLE_STEP;
>> +run->debug.arch.address = *vcpu_pc(vcpu);
>> +return 0;
>> +}
>> +
>> static exit_handle_fn arm_exit_handlers[] = {
>> [ESR_EL2_EC_WFI]= kvm_handle_wfx,
>> [ESR_EL2_EC_CP15_32]= kvm_handle_cp15_32,
>>
Re: [PATCH 5/7] KVM: arm64: guest debug, add support for single-step
Christoffer Dall christoffer.d...@linaro.org writes:
On Tue, Nov 25, 2014 at 04:10:03PM +, Alex Bennée wrote:
This adds support for single-stepping the guest. As userspace can and
will manipulate guest registers before restarting any tweaking of the
registers has to occur just before control is passed back to the guest.
Furthermore while guest debugging is in effect we need to squash the
ability of the guest to single-step itself as we have no easy way of
re-entering the guest after the exception has been delivered to the
hypervisor.
Admittedly this is a corner case, but wouldn't the only really nasty bit
of this be to emulate the guest debug exception?
Well yes - currently this is all squashed by ignoring the guest's wishes
while we are debugging (save for SW breakpoints).
Signed-off-by: Alex Bennée alex.ben...@linaro.org
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 48d26bb..a76daae 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -38,6 +38,7 @@
#include asm/tlbflush.h
#include asm/cacheflush.h
#include asm/virt.h
+#include asm/debug-monitors.h
#include asm/kvm_arm.h
#include asm/kvm_asm.h
#include asm/kvm_mmu.h
@@ -300,6 +301,17 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
kvm_arm_set_running_vcpu(NULL);
}
+/**
+ * kvm_arch_vcpu_ioctl_set_guest_debug - Setup guest debugging
+ * @kvm:pointer to the KVM struct
+ * @kvm_guest_debug: the ioctl data buffer
+ *
+ * This sets up the VM for guest debugging. Care has to be taken when
+ * manipulating guest registers as these will be set/cleared by the
+ * hyper-visor controller, typically before each kvm_run event. As a
hypervisor
+ * result modification of the guest registers needs to take place
+ * after they have been restored in the hyp.S trampoline code.
I don't understand this??
We can't use GET/SET one reg to manipulate the registers we want as
these are the guest visible versions and subject to modification by
userspace. This is why the debugging code makes it's changes after the
guest state has been restored.
+ */
int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
struct kvm_guest_debug *dbg)
{
@@ -317,8 +329,8 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu
*vcpu,
/* Single Step */
if (vcpu-guest_debug KVM_GUESTDBG_SINGLESTEP) {
-kvm_info(SS requested, not yet implemented\n);
-return -EINVAL;
+kvm_info(SS requested\n);
+route_el2 = true;
}
/* Software Break Points */
diff --git a/arch/arm64/kernel/asm-offsets.c
b/arch/arm64/kernel/asm-offsets.c
index 8da1043..78e5ae1 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -121,6 +121,7 @@ int main(void)
DEFINE(VCPU_FAR_EL2, offsetof(struct kvm_vcpu,
arch.fault.far_el2));
DEFINE(VCPU_HPFAR_EL2,offsetof(struct kvm_vcpu,
arch.fault.hpfar_el2));
DEFINE(VCPU_DEBUG_FLAGS, offsetof(struct kvm_vcpu, arch.debug_flags));
+ DEFINE(GUEST_DEBUG, offsetof(struct kvm_vcpu, guest_debug));
DEFINE(VCPU_HCR_EL2, offsetof(struct kvm_vcpu,
arch.hcr_el2));
DEFINE(VCPU_MDCR_EL2, offsetof(struct kvm_vcpu, arch.mdcr_el2));
DEFINE(VCPU_IRQ_LINES,offsetof(struct kvm_vcpu, arch.irq_lines));
diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index 28dc92b..6def054 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -91,6 +91,25 @@ static int kvm_handle_bkpt(struct kvm_vcpu *vcpu, struct
kvm_run *run)
return 0;
}
+/**
+ * kvm_handle_ss - handle single step exceptions
+ *
+ * @vcpu: the vcpu pointer
+ *
+ * See: ARM ARM D2.12 for the details. While the host is routing debug
+ * exceptions to it's handlers we have to suppress the ability of the
its handlers
+ * guest to trigger exceptions.
not really sure why this comment is here? Does it really help anyone
reading this specific function or does it just confuse people more?
+ */
+static int kvm_handle_ss(struct kvm_vcpu *vcpu, struct kvm_run *run)
+{
+WARN_ON(!(vcpu-guest_debug KVM_GUESTDBG_SINGLESTEP));
is this something that can actually happen or should it be a BUG_ON() -
which may even go away once you're doing hacking on this?
It shouldn't happen. I was treating more like an assert, failure of
which would indicate something has gone wrong somewhere although
generally not worth bringing the kernel down for.
+
+run-exit_reason = KVM_EXIT_DEBUG;
+run-debug.arch.exit_type = KVM_DEBUG_EXIT_SINGLE_STEP;
+run-debug.arch.address = *vcpu_pc(vcpu);
+return 0;
+}
+
static exit_handle_fn arm_exit_handlers[] = {
[ESR_EL2_EC_WFI]= kvm_handle_wfx,
[ESR_EL2_EC_CP15_32]= kvm_handle_cp15_32,
@@ -105,6 +124,7 @@ static exit_handle_fn arm_exit_handlers[] = {
Re: [PATCH 5/7] KVM: arm64: guest debug, add support for single-step
On Tue, Nov 25, 2014 at 04:10:03PM +, Alex Bennée wrote:
> This adds support for single-stepping the guest. As userspace can and
> will manipulate guest registers before restarting any tweaking of the
> registers has to occur just before control is passed back to the guest.
> Furthermore while guest debugging is in effect we need to squash the
> ability of the guest to single-step itself as we have no easy way of
> re-entering the guest after the exception has been delivered to the
> hypervisor.
Admittedly this is a corner case, but wouldn't the only really nasty bit
of this be to emulate the guest debug exception?
>
> Signed-off-by: Alex Bennée
>
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index 48d26bb..a76daae 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -38,6 +38,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -300,6 +301,17 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
> kvm_arm_set_running_vcpu(NULL);
> }
>
> +/**
> + * kvm_arch_vcpu_ioctl_set_guest_debug - Setup guest debugging
> + * @kvm: pointer to the KVM struct
> + * @kvm_guest_debug: the ioctl data buffer
> + *
> + * This sets up the VM for guest debugging. Care has to be taken when
> + * manipulating guest registers as these will be set/cleared by the
> + * hyper-visor controller, typically before each kvm_run event. As a
hypervisor
> + * result modification of the guest registers needs to take place
> + * after they have been restored in the hyp.S trampoline code.
I don't understand this??
> + */
> int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
> struct kvm_guest_debug *dbg)
> {
> @@ -317,8 +329,8 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu
> *vcpu,
>
> /* Single Step */
> if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) {
> - kvm_info("SS requested, not yet implemented\n");
> - return -EINVAL;
> + kvm_info("SS requested\n");
> + route_el2 = true;
> }
>
> /* Software Break Points */
> diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
> index 8da1043..78e5ae1 100644
> --- a/arch/arm64/kernel/asm-offsets.c
> +++ b/arch/arm64/kernel/asm-offsets.c
> @@ -121,6 +121,7 @@ int main(void)
>DEFINE(VCPU_FAR_EL2, offsetof(struct kvm_vcpu,
> arch.fault.far_el2));
>DEFINE(VCPU_HPFAR_EL2, offsetof(struct kvm_vcpu,
> arch.fault.hpfar_el2));
>DEFINE(VCPU_DEBUG_FLAGS, offsetof(struct kvm_vcpu, arch.debug_flags));
> + DEFINE(GUEST_DEBUG,offsetof(struct kvm_vcpu, guest_debug));
>DEFINE(VCPU_HCR_EL2, offsetof(struct kvm_vcpu,
> arch.hcr_el2));
>DEFINE(VCPU_MDCR_EL2, offsetof(struct kvm_vcpu, arch.mdcr_el2));
>DEFINE(VCPU_IRQ_LINES, offsetof(struct kvm_vcpu, arch.irq_lines));
> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
> index 28dc92b..6def054 100644
> --- a/arch/arm64/kvm/handle_exit.c
> +++ b/arch/arm64/kvm/handle_exit.c
> @@ -91,6 +91,25 @@ static int kvm_handle_bkpt(struct kvm_vcpu *vcpu, struct
> kvm_run *run)
> return 0;
> }
>
> +/**
> + * kvm_handle_ss - handle single step exceptions
> + *
> + * @vcpu:the vcpu pointer
> + *
> + * See: ARM ARM D2.12 for the details. While the host is routing debug
> + * exceptions to it's handlers we have to suppress the ability of the
its handlers
> + * guest to trigger exceptions.
not really sure why this comment is here? Does it really help anyone
reading this specific function or does it just confuse people more?
> + */
> +static int kvm_handle_ss(struct kvm_vcpu *vcpu, struct kvm_run *run)
> +{
> + WARN_ON(!(vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP));
is this something that can actually happen or should it be a BUG_ON() -
which may even go away once you're doing hacking on this?
> +
> + run->exit_reason = KVM_EXIT_DEBUG;
> + run->debug.arch.exit_type = KVM_DEBUG_EXIT_SINGLE_STEP;
> + run->debug.arch.address = *vcpu_pc(vcpu);
> + return 0;
> +}
> +
> static exit_handle_fn arm_exit_handlers[] = {
> [ESR_EL2_EC_WFI]= kvm_handle_wfx,
> [ESR_EL2_EC_CP15_32]= kvm_handle_cp15_32,
> @@ -105,6 +124,7 @@ static exit_handle_fn arm_exit_handlers[] = {
> [ESR_EL2_EC_SYS64] = kvm_handle_sys_reg,
> [ESR_EL2_EC_IABT] = kvm_handle_guest_abort,
> [ESR_EL2_EC_DABT] = kvm_handle_guest_abort,
> + [ESR_EL2_EC_SOFTSTP]= kvm_handle_ss,
> [ESR_EL2_EC_BKPT32] = kvm_handle_bkpt,
> [ESR_EL2_EC_BRK64] = kvm_handle_bkpt,
> };
> diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
> index 3c733ea..c0bc218 100644
> --- a/arch/arm64/kvm/hyp.S
> +++ b/arch/arm64/kvm/hyp.S
> @@ -16,6 +16,7 @@
> */
>
> #include
> +#include
>
> #include
> #include
> @@ -168,6 +169,31 @@
> // x19-x29, lr, sp*, elr*,
Re: [PATCH 5/7] KVM: arm64: guest debug, add support for single-step
On 30 November 2014 at 10:10, Christoffer Dall wrote: > In any case, I think it was related to how userspace observes the state > of the CPU, because when you do the MMIO operation emulation in > userspace, currently if you observe the PC though GET_ONE_REG, you'll > see a PC pointing to the next instruction, not the one you're emulating > which is strange. Also if we ever add support for userspace to say "this MMIO should fault" then we definitely need the PC-advance to happen afterwards, not before. > Not sure what the relation to a guest single-stepping itself was. I think it just came up in the course of that discussion, because single-step handling also needs to perform an action (clear PSTATE.SS) as part of the "advance over this insn" operation. But I think that you're right that doing the advance before dropping out to userspace is no worse for singlestep than it is for any other case. -- PMM -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/7] KVM: arm64: guest debug, add support for single-step
On Wed, Nov 26, 2014 at 07:27:06PM +, Peter Maydell wrote: > On 25 November 2014 at 16:10, Alex Bennée wrote: > > This adds support for single-stepping the guest. As userspace can and > > will manipulate guest registers before restarting any tweaking of the > > registers has to occur just before control is passed back to the guest. > > Furthermore while guest debugging is in effect we need to squash the > > ability of the guest to single-step itself as we have no easy way of > > re-entering the guest after the exception has been delivered to the > > hypervisor. > > A corner case I don't think this patch handles: if the debugger > tries to single step an insn which is emulated by the > hypervisor (because it's a load/store which is trapped and > handled as emulated mmio in userspace) then we won't > correctly update the single-step state machine (and so we'll end > up incorrectly stopping after the following insn rather than > before, I think). > > You should be able to achieve this effect by simply always clearing > the guest's PSTATE.SS when you advance the PC to skip the emulated > instruction (cf the comment in the pseudocode SSAdvance() function). > > I think we should also be doing this PC advance on return from > userspace's handling of the mmio rather than before we drop back > to userspace as we do now, but I can't remember why I think that. > Christoffer, I don't suppose you recall, do you? I think it was > you I had this conversation with on IRC a month or so back... > I don't remember clearly, no. Was it not during lunch at LCU we had this conversation? In any case, I think it was related to how userspace observes the state of the CPU, because when you do the MMIO operation emulation in userspace, currently if you observe the PC though GET_ONE_REG, you'll see a PC pointing to the next instruction, not the one you're emulating which is strange. Not sure what the relation to a guest single-stepping itself was. -Christoffer -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/7] KVM: arm64: guest debug, add support for single-step
On Wed, Nov 26, 2014 at 07:27:06PM +, Peter Maydell wrote: On 25 November 2014 at 16:10, Alex Bennée alex.ben...@linaro.org wrote: This adds support for single-stepping the guest. As userspace can and will manipulate guest registers before restarting any tweaking of the registers has to occur just before control is passed back to the guest. Furthermore while guest debugging is in effect we need to squash the ability of the guest to single-step itself as we have no easy way of re-entering the guest after the exception has been delivered to the hypervisor. A corner case I don't think this patch handles: if the debugger tries to single step an insn which is emulated by the hypervisor (because it's a load/store which is trapped and handled as emulated mmio in userspace) then we won't correctly update the single-step state machine (and so we'll end up incorrectly stopping after the following insn rather than before, I think). You should be able to achieve this effect by simply always clearing the guest's PSTATE.SS when you advance the PC to skip the emulated instruction (cf the comment in the pseudocode SSAdvance() function). I think we should also be doing this PC advance on return from userspace's handling of the mmio rather than before we drop back to userspace as we do now, but I can't remember why I think that. Christoffer, I don't suppose you recall, do you? I think it was you I had this conversation with on IRC a month or so back... I don't remember clearly, no. Was it not during lunch at LCU we had this conversation? In any case, I think it was related to how userspace observes the state of the CPU, because when you do the MMIO operation emulation in userspace, currently if you observe the PC though GET_ONE_REG, you'll see a PC pointing to the next instruction, not the one you're emulating which is strange. Not sure what the relation to a guest single-stepping itself was. -Christoffer -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/7] KVM: arm64: guest debug, add support for single-step
On 30 November 2014 at 10:10, Christoffer Dall christoffer.d...@linaro.org wrote: In any case, I think it was related to how userspace observes the state of the CPU, because when you do the MMIO operation emulation in userspace, currently if you observe the PC though GET_ONE_REG, you'll see a PC pointing to the next instruction, not the one you're emulating which is strange. Also if we ever add support for userspace to say this MMIO should fault then we definitely need the PC-advance to happen afterwards, not before. Not sure what the relation to a guest single-stepping itself was. I think it just came up in the course of that discussion, because single-step handling also needs to perform an action (clear PSTATE.SS) as part of the advance over this insn operation. But I think that you're right that doing the advance before dropping out to userspace is no worse for singlestep than it is for any other case. -- PMM -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/7] KVM: arm64: guest debug, add support for single-step
On Tue, Nov 25, 2014 at 04:10:03PM +, Alex Bennée wrote:
This adds support for single-stepping the guest. As userspace can and
will manipulate guest registers before restarting any tweaking of the
registers has to occur just before control is passed back to the guest.
Furthermore while guest debugging is in effect we need to squash the
ability of the guest to single-step itself as we have no easy way of
re-entering the guest after the exception has been delivered to the
hypervisor.
Admittedly this is a corner case, but wouldn't the only really nasty bit
of this be to emulate the guest debug exception?
Signed-off-by: Alex Bennée alex.ben...@linaro.org
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 48d26bb..a76daae 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -38,6 +38,7 @@
#include asm/tlbflush.h
#include asm/cacheflush.h
#include asm/virt.h
+#include asm/debug-monitors.h
#include asm/kvm_arm.h
#include asm/kvm_asm.h
#include asm/kvm_mmu.h
@@ -300,6 +301,17 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
kvm_arm_set_running_vcpu(NULL);
}
+/**
+ * kvm_arch_vcpu_ioctl_set_guest_debug - Setup guest debugging
+ * @kvm: pointer to the KVM struct
+ * @kvm_guest_debug: the ioctl data buffer
+ *
+ * This sets up the VM for guest debugging. Care has to be taken when
+ * manipulating guest registers as these will be set/cleared by the
+ * hyper-visor controller, typically before each kvm_run event. As a
hypervisor
+ * result modification of the guest registers needs to take place
+ * after they have been restored in the hyp.S trampoline code.
I don't understand this??
+ */
int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
struct kvm_guest_debug *dbg)
{
@@ -317,8 +329,8 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu
*vcpu,
/* Single Step */
if (vcpu-guest_debug KVM_GUESTDBG_SINGLESTEP) {
- kvm_info(SS requested, not yet implemented\n);
- return -EINVAL;
+ kvm_info(SS requested\n);
+ route_el2 = true;
}
/* Software Break Points */
diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index 8da1043..78e5ae1 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -121,6 +121,7 @@ int main(void)
DEFINE(VCPU_FAR_EL2, offsetof(struct kvm_vcpu,
arch.fault.far_el2));
DEFINE(VCPU_HPFAR_EL2, offsetof(struct kvm_vcpu,
arch.fault.hpfar_el2));
DEFINE(VCPU_DEBUG_FLAGS, offsetof(struct kvm_vcpu, arch.debug_flags));
+ DEFINE(GUEST_DEBUG,offsetof(struct kvm_vcpu, guest_debug));
DEFINE(VCPU_HCR_EL2, offsetof(struct kvm_vcpu,
arch.hcr_el2));
DEFINE(VCPU_MDCR_EL2, offsetof(struct kvm_vcpu, arch.mdcr_el2));
DEFINE(VCPU_IRQ_LINES, offsetof(struct kvm_vcpu, arch.irq_lines));
diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index 28dc92b..6def054 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -91,6 +91,25 @@ static int kvm_handle_bkpt(struct kvm_vcpu *vcpu, struct
kvm_run *run)
return 0;
}
+/**
+ * kvm_handle_ss - handle single step exceptions
+ *
+ * @vcpu:the vcpu pointer
+ *
+ * See: ARM ARM D2.12 for the details. While the host is routing debug
+ * exceptions to it's handlers we have to suppress the ability of the
its handlers
+ * guest to trigger exceptions.
not really sure why this comment is here? Does it really help anyone
reading this specific function or does it just confuse people more?
+ */
+static int kvm_handle_ss(struct kvm_vcpu *vcpu, struct kvm_run *run)
+{
+ WARN_ON(!(vcpu-guest_debug KVM_GUESTDBG_SINGLESTEP));
is this something that can actually happen or should it be a BUG_ON() -
which may even go away once you're doing hacking on this?
+
+ run-exit_reason = KVM_EXIT_DEBUG;
+ run-debug.arch.exit_type = KVM_DEBUG_EXIT_SINGLE_STEP;
+ run-debug.arch.address = *vcpu_pc(vcpu);
+ return 0;
+}
+
static exit_handle_fn arm_exit_handlers[] = {
[ESR_EL2_EC_WFI]= kvm_handle_wfx,
[ESR_EL2_EC_CP15_32]= kvm_handle_cp15_32,
@@ -105,6 +124,7 @@ static exit_handle_fn arm_exit_handlers[] = {
[ESR_EL2_EC_SYS64] = kvm_handle_sys_reg,
[ESR_EL2_EC_IABT] = kvm_handle_guest_abort,
[ESR_EL2_EC_DABT] = kvm_handle_guest_abort,
+ [ESR_EL2_EC_SOFTSTP]= kvm_handle_ss,
[ESR_EL2_EC_BKPT32] = kvm_handle_bkpt,
[ESR_EL2_EC_BRK64] = kvm_handle_bkpt,
};
diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
index 3c733ea..c0bc218 100644
--- a/arch/arm64/kvm/hyp.S
+++ b/arch/arm64/kvm/hyp.S
@@ -16,6 +16,7 @@
*/
#include linux/linkage.h
+#include linux/kvm.h
#include asm/assembler.h
#include asm/memory.h
@@ -168,6
Re: [PATCH 5/7] KVM: arm64: guest debug, add support for single-step
On 25 November 2014 at 16:10, Alex Bennée wrote: > This adds support for single-stepping the guest. As userspace can and > will manipulate guest registers before restarting any tweaking of the > registers has to occur just before control is passed back to the guest. > Furthermore while guest debugging is in effect we need to squash the > ability of the guest to single-step itself as we have no easy way of > re-entering the guest after the exception has been delivered to the > hypervisor. A corner case I don't think this patch handles: if the debugger tries to single step an insn which is emulated by the hypervisor (because it's a load/store which is trapped and handled as emulated mmio in userspace) then we won't correctly update the single-step state machine (and so we'll end up incorrectly stopping after the following insn rather than before, I think). You should be able to achieve this effect by simply always clearing the guest's PSTATE.SS when you advance the PC to skip the emulated instruction (cf the comment in the pseudocode SSAdvance() function). I think we should also be doing this PC advance on return from userspace's handling of the mmio rather than before we drop back to userspace as we do now, but I can't remember why I think that. Christoffer, I don't suppose you recall, do you? I think it was you I had this conversation with on IRC a month or so back... -- PMM -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/7] KVM: arm64: guest debug, add support for single-step
Andrew Jones writes:
> On Tue, Nov 25, 2014 at 04:10:03PM +, Alex Bennée wrote:
>> This adds support for single-stepping the guest. As userspace can and
>> will manipulate guest registers before restarting any tweaking of the
>> registers has to occur just before control is passed back to the guest.
>> Furthermore while guest debugging is in effect we need to squash the
>> ability of the guest to single-step itself as we have no easy way of
>> re-entering the guest after the exception has been delivered to the
>> hypervisor.
>>
>> Signed-off-by: Alex Bennée
>>
>> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
>> index 48d26bb..a76daae 100644
>> --- a/arch/arm/kvm/arm.c
>> +++ b/arch/arm/kvm/arm.c
>> @@ -38,6 +38,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>> #include
>> #include
>> @@ -300,6 +301,17 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
>> kvm_arm_set_running_vcpu(NULL);
>> }
>>
>> +/**
>> + * kvm_arch_vcpu_ioctl_set_guest_debug - Setup guest debugging
>> + * @kvm:pointer to the KVM struct
>> + * @kvm_guest_debug: the ioctl data buffer
>> + *
>> + * This sets up the VM for guest debugging. Care has to be taken when
>> + * manipulating guest registers as these will be set/cleared by the
>> + * hyper-visor controller, typically before each kvm_run event. As a
>> + * result modification of the guest registers needs to take place
>> + * after they have been restored in the hyp.S trampoline code.
>> + */
>> int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
>> struct kvm_guest_debug *dbg)
>> {
>> @@ -317,8 +329,8 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu
>> *vcpu,
>>
>> /* Single Step */
>> if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) {
>> -kvm_info("SS requested, not yet implemented\n");
>> -return -EINVAL;
>> +kvm_info("SS requested\n");
>> +route_el2 = true;
>> }
>>
>> /* Software Break Points */
>> diff --git a/arch/arm64/kernel/asm-offsets.c
>> b/arch/arm64/kernel/asm-offsets.c
>> index 8da1043..78e5ae1 100644
>> --- a/arch/arm64/kernel/asm-offsets.c
>> +++ b/arch/arm64/kernel/asm-offsets.c
>> @@ -121,6 +121,7 @@ int main(void)
>>DEFINE(VCPU_FAR_EL2, offsetof(struct kvm_vcpu,
>> arch.fault.far_el2));
>>DEFINE(VCPU_HPFAR_EL2,offsetof(struct kvm_vcpu,
>> arch.fault.hpfar_el2));
>>DEFINE(VCPU_DEBUG_FLAGS, offsetof(struct kvm_vcpu, arch.debug_flags));
>> + DEFINE(GUEST_DEBUG, offsetof(struct kvm_vcpu, guest_debug));
>>DEFINE(VCPU_HCR_EL2, offsetof(struct kvm_vcpu,
>> arch.hcr_el2));
>>DEFINE(VCPU_MDCR_EL2, offsetof(struct kvm_vcpu, arch.mdcr_el2));
>>DEFINE(VCPU_IRQ_LINES,offsetof(struct kvm_vcpu, arch.irq_lines));
>> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
>> index 28dc92b..6def054 100644
>> --- a/arch/arm64/kvm/handle_exit.c
>> +++ b/arch/arm64/kvm/handle_exit.c
>> @@ -91,6 +91,25 @@ static int kvm_handle_bkpt(struct kvm_vcpu *vcpu, struct
>> kvm_run *run)
>> return 0;
>> }
>>
>> +/**
>> + * kvm_handle_ss - handle single step exceptions
>> + *
>> + * @vcpu: the vcpu pointer
>
> same @run comment as other handler header in previous patch
Yeah I think I'll be merging them all together given the comments about
passing syndrome info directly.
>> + *
>> + * See: ARM ARM D2.12 for the details. While the host is routing debug
>> + * exceptions to it's handlers we have to suppress the ability of the
>> + * guest to trigger exceptions.
>> + */
>> +static int kvm_handle_ss(struct kvm_vcpu *vcpu, struct kvm_run *run)
>> +{
>> +WARN_ON(!(vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP));
>
> I'm not sure about this WARN_ON. Is there some scenario you were
> thinking of when you put it here? Is there some scenario where this
> could trigger so frequently we kill the log buffer?
The main one I had in mind was not suppressing the guest's attempt to
step while guest debugging was running.
>>
>> -/* for KVM_SET_GUEST_DEBUG */
>> -
>> -#define KVM_GUESTDBG_ENABLE 0x0001
>> -#define KVM_GUESTDBG_SINGLESTEP 0x0002
>> -
>> struct kvm_guest_debug {
>> __u32 control;
>> __u32 pad;
>> @@ -1189,4 +1186,15 @@ struct kvm_assigned_msix_entry {
>> __u16 padding[3];
>> };
>>
>> +#endif /* __ASSEMBLY__ */
>> +
>> +/* for KVM_SET_GUEST_DEBUG */
>> +
>> +#define KVM_GUESTDBG_ENABLE_SHIFT 0
>> +#define KVM_GUESTDBG_ENABLE (1 << KVM_GUESTDBG_ENABLE_SHIFT)
>> +#define KVM_GUESTDBG_SINGLESTEP_SHIFT 1
>> +#define KVM_GUESTDBG_SINGLESTEP (1 << KVM_GUESTDBG_SINGLESTEP_SHIFT)
>
> EALIGN: we can tab these defines up better
Sure, I'll clean those up.
--
Alex Bennée
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at
Re: [PATCH 5/7] KVM: arm64: guest debug, add support for single-step
On Tue, Nov 25, 2014 at 04:10:03PM +, Alex Bennée wrote:
> This adds support for single-stepping the guest. As userspace can and
> will manipulate guest registers before restarting any tweaking of the
> registers has to occur just before control is passed back to the guest.
> Furthermore while guest debugging is in effect we need to squash the
> ability of the guest to single-step itself as we have no easy way of
> re-entering the guest after the exception has been delivered to the
> hypervisor.
>
> Signed-off-by: Alex Bennée
>
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index 48d26bb..a76daae 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -38,6 +38,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -300,6 +301,17 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
> kvm_arm_set_running_vcpu(NULL);
> }
>
> +/**
> + * kvm_arch_vcpu_ioctl_set_guest_debug - Setup guest debugging
> + * @kvm: pointer to the KVM struct
> + * @kvm_guest_debug: the ioctl data buffer
> + *
> + * This sets up the VM for guest debugging. Care has to be taken when
> + * manipulating guest registers as these will be set/cleared by the
> + * hyper-visor controller, typically before each kvm_run event. As a
> + * result modification of the guest registers needs to take place
> + * after they have been restored in the hyp.S trampoline code.
> + */
> int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
> struct kvm_guest_debug *dbg)
> {
> @@ -317,8 +329,8 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu
> *vcpu,
>
> /* Single Step */
> if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) {
> - kvm_info("SS requested, not yet implemented\n");
> - return -EINVAL;
> + kvm_info("SS requested\n");
> + route_el2 = true;
> }
>
> /* Software Break Points */
> diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
> index 8da1043..78e5ae1 100644
> --- a/arch/arm64/kernel/asm-offsets.c
> +++ b/arch/arm64/kernel/asm-offsets.c
> @@ -121,6 +121,7 @@ int main(void)
>DEFINE(VCPU_FAR_EL2, offsetof(struct kvm_vcpu,
> arch.fault.far_el2));
>DEFINE(VCPU_HPFAR_EL2, offsetof(struct kvm_vcpu,
> arch.fault.hpfar_el2));
>DEFINE(VCPU_DEBUG_FLAGS, offsetof(struct kvm_vcpu, arch.debug_flags));
> + DEFINE(GUEST_DEBUG,offsetof(struct kvm_vcpu, guest_debug));
>DEFINE(VCPU_HCR_EL2, offsetof(struct kvm_vcpu,
> arch.hcr_el2));
>DEFINE(VCPU_MDCR_EL2, offsetof(struct kvm_vcpu, arch.mdcr_el2));
>DEFINE(VCPU_IRQ_LINES, offsetof(struct kvm_vcpu, arch.irq_lines));
> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
> index 28dc92b..6def054 100644
> --- a/arch/arm64/kvm/handle_exit.c
> +++ b/arch/arm64/kvm/handle_exit.c
> @@ -91,6 +91,25 @@ static int kvm_handle_bkpt(struct kvm_vcpu *vcpu, struct
> kvm_run *run)
> return 0;
> }
>
> +/**
> + * kvm_handle_ss - handle single step exceptions
> + *
> + * @vcpu:the vcpu pointer
same @run comment as other handler header in previous patch
> + *
> + * See: ARM ARM D2.12 for the details. While the host is routing debug
> + * exceptions to it's handlers we have to suppress the ability of the
> + * guest to trigger exceptions.
> + */
> +static int kvm_handle_ss(struct kvm_vcpu *vcpu, struct kvm_run *run)
> +{
> + WARN_ON(!(vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP));
I'm not sure about this WARN_ON. Is there some scenario you were
thinking of when you put it here? Is there some scenario where this
could trigger so frequently we kill the log buffer?
> +
> + run->exit_reason = KVM_EXIT_DEBUG;
> + run->debug.arch.exit_type = KVM_DEBUG_EXIT_SINGLE_STEP;
> + run->debug.arch.address = *vcpu_pc(vcpu);
> + return 0;
> +}
> +
> static exit_handle_fn arm_exit_handlers[] = {
> [ESR_EL2_EC_WFI]= kvm_handle_wfx,
> [ESR_EL2_EC_CP15_32]= kvm_handle_cp15_32,
> @@ -105,6 +124,7 @@ static exit_handle_fn arm_exit_handlers[] = {
> [ESR_EL2_EC_SYS64] = kvm_handle_sys_reg,
> [ESR_EL2_EC_IABT] = kvm_handle_guest_abort,
> [ESR_EL2_EC_DABT] = kvm_handle_guest_abort,
> + [ESR_EL2_EC_SOFTSTP]= kvm_handle_ss,
> [ESR_EL2_EC_BKPT32] = kvm_handle_bkpt,
> [ESR_EL2_EC_BRK64] = kvm_handle_bkpt,
> };
> diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
> index 3c733ea..c0bc218 100644
> --- a/arch/arm64/kvm/hyp.S
> +++ b/arch/arm64/kvm/hyp.S
> @@ -16,6 +16,7 @@
> */
>
> #include
> +#include
>
> #include
> #include
> @@ -168,6 +169,31 @@
> // x19-x29, lr, sp*, elr*, spsr*
> restore_common_regs
>
> + // After restoring the guest registers but before we return to the guest
> + // we may want to make some final tweaks to support guest debugging.
> +
Re: [PATCH 5/7] KVM: arm64: guest debug, add support for single-step
On Tue, Nov 25, 2014 at 04:10:03PM +, Alex Bennée wrote:
This adds support for single-stepping the guest. As userspace can and
will manipulate guest registers before restarting any tweaking of the
registers has to occur just before control is passed back to the guest.
Furthermore while guest debugging is in effect we need to squash the
ability of the guest to single-step itself as we have no easy way of
re-entering the guest after the exception has been delivered to the
hypervisor.
Signed-off-by: Alex Bennée alex.ben...@linaro.org
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 48d26bb..a76daae 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -38,6 +38,7 @@
#include asm/tlbflush.h
#include asm/cacheflush.h
#include asm/virt.h
+#include asm/debug-monitors.h
#include asm/kvm_arm.h
#include asm/kvm_asm.h
#include asm/kvm_mmu.h
@@ -300,6 +301,17 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
kvm_arm_set_running_vcpu(NULL);
}
+/**
+ * kvm_arch_vcpu_ioctl_set_guest_debug - Setup guest debugging
+ * @kvm: pointer to the KVM struct
+ * @kvm_guest_debug: the ioctl data buffer
+ *
+ * This sets up the VM for guest debugging. Care has to be taken when
+ * manipulating guest registers as these will be set/cleared by the
+ * hyper-visor controller, typically before each kvm_run event. As a
+ * result modification of the guest registers needs to take place
+ * after they have been restored in the hyp.S trampoline code.
+ */
int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
struct kvm_guest_debug *dbg)
{
@@ -317,8 +329,8 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu
*vcpu,
/* Single Step */
if (vcpu-guest_debug KVM_GUESTDBG_SINGLESTEP) {
- kvm_info(SS requested, not yet implemented\n);
- return -EINVAL;
+ kvm_info(SS requested\n);
+ route_el2 = true;
}
/* Software Break Points */
diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index 8da1043..78e5ae1 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -121,6 +121,7 @@ int main(void)
DEFINE(VCPU_FAR_EL2, offsetof(struct kvm_vcpu,
arch.fault.far_el2));
DEFINE(VCPU_HPFAR_EL2, offsetof(struct kvm_vcpu,
arch.fault.hpfar_el2));
DEFINE(VCPU_DEBUG_FLAGS, offsetof(struct kvm_vcpu, arch.debug_flags));
+ DEFINE(GUEST_DEBUG,offsetof(struct kvm_vcpu, guest_debug));
DEFINE(VCPU_HCR_EL2, offsetof(struct kvm_vcpu,
arch.hcr_el2));
DEFINE(VCPU_MDCR_EL2, offsetof(struct kvm_vcpu, arch.mdcr_el2));
DEFINE(VCPU_IRQ_LINES, offsetof(struct kvm_vcpu, arch.irq_lines));
diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index 28dc92b..6def054 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -91,6 +91,25 @@ static int kvm_handle_bkpt(struct kvm_vcpu *vcpu, struct
kvm_run *run)
return 0;
}
+/**
+ * kvm_handle_ss - handle single step exceptions
+ *
+ * @vcpu:the vcpu pointer
same @run comment as other handler header in previous patch
+ *
+ * See: ARM ARM D2.12 for the details. While the host is routing debug
+ * exceptions to it's handlers we have to suppress the ability of the
+ * guest to trigger exceptions.
+ */
+static int kvm_handle_ss(struct kvm_vcpu *vcpu, struct kvm_run *run)
+{
+ WARN_ON(!(vcpu-guest_debug KVM_GUESTDBG_SINGLESTEP));
I'm not sure about this WARN_ON. Is there some scenario you were
thinking of when you put it here? Is there some scenario where this
could trigger so frequently we kill the log buffer?
+
+ run-exit_reason = KVM_EXIT_DEBUG;
+ run-debug.arch.exit_type = KVM_DEBUG_EXIT_SINGLE_STEP;
+ run-debug.arch.address = *vcpu_pc(vcpu);
+ return 0;
+}
+
static exit_handle_fn arm_exit_handlers[] = {
[ESR_EL2_EC_WFI]= kvm_handle_wfx,
[ESR_EL2_EC_CP15_32]= kvm_handle_cp15_32,
@@ -105,6 +124,7 @@ static exit_handle_fn arm_exit_handlers[] = {
[ESR_EL2_EC_SYS64] = kvm_handle_sys_reg,
[ESR_EL2_EC_IABT] = kvm_handle_guest_abort,
[ESR_EL2_EC_DABT] = kvm_handle_guest_abort,
+ [ESR_EL2_EC_SOFTSTP]= kvm_handle_ss,
[ESR_EL2_EC_BKPT32] = kvm_handle_bkpt,
[ESR_EL2_EC_BRK64] = kvm_handle_bkpt,
};
diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
index 3c733ea..c0bc218 100644
--- a/arch/arm64/kvm/hyp.S
+++ b/arch/arm64/kvm/hyp.S
@@ -16,6 +16,7 @@
*/
#include linux/linkage.h
+#include linux/kvm.h
#include asm/assembler.h
#include asm/memory.h
@@ -168,6 +169,31 @@
// x19-x29, lr, sp*, elr*, spsr*
restore_common_regs
+ // After restoring the guest registers but before we return to the guest
+ // we may want to make some final
Re: [PATCH 5/7] KVM: arm64: guest debug, add support for single-step
Andrew Jones drjo...@redhat.com writes:
On Tue, Nov 25, 2014 at 04:10:03PM +, Alex Bennée wrote:
This adds support for single-stepping the guest. As userspace can and
will manipulate guest registers before restarting any tweaking of the
registers has to occur just before control is passed back to the guest.
Furthermore while guest debugging is in effect we need to squash the
ability of the guest to single-step itself as we have no easy way of
re-entering the guest after the exception has been delivered to the
hypervisor.
Signed-off-by: Alex Bennée alex.ben...@linaro.org
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 48d26bb..a76daae 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -38,6 +38,7 @@
#include asm/tlbflush.h
#include asm/cacheflush.h
#include asm/virt.h
+#include asm/debug-monitors.h
#include asm/kvm_arm.h
#include asm/kvm_asm.h
#include asm/kvm_mmu.h
@@ -300,6 +301,17 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
kvm_arm_set_running_vcpu(NULL);
}
+/**
+ * kvm_arch_vcpu_ioctl_set_guest_debug - Setup guest debugging
+ * @kvm:pointer to the KVM struct
+ * @kvm_guest_debug: the ioctl data buffer
+ *
+ * This sets up the VM for guest debugging. Care has to be taken when
+ * manipulating guest registers as these will be set/cleared by the
+ * hyper-visor controller, typically before each kvm_run event. As a
+ * result modification of the guest registers needs to take place
+ * after they have been restored in the hyp.S trampoline code.
+ */
int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
struct kvm_guest_debug *dbg)
{
@@ -317,8 +329,8 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu
*vcpu,
/* Single Step */
if (vcpu-guest_debug KVM_GUESTDBG_SINGLESTEP) {
-kvm_info(SS requested, not yet implemented\n);
-return -EINVAL;
+kvm_info(SS requested\n);
+route_el2 = true;
}
/* Software Break Points */
diff --git a/arch/arm64/kernel/asm-offsets.c
b/arch/arm64/kernel/asm-offsets.c
index 8da1043..78e5ae1 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -121,6 +121,7 @@ int main(void)
DEFINE(VCPU_FAR_EL2, offsetof(struct kvm_vcpu,
arch.fault.far_el2));
DEFINE(VCPU_HPFAR_EL2,offsetof(struct kvm_vcpu,
arch.fault.hpfar_el2));
DEFINE(VCPU_DEBUG_FLAGS, offsetof(struct kvm_vcpu, arch.debug_flags));
+ DEFINE(GUEST_DEBUG, offsetof(struct kvm_vcpu, guest_debug));
DEFINE(VCPU_HCR_EL2, offsetof(struct kvm_vcpu,
arch.hcr_el2));
DEFINE(VCPU_MDCR_EL2, offsetof(struct kvm_vcpu, arch.mdcr_el2));
DEFINE(VCPU_IRQ_LINES,offsetof(struct kvm_vcpu, arch.irq_lines));
diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index 28dc92b..6def054 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -91,6 +91,25 @@ static int kvm_handle_bkpt(struct kvm_vcpu *vcpu, struct
kvm_run *run)
return 0;
}
+/**
+ * kvm_handle_ss - handle single step exceptions
+ *
+ * @vcpu: the vcpu pointer
same @run comment as other handler header in previous patch
Yeah I think I'll be merging them all together given the comments about
passing syndrome info directly.
+ *
+ * See: ARM ARM D2.12 for the details. While the host is routing debug
+ * exceptions to it's handlers we have to suppress the ability of the
+ * guest to trigger exceptions.
+ */
+static int kvm_handle_ss(struct kvm_vcpu *vcpu, struct kvm_run *run)
+{
+WARN_ON(!(vcpu-guest_debug KVM_GUESTDBG_SINGLESTEP));
I'm not sure about this WARN_ON. Is there some scenario you were
thinking of when you put it here? Is there some scenario where this
could trigger so frequently we kill the log buffer?
The main one I had in mind was not suppressing the guest's attempt to
step while guest debugging was running.
snip
-/* for KVM_SET_GUEST_DEBUG */
-
-#define KVM_GUESTDBG_ENABLE 0x0001
-#define KVM_GUESTDBG_SINGLESTEP 0x0002
-
struct kvm_guest_debug {
__u32 control;
__u32 pad;
@@ -1189,4 +1186,15 @@ struct kvm_assigned_msix_entry {
__u16 padding[3];
};
+#endif /* __ASSEMBLY__ */
+
+/* for KVM_SET_GUEST_DEBUG */
+
+#define KVM_GUESTDBG_ENABLE_SHIFT 0
+#define KVM_GUESTDBG_ENABLE (1 KVM_GUESTDBG_ENABLE_SHIFT)
+#define KVM_GUESTDBG_SINGLESTEP_SHIFT 1
+#define KVM_GUESTDBG_SINGLESTEP (1 KVM_GUESTDBG_SINGLESTEP_SHIFT)
EALIGN: we can tab these defines up better
Sure, I'll clean those up.
--
Alex Bennée
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/7] KVM: arm64: guest debug, add support for single-step
On 25 November 2014 at 16:10, Alex Bennée alex.ben...@linaro.org wrote: This adds support for single-stepping the guest. As userspace can and will manipulate guest registers before restarting any tweaking of the registers has to occur just before control is passed back to the guest. Furthermore while guest debugging is in effect we need to squash the ability of the guest to single-step itself as we have no easy way of re-entering the guest after the exception has been delivered to the hypervisor. A corner case I don't think this patch handles: if the debugger tries to single step an insn which is emulated by the hypervisor (because it's a load/store which is trapped and handled as emulated mmio in userspace) then we won't correctly update the single-step state machine (and so we'll end up incorrectly stopping after the following insn rather than before, I think). You should be able to achieve this effect by simply always clearing the guest's PSTATE.SS when you advance the PC to skip the emulated instruction (cf the comment in the pseudocode SSAdvance() function). I think we should also be doing this PC advance on return from userspace's handling of the mmio rather than before we drop back to userspace as we do now, but I can't remember why I think that. Christoffer, I don't suppose you recall, do you? I think it was you I had this conversation with on IRC a month or so back... -- PMM -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 5/7] KVM: arm64: guest debug, add support for single-step
This adds support for single-stepping the guest. As userspace can and
will manipulate guest registers before restarting any tweaking of the
registers has to occur just before control is passed back to the guest.
Furthermore while guest debugging is in effect we need to squash the
ability of the guest to single-step itself as we have no easy way of
re-entering the guest after the exception has been delivered to the
hypervisor.
Signed-off-by: Alex Bennée
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 48d26bb..a76daae 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -38,6 +38,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -300,6 +301,17 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
kvm_arm_set_running_vcpu(NULL);
}
+/**
+ * kvm_arch_vcpu_ioctl_set_guest_debug - Setup guest debugging
+ * @kvm: pointer to the KVM struct
+ * @kvm_guest_debug: the ioctl data buffer
+ *
+ * This sets up the VM for guest debugging. Care has to be taken when
+ * manipulating guest registers as these will be set/cleared by the
+ * hyper-visor controller, typically before each kvm_run event. As a
+ * result modification of the guest registers needs to take place
+ * after they have been restored in the hyp.S trampoline code.
+ */
int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
struct kvm_guest_debug *dbg)
{
@@ -317,8 +329,8 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu
*vcpu,
/* Single Step */
if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) {
- kvm_info("SS requested, not yet implemented\n");
- return -EINVAL;
+ kvm_info("SS requested\n");
+ route_el2 = true;
}
/* Software Break Points */
diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index 8da1043..78e5ae1 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -121,6 +121,7 @@ int main(void)
DEFINE(VCPU_FAR_EL2, offsetof(struct kvm_vcpu, arch.fault.far_el2));
DEFINE(VCPU_HPFAR_EL2, offsetof(struct kvm_vcpu,
arch.fault.hpfar_el2));
DEFINE(VCPU_DEBUG_FLAGS, offsetof(struct kvm_vcpu, arch.debug_flags));
+ DEFINE(GUEST_DEBUG, offsetof(struct kvm_vcpu, guest_debug));
DEFINE(VCPU_HCR_EL2, offsetof(struct kvm_vcpu, arch.hcr_el2));
DEFINE(VCPU_MDCR_EL2,offsetof(struct kvm_vcpu, arch.mdcr_el2));
DEFINE(VCPU_IRQ_LINES, offsetof(struct kvm_vcpu, arch.irq_lines));
diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index 28dc92b..6def054 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -91,6 +91,25 @@ static int kvm_handle_bkpt(struct kvm_vcpu *vcpu, struct
kvm_run *run)
return 0;
}
+/**
+ * kvm_handle_ss - handle single step exceptions
+ *
+ * @vcpu: the vcpu pointer
+ *
+ * See: ARM ARM D2.12 for the details. While the host is routing debug
+ * exceptions to it's handlers we have to suppress the ability of the
+ * guest to trigger exceptions.
+ */
+static int kvm_handle_ss(struct kvm_vcpu *vcpu, struct kvm_run *run)
+{
+ WARN_ON(!(vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP));
+
+ run->exit_reason = KVM_EXIT_DEBUG;
+ run->debug.arch.exit_type = KVM_DEBUG_EXIT_SINGLE_STEP;
+ run->debug.arch.address = *vcpu_pc(vcpu);
+ return 0;
+}
+
static exit_handle_fn arm_exit_handlers[] = {
[ESR_EL2_EC_WFI]= kvm_handle_wfx,
[ESR_EL2_EC_CP15_32]= kvm_handle_cp15_32,
@@ -105,6 +124,7 @@ static exit_handle_fn arm_exit_handlers[] = {
[ESR_EL2_EC_SYS64] = kvm_handle_sys_reg,
[ESR_EL2_EC_IABT] = kvm_handle_guest_abort,
[ESR_EL2_EC_DABT] = kvm_handle_guest_abort,
+ [ESR_EL2_EC_SOFTSTP]= kvm_handle_ss,
[ESR_EL2_EC_BKPT32] = kvm_handle_bkpt,
[ESR_EL2_EC_BRK64] = kvm_handle_bkpt,
};
diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
index 3c733ea..c0bc218 100644
--- a/arch/arm64/kvm/hyp.S
+++ b/arch/arm64/kvm/hyp.S
@@ -16,6 +16,7 @@
*/
#include
+#include
#include
#include
@@ -168,6 +169,31 @@
// x19-x29, lr, sp*, elr*, spsr*
restore_common_regs
+ // After restoring the guest registers but before we return to the guest
+ // we may want to make some final tweaks to support guest debugging.
+ ldr x3, [x0, #GUEST_DEBUG]
+ tbz x3, #KVM_GUESTDBG_ENABLE_SHIFT, 2f // No guest debug
+
+ // x0 - preserved as VCPU ptr
+ // x1 - spsr
+ // x2 - mdscr
+ mrs x1, spsr_el2
+ mrs x2, mdscr_el1
+
+ // See ARM ARM D2.12.3 The software step state machine
+ // If we are doing Single Step - set MDSCR_EL1.SS and PSTATE.SS
+ orr x1, x1, #DBG_SPSR_SS
+ orr x2, x2, #DBG_MDSCR_SS
+ tbnzx3,
[PATCH 5/7] KVM: arm64: guest debug, add support for single-step
This adds support for single-stepping the guest. As userspace can and
will manipulate guest registers before restarting any tweaking of the
registers has to occur just before control is passed back to the guest.
Furthermore while guest debugging is in effect we need to squash the
ability of the guest to single-step itself as we have no easy way of
re-entering the guest after the exception has been delivered to the
hypervisor.
Signed-off-by: Alex Bennée alex.ben...@linaro.org
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 48d26bb..a76daae 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -38,6 +38,7 @@
#include asm/tlbflush.h
#include asm/cacheflush.h
#include asm/virt.h
+#include asm/debug-monitors.h
#include asm/kvm_arm.h
#include asm/kvm_asm.h
#include asm/kvm_mmu.h
@@ -300,6 +301,17 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
kvm_arm_set_running_vcpu(NULL);
}
+/**
+ * kvm_arch_vcpu_ioctl_set_guest_debug - Setup guest debugging
+ * @kvm: pointer to the KVM struct
+ * @kvm_guest_debug: the ioctl data buffer
+ *
+ * This sets up the VM for guest debugging. Care has to be taken when
+ * manipulating guest registers as these will be set/cleared by the
+ * hyper-visor controller, typically before each kvm_run event. As a
+ * result modification of the guest registers needs to take place
+ * after they have been restored in the hyp.S trampoline code.
+ */
int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
struct kvm_guest_debug *dbg)
{
@@ -317,8 +329,8 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu
*vcpu,
/* Single Step */
if (vcpu-guest_debug KVM_GUESTDBG_SINGLESTEP) {
- kvm_info(SS requested, not yet implemented\n);
- return -EINVAL;
+ kvm_info(SS requested\n);
+ route_el2 = true;
}
/* Software Break Points */
diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index 8da1043..78e5ae1 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -121,6 +121,7 @@ int main(void)
DEFINE(VCPU_FAR_EL2, offsetof(struct kvm_vcpu, arch.fault.far_el2));
DEFINE(VCPU_HPFAR_EL2, offsetof(struct kvm_vcpu,
arch.fault.hpfar_el2));
DEFINE(VCPU_DEBUG_FLAGS, offsetof(struct kvm_vcpu, arch.debug_flags));
+ DEFINE(GUEST_DEBUG, offsetof(struct kvm_vcpu, guest_debug));
DEFINE(VCPU_HCR_EL2, offsetof(struct kvm_vcpu, arch.hcr_el2));
DEFINE(VCPU_MDCR_EL2,offsetof(struct kvm_vcpu, arch.mdcr_el2));
DEFINE(VCPU_IRQ_LINES, offsetof(struct kvm_vcpu, arch.irq_lines));
diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index 28dc92b..6def054 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -91,6 +91,25 @@ static int kvm_handle_bkpt(struct kvm_vcpu *vcpu, struct
kvm_run *run)
return 0;
}
+/**
+ * kvm_handle_ss - handle single step exceptions
+ *
+ * @vcpu: the vcpu pointer
+ *
+ * See: ARM ARM D2.12 for the details. While the host is routing debug
+ * exceptions to it's handlers we have to suppress the ability of the
+ * guest to trigger exceptions.
+ */
+static int kvm_handle_ss(struct kvm_vcpu *vcpu, struct kvm_run *run)
+{
+ WARN_ON(!(vcpu-guest_debug KVM_GUESTDBG_SINGLESTEP));
+
+ run-exit_reason = KVM_EXIT_DEBUG;
+ run-debug.arch.exit_type = KVM_DEBUG_EXIT_SINGLE_STEP;
+ run-debug.arch.address = *vcpu_pc(vcpu);
+ return 0;
+}
+
static exit_handle_fn arm_exit_handlers[] = {
[ESR_EL2_EC_WFI]= kvm_handle_wfx,
[ESR_EL2_EC_CP15_32]= kvm_handle_cp15_32,
@@ -105,6 +124,7 @@ static exit_handle_fn arm_exit_handlers[] = {
[ESR_EL2_EC_SYS64] = kvm_handle_sys_reg,
[ESR_EL2_EC_IABT] = kvm_handle_guest_abort,
[ESR_EL2_EC_DABT] = kvm_handle_guest_abort,
+ [ESR_EL2_EC_SOFTSTP]= kvm_handle_ss,
[ESR_EL2_EC_BKPT32] = kvm_handle_bkpt,
[ESR_EL2_EC_BRK64] = kvm_handle_bkpt,
};
diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
index 3c733ea..c0bc218 100644
--- a/arch/arm64/kvm/hyp.S
+++ b/arch/arm64/kvm/hyp.S
@@ -16,6 +16,7 @@
*/
#include linux/linkage.h
+#include linux/kvm.h
#include asm/assembler.h
#include asm/memory.h
@@ -168,6 +169,31 @@
// x19-x29, lr, sp*, elr*, spsr*
restore_common_regs
+ // After restoring the guest registers but before we return to the guest
+ // we may want to make some final tweaks to support guest debugging.
+ ldr x3, [x0, #GUEST_DEBUG]
+ tbz x3, #KVM_GUESTDBG_ENABLE_SHIFT, 2f // No guest debug
+
+ // x0 - preserved as VCPU ptr
+ // x1 - spsr
+ // x2 - mdscr
+ mrs x1, spsr_el2
+ mrs x2, mdscr_el1
+
+ // See ARM ARM D2.12.3 The software step state machine
+ // If we are doing Single
