On Sat, Nov 03, 2018 at 03:05:39PM +0200, Andy Shevchenko wrote:
> > +static void detect_sgx(struct cpuinfo_x86 *c)
> > +{
> > + bool unsupported = false;
> > + unsigned long long fc;
> > +
> > + rdmsrl(MSR_IA32_FEATURE_CONTROL, fc);
> > + if (!(fc & FEATURE_CONTROL_LOCKED)) {
> > + pr_err_once("sgx: IA32_FEATURE_CONTROL MSR is not locked\n");
> > + unsupported = true;
> > + } else if (!(fc & FEATURE_CONTROL_SGX_ENABLE)) {
> > + pr_err_once("sgx: not enabled in IA32_FEATURE_CONTROL MSR\n");
> > + unsupported = true;
> > + } else if (!cpu_has(c, X86_FEATURE_SGX1)) {
> > + pr_err_once("sgx: SGX1 instruction set not supported\n");
> > + unsupported = true;
> > + }
>
> If you do
>
> } else {
> /* Supported */
> return;
> }
Agree. Would this be a more clean flow in the attached patch?
/Jarkko
>From 3b863a7db00cefffc15df918a5132c35ea313c27 Mon Sep 17 00:00:00 2001
From: Jarkko Sakkinen <jarkko.sakkinen@xxxxxxxxxxxxxxx>
Date: Mon, 5 Nov 2018 16:06:06 +0200
Subject: [PATCH] x86/cpu/intel: clean up detect_sgx() flow
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@xxxxxxxxxxxxxxx>
---
arch/x86/kernel/cpu/intel.c | 32 ++++++++++++++++++++------------
1 file changed, 20 insertions(+), 12 deletions(-)
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index bc52c52f7025..8a20a193d399 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -598,28 +598,36 @@ static void detect_tme(struct cpuinfo_x86 *c)
static void detect_sgx(struct cpuinfo_x86 *c)
{
- bool unsupported = false;
unsigned long long fc;
rdmsrl(MSR_IA32_FEATURE_CONTROL, fc);
if (!(fc & FEATURE_CONTROL_LOCKED)) {
pr_err_once("sgx: IA32_FEATURE_CONTROL MSR is not locked\n");
- unsupported = true;
- } else if (!(fc & FEATURE_CONTROL_SGX_ENABLE)) {
+ goto out_unsupported;
+ }
+
+ if (!(fc & FEATURE_CONTROL_SGX_ENABLE)) {
pr_err_once("sgx: not enabled in IA32_FEATURE_CONTROL MSR\n");
- unsupported = true;
- } else if (!cpu_has(c, X86_FEATURE_SGX1)) {
+ goto out_unsupported;
+ }
+
+ if (!cpu_has(c, X86_FEATURE_SGX1)) {
pr_err_once("sgx: SGX1 instruction set not supported\n");
- unsupported = true;
+ goto out_unsupported;
}
- if (unsupported) {
- setup_clear_cpu_cap(X86_FEATURE_SGX);
- setup_clear_cpu_cap(X86_FEATURE_SGX1);
- setup_clear_cpu_cap(X86_FEATURE_SGX2);
+ if (!(fc & FEATURE_CONTROL_SGX_LE_WR)) {
+ pr_info_once("sgx: launch control MSRs are not writable\n");
+ goto out_msrs_rdonly;
}
- if (unsupported || !(fc & FEATURE_CONTROL_SGX_LE_WR))
- setup_clear_cpu_cap(X86_FEATURE_SGX_LC);
+
+ return;
+out_unsupported:
+ setup_clear_cpu_cap(X86_FEATURE_SGX);
+ setup_clear_cpu_cap(X86_FEATURE_SGX1);
+ setup_clear_cpu_cap(X86_FEATURE_SGX2);
+out_msrs_rdonly:
+ setup_clear_cpu_cap(X86_FEATURE_SGX_LC);
}
static void init_intel_energy_perf(struct cpuinfo_x86 *c)
--
2.19.1
