lkml.org 
[lkml]   [2021]   [Dec]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 01/25] x86/sgx: Add shortlog descriptions to ENCLS wrappers
From
Date
On Mon, 2021-12-06 at 13:13 -0800, Reinette Chatre wrote:
> > * "Create an SECS page in the Enclave Page Cache (EPC)"
> > * "Add a Version Array (VA) page to the Enclave Page Cache (EPC)"
> >
> > They should have similar descriptions, e.g.
> >
> > * "Initialize an EPC page into SGX Enclave Control Structure (SECS) page."
> > * "Initialize an EPC page into Version Array (VA) page."
>
> Will do. Did you intentionally omit the articles or would you be ok if I
> change it to:
>
> "Initialize an EPC page into an SGX Enclave Control Structure (SECS) page."
> "Initialize an EPC page into a Version Array (VA) page."
>
> I also notice that you prefer the comments to end with a period and I
> will do so for all in the next version.

Looks fine to me.

> > > +/* Extend uninitialized enclave measurement */
> > > static inline int __eextend(void *secs, void *addr)
> > > {
> > > return __encls_2(EEXTEND, secs, addr);
> > > }
> >
> > That description does not make __eextend any less cryptic.
> >
> > Something like this would be already more informative:
> >
> > /* Hash a 256 byte region of an enclave page to SECS:MRENCLAVE. */
>
> Thank you, I will use this description.
>
> >
> > This same remark applies to the rest of these comments. They should
> > provide a clue what the wrapper does rather than an English open coded
> > function name.
>
> Please see below for another attempt that includes your proposed changes
> so far. What do you think?
>
> __ecreate():
> /* Initialize an EPC page into an SGX Enclave Control Structure (SECS)
> page. */
>
> __eextend():
> /* Hash a 256 byte region of an enclave page to SECS:MRENCLAVE. */
>
> __eadd():
> /* Copy a source page from non-enclave memory into the EPC. */

Perhaps:

/* 
* Associate an EPC page to an enclave either as a REG or TCS page
* populated with the provided data.
*/

This is more aligned with your description for __eremove().

>
> __einit():
> /* Finalize enclave build, initialize enclave for user code execution */
>
> __eremove():
> /* Disassociate EPC page from its enclave and mark it as unused. */
>
> __edbgwr():
> /* Copy data to an EPC page belonging to a debug enclave. */
>
> __edbgrd():
> /* Copy data from an EPC page belonging to a debug enclave. */
>
> __etrack():
> /* Track that software has completed the required TLB address clears. */
>
> __eldu():
> /* Load, verify, and unblock an Enclave Page Cache (EPC) page. */
>
> __eblock():
> /* Make EPC page inaccessible to enclave, ready to be written to memory. */
>
> __epa():
> /* Initialize an EPC page into a Version Array (VA) page. */
>
> __ewb():
> /* Invalidate an EPC page and write it out to main memory. */
>
>
> Reinette

/Jarkko

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