From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex =?utf-8?Q?Benn=C3=A9e?= Subject: Re: [PATCH v2 3/4] target-arm: kvm - support for single step Date: Tue, 21 Apr 2015 13:56:45 +0100 Message-ID: <87tww9vdv6.fsf@linaro.org> References: <1427816446-31586-1-git-send-email-alex.bennee@linaro.org> <1427816446-31586-4-git-send-email-alex.bennee@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-reply-to: Sender: kvm-owner@vger.kernel.org To: Peter Maydell Cc: QEMU Developers , Christoffer Dall , Zhichao Huang , kvm-devel , arm-mail-list , "kvmarm@lists.cs.columbia.edu" , Marc Zyngier , Paolo Bonzini List-Id: kvmarm@lists.cs.columbia.edu Peter Maydell writes: > On 31 March 2015 at 16:40, Alex Benn=C3=A9e = wrote: >> This adds support for single-step. There isn't much to do on the QEM= U >> side as after we set-up the request for single step via the debug io= ctl >> it is all handled within the kernel. >> >> Signed-off-by: Alex Benn=C3=A9e >> >> --- >> v2 >> - convert to using HSR_EC >> >> diff --git a/target-arm/kvm.c b/target-arm/kvm.c >> index 290c1fe..ae0f8b2 100644 >> --- a/target-arm/kvm.c >> +++ b/target-arm/kvm.c >> @@ -475,6 +475,7 @@ void kvm_arch_post_run(CPUState *cs, struct kvm_= run *run) >> */ >> >> #define HSR_EC_SHIFT 26 >> +#define HSR_EC_SOFT_STEP 0x32 >> #define HSR_EC_SW_BKPT 0x3c > > We already include internals.h in this file, so can you just use > the EC_* constants and ARM_EL_EC_SHIFT rather than defining > new ones? (Applies for patch 1 as well.) > >> static int kvm_handle_debug(CPUState *cs, struct kvm_run *run) >> @@ -483,6 +484,13 @@ static int kvm_handle_debug(CPUState *cs, struc= t kvm_run *run) >> int hsr_ec =3D arch_info->hsr >> HSR_EC_SHIFT; >> >> switch (hsr_ec) { >> + case HSR_EC_SOFT_STEP: >> + if (cs->singlestep_enabled) { >> + return true; >> + } else { >> + error_report("Came out of SINGLE STEP when not enabled"= ); > > This can only happen if there's a kernel bug, right? Sure. Should we report it differently? abort() out? > >> + } >> + break; >> case HSR_EC_SW_BKPT: >> if (kvm_find_sw_breakpoint(cs, arch_info->pc)) { >> return true; >> @@ -542,6 +550,9 @@ int kvm_arch_on_sigbus(int code, void *addr) >> >> void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_deb= ug *dbg) >> { >> + if (cs->singlestep_enabled) { >> + dbg->control |=3D KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_SINGLE= STEP; >> + } >> if (kvm_sw_breakpoints_active(cs)) { >> dbg->control |=3D KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW= _BP; >> } >> -- >> 2.3.4 >> > > > thanks > -- PMM --=20 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, 21 Apr 2015 13:56:45 +0100 Subject: [PATCH v2 3/4] target-arm: kvm - support for single step In-Reply-To: References: <1427816446-31586-1-git-send-email-alex.bennee@linaro.org> <1427816446-31586-4-git-send-email-alex.bennee@linaro.org> Message-ID: <87tww9vdv6.fsf@linaro.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Peter Maydell writes: > On 31 March 2015 at 16:40, 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 >> >> diff --git a/target-arm/kvm.c b/target-arm/kvm.c >> index 290c1fe..ae0f8b2 100644 >> --- a/target-arm/kvm.c >> +++ b/target-arm/kvm.c >> @@ -475,6 +475,7 @@ void kvm_arch_post_run(CPUState *cs, struct kvm_run *run) >> */ >> >> #define HSR_EC_SHIFT 26 >> +#define HSR_EC_SOFT_STEP 0x32 >> #define HSR_EC_SW_BKPT 0x3c > > We already include internals.h in this file, so can you just use > the EC_* constants and ARM_EL_EC_SHIFT rather than defining > new ones? (Applies for patch 1 as well.) > >> static int kvm_handle_debug(CPUState *cs, struct kvm_run *run) >> @@ -483,6 +484,13 @@ static int kvm_handle_debug(CPUState *cs, struct kvm_run *run) >> int hsr_ec = arch_info->hsr >> HSR_EC_SHIFT; >> >> switch (hsr_ec) { >> + case HSR_EC_SOFT_STEP: >> + if (cs->singlestep_enabled) { >> + return true; >> + } else { >> + error_report("Came out of SINGLE STEP when not enabled"); > > This can only happen if there's a kernel bug, right? Sure. Should we report it differently? abort() out? > >> + } >> + break; >> case HSR_EC_SW_BKPT: >> if (kvm_find_sw_breakpoint(cs, arch_info->pc)) { >> return true; >> @@ -542,6 +550,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; >> + } >> if (kvm_sw_breakpoints_active(cs)) { >> dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP; >> } >> -- >> 2.3.4 >> > > > thanks > -- PMM -- Alex Benn?e From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex =?utf-8?Q?Benn=C3=A9e?= Subject: Re: [PATCH v2 3/4] target-arm: kvm - support for single step Date: Tue, 21 Apr 2015 13:56:45 +0100 Message-ID: <87tww9vdv6.fsf@linaro.org> References: <1427816446-31586-1-git-send-email-alex.bennee@linaro.org> <1427816446-31586-4-git-send-email-alex.bennee@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: QEMU Developers , Christoffer Dall , Zhichao Huang , kvm-devel , arm-mail-list , "kvmarm\@lists.cs.columbia.edu" , Marc Zyngier , Paolo Bonzini To: Peter Maydell Return-path: Received: from static.88-198-71-155.clients.your-server.de ([88.198.71.155]:43678 "EHLO socrates.bennee.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751027AbbDUM4Y (ORCPT ); Tue, 21 Apr 2015 08:56:24 -0400 In-reply-to: Sender: kvm-owner@vger.kernel.org List-ID: Peter Maydell writes: > On 31 March 2015 at 16:40, Alex Benn=C3=A9e = wrote: >> This adds support for single-step. There isn't much to do on the QEM= U >> side as after we set-up the request for single step via the debug io= ctl >> it is all handled within the kernel. >> >> Signed-off-by: Alex Benn=C3=A9e >> >> --- >> v2 >> - convert to using HSR_EC >> >> diff --git a/target-arm/kvm.c b/target-arm/kvm.c >> index 290c1fe..ae0f8b2 100644 >> --- a/target-arm/kvm.c >> +++ b/target-arm/kvm.c >> @@ -475,6 +475,7 @@ void kvm_arch_post_run(CPUState *cs, struct kvm_= run *run) >> */ >> >> #define HSR_EC_SHIFT 26 >> +#define HSR_EC_SOFT_STEP 0x32 >> #define HSR_EC_SW_BKPT 0x3c > > We already include internals.h in this file, so can you just use > the EC_* constants and ARM_EL_EC_SHIFT rather than defining > new ones? (Applies for patch 1 as well.) > >> static int kvm_handle_debug(CPUState *cs, struct kvm_run *run) >> @@ -483,6 +484,13 @@ static int kvm_handle_debug(CPUState *cs, struc= t kvm_run *run) >> int hsr_ec =3D arch_info->hsr >> HSR_EC_SHIFT; >> >> switch (hsr_ec) { >> + case HSR_EC_SOFT_STEP: >> + if (cs->singlestep_enabled) { >> + return true; >> + } else { >> + error_report("Came out of SINGLE STEP when not enabled"= ); > > This can only happen if there's a kernel bug, right? Sure. Should we report it differently? abort() out? > >> + } >> + break; >> case HSR_EC_SW_BKPT: >> if (kvm_find_sw_breakpoint(cs, arch_info->pc)) { >> return true; >> @@ -542,6 +550,9 @@ int kvm_arch_on_sigbus(int code, void *addr) >> >> void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_deb= ug *dbg) >> { >> + if (cs->singlestep_enabled) { >> + dbg->control |=3D KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_SINGLE= STEP; >> + } >> if (kvm_sw_breakpoints_active(cs)) { >> dbg->control |=3D KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW= _BP; >> } >> -- >> 2.3.4 >> > > > thanks > -- PMM --=20 Alex Benn=C3=A9e From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38602) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YkXj3-0000RH-Qx for qemu-devel@nongnu.org; Tue, 21 Apr 2015 08:56:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YkXiz-0005Ow-Qn for qemu-devel@nongnu.org; Tue, 21 Apr 2015 08:56:29 -0400 Received: from static.88-198-71-155.clients.your-server.de ([88.198.71.155]:49616 helo=socrates.bennee.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YkXiz-0005Oo-Ks for qemu-devel@nongnu.org; Tue, 21 Apr 2015 08:56:25 -0400 References: <1427816446-31586-1-git-send-email-alex.bennee@linaro.org> <1427816446-31586-4-git-send-email-alex.bennee@linaro.org> From: Alex =?utf-8?Q?Benn=C3=A9e?= In-reply-to: Date: Tue, 21 Apr 2015 13:56:45 +0100 Message-ID: <87tww9vdv6.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH v2 3/4] 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 , Christoffer Dall , Zhichao Huang , Paolo Bonzini , "kvmarm@lists.cs.columbia.edu" , arm-mail-list Peter Maydell writes: > On 31 March 2015 at 16:40, 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 >> >> diff --git a/target-arm/kvm.c b/target-arm/kvm.c >> index 290c1fe..ae0f8b2 100644 >> --- a/target-arm/kvm.c >> +++ b/target-arm/kvm.c >> @@ -475,6 +475,7 @@ void kvm_arch_post_run(CPUState *cs, struct kvm_run *run) >> */ >> >> #define HSR_EC_SHIFT 26 >> +#define HSR_EC_SOFT_STEP 0x32 >> #define HSR_EC_SW_BKPT 0x3c > > We already include internals.h in this file, so can you just use > the EC_* constants and ARM_EL_EC_SHIFT rather than defining > new ones? (Applies for patch 1 as well.) > >> static int kvm_handle_debug(CPUState *cs, struct kvm_run *run) >> @@ -483,6 +484,13 @@ static int kvm_handle_debug(CPUState *cs, struct kvm_run *run) >> int hsr_ec = arch_info->hsr >> HSR_EC_SHIFT; >> >> switch (hsr_ec) { >> + case HSR_EC_SOFT_STEP: >> + if (cs->singlestep_enabled) { >> + return true; >> + } else { >> + error_report("Came out of SINGLE STEP when not enabled"); > > This can only happen if there's a kernel bug, right? Sure. Should we report it differently? abort() out? > >> + } >> + break; >> case HSR_EC_SW_BKPT: >> if (kvm_find_sw_breakpoint(cs, arch_info->pc)) { >> return true; >> @@ -542,6 +550,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; >> + } >> if (kvm_sw_breakpoints_active(cs)) { >> dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP; >> } >> -- >> 2.3.4 >> > > > thanks > -- PMM -- Alex Bennée