Michael Niewöhner 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 33:
(4 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?
if you had looked into get_max_prmrr_size(), you'd know ;) get_max_prmrr_size has a parameter `limit`, while limit=0 means "no limit"
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?). […]
what do you mean with "and in multiple places"?
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?
Because SGX depends on PRMRR >= 32 MiB
https://review.coreboot.org/c/coreboot/+/35799/5/src/soc/intel/skylake/memma... File src/soc/intel/skylake/memmap.c:
https://review.coreboot.org/c/coreboot/+/35799/5/src/soc/intel/skylake/memma... PS5, Line 89:
readability? can remove it if you insist
superseeded