[RFC PATCH v4 0/5] Add Svadu Extension Support

[RFC PATCH v4 0/5] Add Svadu Extension Support

[Yong-Xuan Wang]

Svadu is a RISC-V extension for hardware updating of PTE A/D bits. This
patch set adds support to enable Svadu extension for both host and guest
OS.

For backward-compatibility, Svadu extension should be enabled through the
SBI FWFT extension. This patchset is based on the Linux implementation of
FWFT extension branch by Clément Léger [1], and can be verified with the
OpenSBI FWFT patchset [2].

[1] https://github.com/rivosinc/linux/commits/dev/cleger/fwft/
[2] https://lists.infradead.org/pipermail/opensbi/2024-May/006927.html

---
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 (5):
  RISC-V: Detect and Enable Svadu Extension Support
  dt-bindings: riscv: Add Svadu Entry
  RISC-V: KVM: Add Svadu Extension Support for Guest/VM
  RISC-V: KVM: add support for SBI_FWFT_PTE_AD_HW_UPDATING
  KVM: riscv: selftests: Add Svadu Extension to get-reg-list testt

 .../devicetree/bindings/riscv/extensions.yaml |  6 +++
 arch/riscv/Kconfig                            |  1 +
 arch/riscv/include/asm/csr.h                  |  1 +
 arch/riscv/include/asm/hwcap.h                |  1 +
 arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h    |  2 +-
 arch/riscv/include/asm/pgtable.h              |  8 +++-
 arch/riscv/include/uapi/asm/kvm.h             |  1 +
 arch/riscv/kernel/cpufeature.c                | 11 ++++++
 arch/riscv/kvm/vcpu_onereg.c                  |  1 +
 arch/riscv/kvm/vcpu_sbi_fwft.c                | 38 ++++++++++++++++++-
 .../selftests/kvm/riscv/get-reg-list.c        |  4 ++
 11 files changed, 71 insertions(+), 3 deletions(-)

-- 
2.17.1

[RFC PATCH v4 1/5] RISC-V: Detect and Enable Svadu Extension Support

[Yong-Xuan Wang]

Svadu is a RISC-V extension for hardware updating of PTE A/D bits.

In this patch we detect Svadu extension support from DTB and enable it
with SBI FWFT extension. Also we add arch_has_hw_pte_young() to enable
optimization in MGLRU and __wp_page_copy_user() if Svadu extension is
available.

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: Conor Dooley <conor.dooley@microchip.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   |  1 +
 arch/riscv/include/asm/pgtable.h |  8 +++++++-
 arch/riscv/kernel/cpufeature.c   | 11 +++++++++++
 5 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index be09c8836d56..30fa558ee284 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -34,6 +34,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 2468c55933cd..2ac270ad4acd 100644
--- a/arch/riscv/include/asm/csr.h
+++ b/arch/riscv/include/asm/csr.h
@@ -194,6 +194,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..8d539e3f4e11 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -81,6 +81,7 @@
 #define RISCV_ISA_EXT_ZTSO		72
 #define RISCV_ISA_EXT_ZACAS		73
 #define RISCV_ISA_EXT_XANDESPMU		74
+#define RISCV_ISA_EXT_SVADU		75
 
 #define RISCV_ISA_EXT_XLINUXENVCFG	127
 
diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
index 9f8ea0e33eb1..1f1b326ccf63 100644
--- a/arch/riscv/include/asm/pgtable.h
+++ b/arch/riscv/include/asm/pgtable.h
@@ -117,6 +117,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)
 
@@ -285,7 +286,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)
 {
@@ -621,6 +621,12 @@ static inline pgprot_t pgprot_writecombine(pgprot_t _prot)
 	return __pgprot(prot);
 }
 
+#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 3ed2359eae35..b023908c5932 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -93,6 +93,16 @@ static bool riscv_isa_extension_check(int id)
 			return false;
 		}
 		return true;
+	case RISCV_ISA_EXT_SVADU:
+		if (sbi_probe_extension(SBI_EXT_FWFT) > 0) {
+			struct sbiret ret;
+
+			ret = sbi_ecall(SBI_EXT_FWFT, SBI_EXT_FWFT_SET, SBI_FWFT_PTE_AD_HW_UPDATING,
+					1, 0, 0, 0, 0);
+
+			return ret.error == SBI_SUCCESS;
+		}
+		return false;
 	case RISCV_ISA_EXT_INVALID:
 		return false;
 	}
@@ -301,6 +311,7 @@ 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_SUPERSET(svadu, RISCV_ISA_EXT_SVADU, riscv_xlinuxenvcfg_exts),
 	__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.17.1

[RFC PATCH v4 2/5] dt-bindings: riscv: Add Svadu Entry

[Yong-Xuan Wang]

Add an entry for the Svadu extension to the riscv,isa-extensions property.

Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
---
 Documentation/devicetree/bindings/riscv/extensions.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml
index 468c646247aa..598a5841920f 100644
--- a/Documentation/devicetree/bindings/riscv/extensions.yaml
+++ b/Documentation/devicetree/bindings/riscv/extensions.yaml
@@ -153,6 +153,12 @@ properties:
             ratified at commit 3f9ed34 ("Add ability to manually trigger
             workflow. (#2)") of riscv-time-compare.
 
+        - 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.
+
         - const: svinval
           description:
             The standard Svinval supervisor-level extension for fine-grained
-- 
2.17.1

[RFC PATCH v4 3/5] RISC-V: KVM: Add Svadu Extension Support for Guest/VM

[Yong-Xuan Wang]

We extend the KVM ISA extension ONE_REG interface to allow VMM tools to
detect and enable Svadu extension for Guest/VM. The ADUE bit in henvcfg
is cleared by default for backward-compatibility.

Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
---
 arch/riscv/include/uapi/asm/kvm.h | 1 +
 arch/riscv/kvm/vcpu_onereg.c      | 1 +
 2 files changed, 2 insertions(+)

diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h
index a59a8448deea..bcf99264560d 100644
--- a/arch/riscv/include/uapi/asm/kvm.h
+++ b/arch/riscv/include/uapi/asm/kvm.h
@@ -167,6 +167,7 @@ enum KVM_RISCV_ISA_EXT_ID {
 	KVM_RISCV_ISA_EXT_ZFA,
 	KVM_RISCV_ISA_EXT_ZTSO,
 	KVM_RISCV_ISA_EXT_ZACAS,
+	KVM_RISCV_ISA_EXT_SVADU,
 	KVM_RISCV_ISA_EXT_MAX,
 };
 
diff --git a/arch/riscv/kvm/vcpu_onereg.c b/arch/riscv/kvm/vcpu_onereg.c
index 994adc26db4b..4166665e215d 100644
--- a/arch/riscv/kvm/vcpu_onereg.c
+++ b/arch/riscv/kvm/vcpu_onereg.c
@@ -37,6 +37,7 @@ static const unsigned long kvm_isa_ext_arr[] = {
 	KVM_ISA_EXT_ARR(SMSTATEEN),
 	KVM_ISA_EXT_ARR(SSAIA),
 	KVM_ISA_EXT_ARR(SSTC),
+	KVM_ISA_EXT_ARR(SVADU),
 	KVM_ISA_EXT_ARR(SVINVAL),
 	KVM_ISA_EXT_ARR(SVNAPOT),
 	KVM_ISA_EXT_ARR(SVPBMT),
-- 
2.17.1

[RFC PATCH v4 4/5] RISC-V: KVM: add support for SBI_FWFT_PTE_AD_HW_UPDATING

[Yong-Xuan Wang]

Add support for SBI_FWFT_PTE_AD_HW_UPDATING to set the PTE A/D bits
updating behavior for Guest/VM.

Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
---
 arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h |  2 +-
 arch/riscv/kvm/vcpu_sbi_fwft.c             | 38 +++++++++++++++++++++-
 2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h b/arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h
index 7b7bcc5c8fee..3614a44e0a4a 100644
--- a/arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h
+++ b/arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h
@@ -11,7 +11,7 @@
 
 #include <asm/sbi.h>
 
-#define KVM_SBI_FWFT_FEATURE_COUNT	1
+#define KVM_SBI_FWFT_FEATURE_COUNT	2
 
 struct kvm_sbi_fwft_config;
 struct kvm_vcpu;
diff --git a/arch/riscv/kvm/vcpu_sbi_fwft.c b/arch/riscv/kvm/vcpu_sbi_fwft.c
index 89ec263c250d..14ef74023340 100644
--- a/arch/riscv/kvm/vcpu_sbi_fwft.c
+++ b/arch/riscv/kvm/vcpu_sbi_fwft.c
@@ -71,6 +71,36 @@ static int kvm_sbi_fwft_get_misaligned_delegation(struct kvm_vcpu *vcpu,
 	return SBI_SUCCESS;
 }
 
+static int kvm_sbi_fwft_adue_supported(struct kvm_vcpu *vcpu)
+{
+	if (!riscv_isa_extension_available(vcpu->arch.isa, SVADU))
+		return SBI_ERR_NOT_SUPPORTED;
+
+	return 0;
+}
+
+static int kvm_sbi_fwft_set_adue(struct kvm_vcpu *vcpu, struct kvm_sbi_fwft_config *conf,
+				 unsigned long value)
+{
+	if (value)
+		vcpu->arch.cfg.henvcfg |= ENVCFG_ADUE;
+	else
+		vcpu->arch.cfg.henvcfg &= ~ENVCFG_ADUE;
+
+	return SBI_SUCCESS;
+}
+
+static int kvm_sbi_fwft_get_adue(struct kvm_vcpu *vcpu, struct kvm_sbi_fwft_config *conf,
+				 unsigned long *value)
+{
+	if (!riscv_isa_extension_available(vcpu->arch.isa, SVADU))
+		return SBI_ERR_NOT_SUPPORTED;
+
+	*value = !!(vcpu->arch.cfg.henvcfg & ENVCFG_ADUE);
+
+	return SBI_SUCCESS;
+}
+
 static struct kvm_sbi_fwft_config *
 kvm_sbi_fwft_get_config(struct kvm_vcpu *vcpu, enum sbi_fwft_feature_t feature)
 {
@@ -177,7 +207,13 @@ static const struct kvm_sbi_fwft_feature features[] = {
 		.supported = kvm_sbi_fwft_misaligned_delegation_supported,
 		.set = kvm_sbi_fwft_set_misaligned_delegation,
 		.get = kvm_sbi_fwft_get_misaligned_delegation,
-	}
+	},
+	{
+		.id = SBI_FWFT_PTE_AD_HW_UPDATING,
+		.supported = kvm_sbi_fwft_adue_supported,
+		.set = kvm_sbi_fwft_set_adue,
+		.get = kvm_sbi_fwft_get_adue,
+	},
 };
 
 static_assert(ARRAY_SIZE(features) == KVM_SBI_FWFT_FEATURE_COUNT);
-- 
2.17.1

[RFC PATCH v4 5/5] KVM: riscv: selftests: Add Svadu Extension to get-reg-list testt

[Yong-Xuan Wang]

Update the get-reg-list test to test the Svadu Extension is available for
guest OS.

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

diff --git a/tools/testing/selftests/kvm/riscv/get-reg-list.c b/tools/testing/selftests/kvm/riscv/get-reg-list.c
index b882b7b9b785..3e71b7e40dbf 100644
--- a/tools/testing/selftests/kvm/riscv/get-reg-list.c
+++ b/tools/testing/selftests/kvm/riscv/get-reg-list.c
@@ -44,6 +44,7 @@ bool filter_reg(__u64 reg)
 	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_SMSTATEEN:
 	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_SSTC:
+	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:
@@ -409,6 +410,7 @@ static const char *isa_ext_single_id_to_str(__u64 reg_off)
 		KVM_ISA_EXT_ARR(SMSTATEEN),
 		KVM_ISA_EXT_ARR(SSAIA),
 		KVM_ISA_EXT_ARR(SSTC),
+		KVM_ISA_EXT_ARR(SVADU),
 		KVM_ISA_EXT_ARR(SVINVAL),
 		KVM_ISA_EXT_ARR(SVNAPOT),
 		KVM_ISA_EXT_ARR(SVPBMT),
@@ -932,6 +934,7 @@ KVM_ISA_EXT_SUBLIST_CONFIG(fp_d, FP_D);
 KVM_ISA_EXT_SIMPLE_CONFIG(h, H);
 KVM_ISA_EXT_SUBLIST_CONFIG(smstateen, SMSTATEEN);
 KVM_ISA_EXT_SIMPLE_CONFIG(sstc, SSTC);
+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);
@@ -987,6 +990,7 @@ struct vcpu_reg_list *vcpu_configs[] = {
 	&config_h,
 	&config_smstateen,
 	&config_sstc,
+	&config_svadu,
 	&config_svinval,
 	&config_svnapot,
 	&config_svpbmt,
-- 
2.17.1

Re: [RFC PATCH v4 2/5] dt-bindings: riscv: Add Svadu Entry

[Conor Dooley]

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

On Fri, May 24, 2024 at 06:33:02PM +0800, Yong-Xuan Wang wrote:
> Add an entry for the Svadu extension to the riscv,isa-extensions property.
> 
> Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

I'm going to un-ack this, not because you did something wrong per se,
but because there's some discussion on the OpenSBI list about what is
and what is not backwards compatible and how an OS should interpret
svade and svadu:
https://lists.infradead.org/pipermail/opensbi/2024-May/006949.html

Thanks,
Conor.

> ---
>  Documentation/devicetree/bindings/riscv/extensions.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml
> index 468c646247aa..598a5841920f 100644
> --- a/Documentation/devicetree/bindings/riscv/extensions.yaml
> +++ b/Documentation/devicetree/bindings/riscv/extensions.yaml
> @@ -153,6 +153,12 @@ properties:
>              ratified at commit 3f9ed34 ("Add ability to manually trigger
>              workflow. (#2)") of riscv-time-compare.
>  
> +        - 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.
> +
>          - const: svinval
>            description:
>              The standard Svinval supervisor-level extension for fine-grained
> -- 
> 2.17.1
> 

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

Re: [RFC PATCH v4 1/5] RISC-V: Detect and Enable Svadu Extension Support

[Andrew Jones]

On Fri, May 24, 2024 at 06:33:01PM GMT, Yong-Xuan Wang wrote:
> Svadu is a RISC-V extension for hardware updating of PTE A/D bits.
> 
> In this patch we detect Svadu extension support from DTB and enable it
> with SBI FWFT extension. Also we add arch_has_hw_pte_young() to enable
> optimization in MGLRU and __wp_page_copy_user() if Svadu extension is
> available.
> 
> 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: Conor Dooley <conor.dooley@microchip.com>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

I think this patch changed too much to keep r-b's. We didn't have the
FWFT part before.

> ---
>  arch/riscv/Kconfig               |  1 +
>  arch/riscv/include/asm/csr.h     |  1 +
>  arch/riscv/include/asm/hwcap.h   |  1 +
>  arch/riscv/include/asm/pgtable.h |  8 +++++++-
>  arch/riscv/kernel/cpufeature.c   | 11 +++++++++++
>  5 files changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index be09c8836d56..30fa558ee284 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -34,6 +34,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 2468c55933cd..2ac270ad4acd 100644
> --- a/arch/riscv/include/asm/csr.h
> +++ b/arch/riscv/include/asm/csr.h
> @@ -194,6 +194,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..8d539e3f4e11 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -81,6 +81,7 @@
>  #define RISCV_ISA_EXT_ZTSO		72
>  #define RISCV_ISA_EXT_ZACAS		73
>  #define RISCV_ISA_EXT_XANDESPMU		74
> +#define RISCV_ISA_EXT_SVADU		75
>  
>  #define RISCV_ISA_EXT_XLINUXENVCFG	127
>  
> diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
> index 9f8ea0e33eb1..1f1b326ccf63 100644
> --- a/arch/riscv/include/asm/pgtable.h
> +++ b/arch/riscv/include/asm/pgtable.h
> @@ -117,6 +117,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)
>  
> @@ -285,7 +286,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)
>  {
> @@ -621,6 +621,12 @@ static inline pgprot_t pgprot_writecombine(pgprot_t _prot)
>  	return __pgprot(prot);
>  }
>  
> +#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 3ed2359eae35..b023908c5932 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -93,6 +93,16 @@ static bool riscv_isa_extension_check(int id)
>  			return false;
>  		}
>  		return true;
> +	case RISCV_ISA_EXT_SVADU:
> +		if (sbi_probe_extension(SBI_EXT_FWFT) > 0) {

I think we've decided the appropriate way to prove for SBI extensions is
to first ensure the SBI version and then do the probe, like we do for STA
in has_pv_steal_clock()

> +			struct sbiret ret;
> +
> +			ret = sbi_ecall(SBI_EXT_FWFT, SBI_EXT_FWFT_SET, SBI_FWFT_PTE_AD_HW_UPDATING,
> +					1, 0, 0, 0, 0);
> +
> +			return ret.error == SBI_SUCCESS;
> +		}
> +		return false;
>  	case RISCV_ISA_EXT_INVALID:
>  		return false;
>  	}
> @@ -301,6 +311,7 @@ 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_SUPERSET(svadu, RISCV_ISA_EXT_SVADU, riscv_xlinuxenvcfg_exts),

We do we need XLINUXENVCFG?

Thanks,
drew

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

Re: [RFC PATCH v4 3/5] RISC-V: KVM: Add Svadu Extension Support for Guest/VM

[Andrew Jones]

On Fri, May 24, 2024 at 06:33:03PM GMT, Yong-Xuan Wang wrote:
> We extend the KVM ISA extension ONE_REG interface to allow VMM tools to
> detect and enable Svadu extension for Guest/VM. The ADUE bit in henvcfg
> is cleared by default for backward-compatibility.
> 
> Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> ---
>  arch/riscv/include/uapi/asm/kvm.h | 1 +
>  arch/riscv/kvm/vcpu_onereg.c      | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h
> index a59a8448deea..bcf99264560d 100644
> --- a/arch/riscv/include/uapi/asm/kvm.h
> +++ b/arch/riscv/include/uapi/asm/kvm.h
> @@ -167,6 +167,7 @@ enum KVM_RISCV_ISA_EXT_ID {
>  	KVM_RISCV_ISA_EXT_ZFA,
>  	KVM_RISCV_ISA_EXT_ZTSO,
>  	KVM_RISCV_ISA_EXT_ZACAS,
> +	KVM_RISCV_ISA_EXT_SVADU,
>  	KVM_RISCV_ISA_EXT_MAX,
>  };
>  
> diff --git a/arch/riscv/kvm/vcpu_onereg.c b/arch/riscv/kvm/vcpu_onereg.c
> index 994adc26db4b..4166665e215d 100644
> --- a/arch/riscv/kvm/vcpu_onereg.c
> +++ b/arch/riscv/kvm/vcpu_onereg.c
> @@ -37,6 +37,7 @@ static const unsigned long kvm_isa_ext_arr[] = {
>  	KVM_ISA_EXT_ARR(SMSTATEEN),
>  	KVM_ISA_EXT_ARR(SSAIA),
>  	KVM_ISA_EXT_ARR(SSTC),
> +	KVM_ISA_EXT_ARR(SVADU),
>  	KVM_ISA_EXT_ARR(SVINVAL),
>  	KVM_ISA_EXT_ARR(SVNAPOT),
>  	KVM_ISA_EXT_ARR(SVPBMT),
> -- 
> 2.17.1
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

Re: [RFC PATCH v4 4/5] RISC-V: KVM: add support for SBI_FWFT_PTE_AD_HW_UPDATING

[Andrew Jones]

On Fri, May 24, 2024 at 06:33:04PM GMT, Yong-Xuan Wang wrote:
> Add support for SBI_FWFT_PTE_AD_HW_UPDATING to set the PTE A/D bits
> updating behavior for Guest/VM.
> 
> Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> ---
>  arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h |  2 +-
>  arch/riscv/kvm/vcpu_sbi_fwft.c             | 38 +++++++++++++++++++++-
>  2 files changed, 38 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h b/arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h
> index 7b7bcc5c8fee..3614a44e0a4a 100644
> --- a/arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h
> +++ b/arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h
> @@ -11,7 +11,7 @@
>  
>  #include <asm/sbi.h>
>  
> -#define KVM_SBI_FWFT_FEATURE_COUNT	1
> +#define KVM_SBI_FWFT_FEATURE_COUNT	2
>  
>  struct kvm_sbi_fwft_config;
>  struct kvm_vcpu;
> diff --git a/arch/riscv/kvm/vcpu_sbi_fwft.c b/arch/riscv/kvm/vcpu_sbi_fwft.c
> index 89ec263c250d..14ef74023340 100644
> --- a/arch/riscv/kvm/vcpu_sbi_fwft.c
> +++ b/arch/riscv/kvm/vcpu_sbi_fwft.c
> @@ -71,6 +71,36 @@ static int kvm_sbi_fwft_get_misaligned_delegation(struct kvm_vcpu *vcpu,
>  	return SBI_SUCCESS;
>  }
>  
> +static int kvm_sbi_fwft_adue_supported(struct kvm_vcpu *vcpu)
> +{
> +	if (!riscv_isa_extension_available(vcpu->arch.isa, SVADU))
> +		return SBI_ERR_NOT_SUPPORTED;
> +
> +	return 0;
> +}
> +
> +static int kvm_sbi_fwft_set_adue(struct kvm_vcpu *vcpu, struct kvm_sbi_fwft_config *conf,
> +				 unsigned long value)
> +{
> +	if (value)
> +		vcpu->arch.cfg.henvcfg |= ENVCFG_ADUE;
> +	else
> +		vcpu->arch.cfg.henvcfg &= ~ENVCFG_ADUE;
> +
> +	return SBI_SUCCESS;
> +}
> +
> +static int kvm_sbi_fwft_get_adue(struct kvm_vcpu *vcpu, struct kvm_sbi_fwft_config *conf,
> +				 unsigned long *value)
> +{
> +	if (!riscv_isa_extension_available(vcpu->arch.isa, SVADU))
> +		return SBI_ERR_NOT_SUPPORTED;
> +
> +	*value = !!(vcpu->arch.cfg.henvcfg & ENVCFG_ADUE);
> +
> +	return SBI_SUCCESS;
> +}
> +
>  static struct kvm_sbi_fwft_config *
>  kvm_sbi_fwft_get_config(struct kvm_vcpu *vcpu, enum sbi_fwft_feature_t feature)
>  {
> @@ -177,7 +207,13 @@ static const struct kvm_sbi_fwft_feature features[] = {
>  		.supported = kvm_sbi_fwft_misaligned_delegation_supported,
>  		.set = kvm_sbi_fwft_set_misaligned_delegation,
>  		.get = kvm_sbi_fwft_get_misaligned_delegation,
> -	}
> +	},
> +	{
> +		.id = SBI_FWFT_PTE_AD_HW_UPDATING,
> +		.supported = kvm_sbi_fwft_adue_supported,
> +		.set = kvm_sbi_fwft_set_adue,
> +		.get = kvm_sbi_fwft_get_adue,
> +	},
>  };
>  
>  static_assert(ARRAY_SIZE(features) == KVM_SBI_FWFT_FEATURE_COUNT);
> -- 
> 2.17.1
> 
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

Re: [RFC PATCH v4 5/5] KVM: riscv: selftests: Add Svadu Extension to get-reg-list testt

[Andrew Jones]

On Fri, May 24, 2024 at 06:33:05PM GMT, Yong-Xuan Wang wrote:
> Update the get-reg-list test to test the Svadu Extension is available for
> guest OS.
> 
> Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> ---
>  tools/testing/selftests/kvm/riscv/get-reg-list.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tools/testing/selftests/kvm/riscv/get-reg-list.c b/tools/testing/selftests/kvm/riscv/get-reg-list.c
> index b882b7b9b785..3e71b7e40dbf 100644
> --- a/tools/testing/selftests/kvm/riscv/get-reg-list.c
> +++ b/tools/testing/selftests/kvm/riscv/get-reg-list.c
> @@ -44,6 +44,7 @@ bool filter_reg(__u64 reg)
>  	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_SMSTATEEN:
>  	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_SSTC:
> +	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:
> @@ -409,6 +410,7 @@ static const char *isa_ext_single_id_to_str(__u64 reg_off)
>  		KVM_ISA_EXT_ARR(SMSTATEEN),
>  		KVM_ISA_EXT_ARR(SSAIA),
>  		KVM_ISA_EXT_ARR(SSTC),
> +		KVM_ISA_EXT_ARR(SVADU),
>  		KVM_ISA_EXT_ARR(SVINVAL),
>  		KVM_ISA_EXT_ARR(SVNAPOT),
>  		KVM_ISA_EXT_ARR(SVPBMT),
> @@ -932,6 +934,7 @@ KVM_ISA_EXT_SUBLIST_CONFIG(fp_d, FP_D);
>  KVM_ISA_EXT_SIMPLE_CONFIG(h, H);
>  KVM_ISA_EXT_SUBLIST_CONFIG(smstateen, SMSTATEEN);
>  KVM_ISA_EXT_SIMPLE_CONFIG(sstc, SSTC);
> +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);
> @@ -987,6 +990,7 @@ struct vcpu_reg_list *vcpu_configs[] = {
>  	&config_h,
>  	&config_smstateen,
>  	&config_sstc,
> +	&config_svadu,
>  	&config_svinval,
>  	&config_svnapot,
>  	&config_svpbmt,
> -- 
> 2.17.1
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

Re: [RFC PATCH v4 4/5] RISC-V: KVM: add support for SBI_FWFT_PTE_AD_HW_UPDATING

[Clément Léger]

On 24/05/2024 12:33, Yong-Xuan Wang wrote:
> Add support for SBI_FWFT_PTE_AD_HW_UPDATING to set the PTE A/D bits
> updating behavior for Guest/VM.
> 
> Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> ---
>  arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h |  2 +-
>  arch/riscv/kvm/vcpu_sbi_fwft.c             | 38 +++++++++++++++++++++-
>  2 files changed, 38 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h b/arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h
> index 7b7bcc5c8fee..3614a44e0a4a 100644
> --- a/arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h
> +++ b/arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h
> @@ -11,7 +11,7 @@
>  
>  #include <asm/sbi.h>
>  
> -#define KVM_SBI_FWFT_FEATURE_COUNT	1
> +#define KVM_SBI_FWFT_FEATURE_COUNT	2
>  
>  struct kvm_sbi_fwft_config;
>  struct kvm_vcpu;
> diff --git a/arch/riscv/kvm/vcpu_sbi_fwft.c b/arch/riscv/kvm/vcpu_sbi_fwft.c
> index 89ec263c250d..14ef74023340 100644
> --- a/arch/riscv/kvm/vcpu_sbi_fwft.c
> +++ b/arch/riscv/kvm/vcpu_sbi_fwft.c
> @@ -71,6 +71,36 @@ static int kvm_sbi_fwft_get_misaligned_delegation(struct kvm_vcpu *vcpu,
>  	return SBI_SUCCESS;
>  }
>  
> +static int kvm_sbi_fwft_adue_supported(struct kvm_vcpu *vcpu)
> +{
> +	if (!riscv_isa_extension_available(vcpu->arch.isa, SVADU))
> +		return SBI_ERR_NOT_SUPPORTED;
> +
> +	return 0;
> +}
> +
> +static int kvm_sbi_fwft_set_adue(struct kvm_vcpu *vcpu, struct kvm_sbi_fwft_config *conf,
> +				 unsigned long value)
> +{
> +	if (value)
> +		vcpu->arch.cfg.henvcfg |= ENVCFG_ADUE;
> +	else
> +		vcpu->arch.cfg.henvcfg &= ~ENVCFG_ADUE;
> +
> +	return SBI_SUCCESS;
> +}
> +
> +static int kvm_sbi_fwft_get_adue(struct kvm_vcpu *vcpu, struct kvm_sbi_fwft_config *conf,
> +				 unsigned long *value)
> +{
> +	if (!riscv_isa_extension_available(vcpu->arch.isa, SVADU))
> +		return SBI_ERR_NOT_SUPPORTED;
> +
> +	*value = !!(vcpu->arch.cfg.henvcfg & ENVCFG_ADUE);
> +
> +	return SBI_SUCCESS;
> +}

Hi Yong-Xuan,

vcpu->arch.cfg.henvcfg seems to be used to update the HENVCFG CSR  only
during vcpu_load()/vcpu_put(). So if this extension updates it there and
stays in the execution loop (kvm_arch_vcpu_ioctl_run()) then, it seems
like the HENVCFG CSR won't be updated immediately but on the next
vcpu_load(). Is there something I'm missing ?

Thanks,

Clément Léger

> +
>  static struct kvm_sbi_fwft_config *
>  kvm_sbi_fwft_get_config(struct kvm_vcpu *vcpu, enum sbi_fwft_feature_t feature)
>  {
> @@ -177,7 +207,13 @@ static const struct kvm_sbi_fwft_feature features[] = {
>  		.supported = kvm_sbi_fwft_misaligned_delegation_supported,
>  		.set = kvm_sbi_fwft_set_misaligned_delegation,
>  		.get = kvm_sbi_fwft_get_misaligned_delegation,
> -	}
> +	},
> +	{
> +		.id = SBI_FWFT_PTE_AD_HW_UPDATING,
> +		.supported = kvm_sbi_fwft_adue_supported,
> +		.set = kvm_sbi_fwft_set_adue,
> +		.get = kvm_sbi_fwft_get_adue,
> +	},
>  };
>  
>  static_assert(ARRAY_SIZE(features) == KVM_SBI_FWFT_FEATURE_COUNT);

Re: [RFC PATCH v4 2/5] dt-bindings: riscv: Add Svadu Entry

[Yong-Xuan Wang]

Hi Conor,

On Mon, May 27, 2024 at 11:09 PM Conor Dooley <conor@kernel.org> wrote:
>
> On Fri, May 24, 2024 at 06:33:02PM +0800, Yong-Xuan Wang wrote:
> > Add an entry for the Svadu extension to the riscv,isa-extensions property.
> >
> > Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> > Acked-by: Conor Dooley <conor.dooley@microchip.com>
> > Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
>
> I'm going to un-ack this, not because you did something wrong per se,
> but because there's some discussion on the OpenSBI list about what is
> and what is not backwards compatible and how an OS should interpret
> svade and svadu:
> https://lists.infradead.org/pipermail/opensbi/2024-May/006949.html
>
> Thanks,
> Conor.
>

ok. I will remove it in the next version.

Regards,
Yong-Xuan

> > ---
> >  Documentation/devicetree/bindings/riscv/extensions.yaml | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml
> > index 468c646247aa..598a5841920f 100644
> > --- a/Documentation/devicetree/bindings/riscv/extensions.yaml
> > +++ b/Documentation/devicetree/bindings/riscv/extensions.yaml
> > @@ -153,6 +153,12 @@ properties:
> >              ratified at commit 3f9ed34 ("Add ability to manually trigger
> >              workflow. (#2)") of riscv-time-compare.
> >
> > +        - 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.
> > +
> >          - const: svinval
> >            description:
> >              The standard Svinval supervisor-level extension for fine-grained
> > --
> > 2.17.1
> >

Re: [RFC PATCH v4 1/5] RISC-V: Detect and Enable Svadu Extension Support

[Yong-Xuan Wang]

Hi Andrew,

On Tue, May 28, 2024 at 12:25 AM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> On Fri, May 24, 2024 at 06:33:01PM GMT, Yong-Xuan Wang wrote:
> > Svadu is a RISC-V extension for hardware updating of PTE A/D bits.
> >
> > In this patch we detect Svadu extension support from DTB and enable it
> > with SBI FWFT extension. Also we add arch_has_hw_pte_young() to enable
> > optimization in MGLRU and __wp_page_copy_user() if Svadu extension is
> > available.
> >
> > 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: Conor Dooley <conor.dooley@microchip.com>
> > Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
>
> I think this patch changed too much to keep r-b's. We didn't have the
> FWFT part before.
>

ok, I will remove them.

> > ---
> >  arch/riscv/Kconfig               |  1 +
> >  arch/riscv/include/asm/csr.h     |  1 +
> >  arch/riscv/include/asm/hwcap.h   |  1 +
> >  arch/riscv/include/asm/pgtable.h |  8 +++++++-
> >  arch/riscv/kernel/cpufeature.c   | 11 +++++++++++
> >  5 files changed, 21 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index be09c8836d56..30fa558ee284 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -34,6 +34,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 2468c55933cd..2ac270ad4acd 100644
> > --- a/arch/riscv/include/asm/csr.h
> > +++ b/arch/riscv/include/asm/csr.h
> > @@ -194,6 +194,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..8d539e3f4e11 100644
> > --- a/arch/riscv/include/asm/hwcap.h
> > +++ b/arch/riscv/include/asm/hwcap.h
> > @@ -81,6 +81,7 @@
> >  #define RISCV_ISA_EXT_ZTSO           72
> >  #define RISCV_ISA_EXT_ZACAS          73
> >  #define RISCV_ISA_EXT_XANDESPMU              74
> > +#define RISCV_ISA_EXT_SVADU          75
> >
> >  #define RISCV_ISA_EXT_XLINUXENVCFG   127
> >
> > diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
> > index 9f8ea0e33eb1..1f1b326ccf63 100644
> > --- a/arch/riscv/include/asm/pgtable.h
> > +++ b/arch/riscv/include/asm/pgtable.h
> > @@ -117,6 +117,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)
> >
> > @@ -285,7 +286,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)
> >  {
> > @@ -621,6 +621,12 @@ static inline pgprot_t pgprot_writecombine(pgprot_t _prot)
> >       return __pgprot(prot);
> >  }
> >
> > +#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 3ed2359eae35..b023908c5932 100644
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
> > @@ -93,6 +93,16 @@ static bool riscv_isa_extension_check(int id)
> >                       return false;
> >               }
> >               return true;
> > +     case RISCV_ISA_EXT_SVADU:
> > +             if (sbi_probe_extension(SBI_EXT_FWFT) > 0) {
>
> I think we've decided the appropriate way to prove for SBI extensions is
> to first ensure the SBI version and then do the probe, like we do for STA
> in has_pv_steal_clock()
>

ok, I will add the SBI version check.

> > +                     struct sbiret ret;
> > +
> > +                     ret = sbi_ecall(SBI_EXT_FWFT, SBI_EXT_FWFT_SET, SBI_FWFT_PTE_AD_HW_UPDATING,
> > +                                     1, 0, 0, 0, 0);
> > +
> > +                     return ret.error == SBI_SUCCESS;
> > +             }
> > +             return false;
> >       case RISCV_ISA_EXT_INVALID:
> >               return false;
> >       }
> > @@ -301,6 +311,7 @@ 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_SUPERSET(svadu, RISCV_ISA_EXT_SVADU, riscv_xlinuxenvcfg_exts),
>
> We do we need XLINUXENVCFG?
>

Sorry, I misunderstood the comments of riscv_xlinuxenvcfg_exts, I will
remove it in the next version.

Regards,
Yong-Xuan


> Thanks,
> drew
>
> >       __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.17.1
> >

Re: [RFC PATCH v4 4/5] RISC-V: KVM: add support for SBI_FWFT_PTE_AD_HW_UPDATING

[Yong-Xuan Wang]

On Tue, May 28, 2024 at 6:15 PM Clément Léger <cleger@rivosinc.com> wrote:
>
>
>
> On 24/05/2024 12:33, Yong-Xuan Wang wrote:
> > Add support for SBI_FWFT_PTE_AD_HW_UPDATING to set the PTE A/D bits
> > updating behavior for Guest/VM.
> >
> > Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> > ---
> >  arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h |  2 +-
> >  arch/riscv/kvm/vcpu_sbi_fwft.c             | 38 +++++++++++++++++++++-
> >  2 files changed, 38 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h b/arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h
> > index 7b7bcc5c8fee..3614a44e0a4a 100644
> > --- a/arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h
> > +++ b/arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h
> > @@ -11,7 +11,7 @@
> >
> >  #include <asm/sbi.h>
> >
> > -#define KVM_SBI_FWFT_FEATURE_COUNT   1
> > +#define KVM_SBI_FWFT_FEATURE_COUNT   2
> >
> >  struct kvm_sbi_fwft_config;
> >  struct kvm_vcpu;
> > diff --git a/arch/riscv/kvm/vcpu_sbi_fwft.c b/arch/riscv/kvm/vcpu_sbi_fwft.c
> > index 89ec263c250d..14ef74023340 100644
> > --- a/arch/riscv/kvm/vcpu_sbi_fwft.c
> > +++ b/arch/riscv/kvm/vcpu_sbi_fwft.c
> > @@ -71,6 +71,36 @@ static int kvm_sbi_fwft_get_misaligned_delegation(struct kvm_vcpu *vcpu,
> >       return SBI_SUCCESS;
> >  }
> >
> > +static int kvm_sbi_fwft_adue_supported(struct kvm_vcpu *vcpu)
> > +{
> > +     if (!riscv_isa_extension_available(vcpu->arch.isa, SVADU))
> > +             return SBI_ERR_NOT_SUPPORTED;
> > +
> > +     return 0;
> > +}
> > +
> > +static int kvm_sbi_fwft_set_adue(struct kvm_vcpu *vcpu, struct kvm_sbi_fwft_config *conf,
> > +                              unsigned long value)
> > +{
> > +     if (value)
> > +             vcpu->arch.cfg.henvcfg |= ENVCFG_ADUE;
> > +     else
> > +             vcpu->arch.cfg.henvcfg &= ~ENVCFG_ADUE;
> > +
> > +     return SBI_SUCCESS;
> > +}
> > +
> > +static int kvm_sbi_fwft_get_adue(struct kvm_vcpu *vcpu, struct kvm_sbi_fwft_config *conf,
> > +                              unsigned long *value)
> > +{
> > +     if (!riscv_isa_extension_available(vcpu->arch.isa, SVADU))
> > +             return SBI_ERR_NOT_SUPPORTED;
> > +
> > +     *value = !!(vcpu->arch.cfg.henvcfg & ENVCFG_ADUE);
> > +
> > +     return SBI_SUCCESS;
> > +}
>
> Hi Yong-Xuan,
>
> vcpu->arch.cfg.henvcfg seems to be used to update the HENVCFG CSR  only
> during vcpu_load()/vcpu_put(). So if this extension updates it there and
> stays in the execution loop (kvm_arch_vcpu_ioctl_run()) then, it seems
> like the HENVCFG CSR won't be updated immediately but on the next
> vcpu_load(). Is there something I'm missing ?
>
> Thanks,
>
> Clément Léger
>

Hi Clément,

That's right. I will fix it in the next version. Thank you!

Regards,
Yong-Xuan

> > +
> >  static struct kvm_sbi_fwft_config *
> >  kvm_sbi_fwft_get_config(struct kvm_vcpu *vcpu, enum sbi_fwft_feature_t feature)
> >  {
> > @@ -177,7 +207,13 @@ static const struct kvm_sbi_fwft_feature features[] = {
> >               .supported = kvm_sbi_fwft_misaligned_delegation_supported,
> >               .set = kvm_sbi_fwft_set_misaligned_delegation,
> >               .get = kvm_sbi_fwft_get_misaligned_delegation,
> > -     }
> > +     },
> > +     {
> > +             .id = SBI_FWFT_PTE_AD_HW_UPDATING,
> > +             .supported = kvm_sbi_fwft_adue_supported,
> > +             .set = kvm_sbi_fwft_set_adue,
> > +             .get = kvm_sbi_fwft_get_adue,
> > +     },
> >  };
> >
> >  static_assert(ARRAY_SIZE(features) == KVM_SBI_FWFT_FEATURE_COUNT);

Re: [RFC PATCH v4 1/5] RISC-V: Detect and Enable Svadu Extension Support

[Alexandre Ghiti]

Hi Yong-Xuan,

On 27/05/2024 18:25, Andrew Jones wrote:
> On Fri, May 24, 2024 at 06:33:01PM GMT, Yong-Xuan Wang wrote:
>> Svadu is a RISC-V extension for hardware updating of PTE A/D bits.
>>
>> In this patch we detect Svadu extension support from DTB and enable it
>> with SBI FWFT extension. Also we add arch_has_hw_pte_young() to enable
>> optimization in MGLRU and __wp_page_copy_user() if Svadu extension is
>> available.


So we talked about this yesterday during the linux-riscv patchwork 
meeting. We came to the conclusion that we should not wait for the SBI 
FWFT extension to enable Svadu but instead, it should be enabled by 
default by openSBI if the extension is present in the device tree. This 
is because we did not find any backward compatibility issues, meaning 
that enabling Svadu should not break any S-mode software. This is what 
you did in your previous versions of this patchset so the changes should 
be easy. This behaviour must be added to the dtbinding description of 
the Svadu extension.

Another thing that we discussed yesterday. There exist 2 schemes to 
manage the A/D bits updates, Svade and Svadu. If a platform supports 
both extensions and both are present in the device tree, it is M-mode 
firmware's responsibility to provide a "sane" device tree to the S-mode 
software, meaning the device tree can not contain both extensions. And 
because on such platforms, Svadu is more performant than Svade, Svadu 
should be enabled by the M-mode firmware and only Svadu should be 
present in the device tree.

I hope that clearly explains what we discussed yesterday, let me know if 
you (or anyone else) need more explanations. If no one is opposed to 
this solution, do you think you can implement this behaviour? If not, I 
can deal with it, just let me know.

Thanks


>>
>> 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: Conor Dooley <conor.dooley@microchip.com>
>> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> I think this patch changed too much to keep r-b's. We didn't have the
> FWFT part before.
>
>> ---
>>   arch/riscv/Kconfig               |  1 +
>>   arch/riscv/include/asm/csr.h     |  1 +
>>   arch/riscv/include/asm/hwcap.h   |  1 +
>>   arch/riscv/include/asm/pgtable.h |  8 +++++++-
>>   arch/riscv/kernel/cpufeature.c   | 11 +++++++++++
>>   5 files changed, 21 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>> index be09c8836d56..30fa558ee284 100644
>> --- a/arch/riscv/Kconfig
>> +++ b/arch/riscv/Kconfig
>> @@ -34,6 +34,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 2468c55933cd..2ac270ad4acd 100644
>> --- a/arch/riscv/include/asm/csr.h
>> +++ b/arch/riscv/include/asm/csr.h
>> @@ -194,6 +194,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..8d539e3f4e11 100644
>> --- a/arch/riscv/include/asm/hwcap.h
>> +++ b/arch/riscv/include/asm/hwcap.h
>> @@ -81,6 +81,7 @@
>>   #define RISCV_ISA_EXT_ZTSO		72
>>   #define RISCV_ISA_EXT_ZACAS		73
>>   #define RISCV_ISA_EXT_XANDESPMU		74
>> +#define RISCV_ISA_EXT_SVADU		75
>>   
>>   #define RISCV_ISA_EXT_XLINUXENVCFG	127
>>   
>> diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
>> index 9f8ea0e33eb1..1f1b326ccf63 100644
>> --- a/arch/riscv/include/asm/pgtable.h
>> +++ b/arch/riscv/include/asm/pgtable.h
>> @@ -117,6 +117,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)
>>   
>> @@ -285,7 +286,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)
>>   {
>> @@ -621,6 +621,12 @@ static inline pgprot_t pgprot_writecombine(pgprot_t _prot)
>>   	return __pgprot(prot);
>>   }
>>   
>> +#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 3ed2359eae35..b023908c5932 100644
>> --- a/arch/riscv/kernel/cpufeature.c
>> +++ b/arch/riscv/kernel/cpufeature.c
>> @@ -93,6 +93,16 @@ static bool riscv_isa_extension_check(int id)
>>   			return false;
>>   		}
>>   		return true;
>> +	case RISCV_ISA_EXT_SVADU:
>> +		if (sbi_probe_extension(SBI_EXT_FWFT) > 0) {
> I think we've decided the appropriate way to prove for SBI extensions is
> to first ensure the SBI version and then do the probe, like we do for STA
> in has_pv_steal_clock()
>
>> +			struct sbiret ret;
>> +
>> +			ret = sbi_ecall(SBI_EXT_FWFT, SBI_EXT_FWFT_SET, SBI_FWFT_PTE_AD_HW_UPDATING,
>> +					1, 0, 0, 0, 0);
>> +
>> +			return ret.error == SBI_SUCCESS;
>> +		}
>> +		return false;
>>   	case RISCV_ISA_EXT_INVALID:
>>   		return false;
>>   	}
>> @@ -301,6 +311,7 @@ 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_SUPERSET(svadu, RISCV_ISA_EXT_SVADU, riscv_xlinuxenvcfg_exts),
> We do we need XLINUXENVCFG?
>
> Thanks,
> drew
>
>>   	__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.17.1
>>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [RFC PATCH v4 1/5] RISC-V: Detect and Enable Svadu Extension Support

[Andrew Jones]

On Thu, May 30, 2024 at 10:19:12AM GMT, Alexandre Ghiti wrote:
> Hi Yong-Xuan,
> 
> On 27/05/2024 18:25, Andrew Jones wrote:
> > On Fri, May 24, 2024 at 06:33:01PM GMT, Yong-Xuan Wang wrote:
> > > Svadu is a RISC-V extension for hardware updating of PTE A/D bits.
> > > 
> > > In this patch we detect Svadu extension support from DTB and enable it
> > > with SBI FWFT extension. Also we add arch_has_hw_pte_young() to enable
> > > optimization in MGLRU and __wp_page_copy_user() if Svadu extension is
> > > available.
> 
> 
> So we talked about this yesterday during the linux-riscv patchwork meeting.
> We came to the conclusion that we should not wait for the SBI FWFT extension
> to enable Svadu but instead, it should be enabled by default by openSBI if
> the extension is present in the device tree. This is because we did not find
> any backward compatibility issues, meaning that enabling Svadu should not
> break any S-mode software.

Unfortunately I joined yesterday's patchwork call late and missed this
discussion. I'm still not sure how we avoid concerns with S-mode software
expecting exceptions by purposely not setting A/D bits, but then not
getting those exceptions.

> This is what you did in your previous versions of
> this patchset so the changes should be easy. This behaviour must be added to
> the dtbinding description of the Svadu extension.
> 
> Another thing that we discussed yesterday. There exist 2 schemes to manage
> the A/D bits updates, Svade and Svadu. If a platform supports both
> extensions and both are present in the device tree, it is M-mode firmware's
> responsibility to provide a "sane" device tree to the S-mode software,
> meaning the device tree can not contain both extensions. And because on such
> platforms, Svadu is more performant than Svade, Svadu should be enabled by
> the M-mode firmware and only Svadu should be present in the device tree.

I'm not sure firmware will be able to choose svadu when it's available.
For example, platforms which want to conform to the upcoming "Server
Platform" specification must also conform to the RVA23 profile, which
mandates Svade and lists Svadu as an optional extension. This implies to
me that S-mode should be boot with both svade and svadu in the DT and with
svade being the active one. Then, S-mode can choose to request switching
to svadu with FWFT.

Thanks,
drew

> 
> I hope that clearly explains what we discussed yesterday, let me know if you
> (or anyone else) need more explanations. If no one is opposed to this
> solution, do you think you can implement this behaviour? If not, I can deal
> with it, just let me know.
> 
> Thanks
> 
> 
> > > 
> > > 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: Conor Dooley <conor.dooley@microchip.com>
> > > Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> > I think this patch changed too much to keep r-b's. We didn't have the
> > FWFT part before.
> > 
> > > ---
> > >   arch/riscv/Kconfig               |  1 +
> > >   arch/riscv/include/asm/csr.h     |  1 +
> > >   arch/riscv/include/asm/hwcap.h   |  1 +
> > >   arch/riscv/include/asm/pgtable.h |  8 +++++++-
> > >   arch/riscv/kernel/cpufeature.c   | 11 +++++++++++
> > >   5 files changed, 21 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > index be09c8836d56..30fa558ee284 100644
> > > --- a/arch/riscv/Kconfig
> > > +++ b/arch/riscv/Kconfig
> > > @@ -34,6 +34,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 2468c55933cd..2ac270ad4acd 100644
> > > --- a/arch/riscv/include/asm/csr.h
> > > +++ b/arch/riscv/include/asm/csr.h
> > > @@ -194,6 +194,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..8d539e3f4e11 100644
> > > --- a/arch/riscv/include/asm/hwcap.h
> > > +++ b/arch/riscv/include/asm/hwcap.h
> > > @@ -81,6 +81,7 @@
> > >   #define RISCV_ISA_EXT_ZTSO		72
> > >   #define RISCV_ISA_EXT_ZACAS		73
> > >   #define RISCV_ISA_EXT_XANDESPMU		74
> > > +#define RISCV_ISA_EXT_SVADU		75
> > >   #define RISCV_ISA_EXT_XLINUXENVCFG	127
> > > diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
> > > index 9f8ea0e33eb1..1f1b326ccf63 100644
> > > --- a/arch/riscv/include/asm/pgtable.h
> > > +++ b/arch/riscv/include/asm/pgtable.h
> > > @@ -117,6 +117,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)
> > > @@ -285,7 +286,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)
> > >   {
> > > @@ -621,6 +621,12 @@ static inline pgprot_t pgprot_writecombine(pgprot_t _prot)
> > >   	return __pgprot(prot);
> > >   }
> > > +#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 3ed2359eae35..b023908c5932 100644
> > > --- a/arch/riscv/kernel/cpufeature.c
> > > +++ b/arch/riscv/kernel/cpufeature.c
> > > @@ -93,6 +93,16 @@ static bool riscv_isa_extension_check(int id)
> > >   			return false;
> > >   		}
> > >   		return true;
> > > +	case RISCV_ISA_EXT_SVADU:
> > > +		if (sbi_probe_extension(SBI_EXT_FWFT) > 0) {
> > I think we've decided the appropriate way to prove for SBI extensions is
> > to first ensure the SBI version and then do the probe, like we do for STA
> > in has_pv_steal_clock()
> > 
> > > +			struct sbiret ret;
> > > +
> > > +			ret = sbi_ecall(SBI_EXT_FWFT, SBI_EXT_FWFT_SET, SBI_FWFT_PTE_AD_HW_UPDATING,
> > > +					1, 0, 0, 0, 0);
> > > +
> > > +			return ret.error == SBI_SUCCESS;
> > > +		}
> > > +		return false;
> > >   	case RISCV_ISA_EXT_INVALID:
> > >   		return false;
> > >   	}
> > > @@ -301,6 +311,7 @@ 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_SUPERSET(svadu, RISCV_ISA_EXT_SVADU, riscv_xlinuxenvcfg_exts),
> > We do we need XLINUXENVCFG?
> > 
> > Thanks,
> > drew
> > 
> > >   	__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.17.1
> > > 
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [RFC PATCH v4 1/5] RISC-V: Detect and Enable Svadu Extension Support

[Alexandre Ghiti]

Hi Andrew,

On 30/05/2024 10:47, Andrew Jones wrote:
> On Thu, May 30, 2024 at 10:19:12AM GMT, Alexandre Ghiti wrote:
>> Hi Yong-Xuan,
>>
>> On 27/05/2024 18:25, Andrew Jones wrote:
>>> On Fri, May 24, 2024 at 06:33:01PM GMT, Yong-Xuan Wang wrote:
>>>> Svadu is a RISC-V extension for hardware updating of PTE A/D bits.
>>>>
>>>> In this patch we detect Svadu extension support from DTB and enable it
>>>> with SBI FWFT extension. Also we add arch_has_hw_pte_young() to enable
>>>> optimization in MGLRU and __wp_page_copy_user() if Svadu extension is
>>>> available.
>>
>> So we talked about this yesterday during the linux-riscv patchwork meeting.
>> We came to the conclusion that we should not wait for the SBI FWFT extension
>> to enable Svadu but instead, it should be enabled by default by openSBI if
>> the extension is present in the device tree. This is because we did not find
>> any backward compatibility issues, meaning that enabling Svadu should not
>> break any S-mode software.
> Unfortunately I joined yesterday's patchwork call late and missed this
> discussion. I'm still not sure how we avoid concerns with S-mode software
> expecting exceptions by purposely not setting A/D bits, but then not
> getting those exceptions.


Most other architectures implement hardware A/D updates, so I don't see 
what's specific in riscv. In addition, if an OS really needs the 
exceptions, it can always play with the page table permissions to 
achieve such behaviour.


>
>> This is what you did in your previous versions of
>> this patchset so the changes should be easy. This behaviour must be added to
>> the dtbinding description of the Svadu extension.
>>
>> Another thing that we discussed yesterday. There exist 2 schemes to manage
>> the A/D bits updates, Svade and Svadu. If a platform supports both
>> extensions and both are present in the device tree, it is M-mode firmware's
>> responsibility to provide a "sane" device tree to the S-mode software,
>> meaning the device tree can not contain both extensions. And because on such
>> platforms, Svadu is more performant than Svade, Svadu should be enabled by
>> the M-mode firmware and only Svadu should be present in the device tree.
> I'm not sure firmware will be able to choose svadu when it's available.
> For example, platforms which want to conform to the upcoming "Server
> Platform" specification must also conform to the RVA23 profile, which
> mandates Svade and lists Svadu as an optional extension. This implies to
> me that S-mode should be boot with both svade and svadu in the DT and with
> svade being the active one. Then, S-mode can choose to request switching
> to svadu with FWFT.


The problem is that FWFT is not there and won't be there for ~1y 
(according to Anup). So in the meantime, we prevent all uarchs that 
support Svadu to take advantage of this.


>
> Thanks,
> drew
>
>> I hope that clearly explains what we discussed yesterday, let me know if you
>> (or anyone else) need more explanations. If no one is opposed to this
>> solution, do you think you can implement this behaviour? If not, I can deal
>> with it, just let me know.
>>
>> Thanks
>>
>>
>>>> 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: Conor Dooley <conor.dooley@microchip.com>
>>>> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
>>> I think this patch changed too much to keep r-b's. We didn't have the
>>> FWFT part before.
>>>
>>>> ---
>>>>    arch/riscv/Kconfig               |  1 +
>>>>    arch/riscv/include/asm/csr.h     |  1 +
>>>>    arch/riscv/include/asm/hwcap.h   |  1 +
>>>>    arch/riscv/include/asm/pgtable.h |  8 +++++++-
>>>>    arch/riscv/kernel/cpufeature.c   | 11 +++++++++++
>>>>    5 files changed, 21 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>>>> index be09c8836d56..30fa558ee284 100644
>>>> --- a/arch/riscv/Kconfig
>>>> +++ b/arch/riscv/Kconfig
>>>> @@ -34,6 +34,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 2468c55933cd..2ac270ad4acd 100644
>>>> --- a/arch/riscv/include/asm/csr.h
>>>> +++ b/arch/riscv/include/asm/csr.h
>>>> @@ -194,6 +194,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..8d539e3f4e11 100644
>>>> --- a/arch/riscv/include/asm/hwcap.h
>>>> +++ b/arch/riscv/include/asm/hwcap.h
>>>> @@ -81,6 +81,7 @@
>>>>    #define RISCV_ISA_EXT_ZTSO		72
>>>>    #define RISCV_ISA_EXT_ZACAS		73
>>>>    #define RISCV_ISA_EXT_XANDESPMU		74
>>>> +#define RISCV_ISA_EXT_SVADU		75
>>>>    #define RISCV_ISA_EXT_XLINUXENVCFG	127
>>>> diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
>>>> index 9f8ea0e33eb1..1f1b326ccf63 100644
>>>> --- a/arch/riscv/include/asm/pgtable.h
>>>> +++ b/arch/riscv/include/asm/pgtable.h
>>>> @@ -117,6 +117,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)
>>>> @@ -285,7 +286,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)
>>>>    {
>>>> @@ -621,6 +621,12 @@ static inline pgprot_t pgprot_writecombine(pgprot_t _prot)
>>>>    	return __pgprot(prot);
>>>>    }
>>>> +#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 3ed2359eae35..b023908c5932 100644
>>>> --- a/arch/riscv/kernel/cpufeature.c
>>>> +++ b/arch/riscv/kernel/cpufeature.c
>>>> @@ -93,6 +93,16 @@ static bool riscv_isa_extension_check(int id)
>>>>    			return false;
>>>>    		}
>>>>    		return true;
>>>> +	case RISCV_ISA_EXT_SVADU:
>>>> +		if (sbi_probe_extension(SBI_EXT_FWFT) > 0) {
>>> I think we've decided the appropriate way to prove for SBI extensions is
>>> to first ensure the SBI version and then do the probe, like we do for STA
>>> in has_pv_steal_clock()
>>>
>>>> +			struct sbiret ret;
>>>> +
>>>> +			ret = sbi_ecall(SBI_EXT_FWFT, SBI_EXT_FWFT_SET, SBI_FWFT_PTE_AD_HW_UPDATING,
>>>> +					1, 0, 0, 0, 0);
>>>> +
>>>> +			return ret.error == SBI_SUCCESS;
>>>> +		}
>>>> +		return false;
>>>>    	case RISCV_ISA_EXT_INVALID:
>>>>    		return false;
>>>>    	}
>>>> @@ -301,6 +311,7 @@ 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_SUPERSET(svadu, RISCV_ISA_EXT_SVADU, riscv_xlinuxenvcfg_exts),
>>> We do we need XLINUXENVCFG?
>>>
>>> Thanks,
>>> drew
>>>
>>>>    	__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.17.1
>>>>
>>> _______________________________________________
>>> linux-riscv mailing list
>>> linux-riscv@lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [RFC PATCH v4 1/5] RISC-V: Detect and Enable Svadu Extension Support

[Anup Patel]

On Thu, May 30, 2024 at 2:31 PM Alexandre Ghiti <alex@ghiti.fr> wrote:
>
> Hi Andrew,
>
> On 30/05/2024 10:47, Andrew Jones wrote:
> > On Thu, May 30, 2024 at 10:19:12AM GMT, Alexandre Ghiti wrote:
> >> Hi Yong-Xuan,
> >>
> >> On 27/05/2024 18:25, Andrew Jones wrote:
> >>> On Fri, May 24, 2024 at 06:33:01PM GMT, Yong-Xuan Wang wrote:
> >>>> Svadu is a RISC-V extension for hardware updating of PTE A/D bits.
> >>>>
> >>>> In this patch we detect Svadu extension support from DTB and enable it
> >>>> with SBI FWFT extension. Also we add arch_has_hw_pte_young() to enable
> >>>> optimization in MGLRU and __wp_page_copy_user() if Svadu extension is
> >>>> available.
> >>
> >> So we talked about this yesterday during the linux-riscv patchwork meeting.
> >> We came to the conclusion that we should not wait for the SBI FWFT extension
> >> to enable Svadu but instead, it should be enabled by default by openSBI if
> >> the extension is present in the device tree. This is because we did not find
> >> any backward compatibility issues, meaning that enabling Svadu should not
> >> break any S-mode software.
> > Unfortunately I joined yesterday's patchwork call late and missed this
> > discussion. I'm still not sure how we avoid concerns with S-mode software
> > expecting exceptions by purposely not setting A/D bits, but then not
> > getting those exceptions.
>
>
> Most other architectures implement hardware A/D updates, so I don't see
> what's specific in riscv. In addition, if an OS really needs the
> exceptions, it can always play with the page table permissions to
> achieve such behaviour.
>
>
> >
> >> This is what you did in your previous versions of
> >> this patchset so the changes should be easy. This behaviour must be added to
> >> the dtbinding description of the Svadu extension.
> >>
> >> Another thing that we discussed yesterday. There exist 2 schemes to manage
> >> the A/D bits updates, Svade and Svadu. If a platform supports both
> >> extensions and both are present in the device tree, it is M-mode firmware's
> >> responsibility to provide a "sane" device tree to the S-mode software,
> >> meaning the device tree can not contain both extensions. And because on such
> >> platforms, Svadu is more performant than Svade, Svadu should be enabled by
> >> the M-mode firmware and only Svadu should be present in the device tree.
> > I'm not sure firmware will be able to choose svadu when it's available.
> > For example, platforms which want to conform to the upcoming "Server
> > Platform" specification must also conform to the RVA23 profile, which
> > mandates Svade and lists Svadu as an optional extension. This implies to
> > me that S-mode should be boot with both svade and svadu in the DT and with
> > svade being the active one. Then, S-mode can choose to request switching
> > to svadu with FWFT.
>
>
> The problem is that FWFT is not there and won't be there for ~1y
> (according to Anup). So in the meantime, we prevent all uarchs that
> support Svadu to take advantage of this.

SBI v3.0 is expected to freeze by the end of this year so I don't
think we will have to wait for ~1yr.

Plus nothing stops a company to apply patches themselves to
test on their implementations. Quite a few companies have internal
forks of Linux where they track upstream patches until they are
merged.

Regards,
Anup

>
>
> >
> > Thanks,
> > drew
> >
> >> I hope that clearly explains what we discussed yesterday, let me know if you
> >> (or anyone else) need more explanations. If no one is opposed to this
> >> solution, do you think you can implement this behaviour? If not, I can deal
> >> with it, just let me know.
> >>
> >> Thanks
> >>
> >>
> >>>> 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: Conor Dooley <conor.dooley@microchip.com>
> >>>> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> >>> I think this patch changed too much to keep r-b's. We didn't have the
> >>> FWFT part before.
> >>>
> >>>> ---
> >>>>    arch/riscv/Kconfig               |  1 +
> >>>>    arch/riscv/include/asm/csr.h     |  1 +
> >>>>    arch/riscv/include/asm/hwcap.h   |  1 +
> >>>>    arch/riscv/include/asm/pgtable.h |  8 +++++++-
> >>>>    arch/riscv/kernel/cpufeature.c   | 11 +++++++++++
> >>>>    5 files changed, 21 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> >>>> index be09c8836d56..30fa558ee284 100644
> >>>> --- a/arch/riscv/Kconfig
> >>>> +++ b/arch/riscv/Kconfig
> >>>> @@ -34,6 +34,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 2468c55933cd..2ac270ad4acd 100644
> >>>> --- a/arch/riscv/include/asm/csr.h
> >>>> +++ b/arch/riscv/include/asm/csr.h
> >>>> @@ -194,6 +194,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..8d539e3f4e11 100644
> >>>> --- a/arch/riscv/include/asm/hwcap.h
> >>>> +++ b/arch/riscv/include/asm/hwcap.h
> >>>> @@ -81,6 +81,7 @@
> >>>>    #define RISCV_ISA_EXT_ZTSO              72
> >>>>    #define RISCV_ISA_EXT_ZACAS             73
> >>>>    #define RISCV_ISA_EXT_XANDESPMU         74
> >>>> +#define RISCV_ISA_EXT_SVADU               75
> >>>>    #define RISCV_ISA_EXT_XLINUXENVCFG      127
> >>>> diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
> >>>> index 9f8ea0e33eb1..1f1b326ccf63 100644
> >>>> --- a/arch/riscv/include/asm/pgtable.h
> >>>> +++ b/arch/riscv/include/asm/pgtable.h
> >>>> @@ -117,6 +117,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)
> >>>> @@ -285,7 +286,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)
> >>>>    {
> >>>> @@ -621,6 +621,12 @@ static inline pgprot_t pgprot_writecombine(pgprot_t _prot)
> >>>>            return __pgprot(prot);
> >>>>    }
> >>>> +#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 3ed2359eae35..b023908c5932 100644
> >>>> --- a/arch/riscv/kernel/cpufeature.c
> >>>> +++ b/arch/riscv/kernel/cpufeature.c
> >>>> @@ -93,6 +93,16 @@ static bool riscv_isa_extension_check(int id)
> >>>>                            return false;
> >>>>                    }
> >>>>                    return true;
> >>>> +  case RISCV_ISA_EXT_SVADU:
> >>>> +          if (sbi_probe_extension(SBI_EXT_FWFT) > 0) {
> >>> I think we've decided the appropriate way to prove for SBI extensions is
> >>> to first ensure the SBI version and then do the probe, like we do for STA
> >>> in has_pv_steal_clock()
> >>>
> >>>> +                  struct sbiret ret;
> >>>> +
> >>>> +                  ret = sbi_ecall(SBI_EXT_FWFT, SBI_EXT_FWFT_SET, SBI_FWFT_PTE_AD_HW_UPDATING,
> >>>> +                                  1, 0, 0, 0, 0);
> >>>> +
> >>>> +                  return ret.error == SBI_SUCCESS;
> >>>> +          }
> >>>> +          return false;
> >>>>            case RISCV_ISA_EXT_INVALID:
> >>>>                    return false;
> >>>>            }
> >>>> @@ -301,6 +311,7 @@ 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_SUPERSET(svadu, RISCV_ISA_EXT_SVADU, riscv_xlinuxenvcfg_exts),
> >>> We do we need XLINUXENVCFG?
> >>>
> >>> Thanks,
> >>> drew
> >>>
> >>>>            __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.17.1
> >>>>
> >>> _______________________________________________
> >>> linux-riscv mailing list
> >>> linux-riscv@lists.infradead.org
> >>> http://lists.infradead.org/mailman/listinfo/linux-riscv
>
> --
> kvm-riscv mailing list
> kvm-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kvm-riscv

Re: [RFC PATCH v4 1/5] RISC-V: Detect and Enable Svadu Extension Support

[Andrew Jones]

On Thu, May 30, 2024 at 11:01:20AM GMT, Alexandre Ghiti wrote:
> Hi Andrew,
> 
> On 30/05/2024 10:47, Andrew Jones wrote:
> > On Thu, May 30, 2024 at 10:19:12AM GMT, Alexandre Ghiti wrote:
> > > Hi Yong-Xuan,
> > > 
> > > On 27/05/2024 18:25, Andrew Jones wrote:
> > > > On Fri, May 24, 2024 at 06:33:01PM GMT, Yong-Xuan Wang wrote:
> > > > > Svadu is a RISC-V extension for hardware updating of PTE A/D bits.
> > > > > 
> > > > > In this patch we detect Svadu extension support from DTB and enable it
> > > > > with SBI FWFT extension. Also we add arch_has_hw_pte_young() to enable
> > > > > optimization in MGLRU and __wp_page_copy_user() if Svadu extension is
> > > > > available.
> > > 
> > > So we talked about this yesterday during the linux-riscv patchwork meeting.
> > > We came to the conclusion that we should not wait for the SBI FWFT extension
> > > to enable Svadu but instead, it should be enabled by default by openSBI if
> > > the extension is present in the device tree. This is because we did not find
> > > any backward compatibility issues, meaning that enabling Svadu should not
> > > break any S-mode software.
> > Unfortunately I joined yesterday's patchwork call late and missed this
> > discussion. I'm still not sure how we avoid concerns with S-mode software
> > expecting exceptions by purposely not setting A/D bits, but then not
> > getting those exceptions.
> 
> 
> Most other architectures implement hardware A/D updates, so I don't see
> what's specific in riscv. In addition, if an OS really needs the exceptions,
> it can always play with the page table permissions to achieve such
> behaviour.

Hmm, yeah we're probably pretty safe since sorting this out is just one of
many things an OS will have to learn to manage when getting ported. Also,
handling both svade and svadu at boot is trivial since the OS simply needs
to set the A/D bits when creating the PTEs or have exception handlers
which do nothing but set the bits ready just in case.

> 
> 
> > 
> > > This is what you did in your previous versions of
> > > this patchset so the changes should be easy. This behaviour must be added to
> > > the dtbinding description of the Svadu extension.
> > > 
> > > Another thing that we discussed yesterday. There exist 2 schemes to manage
> > > the A/D bits updates, Svade and Svadu. If a platform supports both
> > > extensions and both are present in the device tree, it is M-mode firmware's
> > > responsibility to provide a "sane" device tree to the S-mode software,
> > > meaning the device tree can not contain both extensions. And because on such
> > > platforms, Svadu is more performant than Svade, Svadu should be enabled by
> > > the M-mode firmware and only Svadu should be present in the device tree.
> > I'm not sure firmware will be able to choose svadu when it's available.
> > For example, platforms which want to conform to the upcoming "Server
> > Platform" specification must also conform to the RVA23 profile, which
> > mandates Svade and lists Svadu as an optional extension. This implies to
> > me that S-mode should be boot with both svade and svadu in the DT and with
> > svade being the active one. Then, S-mode can choose to request switching
> > to svadu with FWFT.
> 
> 
> The problem is that FWFT is not there and won't be there for ~1y (according
> to Anup). So in the meantime, we prevent all uarchs that support Svadu to
> take advantage of this.

I think we should have documented behaviors for all four possibilities

 1. Neither svade nor svadu in DT -- current behavior
 2. Only svade in DT -- current behavior
 3. Only svadu in DT -- expect hardware A/D updating
 4. Both svade and svadu in DT -- current behavior, but, if we have FWFT,
    then use it to switch to svadu. If we don't have FWFT, then, oh well...

Platforms/firmwares that aren't concerned with the profiles can choose (3)
and Linux is fine. Those that do want to conform to the profile will
choose (4) but Linux won't get the benefit of svadu until it also gets
FWFT.

IOW, I think your proposal is fine except for wanting to document in the
DT bindings that only svade or svadu may be provided, since I think we'll
want both to be allowed eventually.

Thanks,
drew

Re: [RFC PATCH v4 1/5] RISC-V: Detect and Enable Svadu Extension Support

[Yong-Xuan Wang]

Hi Alexandre,

On Thu, May 30, 2024 at 4:19 PM Alexandre Ghiti <alex@ghiti.fr> wrote:
>
> Hi Yong-Xuan,
>
> On 27/05/2024 18:25, Andrew Jones wrote:
> > On Fri, May 24, 2024 at 06:33:01PM GMT, Yong-Xuan Wang wrote:
> >> Svadu is a RISC-V extension for hardware updating of PTE A/D bits.
> >>
> >> In this patch we detect Svadu extension support from DTB and enable it
> >> with SBI FWFT extension. Also we add arch_has_hw_pte_young() to enable
> >> optimization in MGLRU and __wp_page_copy_user() if Svadu extension is
> >> available.
>
>
> So we talked about this yesterday during the linux-riscv patchwork
> meeting. We came to the conclusion that we should not wait for the SBI
> FWFT extension to enable Svadu but instead, it should be enabled by
> default by openSBI if the extension is present in the device tree. This
> is because we did not find any backward compatibility issues, meaning
> that enabling Svadu should not break any S-mode software. This is what
> you did in your previous versions of this patchset so the changes should
> be easy. This behaviour must be added to the dtbinding description of
> the Svadu extension.
>
> Another thing that we discussed yesterday. There exist 2 schemes to
> manage the A/D bits updates, Svade and Svadu. If a platform supports
> both extensions and both are present in the device tree, it is M-mode
> firmware's responsibility to provide a "sane" device tree to the S-mode
> software, meaning the device tree can not contain both extensions. And
> because on such platforms, Svadu is more performant than Svade, Svadu
> should be enabled by the M-mode firmware and only Svadu should be
> present in the device tree.
>
> I hope that clearly explains what we discussed yesterday, let me know if
> you (or anyone else) need more explanations. If no one is opposed to
> this solution, do you think you can implement this behaviour? If not, I
> can deal with it, just let me know.
>
> Thanks
>
>

Sure, I can do it. Just to confirm, which extension should be removed
when both Svade and Svadu are present in the DT?

Regards,
Yong-Xuan

> >>
> >> 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: Conor Dooley <conor.dooley@microchip.com>
> >> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> > I think this patch changed too much to keep r-b's. We didn't have the
> > FWFT part before.
> >
> >> ---
> >>   arch/riscv/Kconfig               |  1 +
> >>   arch/riscv/include/asm/csr.h     |  1 +
> >>   arch/riscv/include/asm/hwcap.h   |  1 +
> >>   arch/riscv/include/asm/pgtable.h |  8 +++++++-
> >>   arch/riscv/kernel/cpufeature.c   | 11 +++++++++++
> >>   5 files changed, 21 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> >> index be09c8836d56..30fa558ee284 100644
> >> --- a/arch/riscv/Kconfig
> >> +++ b/arch/riscv/Kconfig
> >> @@ -34,6 +34,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 2468c55933cd..2ac270ad4acd 100644
> >> --- a/arch/riscv/include/asm/csr.h
> >> +++ b/arch/riscv/include/asm/csr.h
> >> @@ -194,6 +194,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..8d539e3f4e11 100644
> >> --- a/arch/riscv/include/asm/hwcap.h
> >> +++ b/arch/riscv/include/asm/hwcap.h
> >> @@ -81,6 +81,7 @@
> >>   #define RISCV_ISA_EXT_ZTSO         72
> >>   #define RISCV_ISA_EXT_ZACAS                73
> >>   #define RISCV_ISA_EXT_XANDESPMU            74
> >> +#define RISCV_ISA_EXT_SVADU         75
> >>
> >>   #define RISCV_ISA_EXT_XLINUXENVCFG 127
> >>
> >> diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
> >> index 9f8ea0e33eb1..1f1b326ccf63 100644
> >> --- a/arch/riscv/include/asm/pgtable.h
> >> +++ b/arch/riscv/include/asm/pgtable.h
> >> @@ -117,6 +117,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)
> >>
> >> @@ -285,7 +286,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)
> >>   {
> >> @@ -621,6 +621,12 @@ static inline pgprot_t pgprot_writecombine(pgprot_t _prot)
> >>      return __pgprot(prot);
> >>   }
> >>
> >> +#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 3ed2359eae35..b023908c5932 100644
> >> --- a/arch/riscv/kernel/cpufeature.c
> >> +++ b/arch/riscv/kernel/cpufeature.c
> >> @@ -93,6 +93,16 @@ static bool riscv_isa_extension_check(int id)
> >>                      return false;
> >>              }
> >>              return true;
> >> +    case RISCV_ISA_EXT_SVADU:
> >> +            if (sbi_probe_extension(SBI_EXT_FWFT) > 0) {
> > I think we've decided the appropriate way to prove for SBI extensions is
> > to first ensure the SBI version and then do the probe, like we do for STA
> > in has_pv_steal_clock()
> >
> >> +                    struct sbiret ret;
> >> +
> >> +                    ret = sbi_ecall(SBI_EXT_FWFT, SBI_EXT_FWFT_SET, SBI_FWFT_PTE_AD_HW_UPDATING,
> >> +                                    1, 0, 0, 0, 0);
> >> +
> >> +                    return ret.error == SBI_SUCCESS;
> >> +            }
> >> +            return false;
> >>      case RISCV_ISA_EXT_INVALID:
> >>              return false;
> >>      }
> >> @@ -301,6 +311,7 @@ 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_SUPERSET(svadu, RISCV_ISA_EXT_SVADU, riscv_xlinuxenvcfg_exts),
> > We do we need XLINUXENVCFG?
> >
> > Thanks,
> > drew
> >
> >>      __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.17.1
> >>
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [RFC PATCH v4 1/5] RISC-V: Detect and Enable Svadu Extension Support

[Alexandre Ghiti]

On 30/05/2024 11:24, Andrew Jones wrote:
> On Thu, May 30, 2024 at 11:01:20AM GMT, Alexandre Ghiti wrote:
>> Hi Andrew,
>>
>> On 30/05/2024 10:47, Andrew Jones wrote:
>>> On Thu, May 30, 2024 at 10:19:12AM GMT, Alexandre Ghiti wrote:
>>>> Hi Yong-Xuan,
>>>>
>>>> On 27/05/2024 18:25, Andrew Jones wrote:
>>>>> On Fri, May 24, 2024 at 06:33:01PM GMT, Yong-Xuan Wang wrote:
>>>>>> Svadu is a RISC-V extension for hardware updating of PTE A/D bits.
>>>>>>
>>>>>> In this patch we detect Svadu extension support from DTB and enable it
>>>>>> with SBI FWFT extension. Also we add arch_has_hw_pte_young() to enable
>>>>>> optimization in MGLRU and __wp_page_copy_user() if Svadu extension is
>>>>>> available.
>>>> So we talked about this yesterday during the linux-riscv patchwork meeting.
>>>> We came to the conclusion that we should not wait for the SBI FWFT extension
>>>> to enable Svadu but instead, it should be enabled by default by openSBI if
>>>> the extension is present in the device tree. This is because we did not find
>>>> any backward compatibility issues, meaning that enabling Svadu should not
>>>> break any S-mode software.
>>> Unfortunately I joined yesterday's patchwork call late and missed this
>>> discussion. I'm still not sure how we avoid concerns with S-mode software
>>> expecting exceptions by purposely not setting A/D bits, but then not
>>> getting those exceptions.
>>
>> Most other architectures implement hardware A/D updates, so I don't see
>> what's specific in riscv. In addition, if an OS really needs the exceptions,
>> it can always play with the page table permissions to achieve such
>> behaviour.
> Hmm, yeah we're probably pretty safe since sorting this out is just one of
> many things an OS will have to learn to manage when getting ported. Also,
> handling both svade and svadu at boot is trivial since the OS simply needs
> to set the A/D bits when creating the PTEs or have exception handlers
> which do nothing but set the bits ready just in case.
>
>>
>>>> This is what you did in your previous versions of
>>>> this patchset so the changes should be easy. This behaviour must be added to
>>>> the dtbinding description of the Svadu extension.
>>>>
>>>> Another thing that we discussed yesterday. There exist 2 schemes to manage
>>>> the A/D bits updates, Svade and Svadu. If a platform supports both
>>>> extensions and both are present in the device tree, it is M-mode firmware's
>>>> responsibility to provide a "sane" device tree to the S-mode software,
>>>> meaning the device tree can not contain both extensions. And because on such
>>>> platforms, Svadu is more performant than Svade, Svadu should be enabled by
>>>> the M-mode firmware and only Svadu should be present in the device tree.
>>> I'm not sure firmware will be able to choose svadu when it's available.
>>> For example, platforms which want to conform to the upcoming "Server
>>> Platform" specification must also conform to the RVA23 profile, which
>>> mandates Svade and lists Svadu as an optional extension. This implies to
>>> me that S-mode should be boot with both svade and svadu in the DT and with
>>> svade being the active one. Then, S-mode can choose to request switching
>>> to svadu with FWFT.
>>
>> The problem is that FWFT is not there and won't be there for ~1y (according
>> to Anup). So in the meantime, we prevent all uarchs that support Svadu to
>> take advantage of this.
> I think we should have documented behaviors for all four possibilities
>
>   1. Neither svade nor svadu in DT -- current behavior
>   2. Only svade in DT -- current behavior
>   3. Only svadu in DT -- expect hardware A/D updating
>   4. Both svade and svadu in DT -- current behavior, but, if we have FWFT,
>      then use it to switch to svadu. If we don't have FWFT, then, oh well...
>
> Platforms/firmwares that aren't concerned with the profiles can choose (3)
> and Linux is fine. Those that do want to conform to the profile will
> choose (4) but Linux won't get the benefit of svadu until it also gets
> FWFT.


I think this solution pleases everyone so I'd say we should go for it, 
thanks Andrew!

@Yong-Xuan do you think you can prepare another spin with Andrew's 
proposal implemented?

Thanks,

Alex


>
> IOW, I think your proposal is fine except for wanting to document in the
> DT bindings that only svade or svadu may be provided, since I think we'll
> want both to be allowed eventually.
>
> Thanks,
> drew
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [RFC PATCH v4 1/5] RISC-V: Detect and Enable Svadu Extension Support

[Anup Patel]

On Mon, Jun 3, 2024 at 4:59 PM Alexandre Ghiti <alex@ghiti.fr> wrote:
>
> On 30/05/2024 11:24, Andrew Jones wrote:
> > On Thu, May 30, 2024 at 11:01:20AM GMT, Alexandre Ghiti wrote:
> >> Hi Andrew,
> >>
> >> On 30/05/2024 10:47, Andrew Jones wrote:
> >>> On Thu, May 30, 2024 at 10:19:12AM GMT, Alexandre Ghiti wrote:
> >>>> Hi Yong-Xuan,
> >>>>
> >>>> On 27/05/2024 18:25, Andrew Jones wrote:
> >>>>> On Fri, May 24, 2024 at 06:33:01PM GMT, Yong-Xuan Wang wrote:
> >>>>>> Svadu is a RISC-V extension for hardware updating of PTE A/D bits.
> >>>>>>
> >>>>>> In this patch we detect Svadu extension support from DTB and enable it
> >>>>>> with SBI FWFT extension. Also we add arch_has_hw_pte_young() to enable
> >>>>>> optimization in MGLRU and __wp_page_copy_user() if Svadu extension is
> >>>>>> available.
> >>>> So we talked about this yesterday during the linux-riscv patchwork meeting.
> >>>> We came to the conclusion that we should not wait for the SBI FWFT extension
> >>>> to enable Svadu but instead, it should be enabled by default by openSBI if
> >>>> the extension is present in the device tree. This is because we did not find
> >>>> any backward compatibility issues, meaning that enabling Svadu should not
> >>>> break any S-mode software.
> >>> Unfortunately I joined yesterday's patchwork call late and missed this
> >>> discussion. I'm still not sure how we avoid concerns with S-mode software
> >>> expecting exceptions by purposely not setting A/D bits, but then not
> >>> getting those exceptions.
> >>
> >> Most other architectures implement hardware A/D updates, so I don't see
> >> what's specific in riscv. In addition, if an OS really needs the exceptions,
> >> it can always play with the page table permissions to achieve such
> >> behaviour.
> > Hmm, yeah we're probably pretty safe since sorting this out is just one of
> > many things an OS will have to learn to manage when getting ported. Also,
> > handling both svade and svadu at boot is trivial since the OS simply needs
> > to set the A/D bits when creating the PTEs or have exception handlers
> > which do nothing but set the bits ready just in case.
> >
> >>
> >>>> This is what you did in your previous versions of
> >>>> this patchset so the changes should be easy. This behaviour must be added to
> >>>> the dtbinding description of the Svadu extension.
> >>>>
> >>>> Another thing that we discussed yesterday. There exist 2 schemes to manage
> >>>> the A/D bits updates, Svade and Svadu. If a platform supports both
> >>>> extensions and both are present in the device tree, it is M-mode firmware's
> >>>> responsibility to provide a "sane" device tree to the S-mode software,
> >>>> meaning the device tree can not contain both extensions. And because on such
> >>>> platforms, Svadu is more performant than Svade, Svadu should be enabled by
> >>>> the M-mode firmware and only Svadu should be present in the device tree.
> >>> I'm not sure firmware will be able to choose svadu when it's available.
> >>> For example, platforms which want to conform to the upcoming "Server
> >>> Platform" specification must also conform to the RVA23 profile, which
> >>> mandates Svade and lists Svadu as an optional extension. This implies to
> >>> me that S-mode should be boot with both svade and svadu in the DT and with
> >>> svade being the active one. Then, S-mode can choose to request switching
> >>> to svadu with FWFT.
> >>
> >> The problem is that FWFT is not there and won't be there for ~1y (according
> >> to Anup). So in the meantime, we prevent all uarchs that support Svadu to
> >> take advantage of this.
> > I think we should have documented behaviors for all four possibilities
> >
> >   1. Neither svade nor svadu in DT -- current behavior
> >   2. Only svade in DT -- current behavior
> >   3. Only svadu in DT -- expect hardware A/D updating
> >   4. Both svade and svadu in DT -- current behavior, but, if we have FWFT,
> >      then use it to switch to svadu. If we don't have FWFT, then, oh well...
> >
> > Platforms/firmwares that aren't concerned with the profiles can choose (3)
> > and Linux is fine. Those that do want to conform to the profile will
> > choose (4) but Linux won't get the benefit of svadu until it also gets
> > FWFT.
>
>
> I think this solution pleases everyone so I'd say we should go for it,
> thanks Andrew!

Yes, this looks good to me as well. The key aspect is documenting
the behaviour of these four possibilities.

Regards,
Anup

>
> @Yong-Xuan do you think you can prepare another spin with Andrew's
> proposal implemented?
>
> Thanks,
>
> Alex
>
>
> >
> > IOW, I think your proposal is fine except for wanting to document in the
> > DT bindings that only svade or svadu may be provided, since I think we'll
> > want both to be allowed eventually.
> >
> > Thanks,
> > drew
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [RFC PATCH v4 1/5] RISC-V: Detect and Enable Svadu Extension Support

[Yong-Xuan Wang]

Hi Alexandre,

On Mon, Jun 3, 2024 at 7:29 PM Alexandre Ghiti <alex@ghiti.fr> wrote:
>
> On 30/05/2024 11:24, Andrew Jones wrote:
> > On Thu, May 30, 2024 at 11:01:20AM GMT, Alexandre Ghiti wrote:
> >> Hi Andrew,
> >>
> >> On 30/05/2024 10:47, Andrew Jones wrote:
> >>> On Thu, May 30, 2024 at 10:19:12AM GMT, Alexandre Ghiti wrote:
> >>>> Hi Yong-Xuan,
> >>>>
> >>>> On 27/05/2024 18:25, Andrew Jones wrote:
> >>>>> On Fri, May 24, 2024 at 06:33:01PM GMT, Yong-Xuan Wang wrote:
> >>>>>> Svadu is a RISC-V extension for hardware updating of PTE A/D bits.
> >>>>>>
> >>>>>> In this patch we detect Svadu extension support from DTB and enable it
> >>>>>> with SBI FWFT extension. Also we add arch_has_hw_pte_young() to enable
> >>>>>> optimization in MGLRU and __wp_page_copy_user() if Svadu extension is
> >>>>>> available.
> >>>> So we talked about this yesterday during the linux-riscv patchwork meeting.
> >>>> We came to the conclusion that we should not wait for the SBI FWFT extension
> >>>> to enable Svadu but instead, it should be enabled by default by openSBI if
> >>>> the extension is present in the device tree. This is because we did not find
> >>>> any backward compatibility issues, meaning that enabling Svadu should not
> >>>> break any S-mode software.
> >>> Unfortunately I joined yesterday's patchwork call late and missed this
> >>> discussion. I'm still not sure how we avoid concerns with S-mode software
> >>> expecting exceptions by purposely not setting A/D bits, but then not
> >>> getting those exceptions.
> >>
> >> Most other architectures implement hardware A/D updates, so I don't see
> >> what's specific in riscv. In addition, if an OS really needs the exceptions,
> >> it can always play with the page table permissions to achieve such
> >> behaviour.
> > Hmm, yeah we're probably pretty safe since sorting this out is just one of
> > many things an OS will have to learn to manage when getting ported. Also,
> > handling both svade and svadu at boot is trivial since the OS simply needs
> > to set the A/D bits when creating the PTEs or have exception handlers
> > which do nothing but set the bits ready just in case.
> >
> >>
> >>>> This is what you did in your previous versions of
> >>>> this patchset so the changes should be easy. This behaviour must be added to
> >>>> the dtbinding description of the Svadu extension.
> >>>>
> >>>> Another thing that we discussed yesterday. There exist 2 schemes to manage
> >>>> the A/D bits updates, Svade and Svadu. If a platform supports both
> >>>> extensions and both are present in the device tree, it is M-mode firmware's
> >>>> responsibility to provide a "sane" device tree to the S-mode software,
> >>>> meaning the device tree can not contain both extensions. And because on such
> >>>> platforms, Svadu is more performant than Svade, Svadu should be enabled by
> >>>> the M-mode firmware and only Svadu should be present in the device tree.
> >>> I'm not sure firmware will be able to choose svadu when it's available.
> >>> For example, platforms which want to conform to the upcoming "Server
> >>> Platform" specification must also conform to the RVA23 profile, which
> >>> mandates Svade and lists Svadu as an optional extension. This implies to
> >>> me that S-mode should be boot with both svade and svadu in the DT and with
> >>> svade being the active one. Then, S-mode can choose to request switching
> >>> to svadu with FWFT.
> >>
> >> The problem is that FWFT is not there and won't be there for ~1y (according
> >> to Anup). So in the meantime, we prevent all uarchs that support Svadu to
> >> take advantage of this.
> > I think we should have documented behaviors for all four possibilities
> >
> >   1. Neither svade nor svadu in DT -- current behavior
> >   2. Only svade in DT -- current behavior
> >   3. Only svadu in DT -- expect hardware A/D updating
> >   4. Both svade and svadu in DT -- current behavior, but, if we have FWFT,
> >      then use it to switch to svadu. If we don't have FWFT, then, oh well...
> >
> > Platforms/firmwares that aren't concerned with the profiles can choose (3)
> > and Linux is fine. Those that do want to conform to the profile will
> > choose (4) but Linux won't get the benefit of svadu until it also gets
> > FWFT.
>
>
> I think this solution pleases everyone so I'd say we should go for it,
> thanks Andrew!
>
> @Yong-Xuan do you think you can prepare another spin with Andrew's
> proposal implemented?
>

Ok, the next version will be based on this proposal.

Regards,
Yong-Xuan

> Thanks,
>
> Alex
>
>
> >
> > IOW, I think your proposal is fine except for wanting to document in the
> > DT bindings that only svade or svadu may be provided, since I think we'll
> > want both to be allowed eventually.
> >
> > Thanks,
> > drew
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv