Patch Set 2:

Patch Set 2:

Patch Set 2:

Patch Set 2:

Patch Set 2:

Patch Set 2:

if i remember correctly, this programming sequence requirement was from cannon-lake onwards. skylake/kabylake can still use native (coreboot) sgx init flow.

On SKL LT_MEMORY_LOCK has to be set, too, and FSP does this with SkipMpInit=0. AFAICT native SGX init can't be used on SKL for the same reasons we can't use it on CNL.


From cannon-lake onwards, sgx init needs fsp for the reasons mentioned in https://review.coreboot.org/c/coreboot/+/36356/2/src/soc/intel/cannonlake/finalize.c#116. i guess mp-ppi may also be used for cannon-lake onwards. Please follow Nate's recommendation for that.

PS: I see my gmail was added earlier and that notification was in spam. Thanks for adding my @intel email. I am off next week, I’ll respond the following week if there are follow-up questions.

See CB:36356. Native SGX init is not used in CNL, so there is nothing more to do.

"FSP-S must be able to write and lock some registers that would be immutable after setting LT_LOCK_MEMORY."* I dont think i got into this issue when i wrote native sgx init flow. At that time i was able to activate the SGX and lock it correctly by using native code only. i created enclaves also from the application and checked it was encrypted and working as expected.

@Nate: is this* valid for SKL/KBL as well?

Yes, this is valid and exactly the point, why native SGX init can't be used. LT_MEMORY_LOCK must be set, *before* native SGX init (well, more specifically before microcode reload). That conflicts with the requirement of FSP-S to be able to write registers.

At that time i was able to activate the SGX and lock it correctly by using native code only. i created enclaves also from the application and checked it was encrypted and working as expected

Yes, SGX works. No, not in a secure way with native SGX init without LT_MEMORY_LOCK.

in my original code i.e. native sgx init, i was doing exactly that.
lock LT mem, reload ucode and so on. (my original code below - commit 418535e222ccd9f688facbb7d9663ca4cacc2739) which was working.

void sgx_configure(void)
{
const void *microcode_patch = intel_mp_current_microcode();
        if (!soc_sgx_enabled() || !is_sgx_supported() || !is_prmrr_set()) {
printk(BIOS_ERR, "SGX: pre-conditions not met\n");
return;
}
        /* Enable the SGX feature */
enable_sgx();
        /* Update the owner epoch value */
if (owner_epoch_update() < 0)
return;
        /* Ensure to lock memory before reload microcode patch */
cpu_lock_sgx_memory();
        /* Reload the microcode patch */
intel_microcode_load_unlocked(microcode_patch);
        /* Lock the SGX feature */
lock_sgx();
        /* Activate the SGX feature, if PRMRR config was aprroved by MCHECK */
if (is_prmrr_approved())
activate_sgx();
}

I'll let Nate comment on "FSP-S must be able to write and lock some registers that would be immutable after setting LT_LOCK_MEMORY."*

View Change

To view, visit change 44244. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I4251c6f1155ae7d1f0cd9064af55cd346ec88845
Gerrit-Change-Number: 44244
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Niewöhner
Gerrit-Reviewer: Felix Singer <felixsinger@posteo.net>
Gerrit-Reviewer: Aaron Durbin <adurbin@chromium.org>
Gerrit-Reviewer: Nathaniel L Desimone <nathaniel.l.desimone@intel.com>
Gerrit-Reviewer: Nico Huber <nico.h@gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com>
Gerrit-Reviewer: Patrick Rudolph
Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter@users.sourceforge.net>
Gerrit-Reviewer: Pratik Prajapati <pratik.prajapati@gmail.com>
Gerrit-Reviewer: Pratikkumar V Prajapati <pratikkumar.v.prajapati@intel.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-Comment-Date: Fri, 07 Aug 2020 22:56:44 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment