[coreboot-gerrit] Change in coreboot[master]: soc/intel: generate SSDT instead of using GNVS for SGX

[Angel Pons (Code Review)]

Attention is currently required from:  Felix Singer, Pratik Prajapati, Nico Huber, Furquan Shaikh, Paul Menzel, Subrata Banik, Michael Niewöhner, Andrey Petrov, Kyösti Mälkki, Patrick Rudolph, Pratikkumar V Prajapati.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58394 )

Change subject: soc/intel: generate SSDT instead of using GNVS for SGX
......................................................................

Patch Set 2:

(1 comment)

File src/soc/intel/apollolake/acpi.c:

https://review.coreboot.org/c/coreboot/+/58394/comment/fc2a2693_108a13f3 
PS2, Line 91: SOC_INTEL_COMMON_BLOCK_SGX
> I had a deeper look:
> > Looks like APL is using the wrong Kconfig symbol to guard SGX stuff, `SOC_INTEL_COMMON_BLOCK_SGX` is always true.
>
> While you are right that this is wrong, it would vanish anyways with this change.

One of the guards vanishes, but the one in `southbridge.asl` remains unchanged.
> > otherwise this commit will result in ACPI errors on APL when `SOC_INTEL_COMMON_BLOCK_SGX_ENABLE` is not enabled (SGX ASL is included, but SSDT doesn't provide the SGX externals).
>
> That's not true. This change *does* provide the externals in both cases.
> https://review.coreboot.org/c/coreboot/+/58394/2/src/soc/intel/apollolake/ch...

What both cases? I'm referring to the case where `SOC_INTEL_COMMON_BLOCK_SGX_ENABLE` is false.
> > I'd fix this check and the one in `src/soc/intel/apollolake/acpi/southbridge.asl` in a separate commit
>
> `SOC_INTEL_COMMON_BLOCK_SGX` is selected by SoCs that usually support SGX. `SOC_INTEL_COMMON_BLOCK_SGX_ENABLE` is chosen via menuconfig and means "enable when supported". (There seem to be SoCs without support.)

I know.
> That means: there is that case where `SOC_INTEL_COMMON_BLOCK_SGX_ENABLE` is chosen but SGX is detected to be unsupported at runtime (!). We could only handle EPC presence with acpigen, not via Kconfig atm. so having `SOC_INTEL_COMMON_BLOCK_SGX` in `src/soc/intel/apollolake/acpi/southbridge.asl` and `src/soc/intel/skylake/acpi/pch.asl` is correct right now.

I'm talking about the case where `SOC_INTEL_COMMON_BLOCK_SGX_ENABLE` is *not* chosen. The SGX ASL would be included, but the SSDT generation is guarded with `SOC_INTEL_COMMON_BLOCK_SGX_ENABLE` (see chip.c in this patch):

if (CONFIG(SOC_INTEL_COMMON_BLOCK_SGX_ENABLE))
    	sgx_generate_ssdt();