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

[Daniel Lezcano <daniel.lezcano@linaro.org>]

On 15/04/2019 10:15, Marc Zyngier wrote:
> On 12/04/2019 14:17, Will Deacon wrote:
>> 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>

Hi Marc,

for the arm_arch_timer.c part:

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>


>>> ---
>>>  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.
> 
> Robin and I have been reworking this, see other emails in the same
> thread, and the code looks pretty different now (and not breaking things
> anymore).
> 
>>
>>> +	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?
> 
> Sure, no problem.
> 
>>
>>> +#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.
> 
> Daniel is on Cc, and hopefully v2 will be OK.


-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog


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