Attention is currently required from: Sean Rhodes, Michał Żygowski, Patrick Rudolph, Paul Menzel, Michał Kopeć.
Krystian Hebel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/73981 )
Change subject: drivers/smmstore/ramstage.c: retry smmstore init up 5 times ......................................................................
Patch Set 1: Code-Review-1
(2 comments)
File src/drivers/smmstore/ramstage.c:
https://review.coreboot.org/c/coreboot/+/73981/comment/5f7aa62c_2758708a PS1, Line 64: * Retry 5 times in case the SMI isn't triggered immediately. I/O port write request is sent to northbridge/ICH/PCH part of SoC, from where it used to be driven as SMI# towards CPU. At some point it was replaced by Virtual Legacy Wire (VLW) transactions sent through QuickPath Interconnect (QPI). Both options may introduce delays longer than that time between completing `out` instruction and executing the next one.
I can't find anything concrete in newer documentation, but ICH10 datasheet says this:
System Management Interrupt: SMI# is an active low output synchronous to PCICLK. It is asserted by the ICH10 in response to one of many enabled hardware or software events.
https://review.coreboot.org/c/coreboot/+/73981/comment/c1e963d0_66459ef9 PS1, Line 79: } while (retry--);
Use `wait_ms` macro from `src/include/timer.h`? […]
I'm afraid this can't be done, even `mdelay()` should be removed from the code. On x86, `eax` is used for function return values, it may also be used internally by `mdelay()`, especially if it uses TSC under the hood. If SMI arrives after it is compared with `SMMSTORE_RET_SUCCESS`, further invocations of `SMMSTORE_CMD_INIT` will fail due to `if (store_initialized)` check in store.c. It will be even worse if it's get overwritten after `mdelay()` or any function called by it tries to use it.