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

[Patrick Rudolph (Code Review)]

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 <[e-mail]>
---
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 <[e-mail]>
Gerrit-MessageType: newchange

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

[Arthur Heymans (Code Review)]

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 <[e-mail]>
Signed-off-by: Arthur Heymans <[e-mail]>
---
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 <[e-mail]>
Gerrit-Reviewer: Arthur Heymans <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-Attention: Arthur Heymans <[e-mail]>
Gerrit-MessageType: newpatchset

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

[Angel Pons (Code Review)]

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 <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Arthur Heymans <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-Attention: Patrick Rudolph <[e-mail]>
Gerrit-Attention: Arthur Heymans <[e-mail]>
Gerrit-Comment-Date: Fri, 21 Oct 2022 12:29:36 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

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

[Arthur Heymans (Code Review)]

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 <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Arthur Heymans <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-Attention: Patrick Rudolph <[e-mail]>
Gerrit-Attention: Angel Pons <[e-mail]>
Gerrit-Comment-Date: Fri, 21 Oct 2022 12:37:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <[e-mail]>
Gerrit-MessageType: comment

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

[Arthur Heymans (Code Review)]

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 <[e-mail]>
Signed-off-by: Arthur Heymans <[e-mail]>
---
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 <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Arthur Heymans <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-Attention: Patrick Rudolph <[e-mail]>
Gerrit-Attention: Angel Pons <[e-mail]>
Gerrit-MessageType: newpatchset

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

[Elyes Haouas (Code Review)]

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 <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Arthur Heymans <[e-mail]>
Gerrit-Reviewer: Elyes Haouas <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-Attention: Patrick Rudolph <[e-mail]>
Gerrit-Attention: Angel Pons <[e-mail]>
Gerrit-Attention: Arthur Heymans <[e-mail]>
Gerrit-Comment-Date: Fri, 21 Oct 2022 12:49:19 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

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

[Angel Pons (Code Review)]

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 <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Arthur Heymans <[e-mail]>
Gerrit-Reviewer: Elyes Haouas <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-Attention: Patrick Rudolph <[e-mail]>
Gerrit-Attention: Arthur Heymans <[e-mail]>
Gerrit-Comment-Date: Fri, 21 Oct 2022 12:49:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Angel Pons <[e-mail]>
Comment-In-Reply-To: Arthur Heymans <[e-mail]>
Gerrit-MessageType: comment

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

[Elyes Haouas (Code Review)]

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 <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Arthur Heymans <[e-mail]>
Gerrit-Reviewer: Elyes Haouas <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-Attention: Patrick Rudolph <[e-mail]>
Gerrit-Attention: Arthur Heymans <[e-mail]>
Gerrit-Comment-Date: Sun, 23 Oct 2022 04:53:17 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

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

[Arthur Heymans (Code Review)]

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 <[e-mail]>
Signed-off-by: Arthur Heymans <[e-mail]>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/44869
Reviewed-by: Angel Pons <[e-mail]>
Tested-by: build bot (Jenkins) <[e-mail]>
Reviewed-by: Elyes Haouas <[e-mail]>
---
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 <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Arthur Heymans <[e-mail]>
Gerrit-Reviewer: Elyes Haouas <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-MessageType: merged

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

[9elements QA (Code Review)]

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 <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Arthur Heymans <[e-mail]>
Gerrit-Reviewer: Elyes Haouas <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: 9elements QA <[e-mail]>
Gerrit-Comment-Date: Mon, 24 Oct 2022 13:59:04 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment