From: alex.bennee@linaro•org (Alex Bennée)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH 5/7] KVM: arm64: guest debug, add support for single-step
Date: Wed, 26 Nov 2014 18:00:45 +0000 [thread overview]
Message-ID: <87lhmxlu42.fsf@linaro.org> (raw)
In-Reply-To: <20141126164057.GE3245@hawk.usersys.redhat.com>
Andrew Jones <drjones@redhat•com> writes:
> On Tue, Nov 25, 2014 at 04:10:03PM +0000, 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.bennee@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 0x00000001
>> -#define KVM_GUESTDBG_SINGLESTEP 0x00000002
>> -
>> 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
next prev parent reply other threads:[~2014-11-26 18:00 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-25 16:09 [PATCH 0/7] KVM Guest Debug support for arm64 Alex Bennée
2014-11-25 16:09 ` [PATCH 1/7] KVM: add commentary for kvm_debug_exit_arch struct Alex Bennée
2014-11-26 14:20 ` Andrew Jones
2014-11-25 16:10 ` [PATCH 2/7] KVM: arm: guest debug, define API headers Alex Bennée
2014-11-25 16:19 ` Peter Maydell
2014-11-26 15:04 ` Alex Bennée
2014-11-29 16:20 ` Christoffer Dall
2014-12-01 11:30 ` Alex Bennée
2014-11-25 17:05 ` Paolo Bonzini
2014-11-25 17:13 ` Peter Maydell
2014-11-25 17:22 ` Paolo Bonzini
2014-11-26 13:13 ` Alex Bennée
2014-11-26 13:14 ` Paolo Bonzini
2014-11-26 14:31 ` Andrew Jones
2014-11-26 14:58 ` Alex Bennée
2014-11-26 16:46 ` Paolo Bonzini
2014-11-26 17:47 ` Andrew Jones
2014-11-29 16:20 ` Christoffer Dall
2014-11-25 16:10 ` [PATCH 3/7] KVM: arm: guest debug, add stub KVM_SET_GUEST_DEBUG ioctl Alex Bennée
2014-11-26 14:38 ` Andrew Jones
2014-11-26 15:03 ` Alex Bennée
2014-11-26 16:46 ` Paolo Bonzini
2014-11-29 16:21 ` Christoffer Dall
2014-11-25 16:10 ` [PATCH 4/7] KVM: arm64: guest debug, add SW break point support Alex Bennée
2014-11-26 16:07 ` Andrew Jones
2014-11-26 17:14 ` Peter Maydell
2014-11-29 16:21 ` Christoffer Dall
2014-11-29 16:21 ` Christoffer Dall
2014-12-01 11:33 ` Alex Bennée
2014-11-25 16:10 ` [PATCH 5/7] KVM: arm64: guest debug, add support for single-step Alex Bennée
2014-11-26 16:40 ` Andrew Jones
2014-11-26 18:00 ` Alex Bennée [this message]
2014-11-26 19:27 ` Peter Maydell
2014-11-30 10:10 ` Christoffer Dall
2014-11-30 10:20 ` Peter Maydell
2014-11-30 10:21 ` Christoffer Dall
2014-12-01 11:50 ` Alex Bennée
2014-12-02 13:17 ` Christoffer Dall
2014-11-25 16:10 ` [PATCH 6/7] KVM: arm64: re-factor hyp.S debug register code Alex Bennée
2014-11-26 16:49 ` Andrew Jones
2014-11-30 10:25 ` Christoffer Dall
2014-12-01 11:52 ` Alex Bennée
2014-12-02 13:23 ` Christoffer Dall
2014-11-25 16:10 ` [PATCH 7/7] KVM: arm64: guest debug, HW assisted debug support Alex Bennée
2014-11-26 17:34 ` Andrew Jones
2014-11-30 10:34 ` Christoffer Dall
2014-12-01 11:54 ` Alex Bennée
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87lhmxlu42.fsf@linaro.org \
--to=alex.bennee@linaro$(echo .)org \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox