From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755588AbcBHQwU (ORCPT ); Mon, 8 Feb 2016 11:52:20 -0500 Received: from foss.arm.com ([217.140.101.70]:34458 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753573AbcBHQwT (ORCPT ); Mon, 8 Feb 2016 11:52:19 -0500 Date: Mon, 8 Feb 2016 16:52:15 +0000 From: Catalin Marinas To: Marc Zyngier Cc: kvm@vger.kernel.org, Will Deacon , linux-kernel@vger.kernel.org, Christoffer Dall , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v3 21/23] arm64: hw_breakpoint: Allow EL2 breakpoints if running in HYP Message-ID: <20160208165214.GV6076@e104818-lin.cambridge.arm.com> References: <1454522416-6874-1-git-send-email-marc.zyngier@arm.com> <1454522416-6874-22-git-send-email-marc.zyngier@arm.com> <20160208155625.GQ6076@e104818-lin.cambridge.arm.com> <56B8C638.2010402@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <56B8C638.2010402@arm.com> 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 Mon, Feb 08, 2016 at 04:45:44PM +0000, Marc Zyngier wrote: > I was being overzealous, and your solution is clearly better. I ended up with the following: > > diff --git a/arch/arm64/include/asm/hw_breakpoint.h b/arch/arm64/include/asm/hw_breakpoint.h > index 9732908..c872b2f 100644 > --- a/arch/arm64/include/asm/hw_breakpoint.h > +++ b/arch/arm64/include/asm/hw_breakpoint.h [...] > @@ -62,6 +45,7 @@ static inline void decode_ctrl_reg(u32 reg, > #define AARCH64_ESR_ACCESS_MASK (1 << 6) > > /* Privilege Levels */ > +#define AARCH64_BREAKPOINT_EL2 0 > #define AARCH64_BREAKPOINT_EL1 1 > #define AARCH64_BREAKPOINT_EL0 2 > > @@ -76,6 +60,35 @@ static inline void decode_ctrl_reg(u32 reg, > #define ARM_KERNEL_STEP_ACTIVE 1 > #define ARM_KERNEL_STEP_SUSPEND 2 > > +#define DBG_HMC_HYP (1 << 13) > +#define DBG_SSC_HYP (3 << 14) > + > +static inline u32 encode_ctrl_reg(struct arch_hw_breakpoint_ctrl ctrl) > +{ > + u32 val = (ctrl.len << 5) | (ctrl.type << 3) | ctrl.enabled; > + > + if (is_kernel_in_hyp_mode() && ctrl.privilege == AARCH64_BREAKPOINT_EL1) > + val |= DBG_HMC_HYP | DBG_SSC_HYP; Nitpick, for completeness: val |= DBG_HMC_HYP | DBG_SSC_HYP | (AARCH64_BREAKPOINT_EL2 << 1); > + else > + val |= ctrl.privilege << 1; > + > + return val; > +} > + > +static inline void decode_ctrl_reg(u32 reg, > + struct arch_hw_breakpoint_ctrl *ctrl) > +{ > + ctrl->enabled = reg & 0x1; > + reg >>= 1; > + ctrl->privilege = reg & 0x3; > + if (ctrl->privilege == AARCH64_BREAKPOINT_EL2) > + ctrl->privilege = AARCH64_BREAKPOINT_EL1; > + reg >>= 2; > + ctrl->type = reg & 0x3; > + reg >>= 2; > + ctrl->len = reg & 0xff; > +} > + > /* > * Limits. > * Changing these will require modifications to the register accessors. > > Was that what you had in mind? Looks fine: Acked-by: Catalin Marinas