Change in coreboot[master]: cpu/intel/car/non-evict/cache_as_ram.S: Add support for longmode

List overview All Threads
Download

newer

older

Change in coreboot[master]:...

Patrick Rudolph (Code Review)

21 Aug 2020 21 Aug '20

4:04 p.m.

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

Change subject: cpu/intel/car/non-evict/cache_as_ram.S: Add support for longmode ......................................................................

cpu/intel/car/non-evict/cache_as_ram.S: Add support for longmode

* Use heap for linker script calculated constant to fix relocation symbols in mixed assembly code. * Work around a strange bug that causes the CPU to halt when accessing MMX registers in long mode

Tested on HPZ220: * Still boots in x86_32.

Change-Id: I3e72a0bebf728fb678308006ea3a3aeb92910a84 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/cpu/intel/car/non-evict/cache_as_ram.S 1 file changed, 40 insertions(+), 13 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/44673/1

diff --git a/src/cpu/intel/car/non-evict/cache_as_ram.S b/src/cpu/intel/car/non-evict/cache_as_ram.S index b24f101..481cbee 100644 --- a/src/cpu/intel/car/non-evict/cache_as_ram.S +++ b/src/cpu/intel/car/non-evict/cache_as_ram.S @@ -9,6 +9,27 @@

.global bootblock_pre_c_entry

+/* Trick the linker into supporting x86_64 relocations in 32bit code */ +#ifdef __x86_64__ +rom_mtrr_mask: +.quad _rom_mtrr_mask + +rom_mtrr_base: +.quad _rom_mtrr_base + +car_mtrr_mask: +.quad _car_mtrr_mask +#else +rom_mtrr_mask: +.long _rom_mtrr_mask + +rom_mtrr_base: +.long _rom_mtrr_base + +car_mtrr_mask: +.long _car_mtrr_mask +#endif + .code32 _cache_as_ram_setup:

@@ -83,7 +104,6 @@ movl $MTRR_PHYS_MASK(1), %ecx wrmsr

- post_code(0x23) /* Set Cache-as-RAM base address. */ movl $(MTRR_PHYS_BASE(0)), %ecx @@ -96,20 +116,20 @@ /* Set Cache-as-RAM mask. */ movl $(MTRR_PHYS_MASK(0)), %ecx rdmsr - movl $_car_mtrr_mask, %eax + mov car_mtrr_mask, %eax orl $MTRR_PHYS_MASK_VALID, %eax wrmsr

/* Enable cache for our code in Flash because we do XIP here */ movl $MTRR_PHYS_BASE(1), %ecx xorl %edx, %edx - movl $_rom_mtrr_base, %eax + mov rom_mtrr_base, %eax orl $MTRR_TYPE_WRPROT, %eax wrmsr

movl $MTRR_PHYS_MASK(1), %ecx rdmsr - movl $_rom_mtrr_mask, %eax + mov rom_mtrr_mask, %eax orl $MTRR_PHYS_MASK_VALID, %eax wrmsr

@@ -208,17 +228,24 @@ the pushes below. */ andl $0xfffffff0, %esp

- /* entry64.inc preserves ebx. */ +#ifdef __x86_64__ + /* + * BUG: On some x86 platforms acessing MMX register in long mode + * causes a crash. Retrieve them here and then switch to long mode. + */ + movd %mm2, %eax + push %eax + movd %mm1, %eax /* tsc */ + push %eax + movd %mm0, %eax + push %eax + xorl %eax, %eax + push %eax /* BIST */ + #include <cpu/x86/64bit/entry64.inc>

-#ifdef __x86_64__ - - movd %mm2, %rdi - shld %rdi, 32 - movd %mm1, %rsi - or %rsi, %rdi /* tsc */ - - movd %mm0, %rsi /* BIST */ + pop %rsi /* BIST */ + pop %rdi /* tsc */ #else subl $4, %esp /* push TSC and BIST to stack */

Attachments:

attachment.htm (text/html — 7.5 KB)

Show replies by date

build bot (Jenkins) (Code Review)

21 Aug 21 Aug

4:12 p.m.

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44673 )

Change subject: cpu/intel/car/non-evict/cache_as_ram.S: Add support for longmode ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44673/1/src/cpu/intel/car/non-evict... File src/cpu/intel/car/non-evict/cache_as_ram.S:

https://review.coreboot.org/c/coreboot/+/44673/1/src/cpu/intel/car/non-evict... PS1, Line 233: * BUG: On some x86 platforms acessing MMX register in long mode 'acessing' may be misspelled - perhaps 'accessing'?

build bot (Jenkins) (Code Review)

24 Aug 24 Aug

7:58 a.m.

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44673 )

Change subject: cpu/intel/car/non-evict/cache_as_ram.S: Add support for longmode ......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44673/2/src/cpu/intel/car/non-evict... File src/cpu/intel/car/non-evict/cache_as_ram.S:

https://review.coreboot.org/c/coreboot/+/44673/2/src/cpu/intel/car/non-evict... PS2, Line 233: * BUG: On some x86 platforms acessing MMX register in long mode 'acessing' may be misspelled - perhaps 'accessing'?

Patrick Rudolph (Code Review)

9 Sep 9 Sep

5:32 a.m.

Hello build bot (Jenkins), Patrick Rudolph,

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

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

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

Change subject: cpu/intel/car/non-evict/cache_as_ram.S: Add support for longmode ......................................................................

cpu/intel/car/non-evict/cache_as_ram.S: Add support for longmode

* Use heap for linker script calculated constant to fix relocation symbols in mixed assembly code. * Work around a strange bug that causes the CPU to halt when accessing MMX registers in long mode

Tested on HPZ220: * Still boots in x86_32.

Tested on Lenovo T410: * Doesn't need the MMX register fix in long mode.

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/44673/3

build bot (Jenkins) (Code Review)

5:44 a.m.

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44673 )

Change subject: cpu/intel/car/non-evict/cache_as_ram.S: Add support for longmode ......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44673/3/src/cpu/intel/car/non-evict... File src/cpu/intel/car/non-evict/cache_as_ram.S:

https://review.coreboot.org/c/coreboot/+/44673/3/src/cpu/intel/car/non-evict... PS3, Line 233: * BUG: On some x86 platforms acessing MMX register in long mode 'acessing' may be misspelled - perhaps 'accessing'?

Angel Pons (Code Review)

9:37 a.m.

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

Change subject: cpu/intel/car/non-evict/cache_as_ram.S: Add support for longmode ......................................................................

Patch Set 3: Code-Review+1

(2 comments)

https://review.coreboot.org/c/coreboot/+/44673/3/src/cpu/intel/car/non-evict... File src/cpu/intel/car/non-evict/cache_as_ram.S:

https://review.coreboot.org/c/coreboot/+/44673/3/src/cpu/intel/car/non-evict... PS3, Line 233: * BUG: On some x86 platforms acessing MMX register in long mode

...

'acessing' may be misspelled - perhaps 'accessing'?

Would be good to fix

https://review.coreboot.org/c/coreboot/+/44673/3/src/cpu/intel/car/non-evict... PS3, Line 236: A single space should be enough

-- To view, visit https://review.coreboot.org/c/coreboot/+/44673 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3e72a0bebf728fb678308006ea3a3aeb92910a84 Gerrit-Change-Number: 44673 Gerrit-PatchSet: 3 Gerrit-Owner: Patrick Rudolph patrick.rudolph@9elements.com Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Wed, 09 Sep 2020 09:37:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: comment

Arthur Heymans (Code Review)

10 Sep 10 Sep

7:23 a.m.

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

Change subject: cpu/intel/car/non-evict/cache_as_ram.S: Add support for longmode ......................................................................

Patch Set 3: Code-Review+1

(3 comments)

https://review.coreboot.org/c/coreboot/+/44673/3/src/cpu/intel/car/non-evict... File src/cpu/intel/car/non-evict/cache_as_ram.S:

https://review.coreboot.org/c/coreboot/+/44673/3/src/cpu/intel/car/non-evict... PS3, Line 12: /* Trick the linker into supporting x86_64 relocations in 32bit code */ : #ifdef __x86_64__ : rom_mtrr_mask: : .quad _rom_mtrr_mask : : rom_mtrr_base: : .quad _rom_mtrr_base : : car_mtrr_mask: : .quad _car_mtrr_mask : #else : rom_mtrr_mask: : .long _rom_mtrr_mask : : rom_mtrr_base: : .long _rom_mtrr_base : : car_mtrr_mask: : .long _car_mtrr_mask : #endif I think it would be a good idea to put these in a separatly linked file (with .global), to reduce boilerplate in other CAR setup files.

https://review.coreboot.org/c/coreboot/+/44673/3/src/cpu/intel/car/non-evict... PS3, Line 110: _car_mtrr_start Maybe be consistent about this one?

https://review.coreboot.org/c/coreboot/+/44673/3/src/cpu/intel/car/non-evict... PS3, Line 236:

...

A single space should be enough

This is the string cpuid reports in 0x80000002-0x80000004.

-- To view, visit https://review.coreboot.org/c/coreboot/+/44673 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3e72a0bebf728fb678308006ea3a3aeb92910a84 Gerrit-Change-Number: 44673 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: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 10 Sep 2020 07:23:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Gerrit-MessageType: comment

Angel Pons (Code Review)

7:38 a.m.

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

Change subject: cpu/intel/car/non-evict/cache_as_ram.S: Add support for longmode ......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44673/3/src/cpu/intel/car/non-evict... File src/cpu/intel/car/non-evict/cache_as_ram.S:

https://review.coreboot.org/c/coreboot/+/44673/3/src/cpu/intel/car/non-evict... PS3, Line 236:

...

...
A single space should be enough […]

Ack

-- To view, visit https://review.coreboot.org/c/coreboot/+/44673 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3e72a0bebf728fb678308006ea3a3aeb92910a84 Gerrit-Change-Number: 44673 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: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 10 Sep 2020 07:38:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Comment-In-Reply-To: Arthur Heymans arthur@aheymans.xyz Gerrit-MessageType: comment

Patrick Rudolph (Code Review)

24 Sep 24 Sep

5 p.m.

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

Change subject: cpu/intel/car/non-evict/cache_as_ram.S: Add support for longmode ......................................................................

Patch Set 3:

(2 comments)

https://review.coreboot.org/c/coreboot/+/44673/3/src/cpu/intel/car/non-evict... File src/cpu/intel/car/non-evict/cache_as_ram.S:

...

I think it would be a good idea to put these in a separatly linked file (with . […]

Done

https://review.coreboot.org/c/coreboot/+/44673/3/src/cpu/intel/car/non-evict... PS3, Line 110: _car_mtrr_start

...

Maybe be consistent about this one?

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/44673 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3e72a0bebf728fb678308006ea3a3aeb92910a84 Gerrit-Change-Number: 44673 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: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 24 Sep 2020 17:00:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Arthur Heymans arthur@aheymans.xyz Gerrit-MessageType: comment

Patrick Rudolph (Code Review)

5:13 p.m.

Hello build bot (Jenkins), Angel Pons, Arthur Heymans, Patrick Rudolph,

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

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

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

Change subject: cpu/intel/car/non-evict/cache_as_ram.S: Add support for longmode ......................................................................

cpu/intel/car/non-evict/cache_as_ram.S: Add support for longmode

* Use heap for linker script calculated constant to fix relocation symbols in mixed assembly code. * Work around a strange bug that causes the CPU to halt when accessing MMX registers in long mode

Tested on HPZ220: * Still boots in x86_32.

Tested on Lenovo T410: * Doesn't need the MMX register fix in long mode.

Change-Id: I3e72a0bebf728fb678308006ea3a3aeb92910a84 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- A src/cpu/intel/car/cache_as_ram_symbols.inc M src/cpu/intel/car/non-evict/cache_as_ram.S 2 files changed, 50 insertions(+), 6 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/44673/4

-- To view, visit https://review.coreboot.org/c/coreboot/+/44673 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3e72a0bebf728fb678308006ea3a3aeb92910a84 Gerrit-Change-Number: 44673 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: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

build bot (Jenkins) (Code Review)

5:14 p.m.

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44673 )

Change subject: cpu/intel/car/non-evict/cache_as_ram.S: Add support for longmode ......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44673/4/src/cpu/intel/car/non-evict... File src/cpu/intel/car/non-evict/cache_as_ram.S:

https://review.coreboot.org/c/coreboot/+/44673/4/src/cpu/intel/car/non-evict... PS4, Line 214: * BUG: On some x86 platforms acessing MMX register in long mode 'acessing' may be misspelled - perhaps 'accessing'?

-- To view, visit https://review.coreboot.org/c/coreboot/+/44673 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3e72a0bebf728fb678308006ea3a3aeb92910a84 Gerrit-Change-Number: 44673 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: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 24 Sep 2020 17:14:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Patrick Rudolph (Code Review)

28 Sep 28 Sep

6:50 p.m.

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

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

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

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

Change subject: cpu/intel/car/non-evict/cache_as_ram.S: Add support for longmode ......................................................................

cpu/intel/car/non-evict/cache_as_ram.S: Add support for longmode

* Use heap for linker script calculated constant to fix relocation symbols in mixed assembly code. * Work around a strange bug that causes the CPU to halt when accessing MMX registers in long mode

Tested on HPZ220: * Still boots in x86_32.

Tested on Lenovo T410: * Doesn't need the MMX register fix in long mode.

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/44673/5

-- To view, visit https://review.coreboot.org/c/coreboot/+/44673 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3e72a0bebf728fb678308006ea3a3aeb92910a84 Gerrit-Change-Number: 44673 Gerrit-PatchSet: 5 Gerrit-Owner: Patrick Rudolph patrick.rudolph@9elements.com 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 siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Patrick Rudolph (Code Review)

6:51 p.m.

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

Change subject: cpu/intel/car/non-evict/cache_as_ram.S: Add support for longmode ......................................................................

Patch Set 5:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44673/3/src/cpu/intel/car/non-evict... File src/cpu/intel/car/non-evict/cache_as_ram.S:

https://review.coreboot.org/c/coreboot/+/44673/3/src/cpu/intel/car/non-evict... PS3, Line 233: * BUG: On some x86 platforms acessing MMX register in long mode

...

Would be good to fix

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/44673 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3e72a0bebf728fb678308006ea3a3aeb92910a84 Gerrit-Change-Number: 44673 Gerrit-PatchSet: 5 Gerrit-Owner: Patrick Rudolph patrick.rudolph@9elements.com 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 siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Mon, 28 Sep 2020 18:51:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: build bot (Jenkins) no-reply@coreboot.org Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Gerrit-MessageType: comment

Patrick Rudolph (Code Review)

29 Sep 29 Sep

12:27 p.m.

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

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

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

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

Change subject: cpu/intel/car/non-evict/cache_as_ram.S: Add support for longmode ......................................................................

cpu/intel/car/non-evict/cache_as_ram.S: Add support for longmode

* Use heap for linker script calculated constant to fix relocation symbols in mixed assembly code.

Tested on HPZ220: * Still boots in x86_32.

Tested on Lenovo T410: * Doesn't need the MMX register fix in long mode.

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/44673/6

-- To view, visit https://review.coreboot.org/c/coreboot/+/44673 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3e72a0bebf728fb678308006ea3a3aeb92910a84 Gerrit-Change-Number: 44673 Gerrit-PatchSet: 6 Gerrit-Owner: Patrick Rudolph patrick.rudolph@9elements.com 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 siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Angel Pons (Code Review)

21 Oct 21 Oct

9:33 a.m.

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

Change subject: cpu/intel/car/non-evict/cache_as_ram.S: Add support for longmode ......................................................................

Patch Set 6: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/44673 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3e72a0bebf728fb678308006ea3a3aeb92910a84 Gerrit-Change-Number: 44673 Gerrit-PatchSet: 6 Gerrit-Owner: Patrick Rudolph patrick.rudolph@9elements.com 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 siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Wed, 21 Oct 2020 09:33:45 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Arthur Heymans (Code Review)

9:42 a.m.

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

Change subject: cpu/intel/car/non-evict/cache_as_ram.S: Add support for longmode ......................................................................

Patch Set 6: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/44673/6/src/cpu/intel/car/non-evict... File src/cpu/intel/car/non-evict/cache_as_ram.S:

https://review.coreboot.org/c/coreboot/+/44673/6/src/cpu/intel/car/non-evict... PS6, Line 11: : #include <cpu/intel/car/cache_as_ram_symbols.inc> Is having this as an *.S file linked by included via the Makefile.inc a better idea?

-- To view, visit https://review.coreboot.org/c/coreboot/+/44673 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3e72a0bebf728fb678308006ea3a3aeb92910a84 Gerrit-Change-Number: 44673 Gerrit-PatchSet: 6 Gerrit-Owner: Patrick Rudolph patrick.rudolph@9elements.com 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 siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Wed, 21 Oct 2020 09:42:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Patrick Rudolph (Code Review)

3:08 p.m.

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

Change subject: cpu/intel/car/non-evict/cache_as_ram.S: Add support for longmode ......................................................................

Patch Set 6:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44673/6/src/cpu/intel/car/non-evict... File src/cpu/intel/car/non-evict/cache_as_ram.S:

https://review.coreboot.org/c/coreboot/+/44673/6/src/cpu/intel/car/non-evict... PS6, Line 11: : #include <cpu/intel/car/cache_as_ram_symbols.inc>

...

Is having this as an *.S file linked by included via the Makefile. […]

it's quite common to include assmbly files. That way there's no need for additional globals.

-- To view, visit https://review.coreboot.org/c/coreboot/+/44673 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3e72a0bebf728fb678308006ea3a3aeb92910a84 Gerrit-Change-Number: 44673 Gerrit-PatchSet: 6 Gerrit-Owner: Patrick Rudolph patrick.rudolph@9elements.com 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 siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Wed, 21 Oct 2020 15:08:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Arthur Heymans arthur@aheymans.xyz Gerrit-MessageType: comment

Arthur Heymans (Code Review)

27 Oct 27 Oct

6:39 p.m.

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

Change subject: cpu/intel/car/non-evict/cache_as_ram.S: Add support for longmode ......................................................................

Patch Set 6:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44673/6/src/cpu/intel/car/non-evict... File src/cpu/intel/car/non-evict/cache_as_ram.S:

https://review.coreboot.org/c/coreboot/+/44673/6/src/cpu/intel/car/non-evict... PS6, Line 11: : #include <cpu/intel/car/cache_as_ram_symbols.inc>

...

it's quite common to include assmbly files. That way there's no need for additional globals.

Ack

-- To view, visit https://review.coreboot.org/c/coreboot/+/44673 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3e72a0bebf728fb678308006ea3a3aeb92910a84 Gerrit-Change-Number: 44673 Gerrit-PatchSet: 6 Gerrit-Owner: Patrick Rudolph patrick.rudolph@9elements.com 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 siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Tue, 27 Oct 2020 18:39:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Rudolph patrick.rudolph@9elements.com Comment-In-Reply-To: Arthur Heymans arthur@aheymans.xyz Gerrit-MessageType: comment

Patrick Georgi (Code Review)

2 Nov 2 Nov

6:26 a.m.

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

Change subject: cpu/intel/car/non-evict/cache_as_ram.S: Add support for longmode ......................................................................

cpu/intel/car/non-evict/cache_as_ram.S: Add support for longmode

* Use heap for linker script calculated constant to fix relocation symbols in mixed assembly code.

Tested on HPZ220: * Still boots in x86_32.

Tested on Lenovo T410: * Doesn't need the MMX register fix in long mode.

Change-Id: I3e72a0bebf728fb678308006ea3a3aeb92910a84 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/44673 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Arthur Heymans arthur@aheymans.xyz --- A src/cpu/intel/car/cache_as_ram_symbols.inc M src/cpu/intel/car/non-evict/cache_as_ram.S 2 files changed, 39 insertions(+), 6 deletions(-)

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

diff --git a/src/cpu/intel/car/cache_as_ram_symbols.inc b/src/cpu/intel/car/cache_as_ram_symbols.inc new file mode 100644 index 0000000..857e039 --- /dev/null +++ b/src/cpu/intel/car/cache_as_ram_symbols.inc @@ -0,0 +1,20 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +/* Trick the linker into supporting x86_64 relocations in 32bit code */ +#if ENV_X86_64 +#define uintptr_t quad +#else +#define uintptr_t long +#endif + +rom_mtrr_mask: +.uintptr_t _rom_mtrr_mask + +rom_mtrr_base: +.uintptr_t _rom_mtrr_base + +car_mtrr_mask: +.uintptr_t _car_mtrr_mask + +car_mtrr_start: +.uintptr_t _car_mtrr_start diff --git a/src/cpu/intel/car/non-evict/cache_as_ram.S b/src/cpu/intel/car/non-evict/cache_as_ram.S index d087365..cde1ca3 100644 --- a/src/cpu/intel/car/non-evict/cache_as_ram.S +++ b/src/cpu/intel/car/non-evict/cache_as_ram.S @@ -9,6 +9,8 @@

.global bootblock_pre_c_entry

+#include <cpu/intel/car/cache_as_ram_symbols.inc> + .code32 _cache_as_ram_setup:

@@ -83,11 +85,10 @@ movl $MTRR_PHYS_MASK(1), %ecx wrmsr

- post_code(0x23) /* Set Cache-as-RAM base address. */ movl $(MTRR_PHYS_BASE(0)), %ecx - movl $_car_mtrr_start, %eax + movl car_mtrr_start, %eax orl $MTRR_TYPE_WRBACK, %eax xorl %edx, %edx wrmsr @@ -96,20 +97,20 @@ /* Set Cache-as-RAM mask. */ movl $(MTRR_PHYS_MASK(0)), %ecx rdmsr - movl $_car_mtrr_mask, %eax + mov car_mtrr_mask, %eax orl $MTRR_PHYS_MASK_VALID, %eax wrmsr

/* Enable cache for our code in Flash because we do XIP here */ movl $MTRR_PHYS_BASE(1), %ecx xorl %edx, %edx - movl $_rom_mtrr_base, %eax + mov rom_mtrr_base, %eax orl $MTRR_TYPE_WRPROT, %eax wrmsr

movl $MTRR_PHYS_MASK(1), %ecx rdmsr - movl $_rom_mtrr_mask, %eax + mov rom_mtrr_mask, %eax orl $MTRR_PHYS_MASK_VALID, %eax wrmsr

@@ -207,8 +208,19 @@ /* Need to align stack to 16 bytes at call instruction. Account for the pushes below. */ andl $0xfffffff0, %esp - subl $4, %esp

+#if ENV_X86_64 + + #include <cpu/x86/64bit/entry64.inc> + + movd %mm2, %rdi + shlq $32, %rdi + movd %mm1, %rsi + or %rsi, %rdi + movd %mm0, %rsi + +#else + subl $4, %esp /* push TSC and BIST to stack */ movd %mm0, %eax pushl %eax /* BIST */ @@ -216,6 +228,7 @@ pushl %eax /* tsc[63:32] */ movd %mm1, %eax pushl %eax /* tsc[31:0] */ +#endif

before_c_entry: post_code(0x29)

-- To view, visit https://review.coreboot.org/c/coreboot/+/44673 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3e72a0bebf728fb678308006ea3a3aeb92910a84 Gerrit-Change-Number: 44673 Gerrit-PatchSet: 7 Gerrit-Owner: Patrick Rudolph patrick.rudolph@9elements.com 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 siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: merged

1517

days inactive

1590

days old

coreboot-gerrit@coreboot.org

18 comments

5 participants

tags (0)

participants (5)

Angel Pons (Code Review)
Arthur Heymans (Code Review)
build bot (Jenkins) (Code Review)
Patrick Georgi (Code Review)
Patrick Rudolph (Code Review)