Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44922 )
Change subject: arch/x86/exit_car.S: Fix checking clflush support ......................................................................
arch/x86/exit_car.S: Fix checking clflush support
The BT instruction stores it result in CF and not ZF so use the correct jump instruction.
Change-Id: I704e3c579150fb9b9a292ef0e83050e7bf7cb078 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/arch/x86/exit_car.S 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/44922/1
diff --git a/src/arch/x86/exit_car.S b/src/arch/x86/exit_car.S index dc356b2..fae7899 100644 --- a/src/arch/x86/exit_car.S +++ b/src/arch/x86/exit_car.S @@ -59,7 +59,7 @@ movl $1, %eax cpuid btl $CPUID_FEATURE_CLFLUSH_BIT, %edx - jz skip_clflush + jnc skip_clflush clflush _cbmem_top_ptr
skip_clflush:
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44922 )
Change subject: arch/x86/exit_car.S: Fix checking clflush support ......................................................................
Patch Set 1: Code-Review+2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44922 )
Change subject: arch/x86/exit_car.S: Fix checking clflush support ......................................................................
Patch Set 1: Code-Review+1
(3 comments)
https://review.coreboot.org/c/coreboot/+/44922/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44922/1//COMMIT_MSG@9 PS1, Line 9: it its
https://review.coreboot.org/c/coreboot/+/44922/1//COMMIT_MSG@10 PS1, Line 10: correct jump instruction. Please add on what boards/chipsets this caused problems. For example, why did QEMU i440fx still work?
https://review.coreboot.org/c/coreboot/+/44922/1//COMMIT_MSG@11 PS1, Line 11: It’d be great, if you added a `Fixes:` tag.
Hello build bot (Jenkins), Paul Menzel, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44922
to look at the new patch set (#2).
Change subject: arch/x86/exit_car.S: Fix checking clflush support ......................................................................
arch/x86/exit_car.S: Fix checking clflush support
The BT instruction stores its result in CF and not ZF so use the correct jump instruction.
This fixes a hang in postcar on CPUs lacking support for this instruction. This concerns older pre-SSE2 hardware.
Change-Id: I704e3c579150fb9b9a292ef0e83050e7bf7cb078 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/arch/x86/exit_car.S 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/44922/2
ky0ko has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44922 )
Change subject: arch/x86/exit_car.S: Fix checking clflush support ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44922/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44922/1//COMMIT_MSG@10 PS1, Line 10: correct jump instruction.
Please add on what boards/chipsets this caused problems. […]
this would have broken any real hardware using cpus before the pentium4. this includes anything using the i440bx code, such as the p2b mainboard family, as well as the i440lx code i am working on.
qemu i440fx likely still worked because its current default cpu emulation supports the clcache instruction, despite that being anachronistic for that chipset.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44922 )
Change subject: arch/x86/exit_car.S: Fix checking clflush support ......................................................................
Patch Set 2:
(3 comments)
https://review.coreboot.org/c/coreboot/+/44922/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44922/1//COMMIT_MSG@9 PS1, Line 9: it
its
Done
https://review.coreboot.org/c/coreboot/+/44922/1//COMMIT_MSG@10 PS1, Line 10: correct jump instruction.
this would have broken any real hardware using cpus before the pentium4. […]
Done
https://review.coreboot.org/c/coreboot/+/44922/1//COMMIT_MSG@11 PS1, Line 11:
It’d be great, if you added a `Fixes:` tag.
Done
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44922 )
Change subject: arch/x86/exit_car.S: Fix checking clflush support ......................................................................
arch/x86/exit_car.S: Fix checking clflush support
The BT instruction stores its result in CF and not ZF so use the correct jump instruction.
This fixes a hang in postcar on CPUs lacking support for this instruction. This concerns older pre-SSE2 hardware.
Change-Id: I704e3c579150fb9b9a292ef0e83050e7bf7cb078 Signed-off-by: Arthur Heymans arthur@aheymans.xyz Reviewed-on: https://review.coreboot.org/c/coreboot/+/44922 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Patrick Rudolph siro@das-labor.org Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net --- M src/arch/x86/exit_car.S 1 file changed, 1 insertion(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Patrick Rudolph: Looks good to me, approved
diff --git a/src/arch/x86/exit_car.S b/src/arch/x86/exit_car.S index dc356b2..fae7899 100644 --- a/src/arch/x86/exit_car.S +++ b/src/arch/x86/exit_car.S @@ -59,7 +59,7 @@ movl $1, %eax cpuid btl $CPUID_FEATURE_CLFLUSH_BIT, %edx - jz skip_clflush + jnc skip_clflush clflush _cbmem_top_ptr
skip_clflush:
-
Arthur Heymans (Code Review) -
ky0ko (Code Review)
-
Patrick Georgi (Code Review)
-
Patrick Rudolph (Code Review)
-
Paul Menzel (Code Review)