Re: [PATCH 1/4] arm64: Restrict ARM64_ERRATUM_1188873 mitigation to AArch32

From: Will Deacon <will.deacon@arm.com>
To: Marc Zyngier <marc.zyngier@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/4] arm64: Restrict ARM64_ERRATUM_1188873 mitigation to AArch32
Date: Fri, 12 Apr 2019 14:17:50 +0100	[thread overview]
Message-ID: <20190412131750.GC29218@fuggles.cambridge.arm.com> (raw)
In-Reply-To: <20190408160216.223871-2-marc.zyngier@arm.com>

Hi Marc,

On Mon, Apr 08, 2019 at 05:02:13PM +0100, Marc Zyngier wrote:
> We currently deal with ARM64_ERRATUM_1188873 by always trapping EL0
> accesses for both instruction sets. Although nothing wrong comes out
> of that, people trying to squeeze the last drop of performance from
> buggy HW find this over the top. Oh well.
> 
> Let's change the mitigation by flipping the counter enable bit
> on return to userspace. Non-broken HW gets an extra branch on
> the fast path, which is hopefully not the end of the world.
> The arch timer workaround it also removed.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm64/kernel/entry.S            | 23 +++++++++++++++++++++++
>  drivers/clocksource/arm_arch_timer.c | 15 ---------------
>  2 files changed, 23 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index c50a7a75f2e0..e0aed21ab402 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -336,6 +336,29 @@ alternative_if ARM64_WORKAROUND_845719
>  alternative_else_nop_endif
>  #endif
>  3:
> +#ifdef CONFIG_ARM64_ERRATUM_1188873
> +alternative_if_not ARM64_WORKAROUND_1188873
> +	b	1f
> +alternative_else_nop_endif
> +
> +	ubfx	x0, x22, #4, #1			// Extract PSR_MODE32
> +	eor	x0, x0, #1			// Negate it
> +alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
> +	mrs	x1, cntkctl_el1
> +alternative_else
> +	mrs	x1, cnthctl_el2
> +alternative_endif
> +	ubfx	x2, x1, #1, #1			// Extract EL0VCTEN
> +	cmp	x2, x0

Aren't the flags live at this point (they indicate native vs compat)?
Maybe you can use that instead of re-extracting PSR_MODE32.

> +	b.eq	1f				// Matches, nothing to do
> +	bfi	x1, x0, #1, #1			// Move EL0VCTEN in place

ARCH_TIMER_USR_VCT_ACCESS_EN

> +alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
> +	msr	cntkctl_el1, x1
> +alternative_else
> +	msr	cnthctl_el2, x1
> +alternative_endif
> +1:

Sorry to be a pain, but could you use label '4:' here and update the others
in this macro, please?

> +#endif
>  	apply_ssbd 0, x0, x1
>  	.endif
>  
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index 5fcccc467868..27acc9eb0f7c 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c

I probably need an Ack from Thomas or Daniel on these parts, so I can take
the series via arm64.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel