On Sat, Mar 31, 2018 at 10:21 AM, Nico Huber nico.h@gmx.de wrote:
On 31.03.2018 17:31, Nico Huber wrote:
On 23.03.2018 16:29, Jay Talbott wrote:
Do you think they will resolve the issue that I am seeing?
I don't know. As you top posted, I haven't looked at it yet. I'll have a look. But it seems very likely that you'll have to fix something about your memory map.
Yes, it should help. Can you please send a log of the same configuration as used for the last log but with (at least) the first two of my patches applied?
I would expect the UC-default case to succeed, then, because we can squash the MTRRs above 4GiB together.
Another thought that came about the temporary ROM caching: In your case 2MiB ROM are set up (is that correct? [1]). I think this is why nobody else run into this issue, most have a big (e.g. 16MiB) SPI flash that is easier to handle in the WB-default case. But the code that adds the tem- porary caching confuses me anyway:
(from src/soc/intel/skylake/romstage/romstage_fsp20.c)
/* Cache the ROM as WP just below 4GiB. */ postcar_frame_add_mtrr(&pcf, 0xFFFFFFFF - CONFIG_ROM_SIZE + 1, CONFIG_ROM_SIZE, MTRR_TYPE_WRPROT);Here (and for many other platforms in coreboot), CONFIG_ROM_SIZE is used. Beside your problem of the hard to cache smaller ROM sizes, it could also be too big. e.g. with a 32MiB ROM, we would mark MMIO space as cacheable. Aaron, what do you think? would it hurt to always mark 16MiB on Intel? IIRC, 16MiB is the maximum that is memory mapped of the boot ROM, and I don't think it would hurt if the ROM is actually smaller.
I think that's definitely viable. That'd fix some of these messages. To be fair, the MTRR calculation does end up working. It's just trying really hard temporarily add things in.
Nico
[1] This line of your log seems to suggest that you actually have 16MiB:
SF: Detected FAST_SPI Hardware Sequencer with sector size 0x1000, total 0x1000000
So I suspect your CONFIG_ROM_SIZE is off.