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
