* [PATCH v2 1/3] riscv: Enable cbo.zero only when all harts support Zicboz
2024-06-13 17:14 [PATCH v2 0/3] riscv: Per-thread envcfg CSR support Samuel Holland
@ 2024-06-13 17:14 ` Samuel Holland
2024-06-14 10:31 ` Conor Dooley
2024-06-13 17:14 ` [PATCH v2 2/3] riscv: Add support for per-thread envcfg CSR values Samuel Holland
` (2 subsequent siblings)
3 siblings, 1 reply; 7+ messages in thread
From: Samuel Holland @ 2024-06-13 17:14 UTC (permalink / raw)
To: linux-riscv, Palmer Dabbelt
Cc: Andrew Jones, Conor Dooley, linux-kernel, Deepak Gupta,
Samuel Holland
Currently, we enable cbo.zero for usermode on each hart that supports
the Zicboz extension. This means that the [ms]envcfg CSR value may
differ between harts. Other features, such as pointer masking and CFI,
require setting [ms]envcfg bits on a per-thread basis. The combination
of these two adds quite some complexity and overhead to context
switching, as we would need to maintain two separate masks for the
per-hart and per-thread bits. Andrew Jones, who originally added Zicboz
support, writes[1][2]:
I've approached Zicboz the same way I would approach all
extensions, which is to be per-hart. I'm not currently aware of
a platform that is / will be composed of harts where some have
Zicboz and others don't, but there's nothing stopping a platform
like that from being built.
So, how about we add code that confirms Zicboz is on all harts.
If any hart does not have it, then we complain loudly and disable
it on all the other harts. If it was just a hardware description
bug, then it'll get fixed. If there's actually a platform which
doesn't have Zicboz on all harts, then, when the issue is reported,
we can decide to not support it, support it with defconfig, or
support it under a Kconfig guard which must be enabled by the user.
Let's follow his suggested solution and require the extension to be
available on all harts, so the envcfg CSR value does not need to change
when a thread migrates between harts. Since we are doing this for all
extensions with fields in envcfg, the CSR itself only needs to be saved/
restored when it is present on all harts.
This should not be a regression as no known hardware has asymmetric
Zicboz support, but if anyone reports seeing the warning, we will
re-evaluate our solution.
Link: https://lore.kernel.org/linux-riscv/20240322-168f191eeb8479b2ea169a5e@orel/ [1]
Link: https://lore.kernel.org/linux-riscv/20240323-28943722feb57a41fb0ff488@orel/ [2]
Reviewed-by: Deepak Gupta <debug@rivosinc.com>
Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
---
(no changes since v1)
arch/riscv/kernel/cpufeature.c | 7 ++++++-
arch/riscv/kernel/suspend.c | 4 ++--
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 1d6e4fda00f8..4347c9f91dc3 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -27,6 +27,8 @@
#define NUM_ALPHA_EXTS ('z' - 'a' + 1)
+static bool any_cpu_has_zicboz;
+
unsigned long elf_hwcap __read_mostly;
/* Host ISA bitmap */
@@ -92,6 +94,7 @@ static bool riscv_isa_extension_check(int id)
pr_err("Zicboz disabled as cboz-block-size present, but is not a power-of-2\n");
return false;
}
+ any_cpu_has_zicboz = true;
return true;
case RISCV_ISA_EXT_INVALID:
return false;
@@ -768,8 +771,10 @@ unsigned long riscv_get_elf_hwcap(void)
void riscv_user_isa_enable(void)
{
- if (riscv_cpu_has_extension_unlikely(smp_processor_id(), RISCV_ISA_EXT_ZICBOZ))
+ if (riscv_has_extension_unlikely(RISCV_ISA_EXT_ZICBOZ))
csr_set(CSR_ENVCFG, ENVCFG_CBZE);
+ else if (any_cpu_has_zicboz)
+ pr_warn_once("Zicboz disabled as it is unavailable on some harts\n");
}
#ifdef CONFIG_RISCV_ALTERNATIVE
diff --git a/arch/riscv/kernel/suspend.c b/arch/riscv/kernel/suspend.c
index c8cec0cc5833..9a8a0dc035b2 100644
--- a/arch/riscv/kernel/suspend.c
+++ b/arch/riscv/kernel/suspend.c
@@ -14,7 +14,7 @@
void suspend_save_csrs(struct suspend_context *context)
{
- if (riscv_cpu_has_extension_unlikely(smp_processor_id(), RISCV_ISA_EXT_XLINUXENVCFG))
+ if (riscv_has_extension_unlikely(RISCV_ISA_EXT_XLINUXENVCFG))
context->envcfg = csr_read(CSR_ENVCFG);
context->tvec = csr_read(CSR_TVEC);
context->ie = csr_read(CSR_IE);
@@ -37,7 +37,7 @@ void suspend_save_csrs(struct suspend_context *context)
void suspend_restore_csrs(struct suspend_context *context)
{
csr_write(CSR_SCRATCH, 0);
- if (riscv_cpu_has_extension_unlikely(smp_processor_id(), RISCV_ISA_EXT_XLINUXENVCFG))
+ if (riscv_has_extension_unlikely(RISCV_ISA_EXT_XLINUXENVCFG))
csr_write(CSR_ENVCFG, context->envcfg);
csr_write(CSR_TVEC, context->tvec);
csr_write(CSR_IE, context->ie);
--
2.44.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2 1/3] riscv: Enable cbo.zero only when all harts support Zicboz
2024-06-13 17:14 ` [PATCH v2 1/3] riscv: Enable cbo.zero only when all harts support Zicboz Samuel Holland
@ 2024-06-14 10:31 ` Conor Dooley
0 siblings, 0 replies; 7+ messages in thread
From: Conor Dooley @ 2024-06-14 10:31 UTC (permalink / raw)
To: Samuel Holland
Cc: linux-riscv, Palmer Dabbelt, Andrew Jones, Conor Dooley,
linux-kernel, Deepak Gupta
[-- Attachment #1.1: Type: text/plain, Size: 2265 bytes --]
On Thu, Jun 13, 2024 at 10:14:39AM -0700, Samuel Holland wrote:
> Currently, we enable cbo.zero for usermode on each hart that supports
> the Zicboz extension. This means that the [ms]envcfg CSR value may
> differ between harts. Other features, such as pointer masking and CFI,
> require setting [ms]envcfg bits on a per-thread basis. The combination
> of these two adds quite some complexity and overhead to context
> switching, as we would need to maintain two separate masks for the
> per-hart and per-thread bits. Andrew Jones, who originally added Zicboz
> support, writes[1][2]:
>
> I've approached Zicboz the same way I would approach all
> extensions, which is to be per-hart. I'm not currently aware of
> a platform that is / will be composed of harts where some have
> Zicboz and others don't, but there's nothing stopping a platform
> like that from being built.
>
> So, how about we add code that confirms Zicboz is on all harts.
> If any hart does not have it, then we complain loudly and disable
> it on all the other harts. If it was just a hardware description
> bug, then it'll get fixed. If there's actually a platform which
> doesn't have Zicboz on all harts, then, when the issue is reported,
> we can decide to not support it, support it with defconfig, or
> support it under a Kconfig guard which must be enabled by the user.
>
> Let's follow his suggested solution and require the extension to be
> available on all harts, so the envcfg CSR value does not need to change
> when a thread migrates between harts. Since we are doing this for all
> extensions with fields in envcfg, the CSR itself only needs to be saved/
> restored when it is present on all harts.
>
> This should not be a regression as no known hardware has asymmetric
> Zicboz support, but if anyone reports seeing the warning, we will
> re-evaluate our solution.
>
> Link: https://lore.kernel.org/linux-riscv/20240322-168f191eeb8479b2ea169a5e@orel/ [1]
> Link: https://lore.kernel.org/linux-riscv/20240323-28943722feb57a41fb0ff488@orel/ [2]
> Reviewed-by: Deepak Gupta <debug@rivosinc.com>
> Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 161 bytes --]
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 2/3] riscv: Add support for per-thread envcfg CSR values
2024-06-13 17:14 [PATCH v2 0/3] riscv: Per-thread envcfg CSR support Samuel Holland
2024-06-13 17:14 ` [PATCH v2 1/3] riscv: Enable cbo.zero only when all harts support Zicboz Samuel Holland
@ 2024-06-13 17:14 ` Samuel Holland
2024-06-13 17:14 ` [PATCH v2 3/3] riscv: Call riscv_user_isa_enable() only on the boot hart Samuel Holland
2024-06-21 12:54 ` [PATCH v2 0/3] riscv: Per-thread envcfg CSR support Andrew Jones
3 siblings, 0 replies; 7+ messages in thread
From: Samuel Holland @ 2024-06-13 17:14 UTC (permalink / raw)
To: linux-riscv, Palmer Dabbelt
Cc: Andrew Jones, Conor Dooley, linux-kernel, Deepak Gupta,
Samuel Holland
Some bits in the [ms]envcfg CSR, such as the CFI state and pointer
masking mode, need to be controlled on a per-thread basis. Support this
by keeping a copy of the CSR value in struct thread_struct and writing
it during context switches. It is safe to discard the old CSR value
during the context switch because the CSR is modified only by software,
so the CSR will remain in sync with the copy in thread_struct.
Use ALTERNATIVE directly instead of riscv_has_extension_unlikely() to
minimize branchiness in the context switching code.
Since thread_struct is copied during fork(), setting the value for the
init task sets the default value for all other threads.
Reviewed-by: Deepak Gupta <debug@rivosinc.com>
Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
---
(no changes since v1)
arch/riscv/include/asm/processor.h | 1 +
arch/riscv/include/asm/switch_to.h | 8 ++++++++
arch/riscv/kernel/cpufeature.c | 2 +-
3 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
index 68c3432dc6ea..0838922bd1c8 100644
--- a/arch/riscv/include/asm/processor.h
+++ b/arch/riscv/include/asm/processor.h
@@ -118,6 +118,7 @@ struct thread_struct {
unsigned long s[12]; /* s[0]: frame pointer */
struct __riscv_d_ext_state fstate;
unsigned long bad_cause;
+ unsigned long envcfg;
u32 riscv_v_flags;
u32 vstate_ctrl;
struct __riscv_v_ext_state vstate;
diff --git a/arch/riscv/include/asm/switch_to.h b/arch/riscv/include/asm/switch_to.h
index 7594df37cc9f..9685cd85e57c 100644
--- a/arch/riscv/include/asm/switch_to.h
+++ b/arch/riscv/include/asm/switch_to.h
@@ -70,6 +70,13 @@ static __always_inline bool has_fpu(void) { return false; }
#define __switch_to_fpu(__prev, __next) do { } while (0)
#endif
+static inline void __switch_to_envcfg(struct task_struct *next)
+{
+ asm volatile (ALTERNATIVE("nop", "csrw " __stringify(CSR_ENVCFG) ", %0",
+ 0, RISCV_ISA_EXT_XLINUXENVCFG, 1)
+ :: "r" (next->thread.envcfg) : "memory");
+}
+
extern struct task_struct *__switch_to(struct task_struct *,
struct task_struct *);
@@ -103,6 +110,7 @@ do { \
__switch_to_vector(__prev, __next); \
if (switch_to_should_flush_icache(__next)) \
local_flush_icache_all(); \
+ __switch_to_envcfg(__next); \
((last) = __switch_to(__prev, __next)); \
} while (0)
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 4347c9f91dc3..b5b8773c57e8 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -772,7 +772,7 @@ unsigned long riscv_get_elf_hwcap(void)
void riscv_user_isa_enable(void)
{
if (riscv_has_extension_unlikely(RISCV_ISA_EXT_ZICBOZ))
- csr_set(CSR_ENVCFG, ENVCFG_CBZE);
+ current->thread.envcfg |= ENVCFG_CBZE;
else if (any_cpu_has_zicboz)
pr_warn_once("Zicboz disabled as it is unavailable on some harts\n");
}
--
2.44.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v2 3/3] riscv: Call riscv_user_isa_enable() only on the boot hart
2024-06-13 17:14 [PATCH v2 0/3] riscv: Per-thread envcfg CSR support Samuel Holland
2024-06-13 17:14 ` [PATCH v2 1/3] riscv: Enable cbo.zero only when all harts support Zicboz Samuel Holland
2024-06-13 17:14 ` [PATCH v2 2/3] riscv: Add support for per-thread envcfg CSR values Samuel Holland
@ 2024-06-13 17:14 ` Samuel Holland
2024-06-14 10:32 ` Conor Dooley
2024-06-21 12:54 ` [PATCH v2 0/3] riscv: Per-thread envcfg CSR support Andrew Jones
3 siblings, 1 reply; 7+ messages in thread
From: Samuel Holland @ 2024-06-13 17:14 UTC (permalink / raw)
To: linux-riscv, Palmer Dabbelt
Cc: Andrew Jones, Conor Dooley, linux-kernel, Deepak Gupta,
Samuel Holland
Now that the [ms]envcfg CSR value is maintained per thread, not per
hart, riscv_user_isa_enable() only needs to be called once during boot,
to set the value for the init task. This also allows it to be marked as
__init. riscv_isa_extension_check() sets any_cpu_has_zicboz, so it also
needs to be marked __init; it could have had this annotation already.
Reviewed-by: Deepak Gupta <debug@rivosinc.com>
Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
---
Changes in v2:
- Rebase on riscv/linux.git for-next
arch/riscv/include/asm/cpufeature.h | 2 +-
arch/riscv/kernel/cpufeature.c | 8 ++++----
arch/riscv/kernel/smpboot.c | 2 --
3 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h
index 347805446151..4bf7b7ebf6b3 100644
--- a/arch/riscv/include/asm/cpufeature.h
+++ b/arch/riscv/include/asm/cpufeature.h
@@ -31,7 +31,7 @@ DECLARE_PER_CPU(struct riscv_cpuinfo, riscv_cpuinfo);
/* Per-cpu ISA extensions. */
extern struct riscv_isainfo hart_isa[NR_CPUS];
-void riscv_user_isa_enable(void);
+void __init riscv_user_isa_enable(void);
#if defined(CONFIG_RISCV_MISALIGNED)
bool check_unaligned_access_emulated_all_cpus(void);
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index b5b8773c57e8..d3e3a865b874 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -27,7 +27,7 @@
#define NUM_ALPHA_EXTS ('z' - 'a' + 1)
-static bool any_cpu_has_zicboz;
+static bool any_cpu_has_zicboz __initdata;
unsigned long elf_hwcap __read_mostly;
@@ -74,7 +74,7 @@ bool __riscv_isa_extension_available(const unsigned long *isa_bitmap, unsigned i
}
EXPORT_SYMBOL_GPL(__riscv_isa_extension_available);
-static bool riscv_isa_extension_check(int id)
+static bool __init riscv_isa_extension_check(int id)
{
switch (id) {
case RISCV_ISA_EXT_ZICBOM:
@@ -769,12 +769,12 @@ unsigned long riscv_get_elf_hwcap(void)
return hwcap;
}
-void riscv_user_isa_enable(void)
+void __init riscv_user_isa_enable(void)
{
if (riscv_has_extension_unlikely(RISCV_ISA_EXT_ZICBOZ))
current->thread.envcfg |= ENVCFG_CBZE;
else if (any_cpu_has_zicboz)
- pr_warn_once("Zicboz disabled as it is unavailable on some harts\n");
+ pr_warn("Zicboz disabled as it is unavailable on some harts\n");
}
#ifdef CONFIG_RISCV_ALTERNATIVE
diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index 19baf0d574d3..0646f79e0a02 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -235,8 +235,6 @@ asmlinkage __visible void smp_callin(void)
numa_add_cpu(curr_cpuid);
set_cpu_online(curr_cpuid, true);
- riscv_user_isa_enable();
-
/*
* Remote cache and TLB flushes are ignored while the CPU is offline,
* so flush them both right now just in case.
--
2.44.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2 3/3] riscv: Call riscv_user_isa_enable() only on the boot hart
2024-06-13 17:14 ` [PATCH v2 3/3] riscv: Call riscv_user_isa_enable() only on the boot hart Samuel Holland
@ 2024-06-14 10:32 ` Conor Dooley
0 siblings, 0 replies; 7+ messages in thread
From: Conor Dooley @ 2024-06-14 10:32 UTC (permalink / raw)
To: Samuel Holland
Cc: linux-riscv, Palmer Dabbelt, Andrew Jones, Conor Dooley,
linux-kernel, Deepak Gupta
[-- Attachment #1.1: Type: text/plain, Size: 609 bytes --]
On Thu, Jun 13, 2024 at 10:14:41AM -0700, Samuel Holland wrote:
> Now that the [ms]envcfg CSR value is maintained per thread, not per
> hart, riscv_user_isa_enable() only needs to be called once during boot,
> to set the value for the init task. This also allows it to be marked as
> __init. riscv_isa_extension_check() sets any_cpu_has_zicboz, so it also
> needs to be marked __init; it could have had this annotation already.
>
> Reviewed-by: Deepak Gupta <debug@rivosinc.com>
> Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 161 bytes --]
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 0/3] riscv: Per-thread envcfg CSR support
2024-06-13 17:14 [PATCH v2 0/3] riscv: Per-thread envcfg CSR support Samuel Holland
` (2 preceding siblings ...)
2024-06-13 17:14 ` [PATCH v2 3/3] riscv: Call riscv_user_isa_enable() only on the boot hart Samuel Holland
@ 2024-06-21 12:54 ` Andrew Jones
3 siblings, 0 replies; 7+ messages in thread
From: Andrew Jones @ 2024-06-21 12:54 UTC (permalink / raw)
To: Samuel Holland
Cc: linux-riscv, Palmer Dabbelt, Conor Dooley, linux-kernel,
Deepak Gupta
On Thu, Jun 13, 2024 at 10:14:38AM GMT, Samuel Holland wrote:
> This series (or equivalent) is a prerequisite for both user-mode pointer
> masking and CFI support, as those are per-thread features are controlled
> by fields in the envcfg CSR. These patches are based on v1 of the
> pointer masking series[1], with significant input from both Deepak and
> Andrew. By sending this as a separate series, hopefully we can converge
> on a single implementation of this functionality.
>
> [1]: https://lore.kernel.org/linux-riscv/20240319215915.832127-6-samuel.holland@sifive.com/
>
> Changes in v2:
> - Rebase on riscv/linux.git for-next
>
> Samuel Holland (3):
> riscv: Enable cbo.zero only when all harts support Zicboz
> riscv: Add support for per-thread envcfg CSR values
> riscv: Call riscv_user_isa_enable() only on the boot hart
>
For the series,
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Thanks,
drew
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 7+ messages in thread