[v7] Add Svade and Svadu Extensions Support

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

Posted by Yong-Xuan Wang 1 year, 7 months ago

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

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

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

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

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

Posted by Samuel Holland 1 year, 6 months ago

Hi Yong-Xuan,

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

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

These lines should be sorted alphabetically.

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

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

Regards,
Samuel

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

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

Posted by Yong-Xuan Wang 1 year, 6 months ago

Hi Samuel,

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

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

Regards,
Yong-Xuan

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

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

Posted by Alexandre Ghiti 1 year, 6 months ago

Hi Yong-Xuan,

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


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

Thanks,

Alex

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

Posted by Clément Léger 1 year, 6 months ago


Hi Yong-Xuan,


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

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

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

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

	return 0;
}

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

	return 0;
}

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

Thanks,

Clément



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

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

Posted by Yong-Xuan Wang 1 year, 6 months ago

Hi Clément,

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

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

Regards,
Yong-Xuan

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

[PATCH v7 1/4] RISC-V: Add Svade and Svadu Extensions Support
[PATCH v7 2/4] dt-bindings: riscv: Add Svade and Svadu Entries
[PATCH v7 3/4] RISC-V: KVM: Add Svade and Svadu Extensions Support for Guest/VM
[PATCH v7 4/4] KVM: riscv: selftests: Add Svade and Svadu Extension to get-reg-list test