[PATCH v2 1/3] x86, mpx: add documentation on Intel MPX

[PATCH v2 1/3] x86, mpx: add documentation on Intel MPX

[Qiaowei Ren]

This patch adds the Documentation/intel_mpx.txt file with some
information about Intel MPX.

Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com>
Signed-off-by: Xudong Hao <xudong.hao@intel.com>
Signed-off-by: Liu Jinsong <jinsong.liu@intel.com>
---
 Documentation/x86/intel_mpx.txt |   76 +++++++++++++++++++++++++++++++++++++++
 1 files changed, 76 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/x86/intel_mpx.txt

diff --git a/Documentation/x86/intel_mpx.txt b/Documentation/x86/intel_mpx.txt
new file mode 100644
index 0000000..778d06e
--- /dev/null
+++ b/Documentation/x86/intel_mpx.txt
@@ -0,0 +1,76 @@
+Intel(R) MPX Overview:
+=====================
+
+Intel(R) Memory Protection Extensions (Intel(R) MPX) is a new
+capability introduced into Intel Architecture. Intel MPX can
+increase the robustness of software when it is used in conjunction
+with compiler changes to check that memory references intended
+at compile time do not become unsafe at runtime.
+
+Two of the most important goals of Intel MPX are to provide
+this capability at very low performance overhead for newly
+compiled code, and to provide compatibility mechanisms with
+legacy software components. A direct benefit Intel MPX provides
+is hardening software against malicious attacks designed to
+cause or exploit buffer overruns.
+
+For details about the Intel MPX instructions, see "Intel(R)
+Architecture Instruction Set Extensions Programming Reference".
+
+Intel(R) MPX Programming Model
+------------------------------
+
+Intel MPX introduces new registers and new instructions that
+operate on these registers. Some of the registers added are
+bounds registers which store a pointer's lower bound and upper
+bound limits. Whenever the pointer is used, the requested
+reference is checked against the pointer's associated bounds,
+thereby preventing out-of-bound memory access (such as buffer
+overflows and overruns). Out-of-bounds memory references
+initiate a #BR exception which can then be handled in an
+appropriate manner.
+
+Loading and Storing Bounds using Translation
+--------------------------------------------
+
+Intel MPX defines two instructions for load/store of the linear
+address of a pointer to a buffer, along with the bounds of the
+buffer into a paging structure of extended bounds. Specifically
+when storing extended bounds, the processor will perform address
+translation of the address where the pointer is stored to an
+address in the Bound Table (BT) to determine the store location
+of extended bounds. Loading of an extended bounds performs the
+reverse sequence.
+
+The structure in memory to load/store an extended bound is a
+4-tuple consisting of lower bound, upper bound, pointer value
+and a reserved field. Bound loads and stores access 32-bit or
+64-bit operand size according to the operation mode. Thus,
+a bound table entry is 4*32 bits in 32-bit mode and 4*64 bits
+in 64-bit mode.
+
+The linear address of a bound table is stored in a Bound
+Directory (BD) entry. And the linear address of the bound
+directory is derived from either BNDCFGU or BNDCFGS registers.
+Bounds in memory are stored in Bound Tables (BT) as an extended
+bound, which are accessed via Bound Directory (BD) and address
+translation performed by BNDLDX/BNDSTX instructions.
+
+Bounds Directory (BD) and Bounds Tables (BT) are stored in
+application memory and are allocated by the application (in case
+of kernel use, the structures will be in kernel memory). The
+bound directory and each instance of bound table are in contiguous
+linear memory.
+
+XSAVE/XRESTOR Support of Intel MPX State
+----------------------------------------
+
+Enabling Intel MPX requires an OS to manage two bits in XCR0:
+  - BNDREGS for saving and restoring registers BND0-BND3,
+  - BNDCSR for saving and restoring the user-mode configuration
+(BNDCFGU) and the status register (BNDSTATUS).
+
+The reason for having two separate bits is that BND0-BND3 is
+likely to be volatile state, while BNDCFGU and BNDSTATUS are not.
+Therefore, an OS has flexibility in handling these two states
+differently in saving or restoring them.
-- 
1.7.1

[Qemu-devel] [PATCH v2 1/3] x86, mpx: add documentation on Intel MPX

[Qiaowei Ren]

This patch adds the Documentation/intel_mpx.txt file with some
information about Intel MPX.

Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com>
Signed-off-by: Xudong Hao <xudong.hao@intel.com>
Signed-off-by: Liu Jinsong <jinsong.liu@intel.com>
---
 Documentation/x86/intel_mpx.txt |   76 +++++++++++++++++++++++++++++++++++++++
 1 files changed, 76 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/x86/intel_mpx.txt

diff --git a/Documentation/x86/intel_mpx.txt b/Documentation/x86/intel_mpx.txt
new file mode 100644
index 0000000..778d06e
--- /dev/null
+++ b/Documentation/x86/intel_mpx.txt
@@ -0,0 +1,76 @@
+Intel(R) MPX Overview:
+=====================
+
+Intel(R) Memory Protection Extensions (Intel(R) MPX) is a new
+capability introduced into Intel Architecture. Intel MPX can
+increase the robustness of software when it is used in conjunction
+with compiler changes to check that memory references intended
+at compile time do not become unsafe at runtime.
+
+Two of the most important goals of Intel MPX are to provide
+this capability at very low performance overhead for newly
+compiled code, and to provide compatibility mechanisms with
+legacy software components. A direct benefit Intel MPX provides
+is hardening software against malicious attacks designed to
+cause or exploit buffer overruns.
+
+For details about the Intel MPX instructions, see "Intel(R)
+Architecture Instruction Set Extensions Programming Reference".
+
+Intel(R) MPX Programming Model
+------------------------------
+
+Intel MPX introduces new registers and new instructions that
+operate on these registers. Some of the registers added are
+bounds registers which store a pointer's lower bound and upper
+bound limits. Whenever the pointer is used, the requested
+reference is checked against the pointer's associated bounds,
+thereby preventing out-of-bound memory access (such as buffer
+overflows and overruns). Out-of-bounds memory references
+initiate a #BR exception which can then be handled in an
+appropriate manner.
+
+Loading and Storing Bounds using Translation
+--------------------------------------------
+
+Intel MPX defines two instructions for load/store of the linear
+address of a pointer to a buffer, along with the bounds of the
+buffer into a paging structure of extended bounds. Specifically
+when storing extended bounds, the processor will perform address
+translation of the address where the pointer is stored to an
+address in the Bound Table (BT) to determine the store location
+of extended bounds. Loading of an extended bounds performs the
+reverse sequence.
+
+The structure in memory to load/store an extended bound is a
+4-tuple consisting of lower bound, upper bound, pointer value
+and a reserved field. Bound loads and stores access 32-bit or
+64-bit operand size according to the operation mode. Thus,
+a bound table entry is 4*32 bits in 32-bit mode and 4*64 bits
+in 64-bit mode.
+
+The linear address of a bound table is stored in a Bound
+Directory (BD) entry. And the linear address of the bound
+directory is derived from either BNDCFGU or BNDCFGS registers.
+Bounds in memory are stored in Bound Tables (BT) as an extended
+bound, which are accessed via Bound Directory (BD) and address
+translation performed by BNDLDX/BNDSTX instructions.
+
+Bounds Directory (BD) and Bounds Tables (BT) are stored in
+application memory and are allocated by the application (in case
+of kernel use, the structures will be in kernel memory). The
+bound directory and each instance of bound table are in contiguous
+linear memory.
+
+XSAVE/XRESTOR Support of Intel MPX State
+----------------------------------------
+
+Enabling Intel MPX requires an OS to manage two bits in XCR0:
+  - BNDREGS for saving and restoring registers BND0-BND3,
+  - BNDCSR for saving and restoring the user-mode configuration
+(BNDCFGU) and the status register (BNDSTATUS).
+
+The reason for having two separate bits is that BND0-BND3 is
+likely to be volatile state, while BNDCFGU and BNDSTATUS are not.
+Therefore, an OS has flexibility in handling these two states
+differently in saving or restoring them.
-- 
1.7.1

[PATCH v2 2/3] X86, mpx: Intel MPX CPU feature definition

[Qiaowei Ren]

This patch defines Intel MPX CPU feature.

Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com>
Signed-off-by: Xudong Hao <xudong.hao@intel.com>
Signed-off-by: Liu Jinsong <jinsong.liu@intel.com>
---
 arch/x86/include/asm/cpufeature.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index d3f5c63..ef9f9c2 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 */
-- 
1.7.1

[Qemu-devel] [PATCH v2 2/3] X86, mpx: Intel MPX CPU feature definition

[Qiaowei Ren]

This patch defines Intel MPX CPU feature.

Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com>
Signed-off-by: Xudong Hao <xudong.hao@intel.com>
Signed-off-by: Liu Jinsong <jinsong.liu@intel.com>
---
 arch/x86/include/asm/cpufeature.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index d3f5c63..ef9f9c2 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 */
-- 
1.7.1

[tip:x86/cpufeature] x86, cpufeature: Define the Intel MPX feature flag

[tip-bot for Qiaowei Ren]

Commit-ID:  191f57c137bcce0e3e9313acb77b2f114d15afbb
Gitweb:     http://git.kernel.org/tip/191f57c137bcce0e3e9313acb77b2f114d15afbb
Author:     Qiaowei Ren <qiaowei.ren@intel.com>
AuthorDate: Sat, 7 Dec 2013 08:20:57 +0800
Committer:  H. Peter Anvin <hpa@linux.intel.com>
CommitDate: Fri, 6 Dec 2013 10:21:44 -0800

x86, cpufeature: Define the Intel MPX feature flag

Define the Intel MPX (Memory Protection Extensions) CPU feature flag
in the cpufeature list.

Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com>
Link: http://lkml.kernel.org/r/1386375658-2191-2-git-send-email-qiaowei.ren@intel.com
Signed-off-by: Xudong Hao <xudong.hao@intel.com>
Signed-off-by: Liu Jinsong <jinsong.liu@intel.com>
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
---
 arch/x86/include/asm/cpufeature.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index 89270b4..e099f95 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 */

[PATCH v2 3/3] X86, mpx: Intel MPX xstate feature definition

[Qiaowei Ren]

This patch defines xstate feature and extends struct xsave_hdr_struct
to support Intel MPX.

Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com>
Signed-off-by: Xudong Hao <xudong.hao@intel.com>
Signed-off-by: Liu Jinsong <jinsong.liu@intel.com>
---
 arch/x86/include/asm/processor.h |   12 ++++++++++++
 arch/x86/include/asm/xsave.h     |    5 ++++-
 2 files changed, 16 insertions(+), 1 deletions(-)

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 987c75e..2fe2e75 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -370,6 +370,15 @@ struct ymmh_struct {
 	u32 ymmh_space[64];
 };
 
+struct bndregs_struct {
+	u64 bndregs[8];
+} __packed;
+
+struct bndcsr_struct {
+	u64 cfg_reg_u;
+	u64 status_reg;
+} __packed;
+
 struct xsave_hdr_struct {
 	u64 xstate_bv;
 	u64 reserved1[2];
@@ -380,6 +389,9 @@ struct xsave_struct {
 	struct i387_fxsave_struct i387;
 	struct xsave_hdr_struct xsave_hdr;
 	struct ymmh_struct ymmh;
+	u8 lwp_area[128];
+	struct bndregs_struct bndregs;
+	struct bndcsr_struct bndcsr;
 	/* new processor state extensions will go here */
 } __attribute__ ((packed, aligned (64)));
 
diff --git a/arch/x86/include/asm/xsave.h b/arch/x86/include/asm/xsave.h
index 0415cda..7fa8855 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)
 
@@ -20,10 +22,11 @@
 #define XSAVE_YMM_SIZE	    256
 #define XSAVE_YMM_OFFSET    (XSAVE_HDR_SIZE + XSAVE_HDR_OFFSET)
 
+#define XSTATE_EAGER	(XSTATE_BNDREGS | XSTATE_BNDCSR)
 /*
  * 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_EAGER)
 
 #ifdef CONFIG_X86_64
 #define REX_PREFIX	"0x48, "
-- 
1.7.1

[Qemu-devel] [PATCH v2 3/3] X86, mpx: Intel MPX xstate feature definition

[Qiaowei Ren]

This patch defines xstate feature and extends struct xsave_hdr_struct
to support Intel MPX.

Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com>
Signed-off-by: Xudong Hao <xudong.hao@intel.com>
Signed-off-by: Liu Jinsong <jinsong.liu@intel.com>
---
 arch/x86/include/asm/processor.h |   12 ++++++++++++
 arch/x86/include/asm/xsave.h     |    5 ++++-
 2 files changed, 16 insertions(+), 1 deletions(-)

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 987c75e..2fe2e75 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -370,6 +370,15 @@ struct ymmh_struct {
 	u32 ymmh_space[64];
 };
 
+struct bndregs_struct {
+	u64 bndregs[8];
+} __packed;
+
+struct bndcsr_struct {
+	u64 cfg_reg_u;
+	u64 status_reg;
+} __packed;
+
 struct xsave_hdr_struct {
 	u64 xstate_bv;
 	u64 reserved1[2];
@@ -380,6 +389,9 @@ struct xsave_struct {
 	struct i387_fxsave_struct i387;
 	struct xsave_hdr_struct xsave_hdr;
 	struct ymmh_struct ymmh;
+	u8 lwp_area[128];
+	struct bndregs_struct bndregs;
+	struct bndcsr_struct bndcsr;
 	/* new processor state extensions will go here */
 } __attribute__ ((packed, aligned (64)));
 
diff --git a/arch/x86/include/asm/xsave.h b/arch/x86/include/asm/xsave.h
index 0415cda..7fa8855 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)
 
@@ -20,10 +22,11 @@
 #define XSAVE_YMM_SIZE	    256
 #define XSAVE_YMM_OFFSET    (XSAVE_HDR_SIZE + XSAVE_HDR_OFFSET)
 
+#define XSTATE_EAGER	(XSTATE_BNDREGS | XSTATE_BNDCSR)
 /*
  * 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_EAGER)
 
 #ifdef CONFIG_X86_64
 #define REX_PREFIX	"0x48, "
-- 
1.7.1

Re: [PATCH v2 3/3] X86, mpx: Intel MPX xstate feature definition

[Paolo Bonzini]

Il 07/12/2013 01:20, Qiaowei Ren ha scritto:
> This patch defines xstate feature and extends struct xsave_hdr_struct
> to support Intel MPX.
> 
> Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com>
> Signed-off-by: Xudong Hao <xudong.hao@intel.com>
> Signed-off-by: Liu Jinsong <jinsong.liu@intel.com>
> ---
>  arch/x86/include/asm/processor.h |   12 ++++++++++++
>  arch/x86/include/asm/xsave.h     |    5 ++++-
>  2 files changed, 16 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> index 987c75e..2fe2e75 100644
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -370,6 +370,15 @@ struct ymmh_struct {
>  	u32 ymmh_space[64];
>  };
>  
> +struct bndregs_struct {
> +	u64 bndregs[8];
> +} __packed;
> +
> +struct bndcsr_struct {
> +	u64 cfg_reg_u;
> +	u64 status_reg;
> +} __packed;
> +
>  struct xsave_hdr_struct {
>  	u64 xstate_bv;
>  	u64 reserved1[2];
> @@ -380,6 +389,9 @@ struct xsave_struct {
>  	struct i387_fxsave_struct i387;
>  	struct xsave_hdr_struct xsave_hdr;
>  	struct ymmh_struct ymmh;
> +	u8 lwp_area[128];

Sorry for the back-and-forth, but I think this and the removal of
XSTATE_FLEXIBLE (perhaps XSTATE_LAZY?) makes your v2 worse than v1.

Since Peter already said the same, please undo these changes.

Also, how is XSTATE_EAGER used?  Should MPX be disabled when xsaveopt is
disabled on the kernel command line?  (Liu, how would this affect the
KVM patches, too?)

Paolo

> +#define XSTATE_EAGER	(XSTATE_BNDREGS | XSTATE_BNDCSR)
>  /*
>   * 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_EAGER)
>  
>  #ifdef CONFIG_X86_64
>  #define REX_PREFIX	"0x48, "
> 

Re: [Qemu-devel] [PATCH v2 3/3] X86, mpx: Intel MPX xstate feature definition

[Paolo Bonzini]

Il 07/12/2013 01:20, Qiaowei Ren ha scritto:
> This patch defines xstate feature and extends struct xsave_hdr_struct
> to support Intel MPX.
> 
> Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com>
> Signed-off-by: Xudong Hao <xudong.hao@intel.com>
> Signed-off-by: Liu Jinsong <jinsong.liu@intel.com>
> ---
>  arch/x86/include/asm/processor.h |   12 ++++++++++++
>  arch/x86/include/asm/xsave.h     |    5 ++++-
>  2 files changed, 16 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> index 987c75e..2fe2e75 100644
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -370,6 +370,15 @@ struct ymmh_struct {
>  	u32 ymmh_space[64];
>  };
>  
> +struct bndregs_struct {
> +	u64 bndregs[8];
> +} __packed;
> +
> +struct bndcsr_struct {
> +	u64 cfg_reg_u;
> +	u64 status_reg;
> +} __packed;
> +
>  struct xsave_hdr_struct {
>  	u64 xstate_bv;
>  	u64 reserved1[2];
> @@ -380,6 +389,9 @@ struct xsave_struct {
>  	struct i387_fxsave_struct i387;
>  	struct xsave_hdr_struct xsave_hdr;
>  	struct ymmh_struct ymmh;
> +	u8 lwp_area[128];

Sorry for the back-and-forth, but I think this and the removal of
XSTATE_FLEXIBLE (perhaps XSTATE_LAZY?) makes your v2 worse than v1.

Since Peter already said the same, please undo these changes.

Also, how is XSTATE_EAGER used?  Should MPX be disabled when xsaveopt is
disabled on the kernel command line?  (Liu, how would this affect the
KVM patches, too?)

Paolo

> +#define XSTATE_EAGER	(XSTATE_BNDREGS | XSTATE_BNDCSR)
>  /*
>   * 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_EAGER)
>  
>  #ifdef CONFIG_X86_64
>  #define REX_PREFIX	"0x48, "
> 

Re: [PATCH v2 3/3] X86, mpx: Intel MPX xstate feature definition

[H. Peter Anvin]

On 12/06/2013 09:35 AM, Paolo Bonzini wrote:
> 
> Sorry for the back-and-forth, but I think this and the removal of
> XSTATE_FLEXIBLE (perhaps XSTATE_LAZY?) makes your v2 worse than v1.
> 
> Since Peter already said the same, please undo these changes.
> 
> Also, how is XSTATE_EAGER used?  Should MPX be disabled when xsaveopt is
> disabled on the kernel command line?  (Liu, how would this affect the
> KVM patches, too?)
> 

There are two options: we could disable MPX etc. or we could force eager
saving (using xsave) even if xsaveopt is disabled.  It is a hard call to
make, but I guess I'm leaning towards the latter; we could add an
"lazyxsave" option to explicitly disable all eager features if there is
use for that.

	-hpa

Re: [Qemu-devel] [PATCH v2 3/3] X86, mpx: Intel MPX xstate feature definition

[H. Peter Anvin]

On 12/06/2013 09:35 AM, Paolo Bonzini wrote:
> 
> Sorry for the back-and-forth, but I think this and the removal of
> XSTATE_FLEXIBLE (perhaps XSTATE_LAZY?) makes your v2 worse than v1.
> 
> Since Peter already said the same, please undo these changes.
> 
> Also, how is XSTATE_EAGER used?  Should MPX be disabled when xsaveopt is
> disabled on the kernel command line?  (Liu, how would this affect the
> KVM patches, too?)
> 

There are two options: we could disable MPX etc. or we could force eager
saving (using xsave) even if xsaveopt is disabled.  It is a hard call to
make, but I guess I'm leaning towards the latter; we could add an
"lazyxsave" option to explicitly disable all eager features if there is
use for that.

	-hpa

Re: [PATCH v2 3/3] X86, mpx: Intel MPX xstate feature definition

[Liu, Jinsong]

Paolo Bonzini wrote:
> Il 07/12/2013 01:20, Qiaowei Ren ha scritto:
>> This patch defines xstate feature and extends struct xsave_hdr_struct
>> to support Intel MPX.
>> 
>> Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com>
>> Signed-off-by: Xudong Hao <xudong.hao@intel.com>
>> Signed-off-by: Liu Jinsong <jinsong.liu@intel.com>
>> ---
>>  arch/x86/include/asm/processor.h |   12 ++++++++++++
>>  arch/x86/include/asm/xsave.h     |    5 ++++-
>>  2 files changed, 16 insertions(+), 1 deletions(-)
>> 
>> diff --git a/arch/x86/include/asm/processor.h
>> b/arch/x86/include/asm/processor.h index 987c75e..2fe2e75 100644 ---
>> a/arch/x86/include/asm/processor.h +++
>> b/arch/x86/include/asm/processor.h @@ -370,6 +370,15 @@ struct
>>  	ymmh_struct { u32 ymmh_space[64];
>>  };
>> 
>> +struct bndregs_struct {
>> +	u64 bndregs[8];
>> +} __packed;
>> +
>> +struct bndcsr_struct {
>> +	u64 cfg_reg_u;
>> +	u64 status_reg;
>> +} __packed;
>> +
>>  struct xsave_hdr_struct {
>>  	u64 xstate_bv;
>>  	u64 reserved1[2];
>> @@ -380,6 +389,9 @@ struct xsave_struct {
>>  	struct i387_fxsave_struct i387;
>>  	struct xsave_hdr_struct xsave_hdr;
>>  	struct ymmh_struct ymmh;
>> +	u8 lwp_area[128];
> 
> Sorry for the back-and-forth, but I think this and the removal of
> XSTATE_FLEXIBLE (perhaps XSTATE_LAZY?) makes your v2 worse than v1.
> 
> Since Peter already said the same, please undo these changes.
> 
> Also, how is XSTATE_EAGER used?  Should MPX be disabled when xsaveopt
> is disabled on the kernel command line?  (Liu, how would this affect
> the KVM patches, too?)
> 
> Paolo

Currently seems no, and if needed we can add a new patch at kvm side accordingly when native mpx patches checked in.

Jinsong

> 
>> +#define XSTATE_EAGER	(XSTATE_BNDREGS | XSTATE_BNDCSR)  /*
>>   * 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_EAGER) 
>> 
>>  #ifdef CONFIG_X86_64
>>  #define REX_PREFIX	"0x48, "

Re: [Qemu-devel] [PATCH v2 3/3] X86, mpx: Intel MPX xstate feature definition

[Liu, Jinsong]

Paolo Bonzini wrote:
> Il 07/12/2013 01:20, Qiaowei Ren ha scritto:
>> This patch defines xstate feature and extends struct xsave_hdr_struct
>> to support Intel MPX.
>> 
>> Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com>
>> Signed-off-by: Xudong Hao <xudong.hao@intel.com>
>> Signed-off-by: Liu Jinsong <jinsong.liu@intel.com>
>> ---
>>  arch/x86/include/asm/processor.h |   12 ++++++++++++
>>  arch/x86/include/asm/xsave.h     |    5 ++++-
>>  2 files changed, 16 insertions(+), 1 deletions(-)
>> 
>> diff --git a/arch/x86/include/asm/processor.h
>> b/arch/x86/include/asm/processor.h index 987c75e..2fe2e75 100644 ---
>> a/arch/x86/include/asm/processor.h +++
>> b/arch/x86/include/asm/processor.h @@ -370,6 +370,15 @@ struct
>>  	ymmh_struct { u32 ymmh_space[64];
>>  };
>> 
>> +struct bndregs_struct {
>> +	u64 bndregs[8];
>> +} __packed;
>> +
>> +struct bndcsr_struct {
>> +	u64 cfg_reg_u;
>> +	u64 status_reg;
>> +} __packed;
>> +
>>  struct xsave_hdr_struct {
>>  	u64 xstate_bv;
>>  	u64 reserved1[2];
>> @@ -380,6 +389,9 @@ struct xsave_struct {
>>  	struct i387_fxsave_struct i387;
>>  	struct xsave_hdr_struct xsave_hdr;
>>  	struct ymmh_struct ymmh;
>> +	u8 lwp_area[128];
> 
> Sorry for the back-and-forth, but I think this and the removal of
> XSTATE_FLEXIBLE (perhaps XSTATE_LAZY?) makes your v2 worse than v1.
> 
> Since Peter already said the same, please undo these changes.
> 
> Also, how is XSTATE_EAGER used?  Should MPX be disabled when xsaveopt
> is disabled on the kernel command line?  (Liu, how would this affect
> the KVM patches, too?)
> 
> Paolo

Currently seems no, and if needed we can add a new patch at kvm side accordingly when native mpx patches checked in.

Jinsong

> 
>> +#define XSTATE_EAGER	(XSTATE_BNDREGS | XSTATE_BNDCSR)  /*
>>   * 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_EAGER) 
>> 
>>  #ifdef CONFIG_X86_64
>>  #define REX_PREFIX	"0x48, "

RE: [Qemu-devel] [PATCH v2 3/3] X86, mpx: Intel MPX xstate feature definition

[Liu, Jinsong]

Paolo Bonzini wrote:
> Il 07/12/2013 01:20, Qiaowei Ren ha scritto:
>> This patch defines xstate feature and extends struct xsave_hdr_struct
>> to support Intel MPX.
>> 
>> Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com>
>> Signed-off-by: Xudong Hao <xudong.hao@intel.com>
>> Signed-off-by: Liu Jinsong <jinsong.liu@intel.com>
>> ---
>>  arch/x86/include/asm/processor.h |   12 ++++++++++++
>>  arch/x86/include/asm/xsave.h     |    5 ++++-
>>  2 files changed, 16 insertions(+), 1 deletions(-)
>> 
>> diff --git a/arch/x86/include/asm/processor.h
>> b/arch/x86/include/asm/processor.h index 987c75e..2fe2e75 100644 ---
>> a/arch/x86/include/asm/processor.h +++
>> b/arch/x86/include/asm/processor.h @@ -370,6 +370,15 @@ struct
>>  	ymmh_struct { u32 ymmh_space[64];
>>  };
>> 
>> +struct bndregs_struct {
>> +	u64 bndregs[8];
>> +} __packed;
>> +
>> +struct bndcsr_struct {
>> +	u64 cfg_reg_u;
>> +	u64 status_reg;
>> +} __packed;
>> +
>>  struct xsave_hdr_struct {
>>  	u64 xstate_bv;
>>  	u64 reserved1[2];
>> @@ -380,6 +389,9 @@ struct xsave_struct {
>>  	struct i387_fxsave_struct i387;
>>  	struct xsave_hdr_struct xsave_hdr;
>>  	struct ymmh_struct ymmh;
>> +	u8 lwp_area[128];
> 
> Sorry for the back-and-forth, but I think this and the removal of
> XSTATE_FLEXIBLE (perhaps XSTATE_LAZY?) makes your v2 worse than v1.
> 
> Since Peter already said the same, please undo these changes.
> 
> Also, how is XSTATE_EAGER used?  Should MPX be disabled when xsaveopt
> is disabled on the kernel command line?  (Liu, how would this affect
> the KVM patches, too?)
> 
> Paolo

Currently seems no, and if needed we can add a new patch at kvm side accordingly when native mpx patches checked in.

Jinsong

> 
>> +#define XSTATE_EAGER	(XSTATE_BNDREGS | XSTATE_BNDCSR)  /*
>>   * 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_EAGER) 
>> 
>>  #ifdef CONFIG_X86_64
>>  #define REX_PREFIX	"0x48, "

Re: [Qemu-devel] [PATCH v2 3/3] X86, mpx: Intel MPX xstate feature definition

[H. Peter Anvin]

On 12/06/2013 12:05 PM, Liu, Jinsong wrote:
>>
>> Since Peter already said the same, please undo these changes.
>>
>> Also, how is XSTATE_EAGER used?  Should MPX be disabled when xsaveopt
>> is disabled on the kernel command line?  (Liu, how would this affect
>> the KVM patches, too?)
>>
>> Paolo
> 
> Currently seems no, and if needed we can add a new patch at kvm side accordingly when native mpx patches checked in.
> 

We need to either disable these features in lazy mode, or we need to
force eager mode if these features are to be supported.  The problem
with the latter is that it means forcing eager mode regardless of if
anything actually *uses* these features.

A third option would be to require applications to use a prctl() or
similar to enable eager-save features.

Thoughts?

	-hpa

Re: [Qemu-devel] [PATCH v2 3/3] X86, mpx: Intel MPX xstate feature definition

[H. Peter Anvin]

On 12/06/2013 12:05 PM, Liu, Jinsong wrote:
>>
>> Since Peter already said the same, please undo these changes.
>>
>> Also, how is XSTATE_EAGER used?  Should MPX be disabled when xsaveopt
>> is disabled on the kernel command line?  (Liu, how would this affect
>> the KVM patches, too?)
>>
>> Paolo
> 
> Currently seems no, and if needed we can add a new patch at kvm side accordingly when native mpx patches checked in.
> 

We need to either disable these features in lazy mode, or we need to
force eager mode if these features are to be supported.  The problem
with the latter is that it means forcing eager mode regardless of if
anything actually *uses* these features.

A third option would be to require applications to use a prctl() or
similar to enable eager-save features.

Thoughts?

	-hpa

Re: [PATCH v2 3/3] X86, mpx: Intel MPX xstate feature definition

[Liu, Jinsong]

H. Peter Anvin wrote:
> On 12/06/2013 12:05 PM, Liu, Jinsong wrote:
>>> 
>>> Since Peter already said the same, please undo these changes.
>>> 
>>> Also, how is XSTATE_EAGER used?  Should MPX be disabled when
>>> xsaveopt is disabled on the kernel command line?  (Liu, how would
>>> this affect the KVM patches, too?) 
>>> 
>>> Paolo
>> 
>> Currently seems no, and if needed we can add a new patch at kvm side
>> accordingly when native mpx patches checked in. 
>> 
> 
> We need to either disable these features in lazy mode, or we need to
> force eager mode if these features are to be supported.  The problem
> with the latter is that it means forcing eager mode regardless of if
> anything actually *uses* these features.
> 
> A third option would be to require applications to use a prctl() or
> similar to enable eager-save features.
> 
> Thoughts?
> 
> 	-hpa

The third option seems better -- how does native mpx patches work, force eager?

Thanks,
Jinsong

Re: [Qemu-devel] [PATCH v2 3/3] X86, mpx: Intel MPX xstate feature definition

[Liu, Jinsong]

H. Peter Anvin wrote:
> On 12/06/2013 12:05 PM, Liu, Jinsong wrote:
>>> 
>>> Since Peter already said the same, please undo these changes.
>>> 
>>> Also, how is XSTATE_EAGER used?  Should MPX be disabled when
>>> xsaveopt is disabled on the kernel command line?  (Liu, how would
>>> this affect the KVM patches, too?) 
>>> 
>>> Paolo
>> 
>> Currently seems no, and if needed we can add a new patch at kvm side
>> accordingly when native mpx patches checked in. 
>> 
> 
> We need to either disable these features in lazy mode, or we need to
> force eager mode if these features are to be supported.  The problem
> with the latter is that it means forcing eager mode regardless of if
> anything actually *uses* these features.
> 
> A third option would be to require applications to use a prctl() or
> similar to enable eager-save features.
> 
> Thoughts?
> 
> 	-hpa

The third option seems better -- how does native mpx patches work, force eager?

Thanks,
Jinsong

RE: [Qemu-devel] [PATCH v2 3/3] X86, mpx: Intel MPX xstate feature definition

[Liu, Jinsong]

H. Peter Anvin wrote:
> On 12/06/2013 12:05 PM, Liu, Jinsong wrote:
>>> 
>>> Since Peter already said the same, please undo these changes.
>>> 
>>> Also, how is XSTATE_EAGER used?  Should MPX be disabled when
>>> xsaveopt is disabled on the kernel command line?  (Liu, how would
>>> this affect the KVM patches, too?) 
>>> 
>>> Paolo
>> 
>> Currently seems no, and if needed we can add a new patch at kvm side
>> accordingly when native mpx patches checked in. 
>> 
> 
> We need to either disable these features in lazy mode, or we need to
> force eager mode if these features are to be supported.  The problem
> with the latter is that it means forcing eager mode regardless of if
> anything actually *uses* these features.
> 
> A third option would be to require applications to use a prctl() or
> similar to enable eager-save features.
> 
> Thoughts?
> 
> 	-hpa

The third option seems better -- how does native mpx patches work, force eager?

Thanks,
Jinsong

Re: [PATCH v2 3/3] X86, mpx: Intel MPX xstate feature definition

[Ren, Qiaowei]

> -----Original Message-----
> From: Liu, Jinsong
> Sent: Saturday, December 07, 2013 6:13 AM
> To: H. Peter Anvin; Paolo Bonzini; Ren, Qiaowei
> Cc: kvm@vger.kernel.org; x86@kernel.org; Xudong Hao;
> qemu-devel@nongnu.org; linux-kernel@vger.kernel.org; Ingo Molnar; Thomas
> Gleixner
> Subject: RE: [Qemu-devel] [PATCH v2 3/3] X86, mpx: Intel MPX xstate feature
> definition
> 
> H. Peter Anvin wrote:
> > On 12/06/2013 12:05 PM, Liu, Jinsong wrote:
> >>>
> >>> Since Peter already said the same, please undo these changes.
> >>>
> >>> Also, how is XSTATE_EAGER used?  Should MPX be disabled when
> >>> xsaveopt is disabled on the kernel command line?  (Liu, how would
> >>> this affect the KVM patches, too?)
> >>>
> >>> Paolo
> >>
> >> Currently seems no, and if needed we can add a new patch at kvm side
> >> accordingly when native mpx patches checked in.
> >>
> >
> > We need to either disable these features in lazy mode, or we need to
> > force eager mode if these features are to be supported.  The problem
> > with the latter is that it means forcing eager mode regardless of if
> > anything actually *uses* these features.
> >
> > A third option would be to require applications to use a prctl() or
> > similar to enable eager-save features.
> >
> > Thoughts?
> >
> > 	-hpa
> 
> The third option seems better -- how does native mpx patches work, force
> eager?
> 
It should be the second option, as you can see xsave.c which we remove from this patch. :)

Thanks,
Qiaowei

Re: [Qemu-devel] [PATCH v2 3/3] X86, mpx: Intel MPX xstate feature definition

[Ren, Qiaowei]

> -----Original Message-----
> From: Liu, Jinsong
> Sent: Saturday, December 07, 2013 6:13 AM
> To: H. Peter Anvin; Paolo Bonzini; Ren, Qiaowei
> Cc: kvm@vger.kernel.org; x86@kernel.org; Xudong Hao;
> qemu-devel@nongnu.org; linux-kernel@vger.kernel.org; Ingo Molnar; Thomas
> Gleixner
> Subject: RE: [Qemu-devel] [PATCH v2 3/3] X86, mpx: Intel MPX xstate feature
> definition
> 
> H. Peter Anvin wrote:
> > On 12/06/2013 12:05 PM, Liu, Jinsong wrote:
> >>>
> >>> Since Peter already said the same, please undo these changes.
> >>>
> >>> Also, how is XSTATE_EAGER used?  Should MPX be disabled when
> >>> xsaveopt is disabled on the kernel command line?  (Liu, how would
> >>> this affect the KVM patches, too?)
> >>>
> >>> Paolo
> >>
> >> Currently seems no, and if needed we can add a new patch at kvm side
> >> accordingly when native mpx patches checked in.
> >>
> >
> > We need to either disable these features in lazy mode, or we need to
> > force eager mode if these features are to be supported.  The problem
> > with the latter is that it means forcing eager mode regardless of if
> > anything actually *uses* these features.
> >
> > A third option would be to require applications to use a prctl() or
> > similar to enable eager-save features.
> >
> > Thoughts?
> >
> > 	-hpa
> 
> The third option seems better -- how does native mpx patches work, force
> eager?
> 
It should be the second option, as you can see xsave.c which we remove from this patch. :)

Thanks,
Qiaowei

RE: [Qemu-devel] [PATCH v2 3/3] X86, mpx: Intel MPX xstate feature definition

[Ren, Qiaowei]

> -----Original Message-----
> From: Liu, Jinsong
> Sent: Saturday, December 07, 2013 6:13 AM
> To: H. Peter Anvin; Paolo Bonzini; Ren, Qiaowei
> Cc: kvm@vger.kernel.org; x86@kernel.org; Xudong Hao;
> qemu-devel@nongnu.org; linux-kernel@vger.kernel.org; Ingo Molnar; Thomas
> Gleixner
> Subject: RE: [Qemu-devel] [PATCH v2 3/3] X86, mpx: Intel MPX xstate feature
> definition
> 
> H. Peter Anvin wrote:
> > On 12/06/2013 12:05 PM, Liu, Jinsong wrote:
> >>>
> >>> Since Peter already said the same, please undo these changes.
> >>>
> >>> Also, how is XSTATE_EAGER used?  Should MPX be disabled when
> >>> xsaveopt is disabled on the kernel command line?  (Liu, how would
> >>> this affect the KVM patches, too?)
> >>>
> >>> Paolo
> >>
> >> Currently seems no, and if needed we can add a new patch at kvm side
> >> accordingly when native mpx patches checked in.
> >>
> >
> > We need to either disable these features in lazy mode, or we need to
> > force eager mode if these features are to be supported.  The problem
> > with the latter is that it means forcing eager mode regardless of if
> > anything actually *uses* these features.
> >
> > A third option would be to require applications to use a prctl() or
> > similar to enable eager-save features.
> >
> > Thoughts?
> >
> > 	-hpa
> 
> The third option seems better -- how does native mpx patches work, force
> eager?
> 
It should be the second option, as you can see xsave.c which we remove from this patch. :)

Thanks,
Qiaowei

Re: [PATCH v2 3/3] X86, mpx: Intel MPX xstate feature definition

[H. Peter Anvin]

On 12/06/2013 04:23 PM, Ren, Qiaowei wrote:
>>>
>>> We need to either disable these features in lazy mode, or we need to
>>> force eager mode if these features are to be supported.  The problem
>>> with the latter is that it means forcing eager mode regardless of if
>>> anything actually *uses* these features.
>>>
>>> A third option would be to require applications to use a prctl() or
>>> similar to enable eager-save features.
>>>
>>
>> The third option seems better -- how does native mpx patches work, force
>> eager?
>>
> It should be the second option, as you can see xsave.c which we remove from this patch. :)
> 

Ah yes... I missed the fact that that chunk had been dropped from this
patch.  It really shouldn't be.

I'll substitute the previous version of the patch.

	-hpa

Re: [Qemu-devel] [PATCH v2 3/3] X86, mpx: Intel MPX xstate feature definition

[H. Peter Anvin]

On 12/06/2013 04:23 PM, Ren, Qiaowei wrote:
>>>
>>> We need to either disable these features in lazy mode, or we need to
>>> force eager mode if these features are to be supported.  The problem
>>> with the latter is that it means forcing eager mode regardless of if
>>> anything actually *uses* these features.
>>>
>>> A third option would be to require applications to use a prctl() or
>>> similar to enable eager-save features.
>>>
>>
>> The third option seems better -- how does native mpx patches work, force
>> eager?
>>
> It should be the second option, as you can see xsave.c which we remove from this patch. :)
> 

Ah yes... I missed the fact that that chunk had been dropped from this
patch.  It really shouldn't be.

I'll substitute the previous version of the patch.

	-hpa

Re: [Qemu-devel] [PATCH v2 3/3] X86, mpx: Intel MPX xstate feature definition

[H. Peter Anvin]

On 12/06/2013 04:23 PM, Ren, Qiaowei wrote:
>>>
>>> We need to either disable these features in lazy mode, or we need to
>>> force eager mode if these features are to be supported.  The problem
>>> with the latter is that it means forcing eager mode regardless of if
>>> anything actually *uses* these features.
>>>
>>> A third option would be to require applications to use a prctl() or
>>> similar to enable eager-save features.
>>>
>>
>> The third option seems better -- how does native mpx patches work, force
>> eager?
>>
> It should be the second option, as you can see xsave.c which we remove from this patch. :)
> 

Ah yes... I missed the fact that that chunk had been dropped from this
patch.  It really shouldn't be.

I'll substitute the previous version of the patch.

	-hpa

RE: [Qemu-devel] [PATCH v2 3/3] X86, mpx: Intel MPX xstate feature definition

[Ren, Qiaowei]

> -----Original Message-----
> From: H. Peter Anvin [mailto:hpa@zytor.com]
> Sent: Saturday, December 07, 2013 9:07 AM
> To: Ren, Qiaowei; Liu, Jinsong; Paolo Bonzini
> Cc: kvm@vger.kernel.org; x86@kernel.org; Xudong Hao;
> qemu-devel@nongnu.org; linux-kernel@vger.kernel.org; Ingo Molnar; Thomas
> Gleixner
> Subject: Re: [Qemu-devel] [PATCH v2 3/3] X86, mpx: Intel MPX xstate feature
> definition
> 
> On 12/06/2013 04:23 PM, Ren, Qiaowei wrote:
> >>>
> >>> We need to either disable these features in lazy mode, or we need to
> >>> force eager mode if these features are to be supported.  The problem
> >>> with the latter is that it means forcing eager mode regardless of if
> >>> anything actually *uses* these features.
> >>>
> >>> A third option would be to require applications to use a prctl() or
> >>> similar to enable eager-save features.
> >>>
> >>
> >> The third option seems better -- how does native mpx patches work,
> >> force eager?
> >>
> > It should be the second option, as you can see xsave.c which we remove
> > from this patch. :)
> >
> 
> Ah yes... I missed the fact that that chunk had been dropped from this patch.
> It really shouldn't be.
> 
> I'll substitute the previous version of the patch.
> 
Jinsong think that both kvm and host depend on these feature definition header file, so we firstly submit these files depended on.

Thanks,
Qiaowei

Re: [Qemu-devel] [PATCH v2 3/3] X86, mpx: Intel MPX xstate feature definition

[Ren, Qiaowei]

> -----Original Message-----
> From: H. Peter Anvin [mailto:hpa@zytor.com]
> Sent: Saturday, December 07, 2013 9:07 AM
> To: Ren, Qiaowei; Liu, Jinsong; Paolo Bonzini
> Cc: kvm@vger.kernel.org; x86@kernel.org; Xudong Hao;
> qemu-devel@nongnu.org; linux-kernel@vger.kernel.org; Ingo Molnar; Thomas
> Gleixner
> Subject: Re: [Qemu-devel] [PATCH v2 3/3] X86, mpx: Intel MPX xstate feature
> definition
> 
> On 12/06/2013 04:23 PM, Ren, Qiaowei wrote:
> >>>
> >>> We need to either disable these features in lazy mode, or we need to
> >>> force eager mode if these features are to be supported.  The problem
> >>> with the latter is that it means forcing eager mode regardless of if
> >>> anything actually *uses* these features.
> >>>
> >>> A third option would be to require applications to use a prctl() or
> >>> similar to enable eager-save features.
> >>>
> >>
> >> The third option seems better -- how does native mpx patches work,
> >> force eager?
> >>
> > It should be the second option, as you can see xsave.c which we remove
> > from this patch. :)
> >
> 
> Ah yes... I missed the fact that that chunk had been dropped from this patch.
> It really shouldn't be.
> 
> I'll substitute the previous version of the patch.
> 
Jinsong think that both kvm and host depend on these feature definition header file, so we firstly submit these files depended on.

Thanks,
Qiaowei

RE: [Qemu-devel] [PATCH v2 3/3] X86, mpx: Intel MPX xstate feature definition

[Ren, Qiaowei]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1539 bytes --]



> -----Original Message-----
> From: H. Peter Anvin [mailto:hpa@zytor.com]
> Sent: Saturday, December 07, 2013 9:07 AM
> To: Ren, Qiaowei; Liu, Jinsong; Paolo Bonzini
> Cc: kvm@vger.kernel.org; x86@kernel.org; Xudong Hao;
> qemu-devel@nongnu.org; linux-kernel@vger.kernel.org; Ingo Molnar; Thomas
> Gleixner
> Subject: Re: [Qemu-devel] [PATCH v2 3/3] X86, mpx: Intel MPX xstate feature
> definition
> 
> On 12/06/2013 04:23 PM, Ren, Qiaowei wrote:
> >>>
> >>> We need to either disable these features in lazy mode, or we need to
> >>> force eager mode if these features are to be supported.  The problem
> >>> with the latter is that it means forcing eager mode regardless of if
> >>> anything actually *uses* these features.
> >>>
> >>> A third option would be to require applications to use a prctl() or
> >>> similar to enable eager-save features.
> >>>
> >>
> >> The third option seems better -- how does native mpx patches work,
> >> force eager?
> >>
> > It should be the second option, as you can see xsave.c which we remove
> > from this patch. :)
> >
> 
> Ah yes... I missed the fact that that chunk had been dropped from this patch.
> It really shouldn't be.
> 
> I'll substitute the previous version of the patch.
> 
Jinsong think that both kvm and host depend on these feature definition header file, so we firstly submit these files depended on.

Thanks,
Qiaowei

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [Qemu-devel] [PATCH v2 3/3] X86, mpx: Intel MPX xstate feature definition

[H. Peter Anvin]

On 12/06/2013 05:16 PM, Ren, Qiaowei wrote:
> Jinsong think that both kvm and host depend on these feature definition header file, so we firstly submit these files depended on.

Yes, but we can't turn on the feature without proper protection.  Either
way, they are now in tip:x86/cpufeature.

	-hpa

Re: [Qemu-devel] [PATCH v2 3/3] X86, mpx: Intel MPX xstate feature definition

[H. Peter Anvin]

On 12/06/2013 05:16 PM, Ren, Qiaowei wrote:
> Jinsong think that both kvm and host depend on these feature definition header file, so we firstly submit these files depended on.

Yes, but we can't turn on the feature without proper protection.  Either
way, they are now in tip:x86/cpufeature.

	-hpa

[tip:x86/cpufeature] x86, xsave: Support eager-only xsave features, add MPX support

[tip-bot for Qiaowei Ren]

Commit-ID:  e7d820a5e549b3eb6c3f9467507566565646a669
Gitweb:     http://git.kernel.org/tip/e7d820a5e549b3eb6c3f9467507566565646a669
Author:     Qiaowei Ren <qiaowei.ren@intel.com>
AuthorDate: Thu, 5 Dec 2013 17:15:34 +0800
Committer:  H. Peter Anvin <hpa@linux.intel.com>
CommitDate: Fri, 6 Dec 2013 17:17:42 -0800

x86, xsave: Support eager-only xsave features, add MPX support

Some features, like Intel MPX, work only if the kernel uses eagerfpu
model.  So we should force eagerfpu on unless the user has explicitly
disabled it.

Add definitions for Intel MPX and add it to the supported list.

[ hpa: renamed XSTATE_FLEXIBLE to XSTATE_LAZY and added comments ]

Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com>
Link: http://lkml.kernel.org/r/9E0BE1322F2F2246BD820DA9FC397ADE014A6115@SHSMSX102.ccr.corp.intel.com
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
---
 arch/x86/include/asm/processor.h | 23 +++++++++++++++++++++++
 arch/x86/include/asm/xsave.h     | 14 ++++++++++----
 arch/x86/kernel/xsave.c          | 10 ++++++++++
 3 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 7b034a4..b7845a1 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -370,6 +370,26 @@ struct ymmh_struct {
 	u32 ymmh_space[64];
 };
 
+struct lwp_struct {
+	u64 lwpcb_addr;
+	u32 flags;
+	u32 buf_head_offset;
+	u64 buf_base;
+	u32 buf_size;
+	u32 filters;
+	u64 saved_event_record[4];
+	u32 event_counter[16];
+};
+
+struct bndregs_struct {
+	u64 bndregs[8];
+} __packed;
+
+struct bndcsr_struct {
+	u64 cfg_reg_u;
+	u64 status_reg;
+} __packed;
+
 struct xsave_hdr_struct {
 	u64 xstate_bv;
 	u64 reserved1[2];
@@ -380,6 +400,9 @@ struct xsave_struct {
 	struct i387_fxsave_struct i387;
 	struct xsave_hdr_struct xsave_hdr;
 	struct ymmh_struct ymmh;
+	struct lwp_struct lwp;
+	struct bndregs_struct bndregs;
+	struct bndcsr_struct bndcsr;
 	/* new processor state extensions will go here */
 } __attribute__ ((packed, aligned (64)));
 
diff --git a/arch/x86/include/asm/xsave.h b/arch/x86/include/asm/xsave.h
index 0415cda..5547389 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)
 
@@ -20,10 +22,14 @@
 #define XSAVE_YMM_SIZE	    256
 #define XSAVE_YMM_OFFSET    (XSAVE_HDR_SIZE + XSAVE_HDR_OFFSET)
 
-/*
- * These are the features that the OS can handle currently.
- */
-#define XCNTXT_MASK	(XSTATE_FP | XSTATE_SSE | XSTATE_YMM)
+/* Supported features which support lazy state saving */
+#define XSTATE_LAZY	(XSTATE_FP | XSTATE_SSE | XSTATE_YMM)
+
+/* Supported features which require eager state saving */
+#define XSTATE_EAGER	(XSTATE_BNDREGS | XSTATE_BNDCSR)
+
+/* All currently supported features */
+#define XCNTXT_MASK	(XSTATE_LAZY | XSTATE_EAGER)
 
 #ifdef CONFIG_X86_64
 #define REX_PREFIX	"0x48, "
diff --git a/arch/x86/kernel/xsave.c b/arch/x86/kernel/xsave.c
index 422fd82..a4b451c 100644
--- a/arch/x86/kernel/xsave.c
+++ b/arch/x86/kernel/xsave.c
@@ -562,6 +562,16 @@ static void __init xstate_enable_boot_cpu(void)
 	if (cpu_has_xsaveopt && eagerfpu != DISABLE)
 		eagerfpu = ENABLE;
 
+	if (pcntxt_mask & XSTATE_EAGER) {
+		if (eagerfpu == DISABLE) {
+			pr_err("eagerfpu not present, disabling some xstate features: 0x%llx\n",
+					pcntxt_mask & XSTATE_EAGER);
+			pcntxt_mask &= ~XSTATE_EAGER;
+		} else {
+			eagerfpu = ENABLE;
+		}
+	}
+
 	pr_info("enabled xstate_bv 0x%llx, cntxt size 0x%x\n",
 		pcntxt_mask, xstate_size);
 }