Patch Set 1:

I think you've correctly identified the problem although I'm not sure what other behaviors this change might create. Short of rewriting all the gcccar.inc code, I wonder if a better solution is to make DCACHE_BSP_STACK_SIZE match gcccar's BSP_STACK_SIZE. It seems like a while back I'd tried experimenting with using the CONFIG_ values in gcccar but saw it breaking more than it fixed.

You can't do that. DCACHE_RAM_SIZE has to match BSP_STACK_SIZE. And within DCACHE_RAM_SIZE you need the room for vboot, console, timestamps and .bss too.

For the production branch, you also cannot change CAR layout from what (RO) bootblock was built with originally. And we don't know if 0x4000 is a sufficient DCACHE_BSP_STACK_SIZE, I assume the value was chosen with infamous copy-paste method.

That leaves us with the option of fixing car.ld to match gcccar.inc (or cache_as_ram.S). That is, tell the linker stack grows downwards from _car_region_end. I just hate to put a platform-specific hacks into that file... We do have that alternative definition of stack located at the end, so I don't think it would look that bad to take this approach.

View Change

To view, visit change 34883. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Id91393f1e7faf86b01fdc113e7940893673a27a7
Gerrit-Change-Number: 34883
Gerrit-PatchSet: 1
Gerrit-Owner: Kyösti Mälkki <kyosti.malkki@gmail.com>
Gerrit-Reviewer: Aaron Durbin <adurbin@chromium.org>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki@gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth@google.com>
Gerrit-Reviewer: Richard Spiegel <richard.spiegel@silverbackltd.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net>
Gerrit-Comment-Date: Wed, 21 Aug 2019 17:52:34 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment