From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48523) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aHZ2K-0001T9-P7 for qemu-devel@nongnu.org; Fri, 08 Jan 2016 10:33:09 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aHZ2G-0000hA-Lc for qemu-devel@nongnu.org; Fri, 08 Jan 2016 10:33:08 -0500 Received: from foss.arm.com ([217.140.101.70]:44907) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aHZ2G-0000gM-FW for qemu-devel@nongnu.org; Fri, 08 Jan 2016 10:33:04 -0500 Date: Fri, 8 Jan 2016 15:33:00 +0000 From: Will Deacon Message-ID: <20160108153300.GB11228@arm.com> References: <1452257667-7967-1-git-send-email-lorenzo.pieralisi@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1452257667-7967-1-git-send-email-lorenzo.pieralisi@arm.com> Subject: Re: [Qemu-devel] [PATCH] arm64: kernel: fix PMUv3 registers unconditional access List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Lorenzo Pieralisi Cc: Mark Rutland , Peter Maydell , qemu-devel@nongnu.org, linux-arm-kernel@lists.infradead.org Hi Lorenzo, On Fri, Jan 08, 2016 at 12:54:27PM +0000, Lorenzo Pieralisi wrote: > The Performance Monitors extension is an optional feature of the > AArch64 architecture, therefore, in order to access Performance > Monitors registers safely, the kernel should detect the PMUv3 unit > presence through the ID_AA64DFR0_EL1 register PMUVer field before > accessing them. > > This patch implements a guard by reading the ID_AA64DFR0_EL1 register > PMUVer field to detect the PMUv3 presence and prevent accessing PMUv3 > system registers if the Performance Monitors extension is not > implemented in the core. > > Signed-off-by: Lorenzo Pieralisi > Reported-by: Guenter Roeck > Cc: Will Deacon > Cc: Peter Maydell > Cc: Mark Rutland > --- > Based on arm64 for-next/perf branch. > > Tested on QEMU and Juno, I checked that the reported PMUVer field > is correct on both A57 and A53 (ie == 0x1), it should leave behaviour > unchanged on platforms implementing PMUv3. > > arch/arm64/kernel/head.S | 5 +++++ > arch/arm64/mm/proc-macros.S | 12 ++++++++++++ > arch/arm64/mm/proc.S | 4 ++-- > 3 files changed, 19 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S > index 23cfc08..6146fea 100644 > --- a/arch/arm64/kernel/head.S > +++ b/arch/arm64/kernel/head.S > @@ -512,9 +512,14 @@ CPU_LE( movk x0, #0x30d0, lsl #16 ) // Clear EE and E0E on LE systems > #endif > > /* EL2 debug */ > + mrs x0, id_aa64dfr0_el1 // Check ID_AA64DFR0_EL1 PMUVer > + ubfx x0, x0, #8, #4 > + cmp x0, #1 > + b.ne 4f // Skip if no PMUv3 present This will fail if and when PMUVer gets newer revisions of the PMU architecture (e.g. value 2 to indicate some extended PMU). It looks like we should be treating it as a signed 4-bit field, so we can use sbfx to extract a signed value and then we know the PMU is not present if the value is (signed) less than 1. Will