diff mbox series

powerpc/vdso: allow r30 in vDSO code generation of getrandom

Message ID 20240925175021.1526936-2-Jason@zx2c4.com (mailing list archive)
State Accepted
Commit 4b058c9f281f5b100efbf665dd5a1a05e1654d6d
Headers show
Series powerpc/vdso: allow r30 in vDSO code generation of getrandom | expand

Checks

Context Check Description
snowpatch_ozlabs/github-powerpc_selftests success Successfully ran 8 jobs.
snowpatch_ozlabs/github-powerpc_ppctests success Successfully ran 8 jobs.
snowpatch_ozlabs/github-powerpc_kernel_qemu success Successfully ran 21 jobs.
snowpatch_ozlabs/github-powerpc_sparse success Successfully ran 4 jobs.
snowpatch_ozlabs/github-powerpc_clang success Successfully ran 5 jobs.

Commit Message

Jason A. Donenfeld Sept. 25, 2024, 5:50 p.m. UTC
For gettimeofday, -ffixed-r30 was passed to work around a bug in Go
code, where the vDSO trampoline forgot to save and restore this register
across function calls. But Go requires a different trampoline for every
call, and there's no reason that new Go code needs to be broken and add
more bugs.  So remove -ffixed-r30 for getrandom.

Fixes: 8072b39c3a75 ("powerpc/vdso: Wire up getrandom() vDSO implementation on VDSO64")
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
Michael - can you take this through your tree as a ppc fix?

 arch/powerpc/kernel/vdso/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jason A. Donenfeld Sept. 25, 2024, 6:38 p.m. UTC | #1
On Wed, Sep 25, 2024 at 07:50:22PM +0200, Jason A. Donenfeld wrote:
> For gettimeofday, -ffixed-r30 was passed to work around a bug in Go
> code, where the vDSO trampoline forgot to save and restore this register
> across function calls. But Go requires a different trampoline for every
> call, and there's no reason that new Go code needs to be broken and add
> more bugs.  So remove -ffixed-r30 for getrandom.

Strangely, I am _unable to_ make the Go code not crash with this patch
applied. I'm not quite sure what I'm doing wrong yet, or if this points
to another issue.
Christophe Leroy Sept. 25, 2024, 6:48 p.m. UTC | #2
Le 25/09/2024 à 20:38, Jason A. Donenfeld a écrit :
> On Wed, Sep 25, 2024 at 07:50:22PM +0200, Jason A. Donenfeld wrote:
>> For gettimeofday, -ffixed-r30 was passed to work around a bug in Go
>> code, where the vDSO trampoline forgot to save and restore this register
>> across function calls. But Go requires a different trampoline for every
>> call, and there's no reason that new Go code needs to be broken and add
>> more bugs.  So remove -ffixed-r30 for getrandom.
> 
> Strangely, I am _unable to_ make the Go code not crash with this patch
> applied. I'm not quite sure what I'm doing wrong yet, or if this points
> to another issue.

Do you mean that without this patch the Go code works and with this 
patch it crashes ? That's strange taken into account that the chacha 
function plays up with r30 regardless of this patch.

Christophe
Jason A. Donenfeld Sept. 25, 2024, 11:21 p.m. UTC | #3
On Wed, Sep 25, 2024 at 08:48:31PM +0200, Christophe Leroy wrote:
> 
> 
> Le 25/09/2024 à 20:38, Jason A. Donenfeld a écrit :
> > On Wed, Sep 25, 2024 at 07:50:22PM +0200, Jason A. Donenfeld wrote:
> >> For gettimeofday, -ffixed-r30 was passed to work around a bug in Go
> >> code, where the vDSO trampoline forgot to save and restore this register
> >> across function calls. But Go requires a different trampoline for every
> >> call, and there's no reason that new Go code needs to be broken and add
> >> more bugs.  So remove -ffixed-r30 for getrandom.
> > 
> > Strangely, I am _unable to_ make the Go code not crash with this patch
> > applied. I'm not quite sure what I'm doing wrong yet, or if this points
> > to another issue.
> 
> Do you mean that without this patch the Go code works and with this 
> patch it crashes ? That's strange taken into account that the chacha 
> function plays up with r30 regardless of this patch.

Yea, that's what I meant.

It turned out to be a Go runtime bug, which won't affect other vDSO
functions but does affect getrandom(). I fixed the issue and sent a
patch up to Google. So this is not the kernel's issue.

Therefore, this patch can move forward.

Jason
Michael Ellerman Sept. 30, 2024, 11:36 a.m. UTC | #4
On Wed, 25 Sep 2024 19:50:22 +0200, Jason A. Donenfeld wrote:
> For gettimeofday, -ffixed-r30 was passed to work around a bug in Go
> code, where the vDSO trampoline forgot to save and restore this register
> across function calls. But Go requires a different trampoline for every
> call, and there's no reason that new Go code needs to be broken and add
> more bugs.  So remove -ffixed-r30 for getrandom.
> 
> 
> [...]

Applied to powerpc/fixes.

[1/1] powerpc/vdso: allow r30 in vDSO code generation of getrandom
      https://git.kernel.org/powerpc/c/4b058c9f281f5b100efbf665dd5a1a05e1654d6d

cheers
diff mbox series

Patch

diff --git a/arch/powerpc/kernel/vdso/Makefile b/arch/powerpc/kernel/vdso/Makefile
index 56fb1633529a..31ca5a547004 100644
--- a/arch/powerpc/kernel/vdso/Makefile
+++ b/arch/powerpc/kernel/vdso/Makefile
@@ -22,7 +22,7 @@  endif
 
 ifneq ($(c-getrandom-y),)
   CFLAGS_vgetrandom-32.o += -include $(c-getrandom-y)
-  CFLAGS_vgetrandom-64.o += -include $(c-getrandom-y) $(call cc-option, -ffixed-r30)
+  CFLAGS_vgetrandom-64.o += -include $(c-getrandom-y)
 endif
 
 # Build rules