Marcello Sylvester Bauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29979 )
Change subject: arch/x86/Kconfig: move MAX_REBOOT_CNT option ......................................................................
Patch Set 2:
Patch Set 2:
Everything below the currently defined defaults is wrong.
why are they wrong? I tested it with values below and it behaves like it should.
Tested on what board and tested how? With/without MRC cache might make a difference for instance, plus there are about half a dozen of software reset methods, which one was used?
I only tested the behavior on qemu and especially on my own maschine. (boot, reboot, resume to disk, S3 resume)
Take modern Intel systems for instance. It's possible to reset the CPU only, causing an INIT, the cores reset the x86 register state, BSP restarts at the reset vector. Let's call it Reset#1. This is not enough to rerun coreboot, though. Hence you'll usually find a 0x6 -> 0xcf9 reset (aka. system reset, still a warm reset) early in the romstage: Reset#2. If no mrc.cache is present (bug or not) the raminit will refuse to retrain on warm resets: 0xe -> 0xcf9 (full reset, goes into S5 and back into S0 after a few seconds, hence a cold reset). That'd be Reset#3.
So if MAX_REBOOT_CNT is < 3 and you hit the path described above, it will run into fallback for no reason.
Now i understand dimension of the problem. Thanks for the detailed explanation :)
While that's the only path that comes to mind atm, there may be more like that. If you can prove that a board doesn't have any valid path with 3 resets, you can set MAX_REBOOT_CNT lower, and I won't call it wrong ;)
In other words, this option was hidden on purpose. You can add another option for instance and take the minimum of both or something like that. But you mustn't lower it.
An other option would be to adapt the range in the Kconfig or fix the issue which cause this wrong behavior.
What wrong behaviour?
I meant the reset sequel, which causes a fallback even if nothing failed, but now i understand that it isn't possible.
I will update the entry to warn about any value below.