Pratikkumar V Prajapati has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45087 )
Change subject: soc/intel/tigerlake: Add config option to enable TME ......................................................................
soc/intel/tigerlake: Add config option to enable TME
Add config option to set TmeEnable FSP-M upd. The TME spec is available at: "https://software.intel.com/sites/ default/files/managed/a5/16/Multi-Key-Total-Memory-Encryption- Spec.pdf"
Test: TME ENABLE and LOCK bits get set when Tme is enabled.
Signed-off-by: Pratik Prajapati pratikkumar.v.prajapati@intel.com Change-Id: I181aed2bf4a79005fe42e3e133b5faee91201dad --- M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/romstage/fsp_params.c 2 files changed, 11 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/45087/1
diff --git a/src/soc/intel/tigerlake/Kconfig b/src/soc/intel/tigerlake/Kconfig index 8718f97..42f83ee 100644 --- a/src/soc/intel/tigerlake/Kconfig +++ b/src/soc/intel/tigerlake/Kconfig @@ -215,4 +215,12 @@ config PRERAM_CBMEM_CONSOLE_SIZE hex default 0x1400 + +config INTEL_TME + bool "Total Memory Encryption (TME)" + default n + help + Enable Total Memory Encryption (TME). The spec is available at + "https://software.intel.com/sites/default/files/managed/a5/16/Multi- + Key-Total-Memory-Encryption-Spec.pdf". endif diff --git a/src/soc/intel/tigerlake/romstage/fsp_params.c b/src/soc/intel/tigerlake/romstage/fsp_params.c index 2ba276d..4e47959 100644 --- a/src/soc/intel/tigerlake/romstage/fsp_params.c +++ b/src/soc/intel/tigerlake/romstage/fsp_params.c @@ -208,6 +208,9 @@ /* Skip CPU side PCIe enablement in FSP if device is disabled in devicetree */ dev = pcidev_path_on_root(SA_DEVFN_CPU_PCIE); m_cfg->CpuPcieRpEnableMask = dev && dev->enabled; + + /* Change TmeEnable UPD value according to ENABLE_TME Kconfig */ + m_cfg->TmeEnable = CONFIG(INTEL_TME); }
void platform_fsp_memory_init_params_cb(FSPM_UPD *mupd, uint32_t version)
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45087 )
Change subject: soc/intel/tigerlake: Add config option to enable TME ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/c/coreboot/+/45087/1/src/soc/intel/tigerlake/rom... File src/soc/intel/tigerlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/45087/1/src/soc/intel/tigerlake/rom... PS1, Line 212: /* Change TmeEnable UPD value according to ENABLE_TME Kconfig */ code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/45087/1/src/soc/intel/tigerlake/rom... PS1, Line 213: m_cfg->TmeEnable = CONFIG(INTEL_TME); code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/45087/1/src/soc/intel/tigerlake/rom... PS1, Line 213: m_cfg->TmeEnable = CONFIG(INTEL_TME); please, no spaces at the start of a line
Hello Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45087
to look at the new patch set (#2).
Change subject: soc/intel/tigerlake: Add config option to enable TME ......................................................................
soc/intel/tigerlake: Add config option to enable TME
Add config option to set TmeEnable FSP-M upd. The TME spec is available at: "https://software.intel.com/sites/ default/files/managed/a5/16/Multi-Key-Total-Memory-Encryption- Spec.pdf"
Test: TME ENABLE and LOCK bits get set when Tme is enabled.
Signed-off-by: Pratik Prajapati pratikkumar.v.prajapati@intel.com Change-Id: I181aed2bf4a79005fe42e3e133b5faee91201dad --- M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/romstage/fsp_params.c 2 files changed, 11 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/45087/2
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45087 )
Change subject: soc/intel/tigerlake: Add config option to enable TME ......................................................................
Patch Set 2:
Can there be an option to select whether keys are thrown away on S3/resume or reused (Key_Select) ?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45087 )
Change subject: soc/intel/tigerlake: Add config option to enable TME ......................................................................
Patch Set 2: Code-Review+1
Pratikkumar V Prajapati has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45087 )
Change subject: soc/intel/tigerlake: Add config option to enable TME ......................................................................
Patch Set 2:
Patch Set 2:
Can there be an option to select whether keys are thrown away on S3/resume or reused (Key_Select) ?
What are you trying achieve with this? I dont see TME provides any access to set/retrieve keys. (MKTME allows to set some keys). With TME a new key is generated by HW on each reset, this same key is used during resume also.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45087 )
Change subject: soc/intel/tigerlake: Add config option to enable TME ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45087/2/src/soc/intel/tigerlake/Kco... File src/soc/intel/tigerlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/45087/2/src/soc/intel/tigerlake/Kco... PS2, Line 219: INTEL_TME move this to intel common directory so can be used across for other SoC as well
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45087 )
Change subject: soc/intel/tigerlake: Add config option to enable TME ......................................................................
Patch Set 2:
Patch Set 2:
Patch Set 2:
Can there be an option to select whether keys are thrown away on S3/resume or reused (Key_Select) ?
What are you trying achieve with this? I dont see TME provides any access to set/retrieve keys. (MKTME allows to set some keys). With TME a new key is generated by HW on each reset, this same key is used during resume also.
Isn't Key_Select one of the security features of TME? So can decide whether keys are reused for every resume or not
Hello build bot (Jenkins), Furquan Shaikh, Duncan Laurie, Subrata Banik, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45087
to look at the new patch set (#3).
Change subject: soc/intel/common: Add config option to enable TME/MKTME ......................................................................
soc/intel/common: Add config option to enable TME/MKTME
Add config option to enable TME/MKTME. The spec is available at: "https://software.intel.com/sites/ default/files/managed/a5/16/Multi-Key-Total-Memory-Encryption- Spec.pdf"
Signed-off-by: Pratik Prajapati pratikkumar.v.prajapati@intel.com Change-Id: I181aed2bf4a79005fe42e3e133b5faee91201dad --- M src/soc/intel/common/block/cpu/Kconfig M src/soc/intel/tigerlake/Kconfig 2 files changed, 18 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/45087/3
Pratikkumar V Prajapati has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45087 )
Change subject: soc/intel/common: Add config option to enable TME/MKTME ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45087/2/src/soc/intel/tigerlake/Kco... File src/soc/intel/tigerlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/45087/2/src/soc/intel/tigerlake/Kco... PS2, Line 219: INTEL_TME
move this to intel common directory so can be used across for other SoC as well
Done
Pratikkumar V Prajapati has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45087 )
Change subject: soc/intel/common: Add config option to enable TME/MKTME ......................................................................
Patch Set 3:
Patch Set 2:
Patch Set 2:
Patch Set 2:
Can there be an option to select whether keys are thrown away on S3/resume or reused (Key_Select) ?
What are you trying achieve with this? I dont see TME provides any access to set/retrieve keys. (MKTME allows to set some keys). With TME a new key is generated by HW on each reset, this same key is used during resume also.
Isn't Key_Select one of the security features of TME? So can decide whether keys are reused for every resume or not
With TME, there is no option to select any key. During each reset TME hardware generates a new random key and while entering s3, this key is stored internally. While resuming the same key is restored automatically. BIOS does not have direct access to the key. HW does the restore.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45087 )
Change subject: soc/intel/common: Add config option to enable TME/MKTME ......................................................................
Patch Set 3: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/45087/3/src/soc/intel/tigerlake/Kco... File src/soc/intel/tigerlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/45087/3/src/soc/intel/tigerlake/Kco... PS3, Line 219: INTEL_TME we don't need this anymore. your help text in common code very clear
Hello build bot (Jenkins), Furquan Shaikh, Duncan Laurie, Subrata Banik, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45087
to look at the new patch set (#4).
Change subject: soc/intel/common: Add config option to enable TME/MKTME ......................................................................
soc/intel/common: Add config option to enable TME/MKTME
Add config option to enable TME/MKTME. The spec is available at: "https://software.intel.com/sites/ default/files/managed/a5/16/Multi-Key-Total-Memory-Encryption- Spec.pdf"
Signed-off-by: Pratik Prajapati pratikkumar.v.prajapati@intel.com Change-Id: I181aed2bf4a79005fe42e3e133b5faee91201dad --- M src/soc/intel/common/block/cpu/Kconfig 1 file changed, 10 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/45087/4
Pratikkumar V Prajapati has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45087 )
Change subject: soc/intel/common: Add config option to enable TME/MKTME ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45087/3/src/soc/intel/tigerlake/Kco... File src/soc/intel/tigerlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/45087/3/src/soc/intel/tigerlake/Kco... PS3, Line 219: INTEL_TME
we don't need this anymore. […]
oh ya, i missed that. Thanks
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45087 )
Change subject: soc/intel/common: Add config option to enable TME/MKTME ......................................................................
Patch Set 4:
Since this is just a blanket "enable" bit to the FSP, wow about a little bit of documentation explaining how TME is configured, so users would know what behavior to expect?
For example, in chapter 4 of the doc you linked: " The maximum number of keys available/supported in the processor for MKTME are enumerated. BIOS will need to activate this capability via an MSR (described later) and it must select the number of keys to be supported/used for MKTME during early boot process. Upon activation, all memory (except in the TME exclusion range) attached to the CPU/SoC is encrypted using an AES-XTS 128 bit ephemeral key (platform key) that is generated by the CPU on every boot. "
How would a user know if MKTME is available and enabled (versus just TME), or activate an exclusion range?
Pratikkumar V Prajapati has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45087 )
Change subject: soc/intel/common: Add config option to enable TME/MKTME ......................................................................
Patch Set 4:
Patch Set 4:
Since this is just a blanket "enable" bit to the FSP, wow about a little bit of documentation explaining how TME is configured, so users would know what behavior to expect?
Is there any specific question? I think Nate may help from FSP's point of view.
For example, in chapter 4 of the doc you linked: " The maximum number of keys available/supported in the processor for MKTME are enumerated. BIOS will need to activate this capability via an MSR (described later) and it must select the number of keys to be supported/used for MKTME during early boot process. Upon activation, all memory (except in the TME exclusion range) attached to the CPU/SoC is encrypted using an AES-XTS 128 bit ephemeral key (platform key) that is generated by the CPU on every boot. "
How would a user know if MKTME is available and enabled (versus just TME), or activate an exclusion range?
IA32_TME_CAPABILITY MSR – 981H, bits 50:36 can say TME vs MKTME.
I dont see any UPD params for exclusion range. I am checking with FSP team internally. Nate may also have some idea from FSP side here.
Pratikkumar V Prajapati has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45087 )
Change subject: soc/intel/common: Add config option to enable TME/MKTME ......................................................................
Patch Set 4:
If there are no comments on the code, can we merge the patches?
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45087 )
Change subject: soc/intel/common: Add config option to enable TME/MKTME ......................................................................
Patch Set 4: Code-Review+1
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45087 )
Change subject: soc/intel/common: Add config option to enable TME/MKTME ......................................................................
Patch Set 4: Code-Review+2
please address unresolved comments
Pratikkumar V Prajapati has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45087 )
Change subject: soc/intel/common: Add config option to enable TME/MKTME ......................................................................
Patch Set 4:
Patch Set 4: Code-Review+2
please address unresolved comments
i dont see any unresolved comments. i clarified all ques from Tim
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45087 )
Change subject: soc/intel/common: Add config option to enable TME/MKTME ......................................................................
Patch Set 4:
Patch Set 4:
Patch Set 4: Code-Review+2
please address unresolved comments
i dont see any unresolved comments. i clarified all ques from Tim
All comments all have to be marked 'Done' before a patch can be submitted
Pratikkumar V Prajapati has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45087 )
Change subject: soc/intel/common: Add config option to enable TME/MKTME ......................................................................
Patch Set 4:
(1 comment)
Patch Set 4:
If there are no comments on the code, can we merge the patches?
https://review.coreboot.org/c/coreboot/+/45087/3/src/soc/intel/tigerlake/Kco... File src/soc/intel/tigerlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/45087/3/src/soc/intel/tigerlake/Kco... PS3, Line 219: INTEL_TME
oh ya, i missed that. […]
Done
Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/45087 )
Change subject: soc/intel/common: Add config option to enable TME/MKTME ......................................................................
soc/intel/common: Add config option to enable TME/MKTME
Add config option to enable TME/MKTME. The spec is available at: "https://software.intel.com/sites/ default/files/managed/a5/16/Multi-Key-Total-Memory-Encryption- Spec.pdf"
Signed-off-by: Pratik Prajapati pratikkumar.v.prajapati@intel.com Change-Id: I181aed2bf4a79005fe42e3e133b5faee91201dad Reviewed-on: https://review.coreboot.org/c/coreboot/+/45087 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org Reviewed-by: Subrata Banik subrata.banik@intel.com --- M src/soc/intel/common/block/cpu/Kconfig 1 file changed, 10 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Subrata Banik: Looks good to me, approved Tim Wawrzynczak: Looks good to me, but someone else must approve
diff --git a/src/soc/intel/common/block/cpu/Kconfig b/src/soc/intel/common/block/cpu/Kconfig index 1351cb8..995a956 100644 --- a/src/soc/intel/common/block/cpu/Kconfig +++ b/src/soc/intel/common/block/cpu/Kconfig @@ -88,3 +88,13 @@ help This option allows FSP to make use of MP services PPI published by coreboot to perform multiprocessor initialization. + +config INTEL_TME + bool "Total Memory Encryption (TME)/Multi-key TME (MKTME)" + default n + help + Enable Total Memory Encryption (TME)/Multi-key TME (MKTME). The spec is + available at "https://software.intel.com/sites/default/files/managed/a5 + /16/Multi-Key-Total-Memory-Encryption-Spec.pdf". If CPU supports TME, + it would get enabled. If CPU supports MKTME, this same config option + enables MKTME.