<p>Pratikkumar V Prajapati has uploaded this change for <strong>review</strong>.</p><p><a href="https://review.coreboot.org/21006">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">intel/common/block/sgx: Refactor SGX common code.<br><br>To correct the SGX init sequence; PRMRR on all cores first<br>needs to be set, then follow the SGX init sequence. This<br>patch would refactor the common SGX code (and add needed<br>checks in the init sequence) so that SOC specific code can<br>call SGX init in correct order.<br><br>Change-Id: Ic2fb00edbf6e98de17c12145c6f38eacd99399ad<br>Signed-off-by: Pratik Prajapati <pratikkumar.v.prajapati@intel.com><br>---<br>M src/soc/intel/common/block/include/intelblocks/sgx.h<br>M src/soc/intel/common/block/sgx/sgx.c<br>2 files changed, 47 insertions(+), 11 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/21006/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/src/soc/intel/common/block/include/intelblocks/sgx.h b/src/soc/intel/common/block/include/intelblocks/sgx.h<br>index 03d4ab5..cc2dc7f 100644<br>--- a/src/soc/intel/common/block/include/intelblocks/sgx.h<br>+++ b/src/soc/intel/common/block/include/intelblocks/sgx.h<br>@@ -27,4 +27,9 @@<br>  */<br> void sgx_configure(const void *microcode_patch);<br> <br>+/*<br>+ * Configure core PRMRR<br>+ */<br>+void prmrr_core_configure(void);<br>+<br> #endif   /* SOC_INTEL_COMMON_BLOCK_SGX_H */<br>diff --git a/src/soc/intel/common/block/sgx/sgx.c b/src/soc/intel/common/block/sgx/sgx.c<br>index 5a0b61d..53a68cf 100644<br>--- a/src/soc/intel/common/block/sgx/sgx.c<br>+++ b/src/soc/intel/common/block/sgx/sgx.c<br>@@ -33,11 +33,16 @@<br>      return ((cpuid_regs.ebx & SGX_SUPPORTED) && (msr.lo & PRMRR_SUPPORTED));<br> }<br> <br>-static int configure_core_prmrr(void)<br>+void prmrr_core_configure(void)<br> {<br>         msr_t prmrr_base;<br>     msr_t prmrr_mask;<br>     msr_t msr;<br>+   device_t dev = SA_DEV_ROOT;<br>+  config_t *conf = dev->chip_info;<br>+<br>+       if (!conf->sgx_enable || !is_sgx_supported())<br>+             return;<br> <br>    /*<br>     * PRMRR base and mask are read from the UNCORE PRMRR MSRs<br>@@ -47,13 +52,13 @@<br>       prmrr_mask = rdmsr(UNCORE_PRMRR_PHYS_MASK_MSR);<br>       if (!prmrr_base.lo) {<br>                 printk(BIOS_ERR, "SGX Error: Uncore PRMRR is not set!\n");<br>-         return -1;<br>+           return;<br>       }<br> <br>  msr = rdmsr(PRMRR_PHYS_MASK_MSR);<br>     /* If it is locked don't attempt to write PRMRR MSRs. */<br>  if (msr.lo & PRMRR_PHYS_MASK_LOCK)<br>-               return 0;<br>+            return;<br> <br>    /* Program core PRMRR MSRs */<br>         prmrr_base.lo |= MTRR_TYPE_WRBACK; /* cache writeback mem attrib */<br>@@ -61,7 +66,20 @@<br>       prmrr_mask.lo &= ~PRMRR_PHYS_MASK_VALID; /* Do not set the valid bit */<br>   prmrr_mask.lo |= PRMRR_PHYS_MASK_LOCK; /* Lock it */<br>  wrmsr(PRMRR_PHYS_MASK_MSR, prmrr_mask);<br>-      return 0;<br>+}<br>+<br>+static int is_prmrr_set(void)<br>+{<br>+ msr_t prmrr_base, prmrr_mask;<br>+        prmrr_base = rdmsr(PRMRR_PHYS_BASE_MSR);<br>+     prmrr_mask = rdmsr(PRMRR_PHYS_MASK_MSR);<br>+<br>+  /* if PRMRR base is zero and PRMRR mask is locked<br>+    then PRMRR is not set */<br>+     if (prmrr_base.hi && prmrr_base.lo<br>+           && (prmrr_mask.lo & PRMRR_PHYS_MASK_LOCK))<br>+               return 0;<br>+    return 1;<br> }<br> <br> static void enable_sgx(void)<br>@@ -125,17 +143,28 @@<br>        }<br> }<br> <br>+static int is_prmrr_approved(void)<br>+{<br>+    msr_t msr;<br>+   msr = rdmsr(PRMRR_PHYS_MASK_MSR);<br>+    if (msr.lo & PRMRR_PHYS_MASK_VALID) {<br>+            printk(BIOS_INFO, "SGX: MCHECK aprroved SGX PRMRR\n");<br>+             return 1;<br>+    }<br>+<br>+ printk(BIOS_INFO, "SGX: MCHECK didnot aprrove SGX PRMRR\n");<br>+       return 0;<br>+}<br>+<br> void sgx_configure(const void *microcode_patch)<br> {<br>        device_t dev = SA_DEV_ROOT;<br>   config_t *conf = dev->chip_info;<br> <br>-       if (!conf->sgx_enable || !is_sgx_supported())<br>+     if (!conf->sgx_enable || !is_sgx_supported() || !is_prmrr_set()) {<br>+                printk(BIOS_ERR, "SGX: pre-conditions not met\n");<br>          return;<br>-<br>-   /* Initialize PRMRR core MSRs */<br>-     if (configure_core_prmrr() < 0)<br>-           return;<br>+      }<br> <br>  /* Enable the SGX feature */<br>  enable_sgx();<br>@@ -153,6 +182,8 @@<br>    /* Lock the SGX feature */<br>    lock_sgx();<br> <br>-       /* Activate the SGX feature */<br>-       activate_sgx();<br>+      /* Activate the SGX feature, if PRMRR configuration<br>+  was aprroved by MCHECK */<br>+    if (is_prmrr_approved())<br>+             activate_sgx();<br> }<br></pre><p>To view, visit <a href="https://review.coreboot.org/21006">change 21006</a>. To unsubscribe, visit <a href="https://review.coreboot.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://review.coreboot.org/21006"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: coreboot </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: Ic2fb00edbf6e98de17c12145c6f38eacd99399ad </div>
<div style="display:none"> Gerrit-Change-Number: 21006 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Pratikkumar V Prajapati <pratikkumar.v.prajapati@intel.com> </div>