Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35799 )
Change subject: soc/intel/sgx: convert SGX and PRMRR devicetree options to Kconfig ......................................................................
Patch Set 37:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35799/29/src/soc/intel/apollolake/m... File src/soc/intel/apollolake/memmap.c:
https://review.coreboot.org/c/coreboot/+/35799/29/src/soc/intel/apollolake/m... PS29, Line 39: #ifdef SOC_INTEL_COMMON_BLOCK_SGX_PRMRR_SIZE
One could even take it so far as to not have a get_max_prmrr_size() that takes a parameter because you ensuring you are providing the size through a macro based on Kconfig. In such a scenario when would we ever need a get_max_prmrr_size() with a parameter when it could just be get_prmrr_size() ?
Well, the problem is, that the user may select a wrong value, that is not supported. At Kconfig we can't know what the soc really supports
Sure, and the runtime checks it. However, that's orthogonal to providing a macro that correlates to a value and where that value is inserted from a call site. Right now, you are open coding the use of a macro when this code really shouldn't be bothered with anything aside from the returned value of the prmrr size. i.e. this code doesn't care about the selected Kconfig value.