* [PATCH 1/2] riscv: track effective hardware PTE A/D updates
@ 2026-05-19 3:19 Yunhui Cui
2026-05-19 3:19 ` [PATCH 2/2] riscv: merge hardware A/D updates in PTE accessors Yunhui Cui
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Yunhui Cui @ 2026-05-19 3:19 UTC (permalink / raw)
To: pjw, palmer, aou, alex, akpm, pasha.tatashin, andrew+kernel,
rmclure, debug, baolin.wang, zhangchunyan, cuiyunhui, apopple,
namcao, wangruikang, apatel, liu.xuemei1, ajones, cleger,
charlie, hui.wang, guodong, pincheng.plct, linux-riscv,
linux-kernel
Cc: Qingwei Hu
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@bytedance.com>
Reviewed-by: Qingwei Hu <qingwei.hu@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);
+}
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);
+
+ 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),
--
2.39.5
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] riscv: merge hardware A/D updates in PTE accessors
2026-05-19 3:19 [PATCH 1/2] riscv: track effective hardware PTE A/D updates Yunhui Cui
@ 2026-05-19 3:19 ` Yunhui Cui
2026-05-19 12:05 ` [PATCH 1/2] riscv: track effective hardware PTE A/D updates Michael Ellerman
` (2 subsequent siblings)
3 siblings, 0 replies; 11+ messages in thread
From: Yunhui Cui @ 2026-05-19 3:19 UTC (permalink / raw)
To: pjw, palmer, aou, alex, akpm, pasha.tatashin, andrew+kernel,
rmclure, debug, baolin.wang, zhangchunyan, cuiyunhui, apopple,
namcao, wangruikang, apatel, liu.xuemei1, ajones, cleger,
charlie, hui.wang, guodong, pincheng.plct, linux-riscv,
linux-kernel
Cc: Qingwei Hu
Use cmpxchg-based merges for live RISC-V PTE permission updates so
software changes do not lose concurrently hardware-updated accessed and
dirty state. Cover ptep_set_access_flags(),
ptep_test_and_clear_young(), and ptep_set_wrprotect(), and extend the
same wrprotect handling to the PUD leaf helper used by huge mappings.
Keep the existing Svvptc flush behaviour, but only flush when the
merged PTE value actually changed.
Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
Reviewed-by: Qingwei Hu <qingwei.hu@bytedance.com>
---
arch/riscv/include/asm/pgtable.h | 19 +++++++--
arch/riscv/mm/pgtable.c | 68 ++++++++++++++++++++++++++------
2 files changed, 73 insertions(+), 14 deletions(-)
diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
index 20663a466cf6c..984c37ca8aef7 100644
--- a/arch/riscv/include/asm/pgtable.h
+++ b/arch/riscv/include/asm/pgtable.h
@@ -668,15 +668,21 @@ static inline pte_t ptep_get_and_clear(struct mm_struct *mm,
static inline void ptep_set_wrprotect(struct mm_struct *mm,
unsigned long address, pte_t *ptep)
{
- pte_t read_pte = READ_ONCE(*ptep);
+ pte_t old_pte;
+ pte_t pte;
/*
* ptep_set_wrprotect can be called for shadow stack ranges too.
* shadow stack memory is XWR = 010 and thus clearing _PAGE_WRITE will lead to
* encoding 000b which is wrong encoding with V = 1. This should lead to page fault
* but we dont want this wrong configuration to be set in page tables.
*/
- atomic_long_set((atomic_long_t *)ptep,
- ((pte_val(read_pte) & ~(unsigned long)_PAGE_WRITE) | _PAGE_READ));
+ pte = READ_ONCE(*ptep);
+ do {
+ old_pte = pte;
+ pte = pte_wrprotect(pte);
+ pte_val(pte) = cmpxchg_relaxed(&pte_val(*ptep), pte_val(old_pte),
+ pte_val(pte));
+ } while (pte_val(pte) != pte_val(old_pte));
}
#define __HAVE_ARCH_PTEP_CLEAR_YOUNG_FLUSH
@@ -1030,6 +1036,13 @@ static inline void pmdp_set_wrprotect(struct mm_struct *mm,
ptep_set_wrprotect(mm, address, (pte_t *)pmdp);
}
+#define __HAVE_ARCH_PUDP_SET_WRPROTECT
+static inline void pudp_set_wrprotect(struct mm_struct *mm,
+ unsigned long address, pud_t *pudp)
+{
+ ptep_set_wrprotect(mm, address, (pte_t *)pudp);
+}
+
#define pmdp_establish pmdp_establish
static inline pmd_t pmdp_establish(struct vm_area_struct *vma,
unsigned long address, pmd_t *pmdp, pmd_t pmd)
diff --git a/arch/riscv/mm/pgtable.c b/arch/riscv/mm/pgtable.c
index 9c4427d0b1874..b77e82362442e 100644
--- a/arch/riscv/mm/pgtable.c
+++ b/arch/riscv/mm/pgtable.c
@@ -5,23 +5,55 @@
#include <linux/kernel.h>
#include <linux/pgtable.h>
+#define RISCV_PTE_ACCESS_FLAG_MASK (_PAGE_READ | _PAGE_WRITE | _PAGE_EXEC | \
+ _PAGE_ACCESSED | _PAGE_DIRTY | \
+ _PAGE_SOFT_DIRTY)
+
+static inline unsigned long riscv_pte_access_flags(unsigned long cur,
+ unsigned long entry)
+{
+ unsigned long pteval;
+ unsigned long hw_flags;
+
+ hw_flags = _PAGE_ACCESSED | _PAGE_DIRTY | _PAGE_SOFT_DIRTY;
+ pteval = cur & ~RISCV_PTE_ACCESS_FLAG_MASK;
+ pteval |= entry & (RISCV_PTE_ACCESS_FLAG_MASK & ~hw_flags);
+ pteval |= (cur | entry) & hw_flags;
+
+ return pteval;
+}
+
int ptep_set_access_flags(struct vm_area_struct *vma,
unsigned long address, pte_t *ptep,
pte_t entry, int dirty)
{
+ unsigned long old_pteval;
+ unsigned long new_pteval;
+ unsigned long prev_pteval;
+ bool changed;
+
+ old_pteval = pte_val(ptep_get(ptep));
+ do {
+ new_pteval = riscv_pte_access_flags(old_pteval, pte_val(entry));
+ if (new_pteval == old_pteval)
+ break;
+
+ prev_pteval = cmpxchg_relaxed(&pte_val(*ptep), old_pteval,
+ new_pteval);
+ if (prev_pteval == old_pteval)
+ break;
+
+ old_pteval = prev_pteval;
+ } while (1);
+
+ changed = old_pteval != new_pteval;
if (riscv_has_extension_unlikely(RISCV_ISA_EXT_SVVPTC)) {
- if (!pte_same(ptep_get(ptep), entry)) {
- __set_pte_at(vma->vm_mm, ptep, entry);
- /* Here only not svadu is impacted */
+ if (changed)
flush_tlb_page(vma, address);
- return true;
- }
- return false;
+ return changed;
}
- if (!pte_same(ptep_get(ptep), entry))
- __set_pte_at(vma->vm_mm, ptep, entry);
/*
* update_mmu_cache will unconditionally execute, handling both
* the case that the PTE changed and the spurious fault case.
@@ -32,9 +64,23 @@ int ptep_set_access_flags(struct vm_area_struct *vma,
bool ptep_test_and_clear_young(struct vm_area_struct *vma,
unsigned long address, pte_t *ptep)
{
- if (!pte_young(ptep_get(ptep)))
- return false;
- return test_and_clear_bit(_PAGE_ACCESSED_OFFSET, &pte_val(*ptep));
+ unsigned long old_pteval;
+ unsigned long new_pteval;
+ unsigned long prev_pteval;
+
+ old_pteval = pte_val(ptep_get(ptep));
+ do {
+ if (!(old_pteval & _PAGE_ACCESSED))
+ return false;
+
+ new_pteval = pte_val(pte_mkold(__pte(old_pteval)));
+ prev_pteval = cmpxchg_relaxed(&pte_val(*ptep), old_pteval,
+ new_pteval);
+ if (prev_pteval == old_pteval)
+ return true;
+
+ old_pteval = prev_pteval;
+ } while (1);
}
EXPORT_SYMBOL_GPL(ptep_test_and_clear_young);
--
2.39.5
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] riscv: track effective hardware PTE A/D updates
2026-05-19 3:19 [PATCH 1/2] riscv: track effective hardware PTE A/D updates Yunhui Cui
2026-05-19 3:19 ` [PATCH 2/2] riscv: merge hardware A/D updates in PTE accessors Yunhui Cui
@ 2026-05-19 12:05 ` Michael Ellerman
2026-05-19 15:37 ` Conor Dooley
2026-05-22 7:28 ` [External] " yunhui cui
2026-05-19 17:31 ` Samuel Holland
2026-05-19 19:58 ` Andrew Jones
3 siblings, 2 replies; 11+ messages in thread
From: Michael Ellerman @ 2026-05-19 12:05 UTC (permalink / raw)
To: Yunhui Cui, pjw, palmer, aou, alex, akpm, pasha.tatashin,
andrew+kernel, rmclure, debug, baolin.wang, zhangchunyan,
apopple, namcao, wangruikang, apatel, liu.xuemei1, ajones,
cleger, charlie, hui.wang, guodong, pincheng.plct, linux-riscv,
linux-kernel
Cc: Qingwei Hu
On 19/5/2026 13:19, 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.
I'm not sure what you mean here. The current code doesn't use an
out-of-line function call AFAICS? More comments below ...
> Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> Reviewed-by: Qingwei Hu <qingwei.hu@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);
> +}
>
> 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();
> }
riscv_has_extension_unlikely() uses an asm alternative, ie. it's patched
at boot so there's no runtime cost. But now you've changed it to just
test a bool.
I'm not sure arch_has_hw_pte_young() is a particularly hot path, but
seems like you could use a static key, so that the code is patched to
avoid the runtime test?
> 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,
...
>
> +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;
This block is identical to the tail of the function. I'd probably use
"goto enable", with an "enable" label below.
> + }
> +
> + 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.
> + */
enable:
> + riscv_set_hw_pte_ad_updating(true);
> + pr_info("riscv: hardware PTE A/D updating enabled\n");
pr_info() might be a bit verbose for this. I think printk(KERN_DEBUG ..)
would be better, that way the message is always available in dmesg but
isn't sent to the console
> return 0;
> }
> +arch_initcall(riscv_hw_pte_ad_updating_init);
cheers
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] riscv: track effective hardware PTE A/D updates
2026-05-19 12:05 ` [PATCH 1/2] riscv: track effective hardware PTE A/D updates Michael Ellerman
@ 2026-05-19 15:37 ` Conor Dooley
2026-05-19 17:34 ` Samuel Holland
2026-05-22 7:28 ` [External] " yunhui cui
1 sibling, 1 reply; 11+ messages in thread
From: Conor Dooley @ 2026-05-19 15:37 UTC (permalink / raw)
To: Michael Ellerman
Cc: Yunhui Cui, pjw, palmer, aou, alex, akpm, pasha.tatashin,
andrew+kernel, rmclure, debug, baolin.wang, zhangchunyan,
apopple, namcao, wangruikang, apatel, liu.xuemei1, ajones,
cleger, charlie, hui.wang, guodong, pincheng.plct, linux-riscv,
linux-kernel, Qingwei Hu
[-- Attachment #1: Type: text/plain, Size: 4964 bytes --]
On Tue, May 19, 2026 at 10:05:21PM +1000, Michael Ellerman wrote:
> On 19/5/2026 13:19, 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.
>
> I'm not sure what you mean here. The current code doesn't use an out-of-line
> function call AFAICS? More comments below ...
>
> > Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> > Reviewed-by: Qingwei Hu <qingwei.hu@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);
> > +}
> > 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();
> > }
>
> riscv_has_extension_unlikely() uses an asm alternative, ie. it's patched at
> boot so there's no runtime cost. But now you've changed it to just test a
> bool.
>
> I'm not sure arch_has_hw_pte_young() is a particularly hot path, but seems
> like you could use a static key, so that the code is patched to avoid the
> runtime test?
>
> > 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,
> ...
> > +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;
>
> This block is identical to the tail of the function. I'd probably use "goto
> enable", with an "enable" label below.
Is this code correct though? On DT systems, !svade && !svadu means we
don't actually know if it is hardware or software managed, so printing
that it's hardware managed may not be correct.
I don't understand the mm code enough to know if arch_has_hw_pte_young()
returning true is problematic too, but it probably is?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] riscv: track effective hardware PTE A/D updates
2026-05-19 3:19 [PATCH 1/2] riscv: track effective hardware PTE A/D updates Yunhui Cui
2026-05-19 3:19 ` [PATCH 2/2] riscv: merge hardware A/D updates in PTE accessors Yunhui Cui
2026-05-19 12:05 ` [PATCH 1/2] riscv: track effective hardware PTE A/D updates Michael Ellerman
@ 2026-05-19 17:31 ` Samuel Holland
2026-05-22 7:34 ` [External] " yunhui cui
2026-05-19 19:58 ` Andrew Jones
3 siblings, 1 reply; 11+ messages in thread
From: Samuel Holland @ 2026-05-19 17:31 UTC (permalink / raw)
To: Yunhui Cui
Cc: Qingwei Hu, pjw, palmer, aou, alex, akpm, pasha.tatashin,
andrew+kernel, rmclure, debug, baolin.wang, zhangchunyan,
apopple, namcao, wangruikang, apatel, liu.xuemei1, ajones,
cleger, charlie, hui.wang, guodong, pincheng.plct, linux-riscv,
linux-kernel
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@bytedance.com>
> Reviewed-by: Qingwei Hu <qingwei.hu@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?
>
> 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.
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),
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] riscv: track effective hardware PTE A/D updates
2026-05-19 15:37 ` Conor Dooley
@ 2026-05-19 17:34 ` Samuel Holland
2026-05-19 19:40 ` Conor Dooley
0 siblings, 1 reply; 11+ messages in thread
From: Samuel Holland @ 2026-05-19 17:34 UTC (permalink / raw)
To: Conor Dooley, Michael Ellerman
Cc: Yunhui Cui, pjw, palmer, aou, alex, akpm, pasha.tatashin,
andrew+kernel, rmclure, debug, baolin.wang, zhangchunyan,
apopple, namcao, wangruikang, apatel, liu.xuemei1, ajones,
cleger, charlie, hui.wang, guodong, pincheng.plct, linux-riscv,
linux-kernel, Qingwei Hu
Hi Conor,
On 2026-05-19 10:37 AM, Conor Dooley wrote:
> On Tue, May 19, 2026 at 10:05:21PM +1000, Michael Ellerman wrote:
>> On 19/5/2026 13:19, 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.
>>
>> I'm not sure what you mean here. The current code doesn't use an out-of-line
>> function call AFAICS? More comments below ...
>>
>>> Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
>>> Reviewed-by: Qingwei Hu <qingwei.hu@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);
>>> +}
>>> 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();
>>> }
>>
>> riscv_has_extension_unlikely() uses an asm alternative, ie. it's patched at
>> boot so there's no runtime cost. But now you've changed it to just test a
>> bool.
>>
>> I'm not sure arch_has_hw_pte_young() is a particularly hot path, but seems
>> like you could use a static key, so that the code is patched to avoid the
>> runtime test?
>>
>>> 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,
>> ...
>>> +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;
>>
>> This block is identical to the tail of the function. I'd probably use "goto
>> enable", with an "enable" label below.
>
> Is this code correct though? On DT systems, !svade && !svadu means we
> don't actually know if it is hardware or software managed, so printing
> that it's hardware managed may not be correct.
>
> I don't understand the mm code enough to know if arch_has_hw_pte_young()
> returning true is problematic too, but it probably is?
This block is for !svade && svadu (Svadu is present; we didn't return above), so
I think this block and Michael's comment are correct.
Regards,
Samuel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] riscv: track effective hardware PTE A/D updates
2026-05-19 17:34 ` Samuel Holland
@ 2026-05-19 19:40 ` Conor Dooley
0 siblings, 0 replies; 11+ messages in thread
From: Conor Dooley @ 2026-05-19 19:40 UTC (permalink / raw)
To: Samuel Holland
Cc: Michael Ellerman, Yunhui Cui, pjw, palmer, aou, alex, akpm,
pasha.tatashin, andrew+kernel, rmclure, debug, baolin.wang,
zhangchunyan, apopple, namcao, wangruikang, apatel, liu.xuemei1,
ajones, cleger, charlie, hui.wang, guodong, pincheng.plct,
linux-riscv, linux-kernel, Qingwei Hu
[-- Attachment #1: Type: text/plain, Size: 1877 bytes --]
On Tue, May 19, 2026 at 12:34:44PM -0500, Samuel Holland wrote:
> >>> 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,
> >> ...
> >>> +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;
> >>
> >> This block is identical to the tail of the function. I'd probably use "goto
> >> enable", with an "enable" label below.
> >
> > Is this code correct though? On DT systems, !svade && !svadu means we
> > don't actually know if it is hardware or software managed, so printing
> > that it's hardware managed may not be correct.
> >
> > I don't understand the mm code enough to know if arch_has_hw_pte_young()
> > returning true is problematic too, but it probably is?
>
> This block is for !svade && svadu (Svadu is present; we didn't return above), so
> I think this block and Michael's comment are correct.
D'oh... My bad!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] riscv: track effective hardware PTE A/D updates
2026-05-19 3:19 [PATCH 1/2] riscv: track effective hardware PTE A/D updates Yunhui Cui
` (2 preceding siblings ...)
2026-05-19 17:31 ` Samuel Holland
@ 2026-05-19 19:58 ` Andrew Jones
2026-05-22 7:37 ` [External] " yunhui cui
3 siblings, 1 reply; 11+ messages in thread
From: Andrew Jones @ 2026-05-19 19:58 UTC (permalink / raw)
To: Yunhui Cui
Cc: pjw, palmer, aou, alex, akpm, pasha.tatashin, andrew+kernel,
rmclure, debug, baolin.wang, zhangchunyan, apopple, namcao,
wangruikang, apatel, liu.xuemei1, ajones, cleger, charlie,
hui.wang, guodong, pincheng.plct, linux-riscv, linux-kernel,
Qingwei Hu
On Tue, May 19, 2026 at 11:19:26AM +0800, Yunhui Cui wrote:
> Platforms with Svadu but without Svade are treated as always using
> hardware PTE A/D updates.
This is correct, but I've become sensitive to the word 'always' after
recently reviewing the svadu spec. We should probably change the text in
Documentation/devicetree/bindings/riscv/extensions.yaml to something
like 'assume Svadu to be enabled at boot' because chapter 16 of the spec
states
"""
If the Svadu extension is implemented, the menvcfg.ADUE field is writable.
If the hypervisor extension is additionally implemented, the henvcfg.ADUE
field is also writable.
"""
So, if ADUE is writable then it cannot be hardwired to 1, which means it
can be written to zero. Then, in 3.1.18, we have
"""
When ADUE=0, the implementation behaves as though Svade were implemented
for S-mode and G-stage address translation.
"""
which means a spec-compliant system can never have svadu without svade.
Thanks,
drew
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [External] Re: [PATCH 1/2] riscv: track effective hardware PTE A/D updates
2026-05-19 12:05 ` [PATCH 1/2] riscv: track effective hardware PTE A/D updates Michael Ellerman
2026-05-19 15:37 ` Conor Dooley
@ 2026-05-22 7:28 ` yunhui cui
1 sibling, 0 replies; 11+ messages in thread
From: yunhui cui @ 2026-05-22 7:28 UTC (permalink / raw)
To: Michael Ellerman
Cc: pjw, palmer, aou, alex, akpm, pasha.tatashin, andrew+kernel,
rmclure, debug, baolin.wang, zhangchunyan, apopple, namcao,
wangruikang, apatel, liu.xuemei1, ajones, cleger, charlie,
hui.wang, guodong, pincheng.plct, linux-riscv, linux-kernel,
Qingwei Hu
Hi Michael,
On Tue, May 19, 2026 at 8:05 PM Michael Ellerman <mpe@kernel.org> wrote:
>
> On 19/5/2026 13:19, 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.
>
> I'm not sure what you mean here. The current code doesn't use an
> out-of-line function call AFAICS? More comments below ...
>
> > Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> > Reviewed-by: Qingwei Hu <qingwei.hu@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);
> > +}
> >
> > 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();
> > }
>
> riscv_has_extension_unlikely() uses an asm alternative, ie. it's patched
> at boot so there's no runtime cost. But now you've changed it to just
> test a bool.
>
> I'm not sure arch_has_hw_pte_young() is a particularly hot path, but
> seems like you could use a static key, so that the code is patched to
> avoid the runtime test?
Okay, I will update in the next patch version.
>
> > 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,
> ...
> >
> > +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;
>
> This block is identical to the tail of the function. I'd probably use
> "goto enable", with an "enable" label below.
Okay, I will update in the next patch version.
>
> > + }
> > +
> > + 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.
> > + */
>
> enable:
> > + riscv_set_hw_pte_ad_updating(true);
> > + pr_info("riscv: hardware PTE A/D updating enabled\n");
>
> pr_info() might be a bit verbose for this. I think printk(KERN_DEBUG ..)
> would be better, that way the message is always available in dmesg but
> isn't sent to the console
Will update in next version.
>
> > return 0;
> > }
> > +arch_initcall(riscv_hw_pte_ad_updating_init);
>
> cheers
Thanks,
Yunhui
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [External] Re: [PATCH 1/2] riscv: track effective hardware PTE A/D updates
2026-05-19 17:31 ` Samuel Holland
@ 2026-05-22 7:34 ` yunhui cui
0 siblings, 0 replies; 11+ messages in thread
From: yunhui cui @ 2026-05-22 7:34 UTC (permalink / raw)
To: Samuel Holland
Cc: Qingwei Hu, pjw, palmer, aou, alex, akpm, pasha.tatashin,
andrew+kernel, rmclure, debug, baolin.wang, zhangchunyan,
apopple, namcao, wangruikang, apatel, liu.xuemei1, ajones,
cleger, charlie, hui.wang, guodong, pincheng.plct, linux-riscv,
linux-kernel
Hi Samuel,
On Wed, May 20, 2026 at 1:31 AM Samuel Holland
<samuel.holland@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@bytedance.com>
> > Reviewed-by: Qingwei Hu <qingwei.hu@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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [External] Re: [PATCH 1/2] riscv: track effective hardware PTE A/D updates
2026-05-19 19:58 ` Andrew Jones
@ 2026-05-22 7:37 ` yunhui cui
0 siblings, 0 replies; 11+ messages in thread
From: yunhui cui @ 2026-05-22 7:37 UTC (permalink / raw)
To: Andrew Jones
Cc: pjw, palmer, aou, alex, akpm, pasha.tatashin, andrew+kernel,
rmclure, debug, baolin.wang, zhangchunyan, apopple, namcao,
wangruikang, apatel, liu.xuemei1, ajones, cleger, charlie,
hui.wang, guodong, pincheng.plct, linux-riscv, linux-kernel,
Qingwei Hu
Hi Andrew,
On Wed, May 20, 2026 at 3:58 AM Andrew Jones
<andrew.jones@oss.qualcomm.com> wrote:
>
> On Tue, May 19, 2026 at 11:19:26AM +0800, Yunhui Cui wrote:
> > Platforms with Svadu but without Svade are treated as always using
> > hardware PTE A/D updates.
>
> This is correct, but I've become sensitive to the word 'always' after
> recently reviewing the svadu spec. We should probably change the text in
> Documentation/devicetree/bindings/riscv/extensions.yaml to something
> like 'assume Svadu to be enabled at boot' because chapter 16 of the spec
> states
Good point, I'll reword this in the next version.
>
> """
> If the Svadu extension is implemented, the menvcfg.ADUE field is writable.
> If the hypervisor extension is additionally implemented, the henvcfg.ADUE
> field is also writable.
> """
>
> So, if ADUE is writable then it cannot be hardwired to 1, which means it
> can be written to zero. Then, in 3.1.18, we have
>
> """
> When ADUE=0, the implementation behaves as though Svade were implemented
> for S-mode and G-stage address translation.
> """
>
> which means a spec-compliant system can never have svadu without svade.
>
> Thanks,
> drew
Thanks,
Yunhui
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-05-22 7:37 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-05-19 3:19 [PATCH 1/2] riscv: track effective hardware PTE A/D updates Yunhui Cui
2026-05-19 3:19 ` [PATCH 2/2] riscv: merge hardware A/D updates in PTE accessors Yunhui Cui
2026-05-19 12:05 ` [PATCH 1/2] riscv: track effective hardware PTE A/D updates Michael Ellerman
2026-05-19 15:37 ` Conor Dooley
2026-05-19 17:34 ` Samuel Holland
2026-05-19 19:40 ` Conor Dooley
2026-05-22 7:28 ` [External] " yunhui cui
2026-05-19 17:31 ` Samuel Holland
2026-05-22 7:34 ` [External] " yunhui cui
2026-05-19 19:58 ` Andrew Jones
2026-05-22 7:37 ` [External] " yunhui cui
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®