[External] Re: [PATCH 1/2] riscv: track effective hardware PTE A/D updates

[yunhui cui]

Hi Samuel,

On Wed, May 20, 2026 at 1:31 AM Samuel Holland
<samuel.holland at sifive.com> wrote:
>
> Hi Yunhui,
>
> On 2026-05-18 10:19 PM, Yunhui Cui wrote:
> > Separate Svadu capability discovery from the host's effective ADUE
> > state. Enable SBI FWFT PTE A/D hardware updating on each online CPU
> > through CPUHP when both Svade and Svadu are present, use the resulting
> > runtime state for arch_has_hw_pte_young(), and fall back to
> > software-managed A/D updates when enabling the feature fails.
> >
> > Platforms with Svadu but without Svade are treated as always using
> > hardware PTE A/D updates. Expose the runtime state through an inline
> > getter so hot MM paths avoid an out-of-line function call.
> >
> > Signed-off-by: Yunhui Cui <cuiyunhui at bytedance.com>
> > Reviewed-by: Qingwei Hu <qingwei.hu at bytedance.com>
> > ---
> >  arch/riscv/include/asm/cpufeature.h |  6 +++
> >  arch/riscv/include/asm/pgtable.h    |  8 ++--
> >  arch/riscv/kernel/cpufeature.c      | 73 ++++++++++++++++++++++++++---
> >  3 files changed, 77 insertions(+), 10 deletions(-)
> >
> > diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h
> > index 739fcc84bf7b2..877d71a1ea755 100644
> > --- a/arch/riscv/include/asm/cpufeature.h
> > +++ b/arch/riscv/include/asm/cpufeature.h
> > @@ -128,6 +128,12 @@ struct riscv_isa_ext_data {
> >  extern const struct riscv_isa_ext_data riscv_isa_ext[];
> >  extern const size_t riscv_isa_ext_count;
> >  extern bool riscv_isa_fallback;
> > +extern bool riscv_hw_pte_ad_updating_enabled;
> > +
> > +static __always_inline bool riscv_has_hw_pte_ad_updating(void)
> > +{
> > +     return READ_ONCE(riscv_hw_pte_ad_updating_enabled);
> > +}
>
> Should this use a static key, since it's only updated at boot, and you mention
> it is used in MM hot paths?

Okay, I will update in the next patch version.

>
> >
> >  unsigned long riscv_isa_extension_base(const unsigned long *isa_bitmap);
> >  static __always_inline bool riscv_cpu_has_extension_likely(int cpu, const unsigned long ext)
> > diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
> > index a1a7c6520a095..20663a466cf6c 100644
> > --- a/arch/riscv/include/asm/pgtable.h
> > +++ b/arch/riscv/include/asm/pgtable.h
> > @@ -732,14 +732,14 @@ static inline pgprot_t pgprot_writecombine(pgprot_t _prot)
> >  #define pgprot_dmacoherent pgprot_writecombine
> >
> >  /*
> > - * Both Svade and Svadu control the hardware behavior when the PTE A/D bits need to be set. By
> > - * default the M-mode firmware enables the hardware updating scheme when only Svadu is present in
> > - * DT.
> > + * Both Svade and Svadu control the hardware behavior when the PTE A/D bits
> > + * need to be set. The core MM code only cares whether hardware updating of
> > + * the accessed/dirty state is currently active.
> >   */
> >  #define arch_has_hw_pte_young arch_has_hw_pte_young
> >  static inline bool arch_has_hw_pte_young(void)
> >  {
> > -     return riscv_has_extension_unlikely(RISCV_ISA_EXT_SVADU);
> > +     return riscv_has_hw_pte_ad_updating();
> >  }
> >
> >  /*
> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > index f46aa5602d74d..e46b2d2b49eed 100644
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
> > @@ -35,6 +35,8 @@
> >  static bool any_cpu_has_zicboz;
> >  static bool any_cpu_has_zicbop;
> >  static bool any_cpu_has_zicbom;
> > +bool riscv_hw_pte_ad_updating_enabled __read_mostly;
> > +EXPORT_SYMBOL_GPL(riscv_hw_pte_ad_updating_enabled);
> >
> >  unsigned long elf_hwcap __read_mostly;
> >
> > @@ -287,15 +289,74 @@ static int riscv_ext_zvfbfwma_validate(const struct riscv_isa_ext_data *data,
> >       return -EPROBE_DEFER;
> >  }
> >
> > -static int riscv_ext_svadu_validate(const struct riscv_isa_ext_data *data,
> > -                                 const unsigned long *isa_bitmap)
> > +static void riscv_set_hw_pte_ad_updating(bool enabled)
> > +{
> > +     WRITE_ONCE(riscv_hw_pte_ad_updating_enabled, enabled);
> > +}
> > +
> > +static int riscv_hw_pte_ad_updating_starting(unsigned int cpu)
> > +{
> > +     int ret;
> > +
> > +     ret = sbi_fwft_set(SBI_FWFT_PTE_AD_HW_UPDATING, 1, 0);
> > +     if (ret) {
> > +             if (ret != -EOPNOTSUPP)
> > +                     pr_err("CPU%u failed to enable hardware PTE A/D updating: %d\n",
> > +                            cpu, ret);
> > +             return ret;
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static int riscv_hw_pte_ad_updating_dying(unsigned int cpu)
> >  {
> > -     /* SVADE has already been detected, use SVADE only */
> > -     if (__riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_SVADE))
> > -             return -EOPNOTSUPP;
> > +     int ret;
> > +
> > +     ret = sbi_fwft_set(SBI_FWFT_PTE_AD_HW_UPDATING, 0, 0);
> > +     if (ret)
> > +             pr_warn("CPU%u failed to disable hardware PTE A/D updating: %d\n",
> > +                     cpu, ret);
>
> Why bother disabling the feature when taking a CPU offline? It doesn't create
> any problems to leave it enabled.

Okay, I will update in the next patch version.

>
> Regards,
> Samuel
>
> > +
> > +     return 0;
> > +}
> >
> > +static int __init riscv_hw_pte_ad_updating_init(void)
> > +{
> > +     bool has_svade, has_svadu;
> > +     int state;
> > +
> > +     has_svade = riscv_has_extension_unlikely(RISCV_ISA_EXT_SVADE);
> > +     has_svadu = riscv_has_extension_unlikely(RISCV_ISA_EXT_SVADU);
> > +
> > +     if (!has_svadu)
> > +             return 0;
> > +
> > +     if (!has_svade) {
> > +             riscv_set_hw_pte_ad_updating(true);
> > +             pr_info("riscv: hardware PTE A/D updating enabled\n");
> > +             return 0;
> > +     }
> > +
> > +     state = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
> > +                               "riscv/pte-ad:starting",
> > +                               riscv_hw_pte_ad_updating_starting,
> > +                               riscv_hw_pte_ad_updating_dying);
> > +     if (state < 0) {
> > +             pr_info("riscv: leave PTE A/D updates software-managed (%d)\n",
> > +                     state);
> > +             return 0;
> > +     }
> > +
> > +     /*
> > +      * A successful CPUHP_AP_ONLINE_DYN registration means the startup
> > +      * callback has already succeeded on all online CPUs.
> > +      */
> > +     riscv_set_hw_pte_ad_updating(true);
> > +     pr_info("riscv: hardware PTE A/D updating enabled\n");
> >       return 0;
> >  }
> > +arch_initcall(riscv_hw_pte_ad_updating_init);
> >
> >  static int riscv_cfilp_validate(const struct riscv_isa_ext_data *data,
> >                               const unsigned long *isa_bitmap)
> > @@ -584,7 +645,7 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
> >       __RISCV_ISA_EXT_SUPERSET(ssnpm, RISCV_ISA_EXT_SSNPM, riscv_xlinuxenvcfg_exts),
> >       __RISCV_ISA_EXT_DATA(sstc, RISCV_ISA_EXT_SSTC),
> >       __RISCV_ISA_EXT_DATA(svade, RISCV_ISA_EXT_SVADE),
> > -     __RISCV_ISA_EXT_DATA_VALIDATE(svadu, RISCV_ISA_EXT_SVADU, riscv_ext_svadu_validate),
> > +     __RISCV_ISA_EXT_DATA(svadu, RISCV_ISA_EXT_SVADU),
> >       __RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL),
> >       __RISCV_ISA_EXT_DATA(svnapot, RISCV_ISA_EXT_SVNAPOT),
> >       __RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
>

Thanks,
Yunhui