lkml.org 
[lkml]   [2018]   [Jul]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH v12 08/13] x86/sgx: wrappers for ENCLS opcode leaf functions
On Tue, 3 Jul 2018, Jarkko Sakkinen wrote:

> This commit adds wrappers for Intel(R) SGX ENCLS opcode leaf functions

Add...

> except for ENCLS(EINIT). The ENCLS instruction invokes the privileged
> functions for managing (creation, initialization and swapping) and
> debugging enclaves.
>
> +#define IS_ENCLS_FAULT(r) ((r) & 0xffff0000)
> +#define ENCLS_FAULT_VECTOR(r) ((r) >> 16)
> +
> +#define ENCLS_TO_ERR(r) (IS_ENCLS_FAULT(r) ? -EFAULT : \
> + (r) == SGX_UNMASKED_EVENT ? -EINTR : \
> + (r) == SGX_MAC_COMPARE_FAIL ? -EIO : \
> + (r) == SGX_ENTRYEPOCH_LOCKED ? -EBUSY : -EPERM)

Inlines please along with proper comments.

> +#define __encls_ret_N(rax, inputs...) \
> + ({ \
> + int ret; \
> + asm volatile( \
> + "1: .byte 0x0f, 0x01, 0xcf;\n\t" \
> + "2:\n" \
> + ".section .fixup,\"ax\"\n" \
> + "3: shll $16,%%eax\n" \

SHLL ??? _All_ the macro maze needs proper comments.

> + " jmp 2b\n" \
> + ".previous\n" \
> + _ASM_EXTABLE_FAULT(1b, 3b) \
> + : "=a"(ret) \
> + : "a"(rax), inputs \
> + : "memory"); \
> + ret; \
> + })

....

> +static inline int __emodt(struct sgx_secinfo *secinfo, void *epc)
> +{
> + return __encls_ret_2(EMODT, secinfo, epc);
> +}
> +
> #define SGX_MAX_EPC_BANKS 8
>
> #define SGX_EPC_BANK(epc_page) \
> @@ -39,4 +190,29 @@ extern bool sgx_lc_enabled;
> void *sgx_get_page(struct sgx_epc_page *ptr);
> void sgx_put_page(void *epc_page_ptr);

> +#define SGX_FN(name, params...) \
> +{ \
> + void *epc; \
> + int ret; \
> + epc = sgx_get_page(epc_page); \
> + ret = __##name(params); \
> + sgx_put_page(epc); \

This whole get/put magic is totally pointless. The stuff is 64bit only, so
all it needs is the address, because 'put' is a noop on 64bit.

> + return ret; \
> +}
> +
> +#define BUILD_SGX_FN(fn, name) \
> +static inline int fn(struct sgx_epc_page *epc_page) \
> + SGX_FN(name, epc)
> +BUILD_SGX_FN(sgx_eremove, eremove)
> +BUILD_SGX_FN(sgx_eblock, eblock)
> +BUILD_SGX_FN(sgx_etrack, etrack)
> +BUILD_SGX_FN(sgx_epa, epa)
> +
> +static inline int sgx_emodpr(struct sgx_secinfo *secinfo,
> + struct sgx_epc_page *epc_page)
> + SGX_FN(emodpr, secinfo, epc)
> +static inline int sgx_emodt(struct sgx_secinfo *secinfo,
> + struct sgx_epc_page *epc_page)
> + SGX_FN(emodt, secinfo, epc)

Bah this is really unreadable crap. What's so horribly wrong with writing
this simply as:
static inline int sgx_eremove(struct sgx_epc_page *epc_page)
{
return __encls_ret_1(EREMOVE, epc_page_addr(epc_page));
}
static inline int sgx_emodt(struct sgx_secinfo *secinfo,
struct sgx_epc_page *epc_page)
{
return __encls_ret_2(EREMOVE, secinfo, epc_page_addr(epc_page));
}
instead of all these completely pointless meta functions and build macro
maze around it.

Why? Because then every function which is actually used in code has a
proper prototype instead of nongrepable magic and a gazillion of wrappers.

Thanks,

tglx

\
 
 \ /
  Last update: 2018-07-03 22:17    [W:0.226 / U:0.076 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site