* [PATCH -fixes v3 1/2] riscv: Fix enabling cbo.zero when running in M-mode [not found] <20240214090206.195754-1-samuel.holland@sifive.com> @ 2024-02-14 9:01 ` Samuel Holland 2024-02-14 9:28 ` Andrew Jones 2024-02-14 9:01 ` [PATCH -fixes v3 2/2] riscv: Save/restore envcfg CSR during CPU suspend Samuel Holland 1 sibling, 1 reply; 6+ messages in thread From: Samuel Holland @ 2024-02-14 9:01 UTC (permalink / raw) To: Palmer Dabbelt Cc: Andrew Jones, linux-kernel, Conor Dooley, linux-riscv, Stefan O'Rear, Samuel Holland, stable When the kernel is running in M-mode, the CBZE bit must be set in the menvcfg CSR, not in senvcfg. Cc: <stable@vger.kernel.org> Fixes: 43c16d51a19b ("RISC-V: Enable cbo.zero in usermode") Reviewed-by: Andrew Jones <ajones@ventanamicro.com> Signed-off-by: Samuel Holland <samuel.holland@sifive.com> --- (no changes since v1) 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH -fixes v3 1/2] riscv: Fix enabling cbo.zero when running in M-mode 2024-02-14 9:01 ` [PATCH -fixes v3 1/2] riscv: Fix enabling cbo.zero when running in M-mode Samuel Holland @ 2024-02-14 9:28 ` Andrew Jones 2024-02-27 19:48 ` Alexandre Ghiti 0 siblings, 1 reply; 6+ messages in thread From: Andrew Jones @ 2024-02-14 9:28 UTC (permalink / raw) To: Samuel Holland Cc: Palmer Dabbelt, linux-kernel, Conor Dooley, linux-riscv, Stefan O'Rear, stable On Wed, Feb 14, 2024 at 01:01:56AM -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@vger.kernel.org> > Fixes: 43c16d51a19b ("RISC-V: Enable cbo.zero in usermode") > Reviewed-by: Andrew Jones <ajones@ventanamicro.com> > Signed-off-by: Samuel Holland <samuel.holland@sifive.com> > --- > > (no changes since v1) > > 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 > After our back and forth on how we determine the existence of the *envcfg CSRs, I wonder if we shouldn't put a comment above this riscv_user_isa_enable() function capturing the [current] decision. Something like /* * While the [ms]envcfg CSRs weren't defined until priv spec 1.12, * they're assumed to be present when an extension is present which * specifies [ms]envcfg bit(s). Hence, we don't do any additional * priv spec version checks or CSR probes here. */ Thanks, drew ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH -fixes v3 1/2] riscv: Fix enabling cbo.zero when running in M-mode 2024-02-14 9:28 ` Andrew Jones @ 2024-02-27 19:48 ` Alexandre Ghiti 2024-02-27 20:01 ` Samuel Holland 0 siblings, 1 reply; 6+ messages in thread From: Alexandre Ghiti @ 2024-02-27 19:48 UTC (permalink / raw) To: Andrew Jones, Samuel Holland Cc: Palmer Dabbelt, linux-kernel, Conor Dooley, linux-riscv, Stefan O'Rear, stable Hi Samuel, On 14/02/2024 10:28, Andrew Jones wrote: > On Wed, Feb 14, 2024 at 01:01:56AM -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@vger.kernel.org> >> Fixes: 43c16d51a19b ("RISC-V: Enable cbo.zero in usermode") >> Reviewed-by: Andrew Jones <ajones@ventanamicro.com> >> Signed-off-by: Samuel Holland <samuel.holland@sifive.com> >> --- >> >> (no changes since v1) >> >> 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 >> > After our back and forth on how we determine the existence of the *envcfg > CSRs, I wonder if we shouldn't put a comment above this > riscv_user_isa_enable() function capturing the [current] decision. > > Something like > > /* > * While the [ms]envcfg CSRs weren't defined until priv spec 1.12, > * they're assumed to be present when an extension is present which > * specifies [ms]envcfg bit(s). Hence, we don't do any additional > * priv spec version checks or CSR probes here. > */ I was about to read the whole discussion in v2 to understand the v3...thank you Drew :) I think it really makes sense to add this comment, do you intend to do so Samuel? Thanks, Alex > > Thanks, > drew > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH -fixes v3 1/2] riscv: Fix enabling cbo.zero when running in M-mode 2024-02-27 19:48 ` Alexandre Ghiti @ 2024-02-27 20:01 ` Samuel Holland 0 siblings, 0 replies; 6+ messages in thread From: Samuel Holland @ 2024-02-27 20:01 UTC (permalink / raw) To: Alexandre Ghiti Cc: Palmer Dabbelt, linux-kernel, Conor Dooley, linux-riscv, Stefan O'Rear, stable, Andrew Jones Hi Alex, On 2024-02-27 1:48 PM, Alexandre Ghiti wrote: > Hi Samuel, > > On 14/02/2024 10:28, Andrew Jones wrote: >> On Wed, Feb 14, 2024 at 01:01:56AM -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@vger.kernel.org> >>> Fixes: 43c16d51a19b ("RISC-V: Enable cbo.zero in usermode") >>> Reviewed-by: Andrew Jones <ajones@ventanamicro.com> >>> Signed-off-by: Samuel Holland <samuel.holland@sifive.com> >>> --- >>> >>> (no changes since v1) >>> >>> 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 >>> >> After our back and forth on how we determine the existence of the *envcfg >> CSRs, I wonder if we shouldn't put a comment above this >> riscv_user_isa_enable() function capturing the [current] decision. >> >> Something like >> >> /* >> * While the [ms]envcfg CSRs weren't defined until priv spec 1.12, >> * they're assumed to be present when an extension is present which >> * specifies [ms]envcfg bit(s). Hence, we don't do any additional >> * priv spec version checks or CSR probes here. >> */ > > > I was about to read the whole discussion in v2 to understand the v3...thank you > Drew :) I think it really makes sense to add this comment, do you intend to do > so Samuel? Yes, I am about to send a v4 with the changes from the previous discussion, including a RISCV_ISA_EXT_XLINUXENVCFG bit specifically for the presence of the [ms]envcfg CSR and a comment like the above. Regards, Samuel ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH -fixes v3 2/2] riscv: Save/restore envcfg CSR during CPU suspend [not found] <20240214090206.195754-1-samuel.holland@sifive.com> 2024-02-14 9:01 ` [PATCH -fixes v3 1/2] riscv: Fix enabling cbo.zero when running in M-mode Samuel Holland @ 2024-02-14 9:01 ` Samuel Holland 2024-02-14 9:29 ` Andrew Jones 1 sibling, 1 reply; 6+ messages in thread From: Samuel Holland @ 2024-02-14 9:01 UTC (permalink / raw) To: Palmer Dabbelt Cc: Andrew Jones, linux-kernel, Conor Dooley, linux-riscv, Stefan O'Rear, Samuel Holland, stable 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. The [ms]envcfg CSR was added in version 1.12 of the privileged ISA, and is used by extensions other than Zicboz. However, the kernel currenly has no way to determine the privileged ISA version. Since Zicboz is the only in-kernel user of this CSR so far, use it as a proxy for determining if the CSR is implemented. Cc: <stable@vger.kernel.org> # v6.7+ Fixes: 43c16d51a19b ("RISC-V: Enable cbo.zero in usermode") Signed-off-by: Samuel Holland <samuel.holland@sifive.com> --- Changes in v3: - Check for Zicboz instead of the privileged ISA version Changes in v2: - Check for privileged ISA v1.12 instead of the specific CSR - Use riscv_has_extension_likely() instead of new ALTERNATIVE()s arch/riscv/include/asm/suspend.h | 1 + arch/riscv/kernel/suspend.c | 4 ++++ 2 files changed, 5 insertions(+) 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/suspend.c b/arch/riscv/kernel/suspend.c index 239509367e42..28166006688e 100644 --- a/arch/riscv/kernel/suspend.c +++ b/arch/riscv/kernel/suspend.c @@ -15,6 +15,8 @@ void suspend_save_csrs(struct suspend_context *context) { context->scratch = csr_read(CSR_SCRATCH); + if (riscv_cpu_has_extension_unlikely(smp_processor_id(), RISCV_ISA_EXT_ZICBOZ)) + context->envcfg = csr_read(CSR_ENVCFG); context->tvec = csr_read(CSR_TVEC); context->ie = csr_read(CSR_IE); @@ -36,6 +38,8 @@ void suspend_save_csrs(struct suspend_context *context) void suspend_restore_csrs(struct suspend_context *context) { csr_write(CSR_SCRATCH, context->scratch); + if (riscv_cpu_has_extension_unlikely(smp_processor_id(), RISCV_ISA_EXT_ZICBOZ)) + csr_write(CSR_ENVCFG, context->envcfg); csr_write(CSR_TVEC, context->tvec); csr_write(CSR_IE, context->ie); -- 2.43.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH -fixes v3 2/2] riscv: Save/restore envcfg CSR during CPU suspend 2024-02-14 9:01 ` [PATCH -fixes v3 2/2] riscv: Save/restore envcfg CSR during CPU suspend Samuel Holland @ 2024-02-14 9:29 ` Andrew Jones 0 siblings, 0 replies; 6+ messages in thread From: Andrew Jones @ 2024-02-14 9:29 UTC (permalink / raw) To: Samuel Holland Cc: Palmer Dabbelt, linux-kernel, Conor Dooley, linux-riscv, Stefan O'Rear, stable On Wed, Feb 14, 2024 at 01:01:57AM -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. > > The [ms]envcfg CSR was added in version 1.12 of the privileged ISA, and > is used by extensions other than Zicboz. However, the kernel currenly > has no way to determine the privileged ISA version. Since Zicboz is the > only in-kernel user of this CSR so far, use it as a proxy for > determining if the CSR is implemented. > > Cc: <stable@vger.kernel.org> # v6.7+ > Fixes: 43c16d51a19b ("RISC-V: Enable cbo.zero in usermode") > Signed-off-by: Samuel Holland <samuel.holland@sifive.com> > --- > > Changes in v3: > - Check for Zicboz instead of the privileged ISA version > > Changes in v2: > - Check for privileged ISA v1.12 instead of the specific CSR > - Use riscv_has_extension_likely() instead of new ALTERNATIVE()s > > arch/riscv/include/asm/suspend.h | 1 + > arch/riscv/kernel/suspend.c | 4 ++++ > 2 files changed, 5 insertions(+) > > 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/suspend.c b/arch/riscv/kernel/suspend.c > index 239509367e42..28166006688e 100644 > --- a/arch/riscv/kernel/suspend.c > +++ b/arch/riscv/kernel/suspend.c > @@ -15,6 +15,8 @@ > void suspend_save_csrs(struct suspend_context *context) > { > context->scratch = csr_read(CSR_SCRATCH); > + if (riscv_cpu_has_extension_unlikely(smp_processor_id(), RISCV_ISA_EXT_ZICBOZ)) > + context->envcfg = csr_read(CSR_ENVCFG); > context->tvec = csr_read(CSR_TVEC); > context->ie = csr_read(CSR_IE); > > @@ -36,6 +38,8 @@ void suspend_save_csrs(struct suspend_context *context) > void suspend_restore_csrs(struct suspend_context *context) > { > csr_write(CSR_SCRATCH, context->scratch); > + if (riscv_cpu_has_extension_unlikely(smp_processor_id(), RISCV_ISA_EXT_ZICBOZ)) > + 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> ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-02-27 20:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20240214090206.195754-1-samuel.holland@sifive.com>
2024-02-14 9:01 ` [PATCH -fixes v3 1/2] riscv: Fix enabling cbo.zero when running in M-mode Samuel Holland
2024-02-14 9:28 ` Andrew Jones
2024-02-27 19:48 ` Alexandre Ghiti
2024-02-27 20:01 ` Samuel Holland
2024-02-14 9:01 ` [PATCH -fixes v3 2/2] riscv: Save/restore envcfg CSR during CPU suspend Samuel Holland
2024-02-14 9:29 ` Andrew Jones
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox