Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42074 )
Change subject: [WIP] soc/amd/common: Avoid MMIO aliasing on SMBus ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42074/10/src/soc/amd/common/block/s... File src/soc/amd/common/block/smbus/smbus.c:
https://review.coreboot.org/c/coreboot/+/42074/10/src/soc/amd/common/block/s... PS10, Line 15: union reg_bank { : uint8_t reg8[0x100]; : uint16_t reg16[0x100 / sizeof(uint16_t)]; : };
I'm clearly missing something. What aliasing? This seems really strange as well.
It's a strange compiler optimisation topic indeed.
When we do a write8() the assembly "injector" decides the write could target anything in memory, it cannot determine the target is actually MMIO in this case. If we had previously loaded a variable from stack to register, it will have to be reloaded after write8().
It's not that bad with SMBus, but gets pretty bad with PCI MMCONF in ramstage.
Similarly with write32(), except that it now knows only variables of uint32_t may have been the target. So such variables on the stack will need to be reloaded to registers.
Also, this reg8[0x100] could be reg[0x20] or [0x40] to match how many registers the controller actually decodes. Compiler should be able to detect bad register indexing buildtime already.