[External] Re: [PATCH v2 1/2] riscv: track effective hardware PTE A/D updates
yunhui cui
cuiyunhui at bytedance.com
Mon Jun 8 23:38:16 PDT 2026
Hi Andrew,
On Mon, May 25, 2026 at 4:44 AM Andrew Jones
<andrew.jones at oss.qualcomm.com> wrote:
>
> On Sat, May 23, 2026 at 11:58:05AM +0800, yunhui cui wrote:
> > Hi Andrew,
> >
> > On Sat, May 23, 2026 at 3:58 AM Andrew Jones
> > <andrew.jones at oss.qualcomm.com> wrote:
> > >
> > > On Fri, May 22, 2026 at 10:23:57PM +0800, Yunhui Cui wrote:
> > > > Track the effective hardware PTE A/D update mode separately from
> > > > Svadu capability discovery. When both Svade and Svadu are present,
> > > > enable SBI FWFT PTE A/D hardware updating on each online CPU via
> > > > CPUHP and use the resulting runtime state for arch_has_hw_pte_young().
> > > > Fall back to software-managed A/D updates if enabling hardware updates
> > > > fails.
> > > >
> > > > When Svadu is present without Svade, assume hardware PTE A/D updating
> > > > is enabled from boot, and document that boot-time behavior in the DT
> > > > binding.
> > > >
> > > > Signed-off-by: Yunhui Cui <cuiyunhui at bytedance.com>
> > > > Reviewed-by: Qingwei Hu <qingwei.hu at bytedance.com>
> > > > ---
> > > > .../devicetree/bindings/riscv/extensions.yaml | 6 +-
> > > > arch/riscv/include/asm/cpufeature.h | 6 ++
> > > > arch/riscv/include/asm/pgtable.h | 8 +--
> > > > arch/riscv/kernel/cpufeature.c | 58 +++++++++++++++++--
> > > > 4 files changed, 65 insertions(+), 13 deletions(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml
> > > > index 2b0a8a93bb214..b09888e9988de 100644
> > > > --- a/Documentation/devicetree/bindings/riscv/extensions.yaml
> > > > +++ b/Documentation/devicetree/bindings/riscv/extensions.yaml
> > > > @@ -294,10 +294,10 @@ properties:
> > > > of the PTE A/D bits or page faults when they need updated.
> > > > 2) Only Svade present in DT => Supervisor must assume Svade to be
> > > > always enabled.
> > > > - 3) Only Svadu present in DT => Supervisor must assume Svadu to be
> > > > - always enabled.
> > > > + 3) Only Svadu present in DT => Supervisor must assume Svadu is
> > > > + enabled at boot.
> > > > 4) Both Svade and Svadu present in DT => Supervisor must assume
> > > > - Svadu turned-off at boot time. To use Svadu, supervisor must
> > > > + Svadu is disabled at boot time. To use Svadu, supervisor must
> > > > explicitly enable it using the SBI FWFT extension.
> > >
> > > Looks good to me, but I think dt binding changes are typically in separate
> > > patches.
> >
> > Okay, I'll split the DT binding update into a separate patch.
>
> Make sure the appropriate lists and maintainers are CC'ed too.
>
> >
> > >
> > > >
> > > > - const: svadu
> > > > diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h
> > > > index 739fcc84bf7b2..ad9fad6eee55d 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;
> > > > +DECLARE_STATIC_KEY_FALSE(riscv_hw_pte_ad_updating);
> > > > +
> > > > +static __always_inline bool riscv_has_hw_pte_ad_updating(void)
> > > > +{
> > > > + return static_branch_unlikely(&riscv_hw_pte_ad_updating);
> > > > +}
> > > >
> > > > 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..831dd6a7c1a06 100644
> > > > --- a/arch/riscv/kernel/cpufeature.c
> > > > +++ b/arch/riscv/kernel/cpufeature.c
> > > > @@ -35,6 +35,7 @@
> > > > static bool any_cpu_has_zicboz;
> > > > static bool any_cpu_has_zicbop;
> > > > static bool any_cpu_has_zicbom;
> > > > +DEFINE_STATIC_KEY_FALSE(riscv_hw_pte_ad_updating);
> > > >
> > > > unsigned long elf_hwcap __read_mostly;
> > > >
> > > > @@ -287,15 +288,60 @@ 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(void)
> > > > +{
> > > > + static_branch_enable(&riscv_hw_pte_ad_updating);
> > > > +}
> > > > +
> > > > +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 __init riscv_hw_pte_ad_updating_init(void)
> > > > {
> > > > - /* SVADE has already been detected, use SVADE only */
> > > > - if (__riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_SVADE))
> > > > - return -EOPNOTSUPP;
> > > > + 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)
> > > > + goto enable;
> > > > +
> > > > + state = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
> > >
> > > I don't think CPUHP_AP_ONLINE_DYN is correct. Setting HW A/D is something
> > > that should be done at _STARTING time.
> >
> > I don't think moving this to _STARTING is the right fix, because this
> > callback can fail, while _STARTING callbacks are not supposed to fail.
> >
> > CPUHP_AP_ONLINE_DYN already covers both the CPUs that are online when
> > cpuhp_setup_state() is called and CPUs brought online later, e.g. with
> > maxcpus=. If FWFT setup fails for a later CPU, only that CPU's online
> > operation is aborted.
>
> The problem is that the later hotplugged harts will see the global static
> key so software won't update A/D bits and HW A/D updating won't be active
> on that hart either until after CPUHP_AP_ONLINE_DYN - risking unexpected
> exceptions. Since _STARTING callbacks can't abort hotplug and since we
> really want HW A/D active before the hart takes its first page fault, then
> maybe this needs to be done at secondary startup time.
Agreed. I'll drop the CPUHP_AP_ONLINE_DYN setup in the next version.
Instead, I plan to enable SBI_FWFT_PTE_AD_HW_UPDATING on all currently
online harts during init, before enabling the global
riscv_hw_pte_ad_updating static key. Later secondary harts will enable
the same FWFT feature at the beginning of smp_callin(), before
notify_cpu_starting() and before they are marked online.
If the init-time FWFT setup fails, the static key will remain disabled and
the kernel will fall back to software-managed A/D updates. If a later
secondary hart fails after the static key has already been enabled, that
hart bringup will be aborted. I'll also add rollback for the init-time
partial failure path.
This should remove the window where a hotplugged hart observes the global
static key but has not enabled local FWFT yet.
>
> Thanks,
> drew
Thanks,
Yunhui
More information about the linux-riscv
mailing list