[coreboot-gerrit] Change in ...coreboot[master]: {drivers,soc/intel/braswell}: Implement C_ENVIRONMENT_BOOTBLOCK support

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29662 )

Change subject: {drivers,soc/intel/braswell}: Implement C_ENVIRONMENT_BOOTBLOCK support
......................................................................

Patch Set 47:

(1 comment)

https://review.coreboot.org/#/c/29662/47/src/drivers/intel/fsp1_1/cache_as_r... 
File src/drivers/intel/fsp1_1/cache_as_ram.S:

https://review.coreboot.org/#/c/29662/47/src/drivers/intel/fsp1_1/cache_as_r... 
PS47, Line 57: 	jmp	find_fsp
> > But it should have been. The called code is compiled by a different […]

But you have %esp pointing to memory-mapped SPI flash space here? So you need to somehow convince compiler to build find_fsp() without stack. That very odd-looking union in the implementation is probably silently trying to achieve just that, it was just not commented at all.

At the same time it should be declared with asmlinkage (regparm="0") to tell input parameters are on the stack...