Hi Patrick,
On Tue, 2021-11-09 at 17:43 +0000, Patrick Georgi via coreboot wrote:
Is there any particular concern with having coreboot in that memory region on your platform that makes you ask this or is this just some general curiosity?
Thanks for confirming that we are not doing anything obviously wrong. That's already very helpful.
I was asking, because we see coreboot memcpy'ing to ROM. We map it to 0xFF00_0000 and it's copying 0xee8 bytes of memory from 0xff20_0300 to 0xff20_0300 (the same memory). So this is basically a NOP, but very weird. It happens in the bootblock:
coreboot-4.14-a0aee78c8261804e498b3c31bf4b855fb7e7d1cd Wed Nov 10 08:18:54 UTC 2021 bootblock starting (log level: 8)... FMAP: Found "FLASH" version 1.1 at 0x200000. FMAP: base = 0xff000000 size = 0x1000000 #areas = 3 FMAP: area COREBOOT found @ 200200 (14679552 bytes) CBFS: mcache @0x00014e00 built for 8 files, used 0x1e0 of 0x2000 bytes CBFS: Found 'fallback/romstage' @0x80 size 0x3ba0 in mcache @0x00014e2c WARN - MOVS to ROM at RIP ffffdc2f RCX ee8 ff200300->ff200300
The warning is from us. Improvising a stack trace at this point yields:
cbfs_prog_stage_load -> cbfs_load_and_decompress.isra.5 -> rdev_readat -> mdev_readat -> mdev -> memcpy
In cbfs_prog_stage_load, I can see this code:
size_t fsize = cbfs_load_and_decompress(&rdev, prog_start(pstage), prog_size(pstage), compression, file_hash);
And here prog_start() definitely returns the pointer 0xff20_0300, which points into ROM. Passing a pointer to ROM as a writable pointer feels wrong. Given that the code just does memcpy with src=dst in this buffer, this all is harmless, but it feels a bit like "works by accident".
Am I missing something?
Julian