From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932565AbbDJMjW (ORCPT ); Fri, 10 Apr 2015 08:39:22 -0400 Received: from mx1.redhat.com ([209.132.183.28]:39910 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754884AbbDJMjU (ORCPT ); Fri, 10 Apr 2015 08:39:20 -0400 Date: Fri, 10 Apr 2015 14:38:31 +0200 From: Andrew Jones To: Alex =?iso-8859-1?Q?Benn=E9e?= Cc: kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, christoffer.dall@linaro.org, marc.zyngier@arm.com, peter.maydell@linaro.org, agraf@suse.de, 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 v2 09/10] KVM: arm64: trap nested debug register access Message-ID: <20150410123830.GB3227@hawk.usersys.redhat.com> References: <1427814488-28467-1-git-send-email-alex.bennee@linaro.org> <1427814488-28467-10-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-10-git-send-email-alex.bennee@linaro.org> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Mar 31, 2015 at 04:08:07PM +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. > > - mdscr_el1_bits is renamed as we save the whole register > - any access to mdscr_el1 is now stored in the mirror location > - if we are using HW assisted debug we do the same with DBG[WB][CV]R > > There is one register (MDCCINT_EL1) which guest debug doesn't care about > so this behaves as before. > > Signed-off-by: Alex Bennée > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > index 2c359c9..3d32d45 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -122,10 +122,13 @@ struct kvm_vcpu_arch { > * here. > */ > > - /* Registers pre any guest debug manipulations */ > + /* Registers before any guest debug manipulations. These starting comment /* on own line > + * shadow registers are updated by the kvm_handle_sys_reg > + * trap handler if the guest accesses or updates them > + */ > struct { > u32 pstate_ss_bit; > - u32 mdscr_el1_bits; > + u32 mdscr_el1; > > struct kvm_guest_debug_arch debug_regs; > } debug_saved_regs; > diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c > index 3b368f3..638c111 100644 > --- a/arch/arm64/kvm/debug.c > +++ b/arch/arm64/kvm/debug.c > @@ -55,8 +55,6 @@ 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); > > - trace_kvm_arch_setup_debug_reg32("MDCR_EL2", vcpu->arch.mdcr_el2); > - I guess I'll see this come back in the next patch. You must be playing 'now you see me, now you don't' > /* > * If we are not treating debug registers are dirty we need > * to trap if the guest starts accessing them. > @@ -71,8 +69,10 @@ void kvm_arch_setup_debug(struct kvm_vcpu *vcpu) > /* Save pstate/mdscr */ > vcpu_debug_saved_reg(vcpu, pstate_ss_bit) = > *vcpu_cpsr(vcpu) & DBG_SPSR_SS; > - vcpu_debug_saved_reg(vcpu, mdscr_el1_bits) = > - vcpu_sys_reg(vcpu, MDSCR_EL1) & MDSCR_EL1_DEBUG_BITS; > + > + vcpu_debug_saved_reg(vcpu, mdscr_el1) = > + vcpu_sys_reg(vcpu, MDSCR_EL1); > + > /* > * Single Step (ARM ARM D2.12.3 The software step state > * machine) > @@ -161,9 +161,8 @@ void kvm_arch_clear_debug(struct kvm_vcpu *vcpu) > *vcpu_cpsr(vcpu) &= ~DBG_SPSR_SS; > *vcpu_cpsr(vcpu) |= vcpu_debug_saved_reg(vcpu, pstate_ss_bit); > > - vcpu_sys_reg(vcpu, MDSCR_EL1) &= ~MDSCR_EL1_DEBUG_BITS; > - vcpu_sys_reg(vcpu, MDSCR_EL1) |= > - vcpu_debug_saved_reg(vcpu, mdscr_el1_bits); > + vcpu_sys_reg(vcpu, MDSCR_EL1) = > + vcpu_debug_saved_reg(vcpu, mdscr_el1); > > /* > * If we were using HW debug we need to restore the > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > index be9b188..d43d7d1 100644 > --- a/arch/arm64/kvm/sys_regs.c > +++ b/arch/arm64/kvm/sys_regs.c > @@ -208,39 +208,61 @@ 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 & KVM_GUESTDBG_USE_HW_BP) { > - struct kvm_guest_debug_arch *saved; > - __u64 *val; > - > - saved = &vcpu_debug_saved_reg(vcpu, debug_regs); > - > - if (r->reg >= DBGBCR0_EL1 && r->reg <= DBGBCR15_EL1) > - val = &saved->dbg_bcr[r->reg - DBGBCR0_EL1]; > - else if (r->reg >= DBGBVR0_EL1 && r->reg <= DBGBVR15_EL1) > - val = &saved->dbg_bvr[r->reg - DBGBVR0_EL1]; > - else if (r->reg >= DBGWCR0_EL1 && r->reg <= DBGWCR15_EL1) > - val = &saved->dbg_wcr[r->reg - DBGWCR0_EL1]; > - else if (r->reg >= DBGWVR0_EL1 && r->reg <= DBGWVR15_EL1) > - val = &saved->dbg_wvr[r->reg - DBGWVR0_EL1]; > - else { > - kvm_err("Bad register index %d\n", r->reg); > - return false; > + if (vcpu->guest_debug) { > + > + /* MDSCR_EL1 */ > + if (r->reg == MDSCR_EL1) { > + if (p->is_write) > + vcpu_debug_saved_reg(vcpu, mdscr_el1) = > + *vcpu_reg(vcpu, p->Rt); > + else > + *vcpu_reg(vcpu, p->Rt) = > + vcpu_debug_saved_reg(vcpu, mdscr_el1); With this lines wrapping, {}'s might be nice. > + > + return true; > } > > - if (p->is_write) > - *val = *vcpu_reg(vcpu, p->Rt); > - else > - *vcpu_reg(vcpu, p->Rt) = *val; > + /* MDCCINT_EL1 */ > + if (r->reg == MDCCINT_EL1) > + goto old; "old"? As in the old way this worked? Someday (soon) all this code will be "old". How about just 'out'? Or use some other way to get the flow such that we avoid code duplication, but doesn't require a goto? > + > + /* We only shadow DBG* if guest being debugged */ > + if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP) { > + struct kvm_guest_debug_arch *saved; > + __u64 *val; > + > + saved = &vcpu_debug_saved_reg(vcpu, debug_regs); > + > + if (r->reg >= DBGBCR0_EL1 && r->reg <= DBGBCR15_EL1) > + val = &saved->dbg_bcr[r->reg - DBGBCR0_EL1]; > + else if (r->reg >= DBGBVR0_EL1 && r->reg <= DBGBVR15_EL1) > + val = &saved->dbg_bvr[r->reg - DBGBVR0_EL1]; > + else if (r->reg >= DBGWCR0_EL1 && r->reg <= DBGWCR15_EL1) > + val = &saved->dbg_wcr[r->reg - DBGWCR0_EL1]; > + else if (r->reg >= DBGWVR0_EL1 && r->reg <= DBGWVR15_EL1) > + val = &saved->dbg_wvr[r->reg - DBGWVR0_EL1]; > + else { > + kvm_err("Bad register index %d\n", r->reg); > + return false; > + } > > - } else { > - if (p->is_write) { > - vcpu_sys_reg(vcpu, r->reg) = *vcpu_reg(vcpu, p->Rt); > - vcpu->arch.debug_flags |= KVM_ARM64_DEBUG_DIRTY; > - } else { > - *vcpu_reg(vcpu, p->Rt) = vcpu_sys_reg(vcpu, r->reg); > + if (p->is_write) > + *val = *vcpu_reg(vcpu, p->Rt); > + else > + *vcpu_reg(vcpu, p->Rt) = *val; > + > + return true; > } > } > > +old: > + if (p->is_write) { > + vcpu_sys_reg(vcpu, r->reg) = *vcpu_reg(vcpu, p->Rt); > + vcpu->arch.debug_flags |= KVM_ARM64_DEBUG_DIRTY; > + } else { > + *vcpu_reg(vcpu, p->Rt) = vcpu_sys_reg(vcpu, r->reg); > + } > + > return true; > } > > -- > 2.3.4 > From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Jones Subject: Re: [PATCH v2 09/10] KVM: arm64: trap nested debug register access Date: Fri, 10 Apr 2015 14:38:31 +0200 Message-ID: <20150410123830.GB3227@hawk.usersys.redhat.com> References: <1427814488-28467-1-git-send-email-alex.bennee@linaro.org> <1427814488-28467-10-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, linux-arm-kernel@lists.infradead.org, zhichao.huang@linaro.org, r65777@freescale.com, pbonzini@redhat.com, bp@suse.de, Gleb Natapov , kvmarm@lists.cs.columbia.edu To: Alex =?iso-8859-1?Q?Benn=E9e?= Return-path: Content-Disposition: inline In-Reply-To: <1427814488-28467-10-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 Tue, Mar 31, 2015 at 04:08:07PM +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. > = > - mdscr_el1_bits is renamed as we save the whole register > - any access to mdscr_el1 is now stored in the mirror location > - if we are using HW assisted debug we do the same with DBG[WB][CV]R > = > There is one register (MDCCINT_EL1) which guest debug doesn't care about > so this behaves as before. > = > Signed-off-by: Alex Benn=E9e > = > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/k= vm_host.h > index 2c359c9..3d32d45 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -122,10 +122,13 @@ struct kvm_vcpu_arch { > * here. > */ > = > - /* Registers pre any guest debug manipulations */ > + /* Registers before any guest debug manipulations. These starting comment /* on own line > + * shadow registers are updated by the kvm_handle_sys_reg > + * trap handler if the guest accesses or updates them > + */ > struct { > u32 pstate_ss_bit; > - u32 mdscr_el1_bits; > + u32 mdscr_el1; > = > struct kvm_guest_debug_arch debug_regs; > } debug_saved_regs; > diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c > index 3b368f3..638c111 100644 > --- a/arch/arm64/kvm/debug.c > +++ b/arch/arm64/kvm/debug.c > @@ -55,8 +55,6 @@ void kvm_arch_setup_debug(struct kvm_vcpu *vcpu) > vcpu->arch.mdcr_el2 |=3D (MDCR_EL2_TPM | MDCR_EL2_TPMCR); > vcpu->arch.mdcr_el2 |=3D (MDCR_EL2_TDRA | MDCR_EL2_TDOSA); > = > - trace_kvm_arch_setup_debug_reg32("MDCR_EL2", vcpu->arch.mdcr_el2); > - I guess I'll see this come back in the next patch. You must be playing 'now you see me, now you don't' > /* > * If we are not treating debug registers are dirty we need > * to trap if the guest starts accessing them. > @@ -71,8 +69,10 @@ void kvm_arch_setup_debug(struct kvm_vcpu *vcpu) > /* Save pstate/mdscr */ > vcpu_debug_saved_reg(vcpu, pstate_ss_bit) =3D > *vcpu_cpsr(vcpu) & DBG_SPSR_SS; > - vcpu_debug_saved_reg(vcpu, mdscr_el1_bits) =3D > - vcpu_sys_reg(vcpu, MDSCR_EL1) & MDSCR_EL1_DEBUG_BITS; > + > + vcpu_debug_saved_reg(vcpu, mdscr_el1) =3D > + vcpu_sys_reg(vcpu, MDSCR_EL1); > + > /* > * Single Step (ARM ARM D2.12.3 The software step state > * machine) > @@ -161,9 +161,8 @@ void kvm_arch_clear_debug(struct kvm_vcpu *vcpu) > *vcpu_cpsr(vcpu) &=3D ~DBG_SPSR_SS; > *vcpu_cpsr(vcpu) |=3D vcpu_debug_saved_reg(vcpu, pstate_ss_bit); > = > - vcpu_sys_reg(vcpu, MDSCR_EL1) &=3D ~MDSCR_EL1_DEBUG_BITS; > - vcpu_sys_reg(vcpu, MDSCR_EL1) |=3D > - vcpu_debug_saved_reg(vcpu, mdscr_el1_bits); > + vcpu_sys_reg(vcpu, MDSCR_EL1) =3D > + vcpu_debug_saved_reg(vcpu, mdscr_el1); > = > /* > * If we were using HW debug we need to restore the > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > index be9b188..d43d7d1 100644 > --- a/arch/arm64/kvm/sys_regs.c > +++ b/arch/arm64/kvm/sys_regs.c > @@ -208,39 +208,61 @@ 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 & KVM_GUESTDBG_USE_HW_BP) { > - struct kvm_guest_debug_arch *saved; > - __u64 *val; > - > - saved =3D &vcpu_debug_saved_reg(vcpu, debug_regs); > - > - if (r->reg >=3D DBGBCR0_EL1 && r->reg <=3D DBGBCR15_EL1) > - val =3D &saved->dbg_bcr[r->reg - DBGBCR0_EL1]; > - else if (r->reg >=3D DBGBVR0_EL1 && r->reg <=3D DBGBVR15_EL1) > - val =3D &saved->dbg_bvr[r->reg - DBGBVR0_EL1]; > - else if (r->reg >=3D DBGWCR0_EL1 && r->reg <=3D DBGWCR15_EL1) > - val =3D &saved->dbg_wcr[r->reg - DBGWCR0_EL1]; > - else if (r->reg >=3D DBGWVR0_EL1 && r->reg <=3D DBGWVR15_EL1) > - val =3D &saved->dbg_wvr[r->reg - DBGWVR0_EL1]; > - else { > - kvm_err("Bad register index %d\n", r->reg); > - return false; > + if (vcpu->guest_debug) { > + > + /* MDSCR_EL1 */ > + if (r->reg =3D=3D MDSCR_EL1) { > + if (p->is_write) > + vcpu_debug_saved_reg(vcpu, mdscr_el1) =3D > + *vcpu_reg(vcpu, p->Rt); > + else > + *vcpu_reg(vcpu, p->Rt) =3D > + vcpu_debug_saved_reg(vcpu, mdscr_el1); With this lines wrapping, {}'s might be nice. > + > + return true; > } > = > - if (p->is_write) > - *val =3D *vcpu_reg(vcpu, p->Rt); > - else > - *vcpu_reg(vcpu, p->Rt) =3D *val; > + /* MDCCINT_EL1 */ > + if (r->reg =3D=3D MDCCINT_EL1) > + goto old; "old"? As in the old way this worked? Someday (soon) all this code will be "old". How about just 'out'? Or use some other way to get the flow such that we avoid code duplication, but doesn't require a goto? > + > + /* We only shadow DBG* if guest being debugged */ > + if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP) { > + struct kvm_guest_debug_arch *saved; > + __u64 *val; > + > + saved =3D &vcpu_debug_saved_reg(vcpu, debug_regs); > + > + if (r->reg >=3D DBGBCR0_EL1 && r->reg <=3D DBGBCR15_EL1) > + val =3D &saved->dbg_bcr[r->reg - DBGBCR0_EL1]; > + else if (r->reg >=3D DBGBVR0_EL1 && r->reg <=3D DBGBVR15_EL1) > + val =3D &saved->dbg_bvr[r->reg - DBGBVR0_EL1]; > + else if (r->reg >=3D DBGWCR0_EL1 && r->reg <=3D DBGWCR15_EL1) > + val =3D &saved->dbg_wcr[r->reg - DBGWCR0_EL1]; > + else if (r->reg >=3D DBGWVR0_EL1 && r->reg <=3D DBGWVR15_EL1) > + val =3D &saved->dbg_wvr[r->reg - DBGWVR0_EL1]; > + else { > + kvm_err("Bad register index %d\n", r->reg); > + return false; > + } > = > - } else { > - 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; > - } else { > - *vcpu_reg(vcpu, p->Rt) =3D vcpu_sys_reg(vcpu, r->reg); > + if (p->is_write) > + *val =3D *vcpu_reg(vcpu, p->Rt); > + else > + *vcpu_reg(vcpu, p->Rt) =3D *val; > + > + return true; > } > } > = > +old: > + 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; > + } else { > + *vcpu_reg(vcpu, p->Rt) =3D vcpu_sys_reg(vcpu, r->reg); > + } > + > return true; > } > = > -- = > 2.3.4 > = From mboxrd@z Thu Jan 1 00:00:00 1970 From: drjones@redhat.com (Andrew Jones) Date: Fri, 10 Apr 2015 14:38:31 +0200 Subject: [PATCH v2 09/10] KVM: arm64: trap nested debug register access In-Reply-To: <1427814488-28467-10-git-send-email-alex.bennee@linaro.org> References: <1427814488-28467-1-git-send-email-alex.bennee@linaro.org> <1427814488-28467-10-git-send-email-alex.bennee@linaro.org> Message-ID: <20150410123830.GB3227@hawk.usersys.redhat.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Tue, Mar 31, 2015 at 04:08:07PM +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. > > - mdscr_el1_bits is renamed as we save the whole register > - any access to mdscr_el1 is now stored in the mirror location > - if we are using HW assisted debug we do the same with DBG[WB][CV]R > > There is one register (MDCCINT_EL1) which guest debug doesn't care about > so this behaves as before. > > Signed-off-by: Alex Benn?e > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > index 2c359c9..3d32d45 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -122,10 +122,13 @@ struct kvm_vcpu_arch { > * here. > */ > > - /* Registers pre any guest debug manipulations */ > + /* Registers before any guest debug manipulations. These starting comment /* on own line > + * shadow registers are updated by the kvm_handle_sys_reg > + * trap handler if the guest accesses or updates them > + */ > struct { > u32 pstate_ss_bit; > - u32 mdscr_el1_bits; > + u32 mdscr_el1; > > struct kvm_guest_debug_arch debug_regs; > } debug_saved_regs; > diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c > index 3b368f3..638c111 100644 > --- a/arch/arm64/kvm/debug.c > +++ b/arch/arm64/kvm/debug.c > @@ -55,8 +55,6 @@ 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); > > - trace_kvm_arch_setup_debug_reg32("MDCR_EL2", vcpu->arch.mdcr_el2); > - I guess I'll see this come back in the next patch. You must be playing 'now you see me, now you don't' > /* > * If we are not treating debug registers are dirty we need > * to trap if the guest starts accessing them. > @@ -71,8 +69,10 @@ void kvm_arch_setup_debug(struct kvm_vcpu *vcpu) > /* Save pstate/mdscr */ > vcpu_debug_saved_reg(vcpu, pstate_ss_bit) = > *vcpu_cpsr(vcpu) & DBG_SPSR_SS; > - vcpu_debug_saved_reg(vcpu, mdscr_el1_bits) = > - vcpu_sys_reg(vcpu, MDSCR_EL1) & MDSCR_EL1_DEBUG_BITS; > + > + vcpu_debug_saved_reg(vcpu, mdscr_el1) = > + vcpu_sys_reg(vcpu, MDSCR_EL1); > + > /* > * Single Step (ARM ARM D2.12.3 The software step state > * machine) > @@ -161,9 +161,8 @@ void kvm_arch_clear_debug(struct kvm_vcpu *vcpu) > *vcpu_cpsr(vcpu) &= ~DBG_SPSR_SS; > *vcpu_cpsr(vcpu) |= vcpu_debug_saved_reg(vcpu, pstate_ss_bit); > > - vcpu_sys_reg(vcpu, MDSCR_EL1) &= ~MDSCR_EL1_DEBUG_BITS; > - vcpu_sys_reg(vcpu, MDSCR_EL1) |= > - vcpu_debug_saved_reg(vcpu, mdscr_el1_bits); > + vcpu_sys_reg(vcpu, MDSCR_EL1) = > + vcpu_debug_saved_reg(vcpu, mdscr_el1); > > /* > * If we were using HW debug we need to restore the > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > index be9b188..d43d7d1 100644 > --- a/arch/arm64/kvm/sys_regs.c > +++ b/arch/arm64/kvm/sys_regs.c > @@ -208,39 +208,61 @@ 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 & KVM_GUESTDBG_USE_HW_BP) { > - struct kvm_guest_debug_arch *saved; > - __u64 *val; > - > - saved = &vcpu_debug_saved_reg(vcpu, debug_regs); > - > - if (r->reg >= DBGBCR0_EL1 && r->reg <= DBGBCR15_EL1) > - val = &saved->dbg_bcr[r->reg - DBGBCR0_EL1]; > - else if (r->reg >= DBGBVR0_EL1 && r->reg <= DBGBVR15_EL1) > - val = &saved->dbg_bvr[r->reg - DBGBVR0_EL1]; > - else if (r->reg >= DBGWCR0_EL1 && r->reg <= DBGWCR15_EL1) > - val = &saved->dbg_wcr[r->reg - DBGWCR0_EL1]; > - else if (r->reg >= DBGWVR0_EL1 && r->reg <= DBGWVR15_EL1) > - val = &saved->dbg_wvr[r->reg - DBGWVR0_EL1]; > - else { > - kvm_err("Bad register index %d\n", r->reg); > - return false; > + if (vcpu->guest_debug) { > + > + /* MDSCR_EL1 */ > + if (r->reg == MDSCR_EL1) { > + if (p->is_write) > + vcpu_debug_saved_reg(vcpu, mdscr_el1) = > + *vcpu_reg(vcpu, p->Rt); > + else > + *vcpu_reg(vcpu, p->Rt) = > + vcpu_debug_saved_reg(vcpu, mdscr_el1); With this lines wrapping, {}'s might be nice. > + > + return true; > } > > - if (p->is_write) > - *val = *vcpu_reg(vcpu, p->Rt); > - else > - *vcpu_reg(vcpu, p->Rt) = *val; > + /* MDCCINT_EL1 */ > + if (r->reg == MDCCINT_EL1) > + goto old; "old"? As in the old way this worked? Someday (soon) all this code will be "old". How about just 'out'? Or use some other way to get the flow such that we avoid code duplication, but doesn't require a goto? > + > + /* We only shadow DBG* if guest being debugged */ > + if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP) { > + struct kvm_guest_debug_arch *saved; > + __u64 *val; > + > + saved = &vcpu_debug_saved_reg(vcpu, debug_regs); > + > + if (r->reg >= DBGBCR0_EL1 && r->reg <= DBGBCR15_EL1) > + val = &saved->dbg_bcr[r->reg - DBGBCR0_EL1]; > + else if (r->reg >= DBGBVR0_EL1 && r->reg <= DBGBVR15_EL1) > + val = &saved->dbg_bvr[r->reg - DBGBVR0_EL1]; > + else if (r->reg >= DBGWCR0_EL1 && r->reg <= DBGWCR15_EL1) > + val = &saved->dbg_wcr[r->reg - DBGWCR0_EL1]; > + else if (r->reg >= DBGWVR0_EL1 && r->reg <= DBGWVR15_EL1) > + val = &saved->dbg_wvr[r->reg - DBGWVR0_EL1]; > + else { > + kvm_err("Bad register index %d\n", r->reg); > + return false; > + } > > - } else { > - if (p->is_write) { > - vcpu_sys_reg(vcpu, r->reg) = *vcpu_reg(vcpu, p->Rt); > - vcpu->arch.debug_flags |= KVM_ARM64_DEBUG_DIRTY; > - } else { > - *vcpu_reg(vcpu, p->Rt) = vcpu_sys_reg(vcpu, r->reg); > + if (p->is_write) > + *val = *vcpu_reg(vcpu, p->Rt); > + else > + *vcpu_reg(vcpu, p->Rt) = *val; > + > + return true; > } > } > > +old: > + if (p->is_write) { > + vcpu_sys_reg(vcpu, r->reg) = *vcpu_reg(vcpu, p->Rt); > + vcpu->arch.debug_flags |= KVM_ARM64_DEBUG_DIRTY; > + } else { > + *vcpu_reg(vcpu, p->Rt) = vcpu_sys_reg(vcpu, r->reg); > + } > + > return true; > } > > -- > 2.3.4 >