Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46839 )
Change subject: arch/x86/spinlock: Drop linker section in inline assembly ......................................................................
arch/x86/spinlock: Drop linker section in inline assembly
The linker section isn't required or specified in the linker script. This confuses GCC and prevents garbage collecting functions using spinlocks, which might cause linker errors.
Tested by compiling the C SIPI vector with spinlocks.
Change-Id: Ia4f5a8f4aec0e05bc35359b334845c177867d4a4 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/arch/x86/include/arch/smp/spinlock.h 1 file changed, 1 insertion(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/46839/1
diff --git a/src/arch/x86/include/arch/smp/spinlock.h b/src/arch/x86/include/arch/smp/spinlock.h index 799ac2c..37e3eaf 100644 --- a/src/arch/x86/include/arch/smp/spinlock.h +++ b/src/arch/x86/include/arch/smp/spinlock.h @@ -35,13 +35,11 @@ "\n1:\t" \ "lock ; decb %0\n\t" \ "js 2f\n" \ - ".section .text.lock,"ax"\n" \ "2:\t" \ "cmpb $0,%0\n\t" \ "rep;nop\n\t" \ "jle 2b\n\t" \ - "jmp 1b\n" \ - ".previous" + "jmp 1b\n"
/* * This works. Despite all the confusion.
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46839 )
Change subject: arch/x86/spinlock: Drop linker section in inline assembly ......................................................................
Patch Set 1: Code-Review-1
postcar stage won't boot any more up with this change
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46839
to look at the new patch set (#2).
Change subject: arch/x86/spinlock: Drop linker section in inline assembly ......................................................................
arch/x86/spinlock: Drop linker section in inline assembly
The linker section isn't required or specified in the linker script. This confuses GCC and prevents garbage collecting functions using spinlocks, which might cause linker errors.
Tested by compiling the C SIPI vector with spinlocks. Tested on Supermicro X11SSH. Still boots.
Change-Id: Ia4f5a8f4aec0e05bc35359b334845c177867d4a4 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/arch/x86/include/arch/smp/spinlock.h 1 file changed, 6 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/46839/2
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46839 )
Change subject: arch/x86/spinlock: Drop linker section in inline assembly ......................................................................
Patch Set 2:
Reworked the assembly code to prevent infinite loops.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46839 )
Change subject: arch/x86/spinlock: Drop linker section in inline assembly ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46839/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46839/2//COMMIT_MSG@11 PS2, Line 11: might Why? And does it?
What is the actual problem with garbage collection of linker sections? Was the previous code not referencing the .text.lock section?
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46839 )
Change subject: arch/x86/spinlock: Drop linker section in inline assembly ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46839/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46839/2//COMMIT_MSG@11 PS2, Line 11: might
Why? And does it? […]
It does cause linker errors as those functions aren't garbage collected.
I guess the problem is that the inline assembly code generates an output section, but --gc-sections only works on input sections, as generated by -ffunction-sections.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46839 )
Change subject: arch/x86/spinlock: Drop linker section in inline assembly ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46839/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46839/2//COMMIT_MSG@11 PS2, Line 11: might
It does cause linker errors as those functions aren't garbage collected. […]
A function not being garbage collected shouldn't cause an error. What is the error exactly? It's hard to understand what the actual problem is based on the description. Could you please provide more information in the commit description so it's clear that there's a real issue that needs to be resolved?
Patrick Rudolph has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/46839 )
Change subject: arch/x86/spinlock: Drop linker section in inline assembly ......................................................................
Abandoned
Reworked the rmodule linking, this is no longer required
-
Aaron Durbin (Code Review) -
Patrick Rudolph (Code Review)