mbox series

[V5,00/31] x86/sgx and selftests/sgx: Support SGX2

Message ID cover.1652137848.git.reinette.chatre@intel.com
Headers show
Series x86/sgx and selftests/sgx: Support SGX2 | expand

Message

Reinette Chatre May 10, 2022, 6:08 p.m. UTC
V4: https://lore.kernel.org/lkml/cover.1649878359.git.reinette.chatre@intel.com/

Changes since V4 that directly impact user space:
- SGX_IOC_ENCLAVE_MODIFY_TYPES ioctl()'s struct was renamed
  from struct sgx_enclave_modify_type to
  struct sgx_enclave_modify_types. (Jarkko)

Details about changes since V4 that do not directly impact user space:
- Related function names were changed to match with the struct name
  change:
  sgx_ioc_enclave_modify_type() -> sgx_ioc_enclave_modify_types()
  sgx_enclave_modify_type() -> sgx_enclave_modify_types()
- Revert a SGX_IOC_ENCLAVE_RESTRICT_PERMISSIONS parameter check that
  requires read permission. The hardware does support restricting
  enclave page permission to zero permissions. Replace with
  permission check to ensure read permission is set when write permission
  is set. This is verified early to prevent a later fault of the
  instruction. (Vijay).
- Do not attempt direct reclaim if no EPC pages available during page
  fault. mmap_lock is already held in page fault handler so attempting
  to take it again while running sgx_reclaim_pages() has risk of
  deadlock. This was discovered by lockdep during stress testing.
- Pick up Reviewed-by and Tested-by tags from Jarkko.
- Pick up Tested-by tags from Haitao after testing with Intel SGX SDK/PSW.
- Pick up Tested-by tags from Vijay after testing with Gramine.

V3: https://lore.kernel.org/lkml/cover.1648847675.git.reinette.chatre@intel.com/

Changes since V3 that directly impact user space:
- SGX_IOC_ENCLAVE_RESTRICT_PERMISSIONS ioctl()'s struct
  sgx_enclave_restrict_permissions no longer provides entire secinfo,
  just the new permissions in new "permissions" struct member. (Jarkko)
- Rename SGX_IOC_ENCLAVE_MODIFY_TYPE ioctl() to
  SGX_IOC_ENCLAVE_MODIFY_TYPES. (Jarkko)
- SGX_IOC_ENCLAVE_MODIFY_TYPES ioctl()'s struct sgx_enclave_modify_type
  no longer provides entire secinfo, just the new page type in new
  "page_type" struct member. (Jarkko)

Details about changes since V3 that do not directly impact user space:
- Add new patch to enable VA pages to be added without invoking reclaimer
  directly if no EPC pages are available, failing instead. This enables
  VA pages to be added with enclave's mutex held. Fixes an issue
  encountered by Haitao. More details in new patch "x86/sgx: Support VA page
  allocation without reclaiming".
- While refactoring, change existing code to consistently use
  IS_ALIGNED(). (Jarkko)
- Many patches received a tag from Jarkko.
- Many smaller changes, please refer to individual patches.

V2: https://lore.kernel.org/lkml/cover.1644274683.git.reinette.chatre@intel.com/

Changes since V2 that directly impact user space:
- Maximum allowed permissions of dynamically added pages is RWX,
  previously limited to RW. (Jarkko)
  Dynamically added pages are initially created with architecturally
  limited EPCM permissions of RW. mmap() and mprotect() of these pages
  with RWX permissions would no longer be blocked by SGX driver. PROT_EXEC
  on dynamically added pages will be possible after running ENCLU[EMODPE]
  from within the enclave with appropriate VMA permissions.

- The kernel no longer attempts to track the EPCM runtime permissions. (Jarkko)
  Consequences are:
  - Kernel does not modify PTEs to follow EPCM permissions. User space
    will receive #PF with SGX error code in cases where the V2
    implementation would have resulted in regular (non-SGX) page fault
    error code.
  - SGX_IOC_ENCLAVE_RELAX_PERMISSIONS is removed. This ioctl() was used
    to clear PTEs after permissions were modified from within the enclave
    and ensure correct PTEs are installed. Since PTEs no longer track
    EPCM permissions the changes in EPCM permissions would not impact PTEs.
    As long as new permissions are within the maximum vetted permissions
    (vm_max_prot_bits) only ENCLU[EMODPE] from within enclave is needed,
    as accompanied by appropriate VMA permissions.

- struct sgx_enclave_restrict_perm renamed to
     sgx_enclave_restrict_permissions (Jarkko)

- struct sgx_enclave_modt renamed to struct sgx_enclave_modify_type
  to be consistent with the verbose naming of other SGX uapi structs.

Details about changes since V2 that do not directly impact user space:
- Kernel no longer tracks the runtime EPCM permissions with the aim of
  installing accurate PTEs. (Jarkko)
  - In support of this change the following patches were removed:
    Documentation/x86: Document SGX permission details
    x86/sgx: Support VMA permissions more relaxed than enclave permissions
    x86/sgx: Add pfn_mkwrite() handler for present PTEs
    x86/sgx: Add sgx_encl_page->vm_run_prot_bits for dynamic permission changes
    x86/sgx: Support relaxing of enclave page permissions
  - No more handling of scenarios where VMA permissions may be more
    relaxed than what the EPCM allows. Enclaves are not prevented
    from accessing such pages and the EPCM permissions are entrusted
    to control access as supported by the SGX error code in page faults.
  - No more explicit setting of protection bits in page fault handler.
    Protection bits are inherited from VMA similar to SGX1 support.

- Selftest patches are moved to the end of the series. (Jarkko)

- New patch contributed by Jarkko to avoid duplicated code:
   x86/sgx: Export sgx_encl_page_alloc()

- New patch separating changes from existing patch. (Jarkko)
   x86/sgx: Export sgx_encl_{grow,shrink}()

- New patch to keep one required benefit from the (now removed) kernel
  EPCM permission tracking:
   x86/sgx: Support loading enclave page without VMA permissions check

- Updated cover letter to reflect architecture changes.

- Many smaller changes, please refer to individual patches.

V1: https://lore.kernel.org/linux-sgx/cover.1638381245.git.reinette.chatre@intel.com/

Changes since V1 that directly impact user space:
- SGX2 permission changes changed from a single ioctl() named
  SGX_IOC_PAGE_MODP to two new ioctl()s:
  SGX_IOC_ENCLAVE_RELAX_PERMISSIONS and
  SGX_IOC_ENCLAVE_RESTRICT_PERMISSIONS, supported by two different
  parameter structures (SGX_IOC_ENCLAVE_RELAX_PERMISSIONS does
  not support a result output parameter) (Jarkko).

  User space flow impact: After user space runs ENCLU[EMODPE] it
  needs to call SGX_IOC_ENCLAVE_RELAX_PERMISSIONS to have PTEs
  updated. Previously running SGX_IOC_PAGE_MODP in this scenario
  resulted in EPCM.PR being set but calling
  SGX_IOC_ENCLAVE_RELAX_PERMISSIONS will not result in EPCM.PR
  being set anymore and thus no need for an additional
  ENCLU[EACCEPT].

- SGX_IOC_ENCLAVE_RELAX_PERMISSIONS and
  SGX_IOC_ENCLAVE_RESTRICT_PERMISSIONS
  obtain new permissions from secinfo as parameter instead of
  the permissions directly (Jarkko).

- ioctl() supporting SGX2 page type change is renamed from
  SGX_IOC_PAGE_MODT to SGX_IOC_ENCLAVE_MODIFY_TYPE (Jarkko).

- SGX_IOC_ENCLAVE_MODIFY_TYPE obtains new page type from secinfo
  as parameter instead of the page type directly (Jarkko).

- ioctl() supporting SGX2 page removal is renamed from
  SGX_IOC_PAGE_REMOVE to SGX_IOC_ENCLAVE_REMOVE_PAGES (Jarkko).

- All ioctl() parameter structures have been renamed as a result of the
  ioctl() renaming:
  SGX_IOC_ENCLAVE_RELAX_PERMISSIONS => struct sgx_enclave_relax_perm
  SGX_IOC_ENCLAVE_RESTRICT_PERMISSIONS => struct sgx_enclave_restrict_perm
  SGX_IOC_ENCLAVE_MODIFY_TYPE => struct sgx_enclave_modt
  SGX_IOC_ENCLAVE_REMOVE_PAGES => struct sgx_enclave_remove_pages

Changes since V1 that do not directly impact user space:
- Number of patches in series increased from 25 to 32 primarily because
  of splitting the original submission:
  - Wrappers for the new SGX2 functions are introduced in three separate
    patches replacing the original "x86/sgx: Add wrappers for SGX2
    functions"
    (Jarkko).
  - Moving and renaming sgx_encl_ewb_cpumask() is done with two patches
    replacing the original "x86/sgx: Use more generic name for enclave
    cpumask function" (Jarkko).
  - Support for SGX2 EPCM permission changes is split into two ioctls(),
    one for relaxing and one for restricting permissions, each introduced
    by a new patch replacing the original "x86/sgx: Support enclave page
    permission changes" (Jarkko).
  - Extracted code used by existing ioctls() for usage by new ioctl()s
    into a new utility in new patch "x86/sgx: Create utility to validate
    user provided offset and length" (Dave did not specifically ask for
    this but it addresses his review feedback).
  - Two new Documentation patches to support the SGX2 work
    ("Documentation/x86: Introduce enclave runtime management") and
    a dedicated section on the enclave permission management
    ("Documentation/x86: Document SGX permission details") (Andy).
- Most patches were reworked to improve the language by:
  * aiming to refer to exact item instead of English rephrasing (Jarkko).
  * use ioctl() instead of ioctl throughout (Dave).
  * Use "relaxed" instead of "exceed" when referring to permissions
    (Dave).
- Improved documentation with several additions to
  Documentation/x86/sgx.rst.
- Many smaller changes, please refer to individual patches.

Hi Everybody,

The current Linux kernel support for SGX includes support for SGX1 that
requires that an enclave be created with properties that accommodate all
usages over its (the enclave's) lifetime. This includes properties such
as permissions of enclave pages, the number of enclave pages, and the
number of threads supported by the enclave.

Consequences of this requirement to have the enclave be created to
accommodate all usages include:
* pages needing to support relocated code are required to have RWX
  permissions for their entire lifetime,
* an enclave needs to be created with the maximum stack and heap
  projected to be needed during the enclave's entire lifetime which
  can be longer than the processes running within it,
* an enclave needs to be created with support for the maximum number
  of threads projected to run in the enclave.

Since SGX1 a few more functions were introduced, collectively called
SGX2, that support modifications to an initialized enclave. Hardware
supporting these functions are already available as listed on
https://github.com/ayeks/SGX-hardware

This series adds support for SGX2, also referred to as Enclave Dynamic
Memory Management (EDMM). This includes:

* Support modifying EPCM permissions of regular enclave pages belonging
  to an initialized enclave. Only permission restriction is supported
  via a new ioctl() SGX_IOC_ENCLAVE_RESTRICT_PERMISSIONS. Relaxing of
  EPCM permissions can only be done from within the enclave with the
  SGX instruction ENCLU[EMODPE].

* Support dynamic addition of regular enclave pages to an initialized
  enclave. At creation new pages are architecturally limited to RW EPCM
  permissions but will be accessible with PROT_EXEC after the enclave
  runs ENCLU[EMODPE] to relax EPCM permissions to RWX.
  Pages are dynamically added to an initialized enclave from the SGX
  page fault handler.

* Support expanding an initialized enclave to accommodate more threads.
  More threads can be accommodated by an enclave with the addition of
  Thread Control Structure (TCS) pages that is done by changing the
  type of regular enclave pages to TCS pages using a new ioctl()
  SGX_IOC_ENCLAVE_MODIFY_TYPES.

* Support removing regular and TCS pages from an initialized enclave.
  Removing pages is accomplished in two stages as supported by two new
  ioctl()s SGX_IOC_ENCLAVE_MODIFY_TYPES (same ioctl() as mentioned in
  previous bullet) and SGX_IOC_ENCLAVE_REMOVE_PAGES.

* Tests covering all the new flows, some edge cases, and one
  comprehensive stress scenario.

No additional work is needed to support SGX2 in a virtualized
environment. All tests included in this series passed when run from
a guest as tested with the recent QEMU release based on 6.2.0
that supports SGX.

Patches 1 through 14 prepare the existing code for SGX2 support by
introducing the SGX2 functions, refactoring code, and tracking enclave
page types.

Patches 15 through 21 enable the SGX2 features and include a
Documentation patch.

Patches 22 through 31 test several scenarios of all the enabled
SGX2 features.

This series is based on v5.18-rc5 with recently submitted SGX shmem
fixes applied:
https://lore.kernel.org/linux-sgx/cover.1652131695.git.reinette.chatre@intel.com/
A repo with both series applied is available:
repo: https://github.com/rchatre/linux.git
branch: sgx/sgx2_submitted_v5_plus_rwx

This SGX2 series also applies directly to v5.18-rc5 if done with a 3-way merge
since it and the shmem fixes both make changes to arch/x86/kernel/cpu/sgx/encl.h
but do not have direct conflicts.

Your feedback will be greatly appreciated.

Regards,

Reinette

Jarkko Sakkinen (1):
  x86/sgx: Export sgx_encl_page_alloc()

Reinette Chatre (30):
  x86/sgx: Add short descriptions to ENCLS wrappers
  x86/sgx: Add wrapper for SGX2 EMODPR function
  x86/sgx: Add wrapper for SGX2 EMODT function
  x86/sgx: Add wrapper for SGX2 EAUG function
  x86/sgx: Support loading enclave page without VMA permissions check
  x86/sgx: Export sgx_encl_ewb_cpumask()
  x86/sgx: Rename sgx_encl_ewb_cpumask() as sgx_encl_cpumask()
  x86/sgx: Move PTE zap code to new sgx_zap_enclave_ptes()
  x86/sgx: Make sgx_ipi_cb() available internally
  x86/sgx: Create utility to validate user provided offset and length
  x86/sgx: Keep record of SGX page type
  x86/sgx: Export sgx_encl_{grow,shrink}()
  x86/sgx: Support VA page allocation without reclaiming
  x86/sgx: Support restricting of enclave page permissions
  x86/sgx: Support adding of pages to an initialized enclave
  x86/sgx: Tighten accessible memory range after enclave initialization
  x86/sgx: Support modifying SGX page type
  x86/sgx: Support complete page removal
  x86/sgx: Free up EPC pages directly to support large page ranges
  Documentation/x86: Introduce enclave runtime management section
  selftests/sgx: Add test for EPCM permission changes
  selftests/sgx: Add test for TCS page permission changes
  selftests/sgx: Test two different SGX2 EAUG flows
  selftests/sgx: Introduce dynamic entry point
  selftests/sgx: Introduce TCS initialization enclave operation
  selftests/sgx: Test complete changing of page type flow
  selftests/sgx: Test faulty enclave behavior
  selftests/sgx: Test invalid access to removed enclave page
  selftests/sgx: Test reclaiming of untouched page
  selftests/sgx: Page removal stress test

 Documentation/x86/sgx.rst                     |   15 +
 arch/x86/include/asm/sgx.h                    |    8 +
 arch/x86/include/uapi/asm/sgx.h               |   62 +
 arch/x86/kernel/cpu/sgx/encl.c                |  329 +++-
 arch/x86/kernel/cpu/sgx/encl.h                |   15 +-
 arch/x86/kernel/cpu/sgx/encls.h               |   33 +
 arch/x86/kernel/cpu/sgx/ioctl.c               |  641 +++++++-
 arch/x86/kernel/cpu/sgx/main.c                |   75 +-
 arch/x86/kernel/cpu/sgx/sgx.h                 |    3 +
 tools/testing/selftests/sgx/defines.h         |   23 +
 tools/testing/selftests/sgx/load.c            |   41 +
 tools/testing/selftests/sgx/main.c            | 1435 +++++++++++++++++
 tools/testing/selftests/sgx/main.h            |    1 +
 tools/testing/selftests/sgx/test_encl.c       |   68 +
 .../selftests/sgx/test_encl_bootstrap.S       |    6 +
 15 files changed, 2627 insertions(+), 128 deletions(-)


base-commit: 672c0c5173427e6b3e2a9bbb7be51ceeec78093a
prerequisite-patch-id: 1a738c00922b0ec865f2674c6f4f8be9ff9b1aab
prerequisite-patch-id: 792889ea9bdfae8c150b1be5c16da697bc404422
prerequisite-patch-id: 78ed2d6251ead724bcb96e0f058bb39dca9eba04
prerequisite-patch-id: cbb715e565631a146eb3cd902455ebaa5d489872
prerequisite-patch-id: 3e853bae87d94f8695a48c537ef32a516f415933

Comments

Reinette Chatre May 11, 2022, 6:47 p.m. UTC | #1
Hi Jarkko,

On 5/10/2022 3:22 PM, Jarkko Sakkinen wrote:
> If there is any patch that does not have my reviewed-by, please put it
> there. I was totally happy with v4 already. I went through these, and
> did not see anything worth of complaining about.
> 
> Great job, thank you for doing this.
> 
> I can also add my tag separely to each patch, which have not have it on
> request if that makes things easier in any possible way on request.

Thank you very much. I do appreciate all the feedback and testing.

All patches in this series have some tag from you, a few have "Acked-by"
instead of "Reviewed-by".

Patch 20/31 "x86/sgx: Free up EPC pages directly to support large
page ranges" is the only x86/sgx patch that has an "Acked-by" from you
instead of a "Reviewed-by". All selftests/sgx patches have an "Acked-by"
from you.

Here is a summary of your tags if you would like to make changes:

[PATCH V5 01/31] x86/sgx: Add short descriptions to ENCLS wrappers
	Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

[PATCH V5 02/31] x86/sgx: Add wrapper for SGX2 EMODPR function
	Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

[PATCH V5 03/31] x86/sgx: Add wrapper for SGX2 EMODT function
	Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

[PATCH V5 04/31] x86/sgx: Add wrapper for SGX2 EAUG function
	Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

[PATCH V5 05/31] x86/sgx: Support loading enclave page without VMA
	Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

[PATCH V5 06/31] x86/sgx: Export sgx_encl_ewb_cpumask()
	Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

[PATCH V5 07/31] x86/sgx: Rename sgx_encl_ewb_cpumask() as sgx_encl_cpumask()
	Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

[PATCH V5 08/31] x86/sgx: Move PTE zap code to new sgx_zap_enclave_ptes()
	Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

[PATCH V5 09/31] x86/sgx: Make sgx_ipi_cb() available internally
	Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

[PATCH V5 10/31] x86/sgx: Create utility to validate user provided offset and length
	Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

[PATCH V5 11/31] x86/sgx: Keep record of SGX page type
	Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

[PATCH V5 12/31] x86/sgx: Export sgx_encl_{grow,shrink}()
	Suggested-by: Jarkko Sakkinen <jarkko@kernel.org>
	Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

[PATCH V5 13/31] x86/sgx: Export sgx_encl_page_alloc()
	Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>

[PATCH V5 14/31] x86/sgx: Support VA page allocation without reclaiming
	Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

[PATCH V5 15/31] x86/sgx: Support restricting of enclave page permissions
	Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
	Tested-by: Jarkko Sakkinen <jarkko@kernel.org>

[PATCH V5 16/31] x86/sgx: Support adding of pages to an initialized enclave
	Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
	Tested-by: Jarkko Sakkinen <jarkko@kernel.org>

[PATCH V5 17/31] x86/sgx: Tighten accessible memory range after enclave initialization
	Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

[PATCH V5 18/31] x86/sgx: Support modifying SGX page type
	Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
	Tested-by: Jarkko Sakkinen <jarkko@kernel.org>

[PATCH V5 19/31] x86/sgx: Support complete page removal
	Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
	Tested-by: Jarkko Sakkinen <jarkko@kernel.org>

[PATCH V5 20/31] x86/sgx: Free up EPC pages directly to support large page ranges
	Acked-by: Jarkko Sakkinen <jarkko@kernel.org>

[PATCH V5 21/31] Documentation/x86: Introduce enclave runtime management section
	Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

[PATCH V5 22/31] selftests/sgx: Add test for EPCM permission changes
	Acked-by: Jarkko Sakkinen <jarkko@kernel.org>

[PATCH V5 23/31] selftests/sgx: Add test for TCS page permission changes
	Acked-by: Jarkko Sakkinen <jarkko@kernel.org>

[PATCH V5 24/31] selftests/sgx: Test two different SGX2 EAUG flows
	Acked-by: Jarkko Sakkinen <jarkko@kernel.org>

[PATCH V5 25/31] selftests/sgx: Introduce dynamic entry point
	Acked-by: Jarkko Sakkinen <jarkko@kernel.org>

[PATCH V5 26/31] selftests/sgx: Introduce TCS initialization enclave operation
	Acked-by: Jarkko Sakkinen <jarkko@kernel.org>

[PATCH V5 27/31] selftests/sgx: Test complete changing of page type flow
	Acked-by: Jarkko Sakkinen <jarkko@kernel.org>

[PATCH V5 28/31] selftests/sgx: Test faulty enclave behavior
	Acked-by: Jarkko Sakkinen <jarkko@kernel.org>

[PATCH V5 29/31] selftests/sgx: Test invalid access to removed enclave page
	Acked-by: Jarkko Sakkinen <jarkko@kernel.org>

[PATCH V5 30/31] selftests/sgx: Test reclaiming of untouched page
	Acked-by: Jarkko Sakkinen <jarkko@kernel.org>

[PATCH V5 31/31] selftests/sgx: Page removal stress test
	Acked-by: Jarkko Sakkinen <jarkko@kernel.org>	


Reinette
Jarkko Sakkinen May 12, 2022, 6:11 p.m. UTC | #2
On Wed, May 11, 2022 at 11:47:31AM -0700, Reinette Chatre wrote:
> Hi Jarkko,
> 
> On 5/10/2022 3:22 PM, Jarkko Sakkinen wrote:
> > If there is any patch that does not have my reviewed-by, please put it
> > there. I was totally happy with v4 already. I went through these, and
> > did not see anything worth of complaining about.
> > 
> > Great job, thank you for doing this.
> > 
> > I can also add my tag separely to each patch, which have not have it on
> > request if that makes things easier in any possible way on request.
> 
> Thank you very much. I do appreciate all the feedback and testing.
> 
> All patches in this series have some tag from you, a few have "Acked-by"
> instead of "Reviewed-by".
> 
> Patch 20/31 "x86/sgx: Free up EPC pages directly to support large
> page ranges" is the only x86/sgx patch that has an "Acked-by" from you
> instead of a "Reviewed-by". All selftests/sgx patches have an "Acked-by"
> from you.
> 
> Here is a summary of your tags if you would like to make changes:
> 
> [PATCH V5 01/31] x86/sgx: Add short descriptions to ENCLS wrappers
> 	Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> [PATCH V5 02/31] x86/sgx: Add wrapper for SGX2 EMODPR function
> 	Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> [PATCH V5 03/31] x86/sgx: Add wrapper for SGX2 EMODT function
> 	Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> [PATCH V5 04/31] x86/sgx: Add wrapper for SGX2 EAUG function
> 	Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> [PATCH V5 05/31] x86/sgx: Support loading enclave page without VMA
> 	Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> [PATCH V5 06/31] x86/sgx: Export sgx_encl_ewb_cpumask()
> 	Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> [PATCH V5 07/31] x86/sgx: Rename sgx_encl_ewb_cpumask() as sgx_encl_cpumask()
> 	Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> [PATCH V5 08/31] x86/sgx: Move PTE zap code to new sgx_zap_enclave_ptes()
> 	Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> [PATCH V5 09/31] x86/sgx: Make sgx_ipi_cb() available internally
> 	Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> [PATCH V5 10/31] x86/sgx: Create utility to validate user provided offset and length
> 	Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> [PATCH V5 11/31] x86/sgx: Keep record of SGX page type
> 	Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> [PATCH V5 12/31] x86/sgx: Export sgx_encl_{grow,shrink}()
> 	Suggested-by: Jarkko Sakkinen <jarkko@kernel.org>
> 	Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> [PATCH V5 13/31] x86/sgx: Export sgx_encl_page_alloc()
> 	Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> [PATCH V5 14/31] x86/sgx: Support VA page allocation without reclaiming
> 	Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> [PATCH V5 15/31] x86/sgx: Support restricting of enclave page permissions
> 	Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> 	Tested-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> [PATCH V5 16/31] x86/sgx: Support adding of pages to an initialized enclave
> 	Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> 	Tested-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> [PATCH V5 17/31] x86/sgx: Tighten accessible memory range after enclave initialization
> 	Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> [PATCH V5 18/31] x86/sgx: Support modifying SGX page type
> 	Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> 	Tested-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> [PATCH V5 19/31] x86/sgx: Support complete page removal
> 	Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> 	Tested-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> [PATCH V5 20/31] x86/sgx: Free up EPC pages directly to support large page ranges
> 	Acked-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> [PATCH V5 21/31] Documentation/x86: Introduce enclave runtime management section
> 	Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> [PATCH V5 22/31] selftests/sgx: Add test for EPCM permission changes
> 	Acked-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> [PATCH V5 23/31] selftests/sgx: Add test for TCS page permission changes
> 	Acked-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> [PATCH V5 24/31] selftests/sgx: Test two different SGX2 EAUG flows
> 	Acked-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> [PATCH V5 25/31] selftests/sgx: Introduce dynamic entry point
> 	Acked-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> [PATCH V5 26/31] selftests/sgx: Introduce TCS initialization enclave operation
> 	Acked-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> [PATCH V5 27/31] selftests/sgx: Test complete changing of page type flow
> 	Acked-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> [PATCH V5 28/31] selftests/sgx: Test faulty enclave behavior
> 	Acked-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> [PATCH V5 29/31] selftests/sgx: Test invalid access to removed enclave page
> 	Acked-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> [PATCH V5 30/31] selftests/sgx: Test reclaiming of untouched page
> 	Acked-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> [PATCH V5 31/31] selftests/sgx: Page removal stress test
> 	Acked-by: Jarkko Sakkinen <jarkko@kernel.org>	
> 
> 
> Reinette

It looks good. And yeah, I've been running different versions of this patch
set since April with zero issues, about a month, in our platform. No high
doubts that anything would wrong that could not be later fixed, if problems
arise.

BR, Jarkko
Jarkko Sakkinen June 1, 2022, 2:01 a.m. UTC | #3
On Tue, 2022-05-10 at 11:08 -0700, Reinette Chatre wrote:
> V4: https://lore.kernel.org/lkml/cover.1649878359.git.reinette.chatre@intel.com/
> 
> Changes since V4 that directly impact user space:
> - SGX_IOC_ENCLAVE_MODIFY_TYPES ioctl()'s struct was renamed
>   from struct sgx_enclave_modify_type to
>   struct sgx_enclave_modify_types. (Jarkko)
> 
> Details about changes since V4 that do not directly impact user space:
> - Related function names were changed to match with the struct name
>   change:
>   sgx_ioc_enclave_modify_type() -> sgx_ioc_enclave_modify_types()
>   sgx_enclave_modify_type() -> sgx_enclave_modify_types()
> - Revert a SGX_IOC_ENCLAVE_RESTRICT_PERMISSIONS parameter check that
>   requires read permission. The hardware does support restricting
>   enclave page permission to zero permissions. Replace with
>   permission check to ensure read permission is set when write permission
>   is set. This is verified early to prevent a later fault of the
>   instruction. (Vijay).
> - Do not attempt direct reclaim if no EPC pages available during page
>   fault. mmap_lock is already held in page fault handler so attempting
>   to take it again while running sgx_reclaim_pages() has risk of
>   deadlock. This was discovered by lockdep during stress testing.
> - Pick up Reviewed-by and Tested-by tags from Jarkko.
> - Pick up Tested-by tags from Haitao after testing with Intel SGX SDK/PSW.
> - Pick up Tested-by tags from Vijay after testing with Gramine.
> 
> V3: https://lore.kernel.org/lkml/cover.1648847675.git.reinette.chatre@intel.com/
> 
> Changes since V3 that directly impact user space:
> - SGX_IOC_ENCLAVE_RESTRICT_PERMISSIONS ioctl()'s struct
>   sgx_enclave_restrict_permissions no longer provides entire secinfo,
>   just the new permissions in new "permissions" struct member. (Jarkko)
> - Rename SGX_IOC_ENCLAVE_MODIFY_TYPE ioctl() to
>   SGX_IOC_ENCLAVE_MODIFY_TYPES. (Jarkko)
> - SGX_IOC_ENCLAVE_MODIFY_TYPES ioctl()'s struct sgx_enclave_modify_type
>   no longer provides entire secinfo, just the new page type in new
>   "page_type" struct member. (Jarkko)
> 
> Details about changes since V3 that do not directly impact user space:
> - Add new patch to enable VA pages to be added without invoking reclaimer
>   directly if no EPC pages are available, failing instead. This enables
>   VA pages to be added with enclave's mutex held. Fixes an issue
>   encountered by Haitao. More details in new patch "x86/sgx: Support VA page
>   allocation without reclaiming".
> - While refactoring, change existing code to consistently use
>   IS_ALIGNED(). (Jarkko)
> - Many patches received a tag from Jarkko.
> - Many smaller changes, please refer to individual patches.
> 
> V2: https://lore.kernel.org/lkml/cover.1644274683.git.reinette.chatre@intel.com/
> 
> Changes since V2 that directly impact user space:
> - Maximum allowed permissions of dynamically added pages is RWX,
>   previously limited to RW. (Jarkko)
>   Dynamically added pages are initially created with architecturally
>   limited EPCM permissions of RW. mmap() and mprotect() of these pages
>   with RWX permissions would no longer be blocked by SGX driver. PROT_EXEC
>   on dynamically added pages will be possible after running ENCLU[EMODPE]
>   from within the enclave with appropriate VMA permissions.
> 
> - The kernel no longer attempts to track the EPCM runtime permissions. (Jarkko)
>   Consequences are:
>   - Kernel does not modify PTEs to follow EPCM permissions. User space
>     will receive #PF with SGX error code in cases where the V2
>     implementation would have resulted in regular (non-SGX) page fault
>     error code.
>   - SGX_IOC_ENCLAVE_RELAX_PERMISSIONS is removed. This ioctl() was used
>     to clear PTEs after permissions were modified from within the enclave
>     and ensure correct PTEs are installed. Since PTEs no longer track
>     EPCM permissions the changes in EPCM permissions would not impact PTEs.
>     As long as new permissions are within the maximum vetted permissions
>     (vm_max_prot_bits) only ENCLU[EMODPE] from within enclave is needed,
>     as accompanied by appropriate VMA permissions.
> 
> - struct sgx_enclave_restrict_perm renamed to
>      sgx_enclave_restrict_permissions (Jarkko)
> 
> - struct sgx_enclave_modt renamed to struct sgx_enclave_modify_type
>   to be consistent with the verbose naming of other SGX uapi structs.
> 
> Details about changes since V2 that do not directly impact user space:
> - Kernel no longer tracks the runtime EPCM permissions with the aim of
>   installing accurate PTEs. (Jarkko)
>   - In support of this change the following patches were removed:
>     Documentation/x86: Document SGX permission details
>     x86/sgx: Support VMA permissions more relaxed than enclave permissions
>     x86/sgx: Add pfn_mkwrite() handler for present PTEs
>     x86/sgx: Add sgx_encl_page->vm_run_prot_bits for dynamic permission changes
>     x86/sgx: Support relaxing of enclave page permissions
>   - No more handling of scenarios where VMA permissions may be more
>     relaxed than what the EPCM allows. Enclaves are not prevented
>     from accessing such pages and the EPCM permissions are entrusted
>     to control access as supported by the SGX error code in page faults.
>   - No more explicit setting of protection bits in page fault handler.
>     Protection bits are inherited from VMA similar to SGX1 support.
> 
> - Selftest patches are moved to the end of the series. (Jarkko)
> 
> - New patch contributed by Jarkko to avoid duplicated code:
>    x86/sgx: Export sgx_encl_page_alloc()
> 
> - New patch separating changes from existing patch. (Jarkko)
>    x86/sgx: Export sgx_encl_{grow,shrink}()
> 
> - New patch to keep one required benefit from the (now removed) kernel
>   EPCM permission tracking:
>    x86/sgx: Support loading enclave page without VMA permissions check
> 
> - Updated cover letter to reflect architecture changes.
> 
> - Many smaller changes, please refer to individual patches.
> 
> V1: https://lore.kernel.org/linux-sgx/cover.1638381245.git.reinette.chatre@intel.com/
> 
> Changes since V1 that directly impact user space:
> - SGX2 permission changes changed from a single ioctl() named
>   SGX_IOC_PAGE_MODP to two new ioctl()s:
>   SGX_IOC_ENCLAVE_RELAX_PERMISSIONS and
>   SGX_IOC_ENCLAVE_RESTRICT_PERMISSIONS, supported by two different
>   parameter structures (SGX_IOC_ENCLAVE_RELAX_PERMISSIONS does
>   not support a result output parameter) (Jarkko).
> 
>   User space flow impact: After user space runs ENCLU[EMODPE] it
>   needs to call SGX_IOC_ENCLAVE_RELAX_PERMISSIONS to have PTEs
>   updated. Previously running SGX_IOC_PAGE_MODP in this scenario
>   resulted in EPCM.PR being set but calling
>   SGX_IOC_ENCLAVE_RELAX_PERMISSIONS will not result in EPCM.PR
>   being set anymore and thus no need for an additional
>   ENCLU[EACCEPT].
> 
> - SGX_IOC_ENCLAVE_RELAX_PERMISSIONS and
>   SGX_IOC_ENCLAVE_RESTRICT_PERMISSIONS
>   obtain new permissions from secinfo as parameter instead of
>   the permissions directly (Jarkko).
> 
> - ioctl() supporting SGX2 page type change is renamed from
>   SGX_IOC_PAGE_MODT to SGX_IOC_ENCLAVE_MODIFY_TYPE (Jarkko).
> 
> - SGX_IOC_ENCLAVE_MODIFY_TYPE obtains new page type from secinfo
>   as parameter instead of the page type directly (Jarkko).
> 
> - ioctl() supporting SGX2 page removal is renamed from
>   SGX_IOC_PAGE_REMOVE to SGX_IOC_ENCLAVE_REMOVE_PAGES (Jarkko).
> 
> - All ioctl() parameter structures have been renamed as a result of the
>   ioctl() renaming:
>   SGX_IOC_ENCLAVE_RELAX_PERMISSIONS => struct sgx_enclave_relax_perm
>   SGX_IOC_ENCLAVE_RESTRICT_PERMISSIONS => struct sgx_enclave_restrict_perm
>   SGX_IOC_ENCLAVE_MODIFY_TYPE => struct sgx_enclave_modt
>   SGX_IOC_ENCLAVE_REMOVE_PAGES => struct sgx_enclave_remove_pages
> 
> Changes since V1 that do not directly impact user space:
> - Number of patches in series increased from 25 to 32 primarily because
>   of splitting the original submission:
>   - Wrappers for the new SGX2 functions are introduced in three separate
>     patches replacing the original "x86/sgx: Add wrappers for SGX2
>     functions"
>     (Jarkko).
>   - Moving and renaming sgx_encl_ewb_cpumask() is done with two patches
>     replacing the original "x86/sgx: Use more generic name for enclave
>     cpumask function" (Jarkko).
>   - Support for SGX2 EPCM permission changes is split into two ioctls(),
>     one for relaxing and one for restricting permissions, each introduced
>     by a new patch replacing the original "x86/sgx: Support enclave page
>     permission changes" (Jarkko).
>   - Extracted code used by existing ioctls() for usage by new ioctl()s
>     into a new utility in new patch "x86/sgx: Create utility to validate
>     user provided offset and length" (Dave did not specifically ask for
>     this but it addresses his review feedback).
>   - Two new Documentation patches to support the SGX2 work
>     ("Documentation/x86: Introduce enclave runtime management") and
>     a dedicated section on the enclave permission management
>     ("Documentation/x86: Document SGX permission details") (Andy).
> - Most patches were reworked to improve the language by:
>   * aiming to refer to exact item instead of English rephrasing (Jarkko).
>   * use ioctl() instead of ioctl throughout (Dave).
>   * Use "relaxed" instead of "exceed" when referring to permissions
>     (Dave).
> - Improved documentation with several additions to
>   Documentation/x86/sgx.rst.
> - Many smaller changes, please refer to individual patches.
> 
> Hi Everybody,
> 
> The current Linux kernel support for SGX includes support for SGX1 that
> requires that an enclave be created with properties that accommodate all
> usages over its (the enclave's) lifetime. This includes properties such
> as permissions of enclave pages, the number of enclave pages, and the
> number of threads supported by the enclave.
> 
> Consequences of this requirement to have the enclave be created to
> accommodate all usages include:
> * pages needing to support relocated code are required to have RWX
>   permissions for their entire lifetime,
> * an enclave needs to be created with the maximum stack and heap
>   projected to be needed during the enclave's entire lifetime which
>   can be longer than the processes running within it,
> * an enclave needs to be created with support for the maximum number
>   of threads projected to run in the enclave.
> 
> Since SGX1 a few more functions were introduced, collectively called
> SGX2, that support modifications to an initialized enclave. Hardware
> supporting these functions are already available as listed on
> https://github.com/ayeks/SGX-hardware
> 
> This series adds support for SGX2, also referred to as Enclave Dynamic
> Memory Management (EDMM). This includes:
> 
> * Support modifying EPCM permissions of regular enclave pages belonging
>   to an initialized enclave. Only permission restriction is supported
>   via a new ioctl() SGX_IOC_ENCLAVE_RESTRICT_PERMISSIONS. Relaxing of
>   EPCM permissions can only be done from within the enclave with the
>   SGX instruction ENCLU[EMODPE].
> 
> * Support dynamic addition of regular enclave pages to an initialized
>   enclave. At creation new pages are architecturally limited to RW EPCM
>   permissions but will be accessible with PROT_EXEC after the enclave
>   runs ENCLU[EMODPE] to relax EPCM permissions to RWX.
>   Pages are dynamically added to an initialized enclave from the SGX
>   page fault handler.
> 
> * Support expanding an initialized enclave to accommodate more threads.
>   More threads can be accommodated by an enclave with the addition of
>   Thread Control Structure (TCS) pages that is done by changing the
>   type of regular enclave pages to TCS pages using a new ioctl()
>   SGX_IOC_ENCLAVE_MODIFY_TYPES.
> 
> * Support removing regular and TCS pages from an initialized enclave.
>   Removing pages is accomplished in two stages as supported by two new
>   ioctl()s SGX_IOC_ENCLAVE_MODIFY_TYPES (same ioctl() as mentioned in
>   previous bullet) and SGX_IOC_ENCLAVE_REMOVE_PAGES.
> 
> * Tests covering all the new flows, some edge cases, and one
>   comprehensive stress scenario.
> 
> No additional work is needed to support SGX2 in a virtualized
> environment. All tests included in this series passed when run from
> a guest as tested with the recent QEMU release based on 6.2.0
> that supports SGX.
> 
> Patches 1 through 14 prepare the existing code for SGX2 support by
> introducing the SGX2 functions, refactoring code, and tracking enclave
> page types.
> 
> Patches 15 through 21 enable the SGX2 features and include a
> Documentation patch.
> 
> Patches 22 through 31 test several scenarios of all the enabled
> SGX2 features.
> 
> This series is based on v5.18-rc5 with recently submitted SGX shmem
> fixes applied:
> https://lore.kernel.org/linux-sgx/cover.1652131695.git.reinette.chatre@intel.com/
> A repo with both series applied is available:
> repo: https://github.com/rchatre/linux.git
> branch: sgx/sgx2_submitted_v5_plus_rwx
> 
> This SGX2 series also applies directly to v5.18-rc5 if done with a 3-way merge
> since it and the shmem fixes both make changes to arch/x86/kernel/cpu/sgx/encl.h
> but do not have direct conflicts.
> 
> Your feedback will be greatly appreciated.
> 
> Regards,
> 
> Reinette
> 
> Jarkko Sakkinen (1):
>   x86/sgx: Export sgx_encl_page_alloc()
> 
> Reinette Chatre (30):
>   x86/sgx: Add short descriptions to ENCLS wrappers
>   x86/sgx: Add wrapper for SGX2 EMODPR function
>   x86/sgx: Add wrapper for SGX2 EMODT function
>   x86/sgx: Add wrapper for SGX2 EAUG function
>   x86/sgx: Support loading enclave page without VMA permissions check
>   x86/sgx: Export sgx_encl_ewb_cpumask()
>   x86/sgx: Rename sgx_encl_ewb_cpumask() as sgx_encl_cpumask()
>   x86/sgx: Move PTE zap code to new sgx_zap_enclave_ptes()
>   x86/sgx: Make sgx_ipi_cb() available internally
>   x86/sgx: Create utility to validate user provided offset and length
>   x86/sgx: Keep record of SGX page type
>   x86/sgx: Export sgx_encl_{grow,shrink}()
>   x86/sgx: Support VA page allocation without reclaiming
>   x86/sgx: Support restricting of enclave page permissions
>   x86/sgx: Support adding of pages to an initialized enclave
>   x86/sgx: Tighten accessible memory range after enclave initialization
>   x86/sgx: Support modifying SGX page type
>   x86/sgx: Support complete page removal
>   x86/sgx: Free up EPC pages directly to support large page ranges
>   Documentation/x86: Introduce enclave runtime management section
>   selftests/sgx: Add test for EPCM permission changes
>   selftests/sgx: Add test for TCS page permission changes
>   selftests/sgx: Test two different SGX2 EAUG flows
>   selftests/sgx: Introduce dynamic entry point
>   selftests/sgx: Introduce TCS initialization enclave operation
>   selftests/sgx: Test complete changing of page type flow
>   selftests/sgx: Test faulty enclave behavior
>   selftests/sgx: Test invalid access to removed enclave page
>   selftests/sgx: Test reclaiming of untouched page
>   selftests/sgx: Page removal stress test
> 
>  Documentation/x86/sgx.rst                     |   15 +
>  arch/x86/include/asm/sgx.h                    |    8 +
>  arch/x86/include/uapi/asm/sgx.h               |   62 +
>  arch/x86/kernel/cpu/sgx/encl.c                |  329 +++-
>  arch/x86/kernel/cpu/sgx/encl.h                |   15 +-
>  arch/x86/kernel/cpu/sgx/encls.h               |   33 +
>  arch/x86/kernel/cpu/sgx/ioctl.c               |  641 +++++++-
>  arch/x86/kernel/cpu/sgx/main.c                |   75 +-
>  arch/x86/kernel/cpu/sgx/sgx.h                 |    3 +
>  tools/testing/selftests/sgx/defines.h         |   23 +
>  tools/testing/selftests/sgx/load.c            |   41 +
>  tools/testing/selftests/sgx/main.c            | 1435 +++++++++++++++++
>  tools/testing/selftests/sgx/main.h            |    1 +
>  tools/testing/selftests/sgx/test_encl.c       |   68 +
>  .../selftests/sgx/test_encl_bootstrap.S       |    6 +
>  15 files changed, 2627 insertions(+), 128 deletions(-)
> 
> 
> base-commit: 672c0c5173427e6b3e2a9bbb7be51ceeec78093a
> prerequisite-patch-id: 1a738c00922b0ec865f2674c6f4f8be9ff9b1aab
> prerequisite-patch-id: 792889ea9bdfae8c150b1be5c16da697bc404422
> prerequisite-patch-id: 78ed2d6251ead724bcb96e0f058bb39dca9eba04
> prerequisite-patch-id: cbb715e565631a146eb3cd902455ebaa5d489872
> prerequisite-patch-id: 3e853bae87d94f8695a48c537ef32a516f415933

Is there something preventing to take this into v5.19 merge window?

I don't think this can improve too much out-of-tree anymore.

BR, Jarkko