[PATCH v2 1/2] arm64: Move handling of erratum 1418040 into C code
Will Deacon
will at kernel.org
Fri Jul 31 07:32:28 EDT 2020
On Fri, Jul 31, 2020 at 12:14:40PM +0100, Marc Zyngier wrote:
> On 2020-07-31 11:41, Will Deacon wrote:
> > On Fri, Jul 31, 2020 at 09:33:57AM +0100, Marc Zyngier wrote:
> > > 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.
Yes, please. We can add it back if it's shown to be a problem.
> > > +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?
If you don't mind respinning, that would be best please. You can add my Ack
as well, as I think this is one for Catalin now (I don't know if we're
getting an -rc8 or not).
Will
More information about the linux-arm-kernel
mailing list