Change in coreboot[master]: arch/x86/gdt: Work around assembler bug

List overview All Threads
Download

newer

older

Change in coreboot[master]: acpi:...

Change in coreboot[master]:...

Patrick Rudolph (Code Review)

5 Jul 2020 5 Jul '20

8:54 a.m.

Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43136 )

Change subject: arch/x86/gdt: Work around assembler bug ......................................................................

arch/x86/gdt: Work around assembler bug

The GDT loading did work fine a few month ago, but today it only works in QEMU, but not on real hardware or KVM enabled. This might be related to toolchain changes.

Use 64bit GDT loading on x86_64 and force the assembler to generate a 64bit address load on the GDT. This will make sure no 32bit (signed) displacement op is being generated, which points to the wrong address in longmode.

Veryfied using readelf and made sure no R_X86_64_32S relocation symbol is emitted. Disassembled the romstage ELF and made sure the GDT address is 64bit in size.

Tested on QEMU and KVM enabled QEMU: Doesn't crash any more on KVM.

Signed-off-by: Patrick Rudolph siro@das-labor.org

Change-Id: Ia824f90d9611e6e8db09bd62a05e6f990581f09a --- M src/arch/x86/assembly_entry.S M src/arch/x86/gdt_init.S 2 files changed, 24 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/43136/1

diff --git a/src/arch/x86/assembly_entry.S b/src/arch/x86/assembly_entry.S index fb48469..9ebc09d 100644 --- a/src/arch/x86/assembly_entry.S +++ b/src/arch/x86/assembly_entry.S @@ -15,12 +15,22 @@ #define _STACK_TOP _ecar_stack #endif

+#ifdef __x86_64__ +.code64 +#else +.code32 +#endif + .section ".text._start", "ax", @progbits .global _start _start:

/* Migrate GDT to this text segment */ +#if defined(__x86_64__) + call gdt_init64 +#else call gdt_init +#endif

/* reset stack pointer to CAR/EARLYRAM stack */ mov $_STACK_TOP, %esp diff --git a/src/arch/x86/gdt_init.S b/src/arch/x86/gdt_init.S index 7dd4b94..1558ac6 100644 --- a/src/arch/x86/gdt_init.S +++ b/src/arch/x86/gdt_init.S @@ -20,7 +20,20 @@ .section ".text._gdt64_", "ax", @progbits .globl gdt_init64 gdt_init64: - lgdt gdtptr64 + /* Workaround a bug in the assembler. + * The following code doesn't work: + * lgdt gdtptr64 + * + * The assembler tries to save memory by using 32bit displacement addressing mode. + * Displacements are using signed integers. + * This is fine in protected mode, as the negative address points to the correct + * address > 2GiB, but in long mode this doesn't work at all. + * Tests showed that QEMU can gracefully handle it, but real CPUs can't. + * + * Use the movabs pseudo instruction to force using a 64bit absolute address. + */ + movabs $gdtptr64, %rax + lgdt (%rax) ret

.previous

Attachments:

attachment.htm (text/html — 5.5 KB)

Show replies by date

Patrick Rudolph (Code Review)

5 Jul 5 Jul

8:56 a.m.

Patrick Rudolph has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/43136 )

Change subject: arch/x86/gdt: Work around assembler bug ......................................................................

arch/x86/gdt: Work around assembler bug

The GDT loading did work fine a few month ago, but today it only works in QEMU, but not on real hardware or KVM enabled. This might be related to toolchain changes.

Veryfied using readelf and made sure no R_X86_64_32S relocation symbol is emitted. Disassembled the romstage ELF and made sure the GDT address is 64bit in size.

Tested on QEMU and KVM enabled QEMU: Doesn't crash any more on KVM.

Signed-off-by: Patrick Rudolph siro@das-labor.org

Change-Id: Ia824f90d9611e6e8db09bd62a05e6f990581f09a --- M src/arch/x86/assembly_entry.S M src/arch/x86/gdt_init.S 2 files changed, 24 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/43136/2

Paul Menzel (Code Review)

9:04 a.m.

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43136 )

Change subject: arch/x86/gdt: Work around assembler bug ......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/43136/2//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/43136/2//COMMIT_MSG@25 PS2, Line 25: Please remove the blank line.

-- To view, visit https://review.coreboot.org/c/coreboot/+/43136 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia824f90d9611e6e8db09bd62a05e6f990581f09a Gerrit-Change-Number: 43136 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Sun, 05 Jul 2020 07:04:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Paul Menzel (Code Review)

9:05 a.m.

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43136 )

Change subject: arch/x86/gdt: Work around assembler bug ......................................................................

Patch Set 2:

Looks like an unrelated build error on *congenialbuilder*:

rm: cannot remove '/cb-build/coreboot-gerrit.0/chromeos/GOOGLE_SWANKY': Directory not empty

-- To view, visit https://review.coreboot.org/c/coreboot/+/43136 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia824f90d9611e6e8db09bd62a05e6f990581f09a Gerrit-Change-Number: 43136 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Sun, 05 Jul 2020 07:05:19 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Patrick Rudolph (Code Review)

10:46 a.m.

Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Angel Pons, Arthur Heymans, Kyösti Mälkki, Aaron Durbin,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/43136

to look at the new patch set (#3).

Change subject: arch/x86/gdt: Work around assembler bug ......................................................................

arch/x86/gdt: Work around assembler bug

The GDT loading did work fine on x86_64 a few month ago, but today it only works in QEMU, but not on real hardware or KVM enabled. This might be related to toolchain changes.

Veryfied using readelf and made sure no R_X86_64_32S relocation symbol is emitted. Disassembled the romstage ELF and made sure the GDT address is 64bit in size.

Tested on QEMU and KVM enabled QEMU: Doesn't crash any more on KVM.

Signed-off-by: Patrick Rudolph siro@das-labor.org Change-Id: Ia824f90d9611e6e8db09bd62a05e6f990581f09a --- M src/arch/x86/assembly_entry.S M src/arch/x86/gdt_init.S 2 files changed, 24 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/43136/3

-- To view, visit https://review.coreboot.org/c/coreboot/+/43136 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia824f90d9611e6e8db09bd62a05e6f990581f09a Gerrit-Change-Number: 43136 Gerrit-PatchSet: 3 Gerrit-Owner: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Patrick Rudolph (Code Review)

10:46 a.m.

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43136 )

Change subject: arch/x86/gdt: Work around assembler bug ......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/43136/2//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/43136/2//COMMIT_MSG@25 PS2, Line 25:

...

Please remove the blank line.

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/43136 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia824f90d9611e6e8db09bd62a05e6f990581f09a Gerrit-Change-Number: 43136 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Sun, 05 Jul 2020 08:46:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: comment

Arthur Heymans (Code Review)

1:50 p.m.

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43136 )

Change subject: arch/x86/gdt: Work around assembler bug ......................................................................

Patch Set 3: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/43136 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia824f90d9611e6e8db09bd62a05e6f990581f09a Gerrit-Change-Number: 43136 Gerrit-PatchSet: 3 Gerrit-Owner: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Sun, 05 Jul 2020 11:50:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Angel Pons (Code Review)

2:04 p.m.

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43136 )

Change subject: arch/x86/gdt: Work around assembler bug ......................................................................

Patch Set 3: Code-Review+2

(5 comments)

https://review.coreboot.org/c/coreboot/+/43136/3//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/43136/3//COMMIT_MSG@9 PS3, Line 9: month months

https://review.coreboot.org/c/coreboot/+/43136/3//COMMIT_MSG@10 PS3, Line 10: KVM enabled KVM-enabled QEMU

https://review.coreboot.org/c/coreboot/+/43136/3//COMMIT_MSG@18 PS3, Line 18: Veryfied Verified

https://review.coreboot.org/c/coreboot/+/43136/3//COMMIT_MSG@22 PS3, Line 22: KVM enabled QEMU KVM-enabled QEMU

https://review.coreboot.org/c/coreboot/+/43136/3/src/arch/x86/gdt_init.S File src/arch/x86/gdt_init.S:

https://review.coreboot.org/c/coreboot/+/43136/3/src/arch/x86/gdt_init.S@29 PS3, Line 29: protected mode that's 32-bit mode, right?

-- To view, visit https://review.coreboot.org/c/coreboot/+/43136 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia824f90d9611e6e8db09bd62a05e6f990581f09a Gerrit-Change-Number: 43136 Gerrit-PatchSet: 3 Gerrit-Owner: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Sun, 05 Jul 2020 12:04:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Patrick Rudolph (Code Review)

14 Aug 14 Aug

8 p.m.

Patrick Rudolph has uploaded a new patch set (#4) to the change originally created by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/43136 )

Change subject: arch/x86/gdt: Work around assembler bug ......................................................................

arch/x86/gdt: Work around assembler bug

The GDT loading did work fine on x86_64 a few months ago, but today it only works in QEMU, but not on real hardware or KVM-enabled QEMU. This might be related to toolchain changes.

Verified using readelf and made sure no R_X86_64_32S relocation symbol is emitted. Disassembled the romstage ELF and made sure the GDT address is 64bit in size.

Tested on QEMU and KVM-enabled QEMU: Doesn't crash any more on KVM.

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/43136/4

-- To view, visit https://review.coreboot.org/c/coreboot/+/43136 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia824f90d9611e6e8db09bd62a05e6f990581f09a Gerrit-Change-Number: 43136 Gerrit-PatchSet: 4 Gerrit-Owner: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph patrick.rudolph@9elements.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Patrick Rudolph (Code Review)

8:01 p.m.

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43136 )

Change subject: arch/x86/gdt: Work around assembler bug ......................................................................

Patch Set 4:

(5 comments)

https://review.coreboot.org/c/coreboot/+/43136/3//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/43136/3//COMMIT_MSG@9 PS3, Line 9: month

...

months

Done

https://review.coreboot.org/c/coreboot/+/43136/3//COMMIT_MSG@10 PS3, Line 10: KVM enabled

...

KVM-enabled QEMU

Done

https://review.coreboot.org/c/coreboot/+/43136/3//COMMIT_MSG@18 PS3, Line 18: Veryfied

...

Verified

Done

https://review.coreboot.org/c/coreboot/+/43136/3//COMMIT_MSG@22 PS3, Line 22: KVM enabled QEMU

...

KVM-enabled QEMU

Done

https://review.coreboot.org/c/coreboot/+/43136/3/src/arch/x86/gdt_init.S File src/arch/x86/gdt_init.S:

https://review.coreboot.org/c/coreboot/+/43136/3/src/arch/x86/gdt_init.S@29 PS3, Line 29: protected mode

...

that's 32-bit mode, right?

Yes.

-- To view, visit https://review.coreboot.org/c/coreboot/+/43136 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia824f90d9611e6e8db09bd62a05e6f990581f09a Gerrit-Change-Number: 43136 Gerrit-PatchSet: 4 Gerrit-Owner: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph patrick.rudolph@9elements.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Fri, 14 Aug 2020 18:01:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Gerrit-MessageType: comment

Philipp Deppenwiese (Code Review)

17 Aug 17 Aug

3:54 p.m.

Philipp Deppenwiese has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43136 )

Change subject: arch/x86/gdt: Work around assembler bug ......................................................................

Patch Set 4: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/43136 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia824f90d9611e6e8db09bd62a05e6f990581f09a Gerrit-Change-Number: 43136 Gerrit-PatchSet: 4 Gerrit-Owner: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph patrick.rudolph@9elements.com Gerrit-Reviewer: Philipp Deppenwiese zaolin.daisuki@gmail.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Mon, 17 Aug 2020 13:54:15 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Arthur Heymans (Code Review)

18 Aug 18 Aug

12:30 p.m.

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43136 )

Change subject: arch/x86/gdt: Work around assembler bug ......................................................................

Patch Set 4: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/43136 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia824f90d9611e6e8db09bd62a05e6f990581f09a Gerrit-Change-Number: 43136 Gerrit-PatchSet: 4 Gerrit-Owner: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph patrick.rudolph@9elements.com Gerrit-Reviewer: Philipp Deppenwiese zaolin.daisuki@gmail.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Tue, 18 Aug 2020 10:30:18 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Patrick Rudolph (Code Review)

19 Aug 19 Aug

11:12 a.m.

Hello Philipp Deppenwiese, build bot (Jenkins), Patrick Georgi, Martin Roth, Patrick Rudolph, Angel Pons, Arthur Heymans, Aaron Durbin,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/43136

to look at the new patch set (#5).

Change subject: arch/x86/gdt: Work around assembler bug ......................................................................

arch/x86/gdt: Work around assembler bug

The GDT loading did work fine on x86_64 a few months ago, but today it only works in QEMU, but not on real hardware or KVM-enabled QEMU. This might be related to toolchain changes.

Verified using readelf and made sure no R_X86_64_32S relocation symbol is emitted. Disassembled the romstage ELF and made sure the GDT address is 64bit in size.

Tested on QEMU and KVM-enabled QEMU: Doesn't crash any more on KVM.

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/43136/5

-- To view, visit https://review.coreboot.org/c/coreboot/+/43136 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia824f90d9611e6e8db09bd62a05e6f990581f09a Gerrit-Change-Number: 43136 Gerrit-PatchSet: 5 Gerrit-Owner: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph patrick.rudolph@9elements.com Gerrit-Reviewer: Philipp Deppenwiese zaolin.daisuki@gmail.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Patrick Rudolph (Code Review)

11:13 a.m.

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43136 )

Change subject: arch/x86/gdt: Work around assembler bug ......................................................................

Patch Set 5:

rebased due to merge conflict.

-- To view, visit https://review.coreboot.org/c/coreboot/+/43136 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia824f90d9611e6e8db09bd62a05e6f990581f09a Gerrit-Change-Number: 43136 Gerrit-PatchSet: 5 Gerrit-Owner: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph patrick.rudolph@9elements.com Gerrit-Reviewer: Philipp Deppenwiese zaolin.daisuki@gmail.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Wed, 19 Aug 2020 09:13:43 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Angel Pons (Code Review)

8:16 p.m.

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43136 )

Change subject: arch/x86/gdt: Work around assembler bug ......................................................................

Patch Set 5: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/43136 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia824f90d9611e6e8db09bd62a05e6f990581f09a Gerrit-Change-Number: 43136 Gerrit-PatchSet: 5 Gerrit-Owner: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph patrick.rudolph@9elements.com Gerrit-Reviewer: Philipp Deppenwiese zaolin.daisuki@gmail.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Wed, 19 Aug 2020 18:16:34 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Angel Pons (Code Review)

8:20 p.m.

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43136 )

Change subject: arch/x86/gdt: Work around assembler bug ......................................................................

Patch Set 5:

(1 comment)

https://review.coreboot.org/c/coreboot/+/43136/5/src/arch/x86/assembly_entry... File src/arch/x86/assembly_entry.S:

https://review.coreboot.org/c/coreboot/+/43136/5/src/arch/x86/assembly_entry... PS5, Line 29: #if defined(__x86_64__) For consistency with the lines above:

#ifdef __x86_64__

-- To view, visit https://review.coreboot.org/c/coreboot/+/43136 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia824f90d9611e6e8db09bd62a05e6f990581f09a Gerrit-Change-Number: 43136 Gerrit-PatchSet: 5 Gerrit-Owner: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph patrick.rudolph@9elements.com Gerrit-Reviewer: Philipp Deppenwiese zaolin.daisuki@gmail.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Wed, 19 Aug 2020 18:20:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Patrick Rudolph (Code Review)

20 Aug 20 Aug

3:58 p.m.

Patrick Rudolph has uploaded a new patch set (#6) to the change originally created by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/43136 )

Change subject: arch/x86/gdt: Work around assembler bug ......................................................................

arch/x86/gdt: Work around assembler bug

The GDT loading did work fine on x86_64 a few months ago, but today it only works in QEMU, but not on real hardware or KVM-enabled QEMU. This might be related to toolchain changes.

Verified using readelf and made sure no R_X86_64_32S relocation symbol is emitted. Disassembled the romstage ELF and made sure the GDT address is 64bit in size.

Tested on QEMU and KVM-enabled QEMU: Doesn't crash any more on KVM.

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/43136/6

-- To view, visit https://review.coreboot.org/c/coreboot/+/43136 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia824f90d9611e6e8db09bd62a05e6f990581f09a Gerrit-Change-Number: 43136 Gerrit-PatchSet: 6 Gerrit-Owner: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph patrick.rudolph@9elements.com Gerrit-Reviewer: Philipp Deppenwiese zaolin.daisuki@gmail.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Patrick Rudolph (Code Review)

3:58 p.m.

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43136 )

Change subject: arch/x86/gdt: Work around assembler bug ......................................................................

Patch Set 5:

(1 comment)

https://review.coreboot.org/c/coreboot/+/43136/5/src/arch/x86/assembly_entry... File src/arch/x86/assembly_entry.S:

https://review.coreboot.org/c/coreboot/+/43136/5/src/arch/x86/assembly_entry... PS5, Line 29: #if defined(__x86_64__)

...

For consistency with the lines above: […]

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/43136 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia824f90d9611e6e8db09bd62a05e6f990581f09a Gerrit-Change-Number: 43136 Gerrit-PatchSet: 5 Gerrit-Owner: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph patrick.rudolph@9elements.com Gerrit-Reviewer: Philipp Deppenwiese zaolin.daisuki@gmail.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 20 Aug 2020 13:58:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Gerrit-MessageType: comment

Angel Pons (Code Review)

6:26 p.m.

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43136 )

Change subject: arch/x86/gdt: Work around assembler bug ......................................................................

Patch Set 6: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/43136 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia824f90d9611e6e8db09bd62a05e6f990581f09a Gerrit-Change-Number: 43136 Gerrit-PatchSet: 6 Gerrit-Owner: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph patrick.rudolph@9elements.com Gerrit-Reviewer: Philipp Deppenwiese zaolin.daisuki@gmail.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 20 Aug 2020 16:26:47 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Patrick Rudolph (Code Review)

21 Aug 21 Aug

6:01 p.m.

Patrick Rudolph has submitted this change. ( https://review.coreboot.org/c/coreboot/+/43136 )

Change subject: arch/x86/gdt: Work around assembler bug ......................................................................

arch/x86/gdt: Work around assembler bug

The GDT loading did work fine on x86_64 a few months ago, but today it only works in QEMU, but not on real hardware or KVM-enabled QEMU. This might be related to toolchain changes.

Verified using readelf and made sure no R_X86_64_32S relocation symbol is emitted. Disassembled the romstage ELF and made sure the GDT address is 64bit in size.

Tested on QEMU and KVM-enabled QEMU: Doesn't crash any more on KVM.

Signed-off-by: Patrick Rudolph siro@das-labor.org Change-Id: Ia824f90d9611e6e8db09bd62a05e6f990581f09a Reviewed-on: https://review.coreboot.org/c/coreboot/+/43136 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Angel Pons th3fanbus@gmail.com --- M src/arch/x86/assembly_entry.S M src/arch/x86/gdt_init.S 2 files changed, 24 insertions(+), 1 deletion(-)

Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved

diff --git a/src/arch/x86/assembly_entry.S b/src/arch/x86/assembly_entry.S index 0d8307b..31670c2 100644 --- a/src/arch/x86/assembly_entry.S +++ b/src/arch/x86/assembly_entry.S @@ -15,12 +15,22 @@ #define _STACK_TOP _ecar_stack #endif

+#ifdef __x86_64__ +.code64 +#else +.code32 +#endif + .section ".text._start", "ax", @progbits .global _start _start:

/* Migrate GDT to this text segment */ +#ifdef __x86_64__ + call gdt_init64 +#else call gdt_init +#endif

.previous

-- To view, visit https://review.coreboot.org/c/coreboot/+/43136 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia824f90d9611e6e8db09bd62a05e6f990581f09a Gerrit-Change-Number: 43136 Gerrit-PatchSet: 7 Gerrit-Owner: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph patrick.rudolph@9elements.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Philipp Deppenwiese zaolin.daisuki@gmail.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: merged

9elements QA (Code Review)

6:27 p.m.

9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43136 )

Change subject: arch/x86/gdt: Work around assembler bug ......................................................................

Patch Set 7:

Automatic boot test returned (PASS/FAIL/TOTAL): 6/1/7 "QEMU x86 q35/ich9" (x86_32) using payload TianoCore : SUCCESS : https://lava.9esec.io/r/16076 "QEMU x86 q35/ich9" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/16075 "QEMU x86 i440fx/piix4" (x86_64) using payload SeaBIOS : FAIL : https://lava.9esec.io/r/16074 "QEMU x86 i440fx/piix4" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/16073 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/16072 "HP Compaq 8200 Elite SFF PC" (x86_32) using payload TianoCore : SUCCESS : https://lava.9esec.io/r/16078 "HP Compaq 8200 Elite SFF PC" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/16077

Please note: This test is under development and might not be accurate at all!

-- To view, visit https://review.coreboot.org/c/coreboot/+/43136 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia824f90d9611e6e8db09bd62a05e6f990581f09a Gerrit-Change-Number: 43136 Gerrit-PatchSet: 7 Gerrit-Owner: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph patrick.rudolph@9elements.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Philipp Deppenwiese zaolin.daisuki@gmail.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: 9elements QA hardwaretestrobot@gmail.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Fri, 21 Aug 2020 16:27:43 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1608

days inactive

1655

days old

coreboot-gerrit@coreboot.org

20 comments

6 participants

tags (0)

participants (6)

9elements QA (Code Review)
Angel Pons (Code Review)
Arthur Heymans (Code Review)
Patrick Rudolph (Code Review)
Paul Menzel (Code Review)
Philipp Deppenwiese (Code Review)