lkml.org 
[lkml]   [2021]   [Dec]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 16/25] x86/sgx: Support modifying SGX page type
From
Hi Jarkko,

On 12/4/2021 3:45 PM, Jarkko Sakkinen wrote:
> On Wed, Dec 01, 2021 at 11:23:14AM -0800, Reinette Chatre wrote:
>> Every enclave contains one or more Thread Control Structures (TCS). The
>> TCS contains meta-data used by the hardware to save and restore thread
>> specific information when entering/exiting the enclave. With SGX1 an
>> enclave needs to be created with enough TCSs to support the largest
>> number of threads expecting to use the enclave and enough enclave pages
>> to meet all its anticipated memory demands. In SGX1 all pages remain in
>> the enclave until the enclave is unloaded.
>>
>> Earlier changes added support for the SGX2 feature where pages can be
>> added dynamically to an initialized enclave.
>
> Please remove this paragraph, i.e. do not tie the commit order like
> this.

Will do.

>>
>> SGX2 introduces a new function, ENCLS[EMODT], that is used to change
>> the type of an enclave page from a regular (SGX_PAGE_TYPE_REG) enclave
>> page to a TCS (SGX_PAGE_TYPE_TCS) page or change the type from a
>> regular (SGX_PAGE_TYPE_REG) or TCS (SGX_PAGE_TYPE_TCS)
>> page to a trimmed (SGX_PAGE_TYPE_TRIM) page (setting it up for later
>> removal).
>>
>> With the existing support of dynamically adding regular enclave pages
>> to an initialized enclave and changing the page type to TCS it is
>> possible to dynamically increase the number of threads supported by an
>> enclave.
>>
>> Changing the enclave page type to SGX_PAGE_TYPE_TRIM is the first step
>> of dynamically removing pages from an initialized enclave. The complete
>> page removal flow is:
>> 1) Change the type of the pages to be removed to SGX_PAGE_TYPE_TRIM
>> using the ioctl introduced here.
>> 2) Approve the page removal by running ENCLU[EACCEPT] from within
>> the enclave.
>> 3) Initiate actual page removal using the new ioctl introduced in the
>> following patch.
>>
>> Support changing SGX enclave page types with a new ioctl. With this
>
> What is "a new ioctl"? Why not just write "Add <ioctl name>""?

I do so to reduce the changes required during the ioctl naming
discussion churn.

>> ioctl the user specifies a page range and the enclave page type to be
>> applied to all pages in the provided range. The ioctl itself can return
>> an error code based on failures encountered by the OS. It is also
>> possible for SGX specific failures to be encountered. Add a result
>> output parameter to communicate the SGX return code. It is
>> possible for the enclave page type change request to fail on any page
>> within the provided range. Support partial success by returning
>> the number of pages that were successfully changed.
>>
>> After the page type is changed to SGX_PAGE_TYPE_TRIM the page continues
>> to be accessible from the OS perspective with page table entries and
>> internal state. The page may be moved to swap. Any invalid access
>> (any access except ENCLU[EACCEPT]) will encounter a page fault with
>> SGX flag set in error code until the page is removed. Removal of
>> trimmed enclave pages on user request will be supported in following
>> patch. Trimmed enclave pages are also removed when enclave is unloaded.
>>
>> Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
>
> This is lacking discussion of EPCM interaction, most importanly
> .MODIFY field of an EPCM entry.

I will add that. I have the same question here as in EAUG patch - would
you like a duplicate description in this patch and a new patch that
introduces just the __emodt() wrapper or would you be ok with all new
wrappers introduced together and the detailed description of their
hardware supported flows only present in the patch that uses those wrappers?


>> ---
>> arch/x86/include/uapi/asm/sgx.h | 19 +++
>> arch/x86/kernel/cpu/sgx/ioctl.c | 235 ++++++++++++++++++++++++++++++++
>> 2 files changed, 254 insertions(+)
>>
>> diff --git a/arch/x86/include/uapi/asm/sgx.h b/arch/x86/include/uapi/asm/sgx.h
>> index 24bebc31e336..f70caccd166c 100644
>> --- a/arch/x86/include/uapi/asm/sgx.h
>> +++ b/arch/x86/include/uapi/asm/sgx.h
>> @@ -31,6 +31,8 @@ enum sgx_page_flags {
>> _IO(SGX_MAGIC, 0x04)
>> #define SGX_IOC_PAGE_MODP \
>> _IOWR(SGX_MAGIC, 0x05, struct sgx_page_modp)
>> +#define SGX_IOC_PAGE_MODT \
>> + _IOWR(SGX_MAGIC, 0x06, struct sgx_page_modt)
>
> I'd suggest to change this as SGX_IOC_ENCLAVE_MODIFY_TYPE.

How about SGX_IOC_ENCLAVE_MOD_TYPE to be consistent with your earlier
suggestion of SGX_IOC_ENCLAVE_MOD_PROTECTIONS ?

Reinette

\
 
 \ /
  Last update: 2021-12-06 22:49    [W:0.163 / U:0.096 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site