* [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.