Change in ...coreboot[master]: [TOTEST]nb/intel/sandybridge/mrc.bin: Increase the CAR size
Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33174
Change subject: [TOTEST]nb/intel/sandybridge/mrc.bin: Increase the CAR size ......................................................................
[TOTEST]nb/intel/sandybridge/mrc.bin: Increase the CAR size
The mrc.bin places its stack at an awkward location, close to the bottom of the CAR location, which easily conflicts with other CAR linker symbols. To work around this issue, increase the CAR with 128k to a total of 256k, align the base and make sure the mrc.bin falls in the DCACHE_RAM_MRC_VAR_SIZE.
Change-Id: Ida8ad9a54d29a9ee1301bdcff00d81f548d2b81e Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/northbridge/intel/sandybridge/Kconfig 1 file changed, 3 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/33174/1
diff --git a/src/northbridge/intel/sandybridge/Kconfig b/src/northbridge/intel/sandybridge/Kconfig index 4f9da00..47ef829 100644 --- a/src/northbridge/intel/sandybridge/Kconfig +++ b/src/northbridge/intel/sandybridge/Kconfig @@ -98,16 +98,15 @@
config DCACHE_RAM_BASE hex - default 0xff7e0000 + default 0xff7c0000
config DCACHE_RAM_SIZE hex - default 0x1c000 + default 0x21000
config DCACHE_RAM_MRC_VAR_SIZE hex - default 0x4000 - + default 0x1f000 config MRC_FILE string "Intel System Agent path and filename" default "3rdparty/blobs/northbridge/intel/sandybridge/systemagent-r6.bin"
Hello Kyösti Mälkki, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33174
to look at the new patch set (#2).
Change subject: [TOTEST]nb/intel/sandybridge/mrc.bin: Increase the CAR size ......................................................................
[TOTEST]nb/intel/sandybridge/mrc.bin: Increase the CAR size
The mrc.bin places its stack at an awkward location, close to the bottom of the CAR location, which easily conflicts with other CAR linker symbols. To work around this issue, increase the CAR with 128k to a total of 256k, align the base and make sure the mrc.bin falls in the DCACHE_RAM_MRC_VAR_SIZE.
Change-Id: Ida8ad9a54d29a9ee1301bdcff00d81f548d2b81e Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/arch/x86/car.ld M src/northbridge/intel/sandybridge/Kconfig 2 files changed, 4 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/33174/2
Hello Kyösti Mälkki, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33174
to look at the new patch set (#3).
Change subject: nb/intel/sandybridge/mrc.bin: Increase the CAR size ......................................................................
nb/intel/sandybridge/mrc.bin: Increase the CAR size
The mrc.bin places its stack at an awkward location, close to the bottom of the CAR location, which easily conflicts with other CAR linker symbols. To work around this issue, increase the CAR with 128k to a total of 256k, align the base and make sure the mrc.bin falls in the DCACHE_RAM_MRC_VAR_SIZE.
TESTED on Thinkpad X220. All sandy-/ivy-bridge CPUs ought to have enough cache for this change.
Change-Id: Ida8ad9a54d29a9ee1301bdcff00d81f548d2b81e Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/arch/x86/car.ld M src/northbridge/intel/sandybridge/Kconfig 2 files changed, 4 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/33174/3
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33174 )
Change subject: nb/intel/sandybridge/mrc.bin: Increase the CAR size ......................................................................
Patch Set 3: Code-Review+2
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33174 )
Change subject: nb/intel/sandybridge/mrc.bin: Increase the CAR size ......................................................................
Patch Set 3: Code-Review-1
(1 comment)
I assume this breaks offset DCACHE_RAM_VAR_BASE in raminit-mrc:268. Or you could just remove the runtime test lines 408-418 since there is clean separation now.
https://review.coreboot.org/#/c/33174/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/33174/3//COMMIT_MSG@13 PS3, Line 13: the DCACHE_RAM_MRC_VAR_SIZE. .. the heap/pool of mrc.bin falls in ..
Hello Kyösti Mälkki, build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33174
to look at the new patch set (#4).
Change subject: nb/intel/sandybridge/mrc.bin: Increase the CAR size ......................................................................
nb/intel/sandybridge/mrc.bin: Increase the CAR size
The heap/pool of the mrc.bin falls in an awkward location, close to the bottom of the CAR location, which easily conflicts with other CAR linker symbols. To work around this issue, increase the CAR with 128k to a total of 256k, align the base and make sure the mrc.bin falls in the DCACHE_RAM_MRC_VAR_SIZE.
TESTED on Thinkpad X220. All sandy-/ivy-bridge CPUs ought to have enough cache for this change.
Change-Id: Ida8ad9a54d29a9ee1301bdcff00d81f548d2b81e Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/arch/x86/car.ld M src/northbridge/intel/sandybridge/Kconfig M src/northbridge/intel/sandybridge/raminit_mrc.c 3 files changed, 10 insertions(+), 13 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/33174/4
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33174 )
Change subject: nb/intel/sandybridge/mrc.bin: Increase the CAR size ......................................................................
Patch Set 4: Code-Review+1
(1 comment)
https://review.coreboot.org/#/c/33174/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/33174/4//COMMIT_MSG@13 PS4, Line 13: the DCACHE_RAM_MRC_VAR_SIZE. I was actually looking for a chance in the last sentence, as MRC.bin itself falls inside the WP MTRR.
Hello Kyösti Mälkki, build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33174
to look at the new patch set (#5).
Change subject: nb/intel/sandybridge/mrc.bin: Increase the CAR size ......................................................................
nb/intel/sandybridge/mrc.bin: Increase the CAR size
The heap/pool of the mrc.bin falls in an awkward location, close to the bottom of the CAR location, which easily conflicts with other CAR linker symbols. To work around this issue, increase the CAR with 128k to a total of 256k, align the base and make sure the heap/pool of the mrc.bin falls in the DCACHE_RAM_MRC_VAR_SIZE.
TESTED on Thinkpad X220. All sandy-/ivy-bridge CPUs ought to have enough cache for this change.
Change-Id: Ida8ad9a54d29a9ee1301bdcff00d81f548d2b81e Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/arch/x86/car.ld M src/northbridge/intel/sandybridge/Kconfig M src/northbridge/intel/sandybridge/raminit_mrc.c 3 files changed, 10 insertions(+), 13 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/33174/5
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33174 )
Change subject: nb/intel/sandybridge/mrc.bin: Increase the CAR size ......................................................................
Patch Set 5: Code-Review+2
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33174 )
Change subject: nb/intel/sandybridge/mrc.bin: Increase the CAR size ......................................................................
Patch Set 5: Code-Review+2
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33174 )
Change subject: nb/intel/sandybridge/mrc.bin: Increase the CAR size ......................................................................
Patch Set 5: Code-Review-2
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33174 )
Change subject: nb/intel/sandybridge/mrc.bin: Increase the CAR size ......................................................................
Patch Set 5:
There is a check in the r6-binary (at offset 0x00002542) which checks if the signed difference between the (stack location - the heap location (0xff7e1000) ) is below 1000, for which there is seamingly no reason. This is always true as long as the stack is located above the heap and very likely untrue if the stack is located below the heap. A solution that works is just to skip bailing out (jg -> jmp below cmp). How are we on binary patching that binary? It's literally one byte that needs to be changed.
Hello Kyösti Mälkki, build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33174
to look at the new patch set (#7).
Change subject: nb/intel/sandybridge/mrc.bin: Increase the CAR size ......................................................................
nb/intel/sandybridge/mrc.bin: Increase the CAR size
The heap/pool of the mrc.bin falls in an awkward location, close to the bottom of the CAR location, which easily conflicts with other CAR linker symbols. To work around this issue, increase the CAR with 128k to a total of 256k, align the base and make sure the heap/pool of the mrc.bin falls in the DCACHE_RAM_MRC_VAR_SIZE.
TESTED on Thinkpad X220. All sandy-/ivy-bridge CPUs ought to have enough cache for this change.
Change-Id: Ida8ad9a54d29a9ee1301bdcff00d81f548d2b81e Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/arch/x86/car.ld M src/northbridge/intel/sandybridge/Kconfig M src/northbridge/intel/sandybridge/raminit_mrc.c 3 files changed, 10 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/33174/7
Arthur Heymans has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/33174 )
Change subject: nb/intel/sandybridge/mrc.bin: Increase the CAR size ......................................................................
Abandoned
Not needed anymore with modified mrc.bin
-
Arthur Heymans (Code Review) -
Kyösti Mälkki (Code Review)
-
Nico Huber (Code Review)