[PATCH -fixes 1/2] riscv: Fix enabling cbo.zero when running in M-mode

[PATCH -fixes 1/2] riscv: Fix enabling cbo.zero when running in M-mode

[Samuel Holland]

When the kernel is running in M-mode, the CBZE bit must be set in the
menvcfg CSR, not in senvcfg.

Cc: stable@kernel.org
Fixes: 43c16d51a19b ("RISC-V: Enable cbo.zero in usermode")
Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
---

 arch/riscv/include/asm/csr.h   | 2 ++
 arch/riscv/kernel/cpufeature.c | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
index 510014051f5d..2468c55933cd 100644
--- a/arch/riscv/include/asm/csr.h
+++ b/arch/riscv/include/asm/csr.h
@@ -424,6 +424,7 @@
 # define CSR_STATUS	CSR_MSTATUS
 # define CSR_IE		CSR_MIE
 # define CSR_TVEC	CSR_MTVEC
+# define CSR_ENVCFG	CSR_MENVCFG
 # define CSR_SCRATCH	CSR_MSCRATCH
 # define CSR_EPC	CSR_MEPC
 # define CSR_CAUSE	CSR_MCAUSE
@@ -448,6 +449,7 @@
 # define CSR_STATUS	CSR_SSTATUS
 # define CSR_IE		CSR_SIE
 # define CSR_TVEC	CSR_STVEC
+# define CSR_ENVCFG	CSR_SENVCFG
 # define CSR_SCRATCH	CSR_SSCRATCH
 # define CSR_EPC	CSR_SEPC
 # define CSR_CAUSE	CSR_SCAUSE
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 89920f84d0a3..c5b13f7dd482 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -950,7 +950,7 @@ arch_initcall(check_unaligned_access_all_cpus);
 void riscv_user_isa_enable(void)
 {
 	if (riscv_cpu_has_extension_unlikely(smp_processor_id(), RISCV_ISA_EXT_ZICBOZ))
-		csr_set(CSR_SENVCFG, ENVCFG_CBZE);
+		csr_set(CSR_ENVCFG, ENVCFG_CBZE);
 }
 
 #ifdef CONFIG_RISCV_ALTERNATIVE
-- 
2.43.0

[PATCH -fixes 2/2] riscv: Save/restore envcfg CSR during CPU suspend

[Samuel Holland]

The value of the [ms]envcfg CSR is lost when entering a nonretentive
idle state, so the CSR must be rewritten when resuming the CPU.

Because the [ms]envcfg CSR is part of the base RISC-V privileged ISA
specification, it cannot be detected from the ISA string. However, most
existing hardware is too old to implement this CSR. As a result, it must
be probed at runtime.

Extend the logic for the Zicsr ISA extension to probe for the presence
of specific CSRs. Since the CSR number is encoded as an immediate value
within the csrr instruction, a switch case is necessary for any CSR that
must be probed this way. Use the exception table to handle the illegal
instruction exception raised when the CSR is not implemented.

Cc: stable@kernel.org
Fixes: 43c16d51a19b ("RISC-V: Enable cbo.zero in usermode")
Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
---

 arch/riscv/include/asm/csr.h     | 23 +++++++++++++++++++++++
 arch/riscv/include/asm/suspend.h |  1 +
 arch/riscv/kernel/cpufeature.c   | 23 +++++++++++++++++++++++
 arch/riscv/kernel/suspend.c      |  2 ++
 4 files changed, 49 insertions(+)

diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
index 2468c55933cd..daff95feb817 100644
--- a/arch/riscv/include/asm/csr.h
+++ b/arch/riscv/include/asm/csr.h
@@ -542,6 +542,29 @@
 			      : "memory");			\
 })
 
+#define ALT_CSR_READ(csr)					\
+({								\
+	unsigned long __v;					\
+	__asm__ __volatile__ (					\
+		ALTERNATIVE("li %[v], 0", "csrr %[v], %[r]", 0,	\
+			    csr << 16 | RISCV_ISA_EXT_ZICSR, 1)	\
+		: [v] "=r" (__v)				\
+		: [r] "i" (csr)					\
+		: "memory");					\
+	__v;							\
+})
+
+#define ALT_CSR_WRITE(csr, val)					\
+({								\
+	unsigned long __v = (unsigned long)(val);		\
+	__asm__ __volatile__ (					\
+		ALTERNATIVE("nop", "csrw %[r], %[v]", 0,	\
+			    csr << 16 | RISCV_ISA_EXT_ZICSR, 1)	\
+		: : [r] "i" (csr), [v] "rK" (__v)		\
+		: "memory");					\
+	__v;							\
+})
+
 #endif /* __ASSEMBLY__ */
 
 #endif /* _ASM_RISCV_CSR_H */
diff --git a/arch/riscv/include/asm/suspend.h b/arch/riscv/include/asm/suspend.h
index 02f87867389a..491296a335d0 100644
--- a/arch/riscv/include/asm/suspend.h
+++ b/arch/riscv/include/asm/suspend.h
@@ -14,6 +14,7 @@ struct suspend_context {
 	struct pt_regs regs;
 	/* Saved and restored by high-level functions */
 	unsigned long scratch;
+	unsigned long envcfg;
 	unsigned long tvec;
 	unsigned long ie;
 #ifdef CONFIG_MMU
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index c5b13f7dd482..934090270ae5 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -954,6 +954,27 @@ void riscv_user_isa_enable(void)
 }
 
 #ifdef CONFIG_RISCV_ALTERNATIVE
+static bool riscv_cpufeature_probe_csr(u16 csr)
+{
+	bool ret = false;
+
+	switch (csr) {
+#define PROBE_CSR_CASE(_csr)			\
+	case _csr:				\
+		asm("1:	csrr zero, %[csr]\n"	\
+		    "	li %[r], 1\n"		\
+		    "2:\n"			\
+		    _ASM_EXTABLE(1b, 2b)	\
+			: [r] "+r" (ret)	\
+			: [csr] "i" (_csr));	\
+		break
+	PROBE_CSR_CASE(CSR_ENVCFG);
+#undef PROBE_CSR_CASE
+	}
+
+	return ret;
+}
+
 /*
  * Alternative patch sites consider 48 bits when determining when to patch
  * the old instruction sequence with the new. These bits are broken into a
@@ -974,6 +995,8 @@ static bool riscv_cpufeature_patch_check(u16 id, u16 value)
 		return true;
 
 	switch (id) {
+	case RISCV_ISA_EXT_ZICSR:
+		return riscv_cpufeature_probe_csr(value);
 	case RISCV_ISA_EXT_ZICBOZ:
 		/*
 		 * Zicboz alternative applications provide the maximum
diff --git a/arch/riscv/kernel/suspend.c b/arch/riscv/kernel/suspend.c
index 239509367e42..fe544f12a5c5 100644
--- a/arch/riscv/kernel/suspend.c
+++ b/arch/riscv/kernel/suspend.c
@@ -15,6 +15,7 @@
 void suspend_save_csrs(struct suspend_context *context)
 {
 	context->scratch = csr_read(CSR_SCRATCH);
+	context->envcfg = ALT_CSR_READ(CSR_ENVCFG);
 	context->tvec = csr_read(CSR_TVEC);
 	context->ie = csr_read(CSR_IE);
 
@@ -36,6 +37,7 @@ void suspend_save_csrs(struct suspend_context *context)
 void suspend_restore_csrs(struct suspend_context *context)
 {
 	csr_write(CSR_SCRATCH, context->scratch);
+	ALT_CSR_WRITE(CSR_ENVCFG, context->envcfg);
 	csr_write(CSR_TVEC, context->tvec);
 	csr_write(CSR_IE, context->ie);
 
-- 
2.43.0

Re: [PATCH -fixes 2/2] riscv: Save/restore envcfg CSR during CPU suspend

[Conor Dooley]

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

On Sun, Feb 11, 2024 at 06:26:15PM -0800, Samuel Holland wrote:
> The value of the [ms]envcfg CSR is lost when entering a nonretentive
> idle state, so the CSR must be rewritten when resuming the CPU.
> 
> Because the [ms]envcfg CSR is part of the base RISC-V privileged ISA
> specification, it cannot be detected from the ISA string. However, most
> existing hardware is too old to implement this CSR. As a result, it must
> be probed at runtime.
> 
> Extend the logic for the Zicsr ISA extension to probe for the presence
> of specific CSRs. Since the CSR number is encoded as an immediate value
> within the csrr instruction, a switch case is necessary for any CSR that
> must be probed this way. Use the exception table to handle the illegal
> instruction exception raised when the CSR is not implemented.
> 
> Cc: stable@kernel.org
> Fixes: 43c16d51a19b ("RISC-V: Enable cbo.zero in usermode")
> Signed-off-by: Samuel Holland <samuel.holland@sifive.com>

On nommu:

  /build/tmp.RcQwFfgWA4/arch/riscv/kernel/cpufeature.c:967:21: error: expected ':' or ')' before '_ASM_EXTABLE'
  /build/tmp.RcQwFfgWA4/arch/riscv/kernel/cpufeature.c:967:34: error: invalid suffix "b" on integer constant
  /build/tmp.RcQwFfgWA4/arch/riscv/kernel/cpufeature.c:967:38: error: invalid suffix "b" on integer constant

https://patchwork.kernel.org/project/linux-riscv/patch/20240212022642.1968739-2-samuel.holland@sifive.com/

Cheers,
Conor.

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

Re: [PATCH -fixes 1/2] riscv: Fix enabling cbo.zero when running in M-mode

[Andrew Jones]

On Sun, Feb 11, 2024 at 06:26:14PM -0800, Samuel Holland wrote:
> When the kernel is running in M-mode, the CBZE bit must be set in the
> menvcfg CSR, not in senvcfg.
> 
> Cc: stable@kernel.org
> Fixes: 43c16d51a19b ("RISC-V: Enable cbo.zero in usermode")
> Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
> ---
> 
>  arch/riscv/include/asm/csr.h   | 2 ++
>  arch/riscv/kernel/cpufeature.c | 2 +-
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
> index 510014051f5d..2468c55933cd 100644
> --- a/arch/riscv/include/asm/csr.h
> +++ b/arch/riscv/include/asm/csr.h
> @@ -424,6 +424,7 @@
>  # define CSR_STATUS	CSR_MSTATUS
>  # define CSR_IE		CSR_MIE
>  # define CSR_TVEC	CSR_MTVEC
> +# define CSR_ENVCFG	CSR_MENVCFG
>  # define CSR_SCRATCH	CSR_MSCRATCH
>  # define CSR_EPC	CSR_MEPC
>  # define CSR_CAUSE	CSR_MCAUSE
> @@ -448,6 +449,7 @@
>  # define CSR_STATUS	CSR_SSTATUS
>  # define CSR_IE		CSR_SIE
>  # define CSR_TVEC	CSR_STVEC
> +# define CSR_ENVCFG	CSR_SENVCFG
>  # define CSR_SCRATCH	CSR_SSCRATCH
>  # define CSR_EPC	CSR_SEPC
>  # define CSR_CAUSE	CSR_SCAUSE
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 89920f84d0a3..c5b13f7dd482 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -950,7 +950,7 @@ arch_initcall(check_unaligned_access_all_cpus);
>  void riscv_user_isa_enable(void)
>  {
>  	if (riscv_cpu_has_extension_unlikely(smp_processor_id(), RISCV_ISA_EXT_ZICBOZ))
> -		csr_set(CSR_SENVCFG, ENVCFG_CBZE);
> +		csr_set(CSR_ENVCFG, ENVCFG_CBZE);
>  }
>  
>  #ifdef CONFIG_RISCV_ALTERNATIVE
> -- 
> 2.43.0
>

Deepak has also posted this change[1], but his is missing the Fixes tag.

[1] https://lore.kernel.org/all/20240125024611.1321941-1-debug@rivosinc.com/

Thanks,
drew

Re: [PATCH -fixes 2/2] riscv: Save/restore envcfg CSR during CPU suspend

[Andrew Jones]

On Sun, Feb 11, 2024 at 06:26:15PM -0800, Samuel Holland wrote:
> The value of the [ms]envcfg CSR is lost when entering a nonretentive
> idle state, so the CSR must be rewritten when resuming the CPU.
> 
> Because the [ms]envcfg CSR is part of the base RISC-V privileged ISA
> specification, it cannot be detected from the ISA string. However, most
> existing hardware is too old to implement this CSR. As a result, it must
> be probed at runtime.
> 
> Extend the logic for the Zicsr ISA extension to probe for the presence
> of specific CSRs. Since the CSR number is encoded as an immediate value
> within the csrr instruction, a switch case is necessary for any CSR that
> must be probed this way. Use the exception table to handle the illegal
> instruction exception raised when the CSR is not implemented.
> 
> Cc: stable@kernel.org
> Fixes: 43c16d51a19b ("RISC-V: Enable cbo.zero in usermode")
> Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
> ---
> 
>  arch/riscv/include/asm/csr.h     | 23 +++++++++++++++++++++++
>  arch/riscv/include/asm/suspend.h |  1 +
>  arch/riscv/kernel/cpufeature.c   | 23 +++++++++++++++++++++++
>  arch/riscv/kernel/suspend.c      |  2 ++
>  4 files changed, 49 insertions(+)
> 
> diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
> index 2468c55933cd..daff95feb817 100644
> --- a/arch/riscv/include/asm/csr.h
> +++ b/arch/riscv/include/asm/csr.h
> @@ -542,6 +542,29 @@
>  			      : "memory");			\
>  })
>  
> +#define ALT_CSR_READ(csr)					\
> +({								\
> +	unsigned long __v;					\
> +	__asm__ __volatile__ (					\
> +		ALTERNATIVE("li %[v], 0", "csrr %[v], %[r]", 0,	\
> +			    csr << 16 | RISCV_ISA_EXT_ZICSR, 1)	\
> +		: [v] "=r" (__v)				\
> +		: [r] "i" (csr)					\
> +		: "memory");					\
> +	__v;							\
> +})
> +
> +#define ALT_CSR_WRITE(csr, val)					\
> +({								\
> +	unsigned long __v = (unsigned long)(val);		\
> +	__asm__ __volatile__ (					\
> +		ALTERNATIVE("nop", "csrw %[r], %[v]", 0,	\
> +			    csr << 16 | RISCV_ISA_EXT_ZICSR, 1)	\
> +		: : [r] "i" (csr), [v] "rK" (__v)		\
> +		: "memory");					\
> +	__v;							\
> +})
> +
>  #endif /* __ASSEMBLY__ */
>  
>  #endif /* _ASM_RISCV_CSR_H */
> diff --git a/arch/riscv/include/asm/suspend.h b/arch/riscv/include/asm/suspend.h
> index 02f87867389a..491296a335d0 100644
> --- a/arch/riscv/include/asm/suspend.h
> +++ b/arch/riscv/include/asm/suspend.h
> @@ -14,6 +14,7 @@ struct suspend_context {
>  	struct pt_regs regs;
>  	/* Saved and restored by high-level functions */
>  	unsigned long scratch;
> +	unsigned long envcfg;
>  	unsigned long tvec;
>  	unsigned long ie;
>  #ifdef CONFIG_MMU
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index c5b13f7dd482..934090270ae5 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -954,6 +954,27 @@ void riscv_user_isa_enable(void)
>  }
>  
>  #ifdef CONFIG_RISCV_ALTERNATIVE
> +static bool riscv_cpufeature_probe_csr(u16 csr)
> +{
> +	bool ret = false;
> +
> +	switch (csr) {
> +#define PROBE_CSR_CASE(_csr)			\
> +	case _csr:				\
> +		asm("1:	csrr zero, %[csr]\n"	\
> +		    "	li %[r], 1\n"		\
> +		    "2:\n"			\
> +		    _ASM_EXTABLE(1b, 2b)	\
> +			: [r] "+r" (ret)	\
> +			: [csr] "i" (_csr));	\
> +		break
> +	PROBE_CSR_CASE(CSR_ENVCFG);
> +#undef PROBE_CSR_CASE
> +	}
> +
> +	return ret;
> +}
> +
>  /*
>   * Alternative patch sites consider 48 bits when determining when to patch
>   * the old instruction sequence with the new. These bits are broken into a
> @@ -974,6 +995,8 @@ static bool riscv_cpufeature_patch_check(u16 id, u16 value)
>  		return true;
>  
>  	switch (id) {
> +	case RISCV_ISA_EXT_ZICSR:
> +		return riscv_cpufeature_probe_csr(value);
>  	case RISCV_ISA_EXT_ZICBOZ:
>  		/*
>  		 * Zicboz alternative applications provide the maximum
> diff --git a/arch/riscv/kernel/suspend.c b/arch/riscv/kernel/suspend.c
> index 239509367e42..fe544f12a5c5 100644
> --- a/arch/riscv/kernel/suspend.c
> +++ b/arch/riscv/kernel/suspend.c
> @@ -15,6 +15,7 @@
>  void suspend_save_csrs(struct suspend_context *context)
>  {
>  	context->scratch = csr_read(CSR_SCRATCH);
> +	context->envcfg = ALT_CSR_READ(CSR_ENVCFG);
>  	context->tvec = csr_read(CSR_TVEC);
>  	context->ie = csr_read(CSR_IE);
>  
> @@ -36,6 +37,7 @@ void suspend_save_csrs(struct suspend_context *context)
>  void suspend_restore_csrs(struct suspend_context *context)
>  {
>  	csr_write(CSR_SCRATCH, context->scratch);
> +	ALT_CSR_WRITE(CSR_ENVCFG, context->envcfg);
>  	csr_write(CSR_TVEC, context->tvec);
>  	csr_write(CSR_IE, context->ie);
>  
> -- 
> 2.43.0
>

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

Re: [PATCH -fixes 1/2] riscv: Fix enabling cbo.zero when running in M-mode

[Andrew Jones]

On Sun, Feb 11, 2024 at 06:26:14PM -0800, Samuel Holland wrote:
> When the kernel is running in M-mode, the CBZE bit must be set in the
> menvcfg CSR, not in senvcfg.
> 
> Cc: stable@kernel.org
> Fixes: 43c16d51a19b ("RISC-V: Enable cbo.zero in usermode")
> Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
> ---
> 
>  arch/riscv/include/asm/csr.h   | 2 ++
>  arch/riscv/kernel/cpufeature.c | 2 +-
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
> index 510014051f5d..2468c55933cd 100644
> --- a/arch/riscv/include/asm/csr.h
> +++ b/arch/riscv/include/asm/csr.h
> @@ -424,6 +424,7 @@
>  # define CSR_STATUS	CSR_MSTATUS
>  # define CSR_IE		CSR_MIE
>  # define CSR_TVEC	CSR_MTVEC
> +# define CSR_ENVCFG	CSR_MENVCFG
>  # define CSR_SCRATCH	CSR_MSCRATCH
>  # define CSR_EPC	CSR_MEPC
>  # define CSR_CAUSE	CSR_MCAUSE
> @@ -448,6 +449,7 @@
>  # define CSR_STATUS	CSR_SSTATUS
>  # define CSR_IE		CSR_SIE
>  # define CSR_TVEC	CSR_STVEC
> +# define CSR_ENVCFG	CSR_SENVCFG
>  # define CSR_SCRATCH	CSR_SSCRATCH
>  # define CSR_EPC	CSR_SEPC
>  # define CSR_CAUSE	CSR_SCAUSE
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 89920f84d0a3..c5b13f7dd482 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -950,7 +950,7 @@ arch_initcall(check_unaligned_access_all_cpus);
>  void riscv_user_isa_enable(void)
>  {
>  	if (riscv_cpu_has_extension_unlikely(smp_processor_id(), RISCV_ISA_EXT_ZICBOZ))
> -		csr_set(CSR_SENVCFG, ENVCFG_CBZE);
> +		csr_set(CSR_ENVCFG, ENVCFG_CBZE);
>  }
>  
>  #ifdef CONFIG_RISCV_ALTERNATIVE
> -- 
> 2.43.0
>

In case we take this one instead of Deepak's

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

Re: [PATCH -fixes 2/2] riscv: Save/restore envcfg CSR during CPU suspend

[Stefan O'Rear]

On Sun, Feb 11, 2024, at 9:26 PM, Samuel Holland wrote:
> The value of the [ms]envcfg CSR is lost when entering a nonretentive
> idle state, so the CSR must be rewritten when resuming the CPU.
>
> Because the [ms]envcfg CSR is part of the base RISC-V privileged ISA
> specification, it cannot be detected from the ISA string. However, most
> existing hardware is too old to implement this CSR. As a result, it must
> be probed at runtime.
>
> Extend the logic for the Zicsr ISA extension to probe for the presence
> of specific CSRs. Since the CSR number is encoded as an immediate value
> within the csrr instruction, a switch case is necessary for any CSR that
> must be probed this way. Use the exception table to handle the illegal
> instruction exception raised when the CSR is not implemented.

We support non-conforming extensions, so we can't assume that if an
implementation does not provide the Ss1p12 extension which defines senvcfg,
the corresponding CSR number will not be used for other purposes.

-s

> Cc: stable@kernel.org
> Fixes: 43c16d51a19b ("RISC-V: Enable cbo.zero in usermode")
> Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
> ---
>
>  arch/riscv/include/asm/csr.h     | 23 +++++++++++++++++++++++
>  arch/riscv/include/asm/suspend.h |  1 +
>  arch/riscv/kernel/cpufeature.c   | 23 +++++++++++++++++++++++
>  arch/riscv/kernel/suspend.c      |  2 ++
>  4 files changed, 49 insertions(+)
>
> diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
> index 2468c55933cd..daff95feb817 100644
> --- a/arch/riscv/include/asm/csr.h
> +++ b/arch/riscv/include/asm/csr.h
> @@ -542,6 +542,29 @@
>  			      : "memory");			\
>  })
> 
> +#define ALT_CSR_READ(csr)					\
> +({								\
> +	unsigned long __v;					\
> +	__asm__ __volatile__ (					\
> +		ALTERNATIVE("li %[v], 0", "csrr %[v], %[r]", 0,	\
> +			    csr << 16 | RISCV_ISA_EXT_ZICSR, 1)	\
> +		: [v] "=r" (__v)				\
> +		: [r] "i" (csr)					\
> +		: "memory");					\
> +	__v;							\
> +})
> +
> +#define ALT_CSR_WRITE(csr, val)					\
> +({								\
> +	unsigned long __v = (unsigned long)(val);		\
> +	__asm__ __volatile__ (					\
> +		ALTERNATIVE("nop", "csrw %[r], %[v]", 0,	\
> +			    csr << 16 | RISCV_ISA_EXT_ZICSR, 1)	\
> +		: : [r] "i" (csr), [v] "rK" (__v)		\
> +		: "memory");					\
> +	__v;							\
> +})
> +
>  #endif /* __ASSEMBLY__ */
> 
>  #endif /* _ASM_RISCV_CSR_H */
> diff --git a/arch/riscv/include/asm/suspend.h b/arch/riscv/include/asm/suspend.h
> index 02f87867389a..491296a335d0 100644
> --- a/arch/riscv/include/asm/suspend.h
> +++ b/arch/riscv/include/asm/suspend.h
> @@ -14,6 +14,7 @@ struct suspend_context {
>  	struct pt_regs regs;
>  	/* Saved and restored by high-level functions */
>  	unsigned long scratch;
> +	unsigned long envcfg;
>  	unsigned long tvec;
>  	unsigned long ie;
>  #ifdef CONFIG_MMU
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index c5b13f7dd482..934090270ae5 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -954,6 +954,27 @@ void riscv_user_isa_enable(void)
>  }
> 
>  #ifdef CONFIG_RISCV_ALTERNATIVE
> +static bool riscv_cpufeature_probe_csr(u16 csr)
> +{
> +	bool ret = false;
> +
> +	switch (csr) {
> +#define PROBE_CSR_CASE(_csr)			\
> +	case _csr:				\
> +		asm("1:	csrr zero, %[csr]\n"	\
> +		    "	li %[r], 1\n"		\
> +		    "2:\n"			\
> +		    _ASM_EXTABLE(1b, 2b)	\
> +			: [r] "+r" (ret)	\
> +			: [csr] "i" (_csr));	\
> +		break
> +	PROBE_CSR_CASE(CSR_ENVCFG);
> +#undef PROBE_CSR_CASE
> +	}
> +
> +	return ret;
> +}
> +
>  /*
>   * Alternative patch sites consider 48 bits when determining when to patch
>   * the old instruction sequence with the new. These bits are broken into a
> @@ -974,6 +995,8 @@ static bool riscv_cpufeature_patch_check(u16 id, u16 value)
>  		return true;
> 
>  	switch (id) {
> +	case RISCV_ISA_EXT_ZICSR:
> +		return riscv_cpufeature_probe_csr(value);
>  	case RISCV_ISA_EXT_ZICBOZ:
>  		/*
>  		 * Zicboz alternative applications provide the maximum
> diff --git a/arch/riscv/kernel/suspend.c b/arch/riscv/kernel/suspend.c
> index 239509367e42..fe544f12a5c5 100644
> --- a/arch/riscv/kernel/suspend.c
> +++ b/arch/riscv/kernel/suspend.c
> @@ -15,6 +15,7 @@
>  void suspend_save_csrs(struct suspend_context *context)
>  {
>  	context->scratch = csr_read(CSR_SCRATCH);
> +	context->envcfg = ALT_CSR_READ(CSR_ENVCFG);
>  	context->tvec = csr_read(CSR_TVEC);
>  	context->ie = csr_read(CSR_IE);
> 
> @@ -36,6 +37,7 @@ void suspend_save_csrs(struct suspend_context *context)
>  void suspend_restore_csrs(struct suspend_context *context)
>  {
>  	csr_write(CSR_SCRATCH, context->scratch);
> +	ALT_CSR_WRITE(CSR_ENVCFG, context->envcfg);
>  	csr_write(CSR_TVEC, context->tvec);
>  	csr_write(CSR_IE, context->ie);
> 
> -- 
> 2.43.0
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH -fixes 2/2] riscv: Save/restore envcfg CSR during CPU suspend

[Samuel Holland]

Hi Stefan,

On 2024-02-12 7:21 AM, Stefan O'Rear wrote:
> On Sun, Feb 11, 2024, at 9:26 PM, Samuel Holland wrote:
>> The value of the [ms]envcfg CSR is lost when entering a nonretentive
>> idle state, so the CSR must be rewritten when resuming the CPU.
>>
>> Because the [ms]envcfg CSR is part of the base RISC-V privileged ISA
>> specification, it cannot be detected from the ISA string. However, most
>> existing hardware is too old to implement this CSR. As a result, it must
>> be probed at runtime.
>>
>> Extend the logic for the Zicsr ISA extension to probe for the presence
>> of specific CSRs. Since the CSR number is encoded as an immediate value
>> within the csrr instruction, a switch case is necessary for any CSR that
>> must be probed this way. Use the exception table to handle the illegal
>> instruction exception raised when the CSR is not implemented.
> 
> We support non-conforming extensions, so we can't assume that if an
> implementation does not provide the Ss1p12 extension which defines senvcfg,
> the corresponding CSR number will not be used for other purposes.

Yes, you're right, I'll do this for v2. Though this does prevent us from
supporting hardware which implements senvcfg but not all of the rest of Ss1p12
(e.g. Ss1p11 + Zicboz).

Regards,
Samuel

Re: [PATCH -fixes 1/2] riscv: Fix enabling cbo.zero when running in M-mode

[patchwork-bot+linux-riscv]

Hello:

This series was applied to riscv/linux.git (fixes)
by Palmer Dabbelt <palmer@rivosinc.com>:

On Sun, 11 Feb 2024 18:26:14 -0800 you wrote:
> When the kernel is running in M-mode, the CBZE bit must be set in the
> menvcfg CSR, not in senvcfg.
> 
> Cc: stable@kernel.org
> Fixes: 43c16d51a19b ("RISC-V: Enable cbo.zero in usermode")
> Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
> 
> [...]

Here is the summary with links:
  - [-fixes,1/2] riscv: Fix enabling cbo.zero when running in M-mode
    https://git.kernel.org/riscv/c/3fb3f7164edc
  - [-fixes,2/2] riscv: Save/restore envcfg CSR during CPU suspend
    (no matching commit)

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html