Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31639 )
Change subject: sb/intel/common/firmware: Add an option to use ifdtool ......................................................................
Patch Set 5:
(1 comment)
I think the change to ‘choice’ has two hocks. The first is that if someone did not activate the ME/TXE lock until now and assumes that the regions are all unlocked, they will now come to the default branch – ‘don’t touch’. If now an already been edited descriptor region is used, these protect settings are applied.
You are right, it's probably too late to change the default as people are used to the current behaviour. I don't care much about the default (IMHO, it's odd to do anything with the descriptor at all). Also see inline comment.
And the second issue we are getting with switching to ‘choice’ is a conflict with the lynxpoint Kconfig file. This also contains an option with the name ‘LOCK_MANAGEMENT_ENGINE’.
Yeah, that lynxpoint entry is a no-op and could be dropped.
https://review.coreboot.org/#/c/31639/4/src/southbridge/intel/common/firmwar... File src/southbridge/intel/common/firmware/Kconfig:
https://review.coreboot.org/#/c/31639/4/src/southbridge/intel/common/firmwar... PS4, Line 146: default USE_IFWI_REGION_ACCESS_CONTROL You could also put UNLOCK_FLASH_REGIONS here, which should resemble the old default.