Re: [PATCH v32 08/21] x86/sgx: Initialize metadata for Enclave Page Cache (EPC) sections

2020-06-15 Thread Jarkko Sakkinen
On Mon, Jun 01, 2020 at 08:15:17AM -0700, Randy Dunlap wrote:
> Hi,
> 
> Sorry I didn't respond to v31 with this so that it could
> have been fixed in v32.

v32 was just a quick update to v31 (did both within maybe 2h window).

> 
> On 6/1/20 12:52 AM, Jarkko Sakkinen wrote:
> > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > index 2d3f963fd6f1..d246c6071e8d 100644
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -1948,6 +1948,22 @@ config X86_INTEL_TSX_MODE_AUTO
> >   side channel attacks- equals the tsx=auto command line parameter.
> >  endchoice
> >  
> > +config INTEL_SGX
> > +   bool "Intel SGX"
> > +   depends on X86_64 && CPU_SUP_INTEL
> > +   depends on CRYPTO=y
> > +   depends on CRYPTO_SHA256=y
> > +   select SRCU
> > +   select MMU_NOTIFIER
> > +   help
> > + Intel(R) SGX is a set of CPU instructions that can be used by
> > + applications to set aside private regions of code and data, referred
> > + to as enclaves. An enclave's private memory can only be accessed by
> > + code running within the enclave. Accesses from outside the enclave,
> > + including other enclaves, are disallowed by hardware.
> 
> Either the prompt
>   bool "Intel SGX"
> or the help text should tell us what SGX means.
> (Software Guard eXtensions)

Agreed.

> 
> > +
> > + If unsure, say N.
> > +
> >  config EFI
> > bool "EFI runtime service support"
> > depends on ACPI
> 
> thanks.
> -- 
> ~Randy
> 

Thanks for the feedback. I updated 
https://github.com/jsakkine-intel/linux-sgx.git
accordingly.

/Jarkko


Re: [PATCH v32 08/21] x86/sgx: Initialize metadata for Enclave Page Cache (EPC) sections

2020-06-01 Thread Randy Dunlap
Hi,

Sorry I didn't respond to v31 with this so that it could
have been fixed in v32.

On 6/1/20 12:52 AM, Jarkko Sakkinen wrote:
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 2d3f963fd6f1..d246c6071e8d 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1948,6 +1948,22 @@ config X86_INTEL_TSX_MODE_AUTO
> side channel attacks- equals the tsx=auto command line parameter.
>  endchoice
>  
> +config INTEL_SGX
> + bool "Intel SGX"
> + depends on X86_64 && CPU_SUP_INTEL
> + depends on CRYPTO=y
> + depends on CRYPTO_SHA256=y
> + select SRCU
> + select MMU_NOTIFIER
> + help
> +   Intel(R) SGX is a set of CPU instructions that can be used by
> +   applications to set aside private regions of code and data, referred
> +   to as enclaves. An enclave's private memory can only be accessed by
> +   code running within the enclave. Accesses from outside the enclave,
> +   including other enclaves, are disallowed by hardware.

Either the prompt
bool "Intel SGX"
or the help text should tell us what SGX means.
(Software Guard eXtensions)

> +
> +   If unsure, say N.
> +
>  config EFI
>   bool "EFI runtime service support"
>   depends on ACPI

thanks.
-- 
~Randy



[PATCH v32 08/21] x86/sgx: Initialize metadata for Enclave Page Cache (EPC) sections

2020-06-01 Thread Jarkko Sakkinen
From: Sean Christopherson 

Enumerate Enclave Page Cache (EPC) sections via CPUID and add the data
structures necessary to track EPC pages so that they can be easily borrowed
for different uses.

Embed section index to the first eight bits of the EPC page descriptor.
Existing client hardware supports only a single section, while upcoming
server hardware will support at most eight sections. Thus, eight bits
should be enough for long term needs.

Acked-by: Jethro Beekman 
Signed-off-by: Sean Christopherson 
Co-developed-by: Serge Ayoun 
Signed-off-by: Serge Ayoun 
Co-developed-by: Jarkko Sakkinen 
Signed-off-by: Jarkko Sakkinen 
---
 arch/x86/Kconfig |  16 +++
 arch/x86/kernel/cpu/Makefile |   1 +
 arch/x86/kernel/cpu/sgx/Makefile |   2 +
 arch/x86/kernel/cpu/sgx/main.c   | 216 +++
 arch/x86/kernel/cpu/sgx/sgx.h|  53 
 5 files changed, 288 insertions(+)
 create mode 100644 arch/x86/kernel/cpu/sgx/Makefile
 create mode 100644 arch/x86/kernel/cpu/sgx/main.c
 create mode 100644 arch/x86/kernel/cpu/sgx/sgx.h

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 2d3f963fd6f1..d246c6071e8d 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1948,6 +1948,22 @@ config X86_INTEL_TSX_MODE_AUTO
  side channel attacks- equals the tsx=auto command line parameter.
 endchoice
 
+config INTEL_SGX
+   bool "Intel SGX"
+   depends on X86_64 && CPU_SUP_INTEL
+   depends on CRYPTO=y
+   depends on CRYPTO_SHA256=y
+   select SRCU
+   select MMU_NOTIFIER
+   help
+ Intel(R) SGX is a set of CPU instructions that can be used by
+ applications to set aside private regions of code and data, referred
+ to as enclaves. An enclave's private memory can only be accessed by
+ code running within the enclave. Accesses from outside the enclave,
+ including other enclaves, are disallowed by hardware.
+
+ If unsure, say N.
+
 config EFI
bool "EFI runtime service support"
depends on ACPI
diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile
index 7dc4ad68eb41..45534fb81007 100644
--- a/arch/x86/kernel/cpu/Makefile
+++ b/arch/x86/kernel/cpu/Makefile
@@ -46,6 +46,7 @@ obj-$(CONFIG_X86_MCE) += mce/
 obj-$(CONFIG_MTRR) += mtrr/
 obj-$(CONFIG_MICROCODE)+= microcode/
 obj-$(CONFIG_X86_CPU_RESCTRL)  += resctrl/
+obj-$(CONFIG_INTEL_SGX)+= sgx/
 
 obj-$(CONFIG_X86_LOCAL_APIC)   += perfctr-watchdog.o
 
diff --git a/arch/x86/kernel/cpu/sgx/Makefile b/arch/x86/kernel/cpu/sgx/Makefile
new file mode 100644
index ..79510ce01b3b
--- /dev/null
+++ b/arch/x86/kernel/cpu/sgx/Makefile
@@ -0,0 +1,2 @@
+obj-y += \
+   main.o
diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
new file mode 100644
index ..c5831e3db14a
--- /dev/null
+++ b/arch/x86/kernel/cpu/sgx/main.c
@@ -0,0 +1,216 @@
+// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
+// Copyright(c) 2016-17 Intel Corporation.
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "encls.h"
+
+struct sgx_epc_section sgx_epc_sections[SGX_MAX_EPC_SECTIONS];
+static int sgx_nr_epc_sections;
+static struct task_struct *ksgxswapd_tsk;
+
+static void sgx_sanitize_section(struct sgx_epc_section *section)
+{
+   struct sgx_epc_page *page;
+   LIST_HEAD(secs_list);
+   int ret;
+
+   while (!list_empty(>unsanitized_page_list)) {
+   if (kthread_should_stop())
+   return;
+
+   spin_lock(>lock);
+
+   page = list_first_entry(>unsanitized_page_list,
+   struct sgx_epc_page, list);
+
+   ret = __eremove(sgx_get_epc_addr(page));
+   if (!ret)
+   list_move(>list, >page_list);
+   else
+   list_move_tail(>list, _list);
+
+   spin_unlock(>lock);
+
+   cond_resched();
+   }
+}
+
+static int ksgxswapd(void *p)
+{
+   int i;
+
+   set_freezable();
+
+   /*
+* Reset all pages to uninitialized state. Pages could be in initialized
+* on kmemexec.
+*/
+   for (i = 0; i < sgx_nr_epc_sections; i++)
+   sgx_sanitize_section(_epc_sections[i]);
+
+   /*
+* 2nd round for the SECS pages as they cannot be removed when they
+* still hold child pages.
+*/
+   for (i = 0; i < sgx_nr_epc_sections; i++) {
+   sgx_sanitize_section(_epc_sections[i]);
+
+   /* Should never happen. */
+   if (!list_empty(_epc_sections[i].unsanitized_page_list))
+   WARN(1, "EPC section %d has unsanitized pages.\n", i);
+   }
+
+   return 0;
+}
+
+static bool __init sgx_page_reclaimer_init(void)
+{
+   struct task_struct *tsk;
+
+