[PATCH v12 00/13] Intel SGX1 support

[PATCH v12 00/13] Intel SGX1 support

[Jarkko Sakkinen]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 11978 bytes --]

Intel(R) SGX is a set of CPU instructions that can be used by applications
to set aside private regions of code and data. The code outside the enclave
is disallowed to access the memory inside the enclave by the CPU access
control.  In a way you can think that SGX provides inverted sandbox. It
protects the application from a malicious host.

There is a new hardware unit in the processor called Memory Encryption
Engine (MEE) starting from the Skylake microacrhitecture. BIOS can define
one or many MEE regions that can hold enclave data by configuring them with
PRMRR registers.

The MEE automatically encrypts the data leaving the processor package to
the MEE regions. The data is encrypted using a random key whose life-time
is exactly one power cycle.

The current implementation requires that the firmware sets
IA32_SGXLEPUBKEYHASH* MSRs as writable so that ultimately the kernel can
decide what enclaves it wants run. The implementation does not create
any bottlenecks to support read-only MSRs later on.

You can tell if your CPU supports SGX by looking into /proc/cpuinfo:

	cat /proc/cpuinfo  | grep sgx

v12:
* Split to more narrow scoped commits in order to ease the review process and
  use co-developed-by tag for co-authors of commits instead of listing them in
  the source files.
* Removed cruft EXPORT_SYMBOL() declarations and converted to static variables.
* Removed in-kernel LE i.e. this version of the SGX software stack only
  supports unlocked IA32_SGXLEPUBKEYHASHx MSRs.
* Refined documentation on launching enclaves, swapping and enclave
  construction.
* Refined sgx_arch.h to include alignment information for every struct that
  requires it and removed structs that are not needed without an LE.
* Got rid of SGX_CPUID.
* SGX detection now prints log messages about firmware configuration issues.

v11:
* Polished ENCLS wrappers with refined exception handling.
* ksgxswapd was not stopped (regression in v5) in
  sgx_page_cache_teardown(), which causes a leaked kthread after driver
  deinitialization.
* Shutdown sgx_le_proxy when going to suspend because its EPC pages will be
  invalidated when resuming, which will cause it not function properly
  anymore.
* Set EINITTOKEN.VALID to zero for a token that is passed when
  SGXLEPUBKEYHASH matches MRSIGNER as alloc_page() does not give a zero
  page.
* Fixed the check in sgx_edbgrd() for a TCS page. Allowed to read offsets
  around the flags field, which causes a #GP. Only flags read is readable.
* On read access memcpy() call inside sgx_vma_access() had src and dest
  parameters in wrong order.
* The build issue with CONFIG_KASAN is now fixed. Added undefined symbols
  to LE even if “KASAN_SANITIZE := false” was set in the makefile. 
* Fixed a regression in the #PF handler. If a page has
  SGX_ENCL_PAGE_RESERVED flag the #PF handler should unconditionally fail.
  It did not, which caused weird races when trying to change other parts of
  swapping code.
* EPC management has been refactored to a flat LRU cache and moved to
  arch/x86. The swapper thread reads a cluster of EPC pages and swaps all
  of them. It can now swap from multiple enclaves in the same round.
* For the sake of consistency with SGX_IOC_ENCLAVE_ADD_PAGE, return -EINVAL
  when an enclave is already initialized or dead instead of zero.

v10:
* Cleaned up anon inode based IPC between the ring-0 and ring-3 parts
  of the driver.
* Unset the reserved flag from an enclave page if EDBGRD/WR fails
  (regression in v6).
* Close the anon inode when LE is stopped (regression in v9).
* Update the documentation with a more detailed description of SGX.

v9:
* Replaced kernel-LE IPC based on pipes with an anonymous inode.
  The driver does not require anymore new exports.

v8:
* Check that public key MSRs match the LE public key hash in the
  driver initialization when the MSRs are read-only.
* Fix the race in VA slot allocation by checking the fullness
  immediately after succeesful allocation.
* Fix the race in hash mrsigner calculation between the launch
  enclave and user enclaves by having a separate lock for hash
  calculation.

v7:
* Fixed offset calculation in sgx_edbgr/wr(). Address was masked with PAGE_MASK
  when it should have been masked with ~PAGE_MASK.
* Fixed a memory leak in sgx_ioc_enclave_create().
* Simplified swapping code by using a pointer array for a cluster
  instead of a linked list.
* Squeezed struct sgx_encl_page to 32 bytes.
* Fixed deferencing of an RSA key on OpenSSL 1.1.0.
* Modified TC's CMAC to use kernel AES-NI. Restructured the code
  a bit in order to better align with kernel conventions.

v6:
* Fixed semaphore underrun when accessing /dev/sgx from the launch enclave.
* In sgx_encl_create() s/IS_ERR(secs)/IS_ERR(encl)/.
* Removed virtualization chapter from the documentation.
* Changed the default filename for the signing key as signing_key.pem.
* Reworked EPC management in a way that instead of a linked list of
  struct sgx_epc_page instances there is an array of integers that
  encodes address and bank of an EPC page (the same data as 'pa' field
  earlier). The locking has been moved to the EPC bank level instead
  of a global lock.
* Relaxed locking requirements for EPC management. EPC pages can be
  released back to the EPC bank concurrently.
* Cleaned up ptrace() code.
* Refined commit messages for new architectural constants.
* Sorted includes in every source file.
* Sorted local variable declarations according to the line length in
  every function.
* Style fixes based on Darren's comments to sgx_le.c.

v5:
* Described IPC between the Launch Enclave and kernel in the commit messages.
* Fixed all relevant checkpatch.pl issues that I have forgot fix in earlier
  versions except those that exist in the imported TinyCrypt code.
* Fixed spelling mistakes in the documentation.
* Forgot to check the return value of sgx_drv_subsys_init().
* Encapsulated properly page cache init and teardown.
* Collect epc pages to a temp list in sgx_add_epc_bank
* Removed SGX_ENCLAVE_INIT_ARCH constant.

v4:
* Tied life-cycle of the sgx_le_proxy process to /dev/sgx.
* Removed __exit annotation from sgx_drv_subsys_exit().
* Fixed a leak of a backing page in sgx_process_add_page_req() in the
  case when vm_insert_pfn() fails.
* Removed unused symbol exports for sgx_page_cache.c.
* Updated sgx_alloc_page() to require encl parameter and documented the
  behavior (Sean Christopherson).
* Refactored a more lean API for sgx_encl_find() and documented the behavior.
* Moved #PF handler to sgx_fault.c.
* Replaced subsys_system_register() with plain bus_register().
* Retry EINIT 2nd time only if MSRs are not locked.

v3:
* Check that FEATURE_CONTROL_LOCKED and FEATURE_CONTROL_SGX_ENABLE are set.
* Return -ERESTARTSYS in __sgx_encl_add_page() when sgx_alloc_page() fails.
* Use unused bits in epc_page->pa to store the bank number.
* Removed #ifdef for WQ_NONREENTRANT.
* If mmu_notifier_register() fails with -EINTR, return -ERESTARTSYS.
* Added --remove-section=.got.plt to objcopy flags in order to prevent a
  dummy .got.plt, which will cause an inconsistent size for the LE.
* Documented sgx_encl_* functions.
* Added remark about AES implementation used inside the LE.
* Removed redundant sgx_sys_exit() from le/main.c.
* Fixed struct sgx_secinfo alignment from 128 to 64 bytes.
* Validate miscselect in sgx_encl_create().
* Fixed SSA frame size calculation to take the misc region into account.
* Implemented consistent exception handling to __encls() and __encls_ret().
* Implemented a proper device model in order to allow sysfs attributes
  and in-kernel API.
* Cleaned up various "find enclave" implementations to the unified
  sgx_encl_find().
* Validate that vm_pgoff is zero.
* Discard backing pages with shmem_truncate_range() after EADD.
* Added missing EEXTEND operations to LE signing and launch.
* Fixed SSA size for GPRS region from 168 to 184 bytes.
* Fixed the checks for TCS flags. Now DBGOPTIN is allowed.
* Check that TCS addresses are in ELRANGE and not just page aligned.
* Require kernel to be compiled with X64_64 and CPU_SUP_INTEL.
* Fixed an incorrect value for SGX_ATTR_DEBUG from 0x01 to 0x02.

v2:
* get_rand_uint32() changed the value of the pointer instead of value
  where it is pointing at.
* Launch enclave incorrectly used sigstruct attributes-field instead of
  enclave attributes-field.
* Removed unused struct sgx_add_page_req from sgx_ioctl.c
* Removed unused sgx_has_sgx2.
* Updated arch/x86/include/asm/sgx.h so that it provides stub
  implementations when sgx in not enabled.
* Removed cruft rdmsr-calls from sgx_set_pubkeyhash_msrs().
* return -ENOMEM in sgx_alloc_page() when VA pages consume too much space
* removed unused global sgx_nr_pids
* moved sgx_encl_release to sgx_encl.c
* return -ERESTARTSYS instead of -EINTR in sgx_encl_init()

Jarkko Sakkinen (8):
  x86/sgx: updated MAINTAINERS
  x86/sgx: architectural structures
  x86/sgx: data structures for tracking available EPC pages
  x86/sgx: wrappers for ENCLS opcode leaf functions
  x86/sgx: EPC page allocation routines
  platform/x86: Intel SGX driver
  platform/x86: ptrace() support for the SGX driver
  x86/sgx: driver documentation

Kai Huang (1):
  x86/sgx: add SGX definitions to cpufeature

Sean Christopherson (4):
  x86/sgx: add SGX definitions to msr-index.h
  x86/cpufeatures: add Intel-defined SGX leaf CPUID_12_EAX
  x86/sgx: detect Intel SGX
  x86/sgx: sgx_einit()

 Documentation/index.rst                       |    1 +
 Documentation/x86/intel_sgx.rst               |  185 +++
 MAINTAINERS                                   |    7 +
 arch/x86/Kconfig                              |   19 +
 arch/x86/include/asm/cpufeature.h             |    7 +-
 arch/x86/include/asm/cpufeatures.h            |   10 +-
 arch/x86/include/asm/disabled-features.h      |    3 +-
 arch/x86/include/asm/msr-index.h              |    8 +
 arch/x86/include/asm/required-features.h      |    3 +-
 arch/x86/include/asm/sgx.h                    |  246 ++++
 arch/x86/include/asm/sgx_arch.h               |  202 ++++
 arch/x86/include/asm/sgx_pr.h                 |   29 +
 arch/x86/include/uapi/asm/sgx.h               |  138 +++
 arch/x86/kernel/cpu/Makefile                  |    1 +
 arch/x86/kernel/cpu/common.c                  |    7 +
 arch/x86/kernel/cpu/intel_sgx.c               |  460 ++++++++
 arch/x86/kvm/cpuid.h                          |    1 +
 drivers/platform/x86/Kconfig                  |    2 +
 drivers/platform/x86/Makefile                 |    1 +
 drivers/platform/x86/intel_sgx/Kconfig        |   22 +
 drivers/platform/x86/intel_sgx/Makefile       |   13 +
 drivers/platform/x86/intel_sgx/sgx.h          |  157 +++
 drivers/platform/x86/intel_sgx/sgx_encl.c     | 1011 +++++++++++++++++
 .../platform/x86/intel_sgx/sgx_encl_page.c    |  286 +++++
 drivers/platform/x86/intel_sgx/sgx_fault.c    |  151 +++
 drivers/platform/x86/intel_sgx/sgx_ioctl.c    |  231 ++++
 drivers/platform/x86/intel_sgx/sgx_main.c     |  298 +++++
 drivers/platform/x86/intel_sgx/sgx_vma.c      |  174 +++
 28 files changed, 3668 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/x86/intel_sgx.rst
 create mode 100644 arch/x86/include/asm/sgx.h
 create mode 100644 arch/x86/include/asm/sgx_arch.h
 create mode 100644 arch/x86/include/asm/sgx_pr.h
 create mode 100644 arch/x86/include/uapi/asm/sgx.h
 create mode 100644 arch/x86/kernel/cpu/intel_sgx.c
 create mode 100644 drivers/platform/x86/intel_sgx/Kconfig
 create mode 100644 drivers/platform/x86/intel_sgx/Makefile
 create mode 100644 drivers/platform/x86/intel_sgx/sgx.h
 create mode 100644 drivers/platform/x86/intel_sgx/sgx_encl.c
 create mode 100644 drivers/platform/x86/intel_sgx/sgx_encl_page.c
 create mode 100644 drivers/platform/x86/intel_sgx/sgx_fault.c
 create mode 100644 drivers/platform/x86/intel_sgx/sgx_ioctl.c
 create mode 100644 drivers/platform/x86/intel_sgx/sgx_main.c
 create mode 100644 drivers/platform/x86/intel_sgx/sgx_vma.c

-- 
2.17.1

[PATCH v12 01/13] x86/sgx: updated MAINTAINERS

[Jarkko Sakkinen]

Updated the MAINTAINERS file to point to the Intel SGX subsystem tree.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index ca4afd68530c..12f7c68378e8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7329,6 +7329,13 @@ L:	linux-gpio@vger.kernel.org
 S:	Maintained
 F:	drivers/gpio/gpio-intel-mid.c
 
+INTEL SGX
+M:	Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
+L:	linux-sgx@vger.kernel.org
+Q:	https://patchwork.kernel.org/project/intel-sgx/list/
+F:	drivers/platform/x86/intel_sgx/
+K:	\bSGX_
+
 INVENSENSE MPU-3050 GYROSCOPE DRIVER
 M:	Linus Walleij <linus.walleij@linaro.org>
 L:	linux-iio@vger.kernel.org
-- 
2.17.1

[PATCH v12 02/13] x86/sgx: add SGX definitions to cpufeature

[Jarkko Sakkinen]

From: Kai Huang <kai.huang@linux.intel.com>

Added X86_FEATURE_SGX and X86_FEATURE_SGX_LC definitions that define the
bits CPUID level 7 bits for determining whether the CPU supports SGX and
launch configuration other than the Intel proprietary key. If this the
case, IA32_SGXLEPUBKEYHASHn MSRs (0 < n < 4) are available for defining the
root key for enclaves.

Signed-off-by: Kai Huang <kai.huang@linux.intel.com>
---
 arch/x86/include/asm/cpufeatures.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index fb00a2fca990..54d5269e1b86 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -234,6 +234,7 @@
 /* Intel-defined CPU features, CPUID level 0x00000007:0 (EBX), word 9 */
 #define X86_FEATURE_FSGSBASE		( 9*32+ 0) /* RDFSBASE, WRFSBASE, RDGSBASE, WRGSBASE instructions*/
 #define X86_FEATURE_TSC_ADJUST		( 9*32+ 1) /* TSC adjustment MSR 0x3B */
+#define X86_FEATURE_SGX			( 9*32+ 2) /* Software Guard Extensions */
 #define X86_FEATURE_BMI1		( 9*32+ 3) /* 1st group bit manipulation extensions */
 #define X86_FEATURE_HLE			( 9*32+ 4) /* Hardware Lock Elision */
 #define X86_FEATURE_AVX2		( 9*32+ 5) /* AVX2 instructions */
@@ -327,6 +328,7 @@
 #define X86_FEATURE_LA57		(16*32+16) /* 5-level page tables */
 #define X86_FEATURE_RDPID		(16*32+22) /* RDPID instruction */
 #define X86_FEATURE_CLDEMOTE		(16*32+25) /* CLDEMOTE instruction */
+#define X86_FEATURE_SGX_LC		(16*32+30) /* supports SGX launch configuration */
 
 /* AMD-defined CPU features, CPUID level 0x80000007 (EBX), word 17 */
 #define X86_FEATURE_OVERFLOW_RECOV	(17*32+ 0) /* MCA overflow recovery support */
-- 
2.17.1

[PATCH v12 03/13] x86/sgx: add SGX definitions to msr-index.h

[Jarkko Sakkinen]

From: Sean Christopherson <sean.j.christopherson@intel.com>

ENCLS and ENCLU are usable if and only if SGX_ENABLE is set and After
SGX is activated the IA32_SGXLEPUBKEYHASHn MSRs are writable if
SGX_LC_WR is set and the feature control is locked.

SGX related bits in IA32_FEATURE_CONTROL cannot be set before SGX is
activated by the pre-boot firmware. SGX activation is triggered by
setting bit 0 in the MSR 0x7a. Until SGX is activated, the LE hash MSRs
are writable to allow pre-boot firmware to lock down the LE root key
with a non-Intel value.

Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Co-developed-by: Haim Cohen <haim.cohen@intel.com>
---
 arch/x86/include/asm/msr-index.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index fda2114197b3..a7355fb7344f 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -479,6 +479,8 @@
 #define FEATURE_CONTROL_LOCKED				(1<<0)
 #define FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX	(1<<1)
 #define FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX	(1<<2)
+#define FEATURE_CONTROL_SGX_ENABLE                      (1<<18)
+#define FEATURE_CONTROL_SGX_LE_WR			(1<<17)
 #define FEATURE_CONTROL_LMCE				(1<<20)
 
 #define MSR_IA32_APICBASE		0x0000001b
@@ -545,6 +547,12 @@
 #define PACKAGE_THERM_INT_LOW_ENABLE		(1 << 1)
 #define PACKAGE_THERM_INT_PLN_ENABLE		(1 << 24)
 
+/* Intel SGX MSRs */
+#define MSR_IA32_SGXLEPUBKEYHASH0	0x0000008C
+#define MSR_IA32_SGXLEPUBKEYHASH1	0x0000008D
+#define MSR_IA32_SGXLEPUBKEYHASH2	0x0000008E
+#define MSR_IA32_SGXLEPUBKEYHASH3	0x0000008F
+
 /* Thermal Thresholds Support */
 #define THERM_INT_THRESHOLD0_ENABLE    (1 << 15)
 #define THERM_SHIFT_THRESHOLD0        8
-- 
2.17.1

[PATCH v12 04/13] x86/cpufeatures: add Intel-defined SGX leaf CPUID_12_EAX

[Jarkko Sakkinen]

From: Sean Christopherson <sean.j.christopherson@intel.com>

CPUID_12_EAX is an Intel-defined feature bits leaf dedicated for SGX.
There are currently four documented feature bits, with more expected in
the not-too-distant future.

Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
 arch/x86/include/asm/cpufeature.h        | 7 +++++--
 arch/x86/include/asm/cpufeatures.h       | 8 +++++++-
 arch/x86/include/asm/disabled-features.h | 3 ++-
 arch/x86/include/asm/required-features.h | 3 ++-
 arch/x86/kernel/cpu/common.c             | 7 +++++++
 arch/x86/kvm/cpuid.h                     | 1 +
 6 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index aced6c9290d6..940f0c01d5f8 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -30,6 +30,7 @@ enum cpuid_leafs
 	CPUID_7_ECX,
 	CPUID_8000_0007_EBX,
 	CPUID_7_EDX,
+	CPUID_12_EAX,
 };
 
 #ifdef CONFIG_X86_FEATURE_NAMES
@@ -81,8 +82,9 @@ extern const char * const x86_bug_flags[NBUGINTS*32];
 	   CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 16, feature_bit) ||	\
 	   CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 17, feature_bit) ||	\
 	   CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 18, feature_bit) ||	\
+	   CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 19, feature_bit) ||	\
 	   REQUIRED_MASK_CHECK					  ||	\
-	   BUILD_BUG_ON_ZERO(NCAPINTS != 19))
+	   BUILD_BUG_ON_ZERO(NCAPINTS != 20))
 
 #define DISABLED_MASK_BIT_SET(feature_bit)				\
 	 ( CHECK_BIT_IN_MASK_WORD(DISABLED_MASK,  0, feature_bit) ||	\
@@ -104,8 +106,9 @@ extern const char * const x86_bug_flags[NBUGINTS*32];
 	   CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 16, feature_bit) ||	\
 	   CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 17, feature_bit) ||	\
 	   CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 18, feature_bit) ||	\
+	   CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 19, feature_bit) ||	\
 	   DISABLED_MASK_CHECK					  ||	\
-	   BUILD_BUG_ON_ZERO(NCAPINTS != 19))
+	   BUILD_BUG_ON_ZERO(NCAPINTS != 20))
 
 #define cpu_has(c, bit)							\
 	(__builtin_constant_p(bit) && REQUIRED_MASK_BIT_SET(bit) ? 1 :	\
diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 54d5269e1b86..60ebf79062f0 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -13,7 +13,7 @@
 /*
  * Defines x86 CPU feature bits
  */
-#define NCAPINTS			19	   /* N 32-bit words worth of info */
+#define NCAPINTS			20	   /* N 32-bit words worth of info */
 #define NBUGINTS			1	   /* N 32-bit bug flags */
 
 /*
@@ -344,6 +344,12 @@
 #define X86_FEATURE_ARCH_CAPABILITIES	(18*32+29) /* IA32_ARCH_CAPABILITIES MSR (Intel) */
 #define X86_FEATURE_SPEC_CTRL_SSBD	(18*32+31) /* "" Speculative Store Bypass Disable */
 
+/* Intel SGX CPU features, CPUID level 0x000000012:0 (EAX), word 19 */
+#define X86_FEATURE_SGX1		(19*32+ 0) /* SGX1 leaf functions */
+#define X86_FEATURE_SGX2		(19*32+ 1) /* SGX2 leaf functions */
+#define X86_FEATURE_SGX_ENCLV		(19*32+ 5) /* SGX ENCLV instruction, leafs E[INC|DEC]VIRTCHILD, ESETCONTEXT */
+#define X86_FEATURE_SGX_ENCLS_C		(19*32+ 6) /* SGX ENCLS leafs ERDINFO, ETRACK, ELDBC and ELDUC */
+
 /*
  * BUG word(s)
  */
diff --git a/arch/x86/include/asm/disabled-features.h b/arch/x86/include/asm/disabled-features.h
index 33833d1909af..c81b5d446a3e 100644
--- a/arch/x86/include/asm/disabled-features.h
+++ b/arch/x86/include/asm/disabled-features.h
@@ -78,6 +78,7 @@
 #define DISABLED_MASK16	(DISABLE_PKU|DISABLE_OSPKE|DISABLE_LA57|DISABLE_UMIP)
 #define DISABLED_MASK17	0
 #define DISABLED_MASK18	0
-#define DISABLED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 19)
+#define DISABLED_MASK19	0
+#define DISABLED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 20)
 
 #endif /* _ASM_X86_DISABLED_FEATURES_H */
diff --git a/arch/x86/include/asm/required-features.h b/arch/x86/include/asm/required-features.h
index 6847d85400a8..fa5700097f64 100644
--- a/arch/x86/include/asm/required-features.h
+++ b/arch/x86/include/asm/required-features.h
@@ -101,6 +101,7 @@
 #define REQUIRED_MASK16	0
 #define REQUIRED_MASK17	0
 #define REQUIRED_MASK18	0
-#define REQUIRED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 19)
+#define REQUIRED_MASK19	0
+#define REQUIRED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 20)
 
 #endif /* _ASM_X86_REQUIRED_FEATURES_H */
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 38276f58d3bf..5e712fa6bf4f 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -843,6 +843,13 @@ void get_cpu_cap(struct cpuinfo_x86 *c)
 		}
 	}
 
+	/* Intel SGX features: level 0x00000012 */
+	if (c->cpuid_level >= 0x00000012) {
+		cpuid(0x00000012, &eax, &ebx, &ecx, &edx);
+
+		c->x86_capability[CPUID_12_EAX] = eax;
+	}
+
 	/* AMD-defined flags: level 0x80000001 */
 	eax = cpuid_eax(0x80000000);
 	c->extended_cpuid_level = eax;
diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
index 9a327d5b6d1f..669c1774afdb 100644
--- a/arch/x86/kvm/cpuid.h
+++ b/arch/x86/kvm/cpuid.h
@@ -55,6 +55,7 @@ static const struct cpuid_reg reverse_cpuid[] = {
 	[CPUID_7_ECX]         = {         7, 0, CPUID_ECX},
 	[CPUID_8000_0007_EBX] = {0x80000007, 0, CPUID_EBX},
 	[CPUID_7_EDX]         = {         7, 0, CPUID_EDX},
+	[CPUID_12_EAX]        = {        12, 0, CPUID_EAX},
 };
 
 static __always_inline struct cpuid_reg x86_feature_cpuid(unsigned x86_feature)
-- 
2.17.1

[PATCH v12 05/13] x86/sgx: architectural structures

[Jarkko Sakkinen]

This commit adds arch/x86/include/asm/sgx_arch.h that contains definitions
for data structures used by the SGX.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Co-developed-by: Suresh Siddha <suresh.b.siddha@intel.com>
---
 arch/x86/include/asm/sgx_arch.h | 183 ++++++++++++++++++++++++++++++++
 1 file changed, 183 insertions(+)
 create mode 100644 arch/x86/include/asm/sgx_arch.h

diff --git a/arch/x86/include/asm/sgx_arch.h b/arch/x86/include/asm/sgx_arch.h
new file mode 100644
index 000000000000..41a37eaa3f51
--- /dev/null
+++ b/arch/x86/include/asm/sgx_arch.h
@@ -0,0 +1,183 @@
+// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
+// Copyright(c) 2016-17 Intel Corporation.
+
+#ifndef _ASM_X86_SGX_ARCH_H
+#define _ASM_X86_SGX_ARCH_H
+
+#include <linux/types.h>
+
+#define SGX_CPUID 0x12
+
+enum sgx_cpuid {
+	SGX_CPUID_CAPABILITIES	= 0,
+	SGX_CPUID_ATTRIBUTES	= 1,
+	SGX_CPUID_EPC_BANKS	= 2,
+};
+
+#define SGX_SSA_GPRS_SIZE		182
+#define SGX_SSA_MISC_EXINFO_SIZE	16
+
+enum sgx_misc {
+	SGX_MISC_EXINFO		= 0x01,
+};
+
+#define SGX_MISC_RESERVED_MASK 0xFFFFFFFFFFFFFFFEL
+
+enum sgx_attribute {
+	SGX_ATTR_DEBUG		= 0x02,
+	SGX_ATTR_MODE64BIT	= 0x04,
+	SGX_ATTR_PROVISIONKEY	= 0x10,
+	SGX_ATTR_EINITTOKENKEY	= 0x20,
+};
+
+#define SGX_ATTR_RESERVED_MASK 0xFFFFFFFFFFFFFFC9L
+
+#define SGX_SECS_RESERVED1_SIZE 24
+#define SGX_SECS_RESERVED2_SIZE 32
+#define SGX_SECS_RESERVED3_SIZE 96
+#define SGX_SECS_RESERVED4_SIZE 3836
+
+struct sgx_secs {
+	uint64_t size;
+	uint64_t base;
+	uint32_t ssaframesize;
+	uint32_t miscselect;
+	uint8_t reserved1[SGX_SECS_RESERVED1_SIZE];
+	uint64_t attributes;
+	uint64_t xfrm;
+	uint32_t mrenclave[8];
+	uint8_t reserved2[SGX_SECS_RESERVED2_SIZE];
+	uint32_t mrsigner[8];
+	uint8_t	reserved3[SGX_SECS_RESERVED3_SIZE];
+	uint16_t isvvprodid;
+	uint16_t isvsvn;
+	uint8_t reserved4[SGX_SECS_RESERVED4_SIZE];
+} __packed __aligned(4096);
+
+enum sgx_tcs_flags {
+	SGX_TCS_DBGOPTIN	= 0x01, /* cleared on EADD */
+};
+
+#define SGX_TCS_RESERVED_MASK 0xFFFFFFFFFFFFFFFEL
+
+struct sgx_tcs {
+	uint64_t state;
+	uint64_t flags;
+	uint64_t ossa;
+	uint32_t cssa;
+	uint32_t nssa;
+	uint64_t oentry;
+	uint64_t aep;
+	uint64_t ofsbase;
+	uint64_t ogsbase;
+	uint32_t fslimit;
+	uint32_t gslimit;
+	uint64_t reserved[503];
+} __packed __aligned(4096);
+
+struct sgx_pageinfo {
+	uint64_t linaddr;
+	uint64_t srcpge;
+	union {
+		uint64_t secinfo;
+		uint64_t pcmd;
+	};
+	uint64_t secs;
+} __packed __aligned(32);
+
+
+#define SGX_SECINFO_PERMISSION_MASK	0x0000000000000007L
+#define SGX_SECINFO_PAGE_TYPE_MASK	0x000000000000FF00L
+#define SGX_SECINFO_RESERVED_MASK	0xFFFFFFFFFFFF00F8L
+
+enum sgx_page_type {
+	SGX_PAGE_TYPE_SECS	= 0x00,
+	SGX_PAGE_TYPE_TCS	= 0x01,
+	SGX_PAGE_TYPE_REG	= 0x02,
+	SGX_PAGE_TYPE_VA	= 0x03,
+	SGX_PAGE_TYPE_TRIM	= 0x04,
+};
+
+enum sgx_secinfo_flags {
+	SGX_SECINFO_R		= 0x01,
+	SGX_SECINFO_W		= 0x02,
+	SGX_SECINFO_X		= 0x04,
+	SGX_SECINFO_SECS	= (SGX_PAGE_TYPE_SECS << 8),
+	SGX_SECINFO_TCS		= (SGX_PAGE_TYPE_TCS << 8),
+	SGX_SECINFO_REG		= (SGX_PAGE_TYPE_REG << 8),
+	SGX_SECINFO_VA          = (SGX_PAGE_TYPE_VA << 8),
+	SGX_SECINFO_TRIM	= (SGX_PAGE_TYPE_TRIM << 8),
+};
+
+struct sgx_secinfo {
+	uint64_t flags;
+	uint64_t reserved[7];
+} __packed __aligned(64);
+
+struct sgx_pcmd {
+	struct sgx_secinfo secinfo;
+	uint64_t enclave_id;
+	uint8_t reserved[40];
+	uint8_t mac[16];
+} __packed __aligned(128);
+
+#define SGX_MODULUS_SIZE 384
+
+struct sgx_sigstruct_header {
+	uint64_t header1[2];
+	uint32_t vendor;
+	uint32_t date;
+	uint64_t header2[2];
+	uint32_t swdefined;
+	uint8_t reserved1[84];
+} __packed;
+
+struct sgx_sigstruct_body {
+	uint32_t miscselect;
+	uint32_t miscmask;
+	uint8_t reserved2[20];
+	uint64_t attributes;
+	uint64_t xfrm;
+	uint8_t attributemask[16];
+	uint8_t mrenclave[32];
+	uint8_t reserved3[32];
+	uint16_t isvprodid;
+	uint16_t isvsvn;
+} __packed;
+
+struct sgx_sigstruct {
+	struct sgx_sigstruct_header header;
+	uint8_t modulus[SGX_MODULUS_SIZE];
+	uint32_t exponent;
+	uint8_t signature[SGX_MODULUS_SIZE];
+	struct sgx_sigstruct_body body;
+	uint8_t reserved4[12];
+	uint8_t q1[SGX_MODULUS_SIZE];
+	uint8_t q2[SGX_MODULUS_SIZE];
+} __packed __aligned(4096);
+
+struct sgx_einittoken_payload {
+	uint32_t valid;
+	uint32_t reserved1[11];
+	uint64_t attributes;
+	uint64_t xfrm;
+	uint8_t mrenclave[32];
+	uint8_t reserved2[32];
+	uint8_t mrsigner[32];
+	uint8_t reserved3[32];
+} __packed;
+
+struct sgx_einittoken {
+	struct sgx_einittoken_payload payload;
+	uint8_t cpusvnle[16];
+	uint16_t isvprodidle;
+	uint16_t isvsvnle;
+	uint8_t reserved2[24];
+	uint32_t maskedmiscselectle;
+	uint64_t maskedattributesle;
+	uint64_t maskedxfrmle;
+	uint8_t keyid[32];
+	uint8_t mac[16];
+} __packed __aligned(512);
+
+#endif /* _ASM_X86_SGX_ARCH_H */
-- 
2.17.1

[PATCH v12 06/13] x86/sgx: detect Intel SGX

[Jarkko Sakkinen]

From: Sean Christopherson <sean.j.christopherson@intel.com>

Intel(R) SGX is a set of CPU instructions that can be used by applications
to set aside private regions of code and data. The code outside the enclave
is disallowed to access the memory inside the enclave by the CPU access
control.

This commit adds the check for SGX to arch/x86 and a new config option,
INTEL_SGX_CORE. Exposes a boolean variable 'sgx_enabled' to query whether
or not the SGX support is available.

Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Co-developed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Co-developed-by: Suresh Siddha <suresh.b.siddha@intel.com>
---
 arch/x86/Kconfig                | 19 ++++++++++++
 arch/x86/include/asm/sgx.h      | 12 ++++++++
 arch/x86/include/asm/sgx_pr.h   | 13 ++++++++
 arch/x86/kernel/cpu/Makefile    |  1 +
 arch/x86/kernel/cpu/intel_sgx.c | 54 +++++++++++++++++++++++++++++++++
 5 files changed, 99 insertions(+)
 create mode 100644 arch/x86/include/asm/sgx.h
 create mode 100644 arch/x86/include/asm/sgx_pr.h
 create mode 100644 arch/x86/kernel/cpu/intel_sgx.c

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index c07f492b871a..d2c7e7a2b2ac 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1925,6 +1925,25 @@ config X86_INTEL_MEMORY_PROTECTION_KEYS
 
 	  If unsure, say y.
 
+config INTEL_SGX_CORE
+	prompt "Intel SGX core functionality"
+	def_bool n
+	depends on X86_64 && CPU_SUP_INTEL
+	help
+	Intel Software Guard eXtensions (SGX) is a set of CPU instructions
+	that allows ring 3 applications to create enclaves, private regions
+	of memory that are protected, by hardware, from unauthorized access
+	and/or modification.
+
+	This option enables kernel recognition of SGX, high-level management
+	of the Enclave Page Cache (EPC), tracking and writing of SGX Launch
+	Enclave Hash MSRs, and allows for virtualization of SGX via KVM. By
+	iteslf, this option does not provide SGX support to userspace.
+
+	For details, see Documentation/x86/intel_sgx.rst
+
+	If unsure, say N.
+
 config EFI
 	bool "EFI runtime service support"
 	depends on ACPI
diff --git a/arch/x86/include/asm/sgx.h b/arch/x86/include/asm/sgx.h
new file mode 100644
index 000000000000..2130e639ab49
--- /dev/null
+++ b/arch/x86/include/asm/sgx.h
@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
+// Copyright(c) 2016-18 Intel Corporation.
+
+#ifndef _ASM_X86_SGX_H
+#define _ASM_X86_SGX_H
+
+#include <linux/types.h>
+
+extern bool sgx_enabled;
+extern bool sgx_lc_enabled;
+
+#endif /* _ASM_X86_SGX_H */
diff --git a/arch/x86/include/asm/sgx_pr.h b/arch/x86/include/asm/sgx_pr.h
new file mode 100644
index 000000000000..42a4185d4bc9
--- /dev/null
+++ b/arch/x86/include/asm/sgx_pr.h
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
+// Copyright(c) 2016-17 Intel Corporation.
+
+#ifndef _ASM_X86_SGX_PR_H
+#define _ASM_X86_SGX_PR_H
+
+#include <linux/printk.h>
+#include <linux/ratelimit.h>
+
+#undef pr_fmt
+#define pr_fmt(fmt) "intel_sgx: " fmt
+
+#endif /* _ASM_X86_SGX_PR_H */
diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile
index a66229f51b12..9552ff5b4ec3 100644
--- a/arch/x86/kernel/cpu/Makefile
+++ b/arch/x86/kernel/cpu/Makefile
@@ -36,6 +36,7 @@ obj-$(CONFIG_CPU_SUP_TRANSMETA_32)	+= transmeta.o
 obj-$(CONFIG_CPU_SUP_UMC_32)		+= umc.o
 
 obj-$(CONFIG_INTEL_RDT)	+= intel_rdt.o intel_rdt_rdtgroup.o intel_rdt_monitor.o intel_rdt_ctrlmondata.o
+obj-$(CONFIG_INTEL_SGX_CORE)		+= intel_sgx.o
 
 obj-$(CONFIG_X86_MCE)			+= mcheck/
 obj-$(CONFIG_MTRR)			+= mtrr/
diff --git a/arch/x86/kernel/cpu/intel_sgx.c b/arch/x86/kernel/cpu/intel_sgx.c
new file mode 100644
index 000000000000..77d94115c4cf
--- /dev/null
+++ b/arch/x86/kernel/cpu/intel_sgx.c
@@ -0,0 +1,54 @@
+// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
+// Copyright(c) 2016-17 Intel Corporation.
+
+#include <asm/sgx.h>
+#include <asm/sgx_pr.h>
+#include <linux/freezer.h>
+#include <linux/highmem.h>
+#include <linux/kthread.h>
+#include <linux/ratelimit.h>
+#include <linux/sched/signal.h>
+#include <linux/slab.h>
+
+bool sgx_enabled __ro_after_init;
+EXPORT_SYMBOL(sgx_enabled);
+bool sgx_lc_enabled __ro_after_init;
+EXPORT_SYMBOL(sgx_lc_enabled);
+
+static __init bool sgx_is_enabled(bool *lc_enabled)
+{
+	unsigned long fc;
+
+	if (!boot_cpu_has(X86_FEATURE_SGX))
+		return false;
+
+	if (!boot_cpu_has(X86_FEATURE_SGX1))
+		return false;
+
+	rdmsrl(MSR_IA32_FEATURE_CONTROL, fc);
+	if (!(fc & FEATURE_CONTROL_LOCKED)) {
+		pr_info("IA32_FEATURE_CONTROL MSR is not locked\n");
+		return false;
+	}
+
+	if (!(fc & FEATURE_CONTROL_SGX_ENABLE)) {
+		pr_info("disabled by the firmware\n");
+		return false;
+	}
+
+	if (!(fc & FEATURE_CONTROL_SGX_LE_WR)) {
+		pr_info("IA32_SGXLEPUBKEYHASHn MSRs are not writable\n");
+		return false;
+	}
+
+	*lc_enabled = !!(fc & FEATURE_CONTROL_SGX_LE_WR);
+	return true;
+}
+
+static __init int sgx_init(void)
+{
+	sgx_enabled = sgx_is_enabled(&sgx_lc_enabled);
+	return 0;
+}
+
+arch_initcall(sgx_init);
-- 
2.17.1

[PATCH v12 07/13] x86/sgx: data structures for tracking available EPC pages

[Jarkko Sakkinen]

SGX has a set of data structures to maintain information about the enclaves
and their security properties. BIOS reserves a fixed size region of
physical memory for these structures by setting Processor Reserved Memory
Range Registers (PRMRR). This memory area is called Enclave Page Cache
(EPC).

This commit adds a database of EPC banks for kernel to easily access the
available EPC pages. On UMA architectures there is a singe bank of EPC
pages. On NUMA architectures there is an EPC bank for each node.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Co-developed-by: Serge Ayoun <serge.ayoun@intel.com>
Co-developed-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
 arch/x86/include/asm/sgx.h      |  30 +++++++
 arch/x86/kernel/cpu/intel_sgx.c | 146 +++++++++++++++++++++++++++++++-
 2 files changed, 175 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/sgx.h b/arch/x86/include/asm/sgx.h
index 2130e639ab49..77b2294fcfb0 100644
--- a/arch/x86/include/asm/sgx.h
+++ b/arch/x86/include/asm/sgx.h
@@ -4,9 +4,39 @@
 #ifndef _ASM_X86_SGX_H
 #define _ASM_X86_SGX_H
 
+#include <asm/sgx_arch.h>
+#include <asm/asm.h>
+#include <linux/bitops.h>
+#include <linux/err.h>
+#include <linux/rwsem.h>
 #include <linux/types.h>
 
+#define SGX_MAX_EPC_BANKS 8
+
+#define SGX_EPC_BANK(epc_page) \
+	(&sgx_epc_banks[(unsigned long)(epc_page->desc) & ~PAGE_MASK])
+#define SGX_EPC_PFN(epc_page) PFN_DOWN((unsigned long)(epc_page->desc))
+#define SGX_EPC_ADDR(epc_page) ((unsigned long)(epc_page->desc) & PAGE_MASK)
+
+struct sgx_epc_page {
+	unsigned long desc;
+	struct list_head list;
+};
+
+struct sgx_epc_bank {
+	unsigned long pa;
+	unsigned long va;
+	unsigned long size;
+	struct sgx_epc_page *pages_data;
+	struct sgx_epc_page **pages;
+	atomic_t free_cnt;
+	struct rw_semaphore lock;
+};
+
 extern bool sgx_enabled;
 extern bool sgx_lc_enabled;
 
+void *sgx_get_page(struct sgx_epc_page *ptr);
+void sgx_put_page(void *epc_page_ptr);
+
 #endif /* _ASM_X86_SGX_H */
diff --git a/arch/x86/kernel/cpu/intel_sgx.c b/arch/x86/kernel/cpu/intel_sgx.c
index 77d94115c4cf..60cbc7cfb868 100644
--- a/arch/x86/kernel/cpu/intel_sgx.c
+++ b/arch/x86/kernel/cpu/intel_sgx.c
@@ -6,8 +6,10 @@
 #include <linux/freezer.h>
 #include <linux/highmem.h>
 #include <linux/kthread.h>
+#include <linux/pagemap.h>
 #include <linux/ratelimit.h>
 #include <linux/sched/signal.h>
+#include <linux/shmem_fs.h>
 #include <linux/slab.h>
 
 bool sgx_enabled __ro_after_init;
@@ -15,6 +17,139 @@ EXPORT_SYMBOL(sgx_enabled);
 bool sgx_lc_enabled __ro_after_init;
 EXPORT_SYMBOL(sgx_lc_enabled);
 
+static atomic_t sgx_nr_free_pages = ATOMIC_INIT(0);
+static struct sgx_epc_bank sgx_epc_banks[SGX_MAX_EPC_BANKS];
+static int sgx_nr_epc_banks;
+
+/**
+ * sgx_get_page - pin an EPC page
+ * @page:	an EPC page
+ *
+ * Return: a pointer to the pinned EPC page
+ */
+void *sgx_get_page(struct sgx_epc_page *page)
+{
+	struct sgx_epc_bank *bank = SGX_EPC_BANK(page);
+
+	if (IS_ENABLED(CONFIG_X86_64))
+		return (void *)(bank->va + SGX_EPC_ADDR(page) - bank->pa);
+
+	return kmap_atomic_pfn(SGX_EPC_PFN(page));
+}
+EXPORT_SYMBOL(sgx_get_page);
+
+/**
+ * sgx_put_page - unpin an EPC page
+ * @ptr:	a pointer to the pinned EPC page
+ */
+void sgx_put_page(void *ptr)
+{
+	if (IS_ENABLED(CONFIG_X86_64))
+		return;
+
+	kunmap_atomic(ptr);
+}
+EXPORT_SYMBOL(sgx_put_page);
+
+static __init int sgx_init_epc_bank(unsigned long addr, unsigned long size,
+				    unsigned long index,
+				    struct sgx_epc_bank *bank)
+{
+	unsigned long nr_pages = size >> PAGE_SHIFT;
+	unsigned long i;
+	void *va;
+
+	if (IS_ENABLED(CONFIG_X86_64)) {
+		va = ioremap_cache(addr, size);
+		if (!va)
+			return -ENOMEM;
+	}
+
+	bank->pages_data = kzalloc(nr_pages * sizeof(struct sgx_epc_page),
+				   GFP_KERNEL);
+	if (!bank->pages_data) {
+		if (IS_ENABLED(CONFIG_X86_64))
+			iounmap(va);
+
+		return -ENOMEM;
+	}
+
+	bank->pages = kzalloc(nr_pages * sizeof(struct sgx_epc_page *),
+			      GFP_KERNEL);
+	if (!bank->pages) {
+		if (IS_ENABLED(CONFIG_X86_64))
+			iounmap(va);
+		kfree(bank->pages_data);
+		bank->pages_data = NULL;
+		return -ENOMEM;
+	}
+
+	for (i = 0; i < nr_pages; i++) {
+		bank->pages[i] = &bank->pages_data[i];
+		bank->pages[i]->desc = (addr + (i << PAGE_SHIFT)) | index;
+	}
+
+	bank->pa = addr;
+	bank->size = size;
+	if (IS_ENABLED(CONFIG_X86_64))
+		bank->va = (unsigned long)va;
+
+	atomic_set(&bank->free_cnt, nr_pages);
+	init_rwsem(&bank->lock);
+	atomic_add(nr_pages, &sgx_nr_free_pages);
+	return 0;
+}
+
+static __init void sgx_page_cache_teardown(void)
+{
+	struct sgx_epc_bank *bank;
+	int i;
+
+	for (i = 0; i < sgx_nr_epc_banks; i++) {
+		bank = &sgx_epc_banks[i];
+
+		if (IS_ENABLED(CONFIG_X86_64))
+			iounmap((void *)bank->va);
+
+		kfree(bank->pages);
+		kfree(bank->pages_data);
+	}
+}
+
+static __init int sgx_page_cache_init(void)
+{
+	unsigned long size;
+	unsigned int eax;
+	unsigned int ebx;
+	unsigned int ecx;
+	unsigned int edx;
+	unsigned long pa;
+	int i;
+	int ret;
+
+	for (i = 0; i < SGX_MAX_EPC_BANKS; i++) {
+		cpuid_count(SGX_CPUID, i + SGX_CPUID_EPC_BANKS, &eax, &ebx,
+			    &ecx, &edx);
+		if (!(eax & 0xf))
+			break;
+
+		pa = ((u64)(ebx & 0xfffff) << 32) + (u64)(eax & 0xfffff000);
+		size = ((u64)(edx & 0xfffff) << 32) + (u64)(ecx & 0xfffff000);
+
+		pr_info("EPC bank 0x%lx-0x%lx\n", pa, pa + size);
+
+		ret = sgx_init_epc_bank(pa, size, i, &sgx_epc_banks[i]);
+		if (ret) {
+			sgx_page_cache_teardown();
+			return ret;
+		}
+
+		sgx_nr_epc_banks++;
+	}
+
+	return 0;
+}
+
 static __init bool sgx_is_enabled(bool *lc_enabled)
 {
 	unsigned long fc;
@@ -47,7 +182,16 @@ static __init bool sgx_is_enabled(bool *lc_enabled)
 
 static __init int sgx_init(void)
 {
-	sgx_enabled = sgx_is_enabled(&sgx_lc_enabled);
+	int ret;
+
+	if (!sgx_is_enabled(&sgx_lc_enabled))
+		return 0;
+
+	ret = sgx_page_cache_init();
+	if (ret)
+		return ret;
+
+	sgx_enabled = true;
 	return 0;
 }
 
-- 
2.17.1

[PATCH v12 08/13] x86/sgx: wrappers for ENCLS opcode leaf functions

[Jarkko Sakkinen]

This commit adds wrappers for Intel(R) SGX ENCLS opcode leaf functions
except for ENCLS(EINIT). The ENCLS instruction invokes the privileged
functions for managing (creation, initialization and swapping) and
debugging enclaves.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Co-developed-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
 arch/x86/include/asm/sgx.h      | 176 ++++++++++++++++++++++++++++++++
 arch/x86/include/asm/sgx_arch.h |  19 ++++
 2 files changed, 195 insertions(+)

diff --git a/arch/x86/include/asm/sgx.h b/arch/x86/include/asm/sgx.h
index 77b2294fcfb0..a42c8ed10f7d 100644
--- a/arch/x86/include/asm/sgx.h
+++ b/arch/x86/include/asm/sgx.h
@@ -11,6 +11,157 @@
 #include <linux/rwsem.h>
 #include <linux/types.h>
 
+#define IS_ENCLS_FAULT(r) ((r) & 0xffff0000)
+#define ENCLS_FAULT_VECTOR(r) ((r) >> 16)
+
+#define ENCLS_TO_ERR(r) (IS_ENCLS_FAULT(r) ? -EFAULT :		\
+			(r) == SGX_UNMASKED_EVENT ? -EINTR :	\
+			(r) == SGX_MAC_COMPARE_FAIL ? -EIO :	\
+			(r) == SGX_ENTRYEPOCH_LOCKED ? -EBUSY : -EPERM)
+
+#define __encls_ret_N(rax, inputs...)			\
+	({						\
+	int ret;					\
+	asm volatile(					\
+	"1: .byte 0x0f, 0x01, 0xcf;\n\t"		\
+	"2:\n"						\
+	".section .fixup,\"ax\"\n"			\
+	"3: shll $16,%%eax\n"				\
+	"   jmp 2b\n"					\
+	".previous\n"					\
+	_ASM_EXTABLE_FAULT(1b, 3b)			\
+	: "=a"(ret)					\
+	: "a"(rax), inputs				\
+	: "memory");					\
+	ret;						\
+	})
+
+#define __encls_ret_1(rax, rcx)				\
+	({						\
+	__encls_ret_N(rax, "c"(rcx));			\
+	})
+
+#define __encls_ret_2(rax, rbx, rcx)			\
+	({						\
+	__encls_ret_N(rax, "b"(rbx), "c"(rcx));		\
+	})
+
+#define __encls_ret_3(rax, rbx, rcx, rdx)			\
+	({							\
+	__encls_ret_N(rax, "b"(rbx), "c"(rcx), "d"(rdx));	\
+	})
+
+#define __encls_N(rax, rbx_out, inputs...)		\
+	({						\
+	int ret;					\
+	asm volatile(					\
+	"1: .byte 0x0f, 0x01, 0xcf;\n\t"		\
+	"   xor %%eax,%%eax;\n"				\
+	"2:\n"						\
+	".section .fixup,\"ax\"\n"			\
+	"3: shll $16,%%eax\n"				\
+	"   jmp 2b\n"					\
+	".previous\n"					\
+	_ASM_EXTABLE_FAULT(1b, 3b)				\
+	: "=a"(ret), "=b"(rbx_out)			\
+	: "a"(rax), inputs				\
+	: "memory");					\
+	ret;						\
+	})
+
+#define __encls_2(rax, rbx, rcx)				\
+	({							\
+	unsigned long ign_rbx_out;				\
+	__encls_N(rax, ign_rbx_out, "b"(rbx), "c"(rcx));	\
+	})
+
+#define __encls_1_1(rax, data, rcx)			\
+	({						\
+	unsigned long rbx_out;				\
+	int ret = __encls_N(rax, rbx_out, "c"(rcx));	\
+	if (!ret)					\
+		data = rbx_out;				\
+	ret;						\
+	})
+
+static inline int __ecreate(struct sgx_pageinfo *pginfo, void *secs)
+{
+	return __encls_2(ECREATE, pginfo, secs);
+}
+
+static inline int __eextend(void *secs, void *epc)
+{
+	return __encls_2(EEXTEND, secs, epc);
+}
+
+static inline int __eadd(struct sgx_pageinfo *pginfo, void *epc)
+{
+	return __encls_2(EADD, pginfo, epc);
+}
+
+static inline int __einit(void *sigstruct, struct sgx_einittoken *einittoken,
+			  void *secs)
+{
+	return __encls_ret_3(EINIT, sigstruct, secs, einittoken);
+}
+
+static inline int __eremove(void *epc)
+{
+	return __encls_ret_1(EREMOVE, epc);
+}
+
+static inline int __edbgwr(unsigned long addr, unsigned long *data)
+{
+	return __encls_2(EDGBWR, *data, addr);
+}
+
+static inline int __edbgrd(unsigned long addr, unsigned long *data)
+{
+	return __encls_1_1(EDGBRD, *data, addr);
+}
+
+static inline int __etrack(void *epc)
+{
+	return __encls_ret_1(ETRACK, epc);
+}
+
+static inline int __eldu(struct sgx_pageinfo *pginfo, void *epc, void *va)
+{
+	return __encls_ret_3(ELDU, pginfo, epc, va);
+}
+
+static inline int __eblock(void *epc)
+{
+	return __encls_ret_1(EBLOCK, epc);
+}
+
+static inline int __epa(void *epc)
+{
+	unsigned long rbx = SGX_PAGE_TYPE_VA;
+
+	return __encls_2(EPA, rbx, epc);
+}
+
+static inline int __ewb(struct sgx_pageinfo *pginfo, void *epc, void *va)
+{
+	return __encls_ret_3(EWB, pginfo, epc, va);
+}
+
+static inline int __eaug(struct sgx_pageinfo *pginfo, void *epc)
+{
+	return __encls_2(EAUG, pginfo, epc);
+}
+
+static inline int __emodpr(struct sgx_secinfo *secinfo, void *epc)
+{
+	return __encls_ret_2(EMODPR, secinfo, epc);
+}
+
+static inline int __emodt(struct sgx_secinfo *secinfo, void *epc)
+{
+	return __encls_ret_2(EMODT, secinfo, epc);
+}
+
 #define SGX_MAX_EPC_BANKS 8
 
 #define SGX_EPC_BANK(epc_page) \
@@ -39,4 +190,29 @@ extern bool sgx_lc_enabled;
 void *sgx_get_page(struct sgx_epc_page *ptr);
 void sgx_put_page(void *epc_page_ptr);
 
+#define SGX_FN(name, params...)		\
+{					\
+	void *epc;			\
+	int ret;			\
+	epc = sgx_get_page(epc_page);	\
+	ret = __##name(params);		\
+	sgx_put_page(epc);		\
+	return ret;			\
+}
+
+#define BUILD_SGX_FN(fn, name)				\
+static inline int fn(struct sgx_epc_page *epc_page)	\
+	SGX_FN(name, epc)
+BUILD_SGX_FN(sgx_eremove, eremove)
+BUILD_SGX_FN(sgx_eblock, eblock)
+BUILD_SGX_FN(sgx_etrack, etrack)
+BUILD_SGX_FN(sgx_epa, epa)
+
+static inline int sgx_emodpr(struct sgx_secinfo *secinfo,
+			     struct sgx_epc_page *epc_page)
+	SGX_FN(emodpr, secinfo, epc)
+static inline int sgx_emodt(struct sgx_secinfo *secinfo,
+			    struct sgx_epc_page *epc_page)
+	SGX_FN(emodt, secinfo, epc)
+
 #endif /* _ASM_X86_SGX_H */
diff --git a/arch/x86/include/asm/sgx_arch.h b/arch/x86/include/asm/sgx_arch.h
index 41a37eaa3f51..85e6eb4c11a5 100644
--- a/arch/x86/include/asm/sgx_arch.h
+++ b/arch/x86/include/asm/sgx_arch.h
@@ -14,6 +14,25 @@ enum sgx_cpuid {
 	SGX_CPUID_EPC_BANKS	= 2,
 };
 
+enum sgx_encls_leafs {
+	ECREATE	= 0x0,
+	EADD	= 0x1,
+	EINIT	= 0x2,
+	EREMOVE	= 0x3,
+	EDGBRD	= 0x4,
+	EDGBWR	= 0x5,
+	EEXTEND	= 0x6,
+	ELDB	= 0x7,
+	ELDU	= 0x8,
+	EBLOCK	= 0x9,
+	EPA	= 0xA,
+	EWB	= 0xB,
+	ETRACK	= 0xC,
+	EAUG	= 0xD,
+	EMODPR	= 0xE,
+	EMODT	= 0xF,
+};
+
 #define SGX_SSA_GPRS_SIZE		182
 #define SGX_SSA_MISC_EXINFO_SIZE	16
 
-- 
2.17.1

[PATCH v12 09/13] x86/sgx: EPC page allocation routines

[Jarkko Sakkinen]

SGX has a set of data structures to maintain information about the enclaves
and their security properties. BIOS reserves a fixed size region of
physical memory for these structures by setting Processor Reserved Memory
Range Registers (PRMRR). This memory area is called Enclave Page Cache
(EPC).

This commit implements the routines to allocate and free pages from
different EPC banks. There is also a swapper thread ksgxswapd for EPC
pages that gets woken up by sgx_alloc_page() when we run below the low
watermark. The swapper thread continues swapping pages up until it
reaches the high watermark.

The SGX driver and in future KVM provide a set of callbacks that are
used to reclaim, block and write an EPC pages. Kernel takes the
responsibility of maintaining LRU cache for them.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Co-developed-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
 arch/x86/include/asm/sgx.h      |  26 ++++
 arch/x86/kernel/cpu/intel_sgx.c | 216 ++++++++++++++++++++++++++++++++
 2 files changed, 242 insertions(+)

diff --git a/arch/x86/include/asm/sgx.h b/arch/x86/include/asm/sgx.h
index a42c8ed10f7d..4f5f32b37b5d 100644
--- a/arch/x86/include/asm/sgx.h
+++ b/arch/x86/include/asm/sgx.h
@@ -169,8 +169,23 @@ static inline int __emodt(struct sgx_secinfo *secinfo, void *epc)
 #define SGX_EPC_PFN(epc_page) PFN_DOWN((unsigned long)(epc_page->desc))
 #define SGX_EPC_ADDR(epc_page) ((unsigned long)(epc_page->desc) & PAGE_MASK)
 
+struct sgx_epc_page;
+
+struct sgx_epc_page_ops {
+	bool (*get)(struct sgx_epc_page *epc_page);
+	void (*put)(struct sgx_epc_page *epc_page);
+	bool (*reclaim)(struct sgx_epc_page *epc_page);
+	void (*block)(struct sgx_epc_page *epc_page);
+	void (*write)(struct sgx_epc_page *epc_page);
+};
+
+struct sgx_epc_page_impl {
+	const struct sgx_epc_page_ops *ops;
+};
+
 struct sgx_epc_page {
 	unsigned long desc;
+	struct sgx_epc_page_impl *impl;
 	struct list_head list;
 };
 
@@ -186,9 +201,20 @@ struct sgx_epc_bank {
 
 extern bool sgx_enabled;
 extern bool sgx_lc_enabled;
+extern struct list_head sgx_active_page_list;
+extern struct spinlock sgx_active_page_list_lock;
+
+enum sgx_alloc_flags {
+	SGX_ALLOC_ATOMIC	= BIT(0),
+};
 
+struct sgx_epc_page *sgx_alloc_page(struct sgx_epc_page_impl *impl,
+				    unsigned int flags);
+int sgx_free_page(struct sgx_epc_page *page);
 void *sgx_get_page(struct sgx_epc_page *ptr);
 void sgx_put_page(void *epc_page_ptr);
+struct page *sgx_get_backing(struct file *file, pgoff_t index);
+void sgx_put_backing(struct page *backing_page, bool write);
 
 #define SGX_FN(name, params...)		\
 {					\
diff --git a/arch/x86/kernel/cpu/intel_sgx.c b/arch/x86/kernel/cpu/intel_sgx.c
index 60cbc7cfb868..b52bab8eff99 100644
--- a/arch/x86/kernel/cpu/intel_sgx.c
+++ b/arch/x86/kernel/cpu/intel_sgx.c
@@ -12,14 +12,199 @@
 #include <linux/shmem_fs.h>
 #include <linux/slab.h>
 
+#define SGX_NR_TO_SCAN	16
+#define SGX_NR_LOW_PAGES 32
+#define SGX_NR_HIGH_PAGES 64
+
 bool sgx_enabled __ro_after_init;
 EXPORT_SYMBOL(sgx_enabled);
 bool sgx_lc_enabled __ro_after_init;
 EXPORT_SYMBOL(sgx_lc_enabled);
+LIST_HEAD(sgx_active_page_list);
+EXPORT_SYMBOL(sgx_active_page_list);
+DEFINE_SPINLOCK(sgx_active_page_list_lock);
+EXPORT_SYMBOL(sgx_active_page_list_lock);
 
 static atomic_t sgx_nr_free_pages = ATOMIC_INIT(0);
 static struct sgx_epc_bank sgx_epc_banks[SGX_MAX_EPC_BANKS];
 static int sgx_nr_epc_banks;
+static struct task_struct *ksgxswapd_tsk;
+static DECLARE_WAIT_QUEUE_HEAD(ksgxswapd_waitq);
+
+static void sgx_swap_cluster(void)
+{
+	struct sgx_epc_page *cluster[SGX_NR_TO_SCAN + 1];
+	struct sgx_epc_page *epc_page;
+	int i;
+	int j;
+
+	memset(cluster, 0, sizeof(cluster));
+
+	for (i = 0, j = 0; i < SGX_NR_TO_SCAN; i++) {
+		spin_lock(&sgx_active_page_list_lock);
+		if (list_empty(&sgx_active_page_list)) {
+			spin_unlock(&sgx_active_page_list_lock);
+			break;
+		}
+		epc_page = list_first_entry(&sgx_active_page_list,
+					    struct sgx_epc_page, list);
+		if (!epc_page->impl->ops->get(epc_page)) {
+			list_move_tail(&epc_page->list, &sgx_active_page_list);
+			spin_unlock(&sgx_active_page_list_lock);
+			continue;
+		}
+		list_del(&epc_page->list);
+		spin_unlock(&sgx_active_page_list_lock);
+
+		if (epc_page->impl->ops->reclaim(epc_page)) {
+			cluster[j++] = epc_page;
+		} else {
+			spin_lock(&sgx_active_page_list_lock);
+			list_add_tail(&epc_page->list, &sgx_active_page_list);
+			spin_unlock(&sgx_active_page_list_lock);
+			epc_page->impl->ops->put(epc_page);
+		}
+	}
+
+	for (i = 0; cluster[i]; i++) {
+		epc_page = cluster[i];
+		epc_page->impl->ops->block(epc_page);
+	}
+
+	for (i = 0; cluster[i]; i++) {
+		epc_page = cluster[i];
+		epc_page->impl->ops->write(epc_page);
+		epc_page->impl->ops->put(epc_page);
+		sgx_free_page(epc_page);
+	}
+}
+
+static int ksgxswapd(void *p)
+{
+	set_freezable();
+
+	while (!kthread_should_stop()) {
+		if (try_to_freeze())
+			continue;
+
+		wait_event_freezable(ksgxswapd_waitq, kthread_should_stop() ||
+				     atomic_read(&sgx_nr_free_pages) <
+				     SGX_NR_HIGH_PAGES);
+
+		if (atomic_read(&sgx_nr_free_pages) < SGX_NR_HIGH_PAGES)
+			sgx_swap_cluster();
+	}
+
+	pr_info("%s: done\n", __func__);
+	return 0;
+}
+
+static struct sgx_epc_page *sgx_try_alloc_page(struct sgx_epc_page_impl *impl)
+{
+	struct sgx_epc_bank *bank;
+	struct sgx_epc_page *page = NULL;
+	int i;
+
+	for (i = 0; i < sgx_nr_epc_banks; i++) {
+		bank = &sgx_epc_banks[i];
+
+		down_write(&bank->lock);
+
+		if (atomic_read(&bank->free_cnt))
+			page = bank->pages[atomic_dec_return(&bank->free_cnt)];
+
+		up_write(&bank->lock);
+
+		if (page)
+			break;
+	}
+
+	if (page) {
+		atomic_dec(&sgx_nr_free_pages);
+		page->impl = impl;
+	}
+
+	return page;
+}
+
+/**
+ * sgx_alloc_page - allocate an EPC page
+ * @flags:	allocation flags
+ * @impl:	implementation for the struct sgx_epc_page
+ *
+ * Try to grab a page from the free EPC page list. If there is a free page
+ * available, it is returned to the caller. If called with SGX_ALLOC_ATOMIC,
+ * the function will return immediately if the list is empty. Otherwise, it
+ * will swap pages up until there is a free page available. Upon returning the
+ * low watermark is checked and ksgxswapd is waken up if we are below it.
+ *
+ * Return:
+ *   a &struct sgx_epc_page instace,
+ *   -ENOMEM if all pages are unreclaimable,
+ *   -EBUSY when called with SGX_ALLOC_ATOMIC and out of free pages
+ */
+struct sgx_epc_page *sgx_alloc_page(struct sgx_epc_page_impl *impl,
+				    unsigned int flags)
+{
+	struct sgx_epc_page *entry;
+
+	for ( ; ; ) {
+		entry = sgx_try_alloc_page(impl);
+		if (entry)
+			break;
+
+		if (list_empty(&sgx_active_page_list))
+			return ERR_PTR(-ENOMEM);
+
+		if (flags & SGX_ALLOC_ATOMIC) {
+			entry = ERR_PTR(-EBUSY);
+			break;
+		}
+
+		if (signal_pending(current)) {
+			entry = ERR_PTR(-ERESTARTSYS);
+			break;
+		}
+
+		sgx_swap_cluster();
+		schedule();
+	}
+
+	if (atomic_read(&sgx_nr_free_pages) < SGX_NR_LOW_PAGES)
+		wake_up(&ksgxswapd_waitq);
+
+	return entry;
+}
+EXPORT_SYMBOL(sgx_alloc_page);
+
+/**
+ * sgx_free_page - free an EPC page
+ *
+ * @page:	any EPC page
+ *
+ * Remove an EPC page and insert it back to the list of free pages.
+ *
+ * Return: SGX error code
+ */
+int sgx_free_page(struct sgx_epc_page *page)
+{
+	struct sgx_epc_bank *bank = SGX_EPC_BANK(page);
+	int ret;
+
+	ret = sgx_eremove(page);
+	if (ret) {
+		pr_debug("EREMOVE returned %d\n", ret);
+		return ret;
+	}
+
+	down_read(&bank->lock);
+	bank->pages[atomic_inc_return(&bank->free_cnt) - 1] = page;
+	atomic_inc(&sgx_nr_free_pages);
+	up_read(&bank->lock);
+
+	return 0;
+}
+EXPORT_SYMBOL(sgx_free_page);
 
 /**
  * sgx_get_page - pin an EPC page
@@ -51,6 +236,25 @@ void sgx_put_page(void *ptr)
 }
 EXPORT_SYMBOL(sgx_put_page);
 
+struct page *sgx_get_backing(struct file *file, pgoff_t index)
+{
+	struct inode *inode = file->f_path.dentry->d_inode;
+	struct address_space *mapping = inode->i_mapping;
+	gfp_t gfpmask = mapping_gfp_mask(mapping);
+
+	return shmem_read_mapping_page_gfp(mapping, index, gfpmask);
+}
+EXPORT_SYMBOL(sgx_get_backing);
+
+void sgx_put_backing(struct page *backing_page, bool write)
+{
+	if (write)
+		set_page_dirty(backing_page);
+
+	put_page(backing_page);
+}
+EXPORT_SYMBOL(sgx_put_backing);
+
 static __init int sgx_init_epc_bank(unsigned long addr, unsigned long size,
 				    unsigned long index,
 				    struct sgx_epc_bank *bank)
@@ -114,6 +318,11 @@ static __init void sgx_page_cache_teardown(void)
 		kfree(bank->pages);
 		kfree(bank->pages_data);
 	}
+
+	if (ksgxswapd_tsk) {
+		kthread_stop(ksgxswapd_tsk);
+		ksgxswapd_tsk = NULL;
+	}
 }
 
 static __init int sgx_page_cache_init(void)
@@ -182,6 +391,7 @@ static __init bool sgx_is_enabled(bool *lc_enabled)
 
 static __init int sgx_init(void)
 {
+	struct task_struct *tsk;
 	int ret;
 
 	if (!sgx_is_enabled(&sgx_lc_enabled))
@@ -191,6 +401,12 @@ static __init int sgx_init(void)
 	if (ret)
 		return ret;
 
+	tsk = kthread_run(ksgxswapd, NULL, "ksgxswapd");
+	if (IS_ERR(tsk)) {
+		sgx_page_cache_teardown();
+		return PTR_ERR(tsk);
+	}
+	ksgxswapd_tsk = tsk;
 	sgx_enabled = true;
 	return 0;
 }
-- 
2.17.1

[PATCH v12 10/13] x86/sgx: sgx_einit()

[Jarkko Sakkinen]

From: Sean Christopherson <sean.j.christopherson@intel.com>

Implements function to perform ENCLS(EINIT) leaf function that initializes
an enclave, which can be used by a driver for running enclaves and VMMs.

Writing the LE hash MSRs is extraordinarily expensive, e.g. 3-4x slower
than normal MSRs, so we use a per-cpu cache to track the last known value
of the MSRs to avoid unnecessarily writing the MSRs with the current value.

Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
 arch/x86/include/asm/sgx.h      |  2 ++
 arch/x86/kernel/cpu/intel_sgx.c | 46 +++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+)

diff --git a/arch/x86/include/asm/sgx.h b/arch/x86/include/asm/sgx.h
index 4f5f32b37b5d..bc09b4576d68 100644
--- a/arch/x86/include/asm/sgx.h
+++ b/arch/x86/include/asm/sgx.h
@@ -215,6 +215,8 @@ void *sgx_get_page(struct sgx_epc_page *ptr);
 void sgx_put_page(void *epc_page_ptr);
 struct page *sgx_get_backing(struct file *file, pgoff_t index);
 void sgx_put_backing(struct page *backing_page, bool write);
+int sgx_einit(struct sgx_sigstruct *sigstruct, struct sgx_einittoken *token,
+	      struct sgx_epc_page *secs_page, u64 le_pubkey_hash[4]);
 
 #define SGX_FN(name, params...)		\
 {					\
diff --git a/arch/x86/kernel/cpu/intel_sgx.c b/arch/x86/kernel/cpu/intel_sgx.c
index b52bab8eff99..53dd91cb7a49 100644
--- a/arch/x86/kernel/cpu/intel_sgx.c
+++ b/arch/x86/kernel/cpu/intel_sgx.c
@@ -31,6 +31,12 @@ static int sgx_nr_epc_banks;
 static struct task_struct *ksgxswapd_tsk;
 static DECLARE_WAIT_QUEUE_HEAD(ksgxswapd_waitq);
 
+/*
+ * A cache for last known values of IA32_SGXLEPUBKEYHASHx MSRs. Cache entries
+ * are initialized when they are first used by sgx_einit().
+ */
+static DEFINE_PER_CPU(u64 [4], sgx_le_pubkey_hash_cache);
+
 static void sgx_swap_cluster(void)
 {
 	struct sgx_epc_page *cluster[SGX_NR_TO_SCAN + 1];
@@ -255,6 +261,46 @@ void sgx_put_backing(struct page *backing_page, bool write)
 }
 EXPORT_SYMBOL(sgx_put_backing);
 
+/**
+ * sgx_einit - EINIT an enclave with the appropriate LE pubkey hash
+ * @sigstruct:		a pointer to the enclave's sigstruct
+ * @token:		a pointer to the enclave's EINIT token
+ * @secs_page:		a pointer to the enclave's SECS EPC page
+ * @le_pubkey_hash:	the desired LE pubkey hash for EINIT
+ */
+int sgx_einit(struct sgx_sigstruct *sigstruct, struct sgx_einittoken *token,
+	      struct sgx_epc_page *secs_page, u64 le_pubkey_hash[4])
+{
+	u64 __percpu *cache;
+	void *secs;
+	int i, ret;
+
+	secs = sgx_get_page(secs_page);
+
+	if (!sgx_lc_enabled) {
+		ret = __einit(sigstruct, token, secs);
+		goto out;
+	}
+
+	cache = per_cpu(sgx_le_pubkey_hash_cache, smp_processor_id());
+
+	preempt_disable();
+	for (i = 0; i < 4; i++) {
+		if (le_pubkey_hash[i] == cache[i])
+			continue;
+
+		wrmsrl(MSR_IA32_SGXLEPUBKEYHASH0 + i, le_pubkey_hash[i]);
+		cache[i] = le_pubkey_hash[i];
+	}
+	ret = __einit(sigstruct, token, secs);
+	preempt_enable();
+
+out:
+	sgx_put_page(secs);
+	return ret;
+}
+EXPORT_SYMBOL(sgx_einit);
+
 static __init int sgx_init_epc_bank(unsigned long addr, unsigned long size,
 				    unsigned long index,
 				    struct sgx_epc_bank *bank)
-- 
2.17.1

[PATCH v12 11/13] platform/x86: Intel SGX driver

[Jarkko Sakkinen]

Intel Software Guard eXtensions (SGX) is a set of CPU instructions that
can be used by applications to set aside private regions of code and
data. The code outside the enclave is disallowed to access the memory
inside the enclave by the CPU access control.

SGX driver provides a ioctl API for loading and initializing enclaves.
Address range for enclaves is reserved with mmap() and they are
destroyed with munmap(). Enclave construction, measurement and
initialization is done with the provided the ioctl API.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Co-developed-by: Sean Christopherson <sean.j.christopherson@intel.com
Co-developed-by: Serge Ayoun <serge.ayoun@intel.com>
Co-developed-by: Shay Katz-zamir <shay.katz-zamir@intel.com>
Co-developed-by: Suresh Siddha <suresh.b.siddha@intel.com>
---
 arch/x86/include/asm/sgx_pr.h                 |   16 +
 arch/x86/include/uapi/asm/sgx.h               |  138 +++
 drivers/platform/x86/Kconfig                  |    2 +
 drivers/platform/x86/Makefile                 |    1 +
 drivers/platform/x86/intel_sgx/Kconfig        |   22 +
 drivers/platform/x86/intel_sgx/Makefile       |   13 +
 drivers/platform/x86/intel_sgx/sgx.h          |  157 +++
 drivers/platform/x86/intel_sgx/sgx_encl.c     | 1011 +++++++++++++++++
 .../platform/x86/intel_sgx/sgx_encl_page.c    |  286 +++++
 drivers/platform/x86/intel_sgx/sgx_fault.c    |  151 +++
 drivers/platform/x86/intel_sgx/sgx_ioctl.c    |  231 ++++
 drivers/platform/x86/intel_sgx/sgx_main.c     |  298 +++++
 drivers/platform/x86/intel_sgx/sgx_vma.c      |   58 +
 13 files changed, 2384 insertions(+)
 create mode 100644 arch/x86/include/uapi/asm/sgx.h
 create mode 100644 drivers/platform/x86/intel_sgx/Kconfig
 create mode 100644 drivers/platform/x86/intel_sgx/Makefile
 create mode 100644 drivers/platform/x86/intel_sgx/sgx.h
 create mode 100644 drivers/platform/x86/intel_sgx/sgx_encl.c
 create mode 100644 drivers/platform/x86/intel_sgx/sgx_encl_page.c
 create mode 100644 drivers/platform/x86/intel_sgx/sgx_fault.c
 create mode 100644 drivers/platform/x86/intel_sgx/sgx_ioctl.c
 create mode 100644 drivers/platform/x86/intel_sgx/sgx_main.c
 create mode 100644 drivers/platform/x86/intel_sgx/sgx_vma.c

diff --git a/arch/x86/include/asm/sgx_pr.h b/arch/x86/include/asm/sgx_pr.h
index 42a4185d4bc9..b2baf12430e2 100644
--- a/arch/x86/include/asm/sgx_pr.h
+++ b/arch/x86/include/asm/sgx_pr.h
@@ -10,4 +10,20 @@
 #undef pr_fmt
 #define pr_fmt(fmt) "intel_sgx: " fmt
 
+#define sgx_pr_ratelimited(level, encl, fmt, ...)			\
+	pr_ ## level ## _ratelimited("[%d:0x%p] " fmt,			\
+				     pid_nr((encl)->tgid),		\
+				     (void *)(encl)->base, ##__VA_ARGS__)
+
+#define sgx_dbg(encl, fmt, ...) \
+	sgx_pr_ratelimited(debug, encl, fmt, ##__VA_ARGS__)
+#define sgx_info(encl, fmt, ...) \
+	sgx_pr_ratelimited(info, encl, fmt, ##__VA_ARGS__)
+#define sgx_warn(encl, fmt, ...) \
+	sgx_pr_ratelimited(warn, encl, fmt, ##__VA_ARGS__)
+#define sgx_err(encl, fmt, ...) \
+	sgx_pr_ratelimited(err, encl, fmt, ##__VA_ARGS__)
+#define sgx_crit(encl, fmt, ...) \
+	sgx_pr_ratelimited(crit, encl, fmt, ##__VA_ARGS__)
+
 #endif /* _ASM_X86_SGX_PR_H */
diff --git a/arch/x86/include/uapi/asm/sgx.h b/arch/x86/include/uapi/asm/sgx.h
new file mode 100644
index 000000000000..9bd8907efdaf
--- /dev/null
+++ b/arch/x86/include/uapi/asm/sgx.h
@@ -0,0 +1,138 @@
+/*
+ * This file is provided under a dual BSD/GPLv2 license.  When using or
+ * redistributing this file, you may do so under either license.
+ *
+ * GPL LICENSE SUMMARY
+ *
+ * Copyright(c) 2016-2017 Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * Contact Information:
+ * Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
+ * Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
+ *
+ * BSD LICENSE
+ *
+ * Copyright(c) 2016-2017 Intel Corporation.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ *   * Redistributions of source code must retain the above copyright
+ *     notice, this list of conditions and the following disclaimer.
+ *   * Redistributions in binary form must reproduce the above copyright
+ *     notice, this list of conditions and the following disclaimer in
+ *     the documentation and/or other materials provided with the
+ *     distribution.
+ *   * Neither the name of Intel Corporation nor the names of its
+ *     contributors may be used to endorse or promote products derived
+ *     from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * Authors:
+ *
+ * Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
+ * Suresh Siddha <suresh.b.siddha@intel.com>
+ */
+
+#ifndef _UAPI_ASM_X86_SGX_H
+#define _UAPI_ASM_X86_SGX_H
+
+#include <linux/types.h>
+#include <linux/ioctl.h>
+
+#define SGX_MAGIC 0xA4
+
+#define SGX_IOC_ENCLAVE_CREATE \
+	_IOW(SGX_MAGIC, 0x00, struct sgx_enclave_create)
+#define SGX_IOC_ENCLAVE_ADD_PAGE \
+	_IOW(SGX_MAGIC, 0x01, struct sgx_enclave_add_page)
+#define SGX_IOC_ENCLAVE_INIT \
+	_IOW(SGX_MAGIC, 0x02, struct sgx_enclave_init)
+
+/* SGX leaf instruction return values */
+#define SGX_SUCCESS			0
+#define SGX_INVALID_SIG_STRUCT		1
+#define SGX_INVALID_ATTRIBUTE		2
+#define SGX_BLKSTATE			3
+#define SGX_INVALID_MEASUREMENT		4
+#define SGX_NOTBLOCKABLE		5
+#define SGX_PG_INVLD			6
+#define SGX_LOCKFAIL			7
+#define SGX_INVALID_SIGNATURE		8
+#define SGX_MAC_COMPARE_FAIL		9
+#define SGX_PAGE_NOT_BLOCKED		10
+#define SGX_NOT_TRACKED			11
+#define SGX_VA_SLOT_OCCUPIED		12
+#define SGX_CHILD_PRESENT		13
+#define SGX_ENCLAVE_ACT			14
+#define SGX_ENTRYEPOCH_LOCKED		15
+#define SGX_INVALID_EINITTOKEN		16
+#define SGX_PREV_TRK_INCMPL		17
+#define SGX_PG_IS_SECS			18
+#define SGX_INVALID_CPUSVN		32
+#define SGX_INVALID_ISVSVN		64
+#define SGX_UNMASKED_EVENT		128
+#define SGX_INVALID_KEYNAME		256
+
+/* IOCTL return values */
+#define SGX_POWER_LOST_ENCLAVE		0x40000000
+#define SGX_LE_ROLLBACK			0x40000001
+
+/**
+ * struct sgx_enclave_create - parameter structure for the
+ *                             %SGX_IOC_ENCLAVE_CREATE ioctl
+ * @src:	address for the SECS page data
+ */
+struct sgx_enclave_create  {
+	__u64	src;
+};
+
+/**
+ * struct sgx_enclave_add_page - parameter structure for the
+ *                               %SGX_IOC_ENCLAVE_ADD_PAGE ioctl
+ * @addr:	address within the ELRANGE
+ * @src:	address for the page data
+ * @secinfo:	address for the SECINFO data
+ * @mrmask:	bitmask for the measured 256 byte chunks
+ */
+struct sgx_enclave_add_page {
+	__u64	addr;
+	__u64	src;
+	__u64	secinfo;
+	__u16	mrmask;
+} __attribute__((__packed__));
+
+
+/**
+ * struct sgx_enclave_init - parameter structure for the
+ *                           %SGX_IOC_ENCLAVE_INIT ioctl
+ * @addr:	address within the ELRANGE
+ * @sigstruct:	address for the SIGSTRUCT data
+ */
+struct sgx_enclave_init {
+	__u64	addr;
+	__u64	sigstruct;
+};
+
+#endif /* _UAPI_ASM_X86_SGX_H */
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 566644bb496a..645888c2049d 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -1218,6 +1218,8 @@ config INTEL_CHTDC_TI_PWRBTN
 	  To compile this driver as a module, choose M here: the module
 	  will be called intel_chtdc_ti_pwrbtn.
 
+source "drivers/platform/x86/intel_sgx/Kconfig"
+
 endif # X86_PLATFORM_DEVICES
 
 config PMC_ATOM
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 2ba6cb795338..729208a9c958 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -91,3 +91,4 @@ obj-$(CONFIG_PMC_ATOM)		+= pmc_atom.o
 obj-$(CONFIG_MLX_PLATFORM)	+= mlx-platform.o
 obj-$(CONFIG_INTEL_TURBO_MAX_3) += intel_turbo_max_3.o
 obj-$(CONFIG_INTEL_CHTDC_TI_PWRBTN)	+= intel_chtdc_ti_pwrbtn.o
+obj-$(CONFIG_INTEL_SGX) += intel_sgx/
diff --git a/drivers/platform/x86/intel_sgx/Kconfig b/drivers/platform/x86/intel_sgx/Kconfig
new file mode 100644
index 000000000000..8e6c57c28443
--- /dev/null
+++ b/drivers/platform/x86/intel_sgx/Kconfig
@@ -0,0 +1,22 @@
+#
+# Intel SGX
+#
+
+config INTEL_SGX
+	tristate "Intel(R) SGX Driver"
+	default n
+	depends on X86_64 && CPU_SUP_INTEL
+	select INTEL_SGX_CORE
+	select MMU_NOTIFIER
+	select CRYPTO
+	select CRYPTO_SHA256
+	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.  The code
+	outside the enclave is disallowed to access the memory inside the
+	enclave by the CPU access control.
+
+	The firmware uses PRMRR registers to reserve an area of physical memory
+	called Enclave Page Cache (EPC). There is a hardware unit in the
+	processor called Memory Encryption Engine. The MEE encrypts and decrypts
+	the EPC pages as they enter and leave the processor package.
diff --git a/drivers/platform/x86/intel_sgx/Makefile b/drivers/platform/x86/intel_sgx/Makefile
new file mode 100644
index 000000000000..95f254e30a8b
--- /dev/null
+++ b/drivers/platform/x86/intel_sgx/Makefile
@@ -0,0 +1,13 @@
+#
+# Intel SGX
+#
+
+obj-$(CONFIG_INTEL_SGX) += intel_sgx.o
+
+intel_sgx-$(CONFIG_INTEL_SGX) += \
+	sgx_ioctl.o \
+	sgx_encl.o \
+	sgx_encl_page.o \
+	sgx_main.o \
+	sgx_fault.o \
+	sgx_vma.o \
diff --git a/drivers/platform/x86/intel_sgx/sgx.h b/drivers/platform/x86/intel_sgx/sgx.h
new file mode 100644
index 000000000000..8b7c6ea77346
--- /dev/null
+++ b/drivers/platform/x86/intel_sgx/sgx.h
@@ -0,0 +1,157 @@
+// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
+// Copyright(c) 2016-18 Intel Corporation.
+
+#ifndef __ARCH_INTEL_SGX_H__
+#define __ARCH_INTEL_SGX_H__
+
+#include <crypto/hash.h>
+#include <linux/kref.h>
+#include <linux/mmu_notifier.h>
+#include <linux/mmu_notifier.h>
+#include <linux/radix-tree.h>
+#include <linux/radix-tree.h>
+#include <linux/rbtree.h>
+#include <linux/rwsem.h>
+#include <linux/sched.h>
+#include <linux/workqueue.h>
+#include <asm/sgx.h>
+#include <asm/sgx_pr.h>
+#include <uapi/asm/sgx.h>
+
+#define SGX_EINIT_SPIN_COUNT	20
+#define SGX_EINIT_SLEEP_COUNT	50
+#define SGX_EINIT_SLEEP_TIME	20
+
+#define SGX_VA_SLOT_COUNT 512
+#define SGX_VA_OFFSET_MASK ((SGX_VA_SLOT_COUNT - 1) << 3)
+
+struct sgx_va_page {
+	struct sgx_epc_page *epc_page;
+	DECLARE_BITMAP(slots, SGX_VA_SLOT_COUNT);
+	struct list_head list;
+};
+
+enum sgx_encl_page_flags {
+	SGX_ENCL_PAGE_TCS	= BIT(0),
+	SGX_ENCL_PAGE_RESERVED	= BIT(1),
+	SGX_ENCL_PAGE_LOADED	= BIT(2),
+};
+
+#define SGX_ENCL_PAGE_ADDR(encl_page) ((encl_page)->desc & PAGE_MASK)
+#define SGX_ENCL_PAGE_VA_OFFSET(encl_page) \
+	((encl_page)->desc & SGX_VA_OFFSET_MASK)
+#define SGX_ENCL_PAGE_BACKING_INDEX(encl_page, encl)		\
+({								\
+	pgoff_t index;						\
+	if (!PFN_DOWN(encl_page->desc))				\
+		index = PFN_DOWN(encl->size);			\
+	else							\
+		index = PFN_DOWN(encl_page->desc - encl->base);	\
+	index;							\
+})
+#define SGX_ENCL_PAGE_PCMD_OFFSET(encl_page, encl)		\
+({								\
+	unsigned long ret;					\
+	ret = SGX_ENCL_PAGE_BACKING_INDEX(encl_page, encl);	\
+	((ret & 31) * 128);					\
+})
+
+struct sgx_encl_page {
+	unsigned long desc;
+	union {
+		struct sgx_epc_page *epc_page;
+		struct sgx_va_page *va_page;
+	};
+	struct sgx_encl *encl;
+	struct sgx_epc_page_impl impl;
+};
+
+enum sgx_encl_flags {
+	SGX_ENCL_INITIALIZED	= BIT(0),
+	SGX_ENCL_DEBUG		= BIT(1),
+	SGX_ENCL_SUSPEND	= BIT(2),
+	SGX_ENCL_DEAD		= BIT(3),
+};
+
+struct sgx_encl {
+	unsigned int flags;
+	uint64_t attributes;
+	uint64_t xfrm;
+	unsigned int page_cnt;
+	unsigned int secs_child_cnt;
+	struct mutex lock;
+	struct mm_struct *mm;
+	struct file *backing;
+	struct file *pcmd;
+	struct kref refcount;
+	unsigned long base;
+	unsigned long size;
+	unsigned long ssaframesize;
+	struct list_head va_pages;
+	struct radix_tree_root page_tree;
+	struct list_head add_page_reqs;
+	struct work_struct add_page_work;
+	struct sgx_encl_page secs;
+	struct pid *tgid;
+	struct mmu_notifier mmu_notifier;
+};
+
+extern struct workqueue_struct *sgx_add_page_wq;
+extern u64 sgx_encl_size_max_32;
+extern u64 sgx_encl_size_max_64;
+extern u64 sgx_xfrm_mask;
+extern u32 sgx_misc_reserved;
+extern u32 sgx_xsave_size_tbl[64];
+
+extern const struct vm_operations_struct sgx_vm_ops;
+
+int sgx_encl_find(struct mm_struct *mm, unsigned long addr,
+		  struct vm_area_struct **vma);
+void sgx_invalidate(struct sgx_encl *encl, bool flush_cpus);
+#define SGX_INVD(ret, encl, fmt, ...)			\
+do {							\
+	if (unlikely(ret)) {				\
+		sgx_err(encl, fmt, ##__VA_ARGS__);	\
+		sgx_invalidate(encl, true);		\
+	}						\
+} while (0)
+
+struct sgx_encl *sgx_encl_alloc(struct sgx_secs *secs);
+int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs);
+struct sgx_encl_page *sgx_encl_alloc_page(struct sgx_encl *encl,
+					  unsigned long addr);
+void sgx_encl_free_page(struct sgx_encl_page *encl_page);
+int sgx_encl_add_page(struct sgx_encl *encl, unsigned long addr, void *data,
+		      struct sgx_secinfo *secinfo, unsigned int mrmask);
+int sgx_encl_init(struct sgx_encl *encl, struct sgx_sigstruct *sigstruct,
+		  struct sgx_einittoken *einittoken);
+void sgx_encl_block(struct sgx_encl_page *encl_page);
+void sgx_encl_track(struct sgx_encl *encl);
+int sgx_encl_load_page(struct sgx_encl_page *encl_page,
+		       struct sgx_epc_page *epc_page);
+void sgx_encl_release(struct kref *ref);
+
+long sgx_ioctl(struct file *filep, unsigned int cmd, unsigned long arg);
+#ifdef CONFIG_COMPAT
+long sgx_compat_ioctl(struct file *filep, unsigned int cmd, unsigned long arg);
+#endif
+
+/* Utility functions */
+int sgx_test_and_clear_young(struct sgx_encl_page *page);
+void sgx_flush_cpus(struct sgx_encl *encl);
+
+struct sgx_encl_page *sgx_fault_page(struct vm_area_struct *vma,
+				     unsigned long addr,
+				     bool do_reserve);
+
+extern const struct sgx_epc_page_ops sgx_encl_page_ops;
+
+void sgx_set_epc_page(struct sgx_encl_page *encl_page,
+		      struct sgx_epc_page *epc_page);
+void sgx_set_page_reclaimable(struct sgx_encl_page *encl_page);
+struct sgx_epc_page *sgx_alloc_va_page(unsigned int flags);
+unsigned int sgx_alloc_va_slot(struct sgx_va_page *va_page);
+void sgx_free_va_slot(struct sgx_va_page *va_page, unsigned int offset);
+bool sgx_va_page_full(struct sgx_va_page *va_page);
+
+#endif /* __ARCH_X86_INTEL_SGX_H__ */
diff --git a/drivers/platform/x86/intel_sgx/sgx_encl.c b/drivers/platform/x86/intel_sgx/sgx_encl.c
new file mode 100644
index 000000000000..e3d7fa885489
--- /dev/null
+++ b/drivers/platform/x86/intel_sgx/sgx_encl.c
@@ -0,0 +1,1011 @@
+// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
+// Copyright(c) 2016-18 Intel Corporation.
+
+#include <asm/mman.h>
+#include <linux/delay.h>
+#include <linux/file.h>
+#include <linux/hashtable.h>
+#include <linux/highmem.h>
+#include <linux/ratelimit.h>
+#include <linux/sched/signal.h>
+#include <linux/shmem_fs.h>
+#include <linux/slab.h>
+#include "sgx.h"
+
+struct sgx_add_page_req {
+	struct sgx_encl *encl;
+	struct sgx_encl_page *encl_page;
+	struct sgx_secinfo secinfo;
+	u16 mrmask;
+	struct list_head list;
+};
+
+/**
+ * sgx_encl_find - find an enclave
+ * @mm:		mm struct of the current process
+ * @addr:	address in the ELRANGE
+ * @vma:	the resulting VMA
+ *
+ * Finds an enclave identified by the given address. Gives back the VMA, that
+ * is part of the enclave, located in that address. The VMA is given back if it
+ * is a proper enclave VMA even if an &sgx_encl instance does not exist
+ * yet (enclave creation has not been performed).
+ *
+ * Return:
+ *   0 on success,
+ *   -EINVAL if an enclave was not found,
+ *   -ENOENT if the enclave has not been created yet
+ */
+int sgx_encl_find(struct mm_struct *mm, unsigned long addr,
+		  struct vm_area_struct **vma)
+{
+	struct vm_area_struct *result;
+	struct sgx_encl *encl;
+
+	result = find_vma(mm, addr);
+	if (!result || result->vm_ops != &sgx_vm_ops || addr < result->vm_start)
+		return -EINVAL;
+
+	encl = result->vm_private_data;
+	*vma = result;
+
+	return encl ? 0 : -ENOENT;
+}
+
+static void sgx_encl_free_va_pages(struct sgx_encl *encl)
+{
+	struct sgx_va_page *va_page;
+
+	while (!list_empty(&encl->va_pages)) {
+		va_page = list_first_entry(&encl->va_pages, struct sgx_va_page,
+					   list);
+		list_del(&va_page->list);
+		sgx_free_page(va_page->epc_page);
+		kfree(va_page);
+	}
+}
+
+/**
+ * sgx_invalidate - kill an active enclave
+ *
+ * @encl:	an &sgx_encl instance
+ * @flush_cpus	whether to kick threads out of the enclave
+ *
+ * Clear PTEs for all the entry points, kick threads out of the enclave if
+ * requested and mark enclave state as dead.
+ */
+void sgx_invalidate(struct sgx_encl *encl, bool flush_cpus)
+{
+	struct sgx_encl_page *entry;
+	struct radix_tree_iter iter;
+	struct vm_area_struct *vma;
+	unsigned long addr;
+	void **slot;
+
+	if (encl->flags & SGX_ENCL_DEAD)
+		return;
+
+	radix_tree_for_each_slot(slot, &encl->page_tree, &iter, 0) {
+		entry = *slot;
+		addr = SGX_ENCL_PAGE_ADDR(entry);
+
+		if ((entry->desc & SGX_ENCL_PAGE_LOADED) &&
+		    (entry->desc & SGX_ENCL_PAGE_TCS) &&
+		    !sgx_encl_find(encl->mm, addr, &vma))
+			zap_vma_ptes(vma, addr, PAGE_SIZE);
+	}
+
+	encl->flags |= SGX_ENCL_DEAD;
+
+	if (flush_cpus)
+		sgx_flush_cpus(encl);
+
+	sgx_encl_free_va_pages(encl);
+}
+
+static int sgx_measure(struct sgx_epc_page *secs_page,
+		       struct sgx_epc_page *epc_page,
+		       u16 mrmask)
+{
+	int ret = 0;
+	void *secs;
+	void *epc;
+	int i;
+	int j;
+
+	if (!mrmask)
+		return ret;
+
+	secs = sgx_get_page(secs_page);
+	epc = sgx_get_page(epc_page);
+
+	for (i = 0, j = 1; i < 0x1000 && !ret; i += 0x100, j <<= 1) {
+		if (!(j & mrmask))
+			continue;
+
+		ret = __eextend(secs, (void *)((unsigned long)epc + i));
+	}
+
+	sgx_put_page(epc);
+	sgx_put_page(secs);
+
+	return ret;
+}
+
+static int sgx_eadd(struct sgx_epc_page *secs_page,
+		    struct sgx_epc_page *epc_page,
+		    unsigned long linaddr,
+		    struct sgx_secinfo *secinfo,
+		    struct page *backing)
+{
+	struct sgx_pageinfo pginfo;
+	void *epc_page_vaddr;
+	int ret;
+
+	pginfo.srcpge = (unsigned long)kmap_atomic(backing);
+	pginfo.secs = (unsigned long)sgx_get_page(secs_page);
+	epc_page_vaddr = sgx_get_page(epc_page);
+
+	pginfo.linaddr = linaddr;
+	pginfo.secinfo = (unsigned long)secinfo;
+	ret = __eadd(&pginfo, epc_page_vaddr);
+
+	sgx_put_page(epc_page_vaddr);
+	sgx_put_page((void *)(unsigned long)pginfo.secs);
+	kunmap_atomic((void *)(unsigned long)pginfo.srcpge);
+
+	return ret;
+}
+
+static bool sgx_process_add_page_req(struct sgx_add_page_req *req,
+				     struct sgx_epc_page *epc_page)
+{
+	struct sgx_encl_page *encl_page = req->encl_page;
+	struct sgx_encl *encl = req->encl;
+	struct vm_area_struct *vma;
+	pgoff_t backing_index;
+	struct page *backing;
+	unsigned long addr;
+	int ret;
+
+	if (encl->flags & (SGX_ENCL_SUSPEND | SGX_ENCL_DEAD))
+		return false;
+
+	addr = SGX_ENCL_PAGE_ADDR(encl_page);
+	ret = sgx_encl_find(encl->mm, addr, &vma);
+	if (ret)
+		return false;
+
+	backing_index = SGX_ENCL_PAGE_BACKING_INDEX(encl_page, encl);
+	backing = sgx_get_backing(encl->backing, backing_index);
+	if (IS_ERR(backing))
+		return false;
+
+	ret = vm_insert_pfn(vma, addr, SGX_EPC_PFN(epc_page));
+	if (ret) {
+		sgx_err(encl, "%s: vm_insert_pfn() returned %d\n", __func__,
+			ret);
+		sgx_put_backing(backing, false);
+		return false;
+	}
+
+	ret = sgx_eadd(encl->secs.epc_page, epc_page, addr, &req->secinfo,
+		       backing);
+
+	sgx_put_backing(backing, false);
+	if (ret) {
+		sgx_err(encl, "EADD returned %d\n", ret);
+		zap_vma_ptes(vma, addr, PAGE_SIZE);
+		return false;
+	}
+
+	ret = sgx_measure(encl->secs.epc_page, epc_page, req->mrmask);
+	if (ret) {
+		sgx_err(encl, "EEXTEND returned %d\n", ret);
+		zap_vma_ptes(vma, addr, PAGE_SIZE);
+		return false;
+	}
+
+	encl_page->encl = encl;
+	encl->secs_child_cnt++;
+	sgx_set_epc_page(encl_page, epc_page);
+	sgx_set_page_reclaimable(encl_page);
+	return true;
+}
+
+static void sgx_add_page_worker(struct work_struct *work)
+{
+	struct sgx_add_page_req *req;
+	bool skip_rest = false;
+	bool is_empty = false;
+	struct sgx_encl *encl;
+	struct sgx_epc_page *epc_page;
+
+	encl = container_of(work, struct sgx_encl, add_page_work);
+
+	do {
+		schedule();
+
+		if (encl->flags & SGX_ENCL_DEAD)
+			skip_rest = true;
+
+		mutex_lock(&encl->lock);
+		req = list_first_entry(&encl->add_page_reqs,
+				       struct sgx_add_page_req, list);
+		list_del(&req->list);
+		is_empty = list_empty(&encl->add_page_reqs);
+		mutex_unlock(&encl->lock);
+
+		if (skip_rest)
+			goto next;
+
+		epc_page = sgx_alloc_page(&req->encl_page->impl, 0);
+		down_read(&encl->mm->mmap_sem);
+		mutex_lock(&encl->lock);
+
+		if (IS_ERR(epc_page)) {
+			sgx_invalidate(encl, false);
+			skip_rest = true;
+		} else	if (!sgx_process_add_page_req(req, epc_page)) {
+			sgx_free_page(epc_page);
+			sgx_invalidate(encl, false);
+			skip_rest = true;
+		}
+
+		mutex_unlock(&encl->lock);
+		up_read(&encl->mm->mmap_sem);
+
+next:
+		kfree(req);
+	} while (!kref_put(&encl->refcount, sgx_encl_release) && !is_empty);
+}
+
+static u32 sgx_calc_ssaframesize(u32 miscselect, u64 xfrm)
+{
+	u32 size_max = PAGE_SIZE;
+	u32 size;
+	int i;
+
+	for (i = 2; i < 64; i++) {
+		if (!((1 << i) & xfrm))
+			continue;
+
+		size = SGX_SSA_GPRS_SIZE + sgx_xsave_size_tbl[i];
+		if (miscselect & SGX_MISC_EXINFO)
+			size += SGX_SSA_MISC_EXINFO_SIZE;
+
+		if (size > size_max)
+			size_max = size;
+	}
+
+	return (size_max + PAGE_SIZE - 1) >> PAGE_SHIFT;
+}
+
+static int sgx_validate_secs(const struct sgx_secs *secs,
+			     unsigned long ssaframesize)
+{
+	int i;
+
+	if (secs->size < (2 * PAGE_SIZE) ||
+	    (secs->size & (secs->size - 1)) != 0)
+		return -EINVAL;
+
+	if (secs->base & (secs->size - 1))
+		return -EINVAL;
+
+	if (secs->attributes & SGX_ATTR_RESERVED_MASK ||
+	    secs->miscselect & sgx_misc_reserved)
+		return -EINVAL;
+
+	if (secs->attributes & SGX_ATTR_MODE64BIT) {
+#ifdef CONFIG_X86_64
+		if (secs->size > sgx_encl_size_max_64)
+			return -EINVAL;
+#else
+		return -EINVAL;
+#endif
+	} else {
+		/* On 64-bit architecture allow 32-bit encls only in
+		 * the compatibility mode.
+		 */
+#ifdef CONFIG_X86_64
+		if (!test_thread_flag(TIF_ADDR32))
+			return -EINVAL;
+#endif
+		if (secs->size > sgx_encl_size_max_32)
+			return -EINVAL;
+	}
+
+	if ((secs->xfrm & 0x3) != 0x3 || (secs->xfrm & ~sgx_xfrm_mask))
+		return -EINVAL;
+
+	/* Check that BNDREGS and BNDCSR are equal. */
+	if (((secs->xfrm >> 3) & 1) != ((secs->xfrm >> 4) & 1))
+		return -EINVAL;
+
+	if (!secs->ssaframesize || ssaframesize > secs->ssaframesize)
+		return -EINVAL;
+
+	for (i = 0; i < SGX_SECS_RESERVED1_SIZE; i++)
+		if (secs->reserved1[i])
+			return -EINVAL;
+
+	for (i = 0; i < SGX_SECS_RESERVED2_SIZE; i++)
+		if (secs->reserved2[i])
+			return -EINVAL;
+
+	for (i = 0; i < SGX_SECS_RESERVED3_SIZE; i++)
+		if (secs->reserved3[i])
+			return -EINVAL;
+
+	for (i = 0; i < SGX_SECS_RESERVED4_SIZE; i++)
+		if (secs->reserved4[i])
+			return -EINVAL;
+
+	return 0;
+}
+
+static void sgx_mmu_notifier_release(struct mmu_notifier *mn,
+				     struct mm_struct *mm)
+{
+	struct sgx_encl *encl =
+		container_of(mn, struct sgx_encl, mmu_notifier);
+
+	mutex_lock(&encl->lock);
+	encl->flags |= SGX_ENCL_DEAD;
+	mutex_unlock(&encl->lock);
+}
+
+static const struct mmu_notifier_ops sgx_mmu_notifier_ops = {
+	.release	= sgx_mmu_notifier_release,
+};
+
+static int sgx_encl_grow(struct sgx_encl *encl)
+{
+	struct sgx_va_page *va_page;
+	int ret;
+
+	mutex_lock(&encl->lock);
+	if (!(encl->page_cnt % SGX_VA_SLOT_COUNT)) {
+		mutex_unlock(&encl->lock);
+
+		va_page = kzalloc(sizeof(*va_page), GFP_KERNEL);
+		if (!va_page)
+			return -ENOMEM;
+		va_page->epc_page = sgx_alloc_va_page(0);
+		if (IS_ERR(va_page->epc_page)) {
+			ret = PTR_ERR(va_page->epc_page);
+			kfree(va_page);
+			return ret;
+		}
+
+		mutex_lock(&encl->lock);
+		if (encl->page_cnt % SGX_VA_SLOT_COUNT) {
+			sgx_free_page(va_page->epc_page);
+			kfree(va_page);
+		} else {
+			list_add(&va_page->list, &encl->va_pages);
+		}
+	}
+	encl->page_cnt++;
+	mutex_unlock(&encl->lock);
+	return 0;
+}
+
+/**
+ * sgx_encl_alloc - allocate memory for an enclave and set attributes
+ *
+ * @secs:	SECS data (must be page aligned)
+ *
+ * Allocates a new &sgx_encl instance. Validates SECS attributes, creates
+ * backing storage for the enclave and sets enclave attributes to sane initial
+ * values.
+ *
+ * Return:
+ *   an &sgx_encl instance,
+ *   -errno otherwise
+ */
+struct sgx_encl *sgx_encl_alloc(struct sgx_secs *secs)
+{
+	unsigned long ssaframesize;
+	struct sgx_encl *encl;
+	struct file *backing;
+	struct file *pcmd;
+
+	ssaframesize = sgx_calc_ssaframesize(secs->miscselect, secs->xfrm);
+	if (sgx_validate_secs(secs, ssaframesize))
+		return ERR_PTR(-EINVAL);
+
+	backing = shmem_file_setup("[dev/sgx]", secs->size + PAGE_SIZE,
+				   VM_NORESERVE);
+	if (IS_ERR(backing))
+		return (void *)backing;
+
+	pcmd = shmem_file_setup("[dev/sgx]", (secs->size + PAGE_SIZE) >> 5,
+				VM_NORESERVE);
+	if (IS_ERR(pcmd)) {
+		fput(backing);
+		return (void *)pcmd;
+	}
+
+	encl = kzalloc(sizeof(*encl), GFP_KERNEL);
+	if (!encl) {
+		fput(backing);
+		fput(pcmd);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	encl->attributes = secs->attributes;
+	encl->xfrm = secs->xfrm;
+
+	kref_init(&encl->refcount);
+	INIT_LIST_HEAD(&encl->add_page_reqs);
+	INIT_LIST_HEAD(&encl->va_pages);
+	INIT_RADIX_TREE(&encl->page_tree, GFP_KERNEL);
+	mutex_init(&encl->lock);
+	INIT_WORK(&encl->add_page_work, sgx_add_page_worker);
+
+	encl->mm = current->mm;
+	encl->base = secs->base;
+	encl->size = secs->size;
+	encl->ssaframesize = secs->ssaframesize;
+	encl->backing = backing;
+	encl->pcmd = pcmd;
+
+	return encl;
+}
+
+
+/**
+ * sgx_encl_create - create an enclave
+ *
+ * @encl:	an enclave
+ * @secs:	page aligned SECS data
+ *
+ * Validates SECS attributes, allocates an EPC page for the SECS and creates
+ * the enclave by performing ECREATE.
+ *
+ * Return:
+ *   0 on success,
+ *   -errno otherwise
+ */
+int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs)
+{
+	struct vm_area_struct *vma;
+	struct sgx_pageinfo pginfo;
+	struct sgx_secinfo secinfo;
+	struct sgx_epc_page *secs_epc;
+	void *secs_vaddr;
+	long ret;
+
+	secs_epc = sgx_alloc_page(&encl->secs.impl, 0);
+	if (IS_ERR(secs_epc)) {
+		ret = PTR_ERR(secs_epc);
+		return ret;
+	}
+
+	sgx_set_epc_page(&encl->secs, secs_epc);
+	encl->secs.encl = encl;
+	encl->secs.impl.ops = &sgx_encl_page_ops;
+	encl->tgid = get_pid(task_tgid(current));
+
+	ret = sgx_encl_grow(encl);
+	if (ret)
+		return ret;
+
+	secs_vaddr = sgx_get_page(secs_epc);
+
+	pginfo.srcpge = (unsigned long)secs;
+	pginfo.linaddr = 0;
+	pginfo.secinfo = (unsigned long)&secinfo;
+	pginfo.secs = 0;
+	memset(&secinfo, 0, sizeof(secinfo));
+	ret = __ecreate((void *)&pginfo, secs_vaddr);
+
+	sgx_put_page(secs_vaddr);
+
+	if (ret) {
+		sgx_dbg(encl, "ECREATE returned %ld\n", ret);
+		return ret;
+	}
+
+	if (secs->attributes & SGX_ATTR_DEBUG)
+		encl->flags |= SGX_ENCL_DEBUG;
+
+	encl->mmu_notifier.ops = &sgx_mmu_notifier_ops;
+	ret = mmu_notifier_register(&encl->mmu_notifier, encl->mm);
+	if (ret) {
+		if (ret == -EINTR)
+			ret = -ERESTARTSYS;
+		encl->mmu_notifier.ops = NULL;
+		return ret;
+	}
+
+	down_read(&current->mm->mmap_sem);
+	ret = sgx_encl_find(current->mm, secs->base, &vma);
+	if (ret != -ENOENT) {
+		if (!ret)
+			ret = -EINVAL;
+		up_read(&current->mm->mmap_sem);
+		return ret;
+	}
+
+	if (vma->vm_start != secs->base ||
+	    vma->vm_end != (secs->base + secs->size) ||
+	    vma->vm_pgoff != 0) {
+		ret = -EINVAL;
+		up_read(&current->mm->mmap_sem);
+		return ret;
+	}
+
+	vma->vm_private_data = encl;
+	up_read(&current->mm->mmap_sem);
+	return 0;
+}
+
+static int sgx_validate_secinfo(struct sgx_secinfo *secinfo)
+{
+	u64 page_type = secinfo->flags & SGX_SECINFO_PAGE_TYPE_MASK;
+	u64 perm = secinfo->flags & SGX_SECINFO_PERMISSION_MASK;
+	int i;
+
+	if ((secinfo->flags & SGX_SECINFO_RESERVED_MASK) ||
+	    ((perm & SGX_SECINFO_W) && !(perm & SGX_SECINFO_R)) ||
+	    (page_type != SGX_SECINFO_TCS &&
+	     page_type != SGX_SECINFO_REG))
+		return -EINVAL;
+
+	for (i = 0; i < sizeof(secinfo->reserved) / sizeof(u64); i++)
+		if (secinfo->reserved[i])
+			return -EINVAL;
+
+	return 0;
+}
+
+static bool sgx_validate_offset(struct sgx_encl *encl, unsigned long offset)
+{
+	if (offset & (PAGE_SIZE - 1))
+		return false;
+
+	if (offset >= encl->size)
+		return false;
+
+	return true;
+}
+
+static int sgx_validate_tcs(struct sgx_encl *encl, struct sgx_tcs *tcs)
+{
+	int i;
+
+	if (tcs->flags & SGX_TCS_RESERVED_MASK) {
+		sgx_dbg(encl, "%s: invalid TCS flags = 0x%lx\n",
+			__func__, (unsigned long)tcs->flags);
+		return -EINVAL;
+	}
+
+	if (tcs->flags & SGX_TCS_DBGOPTIN) {
+		sgx_dbg(encl, "%s: DBGOPTIN TCS flag is set, EADD will clear it\n",
+			__func__);
+		return -EINVAL;
+	}
+
+	if (!sgx_validate_offset(encl, tcs->ossa)) {
+		sgx_dbg(encl, "%s: invalid OSSA: 0x%lx\n", __func__,
+			(unsigned long)tcs->ossa);
+		return -EINVAL;
+	}
+
+	if (!sgx_validate_offset(encl, tcs->ofsbase)) {
+		sgx_dbg(encl, "%s: invalid OFSBASE: 0x%lx\n", __func__,
+			(unsigned long)tcs->ofsbase);
+		return -EINVAL;
+	}
+
+	if (!sgx_validate_offset(encl, tcs->ogsbase)) {
+		sgx_dbg(encl, "%s: invalid OGSBASE: 0x%lx\n", __func__,
+			(unsigned long)tcs->ogsbase);
+		return -EINVAL;
+	}
+
+	if ((tcs->fslimit & 0xFFF) != 0xFFF) {
+		sgx_dbg(encl, "%s: invalid FSLIMIT: 0x%x\n", __func__,
+			tcs->fslimit);
+		return -EINVAL;
+	}
+
+	if ((tcs->gslimit & 0xFFF) != 0xFFF) {
+		sgx_dbg(encl, "%s: invalid GSLIMIT: 0x%x\n", __func__,
+			tcs->gslimit);
+		return -EINVAL;
+	}
+
+	for (i = 0; i < sizeof(tcs->reserved) / sizeof(u64); i++)
+		if (tcs->reserved[i])
+			return -EINVAL;
+
+	return 0;
+}
+
+static int __sgx_encl_add_page(struct sgx_encl *encl,
+			       struct sgx_encl_page *encl_page,
+			       void *data,
+			       struct sgx_secinfo *secinfo,
+			       unsigned int mrmask)
+{
+	u64 page_type = secinfo->flags & SGX_SECINFO_PAGE_TYPE_MASK;
+	struct sgx_add_page_req *req = NULL;
+	pgoff_t backing_index;
+	struct page *backing;
+	void *backing_ptr;
+	int empty;
+
+	req = kzalloc(sizeof(*req), GFP_KERNEL);
+	if (!req)
+		return -ENOMEM;
+
+	backing_index = SGX_ENCL_PAGE_BACKING_INDEX(encl_page, encl);
+	backing = sgx_get_backing(encl->backing, backing_index);
+	if (IS_ERR(backing)) {
+		kfree(req);
+		return PTR_ERR(backing);
+	}
+	backing_ptr = kmap(backing);
+	memcpy(backing_ptr, data, PAGE_SIZE);
+	kunmap(backing);
+	if (page_type == SGX_SECINFO_TCS)
+		encl_page->desc |= SGX_ENCL_PAGE_TCS;
+	memcpy(&req->secinfo, secinfo, sizeof(*secinfo));
+	req->encl = encl;
+	req->encl_page = encl_page;
+	req->mrmask = mrmask;
+	empty = list_empty(&encl->add_page_reqs);
+	kref_get(&encl->refcount);
+	list_add_tail(&req->list, &encl->add_page_reqs);
+	if (empty)
+		queue_work(sgx_add_page_wq, &encl->add_page_work);
+	sgx_put_backing(backing, true /* write */);
+	return 0;
+}
+
+/**
+ * sgx_encl_alloc_page - allocate a new enclave page
+ * @encl:	an enclave
+ * @addr:	page address in the ELRANGE
+ *
+ * Return:
+ *   an &sgx_encl_page instance on success,
+ *   -errno otherwise
+ */
+struct sgx_encl_page *sgx_encl_alloc_page(struct sgx_encl *encl,
+					  unsigned long addr)
+{
+	struct sgx_encl_page *encl_page;
+	int ret;
+
+	if (radix_tree_lookup(&encl->page_tree, PFN_DOWN(addr)))
+		return ERR_PTR(-EEXIST);
+	encl_page = kzalloc(sizeof(*encl_page), GFP_KERNEL);
+	if (!encl_page)
+		return ERR_PTR(-ENOMEM);
+	encl_page->desc = addr;
+	encl_page->impl.ops = &sgx_encl_page_ops;
+	encl_page->encl = encl;
+	ret = radix_tree_insert(&encl->page_tree, PFN_DOWN(encl_page->desc),
+				encl_page);
+	if (ret) {
+		kfree(encl_page);
+		return ERR_PTR(ret);
+	}
+	return encl_page;
+}
+
+/**
+ * sgx_encl_free_page - free an enclave page
+ * @encl_page:	an enclave page
+ */
+void sgx_encl_free_page(struct sgx_encl_page *encl_page)
+{
+	radix_tree_delete(&encl_page->encl->page_tree,
+			  PFN_DOWN(encl_page->desc));
+	if (encl_page->desc & SGX_ENCL_PAGE_LOADED) {
+		spin_lock(&sgx_active_page_list_lock);
+		list_del(&encl_page->epc_page->list);
+		spin_unlock(&sgx_active_page_list_lock);
+		sgx_free_page(encl_page->epc_page);
+	}
+	kfree(encl_page);
+}
+
+/**
+ * sgx_encl_add_page - add a page to the enclave
+ *
+ * @encl:	an enclave
+ * @addr:	page address in the ELRANGE
+ * @data:	page data
+ * @secinfo:	page permissions
+ * @mrmask:	bitmask to select the 256 byte chunks to be measured
+ *
+ * Creates a new enclave page and enqueues an EADD operation that will be
+ * processed by a worker thread later on.
+ *
+ * Return:
+ *   0 on success,
+ *   -errno otherwise
+ */
+int sgx_encl_add_page(struct sgx_encl *encl, unsigned long addr, void *data,
+		      struct sgx_secinfo *secinfo, unsigned int mrmask)
+{
+	u64 page_type = secinfo->flags & SGX_SECINFO_PAGE_TYPE_MASK;
+	struct sgx_encl_page *encl_page;
+	int ret;
+
+	if (sgx_validate_secinfo(secinfo))
+		return -EINVAL;
+	if (page_type == SGX_SECINFO_TCS) {
+		ret = sgx_validate_tcs(encl, data);
+		if (ret)
+			return ret;
+	}
+	ret = sgx_encl_grow(encl);
+	if (ret)
+		return ret;
+	mutex_lock(&encl->lock);
+	if (encl->flags & (SGX_ENCL_INITIALIZED | SGX_ENCL_DEAD)) {
+		mutex_unlock(&encl->lock);
+		return -EINVAL;
+	}
+	encl_page = sgx_encl_alloc_page(encl, addr);
+	if (IS_ERR(encl_page)) {
+		mutex_unlock(&encl->lock);
+		return PTR_ERR(encl_page);
+	}
+	ret = __sgx_encl_add_page(encl, encl_page, data, secinfo, mrmask);
+	if (ret)
+		sgx_encl_free_page(encl_page);
+	mutex_unlock(&encl->lock);
+	return ret;
+}
+
+static int __sgx_get_key_hash(struct crypto_shash *tfm, const void *modulus,
+			      void *hash)
+{
+	SHASH_DESC_ON_STACK(shash, tfm);
+
+	shash->tfm = tfm;
+	shash->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
+
+	return crypto_shash_digest(shash, modulus, SGX_MODULUS_SIZE, hash);
+}
+
+static int sgx_get_key_hash(const void *modulus, void *hash)
+{
+	struct crypto_shash *tfm;
+	int ret;
+
+	tfm = crypto_alloc_shash("sha256", 0, CRYPTO_ALG_ASYNC);
+	if (IS_ERR(tfm))
+		return PTR_ERR(tfm);
+
+	ret = __sgx_get_key_hash(tfm, modulus, hash);
+
+	crypto_free_shash(tfm);
+	return ret;
+}
+
+/**
+ * sgx_encl_init - perform EINIT for the given enclave
+ *
+ * @encl:	an enclave
+ * @sigstruct:	SIGSTRUCT for the enclave
+ * @token:	EINITTOKEN for the enclave
+ *
+ * Retries a few times in order to perform EINIT operation on an enclave
+ * because there could be potentially an interrupt storm.
+ *
+ * Return:
+ *   0 on success,
+ *   SGX error code on EINIT failure,
+ *   -errno otherwise
+ */
+int sgx_encl_init(struct sgx_encl *encl, struct sgx_sigstruct *sigstruct,
+		  struct sgx_einittoken *token)
+{
+	u64 mrsigner[4];
+	int ret;
+	int i;
+	int j;
+
+	ret = sgx_get_key_hash(sigstruct->modulus, mrsigner);
+	if (ret)
+		return ret;
+
+	flush_work(&encl->add_page_work);
+
+	mutex_lock(&encl->lock);
+
+	if (encl->flags & SGX_ENCL_INITIALIZED) {
+		mutex_unlock(&encl->lock);
+		return 0;
+	}
+
+	for (i = 0; i < SGX_EINIT_SLEEP_COUNT; i++) {
+		for (j = 0; j < SGX_EINIT_SPIN_COUNT; j++) {
+			ret = sgx_einit(sigstruct, token, encl->secs.epc_page,
+					mrsigner);
+			if (ret == SGX_UNMASKED_EVENT)
+				continue;
+			else
+				break;
+		}
+
+		if (ret != SGX_UNMASKED_EVENT)
+			break;
+
+		msleep_interruptible(SGX_EINIT_SLEEP_TIME);
+		if (signal_pending(current)) {
+			mutex_unlock(&encl->lock);
+			return -ERESTARTSYS;
+		}
+	}
+
+	if (ret > 0)
+		sgx_dbg(encl, "EINIT returned %d\n", ret);
+	else if (!ret)
+		encl->flags |= SGX_ENCL_INITIALIZED;
+	mutex_unlock(&encl->lock);
+
+	return ret;
+}
+
+/**
+ * sgx_encl_block - block an enclave page
+ * @encl_page:	an enclave page
+ *
+ * Changes the state of the associated EPC page to blocked.
+ */
+void sgx_encl_block(struct sgx_encl_page *encl_page)
+{
+	unsigned long addr = SGX_ENCL_PAGE_ADDR(encl_page);
+	struct sgx_encl *encl = encl_page->encl;
+	struct vm_area_struct *vma;
+	int ret;
+
+	if (encl->flags & SGX_ENCL_DEAD)
+		return;
+
+	ret = sgx_encl_find(encl->mm, addr, &vma);
+	if (ret || encl != vma->vm_private_data)
+		return;
+
+	zap_vma_ptes(vma, addr, PAGE_SIZE);
+	ret = sgx_eblock(encl_page->epc_page);
+	SGX_INVD(ret, encl, "EBLOCK returned %d\n", ret);
+}
+
+/**
+ * sgx_encl_track - start tracking pages in the blocked state
+ * @encl:	an enclave
+ *
+ * Start blocking accesses for pages in the blocked state for threads that enter
+ * inside the enclave by executing the ETRACK leaf instruction. This starts a
+ * shootdown sequence for threads that entered before ETRACK.
+ *
+ * The caller must take care (with an IPI when necessary) to make sure that the
+ * previous shootdown sequence was completed before calling this function.  If
+ * this is not the case, the callee prints a critical error to the klog and
+ * kills the enclave.
+ */
+void sgx_encl_track(struct sgx_encl *encl)
+{
+	int ret = sgx_etrack(encl->secs.epc_page);
+
+	SGX_INVD(ret, encl, "ETRACK returned %d\n", ret);
+}
+
+/**
+ * sgx_encl_load_page - load an enclave page
+ * @encl_page:	a &sgx_encl_page
+ * @epc_page:	a &sgx_epc_page
+ *
+ * Loads an enclave page from the regular memory to the EPC. The pages, which
+ * are not children of a SECS (eg SECS itself and VA pages) should set their
+ * address to zero.
+ */
+int sgx_encl_load_page(struct sgx_encl_page *encl_page,
+		       struct sgx_epc_page *epc_page)
+{
+	unsigned long addr = SGX_ENCL_PAGE_ADDR(encl_page);
+	struct sgx_encl *encl = encl_page->encl;
+	struct sgx_pageinfo pginfo;
+	unsigned long pcmd_offset;
+	unsigned long va_offset;
+	void *secs_ptr = NULL;
+	pgoff_t backing_index;
+	struct page *backing;
+	struct page *pcmd;
+	void *epc_ptr;
+	void *va_ptr;
+	int ret;
+
+	backing_index = SGX_ENCL_PAGE_BACKING_INDEX(encl_page, encl);
+	pcmd_offset = SGX_ENCL_PAGE_PCMD_OFFSET(encl_page, encl);
+	va_offset = SGX_ENCL_PAGE_VA_OFFSET(encl_page);
+
+	backing = sgx_get_backing(encl->backing, backing_index);
+	if (IS_ERR(backing))
+		return PTR_ERR(backing);
+
+	pcmd = sgx_get_backing(encl->pcmd, backing_index >> 5);
+	if (IS_ERR(pcmd)) {
+		sgx_put_backing(backing, false);
+		return PTR_ERR(pcmd);
+	}
+
+	if (addr)
+		secs_ptr = sgx_get_page(encl->secs.epc_page);
+
+	epc_ptr = sgx_get_page(epc_page);
+	va_ptr = sgx_get_page(encl_page->va_page->epc_page);
+	pginfo.srcpge = (unsigned long)kmap_atomic(backing);
+	pginfo.pcmd = (unsigned long)kmap_atomic(pcmd) + pcmd_offset;
+	pginfo.linaddr = addr;
+	pginfo.secs = (unsigned long)secs_ptr;
+
+	ret = __eldu(&pginfo, epc_ptr, va_ptr + va_offset);
+	if (ret) {
+		sgx_err(encl, "ELDU returned %d\n", ret);
+		ret = -EFAULT;
+	}
+
+	kunmap_atomic((void *)(unsigned long)(pginfo.pcmd - pcmd_offset));
+	kunmap_atomic((void *)(unsigned long)pginfo.srcpge);
+	sgx_put_page(va_ptr);
+	sgx_put_page(epc_ptr);
+
+	if (addr)
+		sgx_put_page(secs_ptr);
+
+	sgx_put_backing(pcmd, false);
+	sgx_put_backing(backing, false);
+	return ret;
+}
+
+/**
+ * sgx_encl_release - destroy an enclave
+ *
+ * @kref:	address of the kref inside &sgx_encl
+ *
+ * Used togethere with kref_put(). Frees all the resources associated with
+ * the enclave.
+ */
+void sgx_encl_release(struct kref *ref)
+{
+	struct sgx_encl *encl = container_of(ref, struct sgx_encl, refcount);
+	struct sgx_encl_page *entry;
+	struct radix_tree_iter iter;
+	void **slot;
+
+	radix_tree_for_each_slot(slot, &encl->page_tree, &iter, 0) {
+		entry = *slot;
+		sgx_encl_free_page(entry);
+	}
+
+	put_pid(encl->tgid);
+
+	if (encl->mmu_notifier.ops)
+		mmu_notifier_unregister_no_release(&encl->mmu_notifier,
+						   encl->mm);
+
+	sgx_encl_free_va_pages(encl);
+
+	if (encl->secs.desc & SGX_ENCL_PAGE_LOADED)
+		sgx_free_page(encl->secs.epc_page);
+
+	if (encl->backing)
+		fput(encl->backing);
+
+	if (encl->pcmd)
+		fput(encl->pcmd);
+
+	kfree(encl);
+}
diff --git a/drivers/platform/x86/intel_sgx/sgx_encl_page.c b/drivers/platform/x86/intel_sgx/sgx_encl_page.c
new file mode 100644
index 000000000000..65132944c428
--- /dev/null
+++ b/drivers/platform/x86/intel_sgx/sgx_encl_page.c
@@ -0,0 +1,286 @@
+// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
+// Copyright(c) 2016-18 Intel Corporation.
+
+#include <linux/device.h>
+#include <linux/freezer.h>
+#include <linux/highmem.h>
+#include <linux/kthread.h>
+#include <linux/ratelimit.h>
+#include <linux/sched/signal.h>
+#include <linux/slab.h>
+#include "sgx.h"
+
+static bool sgx_encl_page_get(struct sgx_epc_page *epc_page)
+{
+	struct sgx_encl_page *encl_page = container_of(epc_page->impl,
+						       struct sgx_encl_page,
+						       impl);
+	struct sgx_encl *encl = encl_page->encl;
+
+	return kref_get_unless_zero(&encl->refcount) != 0;
+}
+
+static void sgx_encl_page_put(struct sgx_epc_page *epc_page)
+{
+	struct sgx_encl_page *encl_page = container_of(epc_page->impl,
+						       struct sgx_encl_page,
+						       impl);
+	struct sgx_encl *encl = encl_page->encl;
+
+	kref_put(&encl->refcount, sgx_encl_release);
+}
+
+static bool sgx_encl_page_reclaim(struct sgx_epc_page *epc_page)
+{
+	struct sgx_encl_page *encl_page = container_of(epc_page->impl,
+						       struct sgx_encl_page,
+						       impl);
+	struct sgx_encl *encl = encl_page->encl;
+	bool ret = false;
+
+	down_read(&encl->mm->mmap_sem);
+	mutex_lock(&encl->lock);
+	if ((encl->flags & SGX_ENCL_DEAD) ||
+	    (!sgx_test_and_clear_young(encl_page) &&
+	     !(encl_page->desc & SGX_ENCL_PAGE_RESERVED))) {
+		encl_page->desc |= SGX_ENCL_PAGE_RESERVED;
+		ret = true;
+	}
+	mutex_unlock(&encl->lock);
+	up_read(&encl->mm->mmap_sem);
+	return ret;
+}
+
+static void sgx_encl_page_block(struct sgx_epc_page *epc_page)
+{
+	struct sgx_encl_page *encl_page = container_of(epc_page->impl,
+						       struct sgx_encl_page,
+						       impl);
+	struct sgx_encl *encl = encl_page->encl;
+
+	down_read(&encl->mm->mmap_sem);
+	mutex_lock(&encl->lock);
+	sgx_encl_block(encl_page);
+	mutex_unlock(&encl->lock);
+	up_read(&encl->mm->mmap_sem);
+}
+
+static int sgx_ewb(struct sgx_encl *encl, struct sgx_epc_page *epc_page,
+		   struct sgx_va_page *va_page, unsigned int va_offset)
+{
+	struct sgx_encl_page *encl_page = container_of(epc_page->impl,
+						       struct sgx_encl_page,
+						       impl);
+	unsigned long pcmd_offset = SGX_ENCL_PAGE_PCMD_OFFSET(encl_page, encl);
+	struct sgx_pageinfo pginfo;
+	pgoff_t backing_index;
+	struct page *backing;
+	struct page *pcmd;
+	void *epc;
+	void *va;
+	int ret;
+
+	backing_index = SGX_ENCL_PAGE_BACKING_INDEX(encl_page, encl);
+
+	backing = sgx_get_backing(encl->backing, backing_index);
+	if (IS_ERR(backing)) {
+		ret = PTR_ERR(backing);
+		return ret;
+	}
+
+	pcmd = sgx_get_backing(encl->pcmd, backing_index >> 5);
+	if (IS_ERR(pcmd)) {
+		ret = PTR_ERR(pcmd);
+		sgx_put_backing(backing, true);
+		return ret;
+	}
+
+	epc = sgx_get_page(epc_page);
+	va = sgx_get_page(va_page->epc_page);
+
+	pginfo.srcpge = (unsigned long)kmap_atomic(backing);
+	pginfo.pcmd = (unsigned long)kmap_atomic(pcmd) + pcmd_offset;
+	pginfo.linaddr = 0;
+	pginfo.secs = 0;
+	ret = __ewb(&pginfo, epc, (void *)((unsigned long)va + va_offset));
+	kunmap_atomic((void *)(unsigned long)(pginfo.pcmd - pcmd_offset));
+	kunmap_atomic((void *)(unsigned long)pginfo.srcpge);
+
+	sgx_put_page(va);
+	sgx_put_page(epc);
+	sgx_put_backing(pcmd, true);
+	sgx_put_backing(backing, true);
+
+	return ret;
+}
+
+/**
+ * sgx_write_page - write a page to the regular memory
+ *
+ * Writes an EPC page to the shmem file associated with the enclave. Flushes
+ * CPUs and retries if there are hardware threads that can potentially have TLB
+ * entries to the page (indicated by SGX_NOT_TRACKED). Clears the reserved flag
+ * after the page is swapped.
+ *
+ * @epc_page:	an EPC page
+ */
+static void sgx_write_page(struct sgx_epc_page *epc_page, bool do_free)
+{
+	struct sgx_encl_page *encl_page = container_of(epc_page->impl,
+						       struct sgx_encl_page,
+						       impl);
+	struct sgx_encl *encl = encl_page->encl;
+	struct sgx_va_page *va_page;
+	unsigned int va_offset;
+	int ret;
+
+	if (!(encl->flags & SGX_ENCL_DEAD)) {
+		va_page = list_first_entry(&encl->va_pages, struct sgx_va_page,
+					   list);
+		va_offset = sgx_alloc_va_slot(va_page);
+		if (sgx_va_page_full(va_page))
+			list_move_tail(&va_page->list, &encl->va_pages);
+
+		ret = sgx_ewb(encl, epc_page, va_page, va_offset);
+		if (ret == SGX_NOT_TRACKED) {
+			sgx_encl_track(encl);
+			ret = sgx_ewb(encl, epc_page, va_page, va_offset);
+			if (ret == SGX_NOT_TRACKED) {
+				/* slow path, IPI needed */
+				sgx_flush_cpus(encl);
+				ret = sgx_ewb(encl, epc_page, va_page,
+					      va_offset);
+			}
+		}
+		SGX_INVD(ret, encl, "EWB returned %d\n", ret);
+
+		encl_page->desc |= va_offset;
+		encl_page->va_page = va_page;
+		encl_page->desc &= ~SGX_ENCL_PAGE_RESERVED;
+	}
+	encl_page->desc &= ~SGX_ENCL_PAGE_LOADED;
+	if (do_free)
+		sgx_free_page(epc_page);
+}
+
+static void sgx_encl_page_write(struct sgx_epc_page *epc_page)
+{
+	struct sgx_encl_page *encl_page = container_of(epc_page->impl,
+						       struct sgx_encl_page,
+						       impl);
+	struct sgx_encl *encl = encl_page->encl;
+
+	down_read(&encl->mm->mmap_sem);
+	mutex_lock(&encl->lock);
+	sgx_write_page(epc_page, false);
+	encl->secs_child_cnt--;
+	if (!encl->secs_child_cnt && (encl->flags & SGX_ENCL_INITIALIZED))
+		sgx_write_page(encl->secs.epc_page, true);
+	mutex_unlock(&encl->lock);
+	up_read(&encl->mm->mmap_sem);
+}
+
+const struct sgx_epc_page_ops sgx_encl_page_ops = {
+	.get = sgx_encl_page_get,
+	.put = sgx_encl_page_put,
+	.reclaim = sgx_encl_page_reclaim,
+	.block = sgx_encl_page_block,
+	.write = sgx_encl_page_write,
+};
+
+/**
+ * sgx_set_page_reclaimable - associated an EPC page with an enclave page
+ * @encl_page:	an enclave page
+ * @epc_page:	the EPC page to attach to @encl_page
+ */
+void sgx_set_epc_page(struct sgx_encl_page *encl_page,
+		      struct sgx_epc_page *epc_page)
+{
+	encl_page->desc |= SGX_ENCL_PAGE_LOADED;
+	encl_page->epc_page = epc_page;
+}
+
+/**
+ * sgx_set_page_reclaimable - mark an EPC page reclaimable
+ * @encl_page:	an enclave page with a loaded EPC page
+ */
+void sgx_set_page_reclaimable(struct sgx_encl_page *encl_page)
+{
+	sgx_test_and_clear_young(encl_page);
+
+	spin_lock(&sgx_active_page_list_lock);
+	list_add_tail(&encl_page->epc_page->list, &sgx_active_page_list);
+	spin_unlock(&sgx_active_page_list_lock);
+}
+
+/**
+ * sgx_alloc_page - allocate a VA page
+ * @flags:	allocation flags
+ *
+ * Allocates an &sgx_epc_page instance and converts it to a VA page.
+ *
+ * Return:
+ *   a &struct sgx_va_page instance,
+ *   -errno otherwise
+ */
+struct sgx_epc_page *sgx_alloc_va_page(unsigned int flags)
+{
+	struct sgx_epc_page *epc_page;
+	int ret;
+
+	epc_page = sgx_alloc_page(NULL, flags);
+	if (IS_ERR(epc_page))
+		return (void *)epc_page;
+
+	ret = sgx_epa(epc_page);
+	if (ret) {
+		pr_crit("EPA failed\n");
+		sgx_free_page(epc_page);
+		return ERR_PTR(ret);
+	}
+
+	return epc_page;
+}
+
+/**
+ * sgx_alloc_va_slot - allocate a VA slot
+ * @va_page:	a &struct sgx_va_page instance
+ *
+ * Allocates a slot from a &struct sgx_va_page instance.
+ *
+ * Return: offset of the slot inside the VA page
+ */
+unsigned int sgx_alloc_va_slot(struct sgx_va_page *va_page)
+{
+	int slot = find_first_zero_bit(va_page->slots, SGX_VA_SLOT_COUNT);
+
+	if (slot < SGX_VA_SLOT_COUNT)
+		set_bit(slot, va_page->slots);
+
+	return slot << 3;
+}
+
+/**
+ * sgx_free_va_slot - free a VA slot
+ * @va_page:	a &struct sgx_va_page instance
+ * @offset:	offset of the slot inside the VA page
+ *
+ * Frees a slot from a &struct sgx_va_page instance.
+ */
+void sgx_free_va_slot(struct sgx_va_page *va_page, unsigned int offset)
+{
+	clear_bit(offset >> 3, va_page->slots);
+}
+
+/**
+ * sgx_va_page_full - is the VA page full?
+ * @va_page:	a &struct sgx_va_page instance
+ *
+ * Return: true if all slots have been taken
+ */
+bool sgx_va_page_full(struct sgx_va_page *va_page)
+{
+	int slot = find_first_zero_bit(va_page->slots, SGX_VA_SLOT_COUNT);
+
+	return slot == SGX_VA_SLOT_COUNT;
+}
diff --git a/drivers/platform/x86/intel_sgx/sgx_fault.c b/drivers/platform/x86/intel_sgx/sgx_fault.c
new file mode 100644
index 000000000000..f5daf7b223b3
--- /dev/null
+++ b/drivers/platform/x86/intel_sgx/sgx_fault.c
@@ -0,0 +1,151 @@
+// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
+// Copyright(c) 2016-18 Intel Corporation.
+
+#include <linux/highmem.h>
+#include <linux/sched/mm.h>
+#include "sgx.h"
+
+static int sgx_test_and_clear_young_cb(pte_t *ptep, pgtable_t token,
+				       unsigned long addr, void *data)
+{
+	pte_t pte;
+	int ret;
+
+	ret = pte_young(*ptep);
+	if (ret) {
+		pte = pte_mkold(*ptep);
+		set_pte_at((struct mm_struct *)data, addr, ptep, pte);
+	}
+
+	return ret;
+}
+
+/**
+ * sgx_test_and_clear_young() - Test and reset the accessed bit
+ * @page:	enclave page to be tested for recent access
+ *
+ * Checks the Access (A) bit from the PTE corresponding to the
+ * enclave page and clears it.  Returns 1 if the page has been
+ * recently accessed and 0 if not.
+ */
+int sgx_test_and_clear_young(struct sgx_encl_page *page)
+{
+	unsigned long addr = SGX_ENCL_PAGE_ADDR(page);
+	struct sgx_encl *encl = page->encl;
+	struct vm_area_struct *vma;
+	int ret;
+
+	ret = sgx_encl_find(encl->mm, addr, &vma);
+	if (ret)
+		return 0;
+
+	if (encl != vma->vm_private_data)
+		return 0;
+
+	return apply_to_page_range(vma->vm_mm, addr, PAGE_SIZE,
+				   sgx_test_and_clear_young_cb, vma->vm_mm);
+}
+
+static void sgx_ipi_cb(void *info)
+{
+}
+
+void sgx_flush_cpus(struct sgx_encl *encl)
+{
+	on_each_cpu_mask(mm_cpumask(encl->mm), sgx_ipi_cb, NULL, 1);
+}
+
+static struct sgx_epc_page *__sgx_load_faulted_page(
+	struct sgx_encl_page *encl_page)
+{
+	unsigned long va_offset = SGX_ENCL_PAGE_VA_OFFSET(encl_page);
+	struct sgx_encl *encl = encl_page->encl;
+	struct sgx_epc_page *epc_page;
+	int ret;
+
+	epc_page = sgx_alloc_page(&encl_page->impl, SGX_ALLOC_ATOMIC);
+	if (IS_ERR(epc_page))
+		return epc_page;
+	ret = sgx_encl_load_page(encl_page, epc_page);
+	if (ret) {
+		sgx_free_page(epc_page);
+		return ERR_PTR(ret);
+	}
+	sgx_free_va_slot(encl_page->va_page, va_offset);
+	list_move(&encl_page->va_page->list, &encl->va_pages);
+	encl_page->desc &= ~SGX_VA_OFFSET_MASK;
+	sgx_set_epc_page(encl_page, epc_page);
+	return epc_page;
+}
+
+static struct sgx_encl_page *__sgx_fault_page(struct vm_area_struct *vma,
+					      unsigned long addr,
+					      bool do_reserve)
+{
+	struct sgx_encl *encl = vma->vm_private_data;
+	struct sgx_epc_page *epc_page;
+	struct sgx_encl_page *entry;
+	int rc = 0;
+
+	if ((encl->flags & SGX_ENCL_DEAD) ||
+	    !(encl->flags & SGX_ENCL_INITIALIZED))
+		return ERR_PTR(-EFAULT);
+
+	entry = radix_tree_lookup(&encl->page_tree, addr >> PAGE_SHIFT);
+	if (!entry)
+		return ERR_PTR(-EFAULT);
+	if (entry->desc & SGX_ENCL_PAGE_RESERVED) {
+		sgx_dbg(encl, "0x%p is reserved\n",
+			(void *)SGX_ENCL_PAGE_ADDR(entry));
+		return ERR_PTR(-EBUSY);
+	}
+	/* Page was faulted by another thread. */
+	if (entry->desc & SGX_ENCL_PAGE_LOADED) {
+		if (do_reserve)
+			entry->desc |= SGX_ENCL_PAGE_RESERVED;
+		return entry;
+	}
+
+	if (!(encl->secs.desc & SGX_ENCL_PAGE_LOADED)) {
+		epc_page = __sgx_load_faulted_page(&encl->secs);
+		if (IS_ERR(epc_page))
+			return (void *)epc_page;
+	}
+	epc_page = __sgx_load_faulted_page(entry);
+	if (IS_ERR(epc_page))
+		return (void *)epc_page;
+
+	encl->secs_child_cnt++;
+	sgx_set_page_reclaimable(entry);
+	if (do_reserve)
+		entry->desc |= SGX_ENCL_PAGE_RESERVED;
+
+	rc = vm_insert_pfn(vma, addr, SGX_EPC_PFN(entry->epc_page));
+	SGX_INVD(rc, encl, "%s: vm_insert_pfn() returned %d\n", __func__, rc);
+	if (rc)
+		return ERR_PTR(rc);
+
+	return entry;
+}
+
+struct sgx_encl_page *sgx_fault_page(struct vm_area_struct *vma,
+				     unsigned long addr, bool do_reserve)
+{
+	struct sgx_encl *encl = vma->vm_private_data;
+	struct sgx_encl_page *entry;
+
+	/* If process was forked, VMA is still there but vm_private_data is set
+	 * to NULL.
+	 */
+	if (!encl)
+		return ERR_PTR(-EFAULT);
+	do {
+		mutex_lock(&encl->lock);
+		entry = __sgx_fault_page(vma, addr, do_reserve);
+		mutex_unlock(&encl->lock);
+		if (!do_reserve)
+			break;
+	} while (PTR_ERR(entry) == -EBUSY);
+
+	return entry;
+}
diff --git a/drivers/platform/x86/intel_sgx/sgx_ioctl.c b/drivers/platform/x86/intel_sgx/sgx_ioctl.c
new file mode 100644
index 000000000000..6f4d20bb6e33
--- /dev/null
+++ b/drivers/platform/x86/intel_sgx/sgx_ioctl.c
@@ -0,0 +1,231 @@
+// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
+// Copyright(c) 2016-18 Intel Corporation.
+
+#include <asm/mman.h>
+#include <linux/delay.h>
+#include <linux/file.h>
+#include <linux/hashtable.h>
+#include <linux/highmem.h>
+#include <linux/ratelimit.h>
+#include <linux/sched/signal.h>
+#include <linux/slab.h>
+#include "sgx.h"
+
+static int sgx_encl_get(unsigned long addr, struct sgx_encl **encl)
+{
+	struct mm_struct *mm = current->mm;
+	struct vm_area_struct *vma;
+	int ret;
+
+	if (addr & (PAGE_SIZE - 1))
+		return -EINVAL;
+
+	down_read(&mm->mmap_sem);
+
+	ret = sgx_encl_find(mm, addr, &vma);
+	if (!ret) {
+		*encl = vma->vm_private_data;
+
+		if ((*encl)->flags & SGX_ENCL_SUSPEND)
+			ret = SGX_POWER_LOST_ENCLAVE;
+		else
+			kref_get(&(*encl)->refcount);
+	}
+
+	up_read(&mm->mmap_sem);
+	return ret;
+}
+
+/**
+ * sgx_ioc_enclave_create - handler for %SGX_IOC_ENCLAVE_CREATE
+ * @filep:	open file to /dev/sgx
+ * @cmd:	the command value
+ * @arg:	pointer to an &sgx_enclave_create instance
+ *
+ * Validates SECS attributes, allocates an EPC page for the SECS and performs
+ * ECREATE.
+ *
+ * Return:
+ *   0 on success,
+ *   -errno otherwise
+ */
+static long sgx_ioc_enclave_create(struct file *filep, unsigned int cmd,
+				   unsigned long arg)
+{
+	struct sgx_enclave_create *createp = (struct sgx_enclave_create *)arg;
+	struct sgx_secs *secs;
+	struct sgx_encl *encl;
+	int ret;
+
+	secs = kzalloc(sizeof(*secs),  GFP_KERNEL);
+	if (!secs)
+		return -ENOMEM;
+
+	ret = copy_from_user(secs, (void __user *)createp->src, sizeof(*secs));
+	if (ret)
+		goto out;
+
+	encl = sgx_encl_alloc(secs);
+	if (IS_ERR(encl)) {
+		ret = PTR_ERR(encl);
+		goto out;
+	}
+
+	ret = sgx_encl_create(encl, secs);
+	if (ret)
+		kref_put(&encl->refcount, sgx_encl_release);
+
+out:
+	kfree(secs);
+	return ret;
+}
+
+/**
+ * sgx_ioc_enclave_add_page - handler for %SGX_IOC_ENCLAVE_ADD_PAGE
+ *
+ * @filep:	open file to /dev/sgx
+ * @cmd:	the command value
+ * @arg:	pointer to an &sgx_enclave_add_page instance
+ *
+ * Creates a new enclave page and enqueues an EADD operation that will be
+ * processed by a worker thread later on.
+ *
+ * Return:
+ *   0 on success,
+ *   -errno otherwise
+ */
+static long sgx_ioc_enclave_add_page(struct file *filep, unsigned int cmd,
+				     unsigned long arg)
+{
+	struct sgx_enclave_add_page *addp = (void *)arg;
+	struct sgx_secinfo secinfo;
+	struct sgx_encl *encl;
+	struct page *data_page;
+	void *data;
+	int ret;
+
+	ret = sgx_encl_get(addp->addr, &encl);
+	if (ret)
+		return ret;
+
+	if (copy_from_user(&secinfo, (void __user *)addp->secinfo,
+			   sizeof(secinfo))) {
+		kref_put(&encl->refcount, sgx_encl_release);
+		return -EFAULT;
+	}
+
+	data_page = alloc_page(GFP_HIGHUSER);
+	if (!data_page) {
+		kref_put(&encl->refcount, sgx_encl_release);
+		return -ENOMEM;
+	}
+
+	data = kmap(data_page);
+
+	ret = copy_from_user((void *)data, (void __user *)addp->src, PAGE_SIZE);
+	if (ret)
+		goto out;
+
+	ret = sgx_encl_add_page(encl, addp->addr, data, &secinfo, addp->mrmask);
+	if (ret)
+		goto out;
+
+out:
+	kref_put(&encl->refcount, sgx_encl_release);
+	kunmap(data_page);
+	__free_page(data_page);
+	return ret;
+}
+
+/**
+ * sgx_ioc_enclave_init - handler for %SGX_IOC_ENCLAVE_INIT
+ *
+ * @filep:	open file to /dev/sgx
+ * @cmd:	the command value
+ * @arg:	pointer to an &sgx_enclave_init instance
+ *
+ * Flushes the remaining enqueued EADD operations and performs EINIT. Does not
+ * allow the EINITTOKENKEY attribute for an enclave.
+ *
+ * Return:
+ *   0 on success,
+ *   SGX error code on EINIT failure,
+ *   -errno otherwise
+ */
+static long sgx_ioc_enclave_init(struct file *filep, unsigned int cmd,
+				 unsigned long arg)
+{
+	struct sgx_enclave_init *initp = (struct sgx_enclave_init *)arg;
+	struct sgx_sigstruct *sigstruct;
+	struct sgx_einittoken *einittoken;
+	struct sgx_encl *encl;
+	struct page *initp_page;
+	int ret;
+
+	initp_page = alloc_page(GFP_HIGHUSER);
+	if (!initp_page)
+		return -ENOMEM;
+
+	sigstruct = kmap(initp_page);
+	einittoken = (struct sgx_einittoken *)
+		((unsigned long)sigstruct + PAGE_SIZE / 2);
+	memset(einittoken, 0, sizeof(*einittoken));
+
+	ret = copy_from_user(sigstruct, (void __user *)initp->sigstruct,
+			     sizeof(*sigstruct));
+	if (ret)
+		goto out;
+	if (sigstruct->body.attributes & SGX_ATTR_EINITTOKENKEY) {
+		ret = EINVAL;
+		goto out;
+	}
+
+	ret = sgx_encl_get(initp->addr, &encl);
+	if (ret)
+		goto out;
+
+	ret = sgx_encl_init(encl, sigstruct, einittoken);
+
+	kref_put(&encl->refcount, sgx_encl_release);
+
+out:
+	kunmap(initp_page);
+	__free_page(initp_page);
+	return ret;
+}
+
+typedef long (*sgx_ioc_t)(struct file *filep, unsigned int cmd,
+			  unsigned long arg);
+
+long sgx_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
+{
+	char data[256];
+	sgx_ioc_t handler = NULL;
+	long ret;
+
+	switch (cmd) {
+	case SGX_IOC_ENCLAVE_CREATE:
+		handler = sgx_ioc_enclave_create;
+		break;
+	case SGX_IOC_ENCLAVE_ADD_PAGE:
+		handler = sgx_ioc_enclave_add_page;
+		break;
+	case SGX_IOC_ENCLAVE_INIT:
+		handler = sgx_ioc_enclave_init;
+		break;
+	default:
+		return -ENOIOCTLCMD;
+	}
+
+	if (copy_from_user(data, (void __user *)arg, _IOC_SIZE(cmd)))
+		return -EFAULT;
+
+	ret = handler(filep, cmd, (unsigned long)((void *)data));
+	if (!ret && (cmd & IOC_OUT)) {
+		if (copy_to_user((void __user *)arg, data, _IOC_SIZE(cmd)))
+			return -EFAULT;
+	}
+	if (IS_ENCLS_FAULT(ret))
+		return -EFAULT;
+	return ret;
+}
diff --git a/drivers/platform/x86/intel_sgx/sgx_main.c b/drivers/platform/x86/intel_sgx/sgx_main.c
new file mode 100644
index 000000000000..7bf91d7a38cb
--- /dev/null
+++ b/drivers/platform/x86/intel_sgx/sgx_main.c
@@ -0,0 +1,298 @@
+// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
+// Copyright(c) 2016-18 Intel Corporation.
+
+#include <linux/acpi.h>
+#include <linux/cdev.h>
+#include <linux/platform_device.h>
+#include <linux/suspend.h>
+#include "sgx.h"
+
+#define DRV_DESCRIPTION "Intel SGX Driver"
+
+MODULE_DESCRIPTION("Intel SGX Driver");
+MODULE_AUTHOR("Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>");
+MODULE_LICENSE("Dual BSD/GPL");
+
+/*
+ * Global data.
+ */
+
+struct workqueue_struct *sgx_add_page_wq;
+u64 sgx_encl_size_max_32;
+u64 sgx_encl_size_max_64;
+u64 sgx_xfrm_mask = 0x3;
+u32 sgx_misc_reserved;
+u32 sgx_xsave_size_tbl[64];
+
+#ifdef CONFIG_COMPAT
+long sgx_compat_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
+{
+	return sgx_ioctl(filep, cmd, arg);
+}
+#endif
+
+static int sgx_mmap(struct file *file, struct vm_area_struct *vma)
+{
+	vma->vm_ops = &sgx_vm_ops;
+	vma->vm_flags |= VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP | VM_IO |
+			 VM_DONTCOPY;
+
+	return 0;
+}
+
+static unsigned long sgx_get_unmapped_area(struct file *file,
+					   unsigned long addr,
+					   unsigned long len,
+					   unsigned long pgoff,
+					   unsigned long flags)
+{
+	if (len < 2 * PAGE_SIZE || (len & (len - 1)))
+		return -EINVAL;
+
+	/* On 64-bit architecture, allow mmap() to exceed 32-bit encl
+	 * limit only if the task is not running in 32-bit compatibility
+	 * mode.
+	 */
+	if (len > sgx_encl_size_max_32)
+#ifdef CONFIG_X86_64
+		if (test_thread_flag(TIF_ADDR32))
+			return -EINVAL;
+#else
+		return -EINVAL;
+#endif
+
+#ifdef CONFIG_X86_64
+	if (len > sgx_encl_size_max_64)
+		return -EINVAL;
+#endif
+
+	addr = current->mm->get_unmapped_area(file, addr, 2 * len, pgoff,
+					      flags);
+	if (IS_ERR_VALUE(addr))
+		return addr;
+
+	addr = (addr + (len - 1)) & ~(len - 1);
+
+	return addr;
+}
+
+static const struct file_operations sgx_fops = {
+	.owner			= THIS_MODULE,
+	.unlocked_ioctl		= sgx_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl		= sgx_compat_ioctl,
+#endif
+	.mmap			= sgx_mmap,
+	.get_unmapped_area	= sgx_get_unmapped_area,
+};
+
+static int sgx_pm_suspend(struct device *dev)
+{
+	struct sgx_encl_page *encl_page;
+	struct sgx_epc_page *epc_page;
+	struct sgx_encl *encl;
+
+	list_for_each_entry(epc_page, &sgx_active_page_list, list) {
+		encl_page = container_of(epc_page->impl, struct sgx_encl_page,
+					 impl);
+		encl = encl_page->encl;
+		sgx_invalidate(encl, false);
+		encl->flags |= SGX_ENCL_SUSPEND;
+		flush_work(&encl->add_page_work);
+	}
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(sgx_drv_pm, sgx_pm_suspend, NULL);
+
+static struct bus_type sgx_bus_type = {
+	.name	= "sgx",
+};
+
+struct sgx_context {
+	struct device dev;
+	struct cdev cdev;
+};
+
+static dev_t sgx_devt;
+
+static void sgx_dev_release(struct device *dev)
+{
+	struct sgx_context *ctx = container_of(dev, struct sgx_context, dev);
+
+	kfree(ctx);
+}
+
+static struct sgx_context *sgx_ctx_alloc(struct device *parent)
+{
+	struct sgx_context *ctx;
+
+	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return ERR_PTR(-ENOMEM);
+
+	device_initialize(&ctx->dev);
+
+	ctx->dev.bus = &sgx_bus_type;
+	ctx->dev.parent = parent;
+	ctx->dev.devt = MKDEV(MAJOR(sgx_devt), 0);
+	ctx->dev.release = sgx_dev_release;
+
+	dev_set_name(&ctx->dev, "sgx");
+
+	cdev_init(&ctx->cdev, &sgx_fops);
+	ctx->cdev.owner = THIS_MODULE;
+
+	dev_set_drvdata(parent, ctx);
+
+	return ctx;
+}
+
+static struct sgx_context *sgxm_ctx_alloc(struct device *parent)
+{
+	struct sgx_context *ctx;
+	int rc;
+
+	ctx = sgx_ctx_alloc(parent);
+	if (IS_ERR(ctx))
+		return ctx;
+
+	rc = devm_add_action_or_reset(parent, (void (*)(void *))put_device,
+				      &ctx->dev);
+	if (rc) {
+		kfree(ctx);
+		return ERR_PTR(rc);
+	}
+
+	return ctx;
+}
+
+static int sgx_dev_init(struct device *parent)
+{
+	struct sgx_context *sgx_dev;
+	unsigned int eax;
+	unsigned int ebx;
+	unsigned int ecx;
+	unsigned int edx;
+	int ret;
+	int i;
+
+	sgx_dev = sgxm_ctx_alloc(parent);
+
+	cpuid_count(SGX_CPUID, SGX_CPUID_CAPABILITIES, &eax, &ebx, &ecx, &edx);
+	/* Only allow misc bits supported by the driver. */
+	sgx_misc_reserved = ~ebx | SGX_MISC_RESERVED_MASK;
+#ifdef CONFIG_X86_64
+	sgx_encl_size_max_64 = 1ULL << ((edx >> 8) & 0xFF);
+#endif
+	sgx_encl_size_max_32 = 1ULL << (edx & 0xFF);
+
+	if (boot_cpu_has(X86_FEATURE_OSXSAVE)) {
+		cpuid_count(SGX_CPUID, SGX_CPUID_ATTRIBUTES, &eax, &ebx, &ecx,
+			    &edx);
+		sgx_xfrm_mask = (((u64)edx) << 32) + (u64)ecx;
+
+		for (i = 2; i < 64; i++) {
+			cpuid_count(0x0D, i, &eax, &ebx, &ecx, &edx);
+			if ((1 << i) & sgx_xfrm_mask)
+				sgx_xsave_size_tbl[i] = eax + ebx;
+		}
+	}
+
+	sgx_add_page_wq = alloc_workqueue("intel_sgx-add-page-wq",
+					  WQ_UNBOUND | WQ_FREEZABLE, 1);
+	if (!sgx_add_page_wq)
+		return -ENOMEM;
+
+	ret = cdev_device_add(&sgx_dev->cdev, &sgx_dev->dev);
+	if (ret)
+		goto out_workqueue;
+
+	return 0;
+out_workqueue:
+	destroy_workqueue(sgx_add_page_wq);
+	return ret;
+}
+
+static int sgx_drv_probe(struct platform_device *pdev)
+{
+	if (!sgx_enabled || !sgx_lc_enabled)
+		return -ENODEV;
+
+	return sgx_dev_init(&pdev->dev);
+}
+
+static int sgx_drv_remove(struct platform_device *pdev)
+{
+	struct sgx_context *ctx = dev_get_drvdata(&pdev->dev);
+
+	cdev_device_del(&ctx->cdev, &ctx->dev);
+	destroy_workqueue(sgx_add_page_wq);
+
+	return 0;
+}
+
+#ifdef CONFIG_ACPI
+static struct acpi_device_id sgx_device_ids[] = {
+	{"INT0E0C", 0},
+	{"", 0},
+};
+MODULE_DEVICE_TABLE(acpi, sgx_device_ids);
+#endif
+
+static struct platform_driver sgx_drv = {
+	.probe = sgx_drv_probe,
+	.remove = sgx_drv_remove,
+	.driver = {
+		.name			= "intel_sgx",
+		.pm			= &sgx_drv_pm,
+		.acpi_match_table	= ACPI_PTR(sgx_device_ids),
+	},
+};
+
+static int __init sgx_drv_subsys_init(void)
+{
+	int ret;
+
+	ret = bus_register(&sgx_bus_type);
+	if (ret)
+		return ret;
+
+	ret = alloc_chrdev_region(&sgx_devt, 0, 1, "sgx");
+	if (ret < 0) {
+		bus_unregister(&sgx_bus_type);
+		return ret;
+	}
+
+	return 0;
+}
+
+static void sgx_drv_subsys_exit(void)
+{
+	bus_unregister(&sgx_bus_type);
+	unregister_chrdev_region(sgx_devt, 1);
+}
+
+static int __init sgx_drv_init(void)
+{
+	int ret;
+
+	ret = sgx_drv_subsys_init();
+	if (ret)
+		return ret;
+
+	ret = platform_driver_register(&sgx_drv);
+	if (ret)
+		sgx_drv_subsys_exit();
+
+	return ret;
+}
+module_init(sgx_drv_init);
+
+static void __exit sgx_drv_exit(void)
+{
+	platform_driver_unregister(&sgx_drv);
+	sgx_drv_subsys_exit();
+}
+module_exit(sgx_drv_exit);
diff --git a/drivers/platform/x86/intel_sgx/sgx_vma.c b/drivers/platform/x86/intel_sgx/sgx_vma.c
new file mode 100644
index 000000000000..cc0993b4fd40
--- /dev/null
+++ b/drivers/platform/x86/intel_sgx/sgx_vma.c
@@ -0,0 +1,58 @@
+// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
+// Copyright(c) 2016-18 Intel Corporation.
+
+#include <asm/mman.h>
+#include <linux/delay.h>
+#include <linux/file.h>
+#include <linux/hashtable.h>
+#include <linux/highmem.h>
+#include <linux/mm.h>
+#include <linux/ratelimit.h>
+#include <linux/slab.h>
+#include "sgx.h"
+
+static void sgx_vma_open(struct vm_area_struct *vma)
+{
+	struct sgx_encl *encl = vma->vm_private_data;
+
+	if (!encl)
+		return;
+
+	/* kref cannot underflow because ECREATE ioctl checks that there is only
+	 * one single VMA for the enclave before proceeding.
+	 */
+	kref_get(&encl->refcount);
+}
+
+static void sgx_vma_close(struct vm_area_struct *vma)
+{
+	struct sgx_encl *encl = vma->vm_private_data;
+
+	if (!encl)
+		return;
+
+	mutex_lock(&encl->lock);
+	sgx_invalidate(encl, true);
+	mutex_unlock(&encl->lock);
+	kref_put(&encl->refcount, sgx_encl_release);
+}
+
+static int sgx_vma_fault(struct vm_fault *vmf)
+{
+	unsigned long addr = (unsigned long)vmf->address;
+	struct vm_area_struct *vma = vmf->vma;
+	struct sgx_encl_page *entry;
+
+	entry = sgx_fault_page(vma, addr, 0);
+
+	if (!IS_ERR(entry) || PTR_ERR(entry) == -EBUSY)
+		return VM_FAULT_NOPAGE;
+	else
+		return VM_FAULT_SIGBUS;
+}
+
+const struct vm_operations_struct sgx_vm_ops = {
+	.close = sgx_vma_close,
+	.open = sgx_vma_open,
+	.fault = sgx_vma_fault,
+};
-- 
2.17.1

[PATCH v12 12/13] platform/x86: ptrace() support for the SGX driver

[Jarkko Sakkinen]

Implemented VMA callbacks in order to ptrace() debug enclaves. With
debug enclaves data can be read and write the memory word at a time by
using ENCLS(EDBGRD) and ENCLS(EDBGWR) leaf instructions.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/platform/x86/intel_sgx/sgx_encl.c |   2 +-
 drivers/platform/x86/intel_sgx/sgx_vma.c  | 116 ++++++++++++++++++++++
 2 files changed, 117 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/intel_sgx/sgx_encl.c b/drivers/platform/x86/intel_sgx/sgx_encl.c
index e3d7fa885489..20cd9ff7dd75 100644
--- a/drivers/platform/x86/intel_sgx/sgx_encl.c
+++ b/drivers/platform/x86/intel_sgx/sgx_encl.c
@@ -954,7 +954,7 @@ int sgx_encl_load_page(struct sgx_encl_page *encl_page,
 	ret = __eldu(&pginfo, epc_ptr, va_ptr + va_offset);
 	if (ret) {
 		sgx_err(encl, "ELDU returned %d\n", ret);
-		ret = -EFAULT;
+		ret = ENCLS_TO_ERR(ret);
 	}
 
 	kunmap_atomic((void *)(unsigned long)(pginfo.pcmd - pcmd_offset));
diff --git a/drivers/platform/x86/intel_sgx/sgx_vma.c b/drivers/platform/x86/intel_sgx/sgx_vma.c
index cc0993b4fd40..3a856e40c82c 100644
--- a/drivers/platform/x86/intel_sgx/sgx_vma.c
+++ b/drivers/platform/x86/intel_sgx/sgx_vma.c
@@ -51,8 +51,124 @@ static int sgx_vma_fault(struct vm_fault *vmf)
 		return VM_FAULT_SIGBUS;
 }
 
+static int sgx_edbgrd(struct sgx_encl *encl, struct sgx_encl_page *page,
+		      unsigned long addr, void *data)
+{
+	unsigned long offset;
+	void *ptr;
+	int ret;
+
+	offset = addr & ~PAGE_MASK;
+
+	if ((page->desc & SGX_ENCL_PAGE_TCS) &&
+            offset > offsetof(struct sgx_tcs, gslimit))
+		return -ECANCELED;
+
+	ptr = sgx_get_page(page->epc_page);
+	ret = __edbgrd((unsigned long)ptr + offset, data);
+	sgx_put_page(ptr);
+	if (ret) {
+		sgx_dbg(encl, "EDBGRD returned %d\n", ret);
+		return ENCLS_TO_ERR(ret);
+	}
+
+	return 0;
+}
+
+static int sgx_edbgwr(struct sgx_encl *encl, struct sgx_encl_page *page,
+		      unsigned long addr, void *data)
+{
+	unsigned long offset;
+	void *ptr;
+	int ret;
+
+	offset = addr & ~PAGE_MASK;
+
+	/* Writing anything else than flags will cause #GP */
+	if ((page->desc & SGX_ENCL_PAGE_TCS) &&
+	    offset != offsetof(struct sgx_tcs, flags))
+		return -ECANCELED;
+
+	ptr = sgx_get_page(page->epc_page);
+	ret = __edbgwr((unsigned long)ptr + offset, data);
+	sgx_put_page(ptr);
+	if (ret) {
+		sgx_dbg(encl, "EDBGWR returned %d\n", ret);
+		return ENCLS_TO_ERR(ret);
+	}
+
+	return 0;
+}
+
+static int sgx_vma_access(struct vm_area_struct *vma, unsigned long addr,
+			  void *buf, int len, int write)
+{
+	struct sgx_encl *encl = vma->vm_private_data;
+	struct sgx_encl_page *entry = NULL;
+	unsigned long align;
+	char data[sizeof(unsigned long)];
+	int offset;
+	int cnt;
+	int ret = 0;
+	int i;
+
+	/* If process was forked, VMA is still there but vm_private_data is set
+	 * to NULL.
+	 */
+	if (!encl)
+		return -EFAULT;
+
+	if (!(encl->flags & SGX_ENCL_DEBUG) ||
+	    !(encl->flags & SGX_ENCL_INITIALIZED) ||
+	    (encl->flags & SGX_ENCL_DEAD))
+		return -EFAULT;
+
+	for (i = 0; i < len; i += cnt) {
+		if (!entry || !((addr + i) & (PAGE_SIZE - 1))) {
+			if (entry)
+				entry->desc &= ~SGX_ENCL_PAGE_RESERVED;
+
+			entry = sgx_fault_page(vma, (addr + i) & PAGE_MASK,
+					       true);
+			if (IS_ERR(entry)) {
+				ret = PTR_ERR(entry);
+				entry = NULL;
+				break;
+			}
+		}
+
+		/* Locking is not needed because only immutable fields of the
+		 * page are accessed and page itself is reserved so that it
+		 * cannot be swapped out in the middle.
+		 */
+
+		align = ALIGN_DOWN(addr + i, sizeof(unsigned long));
+		offset = (addr + i) & (sizeof(unsigned long) - 1);
+		cnt = sizeof(unsigned long) - offset;
+		cnt = min(cnt, len - i);
+
+		ret = sgx_edbgrd(encl, entry, align, data);
+		if (ret)
+			break;
+		if (write) {
+			memcpy(data + offset, buf + i, cnt);
+			ret = sgx_edbgwr(encl, entry, align, data);
+			if (ret)
+				break;
+		}
+		else
+			memcpy(buf + i,data + offset, cnt);
+	}
+
+	if (entry)
+		entry->desc &= ~SGX_ENCL_PAGE_RESERVED;
+
+	return (ret < 0 && ret != -ECANCELED) ? ret : i;
+}
+
 const struct vm_operations_struct sgx_vm_ops = {
 	.close = sgx_vma_close,
 	.open = sgx_vma_open,
 	.fault = sgx_vma_fault,
+	.access = sgx_vma_access,
 };
-- 
2.17.1

[PATCH v12 13/13] x86/sgx: driver documentation

[Jarkko Sakkinen]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 9135 bytes --]

Documentation of the features of the Software Guard eXtensions used
by the Linux kernel and basic design choices for the core and driver
and functionality.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 Documentation/index.rst         |   1 +
 Documentation/x86/intel_sgx.rst | 185 ++++++++++++++++++++++++++++++++
 2 files changed, 186 insertions(+)
 create mode 100644 Documentation/x86/intel_sgx.rst

diff --git a/Documentation/index.rst b/Documentation/index.rst
index 3b99ab931d41..b9fb92928e8c 100644
--- a/Documentation/index.rst
+++ b/Documentation/index.rst
@@ -100,6 +100,7 @@ implementation.
    :maxdepth: 2
 
    sh/index
+   x86/index
 
 Korean translations
 -------------------
diff --git a/Documentation/x86/intel_sgx.rst b/Documentation/x86/intel_sgx.rst
new file mode 100644
index 000000000000..f6b7979c41f2
--- /dev/null
+++ b/Documentation/x86/intel_sgx.rst
@@ -0,0 +1,185 @@
+===================
+Intel(R) SGX driver
+===================
+
+Introduction
+============
+
+Intel(R) SGX is a set of CPU instructions that can be used by applications to
+set aside private regions of code and data. The code outside the enclave is
+disallowed to access the memory inside the enclave by the CPU access control.
+In a way you can think that SGX provides inverted sandbox. It protects the
+application from a malicious host.
+
+You can tell if your CPU supports SGX by looking into ``/proc/cpuinfo``:
+
+	``cat /proc/cpuinfo  | grep sgx``
+
+Overview of SGX
+===============
+
+SGX has a set of data structures to maintain information about the enclaves and
+their security properties. BIOS reserves a fixed size region of physical memory
+for these structures by setting Processor Reserved Memory Range Registers
+(PRMRR).
+
+This memory range is protected from outside access by the CPU and all the data
+coming in and out of the CPU package is encrypted by a key that is generated for
+each boot cycle.
+
+Enclaves execute in ring-3 in a special enclave submode using pages from the
+reserved memory range. A fixed logical address range for the enclave is reserved
+by ENCLS(ECREATE), a leaf instruction used to create enclaves. It is referred in
+the documentation commonly as the ELRANGE.
+
+Every memory access to the ELRANGE is asserted by the CPU. If the CPU is not
+executing in the enclave mode inside the enclave, #GP is raised. On the other
+hand enclave code can make memory accesses both inside and outside of the
+ELRANGE.
+
+Enclave can only execute code inside the ELRANGE. Instructions that may cause
+VMEXIT, IO instructions and instructions that require a privilege change are
+prohibited inside the enclave. Interrupts and exceptions always cause enclave
+to exit and jump to an address outside the enclave given when the enclave is
+entered by using the leaf instruction ENCLS(EENTER).
+
+Data types
+----------
+
+The protected memory range contains the following data:
+
+* **Enclave Page Cache (EPC):** protected pages
+* **Enclave Page Cache Map (EPCM):** a database that describes the state of the
+  pages and link them to an enclave.
+
+EPC has a number of different types of pages:
+
+* **SGX Enclave Control Structure (SECS)**: describes the global
+  properties of an enclave.
+* **Regular (REG):** code and data pages in the ELRANGE.
+* **Thread Control Structure (TCS):** pages that define entry points inside an
+  enclave. The enclave can only be entered through these entry points and each
+  can host a single hardware thread at a time.
+* **Version Array (VA)**: 64-bit version numbers for pages that have been
+  swapped outside the enclave. Each page contains 512 version numbers.
+
+Launch control
+--------------
+
+To launch an enclave, two structures must be provided for ENCLS(EINIT):
+
+1. **SIGSTRUCT:** signed measurement of the enclave binary.
+2. **EINITTOKEN:** a cryptographic token CMAC-signed with a AES256-key called
+   *launch key*, which is re-generated for each boot cycle.
+
+The CPU holds a SHA256 hash of a 3072-bit RSA public key inside
+IA32_SGXLEPUBKEYHASHn MSRs. Enclaves with a SIGSTRUCT that is signed with this
+key do not require a valid EINITTOKEN and can be authorized with special
+privileges. One of those privileges is ability to acquire the launch key with
+ENCLS(EGETKEY).
+
+**IA32_FEATURE_CONTROL[17]** is used by the BIOS configure whether
+IA32_SGXLEPUBKEYHASH MSRs are read-only or read-write before locking the
+feature control register and handing over control to the operating system.
+
+Enclave construction
+--------------------
+
+The construction is started by filling out the SECS that contains enclave
+address range, privileged attributes and measurement of TCS and REG pages (pages
+that will be mapped to the address range) among the other things. This structure
+is passed out to the ENCLS(ECREATE) together with a physical address of a page
+in EPC that will hold the SECS.
+
+The pages are added with ENCLS(EADD) and measured with ENCLS(EEXTEND) i.e.
+SHA256 hash MRENCLAVE residing in the SECS is extended with the page data.
+
+After all of the pages have been added, the enclave is initialized with
+ENCLS(EINIT). ENCLS(INIT) checks that the SIGSTRUCT is signed with the contained
+public key. If the given EINITTOKEN has the valid bit set, the CPU checks that
+the token is valid (CMAC'd with the launch key). If the token is not valid,
+the CPU will check whether the enclave is signed with a key matching to the
+IA32_SGXLEPUBKEYHASHn MSRs.
+
+Swapping pages
+--------------
+
+Enclave pages can be swapped out with ENCLS(EWB) to the unprotected memory. In
+addition to the EPC page, ENCLS(EWB) takes in a VA page and address for PCMD
+structure (Page Crypto MetaData) as input. The VA page will seal a version
+number for the page. PCMD is 128 byte structure that contains tracking
+information for the page, most importantly its MAC. With these structures the
+enclave is sealed and rollback protected while it resides in the unprotected
+memory.
+
+Before the page can be swapped out it must not have any active TLB references.
+ENCLS(EBLOCK) instruction moves a page to the *blocked* state, which means
+that no new TLB entries can be created to it by the hardware threads.
+
+After this a shootdown sequence is started with ENCLS(ETRACK), which sets an
+increased counter value to the entering hardware threads. ENCLS(EWB) will
+return SGX_NOT_TRACKED error while there are still threads with the earlier
+couner value because that means that there might be hardware thread inside
+the enclave with TLB entries to pages that are to be swapped.
+
+Kernel internals
+================
+
+Requirements
+------------
+
+Because SGX has an ever evolving and expanding feature set, it's possible for
+a BIOS or VMM to configure a system in such a way that not all CPUs are equal,
+e.g. where Launch Control is only enabled on a subset of CPUs.  Linux does
+*not* support such a heterogeneous system configuration, nor does it even
+attempt to play nice in the face of a misconfigured system.  With the exception
+of Launch Control's hash MSRs, which can vary per CPU, Linux assumes that all
+CPUs have a configuration that is identical to the boot CPU.
+
+
+Roles and responsibilities
+--------------------------
+
+SGX introduces system resources, e.g. EPC memory, that must be accessible to
+multiple entities, e.g. the native kernel driver (to expose SGX to userspace)
+and KVM (to expose SGX to VMs), ideally without introducing any dependencies
+between each SGX entity.  To that end, the kernel owns and manages the shared
+system resources, i.e. the EPC and Launch Control MSRs, and defines functions
+that provide appropriate access to the shared resources.  SGX support for
+user space and VMs is left to the SGX platform driver and KVM respectively.
+
+Launching enclaves
+------------------
+
+The current kernel implementation supports only unlocked MSRs i.e.
+FEATURE_CONTROL_SGX_LE_WR must be set. The launch is performed by setting the
+MSRs to the hash of the public key modulus of the enclave signer, which is one
+f the fields in the SIGSTRUCT.
+
+EPC management
+--------------
+
+Due to the unique requirements for swapping EPC pages, and because EPC pages
+(currently) do not have associated page structures, management of the EPC is
+not handled by the standard Linux swapper.  SGX directly handles swapping
+of EPC pages, including a kthread to initiate reclaim and a rudimentary LRU
+mechanism. The consumers of EPC pages, e.g. the SGX driver, are required to
+implement function callbacks that can be invoked by the kernel to age,
+swap, and/or forcefully reclaim a target EPC page.  In effect, the kernel
+controls what happens and when, while the consumers (driver, KVM, etc..) do
+the actual work.
+
+SGX uapi
+========
+
+.. kernel-doc:: drivers/platform/x86/intel_sgx/sgx_ioctl.c
+   :functions: sgx_ioc_enclave_create
+               sgx_ioc_enclave_add_page
+               sgx_ioc_enclave_init
+
+.. kernel-doc:: arch/x86/include/uapi/asm/sgx.h
+
+References
+==========
+
+* System Programming Manual: 39.1.4 Intel® SGX Launch Control Configuration
-- 
2.17.1

Re: [PATCH v12 03/13] x86/sgx: add SGX definitions to msr-index.h

[Dave Hansen]

On 07/03/2018 11:19 AM, Jarkko Sakkinen wrote:
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -479,6 +479,8 @@
>  #define FEATURE_CONTROL_LOCKED				(1<<0)
>  #define FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX	(1<<1)
>  #define FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX	(1<<2)
> +#define FEATURE_CONTROL_SGX_ENABLE                      (1<<18)
> +#define FEATURE_CONTROL_SGX_LE_WR			(1<<17)
>  #define FEATURE_CONTROL_LMCE				(1<<20)

Have you run checkpatch.pl on these?  There's a tabs-vs-spaces issue here.

Re: [PATCH v12 01/13] x86/sgx: updated MAINTAINERS

[Thomas Gleixner]

On Tue, 3 Jul 2018, Jarkko Sakkinen wrote:

> Updated the MAINTAINERS file to point to the Intel SGX subsystem tree.

Update not Updated. Documentation says:

  Describe your changes in imperative mood, e.g. "make xyzzy do frotz"
  instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy
  to do frotz", as if you are giving orders to the codebase to change
  its behaviour.

Applies to later patches as well.

Thanks,

	tglx

Re: [PATCH v12 02/13] x86/sgx: add SGX definitions to cpufeature

[Thomas Gleixner]

On Tue, 3 Jul 2018, Jarkko Sakkinen wrote:

Subject: x86/sgx: add SGX definitions to cpufeature

The prefix is bogus. Not everything you touch is SGX just because.

The proper prefix is 'x86/cpufeatures:' 
The following sentence starts with an uppercase letter.

And with the proper prefix the subject wants to be something like:

  x86/cpufeatures: Add SGX and SGX_LC CPU features

> From: Kai Huang <kai.huang@linux.intel.com>
> 
> Added X86_FEATURE_SGX and X86_FEATURE_SGX_LC definitions that define the

Add the ...

->						definitions that define

That's redundant.

> bits CPUID level 7 bits for determining whether the CPU supports SGX and

bits ... bits ?? 

> launch configuration other than the Intel proprietary key. If this the
> case, IA32_SGXLEPUBKEYHASHn MSRs (0 < n < 4) are available for defining the
> root key for enclaves.
> 
> Signed-off-by: Kai Huang <kai.huang@linux.intel.com>

Lacks your SOB

Thanks,

	tglx

Re: [PATCH v12 03/13] x86/sgx: add SGX definitions to msr-index.h

[Thomas Gleixner]

On Tue, 3 Jul 2018, Jarkko Sakkinen wrote:

x86/msr: Add ....

> From: Sean Christopherson <sean.j.christopherson@intel.com>
> 
> ENCLS and ENCLU are usable if and only if SGX_ENABLE is set and After
> SGX is activated the IA32_SGXLEPUBKEYHASHn MSRs are writable if
> SGX_LC_WR is set and the feature control is locked.
> 
> SGX related bits in IA32_FEATURE_CONTROL cannot be set before SGX is
> activated by the pre-boot firmware. SGX activation is triggered by
> setting bit 0 in the MSR 0x7a. Until SGX is activated, the LE hash MSRs
> are writable to allow pre-boot firmware to lock down the LE root key
> with a non-Intel value.
> 
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> Co-developed-by: Haim Cohen <haim.cohen@intel.com>

Lacks SOB

> ---
>  arch/x86/include/asm/msr-index.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> index fda2114197b3..a7355fb7344f 100644
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -479,6 +479,8 @@
>  #define FEATURE_CONTROL_LOCKED				(1<<0)
>  #define FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX	(1<<1)
>  #define FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX	(1<<2)
> +#define FEATURE_CONTROL_SGX_ENABLE                      (1<<18)

Tabs not spaces please. checkpatch.pl would have told you

> +#define FEATURE_CONTROL_SGX_LE_WR			(1<<17)

Thanks,

	tglx

Re: [PATCH v12 04/13] x86/cpufeatures: add Intel-defined SGX leaf CPUID_12_EAX

[Thomas Gleixner]

On Tue, 3 Jul 2018, Jarkko Sakkinen wrote:

> From: Sean Christopherson <sean.j.christopherson@intel.com>
> 
> CPUID_12_EAX is an Intel-defined feature bits leaf dedicated for SGX.
> There are currently four documented feature bits, with more expected in
> the not-too-distant future.
> 
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>

Lacks SOB

Thanks,

	tglx

Re: [PATCH v12 05/13] x86/sgx: architectural structures

[Dave Hansen]

On 07/03/2018 11:19 AM, Jarkko Sakkinen wrote:
> This commit adds arch/x86/include/asm/sgx_arch.h that contains definitions
> for data structures used by the SGX.
> 
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> Co-developed-by: Suresh Siddha <suresh.b.siddha@intel.com>
> ---
>  arch/x86/include/asm/sgx_arch.h | 183 ++++++++++++++++++++++++++++++++
>  1 file changed, 183 insertions(+)
>  create mode 100644 arch/x86/include/asm/sgx_arch.h
> 
> diff --git a/arch/x86/include/asm/sgx_arch.h b/arch/x86/include/asm/sgx_arch.h
> new file mode 100644
> index 000000000000..41a37eaa3f51
> --- /dev/null
> +++ b/arch/x86/include/asm/sgx_arch.h
> @@ -0,0 +1,183 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
> +// Copyright(c) 2016-17 Intel Corporation.

It would also be anice to have a description of what this contains.
"Don't put software data structures in here, or else"

> +#include <linux/types.h>
> +
> +#define SGX_CPUID 0x12

Shouldn't you be introducing this earlier and using it here?

+	/* Intel SGX features: level 0x00000012 */
+	if (c->cpuid_level >= 0x00000012) {
+		cpuid(0x00000012, &eax, &ebx, &ecx, &edx);
+
+		c->x86_capability[CPUID_12_EAX] = eax;
+	}

> +enum sgx_cpuid {
> +	SGX_CPUID_CAPABILITIES	= 0,
> +	SGX_CPUID_ATTRIBUTES	= 1,
> +	SGX_CPUID_EPC_BANKS	= 2,
> +};
> +
> +#define SGX_SSA_GPRS_SIZE		182
> +#define SGX_SSA_MISC_EXINFO_SIZE	16
> +
> +enum sgx_misc {
> +	SGX_MISC_EXINFO		= 0x01,
> +};
> +
> +#define SGX_MISC_RESERVED_MASK 0xFFFFFFFFFFFFFFFEL
> +
> +enum sgx_attribute {
> +	SGX_ATTR_DEBUG		= 0x02,
> +	SGX_ATTR_MODE64BIT	= 0x04,
> +	SGX_ATTR_PROVISIONKEY	= 0x10,
> +	SGX_ATTR_EINITTOKENKEY	= 0x20,
> +};
> +
> +#define SGX_ATTR_RESERVED_MASK 0xFFFFFFFFFFFFFFC9L
> +
> +#define SGX_SECS_RESERVED1_SIZE 24
> +#define SGX_SECS_RESERVED2_SIZE 32
> +#define SGX_SECS_RESERVED3_SIZE 96
> +#define SGX_SECS_RESERVED4_SIZE 3836
> +
> +struct sgx_secs {
> +	uint64_t size;
> +	uint64_t base;
> +	uint32_t ssaframesize;
> +	uint32_t miscselect;
> +	uint8_t reserved1[SGX_SECS_RESERVED1_SIZE];
> +	uint64_t attributes;
> +	uint64_t xfrm;
> +	uint32_t mrenclave[8];
> +	uint8_t reserved2[SGX_SECS_RESERVED2_SIZE];
> +	uint32_t mrsigner[8];
> +	uint8_t	reserved3[SGX_SECS_RESERVED3_SIZE];
> +	uint16_t isvvprodid;
> +	uint16_t isvsvn;
> +	uint8_t reserved4[SGX_SECS_RESERVED4_SIZE];
> +} __packed __aligned(4096);

It would be nice to align these a _bit_, like:

> +	uint32_t mrsigner[8];
> +	uint8_t	 reserved3[SGX_SECS_RESERVED3_SIZE];
> +	uint16_t isvvprodid;
> +	uint16_t isvsvn;
> +	uint8_t  reserved4[SGX_SECS_RESERVED4_SIZE];

Is the SECS *defined* to be a single page, or can it be bigger?

> +enum sgx_tcs_flags {
> +	SGX_TCS_DBGOPTIN	= 0x01, /* cleared on EADD */
> +};
> +
> +#define SGX_TCS_RESERVED_MASK 0xFFFFFFFFFFFFFFFEL
> +
> +struct sgx_tcs {
> +	uint64_t state;
> +	uint64_t flags;
> +	uint64_t ossa;
> +	uint32_t cssa;
> +	uint32_t nssa;
> +	uint64_t oentry;
> +	uint64_t aep;
> +	uint64_t ofsbase;
> +	uint64_t ogsbase;
> +	uint32_t fslimit;
> +	uint32_t gslimit;
> +	uint64_t reserved[503];
> +} __packed __aligned(4096);

It's interesting that you defined a reserved[] array here with a
hard-coded number and a (nicely-aligned, I'll point out) uint64_t, but
the sgx_secs was defined using #defined reserve sizes and a uint8_t.
Seems like we should just be consistent.  No?

Also, are these truly structures that need alignment?  Or are they just
the *format* of a page (which is obviously aligned)?  For instance, if
we copied the hardware structure into a temporary buffer, we wouldn't
need it aligned.

I sometimes prefer asking for an instance of a variable to be aligned
rather than the type for things like this.

> +struct sgx_pageinfo {
> +	uint64_t linaddr;
> +	uint64_t srcpge;
> +	union {
> +		uint64_t secinfo;
> +		uint64_t pcmd;
> +	};
> +	uint64_t secs;
> +} __packed __aligned(32);

I see these were verbatim names taken from the SDM.  Could you also
please add some small comments about them?  "secs", for instance, is a
pretty common abbreviation for "seconds".  It would really help to have
this at _least_ say:

	uint64_t sgx_secs;

> +#define SGX_SECINFO_PERMISSION_MASK	0x0000000000000007L
> +#define SGX_SECINFO_PAGE_TYPE_MASK	0x000000000000FF00L
> +#define SGX_SECINFO_RESERVED_MASK	0xFFFFFFFFFFFF00F8L
> +
> +enum sgx_page_type {
> +	SGX_PAGE_TYPE_SECS	= 0x00,
> +	SGX_PAGE_TYPE_TCS	= 0x01,
> +	SGX_PAGE_TYPE_REG	= 0x02,
> +	SGX_PAGE_TYPE_VA	= 0x03,
> +	SGX_PAGE_TYPE_TRIM	= 0x04,
> +};
> +
> +enum sgx_secinfo_flags {
> +	SGX_SECINFO_R		= 0x01,
> +	SGX_SECINFO_W		= 0x02,
> +	SGX_SECINFO_X		= 0x04,
> +	SGX_SECINFO_SECS	= (SGX_PAGE_TYPE_SECS << 8),
> +	SGX_SECINFO_TCS		= (SGX_PAGE_TYPE_TCS << 8),
> +	SGX_SECINFO_REG		= (SGX_PAGE_TYPE_REG << 8),
> +	SGX_SECINFO_VA          = (SGX_PAGE_TYPE_VA << 8),
> +	SGX_SECINFO_TRIM	= (SGX_PAGE_TYPE_TRIM << 8),
> +};
> +
> +struct sgx_secinfo {
> +	uint64_t flags;
> +	uint64_t reserved[7];
> +} __packed __aligned(64);
> +
> +struct sgx_pcmd {
> +	struct sgx_secinfo secinfo;
> +	uint64_t enclave_id;
> +	uint8_t reserved[40];
> +	uint8_t mac[16];
> +} __packed __aligned(128);

I don't see any sign of this alignment restriction in the SDM.

> +#define SGX_MODULUS_SIZE 384

This is #defined in a rather odd place.  Why here?  Also, please add a
unit.  SGX_MODULUS_SIZE_BYTES or SGX_MODULUS_BYTES is way better than _SIZE.

> +struct sgx_sigstruct_header {
> +	uint64_t header1[2];
> +	uint32_t vendor;
> +	uint32_t date;
> +	uint64_t header2[2];
> +	uint32_t swdefined;
> +	uint8_t reserved1[84];
> +} __packed;
> +
> +struct sgx_sigstruct_body {
> +	uint32_t miscselect;
> +	uint32_t miscmask;
> +	uint8_t reserved2[20];
> +	uint64_t attributes;
> +	uint64_t xfrm;
> +	uint8_t attributemask[16];

I've hit my limit on silly SDM names. :)

Please make these human-readable: "attribute_mask".  The divergence from
the SDM naming is justified at this point.

> +	uint8_t mrenclave[32];
> +	uint8_t reserved3[32];
> +	uint16_t isvprodid;
> +	uint16_t isvsvn;
> +} __packed;
> +
> +struct sgx_sigstruct {
> +	struct sgx_sigstruct_header header;
> +	uint8_t modulus[SGX_MODULUS_SIZE];
> +	uint32_t exponent;
> +	uint8_t signature[SGX_MODULUS_SIZE];
> +	struct sgx_sigstruct_body body;
> +	uint8_t reserved4[12];
> +	uint8_t q1[SGX_MODULUS_SIZE];
> +	uint8_t q2[SGX_MODULUS_SIZE];
> +} __packed __aligned(4096);

It's interesting that the SDM says "page aligned" in some places and
"4096-byte aligned" in others.  Oh well.

> +struct sgx_einittoken_payload {
> +	uint32_t valid;
> +	uint32_t reserved1[11];

... and back to different types being used for padding.  These really
need to at least be consistent.

> +	uint64_t attributes;
> +	uint64_t xfrm;
> +	uint8_t mrenclave[32];
> +	uint8_t reserved2[32];
> +	uint8_t mrsigner[32];
> +	uint8_t reserved3[32];
> +} __packed;
Just curious, why is this broken out into its own structure?  It doesn't
appear architectural.

> +struct sgx_einittoken {
> +	struct sgx_einittoken_payload payload;
> +	uint8_t cpusvnle[16];
> +	uint16_t isvprodidle;
> +	uint16_t isvsvnle;
> +	uint8_t reserved2[24];
> +	uint32_t maskedmiscselectle;
> +	uint64_t maskedattributesle;
> +	uint64_t maskedxfrmle;
> +	uint8_t keyid[32];
> +	uint8_t mac[16];
> +} __packed __aligned(512);
> +
> +#endif /* _ASM_X86_SGX_ARCH_H */
> 

Re: [PATCH v12 07/13] x86/sgx: data structures for tracking available EPC pages

[Dave Hansen]

On 07/03/2018 11:19 AM, Jarkko Sakkinen wrote:
> This commit adds a database of EPC banks for kernel to easily access the
> available EPC pages. On UMA architectures there is a singe bank of EPC
> pages. On NUMA architectures there is an EPC bank for each node.

Is this universally true?  What about Sub-NUMA-Clustering/Cluster-on-Die
systems?

Re: [PATCH v12 05/13] x86/sgx: architectural structures

[Thomas Gleixner]

On Tue, 3 Jul 2018, Jarkko Sakkinen wrote:

> This commit adds arch/x86/include/asm/sgx_arch.h that contains definitions

This is not a commit. Simply: Add .....

> for data structures used by the SGX.
> 
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> Co-developed-by: Suresh Siddha <suresh.b.siddha@intel.com>

Documentaiton:

  A Co-Developed-by: states that the patch was also created by another developer
  along with the original author.  This is useful at times when multiple people
  work on a single patch.  Note, this person also needs to have a Signed-off-by:
  line in the patch as well.

Please fix all over the place.

> +
> +#define SGX_MISC_RESERVED_MASK 0xFFFFFFFFFFFFFFFEL

L ? ULL I'd say...

> +#define SGX_ATTR_RESERVED_MASK 0xFFFFFFFFFFFFFFC9L

Ditto

> +#define SGX_SECS_RESERVED1_SIZE 24
> +#define SGX_SECS_RESERVED2_SIZE 32
> +#define SGX_SECS_RESERVED3_SIZE 96
> +#define SGX_SECS_RESERVED4_SIZE 3836
> +
> +struct sgx_secs {
> +	uint64_t size;
> +	uint64_t base;
> +	uint32_t ssaframesize;
> +	uint32_t miscselect;
> +	uint8_t reserved1[SGX_SECS_RESERVED1_SIZE];
> +	uint64_t attributes;

Please make these tabular aligned for readility sake

> +	uint32_t	miscselect;
> +	uint8_t		reserved1[SGX_SECS_RESERVED1_SIZE];
> +	uint64_t	attributes;

Hmm? All over the place...

> +enum sgx_tcs_flags {
> +	SGX_TCS_DBGOPTIN	= 0x01, /* cleared on EADD */

Please do not use tail comments. Either put the comment above the define or
use kernel doc.

> +};
> +
> +#define SGX_TCS_RESERVED_MASK 0xFFFFFFFFFFFFFFFEL

See above.

> +#define SGX_SECINFO_PERMISSION_MASK	0x0000000000000007L
> +#define SGX_SECINFO_PAGE_TYPE_MASK	0x000000000000FF00L
> +#define SGX_SECINFO_RESERVED_MASK	0xFFFFFFFFFFFF00F8L

Ditto

Thanks,

	tglx

Re: [PATCH v12 06/13] x86/sgx: detect Intel SGX

[Thomas Gleixner]

On Tue, 3 Jul 2018, Jarkko Sakkinen wrote:
> @@ -0,0 +1,54 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
> +// Copyright(c) 2016-17 Intel Corporation.
> +
> +#include <asm/sgx.h>
> +#include <asm/sgx_pr.h>
> +#include <linux/freezer.h>
> +#include <linux/highmem.h>
> +#include <linux/kthread.h>
> +#include <linux/ratelimit.h>
> +#include <linux/sched/signal.h>
> +#include <linux/slab.h>

The common include ordering is

include linux/....

include asm/...

> +
> +bool sgx_enabled __ro_after_init;
> +EXPORT_SYMBOL(sgx_enabled);


> +bool sgx_lc_enabled __ro_after_init;
> +EXPORT_SYMBOL(sgx_lc_enabled);
> +
> +static __init bool sgx_is_enabled(bool *lc_enabled)
> +{
> +	unsigned long fc;
> +
> +	if (!boot_cpu_has(X86_FEATURE_SGX))
> +		return false;
> +
> +	if (!boot_cpu_has(X86_FEATURE_SGX1))
> +		return false;
> +
> +	rdmsrl(MSR_IA32_FEATURE_CONTROL, fc);
> +	if (!(fc & FEATURE_CONTROL_LOCKED)) {
> +		pr_info("IA32_FEATURE_CONTROL MSR is not locked\n");
> +		return false;
> +	}
> +
> +	if (!(fc & FEATURE_CONTROL_SGX_ENABLE)) {
> +		pr_info("disabled by the firmware\n");
> +		return false;
> +	}
> +
> +	if (!(fc & FEATURE_CONTROL_SGX_LE_WR)) {
> +		pr_info("IA32_SGXLEPUBKEYHASHn MSRs are not writable\n");
> +		return false;
> +	}
> +
> +	*lc_enabled = !!(fc & FEATURE_CONTROL_SGX_LE_WR);
> +	return true;
> +}
> +
> +static __init int sgx_init(void)
> +{
> +	sgx_enabled = sgx_is_enabled(&sgx_lc_enabled);

This is horrible, really. Both variables are global. So what the heck is
wrong with assigning them directly in the function? 

Thanks,

	tglx

Re: [PATCH v12 07/13] x86/sgx: data structures for tracking available EPC pages

[Thomas Gleixner]

On Tue, 3 Jul 2018, Jarkko Sakkinen wrote:

> SGX has a set of data structures to maintain information about the enclaves
> and their security properties. BIOS reserves a fixed size region of
> physical memory for these structures by setting Processor Reserved Memory
> Range Registers (PRMRR). This memory area is called Enclave Page Cache
> (EPC).
> 
> This commit adds a database of EPC banks for kernel to easily access the

Add a ....

> available EPC pages. On UMA architectures there is a singe bank of EPC

singe? Spell checkers exist for a reason.

> pages. On NUMA architectures there is an EPC bank for each node.

> diff --git a/arch/x86/include/asm/sgx.h b/arch/x86/include/asm/sgx.h
> index 2130e639ab49..77b2294fcfb0 100644
> --- a/arch/x86/include/asm/sgx.h
> +++ b/arch/x86/include/asm/sgx.h
> @@ -4,9 +4,39 @@
>  #ifndef _ASM_X86_SGX_H
>  #define _ASM_X86_SGX_H
>  
> +#include <asm/sgx_arch.h>
> +#include <asm/asm.h>

asm includes go below.

> +#include <linux/bitops.h>
> +#include <linux/err.h>
> +#include <linux/rwsem.h>
>  #include <linux/types.h>
>  
> +#define SGX_MAX_EPC_BANKS 8
> +
> +#define SGX_EPC_BANK(epc_page) \
> +	(&sgx_epc_banks[(unsigned long)(epc_page->desc) & ~PAGE_MASK])
> +#define SGX_EPC_PFN(epc_page) PFN_DOWN((unsigned long)(epc_page->desc))
> +#define SGX_EPC_ADDR(epc_page) ((unsigned long)(epc_page->desc) & PAGE_MASK)

And these need to be macros because MACROS_ARE_SO_PROMINENT, while inline
functions are so boring and type safe.

> +static atomic_t sgx_nr_free_pages = ATOMIC_INIT(0);
> +static struct sgx_epc_bank sgx_epc_banks[SGX_MAX_EPC_BANKS];
> +static int sgx_nr_epc_banks;
> +
> +/**
> + * sgx_get_page - pin an EPC page

Description starts with an uppercase letter.

> + * @page:	an EPC page

That's not an EPC page. It's a pointer to an EPC page. Please make these
comments useful and do not add them just that they exist.

> + *
> + * Return: a pointer to the pinned EPC page
> + */
> +void *sgx_get_page(struct sgx_epc_page *page)
> +{
> +	struct sgx_epc_bank *bank = SGX_EPC_BANK(page);
> +
> +	if (IS_ENABLED(CONFIG_X86_64))

And this is needed because the SGX config switch has:

       depends on X86_64 && CPU_SUP_INTEL

So what is this for and why do you need the 32bit implementation at all?

> +		return (void *)(bank->va + SGX_EPC_ADDR(page) - bank->pa);
> +
> +	return kmap_atomic_pfn(SGX_EPC_PFN(page));
> +}
> +EXPORT_SYMBOL(sgx_get_page);
> +
> +/**
> + * sgx_put_page - unpin an EPC page
> + * @ptr:	a pointer to the pinned EPC page
> + */
> +void sgx_put_page(void *ptr)
> +{
> +	if (IS_ENABLED(CONFIG_X86_64))
> +		return;

Ditto

>
> +	kunmap_atomic(ptr);
> +}
> +EXPORT_SYMBOL(sgx_put_page);
> +
> +static __init int sgx_init_epc_bank(unsigned long addr, unsigned long size,
> +				    unsigned long index,
> +				    struct sgx_epc_bank *bank)
> +{
> +	unsigned long nr_pages = size >> PAGE_SHIFT;
> +	unsigned long i;
> +	void *va;
> +
> +	if (IS_ENABLED(CONFIG_X86_64)) {

And more.

> +		va = ioremap_cache(addr, size);
> +		if (!va)
> +			return -ENOMEM;
> +	}
> +
> +	bank->pages_data = kzalloc(nr_pages * sizeof(struct sgx_epc_page),
> +				   GFP_KERNEL);
> +	if (!bank->pages_data) {
> +		if (IS_ENABLED(CONFIG_X86_64))
> +			iounmap(va);
> +
> +		return -ENOMEM;
> +	}

  	if (!bank->pages_data)
		goto out_iomap;

> +
> +	bank->pages = kzalloc(nr_pages * sizeof(struct sgx_epc_page *),
> +			      GFP_KERNEL);
> +	if (!bank->pages) {
> +		if (IS_ENABLED(CONFIG_X86_64))
> +			iounmap(va);
> +		kfree(bank->pages_data);
> +		bank->pages_data = NULL;
> +		return -ENOMEM;
> +	}

  	if (!bank->pages)
		goto out_pdata;

> +	for (i = 0; i < nr_pages; i++) {
> +		bank->pages[i] = &bank->pages_data[i];
> +		bank->pages[i]->desc = (addr + (i << PAGE_SHIFT)) | index;
> +	}
> +
> +	bank->pa = addr;
> +	bank->size = size;
> +	if (IS_ENABLED(CONFIG_X86_64))
> +		bank->va = (unsigned long)va;

Why is bank->va unsigned long and not a void *? That's spare the type casts.

> +
> +	atomic_set(&bank->free_cnt, nr_pages);
> +	init_rwsem(&bank->lock);
> +	atomic_add(nr_pages, &sgx_nr_free_pages);
> +	return 0;

out_iomap:
	iounmap(va);
out_pdata:
	kfree(bank->pages_data);
	bank->pages_data = NULL;
	return -ENOMEM;


> +static __init int sgx_page_cache_init(void)
> +{
> +	unsigned long size;
> +	unsigned int eax;
> +	unsigned int ebx;
> +	unsigned int ecx;
> +	unsigned int edx;
> +	unsigned long pa;
> +	int i;
> +	int ret;

Please aggregate the declarations of the same type. No point in wasting all
the screen space. And please use 'u32' for ea-dx

> +
> +	for (i = 0; i < SGX_MAX_EPC_BANKS; i++) {
> +		cpuid_count(SGX_CPUID, i + SGX_CPUID_EPC_BANKS, &eax, &ebx,
> +			    &ecx, &edx);
> +		if (!(eax & 0xf))
> +			break;
> +
> +		pa = ((u64)(ebx & 0xfffff) << 32) + (u64)(eax & 0xfffff000);
> +		size = ((u64)(edx & 0xfffff) << 32) + (u64)(ecx & 0xfffff000);

These magic constants should be 'U', using uppercase 'F' and defines. Plus
this wants a comment how pa and size are cobbled together from the cpuid
regs.

> +
> +		pr_info("EPC bank 0x%lx-0x%lx\n", pa, pa + size);

	size - 1 because the bank does not reach into the next page.

> +		ret = sgx_init_epc_bank(pa, size, i, &sgx_epc_banks[i]);
> +		if (ret) {
> +			sgx_page_cache_teardown();
> +			return ret;
> +		}
> +
> +		sgx_nr_epc_banks++;
> +	}
> +
> +	return 0;

This returns success even when the number of detected banks is zero.

> + }

This whole thing can be written readable.

{
	int i, cnt = SGX_CPUID_EPC_BANKS;

	for (i = 0; i < SGX_MAX_EPC_BANKS; i++, cnt++) {
	     	u32 eax, ebx, ecx, edx;
		unsigned long pa, size;
		int ret;

		cpuid_count(SGX_CPUID, cnt, &eax, &ebx, &ecx, &edx);
		if (!(eax & 0xF))
			break;

		pa = combine_regs(ebx, eax);
		size = combine_regs(edx, ecx);

		pr_info("EPC bank 0x%lx-0x%lx\n", pa, pa + size);

		ret = sgx_init_epc_bank(pa, size, i, &sgx_epc_banks[i]);
		if (ret) {
			sgx_page_cache_teardown();
			return ret;
		}
		sgx_nr_epc_banks++;
	}

	return sgx_nr_epc_banks ? : -ENODEV;
}

You get the idea...

Thanks,

	tglx

Re: [PATCH v12 08/13] x86/sgx: wrappers for ENCLS opcode leaf functions

[Thomas Gleixner]

On Tue, 3 Jul 2018, Jarkko Sakkinen wrote:

> This commit adds wrappers for Intel(R) SGX ENCLS opcode leaf functions

Add...

> except for ENCLS(EINIT). The ENCLS instruction invokes the privileged
> functions for managing (creation, initialization and swapping) and
> debugging enclaves.
>  
> +#define IS_ENCLS_FAULT(r) ((r) & 0xffff0000)
> +#define ENCLS_FAULT_VECTOR(r) ((r) >> 16)
> +
> +#define ENCLS_TO_ERR(r) (IS_ENCLS_FAULT(r) ? -EFAULT :		\
> +			(r) == SGX_UNMASKED_EVENT ? -EINTR :	\
> +			(r) == SGX_MAC_COMPARE_FAIL ? -EIO :	\
> +			(r) == SGX_ENTRYEPOCH_LOCKED ? -EBUSY : -EPERM)

Inlines please along with proper comments.

> +#define __encls_ret_N(rax, inputs...)			\
> +	({						\
> +	int ret;					\
> +	asm volatile(					\
> +	"1: .byte 0x0f, 0x01, 0xcf;\n\t"		\
> +	"2:\n"						\
> +	".section .fixup,\"ax\"\n"			\
> +	"3: shll $16,%%eax\n"				\

SHLL ??? _All_ the macro maze needs proper comments.

> +	"   jmp 2b\n"					\
> +	".previous\n"					\
> +	_ASM_EXTABLE_FAULT(1b, 3b)			\
> +	: "=a"(ret)					\
> +	: "a"(rax), inputs				\
> +	: "memory");					\
> +	ret;						\
> +	})

....

> +static inline int __emodt(struct sgx_secinfo *secinfo, void *epc)
> +{
> +	return __encls_ret_2(EMODT, secinfo, epc);
> +}
> +
>  #define SGX_MAX_EPC_BANKS 8
>  
>  #define SGX_EPC_BANK(epc_page) \
> @@ -39,4 +190,29 @@ extern bool sgx_lc_enabled;
>  void *sgx_get_page(struct sgx_epc_page *ptr);
>  void sgx_put_page(void *epc_page_ptr);

> +#define SGX_FN(name, params...)		\
> +{					\
> +	void *epc;			\
> +	int ret;			\
> +	epc = sgx_get_page(epc_page);	\
> +	ret = __##name(params);		\
> +	sgx_put_page(epc);		\

This whole get/put magic is totally pointless. The stuff is 64bit only, so
all it needs is the address, because 'put' is a noop on 64bit.

> +	return ret;			\
> +}
> +
> +#define BUILD_SGX_FN(fn, name)				\
> +static inline int fn(struct sgx_epc_page *epc_page)	\
> +	SGX_FN(name, epc)
> +BUILD_SGX_FN(sgx_eremove, eremove)
> +BUILD_SGX_FN(sgx_eblock, eblock)
> +BUILD_SGX_FN(sgx_etrack, etrack)
> +BUILD_SGX_FN(sgx_epa, epa)
> +
> +static inline int sgx_emodpr(struct sgx_secinfo *secinfo,
> +			     struct sgx_epc_page *epc_page)
> +	SGX_FN(emodpr, secinfo, epc)
> +static inline int sgx_emodt(struct sgx_secinfo *secinfo,
> +			    struct sgx_epc_page *epc_page)
> +	SGX_FN(emodt, secinfo, epc)

Bah this is really unreadable crap. What's so horribly wrong with writing
this simply as:

static inline int sgx_eremove(struct sgx_epc_page *epc_page)
{
	return __encls_ret_1(EREMOVE, epc_page_addr(epc_page));
}

static inline int sgx_emodt(struct sgx_secinfo *secinfo,
			    struct sgx_epc_page *epc_page)
{
	return __encls_ret_2(EREMOVE, secinfo, epc_page_addr(epc_page));
}

instead of all these completely pointless meta functions and build macro
maze around it.

Why? Because then every function which is actually used in code has a
proper prototype instead of nongrepable magic and a gazillion of wrappers.

Thanks,

	tglx

Re: [PATCH v12 07/13] x86/sgx: data structures for tracking available EPC pages

[Randy Dunlap]

On 07/03/18 12:46, Thomas Gleixner wrote:
> On Tue, 3 Jul 2018, Jarkko Sakkinen wrote:
> 
>> SGX has a set of data structures to maintain information about the enclaves
>> and their security properties. BIOS reserves a fixed size region of
>> physical memory for these structures by setting Processor Reserved Memory
>> Range Registers (PRMRR). This memory area is called Enclave Page Cache
>> (EPC).
>>
>> This commit adds a database of EPC banks for kernel to easily access the

what kind of database?  How does one query it?

>> diff --git a/arch/x86/include/asm/sgx.h b/arch/x86/include/asm/sgx.h
>> index 2130e639ab49..77b2294fcfb0 100644
>> --- a/arch/x86/include/asm/sgx.h
>> +++ b/arch/x86/include/asm/sgx.h


>> +/**
>> + * sgx_get_page - pin an EPC page
> 
> Description starts with an uppercase letter.

How would someone know that?  It's not documented anywhere.
I happen to disagree with it.


>> +static __init int sgx_page_cache_init(void)
>> +{
>> +	unsigned long size;
>> +	unsigned int eax;
>> +	unsigned int ebx;
>> +	unsigned int ecx;
>> +	unsigned int edx;
>> +	unsigned long pa;
>> +	int i;
>> +	int ret;
> 
> Please aggregate the declarations of the same type. No point in wasting all
> the screen space. And please use 'u32' for ea-dx
> 
>> +
>> +	for (i = 0; i < SGX_MAX_EPC_BANKS; i++) {
>> +		cpuid_count(SGX_CPUID, i + SGX_CPUID_EPC_BANKS, &eax, &ebx,
>> +			    &ecx, &edx);
>> +		if (!(eax & 0xf))
>> +			break;
>> +
>> +		pa = ((u64)(ebx & 0xfffff) << 32) + (u64)(eax & 0xfffff000);
>> +		size = ((u64)(edx & 0xfffff) << 32) + (u64)(ecx & 0xfffff000);
> 
> These magic constants should be 'U', using uppercase 'F' and defines. Plus
> this wants a comment how pa and size are cobbled together from the cpuid
> regs.

Why the uppercase 'F'?

thnx,
-- 
~Randy

Re: [PATCH v12 09/13] x86/sgx: EPC page allocation routines

[Thomas Gleixner]

On Tue, 3 Jul 2018, Jarkko Sakkinen wrote:
>  
> +#define SGX_NR_TO_SCAN	16
> +#define SGX_NR_LOW_PAGES 32
> +#define SGX_NR_HIGH_PAGES 64
> +
>  bool sgx_enabled __ro_after_init;
>  EXPORT_SYMBOL(sgx_enabled);
>  bool sgx_lc_enabled __ro_after_init;
>  EXPORT_SYMBOL(sgx_lc_enabled);
> +LIST_HEAD(sgx_active_page_list);
> +EXPORT_SYMBOL(sgx_active_page_list);
> +DEFINE_SPINLOCK(sgx_active_page_list_lock);
> +EXPORT_SYMBOL(sgx_active_page_list_lock);

Why is all of this exported. If done right then no call site has to fiddle
with the list and the lock at all.

>  static atomic_t sgx_nr_free_pages = ATOMIC_INIT(0);
>  static struct sgx_epc_bank sgx_epc_banks[SGX_MAX_EPC_BANKS];
>  static int sgx_nr_epc_banks;
> +static struct task_struct *ksgxswapd_tsk;
> +static DECLARE_WAIT_QUEUE_HEAD(ksgxswapd_waitq);
> +
> +static void sgx_swap_cluster(void)
> +{
> +	struct sgx_epc_page *cluster[SGX_NR_TO_SCAN + 1];
> +	struct sgx_epc_page *epc_page;
> +	int i;
> +	int j;

  	int i, j;

> +	memset(cluster, 0, sizeof(cluster));
> +
> +	for (i = 0, j = 0; i < SGX_NR_TO_SCAN; i++) {
> +		spin_lock(&sgx_active_page_list_lock);
> +		if (list_empty(&sgx_active_page_list)) {
> +			spin_unlock(&sgx_active_page_list_lock);
> +			break;
> +		}
> +		epc_page = list_first_entry(&sgx_active_page_list,
> +					    struct sgx_epc_page, list);
> +		if (!epc_page->impl->ops->get(epc_page)) {
> +			list_move_tail(&epc_page->list, &sgx_active_page_list);
> +			spin_unlock(&sgx_active_page_list_lock);
> +			continue;
> +		}
> +		list_del(&epc_page->list);
> +		spin_unlock(&sgx_active_page_list_lock);
> +
> +		if (epc_page->impl->ops->reclaim(epc_page)) {
> +			cluster[j++] = epc_page;
> +		} else {
> +			spin_lock(&sgx_active_page_list_lock);
> +			list_add_tail(&epc_page->list, &sgx_active_page_list);
> +			spin_unlock(&sgx_active_page_list_lock);
> +			epc_page->impl->ops->put(epc_page);
> +		}
> +	}
> +
> +	for (i = 0; cluster[i]; i++) {
> +		epc_page = cluster[i];
> +		epc_page->impl->ops->block(epc_page);
> +	}
> +
> +	for (i = 0; cluster[i]; i++) {
> +		epc_page = cluster[i];
> +		epc_page->impl->ops->write(epc_page);
> +		epc_page->impl->ops->put(epc_page);
> +		sgx_free_page(epc_page);
> +	}

Thanks a lot for commenting this piece of art thoughtfully. It's entirely
clear how all of this works now.

> +}
> +
> +static int ksgxswapd(void *p)
> +{
> +	set_freezable();
> +
> +	while (!kthread_should_stop()) {
> +		if (try_to_freeze())
> +			continue;
> +
> +		wait_event_freezable(ksgxswapd_waitq, kthread_should_stop() ||
> +				     atomic_read(&sgx_nr_free_pages) <
> +				     SGX_NR_HIGH_PAGES);
> +
> +		if (atomic_read(&sgx_nr_free_pages) < SGX_NR_HIGH_PAGES)
> +			sgx_swap_cluster();
> +	}
> +
> +	pr_info("%s: done\n", __func__);

Really useful. 

> +	return 0;
> +}
> +
> +static struct sgx_epc_page *sgx_try_alloc_page(struct sgx_epc_page_impl *impl)
> +{
> +	struct sgx_epc_bank *bank;
> +	struct sgx_epc_page *page = NULL;
> +	int i;
> +
> +	for (i = 0; i < sgx_nr_epc_banks; i++) {
> +		bank = &sgx_epc_banks[i];
> +
> +		down_write(&bank->lock);
> +
> +		if (atomic_read(&bank->free_cnt))

And these atomics are required becasue bank->lock protection is not
sufficient or what am I missing here?

> +			page = bank->pages[atomic_dec_return(&bank->free_cnt)];
> +
> +		up_write(&bank->lock);
> +
> +		if (page)
> +			break;
> +	}
> +
> +	if (page) {
> +		atomic_dec(&sgx_nr_free_pages);
> +		page->impl = impl;
> +	}
> +
> +	return page;
> +}
> +
> +/**
> + * sgx_alloc_page - allocate an EPC page
> + * @flags:	allocation flags
> + * @impl:	implementation for the struct sgx_epc_page
> + *
> + * Try to grab a page from the free EPC page list. If there is a free page
> + * available, it is returned to the caller. If called with SGX_ALLOC_ATOMIC,
> + * the function will return immediately if the list is empty. Otherwise, it
> + * will swap pages up until there is a free page available. Upon returning the
> + * low watermark is checked and ksgxswapd is waken up if we are below it.
> + *
> + * Return:
> + *   a &struct sgx_epc_page instace,
> + *   -ENOMEM if all pages are unreclaimable,
> + *   -EBUSY when called with SGX_ALLOC_ATOMIC and out of free pages
> + */
> +struct sgx_epc_page *sgx_alloc_page(struct sgx_epc_page_impl *impl,
> +				    unsigned int flags)
> +{
> +	struct sgx_epc_page *entry;
> +
> +	for ( ; ; ) {
> +		entry = sgx_try_alloc_page(impl);
> +		if (entry)
> +			break;
> +
> +		if (list_empty(&sgx_active_page_list))
> +			return ERR_PTR(-ENOMEM);
> +
> +		if (flags & SGX_ALLOC_ATOMIC) {
> +			entry = ERR_PTR(-EBUSY);
> +			break;
> +		}
> +
> +		if (signal_pending(current)) {
> +			entry = ERR_PTR(-ERESTARTSYS);
> +			break;
> +		}
> +
> +		sgx_swap_cluster();
> +		schedule();
> +	}
> +
> +	if (atomic_read(&sgx_nr_free_pages) < SGX_NR_LOW_PAGES)
> +		wake_up(&ksgxswapd_waitq);

What's the logic of SGX_NR_LOW_PAGES vs. SGX_NR_HIGH_PAGES? 

> +
> +	return entry;
> +}
> +EXPORT_SYMBOL(sgx_alloc_page);
> +
> +/**
> + * sgx_free_page - free an EPC page
> + *
> + * @page:	any EPC page
> + *
> + * Remove an EPC page and insert it back to the list of free pages.
> + *
> + * Return: SGX error code
> + */
> +int sgx_free_page(struct sgx_epc_page *page)
> +{
> +	struct sgx_epc_bank *bank = SGX_EPC_BANK(page);
> +	int ret;
> +
> +	ret = sgx_eremove(page);
> +	if (ret) {
> +		pr_debug("EREMOVE returned %d\n", ret);
> +		return ret;
> +	}
> +
> +	down_read(&bank->lock);
> +	bank->pages[atomic_inc_return(&bank->free_cnt) - 1] = page;
> +	atomic_inc(&sgx_nr_free_pages);
> +	up_read(&bank->lock);

I have hard time to see the benefit of this reader/writer semaphore
here. Both sides which fiddle with the bank pages are doing a simple
de/increment of free_cnt and a store resp. load. So what justifies the
overhead of a rwsem?

>  static __init int sgx_init_epc_bank(unsigned long addr, unsigned long size,
>  				    unsigned long index,
>  				    struct sgx_epc_bank *bank)
> @@ -114,6 +318,11 @@ static __init void sgx_page_cache_teardown(void)
>  		kfree(bank->pages);
>  		kfree(bank->pages_data);
>  	}
> +
> +	if (ksgxswapd_tsk) {
> +		kthread_stop(ksgxswapd_tsk);
> +		ksgxswapd_tsk = NULL;

This stops the thread _AFTER_ freeing all the bank memory. Is that actually
correct?

Thanks,

	tglx

Re: [PATCH v12 07/13] x86/sgx: data structures for tracking available EPC pages

[Thomas Gleixner]

On Tue, 3 Jul 2018, Randy Dunlap wrote:
> On 07/03/18 12:46, Thomas Gleixner wrote:
> > On Tue, 3 Jul 2018, Jarkko Sakkinen wrote:
> > 
> >> SGX has a set of data structures to maintain information about the enclaves
> >> and their security properties. BIOS reserves a fixed size region of
> >> physical memory for these structures by setting Processor Reserved Memory
> >> Range Registers (PRMRR). This memory area is called Enclave Page Cache
> >> (EPC).
> >>
> >> This commit adds a database of EPC banks for kernel to easily access the
> 
> what kind of database?  How does one query it?
> 
> >> diff --git a/arch/x86/include/asm/sgx.h b/arch/x86/include/asm/sgx.h
> >> index 2130e639ab49..77b2294fcfb0 100644
> >> --- a/arch/x86/include/asm/sgx.h
> >> +++ b/arch/x86/include/asm/sgx.h
> 
> 
> >> +/**
> >> + * sgx_get_page - pin an EPC page
> > 
> > Description starts with an uppercase letter.
> 
> How would someone know that?  It's not documented anywhere.

It's a short sentence decribing the function and sentences start usually
with uppercase letters, right?

> I happen to disagree with it.

Then we have to agree that we disagree.

> >> +static __init int sgx_page_cache_init(void)
> >> +{
> >> +	unsigned long size;
> >> +	unsigned int eax;
> >> +	unsigned int ebx;
> >> +	unsigned int ecx;
> >> +	unsigned int edx;
> >> +	unsigned long pa;
> >> +	int i;
> >> +	int ret;
> > 
> > Please aggregate the declarations of the same type. No point in wasting all
> > the screen space. And please use 'u32' for ea-dx
> > 
> >> +
> >> +	for (i = 0; i < SGX_MAX_EPC_BANKS; i++) {
> >> +		cpuid_count(SGX_CPUID, i + SGX_CPUID_EPC_BANKS, &eax, &ebx,
> >> +			    &ecx, &edx);
> >> +		if (!(eax & 0xf))
> >> +			break;
> >> +
> >> +		pa = ((u64)(ebx & 0xfffff) << 32) + (u64)(eax & 0xfffff000);
> >> +		size = ((u64)(edx & 0xfffff) << 32) + (u64)(ecx & 0xfffff000);
> > 
> > These magic constants should be 'U', using uppercase 'F' and defines. Plus
> > this wants a comment how pa and size are cobbled together from the cpuid
> > regs.
> 
> Why the uppercase 'F'?

Makes is more readable IMO, but I'm not religious about that.

Thanks,

	tglx

Re: [PATCH v12 05/13] x86/sgx: architectural structures

[Dave Hansen]

On 07/03/2018 11:19 AM, Jarkko Sakkinen wrote:
> +enum sgx_cpuid {
> +	SGX_CPUID_CAPABILITIES	= 0,
> +	SGX_CPUID_ATTRIBUTES	= 1,
> +	SGX_CPUID_EPC_BANKS	= 2,
> +};

I made a review comment about these last time.  I don't remember
receiving any kind of response from you on it, or seeing it addressed in
these patches in any kind of way.

I've also seen comments from Thomas that sound really familiar such as
the lack of comments on the paging code or the use of a semaphore to
protect things that don't sleep.  I made very, very similar comments on
the last posting.

Is there a reason that those comments were not addressed?

Re: [PATCH v12 09/13] x86/sgx: EPC page allocation routines

[Borislav Petkov]

On Tue, Jul 03, 2018 at 10:41:14PM +0200, Thomas Gleixner wrote:
> On Tue, 3 Jul 2018, Jarkko Sakkinen wrote:
> >  
> > +#define SGX_NR_TO_SCAN	16
> > +#define SGX_NR_LOW_PAGES 32
> > +#define SGX_NR_HIGH_PAGES 64
> > +
> >  bool sgx_enabled __ro_after_init;
> >  EXPORT_SYMBOL(sgx_enabled);
> >  bool sgx_lc_enabled __ro_after_init;
> >  EXPORT_SYMBOL(sgx_lc_enabled);
> > +LIST_HEAD(sgx_active_page_list);
> > +EXPORT_SYMBOL(sgx_active_page_list);
> > +DEFINE_SPINLOCK(sgx_active_page_list_lock);
> > +EXPORT_SYMBOL(sgx_active_page_list_lock);
> 
> Why is all of this exported. If done right then no call site has to fiddle
> with the list and the lock at all.

... and also, why are the other exports not EXPORT_SYMBOL_GPL?

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH v12 01/13] x86/sgx: updated MAINTAINERS

[Jarkko Sakkinen]

On Tue, Jul 03, 2018 at 08:41:25PM +0200, Thomas Gleixner wrote:
> On Tue, 3 Jul 2018, Jarkko Sakkinen wrote:
> 
> > Updated the MAINTAINERS file to point to the Intel SGX subsystem tree.
> 
> Update not Updated. Documentation says:
> 
>   Describe your changes in imperative mood, e.g. "make xyzzy do frotz"
>   instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy
>   to do frotz", as if you are giving orders to the codebase to change
>   its behaviour.
> 
> Applies to later patches as well.
> 
> Thanks,
> 
> 	tglx

Thanks I'll refine to follow this form.

/Jarkko

Re: [PATCH v12 02/13] x86/sgx: add SGX definitions to cpufeature

[Jarkko Sakkinen]

On Tue, Jul 03, 2018 at 08:48:54PM +0200, Thomas Gleixner wrote:
> On Tue, 3 Jul 2018, Jarkko Sakkinen wrote:
> 
> Subject: x86/sgx: add SGX definitions to cpufeature
> 
> The prefix is bogus. Not everything you touch is SGX just because.
> 
> The proper prefix is 'x86/cpufeatures:' 
> The following sentence starts with an uppercase letter.
> 
> And with the proper prefix the subject wants to be something like:
> 
>   x86/cpufeatures: Add SGX and SGX_LC CPU features
> 
> > From: Kai Huang <kai.huang@linux.intel.com>
> > 
> > Added X86_FEATURE_SGX and X86_FEATURE_SGX_LC definitions that define the
> 
> Add the ...
> 
> ->						definitions that define
> 
> That's redundant.
> 
> > bits CPUID level 7 bits for determining whether the CPU supports SGX and
> 
> bits ... bits ?? 
> 
> > launch configuration other than the Intel proprietary key. If this the
> > case, IA32_SGXLEPUBKEYHASHn MSRs (0 < n < 4) are available for defining the
> > root key for enclaves.
> > 
> > Signed-off-by: Kai Huang <kai.huang@linux.intel.com>
> 
> Lacks your SOB
> 
> Thanks,
> 
> 	tglx

Thanks, will refine!

/Jarkko

Re: [PATCH v12 03/13] x86/sgx: add SGX definitions to msr-index.h

[Jarkko Sakkinen]

On Tue, Jul 03, 2018 at 11:31:59AM -0700, Dave Hansen wrote:
> On 07/03/2018 11:19 AM, Jarkko Sakkinen wrote:
> > --- a/arch/x86/include/asm/msr-index.h
> > +++ b/arch/x86/include/asm/msr-index.h
> > @@ -479,6 +479,8 @@
> >  #define FEATURE_CONTROL_LOCKED				(1<<0)
> >  #define FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX	(1<<1)
> >  #define FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX	(1<<2)
> > +#define FEATURE_CONTROL_SGX_ENABLE                      (1<<18)
> > +#define FEATURE_CONTROL_SGX_LE_WR			(1<<17)
> >  #define FEATURE_CONTROL_LMCE				(1<<20)
> 
> Have you run checkpatch.pl on these?  There's a tabs-vs-spaces issue here.

Thought I did. Do not know how that has slipped there but I will fix
it. Thanks.

/Jarkko

Re: [PATCH v12 03/13] x86/sgx: add SGX definitions to msr-index.h

[Jarkko Sakkinen]

On Tue, Jul 03, 2018 at 08:51:23PM +0200, Thomas Gleixner wrote:
> On Tue, 3 Jul 2018, Jarkko Sakkinen wrote:
> 
> x86/msr: Add ....
> 
> > From: Sean Christopherson <sean.j.christopherson@intel.com>
> > 
> > ENCLS and ENCLU are usable if and only if SGX_ENABLE is set and After
> > SGX is activated the IA32_SGXLEPUBKEYHASHn MSRs are writable if
> > SGX_LC_WR is set and the feature control is locked.
> > 
> > SGX related bits in IA32_FEATURE_CONTROL cannot be set before SGX is
> > activated by the pre-boot firmware. SGX activation is triggered by
> > setting bit 0 in the MSR 0x7a. Until SGX is activated, the LE hash MSRs
> > are writable to allow pre-boot firmware to lock down the LE root key
> > with a non-Intel value.
> > 
> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> > Co-developed-by: Haim Cohen <haim.cohen@intel.com>
> 
> Lacks SOB
> 
> > ---
> >  arch/x86/include/asm/msr-index.h | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> > index fda2114197b3..a7355fb7344f 100644
> > --- a/arch/x86/include/asm/msr-index.h
> > +++ b/arch/x86/include/asm/msr-index.h
> > @@ -479,6 +479,8 @@
> >  #define FEATURE_CONTROL_LOCKED				(1<<0)
> >  #define FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX	(1<<1)
> >  #define FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX	(1<<2)
> > +#define FEATURE_CONTROL_SGX_ENABLE                      (1<<18)
> 
> Tabs not spaces please. checkpatch.pl would have told you
> 
> > +#define FEATURE_CONTROL_SGX_LE_WR			(1<<17)
> 
> Thanks,
> 
> 	tglx

Thanks, will refine!

/Jarkko

Re: [PATCH v12 04/13] x86/cpufeatures: add Intel-defined SGX leaf CPUID_12_EAX

[Jarkko Sakkinen]

On Tue, Jul 03, 2018 at 08:54:56PM +0200, Thomas Gleixner wrote:
> On Tue, 3 Jul 2018, Jarkko Sakkinen wrote:
> 
> > From: Sean Christopherson <sean.j.christopherson@intel.com>
> > 
> > CPUID_12_EAX is an Intel-defined feature bits leaf dedicated for SGX.
> > There are currently four documented feature bits, with more expected in
> > the not-too-distant future.
> > 
> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> 
> Lacks SOB
> 
> Thanks,
> 
> 	tglx

Thanks, will refine!

/Jarkko

Re: [PATCH v12 06/13] x86/sgx: detect Intel SGX

[Jarkko Sakkinen]

On Tue, Jul 03, 2018 at 09:09:52PM +0200, Thomas Gleixner wrote:
> On Tue, 3 Jul 2018, Jarkko Sakkinen wrote:
> > @@ -0,0 +1,54 @@
> > +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
> > +// Copyright(c) 2016-17 Intel Corporation.
> > +
> > +#include <asm/sgx.h>
> > +#include <asm/sgx_pr.h>
> > +#include <linux/freezer.h>
> > +#include <linux/highmem.h>
> > +#include <linux/kthread.h>
> > +#include <linux/ratelimit.h>
> > +#include <linux/sched/signal.h>
> > +#include <linux/slab.h>
> 
> The common include ordering is
> 
> include linux/....
> 
> include asm/...
> 
> > +
> > +bool sgx_enabled __ro_after_init;
> > +EXPORT_SYMBOL(sgx_enabled);
> 
> 
> > +bool sgx_lc_enabled __ro_after_init;
> > +EXPORT_SYMBOL(sgx_lc_enabled);
> > +
> > +static __init bool sgx_is_enabled(bool *lc_enabled)
> > +{
> > +	unsigned long fc;
> > +
> > +	if (!boot_cpu_has(X86_FEATURE_SGX))
> > +		return false;
> > +
> > +	if (!boot_cpu_has(X86_FEATURE_SGX1))
> > +		return false;
> > +
> > +	rdmsrl(MSR_IA32_FEATURE_CONTROL, fc);
> > +	if (!(fc & FEATURE_CONTROL_LOCKED)) {
> > +		pr_info("IA32_FEATURE_CONTROL MSR is not locked\n");
> > +		return false;
> > +	}
> > +
> > +	if (!(fc & FEATURE_CONTROL_SGX_ENABLE)) {
> > +		pr_info("disabled by the firmware\n");
> > +		return false;
> > +	}
> > +
> > +	if (!(fc & FEATURE_CONTROL_SGX_LE_WR)) {
> > +		pr_info("IA32_SGXLEPUBKEYHASHn MSRs are not writable\n");
> > +		return false;
> > +	}
> > +
> > +	*lc_enabled = !!(fc & FEATURE_CONTROL_SGX_LE_WR);
> > +	return true;
> > +}
> > +
> > +static __init int sgx_init(void)
> > +{
> > +	sgx_enabled = sgx_is_enabled(&sgx_lc_enabled);
> 
> This is horrible, really. Both variables are global. So what the heck is
> wrong with assigning them directly in the function? 

Fully agreed, does not make any sense.

> Thanks,
> 
> 	tglx

/Jarkko

Re: [PATCH v12 08/13] x86/sgx: wrappers for ENCLS opcode leaf functions

[Jarkko Sakkinen]

On Tue, Jul 03, 2018 at 10:16:12PM +0200, Thomas Gleixner wrote:
> On Tue, 3 Jul 2018, Jarkko Sakkinen wrote:
> 
> > This commit adds wrappers for Intel(R) SGX ENCLS opcode leaf functions
> 
> Add...
> 
> > except for ENCLS(EINIT). The ENCLS instruction invokes the privileged
> > functions for managing (creation, initialization and swapping) and
> > debugging enclaves.
> >  
> > +#define IS_ENCLS_FAULT(r) ((r) & 0xffff0000)
> > +#define ENCLS_FAULT_VECTOR(r) ((r) >> 16)
> > +
> > +#define ENCLS_TO_ERR(r) (IS_ENCLS_FAULT(r) ? -EFAULT :		\
> > +			(r) == SGX_UNMASKED_EVENT ? -EINTR :	\
> > +			(r) == SGX_MAC_COMPARE_FAIL ? -EIO :	\
> > +			(r) == SGX_ENTRYEPOCH_LOCKED ? -EBUSY : -EPERM)
> 
> Inlines please along with proper comments.
> 
> > +#define __encls_ret_N(rax, inputs...)			\
> > +	({						\
> > +	int ret;					\
> > +	asm volatile(					\
> > +	"1: .byte 0x0f, 0x01, 0xcf;\n\t"		\
> > +	"2:\n"						\
> > +	".section .fixup,\"ax\"\n"			\
> > +	"3: shll $16,%%eax\n"				\
> 
> SHLL ??? _All_ the macro maze needs proper comments.

Yeah, agreed.

> > +	"   jmp 2b\n"					\
> > +	".previous\n"					\
> > +	_ASM_EXTABLE_FAULT(1b, 3b)			\
> > +	: "=a"(ret)					\
> > +	: "a"(rax), inputs				\
> > +	: "memory");					\
> > +	ret;						\
> > +	})
> 
> ....
> 
> > +static inline int __emodt(struct sgx_secinfo *secinfo, void *epc)
> > +{
> > +	return __encls_ret_2(EMODT, secinfo, epc);
> > +}
> > +
> >  #define SGX_MAX_EPC_BANKS 8
> >  
> >  #define SGX_EPC_BANK(epc_page) \
> > @@ -39,4 +190,29 @@ extern bool sgx_lc_enabled;
> >  void *sgx_get_page(struct sgx_epc_page *ptr);
> >  void sgx_put_page(void *epc_page_ptr);
> 
> > +#define SGX_FN(name, params...)		\
> > +{					\
> > +	void *epc;			\
> > +	int ret;			\
> > +	epc = sgx_get_page(epc_page);	\
> > +	ret = __##name(params);		\
> > +	sgx_put_page(epc);		\
> 
> This whole get/put magic is totally pointless. The stuff is 64bit only, so
> all it needs is the address, because 'put' is a noop on 64bit.

I did some early/proto versions of SGX code with 32-bit environment. I
guess it is better to strip this stuff simply off.

> 
> > +	return ret;			\
> > +}
> > +
> > +#define BUILD_SGX_FN(fn, name)				\
> > +static inline int fn(struct sgx_epc_page *epc_page)	\
> > +	SGX_FN(name, epc)
> > +BUILD_SGX_FN(sgx_eremove, eremove)
> > +BUILD_SGX_FN(sgx_eblock, eblock)
> > +BUILD_SGX_FN(sgx_etrack, etrack)
> > +BUILD_SGX_FN(sgx_epa, epa)
> > +
> > +static inline int sgx_emodpr(struct sgx_secinfo *secinfo,
> > +			     struct sgx_epc_page *epc_page)
> > +	SGX_FN(emodpr, secinfo, epc)
> > +static inline int sgx_emodt(struct sgx_secinfo *secinfo,
> > +			    struct sgx_epc_page *epc_page)
> > +	SGX_FN(emodt, secinfo, epc)
> 
> Bah this is really unreadable crap. What's so horribly wrong with writing
> this simply as:
> 
> static inline int sgx_eremove(struct sgx_epc_page *epc_page)
> {
> 	return __encls_ret_1(EREMOVE, epc_page_addr(epc_page));
> }
> 
> static inline int sgx_emodt(struct sgx_secinfo *secinfo,
> 			    struct sgx_epc_page *epc_page)
> {
> 	return __encls_ret_2(EREMOVE, secinfo, epc_page_addr(epc_page));
> }
> 
> instead of all these completely pointless meta functions and build macro
> maze around it.
> 
> Why? Because then every function which is actually used in code has a
> proper prototype instead of nongrepable magic and a gazillion of wrappers.

I do agree with you as I would NAK this kind of code from TPM
because this is basically stuff that needs to be written only once
(maybe some minor fixes later on but anyway) and after that the
unrolled form is easier to maintain and debug.

I will do as you adviced.

> Thanks,
> 
> 	tglx

Thank you!

/Jarkko

Re: [PATCH v12 07/13] x86/sgx: data structures for tracking available EPC pages

[Jarkko Sakkinen]

On Tue, Jul 03, 2018 at 12:03:48PM -0700, Dave Hansen wrote:
> On 07/03/2018 11:19 AM, Jarkko Sakkinen wrote:
> > This commit adds a database of EPC banks for kernel to easily access the
> > available EPC pages. On UMA architectures there is a singe bank of EPC
> > pages. On NUMA architectures there is an EPC bank for each node.
> 
> Is this universally true?  What about Sub-NUMA-Clustering/Cluster-on-Die
> systems?

I have to say that I do not have a good answer right now but I will
study this area for the next version.

/Jarkko

Re: [PATCH v12 07/13] x86/sgx: data structures for tracking available EPC pages

[Jarkko Sakkinen]

On Tue, Jul 03, 2018 at 01:26:11PM -0700, Randy Dunlap wrote:
> On 07/03/18 12:46, Thomas Gleixner wrote:
> > On Tue, 3 Jul 2018, Jarkko Sakkinen wrote:
> > 
> >> SGX has a set of data structures to maintain information about the enclaves
> >> and their security properties. BIOS reserves a fixed size region of
> >> physical memory for these structures by setting Processor Reserved Memory
> >> Range Registers (PRMRR). This memory area is called Enclave Page Cache
> >> (EPC).
> >>
> >> This commit adds a database of EPC banks for kernel to easily access the
> 
> what kind of database?  How does one query it?

Have to think of a better word. Thanks!

/Jarkko

Re: [PATCH v12 05/13] x86/sgx: architectural structures

[Jarkko Sakkinen]

On Tue, Jul 03, 2018 at 12:02:31PM -0700, Dave Hansen wrote:
> On 07/03/2018 11:19 AM, Jarkko Sakkinen wrote:
> > This commit adds arch/x86/include/asm/sgx_arch.h that contains definitions
> > for data structures used by the SGX.
> > 
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > Co-developed-by: Suresh Siddha <suresh.b.siddha@intel.com>
> > ---
> >  arch/x86/include/asm/sgx_arch.h | 183 ++++++++++++++++++++++++++++++++
> >  1 file changed, 183 insertions(+)
> >  create mode 100644 arch/x86/include/asm/sgx_arch.h
> > 
> > diff --git a/arch/x86/include/asm/sgx_arch.h b/arch/x86/include/asm/sgx_arch.h
> > new file mode 100644
> > index 000000000000..41a37eaa3f51
> > --- /dev/null
> > +++ b/arch/x86/include/asm/sgx_arch.h
> > @@ -0,0 +1,183 @@
> > +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
> > +// Copyright(c) 2016-17 Intel Corporation.
> 
> It would also be anice to have a description of what this contains.
> "Don't put software data structures in here, or else"

Agreed.

> 
> > +#include <linux/types.h>
> > +
> > +#define SGX_CPUID 0x12
> 
> Shouldn't you be introducing this earlier and using it here?
> 
> +	/* Intel SGX features: level 0x00000012 */
> +	if (c->cpuid_level >= 0x00000012) {
> +		cpuid(0x00000012, &eax, &ebx, &ecx, &edx);
> +
> +		c->x86_capability[CPUID_12_EAX] = eax;
> +	}

Definitely.

> 
> > +enum sgx_cpuid {
> > +	SGX_CPUID_CAPABILITIES	= 0,
> > +	SGX_CPUID_ATTRIBUTES	= 1,
> > +	SGX_CPUID_EPC_BANKS	= 2,
> > +};
> > +
> > +#define SGX_SSA_GPRS_SIZE		182
> > +#define SGX_SSA_MISC_EXINFO_SIZE	16
> > +
> > +enum sgx_misc {
> > +	SGX_MISC_EXINFO		= 0x01,
> > +};
> > +
> > +#define SGX_MISC_RESERVED_MASK 0xFFFFFFFFFFFFFFFEL
> > +
> > +enum sgx_attribute {
> > +	SGX_ATTR_DEBUG		= 0x02,
> > +	SGX_ATTR_MODE64BIT	= 0x04,
> > +	SGX_ATTR_PROVISIONKEY	= 0x10,
> > +	SGX_ATTR_EINITTOKENKEY	= 0x20,
> > +};
> > +
> > +#define SGX_ATTR_RESERVED_MASK 0xFFFFFFFFFFFFFFC9L
> > +
> > +#define SGX_SECS_RESERVED1_SIZE 24
> > +#define SGX_SECS_RESERVED2_SIZE 32
> > +#define SGX_SECS_RESERVED3_SIZE 96
> > +#define SGX_SECS_RESERVED4_SIZE 3836
> > +
> > +struct sgx_secs {
> > +	uint64_t size;
> > +	uint64_t base;
> > +	uint32_t ssaframesize;
> > +	uint32_t miscselect;
> > +	uint8_t reserved1[SGX_SECS_RESERVED1_SIZE];
> > +	uint64_t attributes;
> > +	uint64_t xfrm;
> > +	uint32_t mrenclave[8];
> > +	uint8_t reserved2[SGX_SECS_RESERVED2_SIZE];
> > +	uint32_t mrsigner[8];
> > +	uint8_t	reserved3[SGX_SECS_RESERVED3_SIZE];
> > +	uint16_t isvvprodid;
> > +	uint16_t isvsvn;
> > +	uint8_t reserved4[SGX_SECS_RESERVED4_SIZE];
> > +} __packed __aligned(4096);
> 
> It would be nice to align these a _bit_, like:
> 
> > +	uint32_t mrsigner[8];
> > +	uint8_t	 reserved3[SGX_SECS_RESERVED3_SIZE];
> > +	uint16_t isvvprodid;
> > +	uint16_t isvsvn;
> > +	uint8_t  reserved4[SGX_SECS_RESERVED4_SIZE];

My personal taste is not to align struct fields (I do align enums
and such). Is this important? I would prefer to keep it how it is
but I'm not going to argue about it too much...

> Is the SECS *defined* to be a single page, or can it be bigger?

It is always a single EPC page.

> > +enum sgx_tcs_flags {
> > +	SGX_TCS_DBGOPTIN	= 0x01, /* cleared on EADD */
> > +};
> > +
> > +#define SGX_TCS_RESERVED_MASK 0xFFFFFFFFFFFFFFFEL
> > +
> > +struct sgx_tcs {
> > +	uint64_t state;
> > +	uint64_t flags;
> > +	uint64_t ossa;
> > +	uint32_t cssa;
> > +	uint32_t nssa;
> > +	uint64_t oentry;
> > +	uint64_t aep;
> > +	uint64_t ofsbase;
> > +	uint64_t ogsbase;
> > +	uint32_t fslimit;
> > +	uint32_t gslimit;
> > +	uint64_t reserved[503];
> > +} __packed __aligned(4096);
> 
> It's interesting that you defined a reserved[] array here with a
> hard-coded number and a (nicely-aligned, I'll point out) uint64_t, but
> the sgx_secs was defined using #defined reserve sizes and a uint8_t.
> Seems like we should just be consistent.  No?

Yes :-)

> Also, are these truly structures that need alignment?  Or are they just
> the *format* of a page (which is obviously aligned)?  For instance, if
> we copied the hardware structure into a temporary buffer, we wouldn't
> need it aligned.

You have good point here. For sgx_tcs and sgx_secs there should not be
alignment at all as it is either in EPC or in a temporary buffer. I will
remove alignments for those.

For structs such as secinfo it makes sense to have aligment info because
you usually create them as local variables but CPU requires particular
aligment (like 64 for secinfo).

> I sometimes prefer asking for an instance of a variable to be aligned
> rather than the type for things like this.

This was a really good point, thanks.

> > +struct sgx_pageinfo {
> > +	uint64_t linaddr;
> > +	uint64_t srcpge;
> > +	union {
> > +		uint64_t secinfo;
> > +		uint64_t pcmd;
> > +	};
> > +	uint64_t secs;
> > +} __packed __aligned(32);
> 
> I see these were verbatim names taken from the SDM.  Could you also
> please add some small comments about them?  "secs", for instance, is a
> pretty common abbreviation for "seconds".  It would really help to have
> this at _least_ say:
> 
> 	uint64_t sgx_secs;

I would go with comments as it has advantages when you maintain this
code to use same fields names as SDM:

	uint64_t secs; /* Enclave Control Structure */

I wonder why they had to put that 'S' in the front :-( One thing I
would like to diverse from SDM is to use verbatim ECS as it is more
consistent naming with TCS (Thread Control Structure) and pronouncing
SECS has taken me into embarrassing situations in the past...

> > +#define SGX_SECINFO_PERMISSION_MASK	0x0000000000000007L
> > +#define SGX_SECINFO_PAGE_TYPE_MASK	0x000000000000FF00L
> > +#define SGX_SECINFO_RESERVED_MASK	0xFFFFFFFFFFFF00F8L
> > +
> > +enum sgx_page_type {
> > +	SGX_PAGE_TYPE_SECS	= 0x00,
> > +	SGX_PAGE_TYPE_TCS	= 0x01,
> > +	SGX_PAGE_TYPE_REG	= 0x02,
> > +	SGX_PAGE_TYPE_VA	= 0x03,
> > +	SGX_PAGE_TYPE_TRIM	= 0x04,
> > +};
> > +
> > +enum sgx_secinfo_flags {
> > +	SGX_SECINFO_R		= 0x01,
> > +	SGX_SECINFO_W		= 0x02,
> > +	SGX_SECINFO_X		= 0x04,
> > +	SGX_SECINFO_SECS	= (SGX_PAGE_TYPE_SECS << 8),
> > +	SGX_SECINFO_TCS		= (SGX_PAGE_TYPE_TCS << 8),
> > +	SGX_SECINFO_REG		= (SGX_PAGE_TYPE_REG << 8),
> > +	SGX_SECINFO_VA          = (SGX_PAGE_TYPE_VA << 8),
> > +	SGX_SECINFO_TRIM	= (SGX_PAGE_TYPE_TRIM << 8),
> > +};
> > +
> > +struct sgx_secinfo {
> > +	uint64_t flags;
> > +	uint64_t reserved[7];
> > +} __packed __aligned(64);
> > +
> > +struct sgx_pcmd {
> > +	struct sgx_secinfo secinfo;
> > +	uint64_t enclave_id;
> > +	uint8_t reserved[40];
> > +	uint8_t mac[16];
> > +} __packed __aligned(128);
> 
> I don't see any sign of this alignment restriction in the SDM.

Have to check whether it is needed or a missing item in the SDM.

> > +#define SGX_MODULUS_SIZE 384
> 
> This is #defined in a rather odd place.  Why here?  Also, please add a
> unit.  SGX_MODULUS_SIZE_BYTES or SGX_MODULUS_BYTES is way better than _SIZE.

Ok.

> > +struct sgx_sigstruct_header {
> > +	uint64_t header1[2];
> > +	uint32_t vendor;
> > +	uint32_t date;
> > +	uint64_t header2[2];
> > +	uint32_t swdefined;
> > +	uint8_t reserved1[84];
> > +} __packed;
> > +
> > +struct sgx_sigstruct_body {
> > +	uint32_t miscselect;
> > +	uint32_t miscmask;
> > +	uint8_t reserved2[20];
> > +	uint64_t attributes;
> > +	uint64_t xfrm;
> > +	uint8_t attributemask[16];
> 
> I've hit my limit on silly SDM names. :)
> 
> Please make these human-readable: "attribute_mask".  The divergence from
> the SDM naming is justified at this point.

I can update my naming in a way that for these obvious things I use
kernel naming style.

I would stick the SDM verbatim for some fields like MRENCLAVE because
it is so widely used name also in research papers etc.

> > +	uint8_t mrenclave[32];
> > +	uint8_t reserved3[32];
> > +	uint16_t isvprodid;
> > +	uint16_t isvsvn;
> > +} __packed;
> > +
> > +struct sgx_sigstruct {
> > +	struct sgx_sigstruct_header header;
> > +	uint8_t modulus[SGX_MODULUS_SIZE];
> > +	uint32_t exponent;
> > +	uint8_t signature[SGX_MODULUS_SIZE];
> > +	struct sgx_sigstruct_body body;
> > +	uint8_t reserved4[12];
> > +	uint8_t q1[SGX_MODULUS_SIZE];
> > +	uint8_t q2[SGX_MODULUS_SIZE];
> > +} __packed __aligned(4096);
> 
> It's interesting that the SDM says "page aligned" in some places and
> "4096-byte aligned" in others.  Oh well.
> 
> > +struct sgx_einittoken_payload {
> > +	uint32_t valid;
> > +	uint32_t reserved1[11];
> 
> ... and back to different types being used for padding.  These really
> need to at least be consistent.
> 
> > +	uint64_t attributes;
> > +	uint64_t xfrm;
> > +	uint8_t mrenclave[32];
> > +	uint8_t reserved2[32];
> > +	uint8_t mrsigner[32];
> > +	uint8_t reserved3[32];
> > +} __packed;
> Just curious, why is this broken out into its own structure?  It doesn't
> appear architectural.

For token it does not make much sense. For sigstruct the token CMAC
calculation is done over those inner structs if you run your own LE.

> > +struct sgx_einittoken {
> > +	struct sgx_einittoken_payload payload;
> > +	uint8_t cpusvnle[16];
> > +	uint16_t isvprodidle;
> > +	uint16_t isvsvnle;
> > +	uint8_t reserved2[24];
> > +	uint32_t maskedmiscselectle;
> > +	uint64_t maskedattributesle;
> > +	uint64_t maskedxfrmle;
> > +	uint8_t keyid[32];
> > +	uint8_t mac[16];
> > +} __packed __aligned(512);
> > +
> > +#endif /* _ASM_X86_SGX_ARCH_H */
> > 

/Jarkko

Re: [PATCH v12 05/13] x86/sgx: architectural structures

[Jarkko Sakkinen]

On Tue, Jul 03, 2018 at 09:04:01PM +0200, Thomas Gleixner wrote:
> On Tue, 3 Jul 2018, Jarkko Sakkinen wrote:
> 
> > This commit adds arch/x86/include/asm/sgx_arch.h that contains definitions
> 
> This is not a commit. Simply: Add .....
> 
> > for data structures used by the SGX.
> > 
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > Co-developed-by: Suresh Siddha <suresh.b.siddha@intel.com>
> 
> Documentaiton:
> 
>   A Co-Developed-by: states that the patch was also created by another developer
>   along with the original author.  This is useful at times when multiple people
>   work on a single patch.  Note, this person also needs to have a Signed-off-by:
>   line in the patch as well.
> 
> Please fix all over the place.

I used this tag for the first time and thought that it was undocumented
because checkpatch.pl complained about it. I will fix this.

> > +#define SGX_MISC_RESERVED_MASK 0xFFFFFFFFFFFFFFFEL
> 
> L ? ULL I'd say...

Yes, should be ULL.

> > +#define SGX_ATTR_RESERVED_MASK 0xFFFFFFFFFFFFFFC9L
> 
> Ditto
> 
> > +#define SGX_SECS_RESERVED1_SIZE 24
> > +#define SGX_SECS_RESERVED2_SIZE 32
> > +#define SGX_SECS_RESERVED3_SIZE 96
> > +#define SGX_SECS_RESERVED4_SIZE 3836
> > +
> > +struct sgx_secs {
> > +	uint64_t size;
> > +	uint64_t base;
> > +	uint32_t ssaframesize;
> > +	uint32_t miscselect;
> > +	uint8_t reserved1[SGX_SECS_RESERVED1_SIZE];
> > +	uint64_t attributes;
> 
> Please make these tabular aligned for readility sake

Ok, so it is now +2 for this. I can do that if you really want that.

> > +	uint32_t	miscselect;
> > +	uint8_t		reserved1[SGX_SECS_RESERVED1_SIZE];
> > +	uint64_t	attributes;
> 
> Hmm? All over the place...
> 
> > +enum sgx_tcs_flags {
> > +	SGX_TCS_DBGOPTIN	= 0x01, /* cleared on EADD */
> 
> Please do not use tail comments. Either put the comment above the define or
> use kernel doc.
> 
> > +};
> > +
> > +#define SGX_TCS_RESERVED_MASK 0xFFFFFFFFFFFFFFFEL
> 
> See above.
> 
> > +#define SGX_SECINFO_PERMISSION_MASK	0x0000000000000007L
> > +#define SGX_SECINFO_PAGE_TYPE_MASK	0x000000000000FF00L
> > +#define SGX_SECINFO_RESERVED_MASK	0xFFFFFFFFFFFF00F8L
> 
> Ditto
> 
> Thanks,
> 
> 	tglx

Thank you.

/Jarkko

Re: [PATCH v12 05/13] x86/sgx: architectural structures

[Jarkko Sakkinen]

On Tue, Jul 03, 2018 at 02:22:52PM -0700, Dave Hansen wrote:
> On 07/03/2018 11:19 AM, Jarkko Sakkinen wrote:
> > +enum sgx_cpuid {
> > +	SGX_CPUID_CAPABILITIES	= 0,
> > +	SGX_CPUID_ATTRIBUTES	= 1,
> > +	SGX_CPUID_EPC_BANKS	= 2,
> > +};
> 
> I made a review comment about these last time.  I don't remember
> receiving any kind of response from you on it, or seeing it addressed in
> these patches in any kind of way.
> 
> I've also seen comments from Thomas that sound really familiar such as
> the lack of comments on the paging code or the use of a semaphore to
> protect things that don't sleep.  I made very, very similar comments on
> the last posting.
> 
> Is there a reason that those comments were not addressed?

My sloppiness is the reason.

The feedback has been awesome from you and Thomas for this version,
which will help me to get these things right for the next version.

Thank you.

/Jarkko

Re: [PATCH v12 09/13] x86/sgx: EPC page allocation routines

[Jarkko Sakkinen]

On Tue, Jul 03, 2018 at 10:41:14PM +0200, Thomas Gleixner wrote:
> On Tue, 3 Jul 2018, Jarkko Sakkinen wrote:
> >  
> > +#define SGX_NR_TO_SCAN	16
> > +#define SGX_NR_LOW_PAGES 32
> > +#define SGX_NR_HIGH_PAGES 64
> > +
> >  bool sgx_enabled __ro_after_init;
> >  EXPORT_SYMBOL(sgx_enabled);
> >  bool sgx_lc_enabled __ro_after_init;
> >  EXPORT_SYMBOL(sgx_lc_enabled);
> > +LIST_HEAD(sgx_active_page_list);
> > +EXPORT_SYMBOL(sgx_active_page_list);
> > +DEFINE_SPINLOCK(sgx_active_page_list_lock);
> > +EXPORT_SYMBOL(sgx_active_page_list_lock);
> 
> Why is all of this exported. If done right then no call site has to fiddle
> with the list and the lock at all.

We can fix this in a way that these exports are not needed. Thanks
for pointing this out.

> >  static atomic_t sgx_nr_free_pages = ATOMIC_INIT(0);
> >  static struct sgx_epc_bank sgx_epc_banks[SGX_MAX_EPC_BANKS];
> >  static int sgx_nr_epc_banks;
> > +static struct task_struct *ksgxswapd_tsk;
> > +static DECLARE_WAIT_QUEUE_HEAD(ksgxswapd_waitq);
> > +
> > +static void sgx_swap_cluster(void)
> > +{
> > +	struct sgx_epc_page *cluster[SGX_NR_TO_SCAN + 1];
> > +	struct sgx_epc_page *epc_page;
> > +	int i;
> > +	int j;
> 
>   	int i, j;

I've always preferred single declaration per line even for index
variables but not something that I'm going to argue about too
much.

> > +	memset(cluster, 0, sizeof(cluster));
> > +
> > +	for (i = 0, j = 0; i < SGX_NR_TO_SCAN; i++) {
> > +		spin_lock(&sgx_active_page_list_lock);
> > +		if (list_empty(&sgx_active_page_list)) {
> > +			spin_unlock(&sgx_active_page_list_lock);
> > +			break;
> > +		}
> > +		epc_page = list_first_entry(&sgx_active_page_list,
> > +					    struct sgx_epc_page, list);
> > +		if (!epc_page->impl->ops->get(epc_page)) {
> > +			list_move_tail(&epc_page->list, &sgx_active_page_list);
> > +			spin_unlock(&sgx_active_page_list_lock);
> > +			continue;
> > +		}
> > +		list_del(&epc_page->list);
> > +		spin_unlock(&sgx_active_page_list_lock);
> > +
> > +		if (epc_page->impl->ops->reclaim(epc_page)) {
> > +			cluster[j++] = epc_page;
> > +		} else {
> > +			spin_lock(&sgx_active_page_list_lock);
> > +			list_add_tail(&epc_page->list, &sgx_active_page_list);
> > +			spin_unlock(&sgx_active_page_list_lock);
> > +			epc_page->impl->ops->put(epc_page);
> > +		}
> > +	}
> > +
> > +	for (i = 0; cluster[i]; i++) {
> > +		epc_page = cluster[i];
> > +		epc_page->impl->ops->block(epc_page);
> > +	}
> > +
> > +	for (i = 0; cluster[i]; i++) {
> > +		epc_page = cluster[i];
> > +		epc_page->impl->ops->write(epc_page);
> > +		epc_page->impl->ops->put(epc_page);
> > +		sgx_free_page(epc_page);
> > +	}
> 
> Thanks a lot for commenting this piece of art thoughtfully. It's entirely
> clear how all of this works now.

Got your point.

> > +}
> > +
> > +static int ksgxswapd(void *p)
> > +{
> > +	set_freezable();
> > +
> > +	while (!kthread_should_stop()) {
> > +		if (try_to_freeze())
> > +			continue;
> > +
> > +		wait_event_freezable(ksgxswapd_waitq, kthread_should_stop() ||
> > +				     atomic_read(&sgx_nr_free_pages) <
> > +				     SGX_NR_HIGH_PAGES);
> > +
> > +		if (atomic_read(&sgx_nr_free_pages) < SGX_NR_HIGH_PAGES)
> > +			sgx_swap_cluster();
> > +	}
> > +
> > +	pr_info("%s: done\n", __func__);
> 
> Really useful. 

Forgotten cruft, will remove.

> > +	return 0;
> > +}
> > +
> > +static struct sgx_epc_page *sgx_try_alloc_page(struct sgx_epc_page_impl *impl)
> > +{
> > +	struct sgx_epc_bank *bank;
> > +	struct sgx_epc_page *page = NULL;
> > +	int i;
> > +
> > +	for (i = 0; i < sgx_nr_epc_banks; i++) {
> > +		bank = &sgx_epc_banks[i];
> > +
> > +		down_write(&bank->lock);
> > +
> > +		if (atomic_read(&bank->free_cnt))
> 
> And these atomics are required becasue bank->lock protection is not
> sufficient or what am I missing here?

This is also response to your comment below. It would be better idea
to just use a spinlock I guess. Seeing your and Daves point.

> > +			page = bank->pages[atomic_dec_return(&bank->free_cnt)];
> > +
> > +		up_write(&bank->lock);
> > +
> > +		if (page)
> > +			break;
> > +	}
> > +
> > +	if (page) {
> > +		atomic_dec(&sgx_nr_free_pages);
> > +		page->impl = impl;
> > +	}
> > +
> > +	return page;
> > +}
> > +
> > +/**
> > + * sgx_alloc_page - allocate an EPC page
> > + * @flags:	allocation flags
> > + * @impl:	implementation for the struct sgx_epc_page
> > + *
> > + * Try to grab a page from the free EPC page list. If there is a free page
> > + * available, it is returned to the caller. If called with SGX_ALLOC_ATOMIC,
> > + * the function will return immediately if the list is empty. Otherwise, it
> > + * will swap pages up until there is a free page available. Upon returning the
> > + * low watermark is checked and ksgxswapd is waken up if we are below it.
> > + *
> > + * Return:
> > + *   a &struct sgx_epc_page instace,
> > + *   -ENOMEM if all pages are unreclaimable,
> > + *   -EBUSY when called with SGX_ALLOC_ATOMIC and out of free pages
> > + */
> > +struct sgx_epc_page *sgx_alloc_page(struct sgx_epc_page_impl *impl,
> > +				    unsigned int flags)
> > +{
> > +	struct sgx_epc_page *entry;
> > +
> > +	for ( ; ; ) {
> > +		entry = sgx_try_alloc_page(impl);
> > +		if (entry)
> > +			break;
> > +
> > +		if (list_empty(&sgx_active_page_list))
> > +			return ERR_PTR(-ENOMEM);
> > +
> > +		if (flags & SGX_ALLOC_ATOMIC) {
> > +			entry = ERR_PTR(-EBUSY);
> > +			break;
> > +		}
> > +
> > +		if (signal_pending(current)) {
> > +			entry = ERR_PTR(-ERESTARTSYS);
> > +			break;
> > +		}
> > +
> > +		sgx_swap_cluster();
> > +		schedule();
> > +	}
> > +
> > +	if (atomic_read(&sgx_nr_free_pages) < SGX_NR_LOW_PAGES)
> > +		wake_up(&ksgxswapd_waitq);
> 
> What's the logic of SGX_NR_LOW_PAGES vs. SGX_NR_HIGH_PAGES? 

If the number of pages goes below SGX_NR_LOW_PAGES ksgxswapd swaps pages
up until SGX_NR_HIGH_PAGES is reached.

> 
> > +
> > +	return entry;
> > +}
> > +EXPORT_SYMBOL(sgx_alloc_page);
> > +
> > +/**
> > + * sgx_free_page - free an EPC page
> > + *
> > + * @page:	any EPC page
> > + *
> > + * Remove an EPC page and insert it back to the list of free pages.
> > + *
> > + * Return: SGX error code
> > + */
> > +int sgx_free_page(struct sgx_epc_page *page)
> > +{
> > +	struct sgx_epc_bank *bank = SGX_EPC_BANK(page);
> > +	int ret;
> > +
> > +	ret = sgx_eremove(page);
> > +	if (ret) {
> > +		pr_debug("EREMOVE returned %d\n", ret);
> > +		return ret;
> > +	}
> > +
> > +	down_read(&bank->lock);
> > +	bank->pages[atomic_inc_return(&bank->free_cnt) - 1] = page;
> > +	atomic_inc(&sgx_nr_free_pages);
> > +	up_read(&bank->lock);
> 
> I have hard time to see the benefit of this reader/writer semaphore
> here. Both sides which fiddle with the bank pages are doing a simple
> de/increment of free_cnt and a store resp. load. So what justifies the
> overhead of a rwsem?



> 
> >  static __init int sgx_init_epc_bank(unsigned long addr, unsigned long size,
> >  				    unsigned long index,
> >  				    struct sgx_epc_bank *bank)
> > @@ -114,6 +318,11 @@ static __init void sgx_page_cache_teardown(void)
> >  		kfree(bank->pages);
> >  		kfree(bank->pages_data);
> >  	}
> > +
> > +	if (ksgxswapd_tsk) {
> > +		kthread_stop(ksgxswapd_tsk);
> > +		ksgxswapd_tsk = NULL;
> 
> This stops the thread _AFTER_ freeing all the bank memory. Is that actually
> correct?

Should not cause any actual regressions but is a flakky order anyway
so I will change it.

/Jarkko

Re: [PATCH v12 05/13] x86/sgx: architectural structures

[Dave Hansen]

On 07/03/2018 11:19 AM, Jarkko Sakkinen wrote:
> +struct sgx_secs {
> +	uint64_t size;
> +	uint64_t base;
> +	uint32_t ssaframesize;
> +	uint32_t miscselect;
> +	uint8_t reserved1[SGX_SECS_RESERVED1_SIZE];
> +	uint64_t attributes;
> +	uint64_t xfrm;
> +	uint32_t mrenclave[8];
> +	uint8_t reserved2[SGX_SECS_RESERVED2_SIZE];
> +	uint32_t mrsigner[8];
> +	uint8_t	reserved3[SGX_SECS_RESERVED3_SIZE];
> +	uint16_t isvvprodid;
> +	uint16_t isvsvn;
> +	uint8_t reserved4[SGX_SECS_RESERVED4_SIZE];
> +} __packed __aligned(4096);

Why are the uint* versions in use here?  Those are for userspace ABI,
but this is entirely for in-kernel-use, right?

We've used u8/16/32/64 in arch code in a bunch of places.  They're at
least a bit more compact and easier to read.  It's this:

	u8	foo;
	u64	bar;

vs. this:

	uint8_t	 foo;
	uint64_t bar;

Re: [PATCH v12 03/13] x86/sgx: add SGX definitions to msr-index.h

[Jarkko Sakkinen]

On Tue, Jul 03, 2018 at 11:31:59AM -0700, Dave Hansen wrote:
> On 07/03/2018 11:19 AM, Jarkko Sakkinen wrote:
> > --- a/arch/x86/include/asm/msr-index.h
> > +++ b/arch/x86/include/asm/msr-index.h
> > @@ -479,6 +479,8 @@
> >  #define FEATURE_CONTROL_LOCKED				(1<<0)
> >  #define FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX	(1<<1)
> >  #define FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX	(1<<2)
> > +#define FEATURE_CONTROL_SGX_ENABLE                      (1<<18)
> > +#define FEATURE_CONTROL_SGX_LE_WR			(1<<17)
> >  #define FEATURE_CONTROL_LMCE				(1<<20)
> 
> Have you run checkpatch.pl on these?  There's a tabs-vs-spaces issue here.

High portion of lines in this file are over 80 characters and have this
same incosistent spacing. I'm not sure if this should be fixed bacause
it would be consistent with the formatting that this file uses...

/Jarkko

Re: [PATCH v12 03/13] x86/sgx: add SGX definitions to msr-index.h

[Jarkko Sakkinen]

On Thu, Jul 05, 2018 at 07:05:49PM +0300, Jarkko Sakkinen wrote:
> On Tue, Jul 03, 2018 at 11:31:59AM -0700, Dave Hansen wrote:
> > On 07/03/2018 11:19 AM, Jarkko Sakkinen wrote:
> > > --- a/arch/x86/include/asm/msr-index.h
> > > +++ b/arch/x86/include/asm/msr-index.h
> > > @@ -479,6 +479,8 @@
> > >  #define FEATURE_CONTROL_LOCKED				(1<<0)
> > >  #define FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX	(1<<1)
> > >  #define FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX	(1<<2)
> > > +#define FEATURE_CONTROL_SGX_ENABLE                      (1<<18)
> > > +#define FEATURE_CONTROL_SGX_LE_WR			(1<<17)
> > >  #define FEATURE_CONTROL_LMCE				(1<<20)
> > 
> > Have you run checkpatch.pl on these?  There's a tabs-vs-spaces issue here.
> 
> High portion of lines in this file are over 80 characters and have this
> same incosistent spacing. I'm not sure if this should be fixed bacause
> it would be consistent with the formatting that this file uses...

Ignore! I was looking at the cpufeature patch, this one really has a
formatting issue (apologies).

/Jarkko

Re: [PATCH v12 03/13] x86/sgx: add SGX definitions to msr-index.h

[Dave Hansen]

On 07/05/2018 09:05 AM, Jarkko Sakkinen wrote:
> On Tue, Jul 03, 2018 at 11:31:59AM -0700, Dave Hansen wrote:
>> On 07/03/2018 11:19 AM, Jarkko Sakkinen wrote:
>>> --- a/arch/x86/include/asm/msr-index.h
>>> +++ b/arch/x86/include/asm/msr-index.h
>>> @@ -479,6 +479,8 @@
>>>  #define FEATURE_CONTROL_LOCKED				(1<<0)
>>>  #define FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX	(1<<1)
>>>  #define FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX	(1<<2)
>>> +#define FEATURE_CONTROL_SGX_ENABLE                      (1<<18)
>>> +#define FEATURE_CONTROL_SGX_LE_WR			(1<<17)
>>>  #define FEATURE_CONTROL_LMCE				(1<<20)
>> Have you run checkpatch.pl on these?  There's a tabs-vs-spaces issue here.
> High portion of lines in this file are over 80 characters and have this
> same incosistent spacing. I'm not sure if this should be fixed bacause
> it would be consistent with the formatting that this file uses...

Why does that matter?

Just make sure your patch doesn't introduce any new instances of bad
form unless it's *required* to fit into the file.

Re: [PATCH v12 09/13] x86/sgx: EPC page allocation routines

[Jarkko Sakkinen]

On Wed, Jul 04, 2018 at 06:25:53AM +0200, Borislav Petkov wrote:
> ... and also, why are the other exports not EXPORT_SYMBOL_GPL?

I think they could just as well be that. No reason to use
EXPORT_SYMBOL() even if the source files are dual licensed.

/Jarkko

Re: [PATCH v12 05/13] x86/sgx: architectural structures

[Jarkko Sakkinen]

On Thu, Jul 05, 2018 at 08:31:42AM -0700, Dave Hansen wrote:
> On 07/03/2018 11:19 AM, Jarkko Sakkinen wrote:
> > +struct sgx_secs {
> > +	uint64_t size;
> > +	uint64_t base;
> > +	uint32_t ssaframesize;
> > +	uint32_t miscselect;
> > +	uint8_t reserved1[SGX_SECS_RESERVED1_SIZE];
> > +	uint64_t attributes;
> > +	uint64_t xfrm;
> > +	uint32_t mrenclave[8];
> > +	uint8_t reserved2[SGX_SECS_RESERVED2_SIZE];
> > +	uint32_t mrsigner[8];
> > +	uint8_t	reserved3[SGX_SECS_RESERVED3_SIZE];
> > +	uint16_t isvvprodid;
> > +	uint16_t isvsvn;
> > +	uint8_t reserved4[SGX_SECS_RESERVED4_SIZE];
> > +} __packed __aligned(4096);
> 
> Why are the uint* versions in use here?  Those are for userspace ABI,
> but this is entirely for in-kernel-use, right?
> 
> We've used u8/16/32/64 in arch code in a bunch of places.  They're at
> least a bit more compact and easier to read.  It's this:
> 
> 	u8	foo;
> 	u64	bar;
> 
> vs. this:
> 
> 	uint8_t	 foo;
> 	uint64_t bar;

The reason was that with in-kernel LE these were in fact used by
user space code. Now they can be changed to those that you
suggested.

/Jarkko

Re: [PATCH v12 05/13] x86/sgx: architectural structures

[hpa]

On July 5, 2018 1:09:12 PM PDT, Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote:
>On Thu, Jul 05, 2018 at 08:31:42AM -0700, Dave Hansen wrote:
>> On 07/03/2018 11:19 AM, Jarkko Sakkinen wrote:
>> > +struct sgx_secs {
>> > +	uint64_t size;
>> > +	uint64_t base;
>> > +	uint32_t ssaframesize;
>> > +	uint32_t miscselect;
>> > +	uint8_t reserved1[SGX_SECS_RESERVED1_SIZE];
>> > +	uint64_t attributes;
>> > +	uint64_t xfrm;
>> > +	uint32_t mrenclave[8];
>> > +	uint8_t reserved2[SGX_SECS_RESERVED2_SIZE];
>> > +	uint32_t mrsigner[8];
>> > +	uint8_t	reserved3[SGX_SECS_RESERVED3_SIZE];
>> > +	uint16_t isvvprodid;
>> > +	uint16_t isvsvn;
>> > +	uint8_t reserved4[SGX_SECS_RESERVED4_SIZE];
>> > +} __packed __aligned(4096);
>> 
>> Why are the uint* versions in use here?  Those are for userspace ABI,
>> but this is entirely for in-kernel-use, right?
>> 
>> We've used u8/16/32/64 in arch code in a bunch of places.  They're at
>> least a bit more compact and easier to read.  It's this:
>> 
>> 	u8	foo;
>> 	u64	bar;
>> 
>> vs. this:
>> 
>> 	uint8_t	 foo;
>> 	uint64_t bar;
>
>The reason was that with in-kernel LE these were in fact used by
>user space code. Now they can be changed to those that you
>suggested.
>
>/Jarkko

For things exported to user space use __u* and __s* types... the _t types would actually violate the C standard with respect to namespace pollution.
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [PATCH v12 05/13] x86/sgx: architectural structures

[Andy Shevchenko]

On Fri, Jul 6, 2018 at 12:50 AM,  <hpa@zytor.com> wrote:
> On July 5, 2018 1:09:12 PM PDT, Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote:
>>On Thu, Jul 05, 2018 at 08:31:42AM -0700, Dave Hansen wrote:
>>> On 07/03/2018 11:19 AM, Jarkko Sakkinen wrote:
>>> > +struct sgx_secs {
>>> > +  uint64_t size;
>>> > +  uint64_t base;
>>> > +  uint32_t ssaframesize;
>>> > +  uint32_t miscselect;
>>> > +  uint8_t reserved1[SGX_SECS_RESERVED1_SIZE];
>>> > +  uint64_t attributes;
>>> > +  uint64_t xfrm;
>>> > +  uint32_t mrenclave[8];
>>> > +  uint8_t reserved2[SGX_SECS_RESERVED2_SIZE];
>>> > +  uint32_t mrsigner[8];
>>> > +  uint8_t reserved3[SGX_SECS_RESERVED3_SIZE];
>>> > +  uint16_t isvvprodid;
>>> > +  uint16_t isvsvn;
>>> > +  uint8_t reserved4[SGX_SECS_RESERVED4_SIZE];
>>> > +} __packed __aligned(4096);
>>>
>>> Why are the uint* versions in use here?  Those are for userspace ABI,
>>> but this is entirely for in-kernel-use, right?
>>>
>>> We've used u8/16/32/64 in arch code in a bunch of places.  They're at
>>> least a bit more compact and easier to read.  It's this:
>>>
>>>      u8      foo;
>>>      u64     bar;
>>>
>>> vs. this:
>>>
>>>      uint8_t  foo;
>>>      uint64_t bar;
>>
>>The reason was that with in-kernel LE these were in fact used by
>>user space code. Now they can be changed to those that you
>>suggested.

> For things exported to user space use __u* and __s* types... the _t types would actually violate the C standard with respect to namespace pollution.

Hmm... Coding style 5(d) allows to use uintNN_t in new code (as a
variation of uNN choice).


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v12 05/13] x86/sgx: architectural structures

[Thomas Gleixner]

On Tue, 10 Jul 2018, Andy Shevchenko wrote:
> On Fri, Jul 6, 2018 at 12:50 AM,  <hpa@zytor.com> wrote:
> 
> > For things exported to user space use __u* and __s* types... the _t types would actually violate the C standard with respect to namespace pollution.
> 
> Hmm... Coding style 5(d) allows to use uintNN_t in new code (as a
> variation of uNN choice).

But not for user space exposed headers. See 5(e) for that matter.

Thanks,

	tglx

Re: [PATCH v12 11/13] platform/x86: Intel SGX driver

[Jethro Beekman]

[-- Attachment #1: Type: text/plain, Size: 244 bytes --]

On 2018-07-03 11:19, Jarkko Sakkinen wrote:
> +/* IOCTL return values */
> +#define SGX_POWER_LOST_ENCLAVE		0x40000000
> +#define SGX_LE_ROLLBACK			0x40000001

I don't think SGX_LE_ROLLBACK is used anymore.

Jethro Beekman | Fortanix


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 3994 bytes --]

Re: [PATCH v12 11/13] platform/x86: Intel SGX driver

[Jarkko Sakkinen]

On Wed, Jul 25, 2018 at 08:52:27AM -0700, Jethro Beekman wrote:
> On 2018-07-03 11:19, Jarkko Sakkinen wrote:
> > +/* IOCTL return values */
> > +#define SGX_POWER_LOST_ENCLAVE		0x40000000
> > +#define SGX_LE_ROLLBACK			0x40000001
> 
> I don't think SGX_LE_ROLLBACK is used anymore.
> 
> Jethro Beekman | Fortanix

Correct. Thaks for spotting that out.

I've on a leave for soon past three weeks and one week is remaining but
have been slowly preparing v13 like one day a week so after next week it
shouldn't take much time to get it out.

/Jarkko