[PATCH v2 1/2] arm64: Move handling of erratum 1418040 into C code
Marc Zyngier
maz at kernel.org
Fri Jul 31 07:14:40 EDT 2020
On 2020-07-31 11:41, Will Deacon wrote:
> On Fri, Jul 31, 2020 at 09:33:57AM +0100, Marc Zyngier wrote:
>> Instead of dealing with erratum 1418040 on each entry and exit,
>> let's move the handling to __switch_to() instead, which has
>> several advantages:
>>
>> - It can be applied when it matters (switching between 32 and 64
>> bit tasks).
>> - It is written in C (yay!)
>> - It can rely on static keys rather than alternatives
>>
>> Tested-by: Sai Prakash Ranjan <saiprakash.ranjan at codeaurora.org>
>> Signed-off-by: Marc Zyngier <maz at kernel.org>
>> ---
>> arch/arm64/kernel/entry.S | 21 ---------------------
>> arch/arm64/kernel/process.c | 35 +++++++++++++++++++++++++++++++++++
>> 2 files changed, 35 insertions(+), 21 deletions(-)
>
> [...]
>
>> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
>> index 6089638c7d43..8bbf066224ab 100644
>> --- a/arch/arm64/kernel/process.c
>> +++ b/arch/arm64/kernel/process.c
>> @@ -515,6 +515,40 @@ static void entry_task_switch(struct task_struct
>> *next)
>> __this_cpu_write(__entry_task, next);
>> }
>>
>> +/*
>> + * ARM erratum 1418040 handling, affecting the 32bit view of CNTVCT.
>> + * Assuming the virtual counter is enabled at the beginning of times:
>> + *
>> + * - disable access when switching from a 64bit task to a 32bit task
>> + * - enable access when switching from a 32bit task to a 64bit task
>> + */
>> +static __always_inline
>
> Do we need the __always_inline? None of the other calls from
> __switch_to()
> have it.
Suggestion from Stephen. In my experience, it doesn't change much as
most things get inlined anyway. Happy to drop it.
>
>> +void erratum_1418040_thread_switch(struct task_struct *prev,
>> + struct task_struct *next)
>> +{
>> + bool prev32, next32;
>> + u64 val;
>> +
>> + if (!(IS_ENABLED(CONFIG_ARM64_ERRATUM_1418040) &&
>> + cpus_have_const_cap(ARM64_WORKAROUND_1418040)))
>> + return;
>> +
>> + prev32 = is_compat_thread(task_thread_info(prev));
>> + next32 = is_compat_thread(task_thread_info(next));
>> +
>> + if (prev32 == next32)
>> + return;
>> +
>> + val = read_sysreg(cntkctl_el1);
>> +
>> + if (prev32 & !next32)
>
> I know they're bools but this is perverse!
Well, this is me writing this code, so don't attribute to perversity
what can adequately be explained by a silly typo... ;-)
> Why can't it just be:
>
> if (next32)
> val &= ~ARCH_TIMER_USR_VCT_ACCESS_EN;
> else
> val |= ARCH_TIMER_USR_VCT_ACCESS_EN;
>
> ?
Yup, that's better. Do you want to apply these changes directly,
or should I repin it?
M.
--
Jazz is not dead. It just smells funny...
More information about the linux-arm-kernel
mailing list