Re: [PATCH v30 08/20] x86/sgx: Add functions to allocate and free EPC pages

[Borislav Petkov <bp@alien8.de>]

Lemme reply to all mails with one. :-)

I think Sean almost had it:

>  sgx_alloc_epc_section -> sgx_map_epc_section
>  sgx_free_epc_section  -> sgx_unmap_epc_section

Or even

  sgx_setup_epc_section()
  sgx_free_epc_section()

>  sgx_alloc_page        -> sgx_alloc_epc_page
>  sgx_free_page         -> sgx_free_epc_page
>
>  sgx_try_alloc_page    -> sgx_alloc_epc_page_node
>  __sgx_try_alloc_page  -> sgx_alloc_epc_page_section

And except those last two. Those are allocating a page from the EPC
sections so I'd call them:

sgx_try_alloc_page    -> sgx_alloc_epc_page_section
__sgx_try_alloc_page  -> __sgx_alloc_epc_page_section

former doing the loop, latter doing the per-section list games.

> I'm not sure I follow fully Sean's reasoning but the way alloc is used
> mostly in Linux is to ask through some API the used kernel memory
> allocator to give memory for some kernel data structures.
>
> Agreed that it is not the best match on what we are doing.

Yes, ok, let's stay with "alloc". Agreed.

> I'm going with sgx_grab_page() and sgx_try_grab_page().

And let's simply forget the "try" - all the functions that can fail and
return an error code, they all try. :-)

> So sgx_alloc_epc_section() is most likely going to be read as "SGX,
> allocate an EPC section".

Or, allocate *from* the EPC section if it returns a pointer to a page
and the comment above it says so...

So to sum up:

* sgx_{alloc,setup}_epc_section - sets up the EPC section and the pages
which belong to it.

* sgx_alloc_page - allocates an EPC page

* sgx_alloc_epc_page_section - allocates a page from any EPC section

I think that makes it pretty clear what each function does...

> I'm thinking that you are over-engineering something this :-) Naming is
> never perfect.

I know, but naming stuff right will pay off later.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette