Message ID | 20201003045059.665934-6-jarkko.sakkinen@linux.intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Intel SGX foundations | expand |
On 10/2/20 9:50 PM, Jarkko Sakkinen wrote: > +/** > + * encls_failed() - Check if an ENCLS leaf function failed > + * @ret: the return value of an ENCLS leaf function call > + * > + * Check if an ENCLS leaf function failed. This happens when the leaf function > + * causes a fault that is not caused by an EPCM conflict or when the leaf > + * function returns a non-zero value. > + */ > +static inline bool encls_failed(int ret) > +{ > + int epcm_trapnr; > + > + if (boot_cpu_has(X86_FEATURE_SGX2)) > + epcm_trapnr = X86_TRAP_PF; > + else > + epcm_trapnr = X86_TRAP_GP; So, the SDM makes it sound like the only thing that changes from SGX1->SGX2 is the ENCLS leafs supported. Since the kernel doesn't use any SGX2 leaf functions, this would imply there is some other architecture change which is visible. *But* I don't see any evidence of this in the SDM, at least from a quick scan. Why is this here? > + if (ret & ENCLS_FAULT_FLAG) > + return ENCLS_TRAPNR(ret) != epcm_trapnr; > + > + return !!ret; > +}
On Mon, Oct 19, 2020 at 07:30:32AM -0700, Dave Hansen wrote: > On 10/2/20 9:50 PM, Jarkko Sakkinen wrote: > > +/** > > + * encls_failed() - Check if an ENCLS leaf function failed > > + * @ret: the return value of an ENCLS leaf function call > > + * > > + * Check if an ENCLS leaf function failed. This happens when the leaf function > > + * causes a fault that is not caused by an EPCM conflict or when the leaf > > + * function returns a non-zero value. > > + */ > > +static inline bool encls_failed(int ret) > > +{ > > + int epcm_trapnr; > > + > > + if (boot_cpu_has(X86_FEATURE_SGX2)) > > + epcm_trapnr = X86_TRAP_PF; > > + else > > + epcm_trapnr = X86_TRAP_GP; > > So, the SDM makes it sound like the only thing that changes from > SGX1->SGX2 is the ENCLS leafs supported. Since the kernel doesn't use > any SGX2 leaf functions, this would imply there is some other > architecture change which is visible. *But* I don't see any evidence of > this in the SDM, at least from a quick scan. > > Why is this here? SGX1 CPUs take an erratum on the #PF behavior, e.g. "KBW90 Violation of Intel SGX Access-Control Requirements Produce #GP Instead of #PF". https://www.intel.com/content/dam/www/public/us/en/documents/specification-updates/xeon-e3-1200v6-spec-update.pdf > > + if (ret & ENCLS_FAULT_FLAG) > > + return ENCLS_TRAPNR(ret) != epcm_trapnr; > > + > > + return !!ret; > > +} > >
On 10/19/20 10:38 AM, Sean Christopherson wrote: >>> +static inline bool encls_failed(int ret) >>> +{ >>> + int epcm_trapnr; >>> + >>> + if (boot_cpu_has(X86_FEATURE_SGX2)) >>> + epcm_trapnr = X86_TRAP_PF; >>> + else >>> + epcm_trapnr = X86_TRAP_GP; >> So, the SDM makes it sound like the only thing that changes from >> SGX1->SGX2 is the ENCLS leafs supported. Since the kernel doesn't use >> any SGX2 leaf functions, this would imply there is some other >> architecture change which is visible. *But* I don't see any evidence of >> this in the SDM, at least from a quick scan. >> >> Why is this here? > SGX1 CPUs take an erratum on the #PF behavior, e.g. "KBW90 Violation of Intel > SGX Access-Control Requirements Produce #GP Instead of #PF". > > https://www.intel.com/content/dam/www/public/us/en/documents/specification-updates/xeon-e3-1200v6-spec-update.pdf OK, but that's only for "Intel ® Xeon ® E3-1200 v6 Processor Family", specifically stepping B-0. That's far from a broad erratum. I *see* it in other errata lists, but I still think this is too broad. Also, what if a hypervisor masks the SGX2 cpuid bit on SGX2-capable hardware? Won't the hardware still exhibit the erratum? I don't think we can control model-specific errata behavior with an architectural CPUID bit.
On Mon, Oct 19, 2020 at 10:48:35AM -0700, Dave Hansen wrote: > On 10/19/20 10:38 AM, Sean Christopherson wrote: > >>> +static inline bool encls_failed(int ret) > >>> +{ > >>> + int epcm_trapnr; > >>> + > >>> + if (boot_cpu_has(X86_FEATURE_SGX2)) > >>> + epcm_trapnr = X86_TRAP_PF; > >>> + else > >>> + epcm_trapnr = X86_TRAP_GP; > >> So, the SDM makes it sound like the only thing that changes from > >> SGX1->SGX2 is the ENCLS leafs supported. Since the kernel doesn't use > >> any SGX2 leaf functions, this would imply there is some other > >> architecture change which is visible. *But* I don't see any evidence of > >> this in the SDM, at least from a quick scan. > >> > >> Why is this here? > > SGX1 CPUs take an erratum on the #PF behavior, e.g. "KBW90 Violation of Intel > > SGX Access-Control Requirements Produce #GP Instead of #PF". > > > > https://www.intel.com/content/dam/www/public/us/en/documents/specification-updates/xeon-e3-1200v6-spec-update.pdf > > OK, but that's only for "Intel ® Xeon ® E3-1200 v6 Processor Family", > specifically stepping B-0. That's far from a broad erratum. I *see* it > in other errata lists, but I still think this is too broad. > > Also, what if a hypervisor masks the SGX2 cpuid bit on SGX2-capable > hardware? Won't the hardware still exhibit the erratum? > > I don't think we can control model-specific errata behavior with an > architectural CPUID bit. Hmm, true. Checking for #PF _or_ #GP on SGX1 CPUs would be my first choice. ENCLS #GPs for other reasons, most of which would indicate a kernel bug. It'd be nice to limit the "#GP is expected, sort of" behavior to CPUs that might be affected by an erratum.
On 10/19/20 10:53 AM, Sean Christopherson wrote: >>> SGX1 CPUs take an erratum on the #PF behavior, e.g. "KBW90 Violation of Intel >>> SGX Access-Control Requirements Produce #GP Instead of #PF". >>> >>> https://www.intel.com/content/dam/www/public/us/en/documents/specification-updates/xeon-e3-1200v6-spec-update.pdf >> OK, but that's only for "Intel ® Xeon ® E3-1200 v6 Processor Family", >> specifically stepping B-0. That's far from a broad erratum. I *see* it >> in other errata lists, but I still think this is too broad. >> >> Also, what if a hypervisor masks the SGX2 cpuid bit on SGX2-capable >> hardware? Won't the hardware still exhibit the erratum? >> >> I don't think we can control model-specific errata behavior with an >> architectural CPUID bit. > Hmm, true. Checking for #PF _or_ #GP on SGX1 CPUs would be my first choice. > ENCLS #GPs for other reasons, most of which would indicate a kernel bug. It'd > be nice to limit the "#GP is expected, sort of" behavior to CPUs that might be > affected by an erratum. Yes, agreed. We need a model/family/stepping list of all the affected CPUs, and a normal old match_cpu() or whatever. If a hypervisor lies about model/family/stepping, then the fallout is on them, not the guest.
diff --git a/arch/x86/kernel/cpu/sgx/encls.h b/arch/x86/kernel/cpu/sgx/encls.h new file mode 100644 index 000000000000..a87f15ea5cca --- /dev/null +++ b/arch/x86/kernel/cpu/sgx/encls.h @@ -0,0 +1,238 @@ +/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) */ +#ifndef _X86_ENCLS_H +#define _X86_ENCLS_H + +#include <linux/bitops.h> +#include <linux/err.h> +#include <linux/io.h> +#include <linux/rwsem.h> +#include <linux/types.h> +#include <asm/asm.h> +#include <asm/traps.h> +#include "sgx.h" + +enum sgx_encls_leaf { + ECREATE = 0x00, + EADD = 0x01, + EINIT = 0x02, + EREMOVE = 0x03, + EDGBRD = 0x04, + EDGBWR = 0x05, + EEXTEND = 0x06, + ELDU = 0x08, + EBLOCK = 0x09, + EPA = 0x0A, + EWB = 0x0B, + ETRACK = 0x0C, +}; + +/** + * ENCLS_FAULT_FLAG - flag signifying an ENCLS return code is a trapnr + * + * ENCLS has its own (positive value) error codes and also generates + * ENCLS specific #GP and #PF faults. And the ENCLS values get munged + * with system error codes as everything percolates back up the stack. + * Unfortunately (for us), we need to precisely identify each unique + * error code, e.g. the action taken if EWB fails varies based on the + * type of fault and on the exact SGX error code, i.e. we can't simply + * convert all faults to -EFAULT. + * + * To make all three error types coexist, we set bit 30 to identify an + * ENCLS fault. Bit 31 (technically bits N:31) is used to differentiate + * between positive (faults and SGX error codes) and negative (system + * error codes) values. + */ +#define ENCLS_FAULT_FLAG 0x40000000 + +/* Retrieve the encoded trapnr from the specified return code. */ +#define ENCLS_TRAPNR(r) ((r) & ~ENCLS_FAULT_FLAG) + +/* Issue a WARN() about an ENCLS leaf. */ +#define ENCLS_WARN(r, name) { \ + do { \ + int _r = (r); \ + WARN_ONCE(_r, "%s returned %d (0x%x)\n", (name), _r, _r); \ + } while (0); \ +} + +/** + * encls_failed() - Check if an ENCLS leaf function failed + * @ret: the return value of an ENCLS leaf function call + * + * Check if an ENCLS leaf function failed. This happens when the leaf function + * causes a fault that is not caused by an EPCM conflict or when the leaf + * function returns a non-zero value. + */ +static inline bool encls_failed(int ret) +{ + int epcm_trapnr; + + if (boot_cpu_has(X86_FEATURE_SGX2)) + epcm_trapnr = X86_TRAP_PF; + else + epcm_trapnr = X86_TRAP_GP; + + if (ret & ENCLS_FAULT_FLAG) + return ENCLS_TRAPNR(ret) != epcm_trapnr; + + return !!ret; +} + +/** + * __encls_ret_N - encode an ENCLS leaf that returns an error code in EAX + * @rax: leaf number + * @inputs: asm inputs for the leaf + * + * Emit assembly for an ENCLS leaf that returns an error code, e.g. EREMOVE. + * And because SGX isn't complex enough as it is, leafs that return an error + * code also modify flags. + * + * Return: + * 0 on success, + * SGX error code on failure + */ +#define __encls_ret_N(rax, inputs...) \ + ({ \ + int ret; \ + asm volatile( \ + "1: .byte 0x0f, 0x01, 0xcf;\n\t" \ + "2:\n" \ + ".section .fixup,\"ax\"\n" \ + "3: orl $"__stringify(ENCLS_FAULT_FLAG)",%%eax\n" \ + " jmp 2b\n" \ + ".previous\n" \ + _ASM_EXTABLE_FAULT(1b, 3b) \ + : "=a"(ret) \ + : "a"(rax), inputs \ + : "memory", "cc"); \ + ret; \ + }) + +#define __encls_ret_1(rax, rcx) \ + ({ \ + __encls_ret_N(rax, "c"(rcx)); \ + }) + +#define __encls_ret_2(rax, rbx, rcx) \ + ({ \ + __encls_ret_N(rax, "b"(rbx), "c"(rcx)); \ + }) + +#define __encls_ret_3(rax, rbx, rcx, rdx) \ + ({ \ + __encls_ret_N(rax, "b"(rbx), "c"(rcx), "d"(rdx)); \ + }) + +/** + * __encls_N - encode an ENCLS leaf that doesn't return an error code + * @rax: leaf number + * @rbx_out: optional output variable + * @inputs: asm inputs for the leaf + * + * Emit assembly for an ENCLS leaf that does not return an error code, + * e.g. ECREATE. Leaves without error codes either succeed or fault. + * @rbx_out is an optional parameter for use by EDGBRD, which returns + * the requested value in RBX. + * + * Return: + * 0 on success, + * trapnr with ENCLS_FAULT_FLAG set on fault + */ +#define __encls_N(rax, rbx_out, inputs...) \ + ({ \ + int ret; \ + asm volatile( \ + "1: .byte 0x0f, 0x01, 0xcf;\n\t" \ + " xor %%eax,%%eax;\n" \ + "2:\n" \ + ".section .fixup,\"ax\"\n" \ + "3: orl $"__stringify(ENCLS_FAULT_FLAG)",%%eax\n" \ + " jmp 2b\n" \ + ".previous\n" \ + _ASM_EXTABLE_FAULT(1b, 3b) \ + : "=a"(ret), "=b"(rbx_out) \ + : "a"(rax), inputs \ + : "memory"); \ + ret; \ + }) + +#define __encls_2(rax, rbx, rcx) \ + ({ \ + unsigned long ign_rbx_out; \ + __encls_N(rax, ign_rbx_out, "b"(rbx), "c"(rcx)); \ + }) + +#define __encls_1_1(rax, data, rcx) \ + ({ \ + unsigned long rbx_out; \ + int ret = __encls_N(rax, rbx_out, "c"(rcx)); \ + if (!ret) \ + data = rbx_out; \ + ret; \ + }) + +static inline int __ecreate(struct sgx_pageinfo *pginfo, void *secs) +{ + return __encls_2(ECREATE, pginfo, secs); +} + +static inline int __eextend(void *secs, void *addr) +{ + return __encls_2(EEXTEND, secs, addr); +} + +static inline int __eadd(struct sgx_pageinfo *pginfo, void *addr) +{ + return __encls_2(EADD, pginfo, addr); +} + +static inline int __einit(void *sigstruct, void *token, void *secs) +{ + return __encls_ret_3(EINIT, sigstruct, secs, token); +} + +static inline int __eremove(void *addr) +{ + return __encls_ret_1(EREMOVE, addr); +} + +static inline int __edbgwr(void *addr, unsigned long *data) +{ + return __encls_2(EDGBWR, *data, addr); +} + +static inline int __edbgrd(void *addr, unsigned long *data) +{ + return __encls_1_1(EDGBRD, *data, addr); +} + +static inline int __etrack(void *addr) +{ + return __encls_ret_1(ETRACK, addr); +} + +static inline int __eldu(struct sgx_pageinfo *pginfo, void *addr, + void *va) +{ + return __encls_ret_3(ELDU, pginfo, addr, va); +} + +static inline int __eblock(void *addr) +{ + return __encls_ret_1(EBLOCK, addr); +} + +static inline int __epa(void *addr) +{ + unsigned long rbx = SGX_PAGE_TYPE_VA; + + return __encls_2(EPA, rbx, addr); +} + +static inline int __ewb(struct sgx_pageinfo *pginfo, void *addr, + void *va) +{ + return __encls_ret_3(EWB, pginfo, addr, va); +} + +#endif /* _X86_ENCLS_H */