diff mbox series

[v21,08/28] x86/cpu/intel: Detect SGX support and update caps appropriately

Message ID 20190713170804.2340-9-jarkko.sakkinen@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series Intel SGX foundations | expand

Commit Message

Jarkko Sakkinen July 13, 2019, 5:07 p.m. UTC 
From: Sean Christopherson <sean.j.christopherson@intel.com>

Similar to other large Intel features such as VMX and TXT, SGX must be
explicitly enabled in IA32_FEATURE_CONTROL MSR to be truly usable.
Clear all SGX related capabilities if SGX is not fully enabled in
IA32_FEATURE_CONTROL or if the SGX1 instruction set isn't supported
(impossible on bare metal, theoretically possible in a VM if the VMM is
doing something weird).

Like SGX itself, SGX Launch Control must be explicitly enabled via a
flag in IA32_FEATURE_CONTROL. Clear the SGX_LC capability if Launch
Control is not fully enabled (or obviously if SGX itself is disabled).

Note that clearing X86_FEATURE_SGX_LC creates a bit of a conundrum
regarding the SGXLEPUBKEYHASH MSRs, as it may be desirable to read the
MSRs even if they are not writable, e.g. to query the configured key,
but clearing the capability leaves no breadcrum for discerning whether
or not the MSRs exist.  But, such usage will be rare (KVM is the only
known case at this time) and not performance critical, so it's not
unreasonable to require the use of rdmsr_safe().  Clearing the cap bit
eliminates the need for an additional flag to track whether or not
Launch Control is truly enabled, which is what we care about the vast
majority of the time.

Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Co-developed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 arch/x86/kernel/cpu/intel.c | 71 +++++++++++++++++++++++++++++++++++++
 1 file changed, 71 insertions(+)

Comments

Sean Christopherson July 24, 2019, 7:35 p.m. UTC | #1 
On Sat, Jul 13, 2019 at 08:07:44PM +0300, Jarkko Sakkinen wrote:
>  arch/x86/kernel/cpu/intel.c | 71 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 71 insertions(+)
> 
> diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
> index 8d6d92ebeb54..1503b251d10f 100644
> --- a/arch/x86/kernel/cpu/intel.c
> +++ b/arch/x86/kernel/cpu/intel.c
> @@ -623,6 +623,72 @@ static void detect_tme(struct cpuinfo_x86 *c)
>  	c->x86_phys_bits -= keyid_bits;
>  }
>  
> +static void __maybe_unused detect_sgx(struct cpuinfo_x86 *c)
> +{
> +	unsigned long long fc;
> +
> +	rdmsrl(MSR_IA32_FEATURE_CONTROL, fc);
> +	if (!(fc & FEATURE_CONTROL_LOCKED)) {
> +		pr_err_once("sgx: The feature control MSR is not locked\n");
> +		goto err_unsupported;
> +	}
> +
> +	if (!(fc & FEATURE_CONTROL_SGX_ENABLE)) {
> +		pr_err_once("sgx: SGX is not enabled in IA32_FEATURE_CONTROL MSR\n");
> +		goto err_unsupported;
> +	}
> +
> +	if (!cpu_has(c, X86_FEATURE_SGX1)) {
> +		pr_err_once("sgx: SGX1 instruction set is not supported\n");
> +		goto err_unsupported;
> +	}
> +
> +	if (!(fc & FEATURE_CONTROL_SGX_LE_WR)) {
> +		pr_info_once("sgx: The launch control MSRs are not writable\n");
> +		goto err_msrs_rdonly;
> +	}
> +
> +	return;
> +
> +err_unsupported:
> +	setup_clear_cpu_cap(X86_FEATURE_SGX);
> +	setup_clear_cpu_cap(X86_FEATURE_SGX1);
> +	setup_clear_cpu_cap(X86_FEATURE_SGX2);
> +
> +err_msrs_rdonly:
> +	setup_clear_cpu_cap(X86_FEATURE_SGX_LC);
> +}
> +
> +static void init_intel_energy_perf(struct cpuinfo_x86 *c)
> +{
> +	u64 epb;
> +
> +	/*
> +	 * Initialize MSR_IA32_ENERGY_PERF_BIAS if not already initialized.
> +	 * (x86_energy_perf_policy(8) is available to change it at run-time.)
> +	 */
> +	if (!cpu_has(c, X86_FEATURE_EPB))
> +		return;
> +
> +	rdmsrl(MSR_IA32_ENERGY_PERF_BIAS, epb);
> +	if ((epb & 0xF) != ENERGY_PERF_BIAS_PERFORMANCE)
> +		return;
> +
> +	pr_warn_once("ENERGY_PERF_BIAS: Set to 'normal', was 'performance'\n");
> +	pr_warn_once("ENERGY_PERF_BIAS: View and update with x86_energy_perf_policy(8)\n");
> +	epb = (epb & ~0xF) | ENERGY_PERF_BIAS_NORMAL;
> +	wrmsrl(MSR_IA32_ENERGY_PERF_BIAS, epb);
> +}
> +
> +static void intel_bsp_resume(struct cpuinfo_x86 *c)
> +{
> +	/*
> +	 * MSR_IA32_ENERGY_PERF_BIAS is lost across suspend/resume,
> +	 * so reinitialize it properly like during bootup:
> +	 */
> +	init_intel_energy_perf(c);
> +}
> +
>  static void init_cpuid_fault(struct cpuinfo_x86 *c)
>  {
>  	u64 msr;
> @@ -760,6 +826,11 @@ static void init_intel(struct cpuinfo_x86 *c)
>  	if (cpu_has(c, X86_FEATURE_TME))
>  		detect_tme(c);
>  
> +	if (IS_ENABLED(CONFIG_INTEL_SGX) && cpu_has(c, X86_FEATURE_SGX))
> +		detect_sgx(c);
> +
> +	init_intel_energy_perf(c);

All of the energy_perf additions are bogus, looks like a rebase gone wrong.

> +
>  	init_intel_misc_features(c);
>  }
>  
> -- 
> 2.20.1
>
Jarkko Sakkinen Aug. 2, 2019, 8:48 p.m. UTC | #2 
On Wed, Jul 24, 2019 at 12:35:42PM -0700, Sean Christopherson wrote:
> On Sat, Jul 13, 2019 at 08:07:44PM +0300, Jarkko Sakkinen wrote:
> >  arch/x86/kernel/cpu/intel.c | 71 +++++++++++++++++++++++++++++++++++++
> >  1 file changed, 71 insertions(+)
> > 
> > diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
> > index 8d6d92ebeb54..1503b251d10f 100644
> > --- a/arch/x86/kernel/cpu/intel.c
> > +++ b/arch/x86/kernel/cpu/intel.c
> > @@ -623,6 +623,72 @@ static void detect_tme(struct cpuinfo_x86 *c)
> >  	c->x86_phys_bits -= keyid_bits;
> >  }
> >  
> > +static void __maybe_unused detect_sgx(struct cpuinfo_x86 *c)
> > +{
> > +	unsigned long long fc;
> > +
> > +	rdmsrl(MSR_IA32_FEATURE_CONTROL, fc);
> > +	if (!(fc & FEATURE_CONTROL_LOCKED)) {
> > +		pr_err_once("sgx: The feature control MSR is not locked\n");
> > +		goto err_unsupported;
> > +	}
> > +
> > +	if (!(fc & FEATURE_CONTROL_SGX_ENABLE)) {
> > +		pr_err_once("sgx: SGX is not enabled in IA32_FEATURE_CONTROL MSR\n");
> > +		goto err_unsupported;
> > +	}
> > +
> > +	if (!cpu_has(c, X86_FEATURE_SGX1)) {
> > +		pr_err_once("sgx: SGX1 instruction set is not supported\n");
> > +		goto err_unsupported;
> > +	}
> > +
> > +	if (!(fc & FEATURE_CONTROL_SGX_LE_WR)) {
> > +		pr_info_once("sgx: The launch control MSRs are not writable\n");
> > +		goto err_msrs_rdonly;
> > +	}
> > +
> > +	return;
> > +
> > +err_unsupported:
> > +	setup_clear_cpu_cap(X86_FEATURE_SGX);
> > +	setup_clear_cpu_cap(X86_FEATURE_SGX1);
> > +	setup_clear_cpu_cap(X86_FEATURE_SGX2);
> > +
> > +err_msrs_rdonly:
> > +	setup_clear_cpu_cap(X86_FEATURE_SGX_LC);
> > +}
> > +
> > +static void init_intel_energy_perf(struct cpuinfo_x86 *c)
> > +{
> > +	u64 epb;
> > +
> > +	/*
> > +	 * Initialize MSR_IA32_ENERGY_PERF_BIAS if not already initialized.
> > +	 * (x86_energy_perf_policy(8) is available to change it at run-time.)
> > +	 */
> > +	if (!cpu_has(c, X86_FEATURE_EPB))
> > +		return;
> > +
> > +	rdmsrl(MSR_IA32_ENERGY_PERF_BIAS, epb);
> > +	if ((epb & 0xF) != ENERGY_PERF_BIAS_PERFORMANCE)
> > +		return;
> > +
> > +	pr_warn_once("ENERGY_PERF_BIAS: Set to 'normal', was 'performance'\n");
> > +	pr_warn_once("ENERGY_PERF_BIAS: View and update with x86_energy_perf_policy(8)\n");
> > +	epb = (epb & ~0xF) | ENERGY_PERF_BIAS_NORMAL;
> > +	wrmsrl(MSR_IA32_ENERGY_PERF_BIAS, epb);
> > +}
> > +
> > +static void intel_bsp_resume(struct cpuinfo_x86 *c)
> > +{
> > +	/*
> > +	 * MSR_IA32_ENERGY_PERF_BIAS is lost across suspend/resume,
> > +	 * so reinitialize it properly like during bootup:
> > +	 */
> > +	init_intel_energy_perf(c);
> > +}
> > +
> >  static void init_cpuid_fault(struct cpuinfo_x86 *c)
> >  {
> >  	u64 msr;
> > @@ -760,6 +826,11 @@ static void init_intel(struct cpuinfo_x86 *c)
> >  	if (cpu_has(c, X86_FEATURE_TME))
> >  		detect_tme(c);
> >  
> > +	if (IS_ENABLED(CONFIG_INTEL_SGX) && cpu_has(c, X86_FEATURE_SGX))
> > +		detect_sgx(c);
> > +
> > +	init_intel_energy_perf(c);
> 
> All of the energy_perf additions are bogus, looks like a rebase gone wrong.

Thanks for catching this.

/Jarkko
Jarkko Sakkinen Aug. 7, 2019, 3:17 p.m. UTC | #3 
On Wed, Jul 24, 2019 at 12:35:42PM -0700, Sean Christopherson wrote:
> > +	if (IS_ENABLED(CONFIG_INTEL_SGX) && cpu_has(c, X86_FEATURE_SGX))
> > +		detect_sgx(c);
> > +
> > +	init_intel_energy_perf(c);
> 
> All of the energy_perf additions are bogus, looks like a rebase gone wrong.

Fixed in the master.

/Jarkko
diff mbox series

Patch

diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index 8d6d92ebeb54..1503b251d10f 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -623,6 +623,72 @@  static void detect_tme(struct cpuinfo_x86 *c)
 	c->x86_phys_bits -= keyid_bits;
 }
 
+static void __maybe_unused detect_sgx(struct cpuinfo_x86 *c)
+{
+	unsigned long long fc;
+
+	rdmsrl(MSR_IA32_FEATURE_CONTROL, fc);
+	if (!(fc & FEATURE_CONTROL_LOCKED)) {
+		pr_err_once("sgx: The feature control MSR is not locked\n");
+		goto err_unsupported;
+	}
+
+	if (!(fc & FEATURE_CONTROL_SGX_ENABLE)) {
+		pr_err_once("sgx: SGX is not enabled in IA32_FEATURE_CONTROL MSR\n");
+		goto err_unsupported;
+	}
+
+	if (!cpu_has(c, X86_FEATURE_SGX1)) {
+		pr_err_once("sgx: SGX1 instruction set is not supported\n");
+		goto err_unsupported;
+	}
+
+	if (!(fc & FEATURE_CONTROL_SGX_LE_WR)) {
+		pr_info_once("sgx: The launch control MSRs are not writable\n");
+		goto err_msrs_rdonly;
+	}
+
+	return;
+
+err_unsupported:
+	setup_clear_cpu_cap(X86_FEATURE_SGX);
+	setup_clear_cpu_cap(X86_FEATURE_SGX1);
+	setup_clear_cpu_cap(X86_FEATURE_SGX2);
+
+err_msrs_rdonly:
+	setup_clear_cpu_cap(X86_FEATURE_SGX_LC);
+}
+
+static void init_intel_energy_perf(struct cpuinfo_x86 *c)
+{
+	u64 epb;
+
+	/*
+	 * Initialize MSR_IA32_ENERGY_PERF_BIAS if not already initialized.
+	 * (x86_energy_perf_policy(8) is available to change it at run-time.)
+	 */
+	if (!cpu_has(c, X86_FEATURE_EPB))
+		return;
+
+	rdmsrl(MSR_IA32_ENERGY_PERF_BIAS, epb);
+	if ((epb & 0xF) != ENERGY_PERF_BIAS_PERFORMANCE)
+		return;
+
+	pr_warn_once("ENERGY_PERF_BIAS: Set to 'normal', was 'performance'\n");
+	pr_warn_once("ENERGY_PERF_BIAS: View and update with x86_energy_perf_policy(8)\n");
+	epb = (epb & ~0xF) | ENERGY_PERF_BIAS_NORMAL;
+	wrmsrl(MSR_IA32_ENERGY_PERF_BIAS, epb);
+}
+
+static void intel_bsp_resume(struct cpuinfo_x86 *c)
+{
+	/*
+	 * MSR_IA32_ENERGY_PERF_BIAS is lost across suspend/resume,
+	 * so reinitialize it properly like during bootup:
+	 */
+	init_intel_energy_perf(c);
+}
+
 static void init_cpuid_fault(struct cpuinfo_x86 *c)
 {
 	u64 msr;
@@ -760,6 +826,11 @@  static void init_intel(struct cpuinfo_x86 *c)
 	if (cpu_has(c, X86_FEATURE_TME))
 		detect_tme(c);
 
+	if (IS_ENABLED(CONFIG_INTEL_SGX) && cpu_has(c, X86_FEATURE_SGX))
+		detect_sgx(c);
+
+	init_intel_energy_perf(c);
+
 	init_intel_misc_features(c);
 }