Nico Huber 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:
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?
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.
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?
The option has a misleading name. Maybe that's what causes the confusion. What it actually represents should be something like MAX_EXPECTED_RESET_COUNT_ON_A_SINGLE_REBOOT.