Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43808 )
Change subject: arch/x86: Move cpu_relax() ......................................................................
arch/x86: Move cpu_relax()
It's not related to spinlocks and the actual implementation was also guarded by CONFIG(SMP).
With a single call-site in x86-specific code, empty stubs for other arch are currently not necessary.
Change-Id: I00439e9c1d10c943ab5e404f5d687d316768fa16 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/arch/arm/include/armv4/arch/smp/spinlock.h M src/arch/x86/include/arch/cpu.h M src/arch/x86/include/arch/smp/spinlock.h M src/cpu/x86/tsc/delay_tsc.c M src/include/smp/spinlock.h M src/soc/amd/picasso/psp_verstage/include/arch/smp/spinlock.h 6 files changed, 7 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/43808/1
diff --git a/src/arch/arm/include/armv4/arch/smp/spinlock.h b/src/arch/arm/include/armv4/arch/smp/spinlock.h index 5245bd1..4d40f33 100644 --- a/src/arch/arm/include/armv4/arch/smp/spinlock.h +++ b/src/arch/arm/include/armv4/arch/smp/spinlock.h @@ -9,7 +9,6 @@ #define spin_unlock_wait(lock) do {} while (0) #define spin_lock(lock) do {} while (0) #define spin_unlock(lock) do {} while (0) -#define cpu_relax() do {} while (0)
#include <smp/node.h> #define boot_cpu() 1 diff --git a/src/arch/x86/include/arch/cpu.h b/src/arch/x86/include/arch/cpu.h index 82f470e..b622465 100644 --- a/src/arch/x86/include/arch/cpu.h +++ b/src/arch/x86/include/arch/cpu.h @@ -269,6 +269,12 @@
}
+/* REP NOP (PAUSE) is a good thing to insert into busy-wait loops. */ +static __always_inline void cpu_relax(void) +{ + __asm__ __volatile__("rep;nop" : : : "memory"); +} + #define asmlinkage __attribute__((regparm(0)))
/* diff --git a/src/arch/x86/include/arch/smp/spinlock.h b/src/arch/x86/include/arch/smp/spinlock.h index 4118993..a05d47a 100644 --- a/src/arch/x86/include/arch/smp/spinlock.h +++ b/src/arch/x86/include/arch/smp/spinlock.h @@ -62,12 +62,6 @@ : "=m" (lock->lock) : : "memory"); }
-/* REP NOP (PAUSE) is a good thing to insert into busy-wait loops. */ -static __always_inline void cpu_relax(void) -{ - __asm__ __volatile__("rep;nop" : : : "memory"); -} - #else
#define DECLARE_SPIN_LOCK(x) @@ -76,7 +70,6 @@ #define spin_unlock_wait(lock) do {} while (0) #define spin_lock(lock) do {} while (0) #define spin_unlock(lock) do {} while (0) -#define cpu_relax() do {} while (0)
#endif
diff --git a/src/cpu/x86/tsc/delay_tsc.c b/src/cpu/x86/tsc/delay_tsc.c index 893d41d..9607c2c 100644 --- a/src/cpu/x86/tsc/delay_tsc.c +++ b/src/cpu/x86/tsc/delay_tsc.c @@ -1,8 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0-only */
+#include <arch/cpu.h> #include <cpu/x86/tsc.h> -#include <pc80/i8254.h> -#include <smp/spinlock.h> #include <delay.h> #include <thread.h>
diff --git a/src/include/smp/spinlock.h b/src/include/smp/spinlock.h index 98ab3a7..40dd602 100644 --- a/src/include/smp/spinlock.h +++ b/src/include/smp/spinlock.h @@ -11,7 +11,6 @@ #define spin_unlock_wait(lock) do {} while (0) #define spin_lock(lock) do {} while (0) #define spin_unlock(lock) do {} while (0) -#define cpu_relax() do {} while (0) #endif
#endif /* SMP_SPINLOCK_H */ diff --git a/src/soc/amd/picasso/psp_verstage/include/arch/smp/spinlock.h b/src/soc/amd/picasso/psp_verstage/include/arch/smp/spinlock.h index 5245bd1..4d40f33 100644 --- a/src/soc/amd/picasso/psp_verstage/include/arch/smp/spinlock.h +++ b/src/soc/amd/picasso/psp_verstage/include/arch/smp/spinlock.h @@ -9,7 +9,6 @@ #define spin_unlock_wait(lock) do {} while (0) #define spin_lock(lock) do {} while (0) #define spin_unlock(lock) do {} while (0) -#define cpu_relax() do {} while (0)
#include <smp/node.h> #define boot_cpu() 1
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43808 )
Change subject: arch/x86: Move cpu_relax() ......................................................................
Patch Set 2: Code-Review+2
(2 comments)
https://review.coreboot.org/c/coreboot/+/43808/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43808/2//COMMIT_MSG@13 PS2, Line 13: arch architectures
https://review.coreboot.org/c/coreboot/+/43808/2/src/cpu/x86/tsc/delay_tsc.c File src/cpu/x86/tsc/delay_tsc.c:
https://review.coreboot.org/c/coreboot/+/43808/2/src/cpu/x86/tsc/delay_tsc.c... PS2, Line 4: #include <pc80/i8254.h> Is this gone intentionally?
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43808 )
Change subject: arch/x86: Move cpu_relax() ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43808/2/src/cpu/x86/tsc/delay_tsc.c File src/cpu/x86/tsc/delay_tsc.c:
https://review.coreboot.org/c/coreboot/+/43808/2/src/cpu/x86/tsc/delay_tsc.c... PS2, Line 4: #include <pc80/i8254.h>
Is this gone intentionally?
I can split it if you really want. Implementation of calibrate_tsc_with_pit() once lived in this file.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43808 )
Change subject: arch/x86: Move cpu_relax() ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43808/2/src/cpu/x86/tsc/delay_tsc.c File src/cpu/x86/tsc/delay_tsc.c:
https://review.coreboot.org/c/coreboot/+/43808/2/src/cpu/x86/tsc/delay_tsc.c... PS2, Line 4: #include <pc80/i8254.h>
I can split it if you really want. […]
Just mention it in the commit message, e.g.:
While we're changing includes, also drop an unused header.
Hello build bot (Jenkins), Angel Pons, Julius Werner,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43808
to look at the new patch set (#3).
Change subject: arch/x86: Move cpu_relax() ......................................................................
arch/x86: Move cpu_relax()
It's not related to spinlocks and the actual implementation was also guarded by CONFIG(SMP).
With a single call-site in x86-specific code, empty stubs for other arch are currently not necessary.
Also drop an unused included on a nearby line.
Change-Id: I00439e9c1d10c943ab5e404f5d687d316768fa16 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/arch/arm/include/armv4/arch/smp/spinlock.h M src/arch/x86/include/arch/cpu.h M src/arch/x86/include/arch/smp/spinlock.h M src/cpu/x86/tsc/delay_tsc.c M src/include/smp/spinlock.h M src/soc/amd/picasso/psp_verstage/include/arch/smp/spinlock.h 6 files changed, 7 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/43808/3
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43808 )
Change subject: arch/x86: Move cpu_relax() ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43808/2/src/cpu/x86/tsc/delay_tsc.c File src/cpu/x86/tsc/delay_tsc.c:
https://review.coreboot.org/c/coreboot/+/43808/2/src/cpu/x86/tsc/delay_tsc.c... PS2, Line 4: #include <pc80/i8254.h>
Just mention it in the commit message, e.g.: […]
Done
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43808 )
Change subject: arch/x86: Move cpu_relax() ......................................................................
Patch Set 3: Code-Review+2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43808 )
Change subject: arch/x86: Move cpu_relax() ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43808/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43808/2//COMMIT_MSG@13 PS2, Line 13: arch
architectures
Ack
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/43808 )
Change subject: arch/x86: Move cpu_relax() ......................................................................
arch/x86: Move cpu_relax()
It's not related to spinlocks and the actual implementation was also guarded by CONFIG(SMP).
With a single call-site in x86-specific code, empty stubs for other arch are currently not necessary.
Also drop an unused included on a nearby line.
Change-Id: I00439e9c1d10c943ab5e404f5d687d316768fa16 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/43808 Reviewed-by: Angel Pons th3fanbus@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/arch/arm/include/armv4/arch/smp/spinlock.h M src/arch/x86/include/arch/cpu.h M src/arch/x86/include/arch/smp/spinlock.h M src/cpu/x86/tsc/delay_tsc.c M src/include/smp/spinlock.h M src/soc/amd/picasso/psp_verstage/include/arch/smp/spinlock.h 6 files changed, 7 insertions(+), 12 deletions(-)
Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved
diff --git a/src/arch/arm/include/armv4/arch/smp/spinlock.h b/src/arch/arm/include/armv4/arch/smp/spinlock.h index 5245bd1..4d40f33 100644 --- a/src/arch/arm/include/armv4/arch/smp/spinlock.h +++ b/src/arch/arm/include/armv4/arch/smp/spinlock.h @@ -9,7 +9,6 @@ #define spin_unlock_wait(lock) do {} while (0) #define spin_lock(lock) do {} while (0) #define spin_unlock(lock) do {} while (0) -#define cpu_relax() do {} while (0)
#include <smp/node.h> #define boot_cpu() 1 diff --git a/src/arch/x86/include/arch/cpu.h b/src/arch/x86/include/arch/cpu.h index 82f470e..b622465 100644 --- a/src/arch/x86/include/arch/cpu.h +++ b/src/arch/x86/include/arch/cpu.h @@ -269,6 +269,12 @@
}
+/* REP NOP (PAUSE) is a good thing to insert into busy-wait loops. */ +static __always_inline void cpu_relax(void) +{ + __asm__ __volatile__("rep;nop" : : : "memory"); +} + #define asmlinkage __attribute__((regparm(0)))
/* diff --git a/src/arch/x86/include/arch/smp/spinlock.h b/src/arch/x86/include/arch/smp/spinlock.h index 4118993..a05d47a 100644 --- a/src/arch/x86/include/arch/smp/spinlock.h +++ b/src/arch/x86/include/arch/smp/spinlock.h @@ -62,12 +62,6 @@ : "=m" (lock->lock) : : "memory"); }
-/* REP NOP (PAUSE) is a good thing to insert into busy-wait loops. */ -static __always_inline void cpu_relax(void) -{ - __asm__ __volatile__("rep;nop" : : : "memory"); -} - #else
#define DECLARE_SPIN_LOCK(x) @@ -76,7 +70,6 @@ #define spin_unlock_wait(lock) do {} while (0) #define spin_lock(lock) do {} while (0) #define spin_unlock(lock) do {} while (0) -#define cpu_relax() do {} while (0)
#endif
diff --git a/src/cpu/x86/tsc/delay_tsc.c b/src/cpu/x86/tsc/delay_tsc.c index 893d41d..9607c2c 100644 --- a/src/cpu/x86/tsc/delay_tsc.c +++ b/src/cpu/x86/tsc/delay_tsc.c @@ -1,8 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0-only */
+#include <arch/cpu.h> #include <cpu/x86/tsc.h> -#include <pc80/i8254.h> -#include <smp/spinlock.h> #include <delay.h> #include <thread.h>
diff --git a/src/include/smp/spinlock.h b/src/include/smp/spinlock.h index 98ab3a7..40dd602 100644 --- a/src/include/smp/spinlock.h +++ b/src/include/smp/spinlock.h @@ -11,7 +11,6 @@ #define spin_unlock_wait(lock) do {} while (0) #define spin_lock(lock) do {} while (0) #define spin_unlock(lock) do {} while (0) -#define cpu_relax() do {} while (0) #endif
#endif /* SMP_SPINLOCK_H */ diff --git a/src/soc/amd/picasso/psp_verstage/include/arch/smp/spinlock.h b/src/soc/amd/picasso/psp_verstage/include/arch/smp/spinlock.h index 5245bd1..4d40f33 100644 --- a/src/soc/amd/picasso/psp_verstage/include/arch/smp/spinlock.h +++ b/src/soc/amd/picasso/psp_verstage/include/arch/smp/spinlock.h @@ -9,7 +9,6 @@ #define spin_unlock_wait(lock) do {} while (0) #define spin_lock(lock) do {} while (0) #define spin_unlock(lock) do {} while (0) -#define cpu_relax() do {} while (0)
#include <smp/node.h> #define boot_cpu() 1
-
Angel Pons (Code Review) -
Kyösti Mälkki (Code Review)
-
Patrick Georgi (Code Review)