Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754154AbbDNIZ7 (ORCPT ); Tue, 14 Apr 2015 04:25:59 -0400 Received: from mail-lb0-f176.google.com ([209.85.217.176]:33046 "EHLO mail-lb0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752302AbbDNIZw (ORCPT ); Tue, 14 Apr 2015 04:25:52 -0400 Date: Tue, 14 Apr 2015 10:25:58 +0200 From: Christoffer Dall To: Alex =?iso-8859-1?Q?Benn=E9e?= Cc: kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, marc.zyngier@arm.com, peter.maydell@linaro.org, agraf@suse.de, drjones@redhat.com, pbonzini@redhat.com, zhichao.huang@linaro.org, jan.kiszka@siemens.com, dahi@linux.vnet.ibm.com, r65777@freescale.com, bp@suse.de, Gleb Natapov , Jonathan Corbet , Russell King , Catalin Marinas , Will Deacon , "open list:DOCUMENTATION" , open list Subject: Re: [PATCH v2 06/10] KVM: arm64: guest debug, add SW break point support Message-ID: <20150414082558.GS6186@cbox> References: <1427814488-28467-1-git-send-email-alex.bennee@linaro.org> <1427814488-28467-7-git-send-email-alex.bennee@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1427814488-28467-7-git-send-email-alex.bennee@linaro.org> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5497 Lines: 163 On Tue, Mar 31, 2015 at 04:08:04PM +0100, Alex Benn?e wrote: > This adds support for SW breakpoints inserted by userspace. > > We do this by trapping all BKPT exceptions in the > hypervisor (MDCR_EL2_TDE). you mean trapping all exceptions in the guest to the hypervisor? > The kvm_debug_exit_arch carries the address > of the exception. why? can userspace not simply read out the PC using GET_ONE_REG? > If user-space doesn't know of the breakpoint then we > have a guest inserted breakpoint and the hypervisor needs to start again > and deliver the exception to guest. > > Signed-off-by: Alex Benn?e > > --- > v2 > - update to use new exit struct > - tweak for C setup > - do our setup in debug_setup/clear code > - fixed up comments > > diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt > index 06c5064..17d4f9c 100644 > --- a/Documentation/virtual/kvm/api.txt > +++ b/Documentation/virtual/kvm/api.txt > @@ -2626,7 +2626,7 @@ when running. Common control bits are: > The top 16 bits of the control field are architecture specific control > flags which can include the following: > > - - KVM_GUESTDBG_USE_SW_BP: using software breakpoints [x86] > + - KVM_GUESTDBG_USE_SW_BP: using software breakpoints [x86, arm64] > - KVM_GUESTDBG_USE_HW_BP: using hardware breakpoints [x86, s390] > - KVM_GUESTDBG_INJECT_DB: inject DB type exception [x86] > - KVM_GUESTDBG_INJECT_BP: inject BP type exception [x86] > diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c > index 7ea8b0e..d3bc8dc 100644 > --- a/arch/arm/kvm/arm.c > +++ b/arch/arm/kvm/arm.c > @@ -304,7 +304,7 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu) > kvm_arm_set_running_vcpu(NULL); > } > > -#define KVM_GUESTDBG_VALID (KVM_GUESTDBG_ENABLE) > +#define KVM_GUESTDBG_VALID (KVM_GUESTDBG_ENABLE|KVM_GUESTDBG_USE_SW_BP) nit: spaces around the operator > > int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu, > struct kvm_guest_debug *dbg) > diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c > index 8a29d0b..cff0475 100644 > --- a/arch/arm64/kvm/debug.c > +++ b/arch/arm64/kvm/debug.c > @@ -45,11 +45,18 @@ void kvm_arch_setup_debug(struct kvm_vcpu *vcpu) > vcpu->arch.mdcr_el2 |= (MDCR_EL2_TPM | MDCR_EL2_TPMCR); > vcpu->arch.mdcr_el2 |= (MDCR_EL2_TDRA | MDCR_EL2_TDOSA); > > + /* Trap debug register access? */ other patch > if (!vcpu->arch.debug_flags & KVM_ARM64_DEBUG_DIRTY) > vcpu->arch.mdcr_el2 |= MDCR_EL2_TDA; > else > vcpu->arch.mdcr_el2 &= ~MDCR_EL2_TDA; > > + /* Trap breakpoints? */ > + if (vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP) > + vcpu->arch.mdcr_el2 |= MDCR_EL2_TDE; > + else > + vcpu->arch.mdcr_el2 &= ~MDCR_EL2_TDE; so now you're trapping all debug exceptions, right? what happens if the guest is using the hardware to debug debug stuff and generates other kinds of debug exceptions, like a hardware breakpoint, will we not see an unhandled exception and the guest being forcefully killed? > + > } > > void kvm_arch_clear_debug(struct kvm_vcpu *vcpu) > diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c > index 524fa25..ed1bbb4 100644 > --- a/arch/arm64/kvm/handle_exit.c > +++ b/arch/arm64/kvm/handle_exit.c > @@ -82,6 +82,37 @@ static int kvm_handle_wfx(struct kvm_vcpu *vcpu, struct kvm_run *run) > return 1; > } > > +/** > + * kvm_handle_debug_exception - handle a debug exception instruction handle a software breadkpoint exception > + * > + * @vcpu: the vcpu pointer > + * @run: access to the kvm_run structure for results > + * > + * We route all debug exceptions through the same handler as we all debug exceptions? software breakpoints and all? then why the above shot text? > + * just need to report the PC and the HSR values to userspace. > + * Userspace may decide to re-inject the exception and deliver it to > + * the guest if it wasn't for the host to deal with. now I'm confused - does userspace setup the guest to receive an exception or does it tell KVM to emulate an exception for the guest or do we execute the breakpoint without trapping the debug exception? > + */ > +static int kvm_handle_guest_debug(struct kvm_vcpu *vcpu, struct kvm_run *run) > +{ > + u32 hsr = kvm_vcpu_get_hsr(vcpu); > + > + run->exit_reason = KVM_EXIT_DEBUG; > + run->debug.arch.hsr = hsr; > + > + switch (hsr >> ESR_ELx_EC_SHIFT) { > + case ESR_ELx_EC_BKPT32: > + case ESR_ELx_EC_BRK64: > + run->debug.arch.pc = *vcpu_pc(vcpu); > + break; > + default: > + kvm_err("%s: un-handled case hsr: %#08x\n", > + __func__, (unsigned int) hsr); this should never happen right? > + break; > + } > + return 0; > +} > + > static exit_handle_fn arm_exit_handlers[] = { > [ESR_ELx_EC_WFx] = kvm_handle_wfx, > [ESR_ELx_EC_CP15_32] = kvm_handle_cp15_32, > @@ -96,6 +127,8 @@ static exit_handle_fn arm_exit_handlers[] = { > [ESR_ELx_EC_SYS64] = kvm_handle_sys_reg, > [ESR_ELx_EC_IABT_LOW] = kvm_handle_guest_abort, > [ESR_ELx_EC_DABT_LOW] = kvm_handle_guest_abort, > + [ESR_ELx_EC_BKPT32] = kvm_handle_guest_debug, > + [ESR_ELx_EC_BRK64] = kvm_handle_guest_debug, > }; > > static exit_handle_fn kvm_get_exit_handler(struct kvm_vcpu *vcpu) > -- > 2.3.4 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/