Change in coreboot[master]: arch/x86: x86_64 implies SSE2 support

Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44869 ) Change subject: arch/x86: x86_64 implies SSE2 support ...................................................................... arch/x86: x86_64 implies SSE2 support Enable SSE2 when compiling for x86_64. Tested on qemu. Change-Id: I3cdc584c97016e15513df663a54a7bdb549a73e4 Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com> --- M src/arch/x86/Kconfig M src/arch/x86/bootblock_crt0.S 2 files changed, 11 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/44869/1 diff --git a/src/arch/x86/Kconfig b/src/arch/x86/Kconfig index d906436..d0e4693 100644 --- a/src/arch/x86/Kconfig +++ b/src/arch/x86/Kconfig @@ -33,22 +33,27 @@ config ARCH_BOOTBLOCK_X86_64 bool select ARCH_X86 + select SSE2 config ARCH_VERSTAGE_X86_64 bool select ARCH_X86 + select SSE2 config ARCH_ROMSTAGE_X86_64 bool select ARCH_X86 + select SSE2 config ARCH_POSTCAR_X86_64 bool default ARCH_ROMSTAGE_X86_64 && POSTCAR_STAGE + select SSE2 config ARCH_RAMSTAGE_X86_64 bool select ARCH_X86 + select SSE2 if ARCH_X86 diff --git a/src/arch/x86/bootblock_crt0.S b/src/arch/x86/bootblock_crt0.S index 9f45413..9e17b7e 100644 --- a/src/arch/x86/bootblock_crt0.S +++ b/src/arch/x86/bootblock_crt0.S @@ -52,9 +52,15 @@ #if CONFIG(SSE) enable_sse: +#if ENV_X86_32 mov %cr4, %eax or $CR4_OSFXSR, %ax mov %eax, %cr4 +#else + mov %cr4, %rax + or $CR4_OSFXSR, %ax + mov %rax, %cr4 +#endif #endif /* CONFIG(SSE) */ /* We're done. Now it's up to platform-specific code */ -- To view, visit https://review.coreboot.org/c/coreboot/+/44869 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3cdc584c97016e15513df663a54a7bdb549a73e4 Gerrit-Change-Number: 44869 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-MessageType: newchange

Attention is currently required from: Arthur Heymans. Arthur Heymans has uploaded a new patch set (#2) to the change originally created by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/44869 ) Change subject: arch/x86: x86_64 implies SSE2 support ...................................................................... arch/x86: x86_64 implies SSE2 support Enable SSE2 when compiling for x86_64. Tested on qemu. Change-Id: I3cdc584c97016e15513df663a54a7bdb549a73e4 Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com> Signed-off-by: Arthur Heymans <arthur@aheymans.xyz> --- M src/arch/x86/Kconfig 1 file changed, 20 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/44869/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/44869 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3cdc584c97016e15513df663a54a7bdb549a73e4 Gerrit-Change-Number: 44869 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Attention: Arthur Heymans <arthur@aheymans.xyz> Gerrit-MessageType: newpatchset

Attention is currently required from: Patrick Rudolph, Arthur Heymans. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44869 ) Change subject: arch/x86: x86_64 implies SSE2 support ...................................................................... Patch Set 2: Code-Review+1 (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/44869/comment/1178b5d5_42214ef7 PS2, Line 11: Tested on qemu. Could you add the information from CB:68648 please? Test results are the same with this patch. -- To view, visit https://review.coreboot.org/c/coreboot/+/44869 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3cdc584c97016e15513df663a54a7bdb549a73e4 Gerrit-Change-Number: 44869 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-Attention: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Comment-Date: Fri, 21 Oct 2022 12:29:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Attention is currently required from: Patrick Rudolph, Angel Pons. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44869 ) Change subject: arch/x86: x86_64 implies SSE2 support ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/44869/comment/009fedd3_8ec71bd2 PS2, Line 11: Tested on qemu.
Could you add the information from CB:68648 please? Test results are the same with this patch.
It's not specific to that patch. Other things are broken too without this. Compilers just assume 64bit == SSE2 available. -- To view, visit https://review.coreboot.org/c/coreboot/+/44869 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3cdc584c97016e15513df663a54a7bdb549a73e4 Gerrit-Change-Number: 44869 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-Attention: Angel Pons <th3fanbus@gmail.com> Gerrit-Comment-Date: Fri, 21 Oct 2022 12:37:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus@gmail.com> Gerrit-MessageType: comment

Attention is currently required from: Patrick Rudolph, Angel Pons. Arthur Heymans has uploaded a new patch set (#3) to the change originally created by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/44869 ) Change subject: arch/x86: x86_64 implies SSE2 support ...................................................................... arch/x86: x86_64 implies SSE2 support Enable SSE2 (and SSE) when compiling for x86_64. Compilers often assume SSE2 is present and enabled when targeting x86_64. This fixes: - lzma decompression code is compiled with the -Ofast flag - 'everything' when compiling with clang. This mostly affects qemu targets, which did not have this flag selected yet. TESTED on qemu. Change-Id: I3cdc584c97016e15513df663a54a7bdb549a73e4 Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com> Signed-off-by: Arthur Heymans <arthur@aheymans.xyz> --- M src/arch/x86/Kconfig 1 file changed, 28 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/44869/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/44869 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3cdc584c97016e15513df663a54a7bdb549a73e4 Gerrit-Change-Number: 44869 Gerrit-PatchSet: 3 Gerrit-Owner: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-Attention: Angel Pons <th3fanbus@gmail.com> Gerrit-MessageType: newpatchset

Attention is currently required from: Patrick Rudolph, Angel Pons, Arthur Heymans. Elyes Haouas has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44869 ) Change subject: arch/x86: x86_64 implies SSE2 support ...................................................................... Patch Set 3: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/44869 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3cdc584c97016e15513df663a54a7bdb549a73e4 Gerrit-Change-Number: 44869 Gerrit-PatchSet: 3 Gerrit-Owner: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Elyes Haouas <ehaouas@noos.fr> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-Attention: Angel Pons <th3fanbus@gmail.com> Gerrit-Attention: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Comment-Date: Fri, 21 Oct 2022 12:49:19 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Attention is currently required from: Patrick Rudolph, Arthur Heymans. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44869 ) Change subject: arch/x86: x86_64 implies SSE2 support ...................................................................... Patch Set 3: Code-Review+1 (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/44869/comment/5f3113cf_21c75aa1 PS2, Line 11: Tested on qemu.
Could you add the information from CB:68648 please? Test results are the same with this patch. […] Welp, your reply sounded like you didn't want to update the commit message, so we rewrote the entire message as a suggestion... 😅
``` arch/x86: Enable SSE2 support on x86_64 Enable SSE2 when compiling for x86_64, as compilers assume SSE2 is available and many things will break if it's not the case. For example, commit d91f3a4eaf7f8399db83a9c2d5cba2452fefaf7a ("lib/lzma: Build the source for decompression with flag -Ofast") enabled the -Ofast optimizations for LZMA at build time, but these break loading compressed stages on at least QEMU i440fx when using 64-bit coreboot, which results in a boot loop when attempting to load ramstage. TEST=Ensure that `configs/config.emulation_qemu_x86_i440fx_x86_64` is is able to reach the payload instead of boot-looping when trying to load ramstage. Payload-specific functionality not tested. ``` If you like it, feel free to use it. Maybe add a paragraph about the issues with clang? [And don't mind the wrapping, it's under 72 characters but Gerrit wraps the comments...] -- To view, visit https://review.coreboot.org/c/coreboot/+/44869 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3cdc584c97016e15513df663a54a7bdb549a73e4 Gerrit-Change-Number: 44869 Gerrit-PatchSet: 3 Gerrit-Owner: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Elyes Haouas <ehaouas@noos.fr> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-Attention: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Comment-Date: Fri, 21 Oct 2022 12:49:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Angel Pons <th3fanbus@gmail.com> Comment-In-Reply-To: Arthur Heymans <arthur@aheymans.xyz> Gerrit-MessageType: comment

Attention is currently required from: Patrick Rudolph, Arthur Heymans. Elyes Haouas has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44869 ) Change subject: arch/x86: x86_64 implies SSE2 support ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/44869 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3cdc584c97016e15513df663a54a7bdb549a73e4 Gerrit-Change-Number: 44869 Gerrit-PatchSet: 3 Gerrit-Owner: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Elyes Haouas <ehaouas@noos.fr> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-Attention: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Comment-Date: Sun, 23 Oct 2022 04:53:17 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Arthur Heymans has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44869 ) Change subject: arch/x86: x86_64 implies SSE2 support ...................................................................... arch/x86: x86_64 implies SSE2 support Enable SSE2 (and SSE) when compiling for x86_64. Compilers often assume SSE2 is present and enabled when targeting x86_64. This fixes: - lzma decompression code is compiled with the -Ofast flag - 'everything' when compiling with clang. This mostly affects qemu targets, which did not have this flag selected yet. TESTED on qemu. Change-Id: I3cdc584c97016e15513df663a54a7bdb549a73e4 Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com> Signed-off-by: Arthur Heymans <arthur@aheymans.xyz> Reviewed-on: https://review.coreboot.org/c/coreboot/+/44869 Reviewed-by: Angel Pons <th3fanbus@gmail.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Elyes Haouas <ehaouas@noos.fr> --- M src/arch/x86/Kconfig 1 file changed, 32 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Elyes Haouas: Looks good to me, approved Angel Pons: Looks good to me, but someone else must approve diff --git a/src/arch/x86/Kconfig b/src/arch/x86/Kconfig index c00eb0c..b60f600 100644 --- a/src/arch/x86/Kconfig +++ b/src/arch/x86/Kconfig @@ -39,19 +39,24 @@ config ARCH_BOOTBLOCK_X86_64 bool + select SSE2 config ARCH_VERSTAGE_X86_64 bool + select SSE2 config ARCH_ROMSTAGE_X86_64 bool + select SSE2 config ARCH_POSTCAR_X86_64 bool default ARCH_ROMSTAGE_X86_64 && POSTCAR_STAGE + select SSE2 config ARCH_RAMSTAGE_X86_64 bool + select SSE2 config ARCH_ALL_STAGES_X86_64 bool -- To view, visit https://review.coreboot.org/c/coreboot/+/44869 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3cdc584c97016e15513df663a54a7bdb549a73e4 Gerrit-Change-Number: 44869 Gerrit-PatchSet: 4 Gerrit-Owner: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Elyes Haouas <ehaouas@noos.fr> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-MessageType: merged

9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44869 ) Change subject: arch/x86: x86_64 implies SSE2 support ...................................................................... Patch Set 4: Automatic boot test returned (PASS/FAIL/TOTAL): 14 / 2 / 16 FAIL: x86_32 "Hermes CFL" , build config PRODRIVE_HERMES_ and payload TianoCore_UefiPayloadPkg : https://lava.9esec.io/r/130455 FAIL: x86_32 "Hermes CFL" , build config PRODRIVE_HERMES and payload TianoCore_UefiPayloadPkg : https://lava.9esec.io/r/130454 PASS: x86_32 "ThinkPad T500" , build config LENOVO_T500 and payload SeaBIOS : https://lava.9esec.io/r/130453 PASS: x86_32 "HP Z220 SFF Workstation" , build config HP_Z220_SFF_WORKSTATION and payload LinuxBoot_BB_kexec : https://lava.9esec.io/r/130452 PASS: x86_64 "HP Compaq 8200 Elite SFF PC" , build config HP_COMPAQ_8200_ELITE_SFF_PC.X86_64 and payload TianoCore : https://lava.9esec.io/r/130451 PASS: x86_64 "HP Compaq 8200 Elite SFF PC" , build config HP_COMPAQ_8200_ELITE_SFF_PC.X86_64 and payload SeaBIOS : https://lava.9esec.io/r/130450 PASS: x86_32 "HP Compaq 8200 Elite SFF PC" , build config HP_COMPAQ_8200_ELITE_SFF_PC and payload TianoCore : https://lava.9esec.io/r/130449 PASS: x86_32 "HP Compaq 8200 Elite SFF PC" , build config HP_COMPAQ_8200_ELITE_SFF_PC and payload SeaBIOS : https://lava.9esec.io/r/130448 PASS: x86_32 "QEMU x86 q35/ich9" , build config EMULATION_QEMU_X86_Q35_SMM_TSEG and payload TianoCore : https://lava.9esec.io/r/130447 PASS: x86_32 "QEMU x86 q35/ich9" , build config EMULATION_QEMU_X86_Q35_SMM_TSEG and payload SeaBIOS : https://lava.9esec.io/r/130446 PASS: x86_32 "QEMU x86 q35/ich9" , build config EMULATION_QEMU_X86_Q35 and payload TianoCore : https://lava.9esec.io/r/130445 PASS: x86_32 "QEMU x86 q35/ich9" , build config EMULATION_QEMU_X86_Q35 and payload SeaBIOS : https://lava.9esec.io/r/130444 PASS: x86_64 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX_X86_64 and payload SeaBIOS : https://lava.9esec.io/r/130443 PASS: x86_32 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX_ASAN and payload SeaBIOS : https://lava.9esec.io/r/130442 PASS: x86_32 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX_ and payload SeaBIOS : https://lava.9esec.io/r/130441 PASS: x86_32 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX and payload SeaBIOS : https://lava.9esec.io/r/130440 Please note: This test is under development and might not be accurate at all! -- To view, visit https://review.coreboot.org/c/coreboot/+/44869 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3cdc584c97016e15513df663a54a7bdb549a73e4 Gerrit-Change-Number: 44869 Gerrit-PatchSet: 4 Gerrit-Owner: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Elyes Haouas <ehaouas@noos.fr> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: 9elements QA <hardwaretestrobot@gmail.com> Gerrit-Comment-Date: Mon, 24 Oct 2022 13:59:04 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
participants (5)
-
9elements QA (Code Review)
-
Angel Pons (Code Review)
-
Arthur Heymans (Code Review)
-
Elyes Haouas (Code Review)
-
Patrick Rudolph (Code Review)