[S] Change in coreboot[master]: soc/intel/meteorlake: Set UPDs for TME exclusion range and new key gen

[Subrata Banik (Code Review)]

Attention is currently required from: Jakub Czapiga, Kapil Porwal, Pratikkumar V Prajapati, Tarun Tuli.

Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75626?usp=email )

The change is no longer submittable: All-Comments-Resolved is unsatisfied now.

Change subject: soc/intel/meteorlake: Set UPDs for TME exclusion range and new key gen
......................................................................

Patch Set 13:

(2 comments)

File src/soc/intel/meteorlake/romstage/fsp_params.c:

https://review.coreboot.org/c/coreboot/+/75626/comment/dbe2b2c8_3d4d1bf8 :
PS8, Line 183: 	if (m_cfg->TmeEnable) {
> If more security params are added later then we might be to keep TME at the end of the function, if we return from this function based on if TME is not enabled.
>
> Also, we are not going to call the helper function multiple times so better to keep these couple of lines of code only in fill_fspm_security_params()

Please move the code block inside the separate helper function which would help to optimize the `if` loop as well. Just return if certain config is not enabled.

https://review.coreboot.org/c/coreboot/+/75626/comment/7ff6da7b_f4f62f85 :
PS8, Line 185: TME_EXCLUDE_CBMEM_ENCRYPTION
> Assuming your que is, "if we are enabling TME_GENERATE_NEW_KEY_ON_WARM_BOOT then shouldn't TME_EXCLUDE_CBMEM_ENCRYPTION be auto enabled ? otherwise certain ranges won't be able to recover across power-cycle?"
>
> if TME_GENERATE_NEW_KEY_ON_WARM_BOOT is enabled then nothing will get decrypted from DRAM residual of last warmboot. But if want to access CBMEM, we need to exclude CBMEM from encryption.

ideally in context of coreboot, we would like to ensure that memory encryption technique doesn't impact the coreboot assets or user impact.
> But i don’t think we can enable TME_EXCLUDE_CBMEM_ENCRYPTION config option just because TME_GENERATE_NEW_KEY_ON_WARM_BOOT is enabled.

why not? I don't see a point where the user should be okay to lose the coreboot assets just because they have chosen TME_GENERATE_NEW_KEY_ON_WARM_BOOT kconfig. The basic coreboot persistent assets across warm resets should be usable as is.
> We can enable TME_EXCLUDE_CBMEM_ENCRYPTION separately. i want to keep both of the param independent.

I strongly believe that user should know the relation between those kconfigs

1. TME_GENERATE_NEW_KEY_ON_WARM_BOOT - would create a new memory encryption key on each warm reset hence, one won't be able to access the coreboot assets unless excluding the coreboot asset ranges from the encrypted boundary.

2. TME_EXCLUDE_CBMEM_ENCRYPTION - This is associated with #1 where unless this config is enabled, the user will lost access to the coreboot assets if TME_GENERATE_NEW_KEY_ON_WARM_BOOT is enabled.
> Because our current implementation only supports CBMEM, in future we might want to exclude some other range. (note that only one range can be excluded based in current TME spec and range has to be in physical mem only). So its better to enable TME_EXCLUDE_CBMEM_ENCRYPTION separately.

I agree that, there might be more assets which are platform specific and platform user would like to also keep those assets excluded from encryption range but the current SOC design limitation (i.e., encryption exclusion range can be only linear) will forbid that flexibility from the user.