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 7:
(5 comments)
https://review.coreboot.org/c/coreboot/+/35799/5/src/soc/intel/common/block/... File src/soc/intel/common/block/cpu/cpulib.c:
https://review.coreboot.org/c/coreboot/+/35799/5/src/soc/intel/common/block/... PS5, Line 332: valid
uninitialized?
already fixed in newest patchset
https://review.coreboot.org/c/coreboot/+/35799/5/src/soc/intel/common/block/... PS5, Line 340: valid/MiB)
Will always be 0.
no, but I changed this anyways in the newst patchset :-)
https://review.coreboot.org/c/coreboot/+/35799/7/src/soc/intel/common/block/... File src/soc/intel/common/block/sgx/sgx.c:
https://review.coreboot.org/c/coreboot/+/35799/7/src/soc/intel/common/block/... PS7, Line 28: static bool sgx_param_valid;
the file sgx. […]
?? this is the case already as SOC_INTEL_COMMON_BLOCK_SGX_ENABLE depends on SOC_INTEL_COMMON_BLOCK_SGX; see Kconfig and Makefile.inc
https://review.coreboot.org/c/coreboot/+/35799/7/src/soc/intel/common/block/... PS7, Line 57: if (!CONFIG(SOC_INTEL_COMMON_BLOCK_SGX_ENABLE) || !is_sgx_supported())
that check should be in the function calling prmrr_core_configure()
why? this would change the behaviour which I strictly want to avoid
https://review.coreboot.org/c/coreboot/+/35799/7/src/soc/intel/common/block/... PS7, Line 213: if (!CONFIG(SOC_INTEL_COMMON_BLOCK_SGX_ENABLE) || !is_sgx_supported()) {
should be in the code calling sgx_fill_gnvs()
same as above