Re: [Qemu-devel] [PATCH] arm64: kernel: fix PMUv3 registers unconditional access

[Will Deacon]

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 <address@hidden>
> Reported-by: Guenter Roeck <address@hidden>
> Cc: Will Deacon <address@hidden>
> Cc: Peter Maydell <address@hidden>
> Cc: Mark Rutland <address@hidden>
> ---
> 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