Change in coreboot[master]: soc/intel/skylake: use cpu_lt_lock_memory in cpu_lock_sgx_memory

[Michael Niewöhner (Code Review)]

Michael Niewöhner has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36354 )


Change subject: soc/intel/skylake: use cpu_lt_lock_memory in cpu_lock_sgx_memory
......................................................................

soc/intel/skylake: use cpu_lt_lock_memory in cpu_lock_sgx_memory

Use the new common function to set LT_LOCK_MEMORY prior to SGX
activation.

Change-Id: Iefec0e61c7482a70af60dabc0bec3bf712d8b48a
Signed-off-by: Michael Niewöhner <[e-mail]>
---
M src/soc/intel/skylake/cpu.c
1 file changed, 1 insertion(+), 7 deletions(-)



  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/36354/1

diff --git a/src/soc/intel/skylake/cpu.c b/src/soc/intel/skylake/cpu.c
index 63142b9..6e77938 100644
--- a/src/soc/intel/skylake/cpu.c
+++ b/src/soc/intel/skylake/cpu.c
@@ -578,11 +578,5 @@
 
 void cpu_lock_sgx_memory(void)
 {
-	msr_t msr;
-
-	msr = rdmsr(MSR_LT_LOCK_MEMORY);
-	if ((msr.lo & 1) == 0) {
-		msr.lo |= 1; /* Lock it */
-		wrmsr(MSR_LT_LOCK_MEMORY, msr);
-	}
+	cpu_lt_lock_memory(NULL);
 }

-- 
To view, visit https://review.coreboot.org/c/coreboot/+/36354
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iefec0e61c7482a70af60dabc0bec3bf712d8b48a
Gerrit-Change-Number: 36354
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Niewöhner
Gerrit-Reviewer: Michael Niewöhner
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-MessageType: newchange

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36354 )

Change subject: soc/intel/skylake: use cpu_lt_lock_memory in cpu_lock_sgx_memory
......................................................................


Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/36354/1/src/soc/intel/skylake/cpu.c 
File src/soc/intel/skylake/cpu.c:

https://review.coreboot.org/c/coreboot/+/36354/1/src/soc/intel/skylake/cpu.c... 
PS1, Line 579: void cpu_lock_sgx_memory(void)
Is this API still needed after the next patch?



-- 
To view, visit https://review.coreboot.org/c/coreboot/+/36354
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iefec0e61c7482a70af60dabc0bec3bf712d8b48a
Gerrit-Change-Number: 36354
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Niewöhner
Gerrit-Reviewer: Arthur Heymans <[e-mail]>
Gerrit-Reviewer: Michael Niewöhner
Gerrit-Reviewer: Nico Huber <[e-mail]>
Gerrit-Reviewer: Patrick Georgi <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: Subrata Banik <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-Comment-Date: Wed, 30 Oct 2019 22:35:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

[Michael Niewöhner (Code Review)]

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36354 )

Change subject: soc/intel/skylake: use cpu_lt_lock_memory in cpu_lock_sgx_memory
......................................................................


Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/36354/1/src/soc/intel/skylake/cpu.c 
File src/soc/intel/skylake/cpu.c:

https://review.coreboot.org/c/coreboot/+/36354/1/src/soc/intel/skylake/cpu.c... 
PS1, Line 579: void cpu_lock_sgx_memory(void)
> Is this API still needed after the next patch?
> Yep, because when SGX is enabled the lock *MUST* happen before SGX init; every platform after skl/kbl does not need to do the lock themselves, which is handled there MCHECK

[Michael Niewöhner (Code Review)]

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36354 )

Change subject: soc/intel/skylake: use cpu_lt_lock_memory in cpu_lock_sgx_memory
......................................................................


Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/36354/1/src/soc/intel/skylake/cpu.c 
File src/soc/intel/skylake/cpu.c:

https://review.coreboot.org/c/coreboot/+/36354/1/src/soc/intel/skylake/cpu.c... 
PS1, Line 579: void cpu_lock_sgx_memory(void)
> Yep, because when SGX is enabled the lock *MUST* happen before SGX init; every platform after skl/kb […]
> ... so cpu_lock_sgx_memory() must be empty for those platforms

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36354 )

Change subject: soc/intel/skylake: use cpu_lt_lock_memory in cpu_lock_sgx_memory
......................................................................


Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/36354/1/src/soc/intel/skylake/cpu.c 
File src/soc/intel/skylake/cpu.c:

https://review.coreboot.org/c/coreboot/+/36354/1/src/soc/intel/skylake/cpu.c... 
PS1, Line 579: void cpu_lock_sgx_memory(void)
> ... […]
> Ok, I get it. But if there is no other way of doing it, why have a hook
> for it. A simple `config ..._SGX_LOCK_MEMORY` should work too.

[Michael Niewöhner (Code Review)]

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36354 )

Change subject: soc/intel/skylake: use cpu_lt_lock_memory in cpu_lock_sgx_memory
......................................................................


Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/36354/1/src/soc/intel/skylake/cpu.c 
File src/soc/intel/skylake/cpu.c:

https://review.coreboot.org/c/coreboot/+/36354/1/src/soc/intel/skylake/cpu.c... 
PS1, Line 579: void cpu_lock_sgx_memory(void)
> ... […]
> Done

[Michael Niewöhner (Code Review)]

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36354 )

Change subject: soc/intel/skylake: use cpu_lt_lock_memory in cpu_lock_sgx_memory
......................................................................


Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/36354/1/src/soc/intel/skylake/cpu.c 
File src/soc/intel/skylake/cpu.c:

https://review.coreboot.org/c/coreboot/+/36354/1/src/soc/intel/skylake/cpu.c... 
PS1, Line 579: void cpu_lock_sgx_memory(void)
> Done
> oops. not done. good plan, will do it this way

Change in coreboot[master]: soc/intel/common: sgx: use cpu_lt_lock_memory in sgx setup

[Michael Niewöhner (Code Review)]

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36354 )

Change subject: soc/intel/common: sgx: use cpu_lt_lock_memory in sgx setup
......................................................................


Patch Set 2:

This change is ready for review.


-- 
To view, visit https://review.coreboot.org/c/coreboot/+/36354
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iefec0e61c7482a70af60dabc0bec3bf712d8b48a
Gerrit-Change-Number: 36354
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Niewöhner
Gerrit-Reviewer: Arthur Heymans <[e-mail]>
Gerrit-Reviewer: Michael Niewöhner
Gerrit-Reviewer: Nico Huber <[e-mail]>
Gerrit-Reviewer: Patrick Georgi <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: Subrata Banik <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-Comment-Date: Thu, 31 Oct 2019 18:07:30 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Change in coreboot[master]: soc/intel/common: sgx: use cpu_lt_lock_memory in sgx setup

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36354 )

Change subject: soc/intel/common: sgx: use cpu_lt_lock_memory in sgx setup
......................................................................


Patch Set 2: Code-Review+2


-- 
To view, visit https://review.coreboot.org/c/coreboot/+/36354
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iefec0e61c7482a70af60dabc0bec3bf712d8b48a
Gerrit-Change-Number: 36354
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Niewöhner
Gerrit-Reviewer: Arthur Heymans <[e-mail]>
Gerrit-Reviewer: Michael Niewöhner
Gerrit-Reviewer: Nico Huber <[e-mail]>
Gerrit-Reviewer: Patrick Georgi <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: Subrata Banik <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-Comment-Date: Thu, 31 Oct 2019 21:28:12 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in coreboot[master]: soc/intel/common: sgx: use cpu_lt_lock_memory in sgx setup

[Nico Huber (Code Review)]

Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/36354 )

Change subject: soc/intel/common: sgx: use cpu_lt_lock_memory in sgx setup
......................................................................

soc/intel/common: sgx: use cpu_lt_lock_memory in sgx setup

Use the new common function to set LT_LOCK_MEMORY prior to SGX
activation based on Kconfig.

Change-Id: Iefec0e61c7482a70af60dabc0bec3bf712d8b48a
Signed-off-by: Michael Niewöhner <[e-mail]>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/36354
Tested-by: build bot (Jenkins) <[e-mail]>
Reviewed-by: Nico Huber <[e-mail]>
---
M src/soc/intel/common/block/sgx/sgx.c
1 file changed, 4 insertions(+), 2 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Nico Huber: Looks good to me, approved



diff --git a/src/soc/intel/common/block/sgx/sgx.c b/src/soc/intel/common/block/sgx/sgx.c
index b12e6cf..842eb43 100644
--- a/src/soc/intel/common/block/sgx/sgx.c
+++ b/src/soc/intel/common/block/sgx/sgx.c
@@ -18,6 +18,7 @@
 #include <cpu/x86/mtrr.h>
 #include <cpu/intel/microcode.h>
 #include <cpu/intel/common/common.h>
+#include <intelblocks/cpulib.h>
 #include <intelblocks/mp_init.h>
 #include <intelblocks/msr.h>
 #include <intelblocks/sgx.h>
@@ -216,8 +217,9 @@
 	if (owner_epoch_update() < 0)
 		return;
 
-	/* Ensure to lock memory before reload microcode patch */
-	cpu_lock_sgx_memory();
+	if (CONFIG(SOC_INTEL_COMMON_BLOCK_SGX_LOCK_MEMORY))
+		/* Ensure to lock memory before reload microcode patch */
+		cpu_lt_lock_memory(NULL);
 
 	/*
 	 * Update just on the first CPU in the core. Other siblings

-- 
To view, visit https://review.coreboot.org/c/coreboot/+/36354
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iefec0e61c7482a70af60dabc0bec3bf712d8b48a
Gerrit-Change-Number: 36354
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Niewöhner
Gerrit-Reviewer: Arthur Heymans <[e-mail]>
Gerrit-Reviewer: Michael Niewöhner
Gerrit-Reviewer: Nico Huber <[e-mail]>
Gerrit-Reviewer: Patrick Georgi <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: Subrata Banik <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-MessageType: merged