diff mbox

xen: x86: remove duplicated MSR_IA32_FEATURE_CONTROL definition

Message ID 1466590625-9702-1-git-send-email-kai.huang@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Kai Huang June 22, 2016, 10:17 a.m. UTC 
From: Kai Huang <kai.huang@linux.intel.com>

MSR_IA32_FEATURE_CONTROL was introduced in below commit. Actually this MSR has
already been defined as IA32_FEATURE_CONTROL_MSR. Remove it as a duplication.

Also introduce a new macro for SGX_ENABLE bit in this MSR for better code and
further use. The code of below commit is changed accordingly.

commit 5a211704e8813c4890c8ce8dc4189d1dfb35ecd0
Author: Len Brown <len.brown@intel.com>
Date:   Fri Apr 8 22:31:47 2016 +0200

    mwait-idle: prevent SKL-H boot failure when C8+C9+C10 enabled

    Some SKL-H configurations require "max_cstate=7" to boot.
    While that is an effective workaround, it disables C10.

    ......

One more thinking: actually the SDM says 'Software is considered to have opted
into Intel SGX if and only if IA32_FEATURE_CONTROL.SGX_ENABLE and
IA32_FEATURE_CONTRORL.LOCK are set to 1', therefore maybe below change with the
lock bit checked is more reasonable, but existing code is safe anyway (and I
don't have machine to test).

        /* if SGX is enabled */
        if ((msr & IA32_FEATURE_CONTROL_SGX_ENABLE) &&
		    (msr & IA32_FEATURE_CONTROL_MSR_LOCK))
	            return;

Signed-off-by: Kai Huang <kai.huang@linux.intel.com>
---
 xen/arch/x86/cpu/mwait-idle.c   | 4 ++--
 xen/include/asm-x86/msr-index.h | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

Comments

Jan Beulich June 22, 2016, 11:44 a.m. UTC | #1 
>>> On 22.06.16 at 12:17, <kaih.linux@gmail.com> wrote:
> @@ -288,7 +289,6 @@
>  #define MSR_IA32_PLATFORM_ID		0x00000017
>  #define MSR_IA32_EBL_CR_POWERON		0x0000002a
>  #define MSR_IA32_EBC_FREQUENCY_ID	0x0000002c
> -#define MSR_IA32_FEATURE_CONTROL	0x0000003a
>  #define MSR_IA32_TSC_ADJUST		0x0000003b

The latest when removing the definition here you should have noticed
that this variant follows the conventional naming, so if you want to
consolidate things, it should be the other way around imo. While not
the main reason, it'll also make sure mwait-idle.c won't needlessly
diverge from its Linux original.

Jan
Kai Huang June 24, 2016, 10:38 a.m. UTC | #2 
On 6/22/2016 11:44 PM, Jan Beulich wrote:
>>>> On 22.06.16 at 12:17, <kaih.linux@gmail.com> wrote:
>> @@ -288,7 +289,6 @@
>>  #define MSR_IA32_PLATFORM_ID		0x00000017
>>  #define MSR_IA32_EBL_CR_POWERON		0x0000002a
>>  #define MSR_IA32_EBC_FREQUENCY_ID	0x0000002c
>> -#define MSR_IA32_FEATURE_CONTROL	0x0000003a
>>  #define MSR_IA32_TSC_ADJUST		0x0000003b
>
> The latest when removing the definition here you should have noticed
> that this variant follows the conventional naming, so if you want to
> consolidate things, it should be the other way around imo. While not
> the main reason, it'll also make sure mwait-idle.c won't needlessly
> diverge from its Linux original.

You are right. I'll sent out another patch removing the old one.

Thanks,
-Kai
>
> Jan
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>
diff mbox

Patch

diff --git a/xen/arch/x86/cpu/mwait-idle.c b/xen/arch/x86/cpu/mwait-idle.c
index e062e21..946d598 100644
--- a/xen/arch/x86/cpu/mwait-idle.c
+++ b/xen/arch/x86/cpu/mwait-idle.c
@@ -1003,10 +1003,10 @@  static void __init sklh_idle_state_table_update(void)
 
 	/* if SGX is present */
 	if (boot_cpu_has(X86_FEATURE_SGX)) {
-		rdmsrl(MSR_IA32_FEATURE_CONTROL, msr);
+		rdmsrl(IA32_FEATURE_CONTROL_MSR, msr);
 
 		/* if SGX is enabled */
-		if (msr & (1 << 18))
+		if (msr & IA32_FEATURE_CONTROL_SGX_ENABLE)
 			return;
 	}
 
diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h
index e0f7f8d..f539e24 100644
--- a/xen/include/asm-x86/msr-index.h
+++ b/xen/include/asm-x86/msr-index.h
@@ -139,6 +139,7 @@ 
 #define IA32_FEATURE_CONTROL_MSR_ENABLE_VMXON_OUTSIDE_SMX 0x0004
 #define IA32_FEATURE_CONTROL_MSR_SENTER_PARAM_CTL         0x7f00
 #define IA32_FEATURE_CONTROL_MSR_ENABLE_SENTER            0x8000
+#define IA32_FEATURE_CONTROL_SGX_ENABLE         0x40000
 
 /* K7/K8 MSRs. Not complete. See the architecture manual for a more
    complete list. */
@@ -288,7 +289,6 @@ 
 #define MSR_IA32_PLATFORM_ID		0x00000017
 #define MSR_IA32_EBL_CR_POWERON		0x0000002a
 #define MSR_IA32_EBC_FREQUENCY_ID	0x0000002c
-#define MSR_IA32_FEATURE_CONTROL	0x0000003a
 #define MSR_IA32_TSC_ADJUST		0x0000003b
 
 #define MSR_IA32_APICBASE		0x0000001b