[PATCH v15 00/23] Intel SGX1

[PATCH v15 00/23] Intel SGX1

[Jarkko Sakkinen]

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

v15:
* Split into more digestable size patches.
* Lots of small fixes and clean ups.
* Signal a "plain" SIGSEGV on an EPCM violation.

v14:
* Change the comment about X86_FEATURE_SGX_LC from “SGX launch
  configuration” to “SGX launch control”.
* Move the SGX-related CPU feature flags as part of the Linux defined
  virtual leaf 8.
* Add SGX_ prefix to the constants defining the ENCLS leaf functions.
* Use GENMASK*() and BIT*() in sgx_arch.h instead of raw hex numbers.
* Refine the long description for CONFIG_INTEL_SGX_CORE.
* Do not use pr_*_ratelimited()  in the driver. The use of the rate limited
  versions is legacy cruft from the prototyping phase.
* Detect sleep with SGX_INVALID_EINIT_TOKEN instead of counting power
  cycles.
* Manually prefix with “sgx:” in the core SGX code instead of redefining
  pr_fmt.
* Report if IA32_SGXLEPUBKEYHASHx MSRs are not writable in the driver
  instead of core because it is a driver requirement.
* Change prompt to bool in the entry for CONFIG_INTEL_SGX_CORE because the
  default is ‘n’.
* Rename struct sgx_epc_bank as struct sgx_epc_section in order to match
  the SDM.
* Allocate struct sgx_epc_page instances one at a time.
* Use “__iomem void *” pointers for the mapped EPC memory consistently.
* Retry once on SGX_INVALID_TOKEN in sgx_einit() instead of counting power
  cycles.
* Call enclave swapping operations directly from the driver instead of
  calling them .indirectly through struct sgx_epc_page_ops because indirect
  calls are not required yet as the patch set does not contain the KVM
  support.
* Added special signal SEGV_SGXERR to notify about SGX EPCM violation
  errors.

v13:
* Always use SGX_CPUID constant instead of a hardcoded value.
* Simplified and documented the macros and functions for ENCLS leaves.
* Enable sgx_free_page() to free active enclave pages on demand
  in order to allow sgx_invalidate() to delete enclave pages.
  It no longer performs EREMOVE if a page is in the process of
  being reclaimed.
* Use PM notifier per enclave so that we don't have to traverse
  the global list of active EPC pages to find enclaves.
* Removed unused SGX_LE_ROLLBACK constant from uapi/asm/sgx.h
* Always use ioremap() to map EPC banks as we only support 64-bit kernel.
* Invalidate IA32_SGXLEPUBKEYHASH cache used by sgx_einit() when going
  to sleep.

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 (12):
  x86/sgx: Update MAINTAINERS
  x86/sgx: Define SGX1 and SGX2 ENCLS leafs
  x86/sgx: Add ENCLS architectural error codes
  x86/sgx: Add SGX1 and SGX2 architectural data structures
  x86/sgx: Add definitions for SGX's CPUID leaf and variable sub-leafs
  x86/sgx: Add wrappers for ENCLS leaf functions
  x86/sgx: Add functions to allocate and free EPC pages
  platform/x86: Intel SGX driver
  platform/x86: sgx: Add swapping functionality to the Intel SGX driver
  x86/sgx: Add a simple swapper for the EPC memory manager
  platform/x86: ptrace() support for the SGX driver
  x86/sgx: Driver documentation

Kai Huang (2):
  x86/cpufeatures: Add Intel-defined SGX feature bit
  x86/cpufeatures: Add Intel-defined SGX_LC feature bit

Sean Christopherson (9):
  x86/cpufeatures: Add SGX sub-features (as Linux-defined bits)
  x86/msr: Add IA32_FEATURE_CONTROL.SGX_ENABLE definition
  x86/cpu/intel: Detect SGX support and update caps appropriately
  x86/mm: x86/sgx: Add new 'PF_SGX' page fault error code bit
  x86/mm: x86/sgx: Signal SIGSEGV for userspace #PFs w/ PF_SGX
  x86/msr: Add SGX Launch Control MSR definitions
  x86/cpu/intel: Clear SGX_LC capability if not enabled in
    FEATURE_CONTROL
  x86/sgx: Enumerate and track EPC sections
  x86/sgx: Add sgx_einit() for initializing enclaves

 Documentation/index.rst                       |   1 +
 Documentation/x86/intel_sgx.rst               | 185 ++++
 MAINTAINERS                                   |   7 +
 arch/x86/Kconfig                              |  18 +
 arch/x86/include/asm/cpufeatures.h            |  23 +-
 arch/x86/include/asm/msr-index.h              |   8 +
 arch/x86/include/asm/sgx.h                    | 326 ++++++
 arch/x86/include/asm/sgx_arch.h               | 400 +++++++
 arch/x86/include/asm/traps.h                  |   1 +
 arch/x86/include/uapi/asm/sgx.h               |  59 ++
 arch/x86/include/uapi/asm/sgx_errno.h         |  91 ++
 arch/x86/kernel/cpu/Makefile                  |   1 +
 arch/x86/kernel/cpu/intel.c                   |  29 +
 arch/x86/kernel/cpu/intel_sgx.c               | 479 +++++++++
 arch/x86/kernel/cpu/scattered.c               |   2 +
 arch/x86/mm/fault.c                           |  13 +
 drivers/platform/x86/Kconfig                  |   2 +
 drivers/platform/x86/Makefile                 |   1 +
 drivers/platform/x86/intel_sgx/Kconfig        |  20 +
 drivers/platform/x86/intel_sgx/Makefile       |  14 +
 drivers/platform/x86/intel_sgx/sgx.h          | 212 ++++
 drivers/platform/x86/intel_sgx/sgx_encl.c     | 977 ++++++++++++++++++
 .../platform/x86/intel_sgx/sgx_encl_page.c    | 178 ++++
 drivers/platform/x86/intel_sgx/sgx_fault.c    | 109 ++
 drivers/platform/x86/intel_sgx/sgx_ioctl.c    | 234 +++++
 drivers/platform/x86/intel_sgx/sgx_main.c     | 267 +++++
 drivers/platform/x86/intel_sgx/sgx_util.c     | 156 +++
 drivers/platform/x86/intel_sgx/sgx_vma.c      | 167 +++
 tools/arch/x86/include/asm/cpufeatures.h      |  21 +-
 29 files changed, 3989 insertions(+), 12 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/uapi/asm/sgx.h
 create mode 100644 arch/x86/include/uapi/asm/sgx_errno.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_util.c
 create mode 100644 drivers/platform/x86/intel_sgx/sgx_vma.c

-- 
2.19.1

[PATCH v15 23/23] x86/sgx: Driver documentation

[Jarkko Sakkinen]

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 5db7e87c7cb1..1cdc139adb40 100644
--- a/Documentation/index.rst
+++ b/Documentation/index.rst
@@ -104,6 +104,7 @@ implementation.
    :maxdepth: 2
 
    sh/index
+   x86/index
 
 Filesystem Documentation
 ------------------------
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.19.1

Re: [PATCH v15 23/23] x86/sgx: Driver documentation

[Mike Rapoport]

On Sat, Nov 03, 2018 at 01:11:22AM +0200, Jarkko Sakkinen wrote:
> 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 5db7e87c7cb1..1cdc139adb40 100644
> --- a/Documentation/index.rst
> +++ b/Documentation/index.rst
> @@ -104,6 +104,7 @@ implementation.
>     :maxdepth: 2
> 
>     sh/index
> +   x86/index

It seems there is no Documentation/x86/index.rst, probably you'd want to
create one and link intel_sgx.rst there

> 
>  Filesystem Documentation
>  ------------------------
> 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

comma  ^

> +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.

I think it's better to use "definition list" here

http://docutils.sourceforge.net/docs/ref/rst/restructuredtext.html#definition-lists

> +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.

ditto

> +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.

ditto

> +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

                       EINIT?

> +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.

  of

> +
> +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.19.1
> 

-- 
Sincerely yours,
Mike.

Re: [PATCH v15 23/23] x86/sgx: Driver documentation

[Jarkko Sakkinen]

On Sun, Nov 04, 2018 at 10:15:13AM +0200, Mike Rapoport wrote:
> On Sat, Nov 03, 2018 at 01:11:22AM +0200, Jarkko Sakkinen wrote:
> > 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 5db7e87c7cb1..1cdc139adb40 100644
> > --- a/Documentation/index.rst
> > +++ b/Documentation/index.rst
> > @@ -104,6 +104,7 @@ implementation.
> >     :maxdepth: 2
> > 
> >     sh/index
> > +   x86/index
> 
> It seems there is no Documentation/x86/index.rst, probably you'd want to
> create one and link intel_sgx.rst there

Thanks I'll update this!

> 
> > 
> >  Filesystem Documentation
> >  ------------------------
> > 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
> 
> comma  ^
> 
> > +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.
> 
> I think it's better to use "definition list" here
> 
> http://docutils.sourceforge.net/docs/ref/rst/restructuredtext.html#definition-lists

Ack.

> 
> > +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.
> 
> ditto
> 
> > +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.
> 
> ditto
> 
> > +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
> 
>                        EINIT?

Yep.

> 
> > +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.
> 
>   of
> 
> > +
> > +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.19.1
> > 
> 
> -- 
> Sincerely yours,
> Mike.
> 

Thank for your remarks!

/Jarkko

Re: [PATCH v15 23/23] x86/sgx: Driver documentation

[Dave Hansen]

On 11/2/18 4:11 PM, Jarkko Sakkinen wrote:
>  Documentation/index.rst         |   1 +
>  Documentation/x86/intel_sgx.rst | 185 ++++++++++++++++++++++++++++++++
>  2 files changed, 186 insertions(+)
>  create mode 100644 Documentation/x86/intel_sgx.rst

This patch set establishes an ABI.  It basically sets in stone a bunch
of behaviors that the enclave, the kernel, and the out-of-enclave code
must follow.

There are a bunch of things that the enclave can do to %rsp or %rip, for
instance, that it is capable and/or permitted to do.

The ABI seems entirely undocumented and rather lightly designed, which
seems like something we should fix before this is merged.

Also, for a feature as massive and complicated as this one, it seems
irresponsible to not have a selftest.  Is that not feasible for some reason?

Re: [PATCH v15 23/23] x86/sgx: Driver documentation

[Jarkko Sakkinen]

On Mon, Nov 05, 2018 at 12:27:11PM -0800, Dave Hansen wrote:
> The ABI seems entirely undocumented and rather lightly designed, which
> seems like something we should fix before this is merged.

ABI is documented in arch/x86/include/uapi/asm/sgx.h that from which the
documentation is included to intel_sgx.rst. I'm not saying that there is
no space refine it but it is neither undocumented.

> Also, for a feature as massive and complicated as this one, it seems
> irresponsible to not have a selftest.  Is that not feasible for some reason?

I do have the in-kernel launch enclave stuff backed up here:

https://github.com/jsakkine-intel/sgx-le-host
https://github.com/jsakkine-intel/sgx-le

This is about as simple as it gets without any type of run-time.

/Jarkko

Re: [PATCH v15 23/23] x86/sgx: Driver documentation

[Jarkko Sakkinen]

On Tue, Nov 06, 2018 at 07:49:11AM +0200, Jarkko Sakkinen wrote:
> On Mon, Nov 05, 2018 at 12:27:11PM -0800, Dave Hansen wrote:
> > The ABI seems entirely undocumented and rather lightly designed, which
> > seems like something we should fix before this is merged.
> 
> ABI is documented in arch/x86/include/uapi/asm/sgx.h that from which the
> documentation is included to intel_sgx.rst. I'm not saying that there is
> no space refine it but it is neither undocumented.
> 
> > Also, for a feature as massive and complicated as this one, it seems
> > irresponsible to not have a selftest.  Is that not feasible for some reason?
> 
> I do have the in-kernel launch enclave stuff backed up here:
> 
> https://github.com/jsakkine-intel/sgx-le-host
> https://github.com/jsakkine-intel/sgx-le
> 
> This is about as simple as it gets without any type of run-time.

Of course the enclave could be simplified considerably since now it has
full code for EINITTOKEN generation, which is probably an overkill for a
selftest.

It could be simple as just enter and return to and from enclave or
something in-between in terms of complexity.

Even for the simplest enclave you need a signing tool (see sgxsign.c in
my sgx-le repository), a custom linker script and a sophisticated
makefile.

/Jarkko

Re: [PATCH v15 23/23] x86/sgx: Driver documentation

[Dave Hansen]

On 11/5/18 9:49 PM, Jarkko Sakkinen wrote:
> On Mon, Nov 05, 2018 at 12:27:11PM -0800, Dave Hansen wrote:
>> The ABI seems entirely undocumented and rather lightly designed, which
>> seems like something we should fix before this is merged.
> 
> ABI is documented in arch/x86/include/uapi/asm/sgx.h that from which the
> documentation is included to intel_sgx.rst. I'm not saying that there is
> no space refine it but it is neither undocumented.

I specifically mean the instruction flow around asynchronous exits or
explicit enclave exit calls via EEXIT.  Signals are part of the ABI but
go unmentioned in the documentation.

It's also worth noting that EENTER *can* act (from the kernel's
perspective) like an instruction that both jumps and sets a bunch of
registers (including %rsp).  It's certainly abnormal in that regard.

In fact, in the docs:

> +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).

it's probably a really good idea to explain that the address outside of
the enclave is enclave-provided, and is not, for instance, just the next
instruction after EENTER.

>> Also, for a feature as massive and complicated as this one, it seems
>> irresponsible to not have a selftest.  Is that not feasible for some reason?
> 
> I do have the in-kernel launch enclave stuff backed up here:
> 
> https://github.com/jsakkine-intel/sgx-le-host
> https://github.com/jsakkine-intel/sgx-le
> 
> This is about as simple as it gets without any type of run-time.

Does this code run when I type "make kselftest"?  If not, I think we
should rectify that.

Re: [PATCH v15 23/23] x86/sgx: Driver documentation

[Jarkko Sakkinen]

On Tue, Nov 06, 2018 at 08:45:37AM -0800, Dave Hansen wrote:
> On 11/5/18 9:49 PM, Jarkko Sakkinen wrote:
> > On Mon, Nov 05, 2018 at 12:27:11PM -0800, Dave Hansen wrote:
> >> The ABI seems entirely undocumented and rather lightly designed, which
> >> seems like something we should fix before this is merged.
> > 
> > ABI is documented in arch/x86/include/uapi/asm/sgx.h that from which the
> > documentation is included to intel_sgx.rst. I'm not saying that there is
> > no space refine it but it is neither undocumented.
> 
> I specifically mean the instruction flow around asynchronous exits or
> explicit enclave exit calls via EEXIT.  Signals are part of the ABI but
> go unmentioned in the documentation.

Ok, thanks for clarification. We will document it.
> 
> It's also worth noting that EENTER *can* act (from the kernel's
> perspective) like an instruction that both jumps and sets a bunch of
> registers (including %rsp).  It's certainly abnormal in that regard.

Agreed.

> In fact, in the docs:
> 
> > +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).
> 
> it's probably a really good idea to explain that the address outside of
> the enclave is enclave-provided, and is not, for instance, just the next
> instruction after EENTER.
> 
> >> Also, for a feature as massive and complicated as this one, it seems
> >> irresponsible to not have a selftest.  Is that not feasible for some reason?
> > 
> > I do have the in-kernel launch enclave stuff backed up here:
> > 
> > https://github.com/jsakkine-intel/sgx-le-host
> > https://github.com/jsakkine-intel/sgx-le
> > 
> > This is about as simple as it gets without any type of run-time.
> 
> Does this code run when I type "make kselftest"?  If not, I think we
> should rectify that.

No, it doesn't. It is just my backup for the non-SDK user space code
that I've made that I will use to fork my user space SGX projects in
the future.

I can work-out a selftest (and provide a new patch in the series) but
I'm still wondering what the enclave should do. I would suggest that
we start with an enclave that does just EEXIT and nothing else.

/Jarkko

Re: [PATCH v15 23/23] x86/sgx: Driver documentation

[Dave Hansen]

On 11/7/18 8:30 AM, Jarkko Sakkinen wrote:
>> Does this code run when I type "make kselftest"?  If not, I think we
>> should rectify that.
> No, it doesn't. It is just my backup for the non-SDK user space code
> that I've made that I will use to fork my user space SGX projects in
> the future.
> 
> I can work-out a selftest (and provide a new patch in the series) but
> I'm still wondering what the enclave should do. I would suggest that
> we start with an enclave that does just EEXIT and nothing else.

Yeah, that's a start.  But, a good selftest would include things like
faults and error conditions.

Re: [PATCH v15 23/23] x86/sgx: Driver documentation

[Jarkko Sakkinen]

On Wed, Nov 07, 2018 at 09:09:37AM -0800, Dave Hansen wrote:
> On 11/7/18 8:30 AM, Jarkko Sakkinen wrote:
> >> Does this code run when I type "make kselftest"?  If not, I think we
> >> should rectify that.
> > No, it doesn't. It is just my backup for the non-SDK user space code
> > that I've made that I will use to fork my user space SGX projects in
> > the future.
> > 
> > I can work-out a selftest (and provide a new patch in the series) but
> > I'm still wondering what the enclave should do. I would suggest that
> > we start with an enclave that does just EEXIT and nothing else.
> 
> Yeah, that's a start.  But, a good selftest would include things like
> faults and error conditions.

Great. We can add more entry points to the enclave for different tests
but I'll start with a bare minimum. And yeah but ever goes into next
version I'll document the fault handling.

/Jarkko

Re: [PATCH v15 23/23] x86/sgx: Driver documentation

[Jarkko Sakkinen]

On Thu, Nov 08, 2018 at 04:39:42PM +0200, Jarkko Sakkinen wrote:
> On Wed, Nov 07, 2018 at 09:09:37AM -0800, Dave Hansen wrote:
> > On 11/7/18 8:30 AM, Jarkko Sakkinen wrote:
> > >> Does this code run when I type "make kselftest"?  If not, I think we
> > >> should rectify that.
> > > No, it doesn't. It is just my backup for the non-SDK user space code
> > > that I've made that I will use to fork my user space SGX projects in
> > > the future.
> > > 
> > > I can work-out a selftest (and provide a new patch in the series) but
> > > I'm still wondering what the enclave should do. I would suggest that
> > > we start with an enclave that does just EEXIT and nothing else.
> > 
> > Yeah, that's a start.  But, a good selftest would include things like
> > faults and error conditions.
> 
> Great. We can add more entry points to the enclave for different tests
> but I'll start with a bare minimum. And yeah but ever goes into next
> version I'll document the fault handling.

For the v17 I'll add exactly two test cases:

1. EENTER/EEXIT
2. EENTER/exception

So that it will easier to evaluate and demonstrate exception handling.

/Jarkko

Re: [PATCH v15 23/23] x86/sgx: Driver documentation

[Jarkko Sakkinen]

On Thu, Nov 08, 2018 at 09:20:40PM +0200, Jarkko Sakkinen wrote:
> On Thu, Nov 08, 2018 at 04:39:42PM +0200, Jarkko Sakkinen wrote:
> > On Wed, Nov 07, 2018 at 09:09:37AM -0800, Dave Hansen wrote:
> > > On 11/7/18 8:30 AM, Jarkko Sakkinen wrote:
> > > >> Does this code run when I type "make kselftest"?  If not, I think we
> > > >> should rectify that.
> > > > No, it doesn't. It is just my backup for the non-SDK user space code
> > > > that I've made that I will use to fork my user space SGX projects in
> > > > the future.
> > > > 
> > > > I can work-out a selftest (and provide a new patch in the series) but
> > > > I'm still wondering what the enclave should do. I would suggest that
> > > > we start with an enclave that does just EEXIT and nothing else.
> > > 
> > > Yeah, that's a start.  But, a good selftest would include things like
> > > faults and error conditions.
> > 
> > Great. We can add more entry points to the enclave for different tests
> > but I'll start with a bare minimum. And yeah but ever goes into next
> > version I'll document the fault handling.
> 
> For the v17 I'll add exactly two test cases:
> 
> 1. EENTER/EEXIT
> 2. EENTER/exception
> 
> So that it will easier to evaluate and demonstrate exception handling.
> 
> /Jarkko

Here is my test program now:

https://github.com/jsakkine-intel/sgx-selftest

It is ~1100 lines ATM. Next I'll deploy it to the kernel tree. It has
only (1) now but I'll add (2) too when I convert this to a kernel patch
(probably by doing sgx_call() with a NULL pointer).

/Jarkko

Re: [PATCH v15 23/23] x86/sgx: Driver documentation

[Jarkko Sakkinen]

On Mon, Nov 05, 2018 at 12:27:11PM -0800, Dave Hansen wrote:
> This patch set establishes an ABI.  It basically sets in stone a bunch
> of behaviors that the enclave, the kernel, and the out-of-enclave code
> must follow.
> 
> There are a bunch of things that the enclave can do to %rsp or %rip, for
> instance, that it is capable and/or permitted to do.

Just want to ask a follow-up from this so that I understand you better.
Now we are talking abou ioctl API, right? I'm not exactly sure how
EENTER/EEXIT/ERESUME behavior connects to that. I understand the tail
of your message but this paragraph left me a bit puzzled...

/Jarkko