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

[Yu Zhao]

On Mon, Jun 14, 2021 at 11:07 AM Catalin Marinas
<catalin.marinas at arm.com> wrote:
>
> On Sun, Jun 13, 2021 at 03:47:28PM -0600, Yu Zhao wrote:
> > cow_user_page() doesn't disable preemption, and it triggers the
> > warning in arch_faults_on_old_pte() when CONFIG_PREEMPT_COUNT=y.
>
> I'm surprised we haven't seen this warning until now. We probably
> haven't exercised this path with the right config options.

Yeah, we only started hitting it recently -- it requires PFN-mapped
PTEs, e.g., DAX in addition to CONFIG_PREEMPT_COUNT=y. I'll include
this information in the next version.

> > Converting the Access flag support to a system-wide feature to avoid
> > reading ID_AA64MMFR1_EL1 on local CPUs when determining the h/w cap.
> >
> > Note that though the Access flag support is a non-conflicting feature,
> > we require all late CPUs to have it if the boot CPU does. Otherwise
> > the feature won't be enabled regardless of the capabilities of late
> > CPUs.
> >
> > If there are h/w implementations that break this rule, they will have
> > to add errata, unless they can provide justifications to switch to the
> > less strict ARM64_CPUCAP_WEAK_LOCAL_CPU_FEATURE.
> >
> > Signed-off-by: Yu Zhao <yuzhao at google.com>
>
> I'm ok in principle but one small issue below.
>
> > 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.)

> 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.