Patchwork [1/4] X86: Intel MPX definiation

login
register
mail settings
Submitter Liu, Jinsong
Date Dec. 2, 2013, 4:43 p.m.
Message ID <DE8DF0795D48FD4CA783C40EC8292335013F60D2@SHSMSX101.ccr.corp.intel.com>
Download mbox | patch
Permalink /patch/295960/
State New
Headers show

Comments

Liu, Jinsong - Dec. 2, 2013, 4:43 p.m.
From fbfa537f690eca139a96c6b2636ab5130bf57716 Mon Sep 17 00:00:00 2001
From: Liu Jinsong <jinsong.liu@intel.com>
Date: Fri, 29 Nov 2013 01:27:00 +0800
Subject: [PATCH 1/4] X86: Intel MPX definiation

Signed-off-by: Xudong Hao <xudong.hao@intel.com>
Signed-off-by: Liu Jinsong <jinsong.liu@intel.com>
---
 arch/x86/include/asm/cpufeature.h |    2 ++
 arch/x86/include/asm/xsave.h      |    5 ++++-
 2 files changed, 6 insertions(+), 1 deletions(-)
Paolo Bonzini - Dec. 2, 2013, 5:39 p.m.
Il 02/12/2013 17:43, Liu, Jinsong ha scritto:
> From fbfa537f690eca139a96c6b2636ab5130bf57716 Mon Sep 17 00:00:00 2001
> From: Liu Jinsong <jinsong.liu@intel.com>
> Date: Fri, 29 Nov 2013 01:27:00 +0800
> Subject: [PATCH 1/4] X86: Intel MPX definiation
> 
> Signed-off-by: Xudong Hao <xudong.hao@intel.com>
> Signed-off-by: Liu Jinsong <jinsong.liu@intel.com>
> ---
>  arch/x86/include/asm/cpufeature.h |    2 ++
>  arch/x86/include/asm/xsave.h      |    5 ++++-
>  2 files changed, 6 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
> index 89270b4..1b00b01 100644
> --- a/arch/x86/include/asm/cpufeature.h
> +++ b/arch/x86/include/asm/cpufeature.h
> @@ -216,6 +216,7 @@
>  #define X86_FEATURE_ERMS	(9*32+ 9) /* Enhanced REP MOVSB/STOSB */
>  #define X86_FEATURE_INVPCID	(9*32+10) /* Invalidate Processor Context ID */
>  #define X86_FEATURE_RTM		(9*32+11) /* Restricted Transactional Memory */
> +#define X86_FEATURE_MPX		(9*32+14) /* Memory Protection Extension */
>  #define X86_FEATURE_RDSEED	(9*32+18) /* The RDSEED instruction */
>  #define X86_FEATURE_ADX		(9*32+19) /* The ADCX and ADOX instructions */
>  #define X86_FEATURE_SMAP	(9*32+20) /* Supervisor Mode Access Prevention */
> @@ -330,6 +331,7 @@ extern const char * const x86_power_flags[32];
>  #define cpu_has_perfctr_l2	boot_cpu_has(X86_FEATURE_PERFCTR_L2)
>  #define cpu_has_cx8		boot_cpu_has(X86_FEATURE_CX8)
>  #define cpu_has_cx16		boot_cpu_has(X86_FEATURE_CX16)
> +#define cpu_has_mpx		boot_cpu_has(X86_FEATURE_MPX)
>  #define cpu_has_eager_fpu	boot_cpu_has(X86_FEATURE_EAGER_FPU)
>  #define cpu_has_topoext		boot_cpu_has(X86_FEATURE_TOPOEXT)
>  
> diff --git a/arch/x86/include/asm/xsave.h b/arch/x86/include/asm/xsave.h
> index 0415cda..d3e3ea5 100644
> --- a/arch/x86/include/asm/xsave.h
> +++ b/arch/x86/include/asm/xsave.h
> @@ -9,6 +9,8 @@
>  #define XSTATE_FP	0x1
>  #define XSTATE_SSE	0x2
>  #define XSTATE_YMM	0x4
> +#define XSTATE_BNDREGS	0x8
> +#define XSTATE_BNDCSR	0x10
>  
>  #define XSTATE_FPSSE	(XSTATE_FP | XSTATE_SSE)
>  
> @@ -23,7 +25,8 @@
>  /*
>   * These are the features that the OS can handle currently.
>   */
> -#define XCNTXT_MASK	(XSTATE_FP | XSTATE_SSE | XSTATE_YMM)
> +#define XCNTXT_MASK	(XSTATE_FP | XSTATE_SSE | XSTATE_YMM | \
> +			XSTATE_BNDREGS | XSTATE_BNDCSR)
>  
>  #ifdef CONFIG_X86_64
>  #define REX_PREFIX	"0x48, "
> 

The patches look good.  Please include a cover letter (patch 0/n) next
time, and detail the changes from previous submissions.

It would also be nice to have support of the new feature for nested VMX
as well (including kvm-unit-tests coverage).

Paolo
Paolo Bonzini - Dec. 5, 2013, 4:08 p.m.
Il 02/12/2013 17:43, Liu, Jinsong ha scritto:
> From fbfa537f690eca139a96c6b2636ab5130bf57716 Mon Sep 17 00:00:00 2001
> From: Liu Jinsong <jinsong.liu@intel.com>
> Date: Fri, 29 Nov 2013 01:27:00 +0800
> Subject: [PATCH 1/4] X86: Intel MPX definiation
> 
> Signed-off-by: Xudong Hao <xudong.hao@intel.com>
> Signed-off-by: Liu Jinsong <jinsong.liu@intel.com>
> ---
>  arch/x86/include/asm/cpufeature.h |    2 ++
>  arch/x86/include/asm/xsave.h      |    5 ++++-
>  2 files changed, 6 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
> index 89270b4..1b00b01 100644
> --- a/arch/x86/include/asm/cpufeature.h
> +++ b/arch/x86/include/asm/cpufeature.h
> @@ -216,6 +216,7 @@
>  #define X86_FEATURE_ERMS	(9*32+ 9) /* Enhanced REP MOVSB/STOSB */
>  #define X86_FEATURE_INVPCID	(9*32+10) /* Invalidate Processor Context ID */
>  #define X86_FEATURE_RTM		(9*32+11) /* Restricted Transactional Memory */
> +#define X86_FEATURE_MPX		(9*32+14) /* Memory Protection Extension */
>  #define X86_FEATURE_RDSEED	(9*32+18) /* The RDSEED instruction */
>  #define X86_FEATURE_ADX		(9*32+19) /* The ADCX and ADOX instructions */
>  #define X86_FEATURE_SMAP	(9*32+20) /* Supervisor Mode Access Prevention */
> @@ -330,6 +331,7 @@ extern const char * const x86_power_flags[32];
>  #define cpu_has_perfctr_l2	boot_cpu_has(X86_FEATURE_PERFCTR_L2)
>  #define cpu_has_cx8		boot_cpu_has(X86_FEATURE_CX8)
>  #define cpu_has_cx16		boot_cpu_has(X86_FEATURE_CX16)
> +#define cpu_has_mpx		boot_cpu_has(X86_FEATURE_MPX)
>  #define cpu_has_eager_fpu	boot_cpu_has(X86_FEATURE_EAGER_FPU)
>  #define cpu_has_topoext		boot_cpu_has(X86_FEATURE_TOPOEXT)
>  
> diff --git a/arch/x86/include/asm/xsave.h b/arch/x86/include/asm/xsave.h
> index 0415cda..d3e3ea5 100644
> --- a/arch/x86/include/asm/xsave.h
> +++ b/arch/x86/include/asm/xsave.h
> @@ -9,6 +9,8 @@
>  #define XSTATE_FP	0x1
>  #define XSTATE_SSE	0x2
>  #define XSTATE_YMM	0x4
> +#define XSTATE_BNDREGS	0x8
> +#define XSTATE_BNDCSR	0x10
>  
>  #define XSTATE_FPSSE	(XSTATE_FP | XSTATE_SSE)
>  
> @@ -23,7 +25,8 @@
>  /*
>   * These are the features that the OS can handle currently.
>   */
> -#define XCNTXT_MASK	(XSTATE_FP | XSTATE_SSE | XSTATE_YMM)
> +#define XCNTXT_MASK	(XSTATE_FP | XSTATE_SSE | XSTATE_YMM | \
> +			XSTATE_BNDREGS | XSTATE_BNDCSR)
>  
>  #ifdef CONFIG_X86_64
>  #define REX_PREFIX	"0x48, "
> 

hpa/Ingo/Thomas, can you give your Acked-by for this patch?

I'm not sure of the consequences of changing XCNTXT_MASK.  This series
(which was submitted with the wrong threading) wants it so that KVM can
use fpu_save_init and fpu_restore_checking to save and restore the MPX
state of the guest.

Paolo
H. Peter Anvin - Dec. 5, 2013, 10:59 p.m.
On 12/05/2013 08:08 AM, Paolo Bonzini wrote:
> Il 02/12/2013 17:43, Liu, Jinsong ha scritto:
>> From fbfa537f690eca139a96c6b2636ab5130bf57716 Mon Sep 17 00:00:00 2001
>> From: Liu Jinsong <jinsong.liu@intel.com>
>> Date: Fri, 29 Nov 2013 01:27:00 +0800
>> Subject: [PATCH 1/4] X86: Intel MPX definiation
>>
>> Signed-off-by: Xudong Hao <xudong.hao@intel.com>
>> Signed-off-by: Liu Jinsong <jinsong.liu@intel.com>
>> ---
>>  arch/x86/include/asm/cpufeature.h |    2 ++
>>  arch/x86/include/asm/xsave.h      |    5 ++++-
>>  2 files changed, 6 insertions(+), 1 deletions(-)
>>
> 
> hpa/Ingo/Thomas, can you give your Acked-by for this patch?
> 
> I'm not sure of the consequences of changing XCNTXT_MASK.  This series
> (which was submitted with the wrong threading) wants it so that KVM can
> use fpu_save_init and fpu_restore_checking to save and restore the MPX
> state of the guest.
> 

Hi, I'm currently reviewing internally another set of patches for MPX
support which would at least in part conflict with these.  I don't see
the rest of the series -- where was it posted?

Either way:

1. asm/cpufeatures.h patches should always be separate, as we put those
into a special branch into the -tip tree since they touch so many other
things.

2. Enabling MPX is only safe with XSTATE_EAGER, which Qiaowei's patchset
has done correctly.

	-hpa
Paolo Bonzini - Dec. 6, 2013, 8:22 a.m.
Il 05/12/2013 23:59, H. Peter Anvin ha scritto:
> Hi, I'm currently reviewing internally another set of patches for MPX
> support which would at least in part conflict with these.  I don't see
> the rest of the series -- where was it posted?

It was posted to kvm-devel and not threaded. :(

Paolo

Patch

diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index 89270b4..1b00b01 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -216,6 +216,7 @@ 
 #define X86_FEATURE_ERMS	(9*32+ 9) /* Enhanced REP MOVSB/STOSB */
 #define X86_FEATURE_INVPCID	(9*32+10) /* Invalidate Processor Context ID */
 #define X86_FEATURE_RTM		(9*32+11) /* Restricted Transactional Memory */
+#define X86_FEATURE_MPX		(9*32+14) /* Memory Protection Extension */
 #define X86_FEATURE_RDSEED	(9*32+18) /* The RDSEED instruction */
 #define X86_FEATURE_ADX		(9*32+19) /* The ADCX and ADOX instructions */
 #define X86_FEATURE_SMAP	(9*32+20) /* Supervisor Mode Access Prevention */
@@ -330,6 +331,7 @@  extern const char * const x86_power_flags[32];
 #define cpu_has_perfctr_l2	boot_cpu_has(X86_FEATURE_PERFCTR_L2)
 #define cpu_has_cx8		boot_cpu_has(X86_FEATURE_CX8)
 #define cpu_has_cx16		boot_cpu_has(X86_FEATURE_CX16)
+#define cpu_has_mpx		boot_cpu_has(X86_FEATURE_MPX)
 #define cpu_has_eager_fpu	boot_cpu_has(X86_FEATURE_EAGER_FPU)
 #define cpu_has_topoext		boot_cpu_has(X86_FEATURE_TOPOEXT)
 
diff --git a/arch/x86/include/asm/xsave.h b/arch/x86/include/asm/xsave.h
index 0415cda..d3e3ea5 100644
--- a/arch/x86/include/asm/xsave.h
+++ b/arch/x86/include/asm/xsave.h
@@ -9,6 +9,8 @@ 
 #define XSTATE_FP	0x1
 #define XSTATE_SSE	0x2
 #define XSTATE_YMM	0x4
+#define XSTATE_BNDREGS	0x8
+#define XSTATE_BNDCSR	0x10
 
 #define XSTATE_FPSSE	(XSTATE_FP | XSTATE_SSE)
 
@@ -23,7 +25,8 @@ 
 /*
  * These are the features that the OS can handle currently.
  */
-#define XCNTXT_MASK	(XSTATE_FP | XSTATE_SSE | XSTATE_YMM)
+#define XCNTXT_MASK	(XSTATE_FP | XSTATE_SSE | XSTATE_YMM | \
+			XSTATE_BNDREGS | XSTATE_BNDCSR)
 
 #ifdef CONFIG_X86_64
 #define REX_PREFIX	"0x48, "