Re: [PATCH 02/25] x86/sgx: Add wrappers for SGX2 functions

[Jarkko Sakkinen]

On Wed, Dec 01, 2021 at 11:23:00AM -0800, Reinette Chatre wrote:
> The SGX ENCLS instruction uses EAX to specify an SGX function and
> may require additional registers, depending on the SGX function.
> ENCLS invokes the specified privileged SGX function for managing
> and debugging enclaves. Several macros are used to wrap the ENCLS
> functionality.
> 
> Add ENCLS wrappers for the SGX2 EMODPR, EMODT, and EAUG functions
> that can make changes to pages of an initialized SGX enclave. The
> EMODPR function is used to restrict enclave page permissions
> as maintained within the enclave (Enclave Page Cache Map (EPCM)
> permissions). The EMODT function is used to change the type of an
> enclave page. The EAUG function is used to dynamically add enclave
> pages to an initialized enclave.
> 
> EMODPR and EMODT accepts two parameters and can fault as well as return
> an SGX error code. EAUG also accepts two parameters but does not return
> an SGX error code. Use existing macros for all new functions.
> 
> Expand enum sgx_return_code with the possible EMODPR and EMODT
> return codes.

These implementation details only obfuscate this commit message, and
it is way too high-level to be useful e.g. for kernel maintenance.

I'd replace it with something like:

"
Add wrappers for ENCLS leaf functions EAUG, EMODT and EMODPR,
which roughly take two steps:

1. EAUG creates a new EPCM entry.
   EMODT and EMODPR modify an existing EPCM entry.
2. Set either .PR = 1 (EMODPR), .MODIFY = 1 (EMODT) or .PENDING = 1 (AUG).

The bit is reset by the enclave by invoking ENCLU leaf function EACCEPT
or EACCEPTCOPY, which will result the EPCM change becoming effective.
"

The current commit message is also not addressing these:

1. What happens if enclaves accesses a memory address with either .PR,
   .MODIFY or .PENDING set in EPCM, other than by the means of EACCEPT
   or EACCEPTCOPY?
2. The calling conditions (e.g. concerning TLB's and ETRACK/IPI/etc
   dance related to it).


If this information was properly contained here, discussing about the
following commits would be much easier.

/Jarkko