[PATCH v7 0/4] Add Svade and Svadu Extensions Support

[PATCH v7 0/4] Add Svade and Svadu Extensions Support

[Yong-Xuan Wang]

Svade and Svadu extensions represent two schemes for managing the PTE A/D
bit. When the PTE A/D bits need to be set, Svade extension intdicates that
a related page fault will be raised. In contrast, the Svadu extension
supports hardware updating of PTE A/D bits. This series enables Svade and
Svadu extensions for both host and guest OS.

Regrading the mailing thread[1], we have 4 possible combinations of
these extensions in the device tree, the default hardware behavior for
these possibilities are:
1) Neither Svade nor Svadu present in DT => It is technically
   unknown whether the platform uses Svade or Svadu. Supervisor
   software should be prepared to handle either hardware updating
   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.
4) Both Svade and Svadu present in DT => Supervisor must assume
   Svadu turned-off at boot time. To use Svadu, supervisor must
   explicitly enable it using the SBI FWFT extension.

The Svade extension is mandatory and the Svadu extension is optional in
RVA23 profile. Platforms want to take the advantage of Svadu can choose
3. Those are aware of the profile can choose 4, and Linux won't get the
benefit of svadu until the SBI FWFT extension is available.

[1] https://lore.kernel.org/linux-kernel/20240527-e9845c06619bca5cd285098c@orel/T/#m29644eb88e241ec282df4ccd5199514e913b06ee

---
v7:
- fix alignment in PATCH1
- update the dtbinding in PATCH2 (Conor, Jessica)

v6:
- reflect the platform's behavior by riscv_isa_extension_available() and
  update the the arch_has_hw_pte_young() in PATCH1 (Conor, Andrew)
- update the dtbinding in PATCH2 (Alexandre, Andrew, Anup, Conor)
- update the henvcfg condition in PATCH3 (Andrew)
- check if Svade is allowed to disabled based on arch_has_hw_pte_young()
  in PATCH3

v5:
- remove all Acked-by and Reviewed-by (Conor, Andrew)
- add Svade support
- update the arch_has_hw_pte_young() in PATCH1
- update the dtbinding in PATCH2 (Alexandre, Andrew)
- check the availibility of Svadu for Guest/VM based on
  arch_has_hw_pte_young() in PATCH3

v4:
- fix 32bit kernel build error in PATCH1 (Conor)
- update the status of Svadu extension to ratified in PATCH2
- add the PATCH4 to suporrt SBI_FWFT_PTE_AD_HW_UPDATING for guest OS
- update the PATCH1 and PATCH3 to integrate with FWFT extension
- rebase PATCH5 on the lastest get-reg-list test (Andrew)

v3:
- fix the control bit name to ADUE in PATCH1 and PATCH3
- update get-reg-list in PATCH4

v2:
- add Co-developed-by: in PATCH1
- use riscv_has_extension_unlikely() to runtime patch the branch in PATCH1
- update dt-binding

Yong-Xuan Wang (4):
  RISC-V: Add Svade and Svadu Extensions Support
  dt-bindings: riscv: Add Svade and Svadu Entries
  RISC-V: KVM: Add Svade and Svadu Extensions Support for Guest/VM
  KVM: riscv: selftests: Add Svade and Svadu Extension to get-reg-list
    test

 .../devicetree/bindings/riscv/extensions.yaml | 28 ++++++++++++++++
 arch/riscv/Kconfig                            |  1 +
 arch/riscv/include/asm/csr.h                  |  1 +
 arch/riscv/include/asm/hwcap.h                |  2 ++
 arch/riscv/include/asm/pgtable.h              | 13 +++++++-
 arch/riscv/include/uapi/asm/kvm.h             |  2 ++
 arch/riscv/kernel/cpufeature.c                | 32 +++++++++++++++++++
 arch/riscv/kvm/vcpu.c                         |  3 ++
 arch/riscv/kvm/vcpu_onereg.c                  | 15 +++++++++
 .../selftests/kvm/riscv/get-reg-list.c        |  8 +++++
 10 files changed, 104 insertions(+), 1 deletion(-)

-- 
2.17.1

[PATCH v7 1/4] RISC-V: Add Svade and Svadu Extensions Support

[Yong-Xuan Wang]

Svade and Svadu extensions represent two schemes for managing the PTE A/D
bits. When the PTE A/D bits need to be set, Svade extension intdicates
that a related page fault will be raised. In contrast, the Svadu extension
supports hardware updating of PTE A/D bits. Since the Svade extension is
mandatory and the Svadu extension is optional in RVA23 profile, by default
the M-mode firmware will enable the Svadu extension in the menvcfg CSR
when only Svadu is present in DT.

This patch detects Svade and Svadu extensions from DT and adds
arch_has_hw_pte_young() to enable optimization in MGLRU and
__wp_page_copy_user() when we have the PTE A/D bits hardware updating
support.

Co-developed-by: Jinyu Tang <tjytimi@163.com>
Signed-off-by: Jinyu Tang <tjytimi@163.com>
Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
---
 arch/riscv/Kconfig               |  1 +
 arch/riscv/include/asm/csr.h     |  1 +
 arch/riscv/include/asm/hwcap.h   |  2 ++
 arch/riscv/include/asm/pgtable.h | 13 ++++++++++++-
 arch/riscv/kernel/cpufeature.c   | 32 ++++++++++++++++++++++++++++++++
 5 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 0525ee2d63c7..3d705e28ff85 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -36,6 +36,7 @@ config RISCV
 	select ARCH_HAS_PMEM_API
 	select ARCH_HAS_PREPARE_SYNC_CORE_CMD
 	select ARCH_HAS_PTE_SPECIAL
+	select ARCH_HAS_HW_PTE_YOUNG
 	select ARCH_HAS_SET_DIRECT_MAP if MMU
 	select ARCH_HAS_SET_MEMORY if MMU
 	select ARCH_HAS_STRICT_KERNEL_RWX if MMU && !XIP_KERNEL
diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
index 25966995da04..524cd4131c71 100644
--- a/arch/riscv/include/asm/csr.h
+++ b/arch/riscv/include/asm/csr.h
@@ -195,6 +195,7 @@
 /* xENVCFG flags */
 #define ENVCFG_STCE			(_AC(1, ULL) << 63)
 #define ENVCFG_PBMTE			(_AC(1, ULL) << 62)
+#define ENVCFG_ADUE			(_AC(1, ULL) << 61)
 #define ENVCFG_CBZE			(_AC(1, UL) << 7)
 #define ENVCFG_CBCFE			(_AC(1, UL) << 6)
 #define ENVCFG_CBIE_SHIFT		4
diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index e17d0078a651..35d7aa49785d 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -81,6 +81,8 @@
 #define RISCV_ISA_EXT_ZTSO		72
 #define RISCV_ISA_EXT_ZACAS		73
 #define RISCV_ISA_EXT_XANDESPMU		74
+#define RISCV_ISA_EXT_SVADE             75
+#define RISCV_ISA_EXT_SVADU		76
 
 #define RISCV_ISA_EXT_XLINUXENVCFG	127
 
diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
index aad8b8ca51f1..ec0cdacd7da0 100644
--- a/arch/riscv/include/asm/pgtable.h
+++ b/arch/riscv/include/asm/pgtable.h
@@ -120,6 +120,7 @@
 #include <asm/tlbflush.h>
 #include <linux/mm_types.h>
 #include <asm/compat.h>
+#include <asm/cpufeature.h>
 
 #define __page_val_to_pfn(_val)  (((_val) & _PAGE_PFN_MASK) >> _PAGE_PFN_SHIFT)
 
@@ -288,7 +289,6 @@ static inline pte_t pud_pte(pud_t pud)
 }
 
 #ifdef CONFIG_RISCV_ISA_SVNAPOT
-#include <asm/cpufeature.h>
 
 static __always_inline bool has_svnapot(void)
 {
@@ -624,6 +624,17 @@ static inline pgprot_t pgprot_writecombine(pgprot_t _prot)
 	return __pgprot(prot);
 }
 
+/*
+ * 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.
+ */
+#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);
+}
+
 /*
  * THP functions
  */
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 5ef48cb20ee1..b2c3fe945e89 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -301,6 +301,8 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
 	__RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
 	__RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
 	__RISCV_ISA_EXT_DATA(sstc, RISCV_ISA_EXT_SSTC),
+	__RISCV_ISA_EXT_DATA(svade, RISCV_ISA_EXT_SVADE),
+	__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),
@@ -554,6 +556,21 @@ static void __init riscv_fill_hwcap_from_isa_string(unsigned long *isa2hwcap)
 			clear_bit(RISCV_ISA_EXT_v, isainfo->isa);
 		}
 
+		/*
+		 * When neither Svade nor Svadu present in DT, it is technically
+		 * unknown whether the platform uses Svade or Svadu. Supervisor may
+		 * assume Svade to be present and enabled or it can discover based
+		 * on mvendorid, marchid, and mimpid. When both Svade and Svadu present
+		 * in DT, supervisor must assume Svadu turned-off at boot time. To use
+		 * Svadu, supervisor must explicitly enable it using the SBI FWFT extension.
+		 */
+		if (!test_bit(RISCV_ISA_EXT_SVADE, isainfo->isa) &&
+		    !test_bit(RISCV_ISA_EXT_SVADU, isainfo->isa))
+			set_bit(RISCV_ISA_EXT_SVADE, isainfo->isa);
+		else if (test_bit(RISCV_ISA_EXT_SVADE, isainfo->isa) &&
+			 test_bit(RISCV_ISA_EXT_SVADU, isainfo->isa))
+			clear_bit(RISCV_ISA_EXT_SVADU, isainfo->isa);
+
 		/*
 		 * All "okay" hart should have same isa. Set HWCAP based on
 		 * common capabilities of every "okay" hart, in case they don't
@@ -619,6 +636,21 @@ static int __init riscv_fill_hwcap_from_ext_list(unsigned long *isa2hwcap)
 
 		of_node_put(cpu_node);
 
+		/*
+		 * When neither Svade nor Svadu present in DT, it is technically
+		 * unknown whether the platform uses Svade or Svadu. Supervisor may
+		 * assume Svade to be present and enabled or it can discover based
+		 * on mvendorid, marchid, and mimpid. When both Svade and Svadu present
+		 * in DT, supervisor must assume Svadu turned-off at boot time. To use
+		 * Svadu, supervisor must explicitly enable it using the SBI FWFT extension.
+		 */
+		if (!test_bit(RISCV_ISA_EXT_SVADE, isainfo->isa) &&
+		    !test_bit(RISCV_ISA_EXT_SVADU, isainfo->isa))
+			set_bit(RISCV_ISA_EXT_SVADE, isainfo->isa);
+		else if (test_bit(RISCV_ISA_EXT_SVADE, isainfo->isa) &&
+			 test_bit(RISCV_ISA_EXT_SVADU, isainfo->isa))
+			clear_bit(RISCV_ISA_EXT_SVADU, isainfo->isa);
+
 		/*
 		 * All "okay" harts should have same isa. Set HWCAP based on
 		 * common capabilities of every "okay" hart, in case they don't.
-- 
2.17.1

[PATCH v7 2/4] dt-bindings: riscv: Add Svade and Svadu Entries

[Yong-Xuan Wang]

Add entries for the Svade and Svadu extensions to the riscv,isa-extensions
property.

Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
---
 .../devicetree/bindings/riscv/extensions.yaml | 28 +++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml
index 468c646247aa..e91a6f4ede38 100644
--- a/Documentation/devicetree/bindings/riscv/extensions.yaml
+++ b/Documentation/devicetree/bindings/riscv/extensions.yaml
@@ -153,6 +153,34 @@ properties:
             ratified at commit 3f9ed34 ("Add ability to manually trigger
             workflow. (#2)") of riscv-time-compare.
 
+        - const: svade
+          description: |
+            The standard Svade supervisor-level extension for SW-managed PTE A/D
+            bit updates as ratified in the 20240213 version of the privileged
+            ISA specification.
+
+            Both Svade and Svadu extensions control the hardware behavior when
+            the PTE A/D bits need to be set. The default behavior for the four
+            possible combinations of these extensions in the device tree are:
+            1) Neither Svade nor Svadu present in DT => It is technically
+               unknown whether the platform uses Svade or Svadu. Supervisor
+               software should be prepared to handle either hardware updating
+               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.
+            4) Both Svade and Svadu present in DT => Supervisor must assume
+               Svadu turned-off at boot time. To use Svadu, supervisor must
+               explicitly enable it using the SBI FWFT extension.
+
+        - const: svadu
+          description: |
+            The standard Svadu supervisor-level extension for hardware updating
+            of PTE A/D bits as ratified at commit c1abccf ("Merge pull request
+            #25 from ved-rivos/ratified") of riscv-svadu. Please refer to Svade
+            dt-binding description for more details.
+
         - const: svinval
           description:
             The standard Svinval supervisor-level extension for fine-grained
-- 
2.17.1

[PATCH v7 3/4] RISC-V: KVM: Add Svade and Svadu Extensions Support for Guest/VM

[Yong-Xuan Wang]

We extend the KVM ISA extension ONE_REG interface to allow VMM tools to
detect and enable Svade and Svadu extensions for Guest/VM. Since the
henvcfg.ADUE is read-only zero if the menvcfg.ADUE is zero, the Svadu
extension is available for Guest/VM and the Svade extension is allowed
to disabledonly when arch_has_hw_pte_young() is true.

Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
---
 arch/riscv/include/uapi/asm/kvm.h |  2 ++
 arch/riscv/kvm/vcpu.c             |  3 +++
 arch/riscv/kvm/vcpu_onereg.c      | 15 +++++++++++++++
 3 files changed, 20 insertions(+)

diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h
index e878e7cc3978..a5e0c35d7e9a 100644
--- a/arch/riscv/include/uapi/asm/kvm.h
+++ b/arch/riscv/include/uapi/asm/kvm.h
@@ -168,6 +168,8 @@ enum KVM_RISCV_ISA_EXT_ID {
 	KVM_RISCV_ISA_EXT_ZTSO,
 	KVM_RISCV_ISA_EXT_ZACAS,
 	KVM_RISCV_ISA_EXT_SSCOFPMF,
+	KVM_RISCV_ISA_EXT_SVADE,
+	KVM_RISCV_ISA_EXT_SVADU,
 	KVM_RISCV_ISA_EXT_MAX,
 };
 
diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
index 17e21df36cc1..64a15af459e0 100644
--- a/arch/riscv/kvm/vcpu.c
+++ b/arch/riscv/kvm/vcpu.c
@@ -540,6 +540,9 @@ static void kvm_riscv_vcpu_setup_config(struct kvm_vcpu *vcpu)
 	if (riscv_isa_extension_available(isa, ZICBOZ))
 		cfg->henvcfg |= ENVCFG_CBZE;
 
+	if (riscv_isa_extension_available(isa, SVADU))
+		cfg->henvcfg |= ENVCFG_ADUE;
+
 	if (riscv_has_extension_unlikely(RISCV_ISA_EXT_SMSTATEEN)) {
 		cfg->hstateen0 |= SMSTATEEN0_HSENVCFG;
 		if (riscv_isa_extension_available(isa, SSAIA))
diff --git a/arch/riscv/kvm/vcpu_onereg.c b/arch/riscv/kvm/vcpu_onereg.c
index 62874fbca29f..474fdeafe9fe 100644
--- a/arch/riscv/kvm/vcpu_onereg.c
+++ b/arch/riscv/kvm/vcpu_onereg.c
@@ -15,6 +15,7 @@
 #include <asm/cacheflush.h>
 #include <asm/cpufeature.h>
 #include <asm/kvm_vcpu_vector.h>
+#include <asm/pgtable.h>
 #include <asm/vector.h>
 
 #define KVM_RISCV_BASE_ISA_MASK		GENMASK(25, 0)
@@ -38,6 +39,8 @@ static const unsigned long kvm_isa_ext_arr[] = {
 	KVM_ISA_EXT_ARR(SSAIA),
 	KVM_ISA_EXT_ARR(SSCOFPMF),
 	KVM_ISA_EXT_ARR(SSTC),
+	KVM_ISA_EXT_ARR(SVADE),
+	KVM_ISA_EXT_ARR(SVADU),
 	KVM_ISA_EXT_ARR(SVINVAL),
 	KVM_ISA_EXT_ARR(SVNAPOT),
 	KVM_ISA_EXT_ARR(SVPBMT),
@@ -105,6 +108,12 @@ static bool kvm_riscv_vcpu_isa_enable_allowed(unsigned long ext)
 		return __riscv_isa_extension_available(NULL, RISCV_ISA_EXT_SSAIA);
 	case KVM_RISCV_ISA_EXT_V:
 		return riscv_v_vstate_ctrl_user_allowed();
+	case KVM_RISCV_ISA_EXT_SVADU:
+		/*
+		 * The henvcfg.ADUE is read-only zero if menvcfg.ADUE is zero.
+		 * Guest OS can use Svadu only when host os enable Svadu.
+		 */
+		return arch_has_hw_pte_young();
 	default:
 		break;
 	}
@@ -167,6 +176,12 @@ static bool kvm_riscv_vcpu_isa_disable_allowed(unsigned long ext)
 	/* Extensions which can be disabled using Smstateen */
 	case KVM_RISCV_ISA_EXT_SSAIA:
 		return riscv_has_extension_unlikely(RISCV_ISA_EXT_SMSTATEEN);
+	case KVM_RISCV_ISA_EXT_SVADE:
+		/*
+		 * The henvcfg.ADUE is read-only zero if menvcfg.ADUE is zero.
+		 * Svade is not allowed to disable when the platform use Svade.
+		 */
+		return arch_has_hw_pte_young();
 	default:
 		break;
 	}
-- 
2.17.1

[PATCH v7 4/4] KVM: riscv: selftests: Add Svade and Svadu Extension to get-reg-list test

[Yong-Xuan Wang]

Update the get-reg-list test to test the Svade and Svadu Extensions are
available for guest OS.

Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
---
 tools/testing/selftests/kvm/riscv/get-reg-list.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/testing/selftests/kvm/riscv/get-reg-list.c b/tools/testing/selftests/kvm/riscv/get-reg-list.c
index 222198dd6d04..1d32351ad55e 100644
--- a/tools/testing/selftests/kvm/riscv/get-reg-list.c
+++ b/tools/testing/selftests/kvm/riscv/get-reg-list.c
@@ -45,6 +45,8 @@ bool filter_reg(__u64 reg)
 	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_SSAIA:
 	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_SSCOFPMF:
 	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_SSTC:
+	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_SVADE:
+	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_SVADU:
 	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_SVINVAL:
 	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_SVNAPOT:
 	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_SVPBMT:
@@ -411,6 +413,8 @@ static const char *isa_ext_single_id_to_str(__u64 reg_off)
 		KVM_ISA_EXT_ARR(SSAIA),
 		KVM_ISA_EXT_ARR(SSCOFPMF),
 		KVM_ISA_EXT_ARR(SSTC),
+		KVM_ISA_EXT_ARR(SVADE),
+		KVM_ISA_EXT_ARR(SVADU),
 		KVM_ISA_EXT_ARR(SVINVAL),
 		KVM_ISA_EXT_ARR(SVNAPOT),
 		KVM_ISA_EXT_ARR(SVPBMT),
@@ -935,6 +939,8 @@ KVM_ISA_EXT_SIMPLE_CONFIG(h, H);
 KVM_ISA_EXT_SUBLIST_CONFIG(smstateen, SMSTATEEN);
 KVM_ISA_EXT_SIMPLE_CONFIG(sscofpmf, SSCOFPMF);
 KVM_ISA_EXT_SIMPLE_CONFIG(sstc, SSTC);
+KVM_ISA_EXT_SIMPLE_CONFIG(svade, SVADE);
+KVM_ISA_EXT_SIMPLE_CONFIG(svadu, SVADU);
 KVM_ISA_EXT_SIMPLE_CONFIG(svinval, SVINVAL);
 KVM_ISA_EXT_SIMPLE_CONFIG(svnapot, SVNAPOT);
 KVM_ISA_EXT_SIMPLE_CONFIG(svpbmt, SVPBMT);
@@ -991,6 +997,8 @@ struct vcpu_reg_list *vcpu_configs[] = {
 	&config_smstateen,
 	&config_sscofpmf,
 	&config_sstc,
+	&config_svade,
+	&config_svadu,
 	&config_svinval,
 	&config_svnapot,
 	&config_svpbmt,
-- 
2.17.1

Re: [PATCH v7 2/4] dt-bindings: riscv: Add Svade and Svadu Entries

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 275 bytes --]

On Fri, Jul 12, 2024 at 04:38:46PM +0800, Yong-Xuan Wang wrote:
> Add entries for the Svade and Svadu extensions to the riscv,isa-extensions
> property.
> 
> Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>

Acked-by: Conor Dooley <conor.dooley@microchip.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v7 1/4] RISC-V: Add Svade and Svadu Extensions Support

[Alexandre Ghiti]

Hi Yong-Xuan,

On 12/07/2024 10:38, Yong-Xuan Wang wrote:
> Svade and Svadu extensions represent two schemes for managing the PTE A/D
> bits. When the PTE A/D bits need to be set, Svade extension intdicates
> that a related page fault will be raised. In contrast, the Svadu extension
> supports hardware updating of PTE A/D bits. Since the Svade extension is
> mandatory and the Svadu extension is optional in RVA23 profile, by default
> the M-mode firmware will enable the Svadu extension in the menvcfg CSR
> when only Svadu is present in DT.
>
> This patch detects Svade and Svadu extensions from DT and adds
> arch_has_hw_pte_young() to enable optimization in MGLRU and
> __wp_page_copy_user() when we have the PTE A/D bits hardware updating
> support.
>
> Co-developed-by: Jinyu Tang <tjytimi@163.com>
> Signed-off-by: Jinyu Tang <tjytimi@163.com>
> Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> ---
>   arch/riscv/Kconfig               |  1 +
>   arch/riscv/include/asm/csr.h     |  1 +
>   arch/riscv/include/asm/hwcap.h   |  2 ++
>   arch/riscv/include/asm/pgtable.h | 13 ++++++++++++-
>   arch/riscv/kernel/cpufeature.c   | 32 ++++++++++++++++++++++++++++++++
>   5 files changed, 48 insertions(+), 1 deletion(-)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 0525ee2d63c7..3d705e28ff85 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -36,6 +36,7 @@ config RISCV
>   	select ARCH_HAS_PMEM_API
>   	select ARCH_HAS_PREPARE_SYNC_CORE_CMD
>   	select ARCH_HAS_PTE_SPECIAL
> +	select ARCH_HAS_HW_PTE_YOUNG
>   	select ARCH_HAS_SET_DIRECT_MAP if MMU
>   	select ARCH_HAS_SET_MEMORY if MMU
>   	select ARCH_HAS_STRICT_KERNEL_RWX if MMU && !XIP_KERNEL
> diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
> index 25966995da04..524cd4131c71 100644
> --- a/arch/riscv/include/asm/csr.h
> +++ b/arch/riscv/include/asm/csr.h
> @@ -195,6 +195,7 @@
>   /* xENVCFG flags */
>   #define ENVCFG_STCE			(_AC(1, ULL) << 63)
>   #define ENVCFG_PBMTE			(_AC(1, ULL) << 62)
> +#define ENVCFG_ADUE			(_AC(1, ULL) << 61)
>   #define ENVCFG_CBZE			(_AC(1, UL) << 7)
>   #define ENVCFG_CBCFE			(_AC(1, UL) << 6)
>   #define ENVCFG_CBIE_SHIFT		4
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index e17d0078a651..35d7aa49785d 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -81,6 +81,8 @@
>   #define RISCV_ISA_EXT_ZTSO		72
>   #define RISCV_ISA_EXT_ZACAS		73
>   #define RISCV_ISA_EXT_XANDESPMU		74
> +#define RISCV_ISA_EXT_SVADE             75
> +#define RISCV_ISA_EXT_SVADU		76
>   
>   #define RISCV_ISA_EXT_XLINUXENVCFG	127
>   
> diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
> index aad8b8ca51f1..ec0cdacd7da0 100644
> --- a/arch/riscv/include/asm/pgtable.h
> +++ b/arch/riscv/include/asm/pgtable.h
> @@ -120,6 +120,7 @@
>   #include <asm/tlbflush.h>
>   #include <linux/mm_types.h>
>   #include <asm/compat.h>
> +#include <asm/cpufeature.h>
>   
>   #define __page_val_to_pfn(_val)  (((_val) & _PAGE_PFN_MASK) >> _PAGE_PFN_SHIFT)
>   
> @@ -288,7 +289,6 @@ static inline pte_t pud_pte(pud_t pud)
>   }
>   
>   #ifdef CONFIG_RISCV_ISA_SVNAPOT
> -#include <asm/cpufeature.h>
>   
>   static __always_inline bool has_svnapot(void)
>   {
> @@ -624,6 +624,17 @@ static inline pgprot_t pgprot_writecombine(pgprot_t _prot)
>   	return __pgprot(prot);
>   }
>   
> +/*
> + * 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.
> + */
> +#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);
> +}
> +
>   /*
>    * THP functions
>    */
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 5ef48cb20ee1..b2c3fe945e89 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -301,6 +301,8 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
>   	__RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
>   	__RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
>   	__RISCV_ISA_EXT_DATA(sstc, RISCV_ISA_EXT_SSTC),
> +	__RISCV_ISA_EXT_DATA(svade, RISCV_ISA_EXT_SVADE),
> +	__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),
> @@ -554,6 +556,21 @@ static void __init riscv_fill_hwcap_from_isa_string(unsigned long *isa2hwcap)
>   			clear_bit(RISCV_ISA_EXT_v, isainfo->isa);
>   		}
>   
> +		/*
> +		 * When neither Svade nor Svadu present in DT, it is technically
> +		 * unknown whether the platform uses Svade or Svadu. Supervisor may
> +		 * assume Svade to be present and enabled or it can discover based
> +		 * on mvendorid, marchid, and mimpid. When both Svade and Svadu present
> +		 * in DT, supervisor must assume Svadu turned-off at boot time. To use
> +		 * Svadu, supervisor must explicitly enable it using the SBI FWFT extension.
> +		 */
> +		if (!test_bit(RISCV_ISA_EXT_SVADE, isainfo->isa) &&
> +		    !test_bit(RISCV_ISA_EXT_SVADU, isainfo->isa))
> +			set_bit(RISCV_ISA_EXT_SVADE, isainfo->isa);
> +		else if (test_bit(RISCV_ISA_EXT_SVADE, isainfo->isa) &&
> +			 test_bit(RISCV_ISA_EXT_SVADU, isainfo->isa))
> +			clear_bit(RISCV_ISA_EXT_SVADU, isainfo->isa);
> +
>   		/*
>   		 * All "okay" hart should have same isa. Set HWCAP based on
>   		 * common capabilities of every "okay" hart, in case they don't
> @@ -619,6 +636,21 @@ static int __init riscv_fill_hwcap_from_ext_list(unsigned long *isa2hwcap)
>   
>   		of_node_put(cpu_node);
>   
> +		/*
> +		 * When neither Svade nor Svadu present in DT, it is technically
> +		 * unknown whether the platform uses Svade or Svadu. Supervisor may
> +		 * assume Svade to be present and enabled or it can discover based
> +		 * on mvendorid, marchid, and mimpid. When both Svade and Svadu present
> +		 * in DT, supervisor must assume Svadu turned-off at boot time. To use
> +		 * Svadu, supervisor must explicitly enable it using the SBI FWFT extension.
> +		 */
> +		if (!test_bit(RISCV_ISA_EXT_SVADE, isainfo->isa) &&
> +		    !test_bit(RISCV_ISA_EXT_SVADU, isainfo->isa))
> +			set_bit(RISCV_ISA_EXT_SVADE, isainfo->isa);
> +		else if (test_bit(RISCV_ISA_EXT_SVADE, isainfo->isa) &&
> +			 test_bit(RISCV_ISA_EXT_SVADU, isainfo->isa))
> +			clear_bit(RISCV_ISA_EXT_SVADU, isainfo->isa);
> +
>   		/*
>   		 * All "okay" harts should have same isa. Set HWCAP based on
>   		 * common capabilities of every "okay" hart, in case they don't.


Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>

Thanks,

Alex

Re: [PATCH v7 2/4] dt-bindings: riscv: Add Svade and Svadu Entries

[Alexandre Ghiti]

On 12/07/2024 10:38, Yong-Xuan Wang wrote:
> Add entries for the Svade and Svadu extensions to the riscv,isa-extensions
> property.
>
> Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> ---
>   .../devicetree/bindings/riscv/extensions.yaml | 28 +++++++++++++++++++
>   1 file changed, 28 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml
> index 468c646247aa..e91a6f4ede38 100644
> --- a/Documentation/devicetree/bindings/riscv/extensions.yaml
> +++ b/Documentation/devicetree/bindings/riscv/extensions.yaml
> @@ -153,6 +153,34 @@ properties:
>               ratified at commit 3f9ed34 ("Add ability to manually trigger
>               workflow. (#2)") of riscv-time-compare.
>   
> +        - const: svade
> +          description: |
> +            The standard Svade supervisor-level extension for SW-managed PTE A/D
> +            bit updates as ratified in the 20240213 version of the privileged
> +            ISA specification.
> +
> +            Both Svade and Svadu extensions control the hardware behavior when
> +            the PTE A/D bits need to be set. The default behavior for the four
> +            possible combinations of these extensions in the device tree are:
> +            1) Neither Svade nor Svadu present in DT => It is technically
> +               unknown whether the platform uses Svade or Svadu. Supervisor
> +               software should be prepared to handle either hardware updating
> +               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.
> +            4) Both Svade and Svadu present in DT => Supervisor must assume
> +               Svadu turned-off at boot time. To use Svadu, supervisor must
> +               explicitly enable it using the SBI FWFT extension.
> +
> +        - const: svadu
> +          description: |
> +            The standard Svadu supervisor-level extension for hardware updating
> +            of PTE A/D bits as ratified at commit c1abccf ("Merge pull request
> +            #25 from ved-rivos/ratified") of riscv-svadu. Please refer to Svade
> +            dt-binding description for more details.
> +
>           - const: svinval
>             description:
>               The standard Svinval supervisor-level extension for fine-grained


Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>

Thanks,

Alex

Re: [PATCH v7 1/4] RISC-V: Add Svade and Svadu Extensions Support

[Samuel Holland]

Hi Yong-Xuan,

Two trivial comments below for if you send another version of the series.

On 2024-07-12 3:38 AM, Yong-Xuan Wang wrote:
> Svade and Svadu extensions represent two schemes for managing the PTE A/D
> bits. When the PTE A/D bits need to be set, Svade extension intdicates
> that a related page fault will be raised. In contrast, the Svadu extension
> supports hardware updating of PTE A/D bits. Since the Svade extension is
> mandatory and the Svadu extension is optional in RVA23 profile, by default
> the M-mode firmware will enable the Svadu extension in the menvcfg CSR
> when only Svadu is present in DT.
> 
> This patch detects Svade and Svadu extensions from DT and adds
> arch_has_hw_pte_young() to enable optimization in MGLRU and
> __wp_page_copy_user() when we have the PTE A/D bits hardware updating
> support.
> 
> Co-developed-by: Jinyu Tang <tjytimi@163.com>
> Signed-off-by: Jinyu Tang <tjytimi@163.com>
> Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> ---
>  arch/riscv/Kconfig               |  1 +
>  arch/riscv/include/asm/csr.h     |  1 +
>  arch/riscv/include/asm/hwcap.h   |  2 ++
>  arch/riscv/include/asm/pgtable.h | 13 ++++++++++++-
>  arch/riscv/kernel/cpufeature.c   | 32 ++++++++++++++++++++++++++++++++
>  5 files changed, 48 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 0525ee2d63c7..3d705e28ff85 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -36,6 +36,7 @@ config RISCV
>  	select ARCH_HAS_PMEM_API
>  	select ARCH_HAS_PREPARE_SYNC_CORE_CMD
>  	select ARCH_HAS_PTE_SPECIAL
> +	select ARCH_HAS_HW_PTE_YOUNG

These lines should be sorted alphabetically.

>  	select ARCH_HAS_SET_DIRECT_MAP if MMU
>  	select ARCH_HAS_SET_MEMORY if MMU
>  	select ARCH_HAS_STRICT_KERNEL_RWX if MMU && !XIP_KERNEL
> diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
> index 25966995da04..524cd4131c71 100644
> --- a/arch/riscv/include/asm/csr.h
> +++ b/arch/riscv/include/asm/csr.h
> @@ -195,6 +195,7 @@
>  /* xENVCFG flags */
>  #define ENVCFG_STCE			(_AC(1, ULL) << 63)
>  #define ENVCFG_PBMTE			(_AC(1, ULL) << 62)
> +#define ENVCFG_ADUE			(_AC(1, ULL) << 61)
>  #define ENVCFG_CBZE			(_AC(1, UL) << 7)
>  #define ENVCFG_CBCFE			(_AC(1, UL) << 6)
>  #define ENVCFG_CBIE_SHIFT		4
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index e17d0078a651..35d7aa49785d 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -81,6 +81,8 @@
>  #define RISCV_ISA_EXT_ZTSO		72
>  #define RISCV_ISA_EXT_ZACAS		73
>  #define RISCV_ISA_EXT_XANDESPMU		74
> +#define RISCV_ISA_EXT_SVADE             75

The number here should be aligned with tabs, like the surrounding lines.

Regards,
Samuel

> +#define RISCV_ISA_EXT_SVADU		76
>  
>  #define RISCV_ISA_EXT_XLINUXENVCFG	127
>  
> diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
> index aad8b8ca51f1..ec0cdacd7da0 100644
> --- a/arch/riscv/include/asm/pgtable.h
> +++ b/arch/riscv/include/asm/pgtable.h
> @@ -120,6 +120,7 @@
>  #include <asm/tlbflush.h>
>  #include <linux/mm_types.h>
>  #include <asm/compat.h>
> +#include <asm/cpufeature.h>
>  
>  #define __page_val_to_pfn(_val)  (((_val) & _PAGE_PFN_MASK) >> _PAGE_PFN_SHIFT)
>  
> @@ -288,7 +289,6 @@ static inline pte_t pud_pte(pud_t pud)
>  }
>  
>  #ifdef CONFIG_RISCV_ISA_SVNAPOT
> -#include <asm/cpufeature.h>
>  
>  static __always_inline bool has_svnapot(void)
>  {
> @@ -624,6 +624,17 @@ static inline pgprot_t pgprot_writecombine(pgprot_t _prot)
>  	return __pgprot(prot);
>  }
>  
> +/*
> + * 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.
> + */
> +#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);
> +}
> +
>  /*
>   * THP functions
>   */
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 5ef48cb20ee1..b2c3fe945e89 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -301,6 +301,8 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
>  	__RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
>  	__RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
>  	__RISCV_ISA_EXT_DATA(sstc, RISCV_ISA_EXT_SSTC),
> +	__RISCV_ISA_EXT_DATA(svade, RISCV_ISA_EXT_SVADE),
> +	__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),
> @@ -554,6 +556,21 @@ static void __init riscv_fill_hwcap_from_isa_string(unsigned long *isa2hwcap)
>  			clear_bit(RISCV_ISA_EXT_v, isainfo->isa);
>  		}
>  
> +		/*
> +		 * When neither Svade nor Svadu present in DT, it is technically
> +		 * unknown whether the platform uses Svade or Svadu. Supervisor may
> +		 * assume Svade to be present and enabled or it can discover based
> +		 * on mvendorid, marchid, and mimpid. When both Svade and Svadu present
> +		 * in DT, supervisor must assume Svadu turned-off at boot time. To use
> +		 * Svadu, supervisor must explicitly enable it using the SBI FWFT extension.
> +		 */
> +		if (!test_bit(RISCV_ISA_EXT_SVADE, isainfo->isa) &&
> +		    !test_bit(RISCV_ISA_EXT_SVADU, isainfo->isa))
> +			set_bit(RISCV_ISA_EXT_SVADE, isainfo->isa);
> +		else if (test_bit(RISCV_ISA_EXT_SVADE, isainfo->isa) &&
> +			 test_bit(RISCV_ISA_EXT_SVADU, isainfo->isa))
> +			clear_bit(RISCV_ISA_EXT_SVADU, isainfo->isa);
> +
>  		/*
>  		 * All "okay" hart should have same isa. Set HWCAP based on
>  		 * common capabilities of every "okay" hart, in case they don't
> @@ -619,6 +636,21 @@ static int __init riscv_fill_hwcap_from_ext_list(unsigned long *isa2hwcap)
>  
>  		of_node_put(cpu_node);
>  
> +		/*
> +		 * When neither Svade nor Svadu present in DT, it is technically
> +		 * unknown whether the platform uses Svade or Svadu. Supervisor may
> +		 * assume Svade to be present and enabled or it can discover based
> +		 * on mvendorid, marchid, and mimpid. When both Svade and Svadu present
> +		 * in DT, supervisor must assume Svadu turned-off at boot time. To use
> +		 * Svadu, supervisor must explicitly enable it using the SBI FWFT extension.
> +		 */
> +		if (!test_bit(RISCV_ISA_EXT_SVADE, isainfo->isa) &&
> +		    !test_bit(RISCV_ISA_EXT_SVADU, isainfo->isa))
> +			set_bit(RISCV_ISA_EXT_SVADE, isainfo->isa);
> +		else if (test_bit(RISCV_ISA_EXT_SVADE, isainfo->isa) &&
> +			 test_bit(RISCV_ISA_EXT_SVADU, isainfo->isa))
> +			clear_bit(RISCV_ISA_EXT_SVADU, isainfo->isa);
> +
>  		/*
>  		 * All "okay" harts should have same isa. Set HWCAP based on
>  		 * common capabilities of every "okay" hart, in case they don't.

Re: [PATCH v7 2/4] dt-bindings: riscv: Add Svade and Svadu Entries

[Samuel Holland]

On 2024-07-12 3:38 AM, Yong-Xuan Wang wrote:
> Add entries for the Svade and Svadu extensions to the riscv,isa-extensions
> property.
> 
> Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> ---
>  .../devicetree/bindings/riscv/extensions.yaml | 28 +++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml
> index 468c646247aa..e91a6f4ede38 100644
> --- a/Documentation/devicetree/bindings/riscv/extensions.yaml
> +++ b/Documentation/devicetree/bindings/riscv/extensions.yaml
> @@ -153,6 +153,34 @@ properties:
>              ratified at commit 3f9ed34 ("Add ability to manually trigger
>              workflow. (#2)") of riscv-time-compare.
>  
> +        - const: svade
> +          description: |
> +            The standard Svade supervisor-level extension for SW-managed PTE A/D
> +            bit updates as ratified in the 20240213 version of the privileged
> +            ISA specification.
> +
> +            Both Svade and Svadu extensions control the hardware behavior when
> +            the PTE A/D bits need to be set. The default behavior for the four
> +            possible combinations of these extensions in the device tree are:
> +            1) Neither Svade nor Svadu present in DT => It is technically
> +               unknown whether the platform uses Svade or Svadu. Supervisor
> +               software should be prepared to handle either hardware updating
> +               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.
> +            4) Both Svade and Svadu present in DT => Supervisor must assume
> +               Svadu turned-off at boot time. To use Svadu, supervisor must
> +               explicitly enable it using the SBI FWFT extension.
> +
> +        - const: svadu
> +          description: |
> +            The standard Svadu supervisor-level extension for hardware updating
> +            of PTE A/D bits as ratified at commit c1abccf ("Merge pull request
> +            #25 from ved-rivos/ratified") of riscv-svadu. Please refer to Svade

Should we be referencing the archived riscv-svadu repository now that Svadu has
been merged to the main privileged ISA manual? Either way:

Reviewed-by: Samuel Holland <samuel.holland@sifive.com>

> +            dt-binding description for more details.
> +
>          - const: svinval
>            description:
>              The standard Svinval supervisor-level extension for fine-grained

Re: [PATCH v7 3/4] RISC-V: KVM: Add Svade and Svadu Extensions Support for Guest/VM

[Samuel Holland]

Hi Yong-Xuan,

On 2024-07-12 3:38 AM, Yong-Xuan Wang wrote:
> We extend the KVM ISA extension ONE_REG interface to allow VMM tools to
> detect and enable Svade and Svadu extensions for Guest/VM. Since the
> henvcfg.ADUE is read-only zero if the menvcfg.ADUE is zero, the Svadu
> extension is available for Guest/VM and the Svade extension is allowed
> to disabledonly when arch_has_hw_pte_young() is true.
> 
> Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> ---
>  arch/riscv/include/uapi/asm/kvm.h |  2 ++
>  arch/riscv/kvm/vcpu.c             |  3 +++
>  arch/riscv/kvm/vcpu_onereg.c      | 15 +++++++++++++++
>  3 files changed, 20 insertions(+)
> 
> diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h
> index e878e7cc3978..a5e0c35d7e9a 100644
> --- a/arch/riscv/include/uapi/asm/kvm.h
> +++ b/arch/riscv/include/uapi/asm/kvm.h
> @@ -168,6 +168,8 @@ enum KVM_RISCV_ISA_EXT_ID {
>  	KVM_RISCV_ISA_EXT_ZTSO,
>  	KVM_RISCV_ISA_EXT_ZACAS,
>  	KVM_RISCV_ISA_EXT_SSCOFPMF,
> +	KVM_RISCV_ISA_EXT_SVADE,
> +	KVM_RISCV_ISA_EXT_SVADU,
>  	KVM_RISCV_ISA_EXT_MAX,
>  };
>  
> diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
> index 17e21df36cc1..64a15af459e0 100644
> --- a/arch/riscv/kvm/vcpu.c
> +++ b/arch/riscv/kvm/vcpu.c
> @@ -540,6 +540,9 @@ static void kvm_riscv_vcpu_setup_config(struct kvm_vcpu *vcpu)
>  	if (riscv_isa_extension_available(isa, ZICBOZ))
>  		cfg->henvcfg |= ENVCFG_CBZE;
>  
> +	if (riscv_isa_extension_available(isa, SVADU))
> +		cfg->henvcfg |= ENVCFG_ADUE;

This is correct for now because patch 1 ensures the host (and therefore also the
guest) never has both Svade and Svadu available. When that changes, this check
will need to add an "&& !riscv_isa_extension_available(isa, SVADE)" condition so
it matches the behavior described in the DT binding. There's no need to resend
to make this addition, but if you do, it wouldn't hurt to include it so it's not
forgotten later. (It looks maybe like v6 only partially implemented Andrew's
suggestion?)

Reviewed-by: Samuel Holland <samuel.holland@sifive.com>

> +
>  	if (riscv_has_extension_unlikely(RISCV_ISA_EXT_SMSTATEEN)) {
>  		cfg->hstateen0 |= SMSTATEEN0_HSENVCFG;
>  		if (riscv_isa_extension_available(isa, SSAIA))
> diff --git a/arch/riscv/kvm/vcpu_onereg.c b/arch/riscv/kvm/vcpu_onereg.c
> index 62874fbca29f..474fdeafe9fe 100644
> --- a/arch/riscv/kvm/vcpu_onereg.c
> +++ b/arch/riscv/kvm/vcpu_onereg.c
> @@ -15,6 +15,7 @@
>  #include <asm/cacheflush.h>
>  #include <asm/cpufeature.h>
>  #include <asm/kvm_vcpu_vector.h>
> +#include <asm/pgtable.h>
>  #include <asm/vector.h>
>  
>  #define KVM_RISCV_BASE_ISA_MASK		GENMASK(25, 0)
> @@ -38,6 +39,8 @@ static const unsigned long kvm_isa_ext_arr[] = {
>  	KVM_ISA_EXT_ARR(SSAIA),
>  	KVM_ISA_EXT_ARR(SSCOFPMF),
>  	KVM_ISA_EXT_ARR(SSTC),
> +	KVM_ISA_EXT_ARR(SVADE),
> +	KVM_ISA_EXT_ARR(SVADU),
>  	KVM_ISA_EXT_ARR(SVINVAL),
>  	KVM_ISA_EXT_ARR(SVNAPOT),
>  	KVM_ISA_EXT_ARR(SVPBMT),
> @@ -105,6 +108,12 @@ static bool kvm_riscv_vcpu_isa_enable_allowed(unsigned long ext)
>  		return __riscv_isa_extension_available(NULL, RISCV_ISA_EXT_SSAIA);
>  	case KVM_RISCV_ISA_EXT_V:
>  		return riscv_v_vstate_ctrl_user_allowed();
> +	case KVM_RISCV_ISA_EXT_SVADU:
> +		/*
> +		 * The henvcfg.ADUE is read-only zero if menvcfg.ADUE is zero.
> +		 * Guest OS can use Svadu only when host os enable Svadu.
> +		 */
> +		return arch_has_hw_pte_young();
>  	default:
>  		break;
>  	}
> @@ -167,6 +176,12 @@ static bool kvm_riscv_vcpu_isa_disable_allowed(unsigned long ext)
>  	/* Extensions which can be disabled using Smstateen */
>  	case KVM_RISCV_ISA_EXT_SSAIA:
>  		return riscv_has_extension_unlikely(RISCV_ISA_EXT_SMSTATEEN);
> +	case KVM_RISCV_ISA_EXT_SVADE:
> +		/*
> +		 * The henvcfg.ADUE is read-only zero if menvcfg.ADUE is zero.
> +		 * Svade is not allowed to disable when the platform use Svade.
> +		 */
> +		return arch_has_hw_pte_young();
>  	default:
>  		break;
>  	}

Re: [PATCH v7 1/4] RISC-V: Add Svade and Svadu Extensions Support

[Yong-Xuan Wang]

Hi Samuel,

On Fri, Jul 19, 2024 at 7:35 AM Samuel Holland
<samuel.holland@sifive.com> wrote:
>
> Hi Yong-Xuan,
>
> Two trivial comments below for if you send another version of the series.
>
> On 2024-07-12 3:38 AM, Yong-Xuan Wang wrote:
> > Svade and Svadu extensions represent two schemes for managing the PTE A/D
> > bits. When the PTE A/D bits need to be set, Svade extension intdicates
> > that a related page fault will be raised. In contrast, the Svadu extension
> > supports hardware updating of PTE A/D bits. Since the Svade extension is
> > mandatory and the Svadu extension is optional in RVA23 profile, by default
> > the M-mode firmware will enable the Svadu extension in the menvcfg CSR
> > when only Svadu is present in DT.
> >
> > This patch detects Svade and Svadu extensions from DT and adds
> > arch_has_hw_pte_young() to enable optimization in MGLRU and
> > __wp_page_copy_user() when we have the PTE A/D bits hardware updating
> > support.
> >
> > Co-developed-by: Jinyu Tang <tjytimi@163.com>
> > Signed-off-by: Jinyu Tang <tjytimi@163.com>
> > Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> > Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> > ---
> >  arch/riscv/Kconfig               |  1 +
> >  arch/riscv/include/asm/csr.h     |  1 +
> >  arch/riscv/include/asm/hwcap.h   |  2 ++
> >  arch/riscv/include/asm/pgtable.h | 13 ++++++++++++-
> >  arch/riscv/kernel/cpufeature.c   | 32 ++++++++++++++++++++++++++++++++
> >  5 files changed, 48 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index 0525ee2d63c7..3d705e28ff85 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -36,6 +36,7 @@ config RISCV
> >       select ARCH_HAS_PMEM_API
> >       select ARCH_HAS_PREPARE_SYNC_CORE_CMD
> >       select ARCH_HAS_PTE_SPECIAL
> > +     select ARCH_HAS_HW_PTE_YOUNG
>
> These lines should be sorted alphabetically.
>
> >       select ARCH_HAS_SET_DIRECT_MAP if MMU
> >       select ARCH_HAS_SET_MEMORY if MMU
> >       select ARCH_HAS_STRICT_KERNEL_RWX if MMU && !XIP_KERNEL
> > diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
> > index 25966995da04..524cd4131c71 100644
> > --- a/arch/riscv/include/asm/csr.h
> > +++ b/arch/riscv/include/asm/csr.h
> > @@ -195,6 +195,7 @@
> >  /* xENVCFG flags */
> >  #define ENVCFG_STCE                  (_AC(1, ULL) << 63)
> >  #define ENVCFG_PBMTE                 (_AC(1, ULL) << 62)
> > +#define ENVCFG_ADUE                  (_AC(1, ULL) << 61)
> >  #define ENVCFG_CBZE                  (_AC(1, UL) << 7)
> >  #define ENVCFG_CBCFE                 (_AC(1, UL) << 6)
> >  #define ENVCFG_CBIE_SHIFT            4
> > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> > index e17d0078a651..35d7aa49785d 100644
> > --- a/arch/riscv/include/asm/hwcap.h
> > +++ b/arch/riscv/include/asm/hwcap.h
> > @@ -81,6 +81,8 @@
> >  #define RISCV_ISA_EXT_ZTSO           72
> >  #define RISCV_ISA_EXT_ZACAS          73
> >  #define RISCV_ISA_EXT_XANDESPMU              74
> > +#define RISCV_ISA_EXT_SVADE             75
>
> The number here should be aligned with tabs, like the surrounding lines.
>
> Regards,
> Samuel
>

I will fix them in the next version. Thank you!

Regards,
Yong-Xuan

> > +#define RISCV_ISA_EXT_SVADU          76
> >
> >  #define RISCV_ISA_EXT_XLINUXENVCFG   127
> >
> > diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
> > index aad8b8ca51f1..ec0cdacd7da0 100644
> > --- a/arch/riscv/include/asm/pgtable.h
> > +++ b/arch/riscv/include/asm/pgtable.h
> > @@ -120,6 +120,7 @@
> >  #include <asm/tlbflush.h>
> >  #include <linux/mm_types.h>
> >  #include <asm/compat.h>
> > +#include <asm/cpufeature.h>
> >
> >  #define __page_val_to_pfn(_val)  (((_val) & _PAGE_PFN_MASK) >> _PAGE_PFN_SHIFT)
> >
> > @@ -288,7 +289,6 @@ static inline pte_t pud_pte(pud_t pud)
> >  }
> >
> >  #ifdef CONFIG_RISCV_ISA_SVNAPOT
> > -#include <asm/cpufeature.h>
> >
> >  static __always_inline bool has_svnapot(void)
> >  {
> > @@ -624,6 +624,17 @@ static inline pgprot_t pgprot_writecombine(pgprot_t _prot)
> >       return __pgprot(prot);
> >  }
> >
> > +/*
> > + * 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.
> > + */
> > +#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);
> > +}
> > +
> >  /*
> >   * THP functions
> >   */
> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > index 5ef48cb20ee1..b2c3fe945e89 100644
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
> > @@ -301,6 +301,8 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
> >       __RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
> >       __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
> >       __RISCV_ISA_EXT_DATA(sstc, RISCV_ISA_EXT_SSTC),
> > +     __RISCV_ISA_EXT_DATA(svade, RISCV_ISA_EXT_SVADE),
> > +     __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),
> > @@ -554,6 +556,21 @@ static void __init riscv_fill_hwcap_from_isa_string(unsigned long *isa2hwcap)
> >                       clear_bit(RISCV_ISA_EXT_v, isainfo->isa);
> >               }
> >
> > +             /*
> > +              * When neither Svade nor Svadu present in DT, it is technically
> > +              * unknown whether the platform uses Svade or Svadu. Supervisor may
> > +              * assume Svade to be present and enabled or it can discover based
> > +              * on mvendorid, marchid, and mimpid. When both Svade and Svadu present
> > +              * in DT, supervisor must assume Svadu turned-off at boot time. To use
> > +              * Svadu, supervisor must explicitly enable it using the SBI FWFT extension.
> > +              */
> > +             if (!test_bit(RISCV_ISA_EXT_SVADE, isainfo->isa) &&
> > +                 !test_bit(RISCV_ISA_EXT_SVADU, isainfo->isa))
> > +                     set_bit(RISCV_ISA_EXT_SVADE, isainfo->isa);
> > +             else if (test_bit(RISCV_ISA_EXT_SVADE, isainfo->isa) &&
> > +                      test_bit(RISCV_ISA_EXT_SVADU, isainfo->isa))
> > +                     clear_bit(RISCV_ISA_EXT_SVADU, isainfo->isa);
> > +
> >               /*
> >                * All "okay" hart should have same isa. Set HWCAP based on
> >                * common capabilities of every "okay" hart, in case they don't
> > @@ -619,6 +636,21 @@ static int __init riscv_fill_hwcap_from_ext_list(unsigned long *isa2hwcap)
> >
> >               of_node_put(cpu_node);
> >
> > +             /*
> > +              * When neither Svade nor Svadu present in DT, it is technically
> > +              * unknown whether the platform uses Svade or Svadu. Supervisor may
> > +              * assume Svade to be present and enabled or it can discover based
> > +              * on mvendorid, marchid, and mimpid. When both Svade and Svadu present
> > +              * in DT, supervisor must assume Svadu turned-off at boot time. To use
> > +              * Svadu, supervisor must explicitly enable it using the SBI FWFT extension.
> > +              */
> > +             if (!test_bit(RISCV_ISA_EXT_SVADE, isainfo->isa) &&
> > +                 !test_bit(RISCV_ISA_EXT_SVADU, isainfo->isa))
> > +                     set_bit(RISCV_ISA_EXT_SVADE, isainfo->isa);
> > +             else if (test_bit(RISCV_ISA_EXT_SVADE, isainfo->isa) &&
> > +                      test_bit(RISCV_ISA_EXT_SVADU, isainfo->isa))
> > +                     clear_bit(RISCV_ISA_EXT_SVADU, isainfo->isa);
> > +
> >               /*
> >                * All "okay" harts should have same isa. Set HWCAP based on
> >                * common capabilities of every "okay" hart, in case they don't.
>

Re: [PATCH v7 3/4] RISC-V: KVM: Add Svade and Svadu Extensions Support for Guest/VM

[Yong-Xuan Wang]

Hi Samuel,

On Fri, Jul 19, 2024 at 8:22 AM Samuel Holland
<samuel.holland@sifive.com> wrote:
>
> Hi Yong-Xuan,
>
> On 2024-07-12 3:38 AM, Yong-Xuan Wang wrote:
> > We extend the KVM ISA extension ONE_REG interface to allow VMM tools to
> > detect and enable Svade and Svadu extensions for Guest/VM. Since the
> > henvcfg.ADUE is read-only zero if the menvcfg.ADUE is zero, the Svadu
> > extension is available for Guest/VM and the Svade extension is allowed
> > to disabledonly when arch_has_hw_pte_young() is true.
> >
> > Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> > Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> > ---
> >  arch/riscv/include/uapi/asm/kvm.h |  2 ++
> >  arch/riscv/kvm/vcpu.c             |  3 +++
> >  arch/riscv/kvm/vcpu_onereg.c      | 15 +++++++++++++++
> >  3 files changed, 20 insertions(+)
> >
> > diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h
> > index e878e7cc3978..a5e0c35d7e9a 100644
> > --- a/arch/riscv/include/uapi/asm/kvm.h
> > +++ b/arch/riscv/include/uapi/asm/kvm.h
> > @@ -168,6 +168,8 @@ enum KVM_RISCV_ISA_EXT_ID {
> >       KVM_RISCV_ISA_EXT_ZTSO,
> >       KVM_RISCV_ISA_EXT_ZACAS,
> >       KVM_RISCV_ISA_EXT_SSCOFPMF,
> > +     KVM_RISCV_ISA_EXT_SVADE,
> > +     KVM_RISCV_ISA_EXT_SVADU,
> >       KVM_RISCV_ISA_EXT_MAX,
> >  };
> >
> > diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
> > index 17e21df36cc1..64a15af459e0 100644
> > --- a/arch/riscv/kvm/vcpu.c
> > +++ b/arch/riscv/kvm/vcpu.c
> > @@ -540,6 +540,9 @@ static void kvm_riscv_vcpu_setup_config(struct kvm_vcpu *vcpu)
> >       if (riscv_isa_extension_available(isa, ZICBOZ))
> >               cfg->henvcfg |= ENVCFG_CBZE;
> >
> > +     if (riscv_isa_extension_available(isa, SVADU))
> > +             cfg->henvcfg |= ENVCFG_ADUE;
>
> This is correct for now because patch 1 ensures the host (and therefore also the
> guest) never has both Svade and Svadu available. When that changes, this check
> will need to add an "&& !riscv_isa_extension_available(isa, SVADE)" condition so
> it matches the behavior described in the DT binding. There's no need to resend
> to make this addition, but if you do, it wouldn't hurt to include it so it's not
> forgotten later. (It looks maybe like v6 only partially implemented Andrew's
> suggestion?)
>
> Reviewed-by: Samuel Holland <samuel.holland@sifive.com>
>

Yeah, since the PATCH1 can ensure that only Svade or Svadu will be used, so I
removed the Svade checking there. I will add it back in the next version. Thank
you!

Regards,
Yong-Xuan

> > +
> >       if (riscv_has_extension_unlikely(RISCV_ISA_EXT_SMSTATEEN)) {
> >               cfg->hstateen0 |= SMSTATEEN0_HSENVCFG;
> >               if (riscv_isa_extension_available(isa, SSAIA))
> > diff --git a/arch/riscv/kvm/vcpu_onereg.c b/arch/riscv/kvm/vcpu_onereg.c
> > index 62874fbca29f..474fdeafe9fe 100644
> > --- a/arch/riscv/kvm/vcpu_onereg.c
> > +++ b/arch/riscv/kvm/vcpu_onereg.c
> > @@ -15,6 +15,7 @@
> >  #include <asm/cacheflush.h>
> >  #include <asm/cpufeature.h>
> >  #include <asm/kvm_vcpu_vector.h>
> > +#include <asm/pgtable.h>
> >  #include <asm/vector.h>
> >
> >  #define KVM_RISCV_BASE_ISA_MASK              GENMASK(25, 0)
> > @@ -38,6 +39,8 @@ static const unsigned long kvm_isa_ext_arr[] = {
> >       KVM_ISA_EXT_ARR(SSAIA),
> >       KVM_ISA_EXT_ARR(SSCOFPMF),
> >       KVM_ISA_EXT_ARR(SSTC),
> > +     KVM_ISA_EXT_ARR(SVADE),
> > +     KVM_ISA_EXT_ARR(SVADU),
> >       KVM_ISA_EXT_ARR(SVINVAL),
> >       KVM_ISA_EXT_ARR(SVNAPOT),
> >       KVM_ISA_EXT_ARR(SVPBMT),
> > @@ -105,6 +108,12 @@ static bool kvm_riscv_vcpu_isa_enable_allowed(unsigned long ext)
> >               return __riscv_isa_extension_available(NULL, RISCV_ISA_EXT_SSAIA);
> >       case KVM_RISCV_ISA_EXT_V:
> >               return riscv_v_vstate_ctrl_user_allowed();
> > +     case KVM_RISCV_ISA_EXT_SVADU:
> > +             /*
> > +              * The henvcfg.ADUE is read-only zero if menvcfg.ADUE is zero.
> > +              * Guest OS can use Svadu only when host os enable Svadu.
> > +              */
> > +             return arch_has_hw_pte_young();
> >       default:
> >               break;
> >       }
> > @@ -167,6 +176,12 @@ static bool kvm_riscv_vcpu_isa_disable_allowed(unsigned long ext)
> >       /* Extensions which can be disabled using Smstateen */
> >       case KVM_RISCV_ISA_EXT_SSAIA:
> >               return riscv_has_extension_unlikely(RISCV_ISA_EXT_SMSTATEEN);
> > +     case KVM_RISCV_ISA_EXT_SVADE:
> > +             /*
> > +              * The henvcfg.ADUE is read-only zero if menvcfg.ADUE is zero.
> > +              * Svade is not allowed to disable when the platform use Svade.
> > +              */
> > +             return arch_has_hw_pte_young();
> >       default:
> >               break;
> >       }
>

Re: [PATCH v7 2/4] dt-bindings: riscv: Add Svade and Svadu Entries

[Yong-Xuan Wang]

Hi Samuel,

On Fri, Jul 19, 2024 at 7:38 AM Samuel Holland
<samuel.holland@sifive.com> wrote:
>
> On 2024-07-12 3:38 AM, Yong-Xuan Wang wrote:
> > Add entries for the Svade and Svadu extensions to the riscv,isa-extensions
> > property.
> >
> > Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> > ---
> >  .../devicetree/bindings/riscv/extensions.yaml | 28 +++++++++++++++++++
> >  1 file changed, 28 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml
> > index 468c646247aa..e91a6f4ede38 100644
> > --- a/Documentation/devicetree/bindings/riscv/extensions.yaml
> > +++ b/Documentation/devicetree/bindings/riscv/extensions.yaml
> > @@ -153,6 +153,34 @@ properties:
> >              ratified at commit 3f9ed34 ("Add ability to manually trigger
> >              workflow. (#2)") of riscv-time-compare.
> >
> > +        - const: svade
> > +          description: |
> > +            The standard Svade supervisor-level extension for SW-managed PTE A/D
> > +            bit updates as ratified in the 20240213 version of the privileged
> > +            ISA specification.
> > +
> > +            Both Svade and Svadu extensions control the hardware behavior when
> > +            the PTE A/D bits need to be set. The default behavior for the four
> > +            possible combinations of these extensions in the device tree are:
> > +            1) Neither Svade nor Svadu present in DT => It is technically
> > +               unknown whether the platform uses Svade or Svadu. Supervisor
> > +               software should be prepared to handle either hardware updating
> > +               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.
> > +            4) Both Svade and Svadu present in DT => Supervisor must assume
> > +               Svadu turned-off at boot time. To use Svadu, supervisor must
> > +               explicitly enable it using the SBI FWFT extension.
> > +
> > +        - const: svadu
> > +          description: |
> > +            The standard Svadu supervisor-level extension for hardware updating
> > +            of PTE A/D bits as ratified at commit c1abccf ("Merge pull request
> > +            #25 from ved-rivos/ratified") of riscv-svadu. Please refer to Svade
>
> Should we be referencing the archived riscv-svadu repository now that Svadu has
> been merged to the main privileged ISA manual? Either way:
>
> Reviewed-by: Samuel Holland <samuel.holland@sifive.com>
>

Yes, this commit is from the archived riscv-svadu repo. Or should I update it to
"commit c1abccf ("Merge pull request  #25 from ved-rivos/ratified") of
riscvarchive/riscv-svadu."?

Regards,
Yong-Xuan

> > +            dt-binding description for more details.
> > +
> >          - const: svinval
> >            description:
> >              The standard Svinval supervisor-level extension for fine-grained
>

Re: [PATCH v7 1/4] RISC-V: Add Svade and Svadu Extensions Support

[Clément Léger]

Hi Yong-Xuan,


On 18/07/2024 18:43, Alexandre Ghiti wrote:
> Hi Yong-Xuan,
> 
> On 12/07/2024 10:38, Yong-Xuan Wang wrote:
>> Svade and Svadu extensions represent two schemes for managing the PTE A/D
>> bits. When the PTE A/D bits need to be set, Svade extension intdicates
>> that a related page fault will be raised. In contrast, the Svadu
>> extension
>> supports hardware updating of PTE A/D bits. Since the Svade extension is
>> mandatory and the Svadu extension is optional in RVA23 profile, by
>> default
>> the M-mode firmware will enable the Svadu extension in the menvcfg CSR
>> when only Svadu is present in DT.
>>
>> This patch detects Svade and Svadu extensions from DT and adds
>> arch_has_hw_pte_young() to enable optimization in MGLRU and
>> __wp_page_copy_user() when we have the PTE A/D bits hardware updating
>> support.
>>
>> Co-developed-by: Jinyu Tang <tjytimi@163.com>
>> Signed-off-by: Jinyu Tang <tjytimi@163.com>
>> Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
>> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
>> ---
>>   arch/riscv/Kconfig               |  1 +
>>   arch/riscv/include/asm/csr.h     |  1 +
>>   arch/riscv/include/asm/hwcap.h   |  2 ++
>>   arch/riscv/include/asm/pgtable.h | 13 ++++++++++++-
>>   arch/riscv/kernel/cpufeature.c   | 32 ++++++++++++++++++++++++++++++++
>>   5 files changed, 48 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>> index 0525ee2d63c7..3d705e28ff85 100644
>> --- a/arch/riscv/Kconfig
>> +++ b/arch/riscv/Kconfig
>> @@ -36,6 +36,7 @@ config RISCV
>>       select ARCH_HAS_PMEM_API
>>       select ARCH_HAS_PREPARE_SYNC_CORE_CMD
>>       select ARCH_HAS_PTE_SPECIAL
>> +    select ARCH_HAS_HW_PTE_YOUNG
>>       select ARCH_HAS_SET_DIRECT_MAP if MMU
>>       select ARCH_HAS_SET_MEMORY if MMU
>>       select ARCH_HAS_STRICT_KERNEL_RWX if MMU && !XIP_KERNEL
>> diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
>> index 25966995da04..524cd4131c71 100644
>> --- a/arch/riscv/include/asm/csr.h
>> +++ b/arch/riscv/include/asm/csr.h
>> @@ -195,6 +195,7 @@
>>   /* xENVCFG flags */
>>   #define ENVCFG_STCE            (_AC(1, ULL) << 63)
>>   #define ENVCFG_PBMTE            (_AC(1, ULL) << 62)
>> +#define ENVCFG_ADUE            (_AC(1, ULL) << 61)
>>   #define ENVCFG_CBZE            (_AC(1, UL) << 7)
>>   #define ENVCFG_CBCFE            (_AC(1, UL) << 6)
>>   #define ENVCFG_CBIE_SHIFT        4
>> diff --git a/arch/riscv/include/asm/hwcap.h
>> b/arch/riscv/include/asm/hwcap.h
>> index e17d0078a651..35d7aa49785d 100644
>> --- a/arch/riscv/include/asm/hwcap.h
>> +++ b/arch/riscv/include/asm/hwcap.h
>> @@ -81,6 +81,8 @@
>>   #define RISCV_ISA_EXT_ZTSO        72
>>   #define RISCV_ISA_EXT_ZACAS        73
>>   #define RISCV_ISA_EXT_XANDESPMU        74
>> +#define RISCV_ISA_EXT_SVADE             75
>> +#define RISCV_ISA_EXT_SVADU        76
>>     #define RISCV_ISA_EXT_XLINUXENVCFG    127
>>   diff --git a/arch/riscv/include/asm/pgtable.h
>> b/arch/riscv/include/asm/pgtable.h
>> index aad8b8ca51f1..ec0cdacd7da0 100644
>> --- a/arch/riscv/include/asm/pgtable.h
>> +++ b/arch/riscv/include/asm/pgtable.h
>> @@ -120,6 +120,7 @@
>>   #include <asm/tlbflush.h>
>>   #include <linux/mm_types.h>
>>   #include <asm/compat.h>
>> +#include <asm/cpufeature.h>
>>     #define __page_val_to_pfn(_val)  (((_val) & _PAGE_PFN_MASK) >>
>> _PAGE_PFN_SHIFT)
>>   @@ -288,7 +289,6 @@ static inline pte_t pud_pte(pud_t pud)
>>   }
>>     #ifdef CONFIG_RISCV_ISA_SVNAPOT
>> -#include <asm/cpufeature.h>
>>     static __always_inline bool has_svnapot(void)
>>   {
>> @@ -624,6 +624,17 @@ static inline pgprot_t
>> pgprot_writecombine(pgprot_t _prot)
>>       return __pgprot(prot);
>>   }
>>   +/*
>> + * 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.
>> + */
>> +#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);
>> +}
>> +
>>   /*
>>    * THP functions
>>    */
>> diff --git a/arch/riscv/kernel/cpufeature.c
>> b/arch/riscv/kernel/cpufeature.c
>> index 5ef48cb20ee1..b2c3fe945e89 100644
>> --- a/arch/riscv/kernel/cpufeature.c
>> +++ b/arch/riscv/kernel/cpufeature.c
>> @@ -301,6 +301,8 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
>>       __RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
>>       __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
>>       __RISCV_ISA_EXT_DATA(sstc, RISCV_ISA_EXT_SSTC),
>> +    __RISCV_ISA_EXT_DATA(svade, RISCV_ISA_EXT_SVADE),
>> +    __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),
>> @@ -554,6 +556,21 @@ static void __init
>> riscv_fill_hwcap_from_isa_string(unsigned long *isa2hwcap)
>>               clear_bit(RISCV_ISA_EXT_v, isainfo->isa);
>>           }
>>   +        /*
>> +         * When neither Svade nor Svadu present in DT, it is technically
>> +         * unknown whether the platform uses Svade or Svadu.
>> Supervisor may
>> +         * assume Svade to be present and enabled or it can discover
>> based
>> +         * on mvendorid, marchid, and mimpid. When both Svade and
>> Svadu present
>> +         * in DT, supervisor must assume Svadu turned-off at boot
>> time. To use
>> +         * Svadu, supervisor must explicitly enable it using the SBI
>> FWFT extension.
>> +         */
>> +        if (!test_bit(RISCV_ISA_EXT_SVADE, isainfo->isa) &&
>> +            !test_bit(RISCV_ISA_EXT_SVADU, isainfo->isa))
>> +            set_bit(RISCV_ISA_EXT_SVADE, isainfo->isa);
>> +        else if (test_bit(RISCV_ISA_EXT_SVADE, isainfo->isa) &&
>> +             test_bit(RISCV_ISA_EXT_SVADU, isainfo->isa))
>> +            clear_bit(RISCV_ISA_EXT_SVADU, isainfo->isa);
>> +
>>           /*
>>            * All "okay" hart should have same isa. Set HWCAP based on
>>            * common capabilities of every "okay" hart, in case they don't
>> @@ -619,6 +636,21 @@ static int __init
>> riscv_fill_hwcap_from_ext_list(unsigned long *isa2hwcap)
>>             of_node_put(cpu_node);
>>   +        /*
>> +         * When neither Svade nor Svadu present in DT, it is technically
>> +         * unknown whether the platform uses Svade or Svadu.
>> Supervisor may
>> +         * assume Svade to be present and enabled or it can discover
>> based
>> +         * on mvendorid, marchid, and mimpid. When both Svade and
>> Svadu present
>> +         * in DT, supervisor must assume Svadu turned-off at boot
>> time. To use
>> +         * Svadu, supervisor must explicitly enable it using the SBI
>> FWFT extension.
>> +         */
>> +        if (!test_bit(RISCV_ISA_EXT_SVADE, isainfo->isa) &&
>> +            !test_bit(RISCV_ISA_EXT_SVADU, isainfo->isa))
>> +            set_bit(RISCV_ISA_EXT_SVADE, isainfo->isa);
>> +        else if (test_bit(RISCV_ISA_EXT_SVADE, isainfo->isa) &&
>> +             test_bit(RISCV_ISA_EXT_SVADU, isainfo->isa))
>> +            clear_bit(RISCV_ISA_EXT_SVADU, isainfo->isa);
>> +

This is a duplicate of the previous chunk of code. Moreover, now that we
have a .validate callback for ISA extension (in for-next), I would
prefer this to be based on that support rather that having duplicated
extension specific handling code.

I think this could be translated (almost) using the following
.validate() callback for SVADU/SVADE extension:

static int riscv_ext_svadu_validate(const struct riscv_isa_ext_data *data,
				  const unsigned long *isa_bitmap)
{
	/* SVADE has already been detected, use SVADE only */
	if (__riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_SVADE))
		return -ENOTSUPP;

	return 0;
}

static int riscv_ext_svade_validate(const struct riscv_isa_ext_data *data,
				  const unsigned long *isa_bitmap)
{
	/* Clear SVADU, it will be enable using the FWFT extension if present */
	clear_bit(RISCV_ISA_EXT_SVADU, isa_bitmap);

	return 0;
}

However, this will not enable SVADE if neither SVADU/SVADE are set (as
done by your patch) but since SVADE does not seems to be used explicitly
in your patch series, I think it is sane to keep it like that.

Thanks,

Clément



>>           /*
>>            * All "okay" harts should have same isa. Set HWCAP based on
>>            * common capabilities of every "okay" hart, in case they
>> don't.
> 
> 
> Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> 
> Thanks,
> 
> Alex
> 

Re: [PATCH v7 2/4] dt-bindings: riscv: Add Svade and Svadu Entries

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3281 bytes --]

On Fri, Jul 19, 2024 at 02:58:59PM +0800, Yong-Xuan Wang wrote:
> Hi Samuel,
> 
> On Fri, Jul 19, 2024 at 7:38 AM Samuel Holland
> <samuel.holland@sifive.com> wrote:
> >
> > On 2024-07-12 3:38 AM, Yong-Xuan Wang wrote:
> > > Add entries for the Svade and Svadu extensions to the riscv,isa-extensions
> > > property.
> > >
> > > Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> > > ---
> > >  .../devicetree/bindings/riscv/extensions.yaml | 28 +++++++++++++++++++
> > >  1 file changed, 28 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml
> > > index 468c646247aa..e91a6f4ede38 100644
> > > --- a/Documentation/devicetree/bindings/riscv/extensions.yaml
> > > +++ b/Documentation/devicetree/bindings/riscv/extensions.yaml
> > > @@ -153,6 +153,34 @@ properties:
> > >              ratified at commit 3f9ed34 ("Add ability to manually trigger
> > >              workflow. (#2)") of riscv-time-compare.
> > >
> > > +        - const: svade
> > > +          description: |
> > > +            The standard Svade supervisor-level extension for SW-managed PTE A/D
> > > +            bit updates as ratified in the 20240213 version of the privileged
> > > +            ISA specification.
> > > +
> > > +            Both Svade and Svadu extensions control the hardware behavior when
> > > +            the PTE A/D bits need to be set. The default behavior for the four
> > > +            possible combinations of these extensions in the device tree are:
> > > +            1) Neither Svade nor Svadu present in DT => It is technically
> > > +               unknown whether the platform uses Svade or Svadu. Supervisor
> > > +               software should be prepared to handle either hardware updating
> > > +               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.
> > > +            4) Both Svade and Svadu present in DT => Supervisor must assume
> > > +               Svadu turned-off at boot time. To use Svadu, supervisor must
> > > +               explicitly enable it using the SBI FWFT extension.
> > > +
> > > +        - const: svadu
> > > +          description: |
> > > +            The standard Svadu supervisor-level extension for hardware updating
> > > +            of PTE A/D bits as ratified at commit c1abccf ("Merge pull request
> > > +            #25 from ved-rivos/ratified") of riscv-svadu. Please refer to Svade
> >
> > Should we be referencing the archived riscv-svadu repository now that Svadu has
> > been merged to the main privileged ISA manual? Either way:
> >
> > Reviewed-by: Samuel Holland <samuel.holland@sifive.com>
> >
> 
> Yes, this commit is from the archived riscv-svadu repo. Or should I update it to
> "commit c1abccf ("Merge pull request  #25 from ved-rivos/ratified") of
> riscvarchive/riscv-svadu."?

I think Samuel was saying that we should use the commit where it was
merged into riscv-isa-manual instead.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v7 2/4] dt-bindings: riscv: Add Svade and Svadu Entries

[Yong-Xuan Wang]

Hi Conor,

On Fri, Jul 19, 2024 at 9:17 PM Conor Dooley <conor@kernel.org> wrote:
>
> On Fri, Jul 19, 2024 at 02:58:59PM +0800, Yong-Xuan Wang wrote:
> > Hi Samuel,
> >
> > On Fri, Jul 19, 2024 at 7:38 AM Samuel Holland
> > <samuel.holland@sifive.com> wrote:
> > >
> > > On 2024-07-12 3:38 AM, Yong-Xuan Wang wrote:
> > > > Add entries for the Svade and Svadu extensions to the riscv,isa-extensions
> > > > property.
> > > >
> > > > Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> > > > ---
> > > >  .../devicetree/bindings/riscv/extensions.yaml | 28 +++++++++++++++++++
> > > >  1 file changed, 28 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml
> > > > index 468c646247aa..e91a6f4ede38 100644
> > > > --- a/Documentation/devicetree/bindings/riscv/extensions.yaml
> > > > +++ b/Documentation/devicetree/bindings/riscv/extensions.yaml
> > > > @@ -153,6 +153,34 @@ properties:
> > > >              ratified at commit 3f9ed34 ("Add ability to manually trigger
> > > >              workflow. (#2)") of riscv-time-compare.
> > > >
> > > > +        - const: svade
> > > > +          description: |
> > > > +            The standard Svade supervisor-level extension for SW-managed PTE A/D
> > > > +            bit updates as ratified in the 20240213 version of the privileged
> > > > +            ISA specification.
> > > > +
> > > > +            Both Svade and Svadu extensions control the hardware behavior when
> > > > +            the PTE A/D bits need to be set. The default behavior for the four
> > > > +            possible combinations of these extensions in the device tree are:
> > > > +            1) Neither Svade nor Svadu present in DT => It is technically
> > > > +               unknown whether the platform uses Svade or Svadu. Supervisor
> > > > +               software should be prepared to handle either hardware updating
> > > > +               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.
> > > > +            4) Both Svade and Svadu present in DT => Supervisor must assume
> > > > +               Svadu turned-off at boot time. To use Svadu, supervisor must
> > > > +               explicitly enable it using the SBI FWFT extension.
> > > > +
> > > > +        - const: svadu
> > > > +          description: |
> > > > +            The standard Svadu supervisor-level extension for hardware updating
> > > > +            of PTE A/D bits as ratified at commit c1abccf ("Merge pull request
> > > > +            #25 from ved-rivos/ratified") of riscv-svadu. Please refer to Svade
> > >
> > > Should we be referencing the archived riscv-svadu repository now that Svadu has
> > > been merged to the main privileged ISA manual? Either way:
> > >
> > > Reviewed-by: Samuel Holland <samuel.holland@sifive.com>
> > >
> >
> > Yes, this commit is from the archived riscv-svadu repo. Or should I update it to
> > "commit c1abccf ("Merge pull request  #25 from ved-rivos/ratified") of
> > riscvarchive/riscv-svadu."?
>
> I think Samuel was saying that we should use the commit where it was
> merged into riscv-isa-manual instead.

Got it. I will update the description in the next version. Thank you!

Re: [PATCH v7 1/4] RISC-V: Add Svade and Svadu Extensions Support

[Yong-Xuan Wang]

Hi Clément,

On Fri, Jul 19, 2024 at 3:38 PM Clément Léger <cleger@rivosinc.com> wrote:
>
>
>
> Hi Yong-Xuan,
>
>
> On 18/07/2024 18:43, Alexandre Ghiti wrote:
> > Hi Yong-Xuan,
> >
> > On 12/07/2024 10:38, Yong-Xuan Wang wrote:
> >> Svade and Svadu extensions represent two schemes for managing the PTE A/D
> >> bits. When the PTE A/D bits need to be set, Svade extension intdicates
> >> that a related page fault will be raised. In contrast, the Svadu
> >> extension
> >> supports hardware updating of PTE A/D bits. Since the Svade extension is
> >> mandatory and the Svadu extension is optional in RVA23 profile, by
> >> default
> >> the M-mode firmware will enable the Svadu extension in the menvcfg CSR
> >> when only Svadu is present in DT.
> >>
> >> This patch detects Svade and Svadu extensions from DT and adds
> >> arch_has_hw_pte_young() to enable optimization in MGLRU and
> >> __wp_page_copy_user() when we have the PTE A/D bits hardware updating
> >> support.
> >>
> >> Co-developed-by: Jinyu Tang <tjytimi@163.com>
> >> Signed-off-by: Jinyu Tang <tjytimi@163.com>
> >> Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> >> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> >> ---
> >>   arch/riscv/Kconfig               |  1 +
> >>   arch/riscv/include/asm/csr.h     |  1 +
> >>   arch/riscv/include/asm/hwcap.h   |  2 ++
> >>   arch/riscv/include/asm/pgtable.h | 13 ++++++++++++-
> >>   arch/riscv/kernel/cpufeature.c   | 32 ++++++++++++++++++++++++++++++++
> >>   5 files changed, 48 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> >> index 0525ee2d63c7..3d705e28ff85 100644
> >> --- a/arch/riscv/Kconfig
> >> +++ b/arch/riscv/Kconfig
> >> @@ -36,6 +36,7 @@ config RISCV
> >>       select ARCH_HAS_PMEM_API
> >>       select ARCH_HAS_PREPARE_SYNC_CORE_CMD
> >>       select ARCH_HAS_PTE_SPECIAL
> >> +    select ARCH_HAS_HW_PTE_YOUNG
> >>       select ARCH_HAS_SET_DIRECT_MAP if MMU
> >>       select ARCH_HAS_SET_MEMORY if MMU
> >>       select ARCH_HAS_STRICT_KERNEL_RWX if MMU && !XIP_KERNEL
> >> diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
> >> index 25966995da04..524cd4131c71 100644
> >> --- a/arch/riscv/include/asm/csr.h
> >> +++ b/arch/riscv/include/asm/csr.h
> >> @@ -195,6 +195,7 @@
> >>   /* xENVCFG flags */
> >>   #define ENVCFG_STCE            (_AC(1, ULL) << 63)
> >>   #define ENVCFG_PBMTE            (_AC(1, ULL) << 62)
> >> +#define ENVCFG_ADUE            (_AC(1, ULL) << 61)
> >>   #define ENVCFG_CBZE            (_AC(1, UL) << 7)
> >>   #define ENVCFG_CBCFE            (_AC(1, UL) << 6)
> >>   #define ENVCFG_CBIE_SHIFT        4
> >> diff --git a/arch/riscv/include/asm/hwcap.h
> >> b/arch/riscv/include/asm/hwcap.h
> >> index e17d0078a651..35d7aa49785d 100644
> >> --- a/arch/riscv/include/asm/hwcap.h
> >> +++ b/arch/riscv/include/asm/hwcap.h
> >> @@ -81,6 +81,8 @@
> >>   #define RISCV_ISA_EXT_ZTSO        72
> >>   #define RISCV_ISA_EXT_ZACAS        73
> >>   #define RISCV_ISA_EXT_XANDESPMU        74
> >> +#define RISCV_ISA_EXT_SVADE             75
> >> +#define RISCV_ISA_EXT_SVADU        76
> >>     #define RISCV_ISA_EXT_XLINUXENVCFG    127
> >>   diff --git a/arch/riscv/include/asm/pgtable.h
> >> b/arch/riscv/include/asm/pgtable.h
> >> index aad8b8ca51f1..ec0cdacd7da0 100644
> >> --- a/arch/riscv/include/asm/pgtable.h
> >> +++ b/arch/riscv/include/asm/pgtable.h
> >> @@ -120,6 +120,7 @@
> >>   #include <asm/tlbflush.h>
> >>   #include <linux/mm_types.h>
> >>   #include <asm/compat.h>
> >> +#include <asm/cpufeature.h>
> >>     #define __page_val_to_pfn(_val)  (((_val) & _PAGE_PFN_MASK) >>
> >> _PAGE_PFN_SHIFT)
> >>   @@ -288,7 +289,6 @@ static inline pte_t pud_pte(pud_t pud)
> >>   }
> >>     #ifdef CONFIG_RISCV_ISA_SVNAPOT
> >> -#include <asm/cpufeature.h>
> >>     static __always_inline bool has_svnapot(void)
> >>   {
> >> @@ -624,6 +624,17 @@ static inline pgprot_t
> >> pgprot_writecombine(pgprot_t _prot)
> >>       return __pgprot(prot);
> >>   }
> >>   +/*
> >> + * 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.
> >> + */
> >> +#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);
> >> +}
> >> +
> >>   /*
> >>    * THP functions
> >>    */
> >> diff --git a/arch/riscv/kernel/cpufeature.c
> >> b/arch/riscv/kernel/cpufeature.c
> >> index 5ef48cb20ee1..b2c3fe945e89 100644
> >> --- a/arch/riscv/kernel/cpufeature.c
> >> +++ b/arch/riscv/kernel/cpufeature.c
> >> @@ -301,6 +301,8 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
> >>       __RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
> >>       __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
> >>       __RISCV_ISA_EXT_DATA(sstc, RISCV_ISA_EXT_SSTC),
> >> +    __RISCV_ISA_EXT_DATA(svade, RISCV_ISA_EXT_SVADE),
> >> +    __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),
> >> @@ -554,6 +556,21 @@ static void __init
> >> riscv_fill_hwcap_from_isa_string(unsigned long *isa2hwcap)
> >>               clear_bit(RISCV_ISA_EXT_v, isainfo->isa);
> >>           }
> >>   +        /*
> >> +         * When neither Svade nor Svadu present in DT, it is technically
> >> +         * unknown whether the platform uses Svade or Svadu.
> >> Supervisor may
> >> +         * assume Svade to be present and enabled or it can discover
> >> based
> >> +         * on mvendorid, marchid, and mimpid. When both Svade and
> >> Svadu present
> >> +         * in DT, supervisor must assume Svadu turned-off at boot
> >> time. To use
> >> +         * Svadu, supervisor must explicitly enable it using the SBI
> >> FWFT extension.
> >> +         */
> >> +        if (!test_bit(RISCV_ISA_EXT_SVADE, isainfo->isa) &&
> >> +            !test_bit(RISCV_ISA_EXT_SVADU, isainfo->isa))
> >> +            set_bit(RISCV_ISA_EXT_SVADE, isainfo->isa);
> >> +        else if (test_bit(RISCV_ISA_EXT_SVADE, isainfo->isa) &&
> >> +             test_bit(RISCV_ISA_EXT_SVADU, isainfo->isa))
> >> +            clear_bit(RISCV_ISA_EXT_SVADU, isainfo->isa);
> >> +
> >>           /*
> >>            * All "okay" hart should have same isa. Set HWCAP based on
> >>            * common capabilities of every "okay" hart, in case they don't
> >> @@ -619,6 +636,21 @@ static int __init
> >> riscv_fill_hwcap_from_ext_list(unsigned long *isa2hwcap)
> >>             of_node_put(cpu_node);
> >>   +        /*
> >> +         * When neither Svade nor Svadu present in DT, it is technically
> >> +         * unknown whether the platform uses Svade or Svadu.
> >> Supervisor may
> >> +         * assume Svade to be present and enabled or it can discover
> >> based
> >> +         * on mvendorid, marchid, and mimpid. When both Svade and
> >> Svadu present
> >> +         * in DT, supervisor must assume Svadu turned-off at boot
> >> time. To use
> >> +         * Svadu, supervisor must explicitly enable it using the SBI
> >> FWFT extension.
> >> +         */
> >> +        if (!test_bit(RISCV_ISA_EXT_SVADE, isainfo->isa) &&
> >> +            !test_bit(RISCV_ISA_EXT_SVADU, isainfo->isa))
> >> +            set_bit(RISCV_ISA_EXT_SVADE, isainfo->isa);
> >> +        else if (test_bit(RISCV_ISA_EXT_SVADE, isainfo->isa) &&
> >> +             test_bit(RISCV_ISA_EXT_SVADU, isainfo->isa))
> >> +            clear_bit(RISCV_ISA_EXT_SVADU, isainfo->isa);
> >> +
>
> This is a duplicate of the previous chunk of code. Moreover, now that we
> have a .validate callback for ISA extension (in for-next), I would
> prefer this to be based on that support rather that having duplicated
> extension specific handling code.
>
> I think this could be translated (almost) using the following
> .validate() callback for SVADU/SVADE extension:
>
> static int riscv_ext_svadu_validate(const struct riscv_isa_ext_data *data,
>                                   const unsigned long *isa_bitmap)
> {
>         /* SVADE has already been detected, use SVADE only */
>         if (__riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_SVADE))
>                 return -ENOTSUPP;
>
>         return 0;
> }
>
> static int riscv_ext_svade_validate(const struct riscv_isa_ext_data *data,
>                                   const unsigned long *isa_bitmap)
> {
>         /* Clear SVADU, it will be enable using the FWFT extension if present */
>         clear_bit(RISCV_ISA_EXT_SVADU, isa_bitmap);
>
>         return 0;
> }
>
> However, this will not enable SVADE if neither SVADU/SVADE are set (as
> done by your patch) but since SVADE does not seems to be used explicitly
> in your patch series, I think it is sane to keep it like that.
>
> Thanks,
>
> Clément
>
>

Got it. I will rebase to the for-next branch. Thank you!

Regards,
Yong-Xuan

>
> >>           /*
> >>            * All "okay" harts should have same isa. Set HWCAP based on
> >>            * common capabilities of every "okay" hart, in case they
> >> don't.
> >
> >
> > Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> >
> > Thanks,
> >
> > Alex
> >

Re: [PATCH v7 2/4] dt-bindings: riscv: Add Svade and Svadu Entries

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1228 bytes --]

On Mon, Jul 22, 2024 at 10:14:11AM +0800, Yong-Xuan Wang wrote:

> > > > > +        - const: svadu
> > > > > +          description: |
> > > > > +            The standard Svadu supervisor-level extension for hardware updating
> > > > > +            of PTE A/D bits as ratified at commit c1abccf ("Merge pull request
> > > > > +            #25 from ved-rivos/ratified") of riscv-svadu. Please refer to Svade
> > > >
> > > > Should we be referencing the archived riscv-svadu repository now that Svadu has
> > > > been merged to the main privileged ISA manual? Either way:
> > > >
> > > > Reviewed-by: Samuel Holland <samuel.holland@sifive.com>
> > > >
> > >
> > > Yes, this commit is from the archived riscv-svadu repo. Or should I update it to
> > > "commit c1abccf ("Merge pull request  #25 from ved-rivos/ratified") of
> > > riscvarchive/riscv-svadu."?
> >
> > I think Samuel was saying that we should use the commit where it was
> > merged into riscv-isa-manual instead.
> 
> Got it. I will update the description in the next version. Thank you!

There's no need (IMO) to send a new version for this alone - but if you
have to send another version for some other reason then do it.

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]