* [PATCH] riscv: uaccess: Only restore the CSR_STATUS SUM bit
@ 2025-05-22 16:09 Cyril Bur
2025-05-26 20:39 ` Alexandre Ghiti
0 siblings, 1 reply; 7+ messages in thread
From: Cyril Bur @ 2025-05-22 16:09 UTC (permalink / raw)
To: samuel.holland, ben.dooks, palmer, alex, linux-kernel
Cc: jszhang, paul.walmsley, charlie, jrtc27, aou
During switch to csrs will OR the value of the register into the
corresponding csr. In this case we're only interested in restoring the
SUM bit not the entire register.
Fixes: 788aa64c0 ("riscv: save the SR_SUM status over switches")
Signed-off-by: Cyril Bur <cyrilbur@tenstorrent.com>
---
I've put the Fixes tag in but I assume this will get squashed into the
patch. Either way I hope this works to fix the immediate issue.
arch/riscv/kernel/entry.S | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
index 00bd0de9faa2..6ed3bd80903d 100644
--- a/arch/riscv/kernel/entry.S
+++ b/arch/riscv/kernel/entry.S
@@ -399,14 +399,18 @@ SYM_FUNC_START(__switch_to)
REG_S s11, TASK_THREAD_S11_RA(a3)
/* save the user space access flag */
- li s0, SR_SUM
- csrr s1, CSR_STATUS
- REG_S s1, TASK_THREAD_STATUS_RA(a3)
+ csrr s0, CSR_STATUS
+ REG_S s0, TASK_THREAD_STATUS_RA(a3)
/* Save the kernel shadow call stack pointer */
scs_save_current
- /* Restore context from next->thread */
+ /*
+ * Restore context from next->thread. csrs will OR the bits from s0 and
+ * only want to restore the SR_SUM bit
+ */
REG_L s0, TASK_THREAD_STATUS_RA(a4)
+ li s1, SR_SUM
+ and s0, s0, s1
csrs CSR_STATUS, s0
REG_L ra, TASK_THREAD_RA_RA(a4)
REG_L sp, TASK_THREAD_SP_RA(a4)
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] riscv: uaccess: Only restore the CSR_STATUS SUM bit 2025-05-22 16:09 [PATCH] riscv: uaccess: Only restore the CSR_STATUS SUM bit Cyril Bur @ 2025-05-26 20:39 ` Alexandre Ghiti 0 siblings, 0 replies; 7+ messages in thread From: Alexandre Ghiti @ 2025-05-26 20:39 UTC (permalink / raw) To: Cyril Bur, samuel.holland, ben.dooks, palmer, linux-kernel Cc: jszhang, paul.walmsley, charlie, jrtc27, aou, linux-riscv, Andy Chiu, Deepak Gupta +cc linux-riscv, Andy, Deepak On 5/22/25 18:09, Cyril Bur wrote: > During switch to csrs will OR the value of the register into the > corresponding csr. In this case we're only interested in restoring the > SUM bit not the entire register. > > Fixes: 788aa64c0 ("riscv: save the SR_SUM status over switches") > Signed-off-by: Cyril Bur <cyrilbur@tenstorrent.com> > --- > I've put the Fixes tag in but I assume this will get squashed into the > patch. Either way I hope this works to fix the immediate issue. > > arch/riscv/kernel/entry.S | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S > index 00bd0de9faa2..6ed3bd80903d 100644 > --- a/arch/riscv/kernel/entry.S > +++ b/arch/riscv/kernel/entry.S > @@ -399,14 +399,18 @@ SYM_FUNC_START(__switch_to) > REG_S s11, TASK_THREAD_S11_RA(a3) > > /* save the user space access flag */ > - li s0, SR_SUM > - csrr s1, CSR_STATUS > - REG_S s1, TASK_THREAD_STATUS_RA(a3) > + csrr s0, CSR_STATUS > + REG_S s0, TASK_THREAD_STATUS_RA(a3) > > /* Save the kernel shadow call stack pointer */ > scs_save_current > - /* Restore context from next->thread */ > + /* > + * Restore context from next->thread. csrs will OR the bits from s0 and > + * only want to restore the SR_SUM bit > + */ > REG_L s0, TASK_THREAD_STATUS_RA(a4) > + li s1, SR_SUM > + and s0, s0, s1 > csrs CSR_STATUS, s0 > REG_L ra, TASK_THREAD_RA_RA(a4) > REG_L sp, TASK_THREAD_SP_RA(a4) To conclude the discussion we had here https://lore.kernel.org/linux-riscv/aDCtATl2N21fBsyT@debug.ba.rivosinc.com/#t, in addition to Cyril's patch above, to me we only have to rename the status field into sum and we're good to go. @Andy, @Deepak @Samuel Do you agree? As this is an important fix (along with 2 other fixes, one for thead vector and vdso static values), I'd like to send another PR soon for inclusion in 6.16-rc1, I did not want to delay the second PR any longer. Thanks for your feedbacks, Alex _______________________________________________ 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] riscv: uaccess: Only restore the CSR_STATUS SUM bit @ 2025-05-26 20:39 ` Alexandre Ghiti 0 siblings, 0 replies; 7+ messages in thread From: Alexandre Ghiti @ 2025-05-26 20:39 UTC (permalink / raw) To: Cyril Bur, samuel.holland, ben.dooks, palmer, linux-kernel Cc: jszhang, paul.walmsley, charlie, jrtc27, aou, linux-riscv, Andy Chiu, Deepak Gupta +cc linux-riscv, Andy, Deepak On 5/22/25 18:09, Cyril Bur wrote: > During switch to csrs will OR the value of the register into the > corresponding csr. In this case we're only interested in restoring the > SUM bit not the entire register. > > Fixes: 788aa64c0 ("riscv: save the SR_SUM status over switches") > Signed-off-by: Cyril Bur <cyrilbur@tenstorrent.com> > --- > I've put the Fixes tag in but I assume this will get squashed into the > patch. Either way I hope this works to fix the immediate issue. > > arch/riscv/kernel/entry.S | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S > index 00bd0de9faa2..6ed3bd80903d 100644 > --- a/arch/riscv/kernel/entry.S > +++ b/arch/riscv/kernel/entry.S > @@ -399,14 +399,18 @@ SYM_FUNC_START(__switch_to) > REG_S s11, TASK_THREAD_S11_RA(a3) > > /* save the user space access flag */ > - li s0, SR_SUM > - csrr s1, CSR_STATUS > - REG_S s1, TASK_THREAD_STATUS_RA(a3) > + csrr s0, CSR_STATUS > + REG_S s0, TASK_THREAD_STATUS_RA(a3) > > /* Save the kernel shadow call stack pointer */ > scs_save_current > - /* Restore context from next->thread */ > + /* > + * Restore context from next->thread. csrs will OR the bits from s0 and > + * only want to restore the SR_SUM bit > + */ > REG_L s0, TASK_THREAD_STATUS_RA(a4) > + li s1, SR_SUM > + and s0, s0, s1 > csrs CSR_STATUS, s0 > REG_L ra, TASK_THREAD_RA_RA(a4) > REG_L sp, TASK_THREAD_SP_RA(a4) To conclude the discussion we had here https://lore.kernel.org/linux-riscv/aDCtATl2N21fBsyT@debug.ba.rivosinc.com/#t, in addition to Cyril's patch above, to me we only have to rename the status field into sum and we're good to go. @Andy, @Deepak @Samuel Do you agree? As this is an important fix (along with 2 other fixes, one for thead vector and vdso static values), I'd like to send another PR soon for inclusion in 6.16-rc1, I did not want to delay the second PR any longer. Thanks for your feedbacks, Alex ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] riscv: uaccess: Only restore the CSR_STATUS SUM bit 2025-05-26 20:39 ` Alexandre Ghiti @ 2025-05-28 16:17 ` Andy Chiu -1 siblings, 0 replies; 7+ messages in thread From: Andy Chiu @ 2025-05-28 16:17 UTC (permalink / raw) To: Alexandre Ghiti Cc: Cyril Bur, samuel.holland, ben.dooks, palmer, linux-kernel, jszhang, paul.walmsley, charlie, jrtc27, aou, linux-riscv, Deepak Gupta On Tue, May 27, 2025 at 4:39 AM Alexandre Ghiti <alex@ghiti.fr> wrote: > > +cc linux-riscv, Andy, Deepak > > On 5/22/25 18:09, Cyril Bur wrote: > > During switch to csrs will OR the value of the register into the > > corresponding csr. In this case we're only interested in restoring the > > SUM bit not the entire register. > > > > Fixes: 788aa64c0 ("riscv: save the SR_SUM status over switches") > > Signed-off-by: Cyril Bur <cyrilbur@tenstorrent.com> > > --- > > I've put the Fixes tag in but I assume this will get squashed into the > > patch. Either way I hope this works to fix the immediate issue. > > > > arch/riscv/kernel/entry.S | 12 ++++++++---- > > 1 file changed, 8 insertions(+), 4 deletions(-) > > > > diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S > > index 00bd0de9faa2..6ed3bd80903d 100644 > > --- a/arch/riscv/kernel/entry.S > > +++ b/arch/riscv/kernel/entry.S > > @@ -399,14 +399,18 @@ SYM_FUNC_START(__switch_to) > > REG_S s11, TASK_THREAD_S11_RA(a3) > > > > /* save the user space access flag */ > > - li s0, SR_SUM > > - csrr s1, CSR_STATUS > > - REG_S s1, TASK_THREAD_STATUS_RA(a3) > > + csrr s0, CSR_STATUS > > + REG_S s0, TASK_THREAD_STATUS_RA(a3) > > > > /* Save the kernel shadow call stack pointer */ > > scs_save_current > > - /* Restore context from next->thread */ > > + /* > > + * Restore context from next->thread. csrs will OR the bits from s0 and > > + * only want to restore the SR_SUM bit > > + */ > > REG_L s0, TASK_THREAD_STATUS_RA(a4) > > + li s1, SR_SUM > > + and s0, s0, s1 > > csrs CSR_STATUS, s0 > > REG_L ra, TASK_THREAD_RA_RA(a4) > > REG_L sp, TASK_THREAD_SP_RA(a4) > > To conclude the discussion we had here > https://lore.kernel.org/linux-riscv/aDCtATl2N21fBsyT@debug.ba.rivosinc.com/#t, > in addition to Cyril's patch above, to me we only have to rename the > status field into sum and we're good to go. @Andy, @Deepak @Samuel Do > you agree? LGTM, thanks! Andy > > As this is an important fix (along with 2 other fixes, one for thead > vector and vdso static values), I'd like to send another PR soon for > inclusion in 6.16-rc1, I did not want to delay the second PR any longer. > > Thanks for your feedbacks, > > Alex > > _______________________________________________ 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] riscv: uaccess: Only restore the CSR_STATUS SUM bit @ 2025-05-28 16:17 ` Andy Chiu 0 siblings, 0 replies; 7+ messages in thread From: Andy Chiu @ 2025-05-28 16:17 UTC (permalink / raw) To: Alexandre Ghiti Cc: Cyril Bur, samuel.holland, ben.dooks, palmer, linux-kernel, jszhang, paul.walmsley, charlie, jrtc27, aou, linux-riscv, Deepak Gupta On Tue, May 27, 2025 at 4:39 AM Alexandre Ghiti <alex@ghiti.fr> wrote: > > +cc linux-riscv, Andy, Deepak > > On 5/22/25 18:09, Cyril Bur wrote: > > During switch to csrs will OR the value of the register into the > > corresponding csr. In this case we're only interested in restoring the > > SUM bit not the entire register. > > > > Fixes: 788aa64c0 ("riscv: save the SR_SUM status over switches") > > Signed-off-by: Cyril Bur <cyrilbur@tenstorrent.com> > > --- > > I've put the Fixes tag in but I assume this will get squashed into the > > patch. Either way I hope this works to fix the immediate issue. > > > > arch/riscv/kernel/entry.S | 12 ++++++++---- > > 1 file changed, 8 insertions(+), 4 deletions(-) > > > > diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S > > index 00bd0de9faa2..6ed3bd80903d 100644 > > --- a/arch/riscv/kernel/entry.S > > +++ b/arch/riscv/kernel/entry.S > > @@ -399,14 +399,18 @@ SYM_FUNC_START(__switch_to) > > REG_S s11, TASK_THREAD_S11_RA(a3) > > > > /* save the user space access flag */ > > - li s0, SR_SUM > > - csrr s1, CSR_STATUS > > - REG_S s1, TASK_THREAD_STATUS_RA(a3) > > + csrr s0, CSR_STATUS > > + REG_S s0, TASK_THREAD_STATUS_RA(a3) > > > > /* Save the kernel shadow call stack pointer */ > > scs_save_current > > - /* Restore context from next->thread */ > > + /* > > + * Restore context from next->thread. csrs will OR the bits from s0 and > > + * only want to restore the SR_SUM bit > > + */ > > REG_L s0, TASK_THREAD_STATUS_RA(a4) > > + li s1, SR_SUM > > + and s0, s0, s1 > > csrs CSR_STATUS, s0 > > REG_L ra, TASK_THREAD_RA_RA(a4) > > REG_L sp, TASK_THREAD_SP_RA(a4) > > To conclude the discussion we had here > https://lore.kernel.org/linux-riscv/aDCtATl2N21fBsyT@debug.ba.rivosinc.com/#t, > in addition to Cyril's patch above, to me we only have to rename the > status field into sum and we're good to go. @Andy, @Deepak @Samuel Do > you agree? LGTM, thanks! Andy > > As this is an important fix (along with 2 other fixes, one for thead > vector and vdso static values), I'd like to send another PR soon for > inclusion in 6.16-rc1, I did not want to delay the second PR any longer. > > Thanks for your feedbacks, > > Alex > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] riscv: uaccess: Only restore the CSR_STATUS SUM bit 2025-05-28 16:17 ` Andy Chiu @ 2025-05-29 17:55 ` Deepak Gupta -1 siblings, 0 replies; 7+ messages in thread From: Deepak Gupta @ 2025-05-29 17:55 UTC (permalink / raw) To: Andy Chiu Cc: Alexandre Ghiti, Cyril Bur, samuel.holland, ben.dooks, palmer, linux-kernel, jszhang, paul.walmsley, charlie, jrtc27, aou, linux-riscv On Thu, May 29, 2025 at 12:17:24AM +0800, Andy Chiu wrote: >On Tue, May 27, 2025 at 4:39 AM Alexandre Ghiti <alex@ghiti.fr> wrote: >> >> +cc linux-riscv, Andy, Deepak >> >> On 5/22/25 18:09, Cyril Bur wrote: >> > During switch to csrs will OR the value of the register into the >> > corresponding csr. In this case we're only interested in restoring the >> > SUM bit not the entire register. >> > >> > Fixes: 788aa64c0 ("riscv: save the SR_SUM status over switches") >> > Signed-off-by: Cyril Bur <cyrilbur@tenstorrent.com> >> > --- >> > I've put the Fixes tag in but I assume this will get squashed into the >> > patch. Either way I hope this works to fix the immediate issue. >> > >> > arch/riscv/kernel/entry.S | 12 ++++++++---- >> > 1 file changed, 8 insertions(+), 4 deletions(-) >> > >> > diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S >> > index 00bd0de9faa2..6ed3bd80903d 100644 >> > --- a/arch/riscv/kernel/entry.S >> > +++ b/arch/riscv/kernel/entry.S >> > @@ -399,14 +399,18 @@ SYM_FUNC_START(__switch_to) >> > REG_S s11, TASK_THREAD_S11_RA(a3) >> > >> > /* save the user space access flag */ >> > - li s0, SR_SUM >> > - csrr s1, CSR_STATUS >> > - REG_S s1, TASK_THREAD_STATUS_RA(a3) >> > + csrr s0, CSR_STATUS >> > + REG_S s0, TASK_THREAD_STATUS_RA(a3) >> > >> > /* Save the kernel shadow call stack pointer */ >> > scs_save_current >> > - /* Restore context from next->thread */ >> > + /* >> > + * Restore context from next->thread. csrs will OR the bits from s0 and >> > + * only want to restore the SR_SUM bit >> > + */ >> > REG_L s0, TASK_THREAD_STATUS_RA(a4) >> > + li s1, SR_SUM >> > + and s0, s0, s1 >> > csrs CSR_STATUS, s0 >> > REG_L ra, TASK_THREAD_RA_RA(a4) >> > REG_L sp, TASK_THREAD_SP_RA(a4) >> >> To conclude the discussion we had here >> https://lore.kernel.org/linux-riscv/aDCtATl2N21fBsyT@debug.ba.rivosinc.com/#t, >> in addition to Cyril's patch above, to me we only have to rename the >> status field into sum and we're good to go. @Andy, @Deepak @Samuel Do >> you agree? > >LGTM, thanks! > >Andy Sounds good to me. > >> >> As this is an important fix (along with 2 other fixes, one for thead >> vector and vdso static values), I'd like to send another PR soon for >> inclusion in 6.16-rc1, I did not want to delay the second PR any longer. >> >> Thanks for your feedbacks, >> >> Alex >> >> _______________________________________________ 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] riscv: uaccess: Only restore the CSR_STATUS SUM bit @ 2025-05-29 17:55 ` Deepak Gupta 0 siblings, 0 replies; 7+ messages in thread From: Deepak Gupta @ 2025-05-29 17:55 UTC (permalink / raw) To: Andy Chiu Cc: Alexandre Ghiti, Cyril Bur, samuel.holland, ben.dooks, palmer, linux-kernel, jszhang, paul.walmsley, charlie, jrtc27, aou, linux-riscv On Thu, May 29, 2025 at 12:17:24AM +0800, Andy Chiu wrote: >On Tue, May 27, 2025 at 4:39 AM Alexandre Ghiti <alex@ghiti.fr> wrote: >> >> +cc linux-riscv, Andy, Deepak >> >> On 5/22/25 18:09, Cyril Bur wrote: >> > During switch to csrs will OR the value of the register into the >> > corresponding csr. In this case we're only interested in restoring the >> > SUM bit not the entire register. >> > >> > Fixes: 788aa64c0 ("riscv: save the SR_SUM status over switches") >> > Signed-off-by: Cyril Bur <cyrilbur@tenstorrent.com> >> > --- >> > I've put the Fixes tag in but I assume this will get squashed into the >> > patch. Either way I hope this works to fix the immediate issue. >> > >> > arch/riscv/kernel/entry.S | 12 ++++++++---- >> > 1 file changed, 8 insertions(+), 4 deletions(-) >> > >> > diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S >> > index 00bd0de9faa2..6ed3bd80903d 100644 >> > --- a/arch/riscv/kernel/entry.S >> > +++ b/arch/riscv/kernel/entry.S >> > @@ -399,14 +399,18 @@ SYM_FUNC_START(__switch_to) >> > REG_S s11, TASK_THREAD_S11_RA(a3) >> > >> > /* save the user space access flag */ >> > - li s0, SR_SUM >> > - csrr s1, CSR_STATUS >> > - REG_S s1, TASK_THREAD_STATUS_RA(a3) >> > + csrr s0, CSR_STATUS >> > + REG_S s0, TASK_THREAD_STATUS_RA(a3) >> > >> > /* Save the kernel shadow call stack pointer */ >> > scs_save_current >> > - /* Restore context from next->thread */ >> > + /* >> > + * Restore context from next->thread. csrs will OR the bits from s0 and >> > + * only want to restore the SR_SUM bit >> > + */ >> > REG_L s0, TASK_THREAD_STATUS_RA(a4) >> > + li s1, SR_SUM >> > + and s0, s0, s1 >> > csrs CSR_STATUS, s0 >> > REG_L ra, TASK_THREAD_RA_RA(a4) >> > REG_L sp, TASK_THREAD_SP_RA(a4) >> >> To conclude the discussion we had here >> https://lore.kernel.org/linux-riscv/aDCtATl2N21fBsyT@debug.ba.rivosinc.com/#t, >> in addition to Cyril's patch above, to me we only have to rename the >> status field into sum and we're good to go. @Andy, @Deepak @Samuel Do >> you agree? > >LGTM, thanks! > >Andy Sounds good to me. > >> >> As this is an important fix (along with 2 other fixes, one for thead >> vector and vdso static values), I'd like to send another PR soon for >> inclusion in 6.16-rc1, I did not want to delay the second PR any longer. >> >> Thanks for your feedbacks, >> >> Alex >> >> ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-05-29 17:55 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-05-22 16:09 [PATCH] riscv: uaccess: Only restore the CSR_STATUS SUM bit Cyril Bur 2025-05-26 20:39 ` Alexandre Ghiti 2025-05-26 20:39 ` Alexandre Ghiti 2025-05-28 16:17 ` Andy Chiu 2025-05-28 16:17 ` Andy Chiu 2025-05-29 17:55 ` Deepak Gupta 2025-05-29 17:55 ` Deepak Gupta
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.