Re: [PATCH] arm64: mm: fix warning in arch_faults_on_old_pte()

[Will Deacon <will@kernel.org>]

On Tue, Jun 15, 2021 at 11:35:04AM +0100, Catalin Marinas wrote:
> On Mon, Jun 14, 2021 at 12:35:06PM -0600, Yu Zhao wrote:
> > On Mon, Jun 14, 2021 at 11:07 AM Catalin Marinas
> > <catalin.marinas@arm.com> wrote:
> > > On Sun, Jun 13, 2021 at 03:47:28PM -0600, Yu Zhao wrote:
> > > > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> > > > index efed2830d141..afdb6e0336ed 100644
> > > > --- a/arch/arm64/kernel/cpufeature.c
> > > > +++ b/arch/arm64/kernel/cpufeature.c
> > > > @@ -1566,6 +1566,14 @@ static bool has_hw_dbm(const struct arm64_cpu_capabilities *cap,
> > > >       return true;
> > > >  }
> > > >
> > > > +static void cpu_enable_hw_af(struct arm64_cpu_capabilities const *cap)
> > > > +{
> > > > +     if (has_cpuid_feature(cap, SCOPE_LOCAL_CPU)) {
> > > > +             u64 val = read_sysreg(tcr_el1);
> > > > +
> > > > +             write_sysreg(val | TCR_HA, tcr_el1);
> > > > +     }
> > > > +}
> > >
> > > This needs an isb(); local_flush_tlb_all() since this bit may be cached
> > > in the TLB. See how we did it in __cpu_enable_hw_dbm().
> > 
> > Thanks. I'll add it. (I omitted it since I saw it as a best effort,
> > not correctness related.)
> 
> The TLBs would eventually be flushed but a local TLBI during boot
> doesn't hurt performance, so I'd rather have it.
> 
> > > Alternatively you could leave the current TCR_AF bit setting as is (in
> > > proc.S) and only add an arm64_features[] entry for AF together with the
> > > arch_faults_on_old_pte() change but without any enable function.
> > >
> > > I'm not sure we have mixed CPUs where only some of them support AF to
> > > benefit from this.
> > 
> > Can we mix CPU versions? For example, can we have A75 (v8.2 with
> > AFDBM) + A53 (v8 w/o AFDBM) configuration? My understanding is that we
> > can't. But I have not been able to find any confirmation on arm.com.
> 
> Oh, don't underestimate the hardware vendors ;). But if they do, with
> this patch they just lose the AF benefits which I don't really mind
> (together with other 8.2 features which are not present in 8.0).

This feels like walking on thin ice to me... it's only a matter of time
before a big/little system is produced where AF is busted on one CPU type,
and then we'll have to undo all of this as the system cap will break late
CPU onlining if you boot on a CPU with working AF.

Given that arch_faults_on_old_pte() is purely a performance hint, why
don't we just set it based on the capabilities of the boot CPU instead?

Will

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