> +config INTEL_SGX_CORE
> + prompt "Intel SGX core functionality
> + depends on X86_64 && CPU_SUP_INTEL
> + help
> + Intel Software Guard eXtensions (SGX) is a set of CPU instructions
> + that allows ring 3 applications to create enclaves; private regions
> + of memory that are protected, by hardware, from unauthorized access
> + and/or modification.
That semicolon needs to be a colon. The second half of that sentence is
not a stand-alone statement.
> + This option enables kernel recognition of SGX, high-level management
> + of the Enclave Page Cache (EPC), tracking and writing of SGX Launch
> + Enclave Hash MSRs, and allows for virtualization of SGX via KVM. By
> + iteslf, this option does not provide SGX support to userspace.
> +
> + For details, see Documentation/x86/intel_sgx.rst
> +
> + If unsure, say N.
> +
> config EFI
> bool "EFI runtime service support"
> depends on ACPI
> diff --git a/arch/x86/include/asm/sgx.h b/arch/x86/include/asm/sgx.h
> new file mode 100644
> index 000000000000..fa3e6e0eb8af
> --- /dev/null
> +++ b/arch/x86/include/asm/sgx.h
> @@ -0,0 +1,25 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
> +// Copyright(c) 2016-18 Intel Corporation.
> +//
> +// Authors:
> +//
> +// Jarkko Sakkinen <jarkko.sakkinen@xxxxxxxxxxxxxxx>
> +// Suresh Siddha <suresh.b.siddha@xxxxxxxxx>
> +// Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
> +
> +#ifndef _ASM_X86_SGX_H
> +#define _ASM_X86_SGX_H
> +
> +#include <linux/types.h>
> +
> +#define SGX_CPUID 0x12
Hey, I just saw 0x12 as a magic, hard-coded number earlier in these
patches. It seems cruel to hard-code it, and then also have a #define
that isn't used.
> +enum sgx_cpuid {
> + SGX_CPUID_CAPABILITIES = 0,
> + SGX_CPUID_ATTRIBUTES = 1,
> + SGX_CPUID_EPC_BANKS = 2,
> +};
These are cpuid *leaves*, right? Please make this clear that these are
hardware-defined values and not some kind of software construct.
> +bool sgx_enabled __ro_after_init = false;
> +EXPORT_SYMBOL(sgx_enabled);
> +
> +static __init bool sgx_is_enabled(void)
> +{
> + unsigned long fc;
> +
> + if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
> + return false;
Not necessary. CPUID does this part for you.
> + if (!boot_cpu_has(X86_FEATURE_SGX))
> + return false;
> +
> + if (!boot_cpu_has(X86_FEATURE_SGX1))
> + return false;
> +
> + rdmsrl(MSR_IA32_FEATURE_CONTROL, fc);
> + if (!(fc & FEATURE_CONTROL_LOCKED))
> + return false;
> +
> + if (!(fc & FEATURE_CONTROL_SGX_ENABLE))
> + return false;
Comments, please. Why would this happen? What would it mean?
