From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753640AbbEHQqu (ORCPT ); Fri, 8 May 2015 12:46:50 -0400 Received: from mail-lb0-f179.google.com ([209.85.217.179]:33178 "EHLO mail-lb0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752930AbbEHQqr (ORCPT ); Fri, 8 May 2015 12:46:47 -0400 Date: Fri, 8 May 2015 18:46:49 +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 , Catalin Marinas , Will Deacon , open list Subject: Re: [PATCH v3 10/12] KVM: arm64: trap nested debug register access Message-ID: <20150508164649.GK24744@cbox> References: <1430929407-3487-1-git-send-email-alex.bennee@linaro.org> <1430989647-22501-3-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: <1430989647-22501-3-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 On Thu, May 07, 2015 at 10:07:13AM +0100, Alex Bennée wrote: > When we are using the hardware registers for guest debug we need to deal > with the guests access to them. There is already a mechanism for dealing > with these accesses so we build on top of that. > > - any access to mdscr_el1 is now stored in the mirror location > - access to DBG[WB][CV]R continues to go to guest's context > > There is one register (MDCCINT_EL1) which guest debug doesn't care about > so this behaves as before. > > Signed-off-by: Alex Bennée > > --- > v3 > - re-factor for better flow and fall through. > - much simpler with debug_ptr (use the guest area as before) > - tweak shadow fn to avoid multi-line if > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > index a44fb32..7aa3b3a 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -132,7 +132,13 @@ struct kvm_vcpu_arch { > * here. > */ > > - /* Guest registers we preserve during guest debugging */ > + /* > + * Guest registers we preserve during guest debugging. > + * > + * These shadow registers are updated by the kvm_handle_sys_reg > + * trap handler if the guest accesses or updates them while we > + * are using guest debug. > + */ > struct { > u32 pstate; > u32 mdscr_el1; > diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c > index 1ab63dd..dc8bca8 100644 > --- a/arch/arm64/kvm/debug.c > +++ b/arch/arm64/kvm/debug.c > @@ -50,8 +50,7 @@ static void restore_guest_debug_regs(struct kvm_vcpu *vcpu) > { > *vcpu_cpsr(vcpu) |= > (vcpu->arch.guest_debug_state.pstate & SPSR_DEBUG_MASK); > - vcpu_sys_reg(vcpu, MDSCR_EL1) |= > - (vcpu->arch.guest_debug_state.mdscr_el1 & MDSCR_EL1_DEBUG_MASK); > + vcpu_sys_reg(vcpu, MDSCR_EL1) = vcpu->arch.guest_debug_state.mdscr_el1; > } > > /** > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > index c370b40..95f422f 100644 > --- a/arch/arm64/kvm/sys_regs.c > +++ b/arch/arm64/kvm/sys_regs.c > @@ -196,11 +196,40 @@ static bool trap_dbgauthstatus_el1(struct kvm_vcpu *vcpu, > * - If the dirty bit is set, save guest registers, restore host > * registers and clear the dirty bit. This ensure that the host can > * now use the debug registers. > + * > + * We also use this mechanism to set-up the debug registers for guest s/set-up/set up/ > + * debugging. If this is the case we want to ensure the guest sees If this is the case, (comma) > + * the right versions of the registers - even if they are not going to > + * be effective while guest debug is using HW debug. > + * > */ > + > +static bool shadow_debug_reg(struct kvm_vcpu *vcpu, > + const struct sys_reg_params *p, > + const struct sys_reg_desc *r) > +{ > + /* MDSCR_EL1 */ > + if (r->reg == MDSCR_EL1) { > + u32 *shadow_mdscr_el1 = &vcpu->arch.guest_debug_state.mdscr_el1; > + > + if (p->is_write) > + *shadow_mdscr_el1 = *vcpu_reg(vcpu, p->Rt); > + else > + *vcpu_reg(vcpu, p->Rt) = *shadow_mdscr_el1; > + > + return true; > + } > + > + return false; > +} > + > static bool trap_debug_regs(struct kvm_vcpu *vcpu, > const struct sys_reg_params *p, > const struct sys_reg_desc *r) > { > + if (vcpu->guest_debug && shadow_debug_reg(vcpu, p, r)) > + return true; > + so do we also have a MDSCR_EL1 in sys_regs and one in guest_debug_state now? If yes, what are the differences between the two? > if (p->is_write) { > vcpu_sys_reg(vcpu, r->reg) = *vcpu_reg(vcpu, p->Rt); > vcpu->arch.debug_flags |= KVM_ARM64_DEBUG_DIRTY; > -- > 2.3.5 > Thanks, -Christoffer From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christoffer Dall Subject: Re: [PATCH v3 10/12] KVM: arm64: trap nested debug register access Date: Fri, 8 May 2015 18:46:49 +0200 Message-ID: <20150508164649.GK24744@cbox> References: <1430929407-3487-1-git-send-email-alex.bennee@linaro.org> <1430989647-22501-3-git-send-email-alex.bennee@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Cc: Catalin Marinas , kvm@vger.kernel.org, marc.zyngier@arm.com, jan.kiszka@siemens.com, Will Deacon , open list , dahi@linux.vnet.ibm.com, zhichao.huang@linaro.org, r65777@freescale.com, pbonzini@redhat.com, bp@suse.de, Gleb Natapov , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org To: Alex =?iso-8859-1?Q?Benn=E9e?= Return-path: Content-Disposition: inline In-Reply-To: <1430989647-22501-3-git-send-email-alex.bennee@linaro.org> 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 List-Id: kvm.vger.kernel.org On Thu, May 07, 2015 at 10:07:13AM +0100, Alex Benn=E9e wrote: > When we are using the hardware registers for guest debug we need to deal > with the guests access to them. There is already a mechanism for dealing > with these accesses so we build on top of that. > = > - any access to mdscr_el1 is now stored in the mirror location > - access to DBG[WB][CV]R continues to go to guest's context > = > There is one register (MDCCINT_EL1) which guest debug doesn't care about > so this behaves as before. > = > Signed-off-by: Alex Benn=E9e > = > --- > v3 > - re-factor for better flow and fall through. > - much simpler with debug_ptr (use the guest area as before) > - tweak shadow fn to avoid multi-line if > = > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/k= vm_host.h > index a44fb32..7aa3b3a 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -132,7 +132,13 @@ struct kvm_vcpu_arch { > * here. > */ > = > - /* Guest registers we preserve during guest debugging */ > + /* > + * Guest registers we preserve during guest debugging. > + * > + * These shadow registers are updated by the kvm_handle_sys_reg > + * trap handler if the guest accesses or updates them while we > + * are using guest debug. > + */ > struct { > u32 pstate; > u32 mdscr_el1; > diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c > index 1ab63dd..dc8bca8 100644 > --- a/arch/arm64/kvm/debug.c > +++ b/arch/arm64/kvm/debug.c > @@ -50,8 +50,7 @@ static void restore_guest_debug_regs(struct kvm_vcpu *v= cpu) > { > *vcpu_cpsr(vcpu) |=3D > (vcpu->arch.guest_debug_state.pstate & SPSR_DEBUG_MASK); > - vcpu_sys_reg(vcpu, MDSCR_EL1) |=3D > - (vcpu->arch.guest_debug_state.mdscr_el1 & MDSCR_EL1_DEBUG_MASK); > + vcpu_sys_reg(vcpu, MDSCR_EL1) =3D vcpu->arch.guest_debug_state.mdscr_el= 1; > } > = > /** > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > index c370b40..95f422f 100644 > --- a/arch/arm64/kvm/sys_regs.c > +++ b/arch/arm64/kvm/sys_regs.c > @@ -196,11 +196,40 @@ static bool trap_dbgauthstatus_el1(struct kvm_vcpu = *vcpu, > * - If the dirty bit is set, save guest registers, restore host > * registers and clear the dirty bit. This ensure that the host can > * now use the debug registers. > + * > + * We also use this mechanism to set-up the debug registers for guest s/set-up/set up/ > + * debugging. If this is the case we want to ensure the guest sees If this is the case, (comma) > + * the right versions of the registers - even if they are not going to > + * be effective while guest debug is using HW debug. > + * > */ > + > +static bool shadow_debug_reg(struct kvm_vcpu *vcpu, > + const struct sys_reg_params *p, > + const struct sys_reg_desc *r) > +{ > + /* MDSCR_EL1 */ > + if (r->reg =3D=3D MDSCR_EL1) { > + u32 *shadow_mdscr_el1 =3D &vcpu->arch.guest_debug_state.mdscr_el1; > + > + if (p->is_write) > + *shadow_mdscr_el1 =3D *vcpu_reg(vcpu, p->Rt); > + else > + *vcpu_reg(vcpu, p->Rt) =3D *shadow_mdscr_el1; > + > + return true; > + } > + > + return false; > +} > + > static bool trap_debug_regs(struct kvm_vcpu *vcpu, > const struct sys_reg_params *p, > const struct sys_reg_desc *r) > { > + if (vcpu->guest_debug && shadow_debug_reg(vcpu, p, r)) > + return true; > + so do we also have a MDSCR_EL1 in sys_regs and one in guest_debug_state now? If yes, what are the differences between the two? > if (p->is_write) { > vcpu_sys_reg(vcpu, r->reg) =3D *vcpu_reg(vcpu, p->Rt); > vcpu->arch.debug_flags |=3D KVM_ARM64_DEBUG_DIRTY; > -- = > 2.3.5 > = Thanks, -Christoffer From mboxrd@z Thu Jan 1 00:00:00 1970 From: christoffer.dall@linaro.org (Christoffer Dall) Date: Fri, 8 May 2015 18:46:49 +0200 Subject: [PATCH v3 10/12] KVM: arm64: trap nested debug register access In-Reply-To: <1430989647-22501-3-git-send-email-alex.bennee@linaro.org> References: <1430929407-3487-1-git-send-email-alex.bennee@linaro.org> <1430989647-22501-3-git-send-email-alex.bennee@linaro.org> Message-ID: <20150508164649.GK24744@cbox> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Thu, May 07, 2015 at 10:07:13AM +0100, Alex Benn?e wrote: > When we are using the hardware registers for guest debug we need to deal > with the guests access to them. There is already a mechanism for dealing > with these accesses so we build on top of that. > > - any access to mdscr_el1 is now stored in the mirror location > - access to DBG[WB][CV]R continues to go to guest's context > > There is one register (MDCCINT_EL1) which guest debug doesn't care about > so this behaves as before. > > Signed-off-by: Alex Benn?e > > --- > v3 > - re-factor for better flow and fall through. > - much simpler with debug_ptr (use the guest area as before) > - tweak shadow fn to avoid multi-line if > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > index a44fb32..7aa3b3a 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -132,7 +132,13 @@ struct kvm_vcpu_arch { > * here. > */ > > - /* Guest registers we preserve during guest debugging */ > + /* > + * Guest registers we preserve during guest debugging. > + * > + * These shadow registers are updated by the kvm_handle_sys_reg > + * trap handler if the guest accesses or updates them while we > + * are using guest debug. > + */ > struct { > u32 pstate; > u32 mdscr_el1; > diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c > index 1ab63dd..dc8bca8 100644 > --- a/arch/arm64/kvm/debug.c > +++ b/arch/arm64/kvm/debug.c > @@ -50,8 +50,7 @@ static void restore_guest_debug_regs(struct kvm_vcpu *vcpu) > { > *vcpu_cpsr(vcpu) |= > (vcpu->arch.guest_debug_state.pstate & SPSR_DEBUG_MASK); > - vcpu_sys_reg(vcpu, MDSCR_EL1) |= > - (vcpu->arch.guest_debug_state.mdscr_el1 & MDSCR_EL1_DEBUG_MASK); > + vcpu_sys_reg(vcpu, MDSCR_EL1) = vcpu->arch.guest_debug_state.mdscr_el1; > } > > /** > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > index c370b40..95f422f 100644 > --- a/arch/arm64/kvm/sys_regs.c > +++ b/arch/arm64/kvm/sys_regs.c > @@ -196,11 +196,40 @@ static bool trap_dbgauthstatus_el1(struct kvm_vcpu *vcpu, > * - If the dirty bit is set, save guest registers, restore host > * registers and clear the dirty bit. This ensure that the host can > * now use the debug registers. > + * > + * We also use this mechanism to set-up the debug registers for guest s/set-up/set up/ > + * debugging. If this is the case we want to ensure the guest sees If this is the case, (comma) > + * the right versions of the registers - even if they are not going to > + * be effective while guest debug is using HW debug. > + * > */ > + > +static bool shadow_debug_reg(struct kvm_vcpu *vcpu, > + const struct sys_reg_params *p, > + const struct sys_reg_desc *r) > +{ > + /* MDSCR_EL1 */ > + if (r->reg == MDSCR_EL1) { > + u32 *shadow_mdscr_el1 = &vcpu->arch.guest_debug_state.mdscr_el1; > + > + if (p->is_write) > + *shadow_mdscr_el1 = *vcpu_reg(vcpu, p->Rt); > + else > + *vcpu_reg(vcpu, p->Rt) = *shadow_mdscr_el1; > + > + return true; > + } > + > + return false; > +} > + > static bool trap_debug_regs(struct kvm_vcpu *vcpu, > const struct sys_reg_params *p, > const struct sys_reg_desc *r) > { > + if (vcpu->guest_debug && shadow_debug_reg(vcpu, p, r)) > + return true; > + so do we also have a MDSCR_EL1 in sys_regs and one in guest_debug_state now? If yes, what are the differences between the two? > if (p->is_write) { > vcpu_sys_reg(vcpu, r->reg) = *vcpu_reg(vcpu, p->Rt); > vcpu->arch.debug_flags |= KVM_ARM64_DEBUG_DIRTY; > -- > 2.3.5 > Thanks, -Christoffer