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:3.321 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site