linux-kernel - Re: [PATCH v5 5/7] arm64: head: Init PMSCR_EL2.{PA,PCT} when entered at EL2 without VHE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20171002100301.GC20737@leverpostej>
Date:   Mon, 2 Oct 2017 11:03:01 +0100
From:   Mark Rutland <mark.rutland@....com>
To:     Will Deacon <will.deacon@....com>
Cc:     linux-arm-kernel@...ts.infradead.org, marc.zyngier@....com,
        kim.phillips@....com, tglx@...utronix.de, peterz@...radead.org,
        alexander.shishkin@...ux.intel.com, robh@...nel.org,
        suzuki.poulose@....com, pawel.moll@....com,
        mathieu.poirier@...aro.org, mingo@...hat.com,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 5/7] arm64: head: Init PMSCR_EL2.{PA,PCT} when entered
 at EL2 without VHE

On Thu, Sep 28, 2017 at 03:09:49PM +0100, Will Deacon wrote:
> When booting at EL2, ensure that we permit the EL1 host to sample
> physical addresses and physical counter values using SPE.
> 
> Signed-off-by: Will Deacon <will.deacon@....com>
> ---
>  arch/arm64/kernel/head.S | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> index 7434ec0c7a27..c370e270ae55 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -479,14 +479,21 @@ set_hcr:
>  
>  	/* Statistical profiling */
>  	ubfx	x0, x1, #32, #4			// Check ID_AA64DFR0_EL1 PMSVer
> -	cbz	x0, 6f				// Skip if SPE not present
> -	cbnz	x2, 5f				// VHE?
> +	cbz	x0, 7f				// Skip if SPE not present
> +	cbnz	x2, 6f				// VHE?
> +	mrs_s	x4, SYS_PMBIDR_EL1		// If SPE available at EL2,
> +	and	x4, x4, #(1 << SYS_PMBIDR_EL1_P_SHIFT)
> +	cbnz	x4, 5f				// then permit sampling of physical
> +	mov	x4, #(1 << SYS_PMSCR_EL2_PCT_SHIFT | \
> +		      1 << SYS_PMSCR_EL2_PA_SHIFT)
> +	msr_s	SYS_PMSCR_EL2, x4		// addresses and physical counter

As a general note, I find these right-hand-side comments difficult to
read, especially here with multiple line gaps mid-sentence.

If I'm alone in that view, there's no reason to change them, but
otherwise it might make sense to replace the more complex comments with
/* */ comment blocks on their own lines...

The logic looks fine to me, so FWIW:

Acked-by: Mark Rutland <mark.rutland@....com>

Mark.

> +5:
>  	mov	x1, #(MDCR_EL2_E2PB_MASK << MDCR_EL2_E2PB_SHIFT)
>  	orr	x3, x3, x1			// If we don't have VHE, then
> -	b	6f				// use EL1&0 translation.
> -5:						// For VHE, use EL2 translation
> +	b	7f				// use EL1&0 translation.
> +6:						// For VHE, use EL2 translation
>  	orr	x3, x3, #MDCR_EL2_TPMS		// and disable access from EL1
> -6:
> +7:
>  	msr	mdcr_el2, x3			// Configure debug traps
>  
>  	/* Stage-2 translation */
> -- 
> 2.1.4
>