diff mbox series

[RFC,v3,02/27] x86/cpufeatures: Make SGX_LC feature bit depend on SGX bit

Message ID bdca25f260a895fcc39b2fb59e1155102a210aa0.1611634586.git.kai.huang@intel.com (mailing list archive)
State New, archived
Headers show
Series KVM SGX virtualization support | expand

Commit Message

Huang, Kai Jan. 26, 2021, 9:30 a.m. UTC
Move SGX_LC feature bit to CPUID dependency table as well, along with
new added SGX1 and SGX2 bit, to make clearing all SGX feature bits
easier. Also remove clear_sgx_caps() since it is just a wrapper of
setup_clear_cpu_cap(X86_FEATURE_SGX) now.

Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Kai Huang <kai.huang@intel.com>
---
 arch/x86/kernel/cpu/cpuid-deps.c |  1 +
 arch/x86/kernel/cpu/feat_ctl.c   | 12 +++---------
 2 files changed, 4 insertions(+), 9 deletions(-)

Comments

Jarkko Sakkinen Jan. 30, 2021, 1:22 p.m. UTC | #1
On Tue, Jan 26, 2021 at 10:30:17PM +1300, Kai Huang wrote:
> Move SGX_LC feature bit to CPUID dependency table as well, along with
> new added SGX1 and SGX2 bit, to make clearing all SGX feature bits
> easier. Also remove clear_sgx_caps() since it is just a wrapper of
> setup_clear_cpu_cap(X86_FEATURE_SGX) now.
> 
> Suggested-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Kai Huang <kai.huang@intel.com>

Acked-by: Jarkko Sakkinen <jarkko@kernel.org>

So could this be an improvement to the existing code? If so, then
this should be the first patch. Also, I think that then this can
be merged independently from rest of the patch set.

/Jarkko
Huang, Kai Feb. 1, 2021, 12:08 a.m. UTC | #2
On Sat, 30 Jan 2021 15:22:49 +0200 Jarkko Sakkinen wrote:
> On Tue, Jan 26, 2021 at 10:30:17PM +1300, Kai Huang wrote:
> > Move SGX_LC feature bit to CPUID dependency table as well, along with
> > new added SGX1 and SGX2 bit, to make clearing all SGX feature bits
> > easier. Also remove clear_sgx_caps() since it is just a wrapper of
> > setup_clear_cpu_cap(X86_FEATURE_SGX) now.
> > 
> > Suggested-by: Sean Christopherson <seanjc@google.com>
> > Signed-off-by: Kai Huang <kai.huang@intel.com>
> 
> Acked-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> So could this be an improvement to the existing code? If so, then
> this should be the first patch. Also, I think that then this can
> be merged independently from rest of the patch set.

W/o SGX1/SGX2, I don't know whether it is worth to put SGX_LC into cpuid
dependency table, and kill clear_sgx_caps(). And since both you and Dave
already provided Acked-by, I am a little bit reluctant to switch the order
(because obviously both patches will be different).

Let me know if you still want me to switch patch order.
diff mbox series

Patch

diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c
index 5cf965580dd4..defda61f372d 100644
--- a/arch/x86/kernel/cpu/cpuid-deps.c
+++ b/arch/x86/kernel/cpu/cpuid-deps.c
@@ -72,6 +72,7 @@  static const struct cpuid_dep cpuid_deps[] = {
 	{ X86_FEATURE_AVX512_FP16,		X86_FEATURE_AVX512BW  },
 	{ X86_FEATURE_ENQCMD,			X86_FEATURE_XSAVES    },
 	{ X86_FEATURE_PER_THREAD_MBA,		X86_FEATURE_MBA       },
+	{ X86_FEATURE_SGX_LC,			X86_FEATURE_SGX	      },
 	{ X86_FEATURE_SGX1,			X86_FEATURE_SGX       },
 	{ X86_FEATURE_SGX2,			X86_FEATURE_SGX1      },
 	{}
diff --git a/arch/x86/kernel/cpu/feat_ctl.c b/arch/x86/kernel/cpu/feat_ctl.c
index 3b1b01f2b248..27533a6e04fa 100644
--- a/arch/x86/kernel/cpu/feat_ctl.c
+++ b/arch/x86/kernel/cpu/feat_ctl.c
@@ -93,15 +93,9 @@  static void init_vmx_capabilities(struct cpuinfo_x86 *c)
 }
 #endif /* CONFIG_X86_VMX_FEATURE_NAMES */
 
-static void clear_sgx_caps(void)
-{
-	setup_clear_cpu_cap(X86_FEATURE_SGX);
-	setup_clear_cpu_cap(X86_FEATURE_SGX_LC);
-}
-
 static int __init nosgx(char *str)
 {
-	clear_sgx_caps();
+	setup_clear_cpu_cap(X86_FEATURE_SGX);
 
 	return 0;
 }
@@ -116,7 +110,7 @@  void init_ia32_feat_ctl(struct cpuinfo_x86 *c)
 
 	if (rdmsrl_safe(MSR_IA32_FEAT_CTL, &msr)) {
 		clear_cpu_cap(c, X86_FEATURE_VMX);
-		clear_sgx_caps();
+		clear_cpu_cap(c, X86_FEATURE_SGX);
 		return;
 	}
 
@@ -177,6 +171,6 @@  void init_ia32_feat_ctl(struct cpuinfo_x86 *c)
 	    !(msr & FEAT_CTL_SGX_LC_ENABLED) || !enable_sgx) {
 		if (enable_sgx)
 			pr_err_once("SGX disabled by BIOS\n");
-		clear_sgx_caps();
+		clear_cpu_cap(c, X86_FEATURE_SGX);
 	}
 }