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 29:
(3 comments)
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 Why wouldn't you provide a default value of 0 for this macro?
https://review.coreboot.org/c/coreboot/+/35799/29/src/soc/intel/common/block... File src/soc/intel/common/block/sgx/Kconfig:
https://review.coreboot.org/c/coreboot/+/35799/29/src/soc/intel/common/block... PS29, Line 40: depends on SOC_INTEL_CANNONLAKE_BASE || SOC_INTEL_ICELAKE So are we going to try and maintain this list here (and in multiple places?). It seems a little brittle.
https://review.coreboot.org/c/coreboot/+/35799/29/src/soc/intel/common/block... PS29, Line 53: depends on !SOC_INTEL_COMMON_BLOCK_SGX_ENABLE Why is the 1MiB carved out from SGX_ENABLE Kconfig?