From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex =?utf-8?Q?Benn=C3=A9e?= Subject: Re: [PATCH v9 3/6] target-arm: kvm - support for single step Date: Tue, 08 Dec 2015 11:49:49 +0000 Message-ID: <87r3ixp3c2.fsf@linaro.org> References: <1447345251-22625-1-git-send-email-alex.bennee@linaro.org> <1447345251-22625-4-git-send-email-alex.bennee@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 225E04113E for ; Tue, 8 Dec 2015 06:48:03 -0500 (EST) Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 3pFbincrENq5 for ; Tue, 8 Dec 2015 06:48:02 -0500 (EST) Received: from mail-wm0-f50.google.com (mail-wm0-f50.google.com [74.125.82.50]) by mm01.cs.columbia.edu (Postfix) with ESMTPS id 3B88240B40 for ; Tue, 8 Dec 2015 06:48:02 -0500 (EST) Received: by wmww144 with SMTP id w144so177772634wmw.1 for ; Tue, 08 Dec 2015 03:49:51 -0800 (PST) In-reply-to: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kvmarm-bounces@lists.cs.columbia.edu Sender: kvmarm-bounces@lists.cs.columbia.edu To: Peter Maydell Cc: kvm-devel , Marc Zyngier , QEMU Developers , qemu-arm@nongnu.org, Zhichao Huang , Paolo Bonzini , "kvmarm@lists.cs.columbia.edu" , arm-mail-list List-Id: kvmarm@lists.cs.columbia.edu ClBldGVyIE1heWRlbGwgPHBldGVyLm1heWRlbGxAbGluYXJvLm9yZz4gd3JpdGVzOgoKPiBPbiAx MiBOb3ZlbWJlciAyMDE1IGF0IDE2OjIwLCBBbGV4IEJlbm7DqWUgPGFsZXguYmVubmVlQGxpbmFy by5vcmc+IHdyb3RlOgo+PiBUaGlzIGFkZHMgc3VwcG9ydCBmb3Igc2luZ2xlLXN0ZXAuIFRoZXJl IGlzbid0IG11Y2ggdG8gZG8gb24gdGhlIFFFTVUKPj4gc2lkZSBhcyBhZnRlciB3ZSBzZXQtdXAg dGhlIHJlcXVlc3QgZm9yIHNpbmdsZSBzdGVwIHZpYSB0aGUgZGVidWcgaW9jdGwKPj4gaXQgaXMg YWxsIGhhbmRsZWQgd2l0aGluIHRoZSBrZXJuZWwuCj4+Cj4+IFNpZ25lZC1vZmYtYnk6IEFsZXgg QmVubsOpZSA8YWxleC5iZW5uZWVAbGluYXJvLm9yZz4KPj4KPj4gLS0tCj4+IHYyCj4+ICAgLSBj b252ZXJ0IHRvIHVzaW5nIEhTUl9FQwo+PiB2Mwo+PiAgIC0gdXNlIGludGVybmFscy5oIGRlZmlu aXRpb25zCj4+IC0tLQo+PiAgdGFyZ2V0LWFybS9rdm0uYyB8IDEwICsrKysrKysrKysKPj4gIDEg ZmlsZSBjaGFuZ2VkLCAxMCBpbnNlcnRpb25zKCspCj4+Cj4+IGRpZmYgLS1naXQgYS90YXJnZXQt YXJtL2t2bS5jIGIvdGFyZ2V0LWFybS9rdm0uYwo+PiBpbmRleCA1MGY3MGVmLi5kNTA1YTdlIDEw MDY0NAo+PiAtLS0gYS90YXJnZXQtYXJtL2t2bS5jCj4+ICsrKyBiL3RhcmdldC1hcm0va3ZtLmMK Pj4gQEAgLTUzNSw2ICs1MzUsMTMgQEAgc3RhdGljIGludCBrdm1faGFuZGxlX2RlYnVnKENQVVN0 YXRlICpjcywgc3RydWN0IGt2bV9ydW4gKnJ1bikKPj4gICAgICBrdm1fY3B1X3N5bmNocm9uaXpl X3N0YXRlKGNzKTsKPj4KPj4gICAgICBzd2l0Y2ggKGhzcl9lYykgewo+PiArICAgIGNhc2UgRUNf U09GVFdBUkVTVEVQOgo+PiArICAgICAgICBpZiAoY3MtPnNpbmdsZXN0ZXBfZW5hYmxlZCkgewo+ PiArICAgICAgICAgICAgcmV0dXJuIHRydWU7Cj4+ICsgICAgICAgIH0gZWxzZSB7Cj4+ICsgICAg ICAgICAgICBlcnJvcl9yZXBvcnQoIkNhbWUgb3V0IG9mIFNJTkdMRSBTVEVQIHdoZW4gbm90IGVu YWJsZWQiKTsKPj4gKyAgICAgICAgfQo+PiArICAgICAgICBicmVhazsKPj4gICAgICBjYXNlIEVD X0FBNjRfQktQVDoKPj4gICAgICAgICAgaWYgKGt2bV9maW5kX3N3X2JyZWFrcG9pbnQoY3MsIGVu di0+cGMpKSB7Cj4+ICAgICAgICAgICAgICByZXR1cm4gdHJ1ZTsKPj4gQEAgLTU5NSw2ICs2MDIs OSBAQCBpbnQga3ZtX2FyY2hfb25fc2lnYnVzKGludCBjb2RlLCB2b2lkICphZGRyKQo+Pgo+PiAg dm9pZCBrdm1fYXJjaF91cGRhdGVfZ3Vlc3RfZGVidWcoQ1BVU3RhdGUgKmNzLCBzdHJ1Y3Qga3Zt X2d1ZXN0X2RlYnVnICpkYmcpCj4+ICB7Cj4+ICsgICAgaWYgKGNzLT5zaW5nbGVzdGVwX2VuYWJs ZWQpIHsKPj4gKyAgICAgICAgZGJnLT5jb250cm9sIHw9IEtWTV9HVUVTVERCR19FTkFCTEUgfCBL Vk1fR1VFU1REQkdfU0lOR0xFU1RFUDsKPj4gKyAgICB9Cj4KPiBEb2Vzbid0IGt2bV91cGRhdGVf Z3Vlc3RfZGVidWcoKSBhbHJlYWR5IHNldCB0aGVzZSBiaXRzLCBvciBhbQo+IEkgbWlzcmVhZGlu ZyBpdD8KClllYWguIFRoaXMgcmFpc2VzIGFuIGludGVyZXN0aW5nIHByb2JsZW0gYWJvdXQgd2hh dCB0byBkbyB3aGVuIHdlIGRvbid0CmhhdmUgdGhlIGNhcGFiaWxpdHkuIEkgY291bGQgc3VwcHJl c3MgdGhvc2UgYml0cyBpbiB0aGUgdXBkYXRlIGZ1bmN0aW9uCmJ1dCB0aGF0IHNlZW1zIGEgYml0 IGhhY2t5LgoKTG9va2luZyBhdCB0aGUgR0RCIGNhcGFiaWxpdHkgY29kZSB0aGVyZSBkb2Vzbid0 IHNlZW0gdG8gcmVwb3J0CmJyZWFrcG9pbnQgY2FwYWJpbGl0eSBzaG9ydCBvZiBqdXN0IGZhaWxp bmcgd2hlbiB5b3UgdHJ5IHRvIHNldCBvbmUuCgo+Cj4+ICAgICAgaWYgKGt2bV9zd19icmVha3Bv aW50c19hY3RpdmUoY3MpKSB7Cj4+ICAgICAgICAgIGRiZy0+Y29udHJvbCB8PSBLVk1fR1VFU1RE QkdfRU5BQkxFIHwgS1ZNX0dVRVNUREJHX1VTRV9TV19CUDsKPj4gICAgICB9Cj4+IC0tCj4+IDIu Ni4zCj4KPiB0aGFua3MKPiAtLSBQTU0KCgotLQpBbGV4IEJlbm7DqWUKX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18Ka3ZtYXJtIG1haWxpbmcgbGlzdAprdm1h cm1AbGlzdHMuY3MuY29sdW1iaWEuZWR1Cmh0dHBzOi8vbGlzdHMuY3MuY29sdW1iaWEuZWR1L21h aWxtYW4vbGlzdGluZm8va3ZtYXJtCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from zen.linaro.local ([81.128.185.34]) by smtp.gmail.com with ESMTPSA id bg10sm2611187wjb.46.2015.12.08.03.49.50 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 08 Dec 2015 03:49:50 -0800 (PST) Received: from zen.linaro.local (localhost [127.0.0.1]) by zen.linaro.local (Postfix) with ESMTPS id 16AE33E01DB; Tue, 8 Dec 2015 11:49:50 +0000 (GMT) References: <1447345251-22625-1-git-send-email-alex.bennee@linaro.org> <1447345251-22625-4-git-send-email-alex.bennee@linaro.org> User-agent: mu4e 0.9.15; emacs 24.5.50.4 From: Alex =?utf-8?Q?Benn=C3=A9e?= To: Peter Maydell Cc: QEMU Developers , qemu-arm@nongnu.org, Christoffer Dall , Zhichao Huang , kvm-devel , arm-mail-list , "kvmarm\@lists.cs.columbia.edu" , Marc Zyngier , Paolo Bonzini Subject: Re: [PATCH v9 3/6] target-arm: kvm - support for single step In-reply-to: Date: Tue, 08 Dec 2015 11:49:49 +0000 Message-ID: <87r3ixp3c2.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-TUID: mEBAX4+23gkT Peter Maydell writes: > On 12 November 2015 at 16:20, Alex Benn=C3=A9e w= rote: >> This adds support for single-step. There isn't much to do on the QEMU >> side as after we set-up the request for single step via the debug ioctl >> it is all handled within the kernel. >> >> Signed-off-by: Alex Benn=C3=A9e >> >> --- >> v2 >> - convert to using HSR_EC >> v3 >> - use internals.h definitions >> --- >> target-arm/kvm.c | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/target-arm/kvm.c b/target-arm/kvm.c >> index 50f70ef..d505a7e 100644 >> --- a/target-arm/kvm.c >> +++ b/target-arm/kvm.c >> @@ -535,6 +535,13 @@ static int kvm_handle_debug(CPUState *cs, struct kv= m_run *run) >> kvm_cpu_synchronize_state(cs); >> >> switch (hsr_ec) { >> + case EC_SOFTWARESTEP: >> + if (cs->singlestep_enabled) { >> + return true; >> + } else { >> + error_report("Came out of SINGLE STEP when not enabled"); >> + } >> + break; >> case EC_AA64_BKPT: >> if (kvm_find_sw_breakpoint(cs, env->pc)) { >> return true; >> @@ -595,6 +602,9 @@ int kvm_arch_on_sigbus(int code, void *addr) >> >> void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *= dbg) >> { >> + if (cs->singlestep_enabled) { >> + dbg->control |=3D KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_SINGLESTEP; >> + } > > Doesn't kvm_update_guest_debug() already set these bits, or am > I misreading it? Yeah. This raises an interesting problem about what to do when we don't have the capability. I could suppress those bits in the update function but that seems a bit hacky. Looking at the GDB capability code there doesn't seem to report breakpoint capability short of just failing when you try to set one. > >> if (kvm_sw_breakpoints_active(cs)) { >> dbg->control |=3D KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP; >> } >> -- >> 2.6.3 > > thanks > -- PMM -- Alex Benn=C3=A9e From mboxrd@z Thu Jan 1 00:00:00 1970 From: alex.bennee@linaro.org (Alex =?utf-8?Q?Benn=C3=A9e?=) Date: Tue, 08 Dec 2015 11:49:49 +0000 Subject: [PATCH v9 3/6] target-arm: kvm - support for single step In-Reply-To: References: <1447345251-22625-1-git-send-email-alex.bennee@linaro.org> <1447345251-22625-4-git-send-email-alex.bennee@linaro.org> Message-ID: <87r3ixp3c2.fsf@linaro.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Peter Maydell writes: > On 12 November 2015 at 16:20, Alex Benn?e wrote: >> This adds support for single-step. There isn't much to do on the QEMU >> side as after we set-up the request for single step via the debug ioctl >> it is all handled within the kernel. >> >> Signed-off-by: Alex Benn?e >> >> --- >> v2 >> - convert to using HSR_EC >> v3 >> - use internals.h definitions >> --- >> target-arm/kvm.c | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/target-arm/kvm.c b/target-arm/kvm.c >> index 50f70ef..d505a7e 100644 >> --- a/target-arm/kvm.c >> +++ b/target-arm/kvm.c >> @@ -535,6 +535,13 @@ static int kvm_handle_debug(CPUState *cs, struct kvm_run *run) >> kvm_cpu_synchronize_state(cs); >> >> switch (hsr_ec) { >> + case EC_SOFTWARESTEP: >> + if (cs->singlestep_enabled) { >> + return true; >> + } else { >> + error_report("Came out of SINGLE STEP when not enabled"); >> + } >> + break; >> case EC_AA64_BKPT: >> if (kvm_find_sw_breakpoint(cs, env->pc)) { >> return true; >> @@ -595,6 +602,9 @@ int kvm_arch_on_sigbus(int code, void *addr) >> >> void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg) >> { >> + if (cs->singlestep_enabled) { >> + dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_SINGLESTEP; >> + } > > Doesn't kvm_update_guest_debug() already set these bits, or am > I misreading it? Yeah. This raises an interesting problem about what to do when we don't have the capability. I could suppress those bits in the update function but that seems a bit hacky. Looking at the GDB capability code there doesn't seem to report breakpoint capability short of just failing when you try to set one. > >> if (kvm_sw_breakpoints_active(cs)) { >> dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP; >> } >> -- >> 2.6.3 > > thanks > -- PMM -- Alex Benn?e From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41047) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a6GmJ-0000dz-RW for qemu-devel@nongnu.org; Tue, 08 Dec 2015 06:49:56 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a6GmG-0005c8-Kr for qemu-devel@nongnu.org; Tue, 08 Dec 2015 06:49:55 -0500 Received: from mail-wm0-x234.google.com ([2a00:1450:400c:c09::234]:34009) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a6GmG-0005bu-Al for qemu-devel@nongnu.org; Tue, 08 Dec 2015 06:49:52 -0500 Received: by wmvv187 with SMTP id v187so209827608wmv.1 for ; Tue, 08 Dec 2015 03:49:51 -0800 (PST) References: <1447345251-22625-1-git-send-email-alex.bennee@linaro.org> <1447345251-22625-4-git-send-email-alex.bennee@linaro.org> From: Alex =?utf-8?Q?Benn=C3=A9e?= In-reply-to: Date: Tue, 08 Dec 2015 11:49:49 +0000 Message-ID: <87r3ixp3c2.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v9 3/6] target-arm: kvm - support for single step List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: kvm-devel , Marc Zyngier , QEMU Developers , qemu-arm@nongnu.org, Christoffer Dall , Zhichao Huang , Paolo Bonzini , "kvmarm@lists.cs.columbia.edu" , arm-mail-list Peter Maydell writes: > On 12 November 2015 at 16:20, Alex Benn=C3=A9e w= rote: >> This adds support for single-step. There isn't much to do on the QEMU >> side as after we set-up the request for single step via the debug ioctl >> it is all handled within the kernel. >> >> Signed-off-by: Alex Benn=C3=A9e >> >> --- >> v2 >> - convert to using HSR_EC >> v3 >> - use internals.h definitions >> --- >> target-arm/kvm.c | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/target-arm/kvm.c b/target-arm/kvm.c >> index 50f70ef..d505a7e 100644 >> --- a/target-arm/kvm.c >> +++ b/target-arm/kvm.c >> @@ -535,6 +535,13 @@ static int kvm_handle_debug(CPUState *cs, struct kv= m_run *run) >> kvm_cpu_synchronize_state(cs); >> >> switch (hsr_ec) { >> + case EC_SOFTWARESTEP: >> + if (cs->singlestep_enabled) { >> + return true; >> + } else { >> + error_report("Came out of SINGLE STEP when not enabled"); >> + } >> + break; >> case EC_AA64_BKPT: >> if (kvm_find_sw_breakpoint(cs, env->pc)) { >> return true; >> @@ -595,6 +602,9 @@ int kvm_arch_on_sigbus(int code, void *addr) >> >> void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *= dbg) >> { >> + if (cs->singlestep_enabled) { >> + dbg->control |=3D KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_SINGLESTEP; >> + } > > Doesn't kvm_update_guest_debug() already set these bits, or am > I misreading it? Yeah. This raises an interesting problem about what to do when we don't have the capability. I could suppress those bits in the update function but that seems a bit hacky. Looking at the GDB capability code there doesn't seem to report breakpoint capability short of just failing when you try to set one. > >> if (kvm_sw_breakpoints_active(cs)) { >> dbg->control |=3D KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP; >> } >> -- >> 2.6.3 > > thanks > -- PMM -- Alex Benn=C3=A9e