[PATCH 01/19] x86, mpx, xsave: Fix up bad get_xsave_addr() assumptions

2015-06-07 Thread Dave Hansen

From: Dave Hansen 

get_xsave_addr() assumes that if an xsave bit is present in the
hardware (pcntxt_mask) that it is present in a given xsave
buffer.  Due to an bug in the xsave code on all of the systems
that have MPX (and thus all the users of this code), that has
been a true assumption.

But, the bug is getting fixed, so our assumption is not going
to hold any more.

It's quite possible (and normal) for an enabled state to be
present on 'pcntxt_mask', but *not* in 'xstate_bv'.  We need
to consult 'xstate_bv'.

Signed-off-by: Dave Hansen 
Reviewed-by: Thomas Gleixner 
---

 b/arch/x86/kernel/fpu/xstate.c |   45 +
 1 file changed, 37 insertions(+), 8 deletions(-)

diff -puN arch/x86/kernel/fpu/xstate.c~consullt-xstate_bv 
arch/x86/kernel/fpu/xstate.c
--- a/arch/x86/kernel/fpu/xstate.c~consullt-xstate_bv   2015-06-01 
10:24:03.025676699 -0700
+++ b/arch/x86/kernel/fpu/xstate.c  2015-06-01 10:24:03.029676880 -0700
@@ -382,19 +382,48 @@ void fpu__resume_cpu(void)
  * This is the API that is called to get xstate address in either
  * standard format or compacted format of xsave area.
  *
+ * Note that if there is no data for the field in the xsave buffer
+ * this will return NULL.
+ *
  * Inputs:
- * xsave: base address of the xsave area;
- * xstate: state which is defined in xsave.h (e.g. XSTATE_FP, XSTATE_SSE,
- * etc.)
+ * xstate: the thread's storage area for all FPU data
+ * xstate_feature: state which is defined in xsave.h (e.g.
+ * XSTATE_FP, XSTATE_SSE, etc...)
  * Output:
- * address of the state in the xsave area.
+ * address of the state in the xsave area, or NULL if the
+ * field is not present in the xsave buffer.
  */
-void *get_xsave_addr(struct xregs_state *xsave, int xstate)
+void *get_xsave_addr(struct xregs_state *xsave, int xstate_feature)
 {
-   int feature = fls64(xstate) - 1;
-   if (!test_bit(feature, (unsigned long *)&xfeatures_mask))
+   int feature_nr = fls64(xstate_feature) - 1;
+   /*
+* Do we even *have* xsave state?
+*/
+   if (!boot_cpu_has(X86_FEATURE_XSAVE))
+   return NULL;
+
+   xsave = ¤t->thread.fpu.state.xsave;
+   /*
+* We should not ever be requesting features that we
+* have not enabled.  Remember that pcntxt_mask is
+* what we write to the XCR0 register.
+*/
+   WARN_ONCE(!(xfeatures_mask & xstate_feature),
+ "get of unsupported state");
+   /*
+* This assumes the last 'xsave*' instruction to
+* have requested that 'xstate_feature' be saved.
+* If it did not, we might be seeing and old value
+* of the field in the buffer.
+*
+* This can happen because the last 'xsave' did not
+* request that this feature be saved (unlikely)
+* or because the "init optimization" caused it
+* to not be saved.
+*/
+   if (!(xsave->header.xfeatures & xstate_feature))
return NULL;
 
-   return (void *)xsave + xstate_comp_offsets[feature];
+   return (void *)xsave + xstate_comp_offsets[feature_nr];
 }
 EXPORT_SYMBOL_GPL(get_xsave_addr);
_
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 01/19] x86, mpx, xsave: Fix up bad get_xsave_addr() assumptions

2015-05-29 Thread Dave Hansen

From: Dave Hansen 

get_xsave_addr() assumes that if an xsave bit is present in the
hardware (pcntxt_mask) that it is present in a given xsave
buffer.  Due to an bug in the xsave code on all of the systems
that have MPX (and thus all the users of this code), that has
been a true assumption.

But, the bug is getting fixed, so our assumption is not going
to hold any more.

It's quite possible (and normal) for an enabled state to be
present on 'pcntxt_mask', but *not* in 'xstate_bv'.  We need
to consult 'xstate_bv'.

Signed-off-by: Dave Hansen 
Reviewed-by: Thomas Gleixner 
---

 b/arch/x86/kernel/fpu/xstate.c |   45 +
 1 file changed, 37 insertions(+), 8 deletions(-)

diff -puN arch/x86/kernel/fpu/xstate.c~consullt-xstate_bv 
arch/x86/kernel/fpu/xstate.c
--- a/arch/x86/kernel/fpu/xstate.c~consullt-xstate_bv   2015-05-27 
09:32:14.540445571 -0700
+++ b/arch/x86/kernel/fpu/xstate.c  2015-05-27 09:32:14.543445706 -0700
@@ -382,19 +382,48 @@ void fpu__resume_cpu(void)
  * This is the API that is called to get xstate address in either
  * standard format or compacted format of xsave area.
  *
+ * Note that if there is no data for the field in the xsave buffer
+ * this will return NULL.
+ *
  * Inputs:
- * xsave: base address of the xsave area;
- * xstate: state which is defined in xsave.h (e.g. XSTATE_FP, XSTATE_SSE,
- * etc.)
+ * xstate: the thread's storage area for all FPU data
+ * xstate_feature: state which is defined in xsave.h (e.g.
+ * XSTATE_FP, XSTATE_SSE, etc...)
  * Output:
- * address of the state in the xsave area.
+ * address of the state in the xsave area, or NULL if the
+ * field is not present in the xsave buffer.
  */
-void *get_xsave_addr(struct xregs_state *xsave, int xstate)
+void *get_xsave_addr(struct xregs_state *xsave, int xstate_feature)
 {
-   int feature = fls64(xstate) - 1;
-   if (!test_bit(feature, (unsigned long *)&xfeatures_mask))
+   int feature_nr = fls64(xstate_feature) - 1;
+   /*
+* Do we even *have* xsave state?
+*/
+   if (!boot_cpu_has(X86_FEATURE_XSAVE))
+   return NULL;
+
+   xsave = ¤t->thread.fpu.state.xsave;
+   /*
+* We should not ever be requesting features that we
+* have not enabled.  Remember that pcntxt_mask is
+* what we write to the XCR0 register.
+*/
+   WARN_ONCE(!(xfeatures_mask & xstate_feature),
+ "get of unsupported state");
+   /*
+* This assumes the last 'xsave*' instruction to
+* have requested that 'xstate_feature' be saved.
+* If it did not, we might be seeing and old value
+* of the field in the buffer.
+*
+* This can happen because the last 'xsave' did not
+* request that this feature be saved (unlikely)
+* or because the "init optimization" caused it
+* to not be saved.
+*/
+   if (!(xsave->header.xfeatures & xstate_feature))
return NULL;
 
-   return (void *)xsave + xstate_comp_offsets[feature];
+   return (void *)xsave + xstate_comp_offsets[feature_nr];
 }
 EXPORT_SYMBOL_GPL(get_xsave_addr);
_
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 01/19] x86, mpx, xsave: Fix up bad get_xsave_addr() assumptions

2015-05-27 Thread Dave Hansen

From: Dave Hansen 

get_xsave_addr() assumes that if an xsave bit is present in the
hardware (pcntxt_mask) that it is present in a given xsave
buffer.  Due to an bug in the xsave code on all of the systems
that have MPX (and thus all the users of this code), that has
been a true assumption.

But, the bug is getting fixed, so our assumption is not going
to hold any more.

It's quite possible (and normal) for an enabled state to be
present on 'pcntxt_mask', but *not* in 'xstate_bv'.  We need
to consult 'xstate_bv'.

Signed-off-by: Dave Hansen 
Reviewed-by: Thomas Gleixner 
---

 b/arch/x86/kernel/fpu/xstate.c |   45 +
 1 file changed, 37 insertions(+), 8 deletions(-)

diff -puN arch/x86/kernel/fpu/xstate.c~consullt-xstate_bv 
arch/x86/kernel/fpu/xstate.c
--- a/arch/x86/kernel/fpu/xstate.c~consullt-xstate_bv   2015-05-27 
09:32:14.540445571 -0700
+++ b/arch/x86/kernel/fpu/xstate.c  2015-05-27 09:32:14.543445706 -0700
@@ -382,19 +382,48 @@ void fpu__resume_cpu(void)
  * This is the API that is called to get xstate address in either
  * standard format or compacted format of xsave area.
  *
+ * Note that if there is no data for the field in the xsave buffer
+ * this will return NULL.
+ *
  * Inputs:
- * xsave: base address of the xsave area;
- * xstate: state which is defined in xsave.h (e.g. XSTATE_FP, XSTATE_SSE,
- * etc.)
+ * xstate: the thread's storage area for all FPU data
+ * xstate_feature: state which is defined in xsave.h (e.g.
+ * XSTATE_FP, XSTATE_SSE, etc...)
  * Output:
- * address of the state in the xsave area.
+ * address of the state in the xsave area, or NULL if the
+ * field is not present in the xsave buffer.
  */
-void *get_xsave_addr(struct xregs_state *xsave, int xstate)
+void *get_xsave_addr(struct xregs_state *xsave, int xstate_feature)
 {
-   int feature = fls64(xstate) - 1;
-   if (!test_bit(feature, (unsigned long *)&xfeatures_mask))
+   int feature_nr = fls64(xstate_feature) - 1;
+   /*
+* Do we even *have* xsave state?
+*/
+   if (!boot_cpu_has(X86_FEATURE_XSAVE))
+   return NULL;
+
+   xsave = ¤t->thread.fpu.state.xsave;
+   /*
+* We should not ever be requesting features that we
+* have not enabled.  Remember that pcntxt_mask is
+* what we write to the XCR0 register.
+*/
+   WARN_ONCE(!(xfeatures_mask & xstate_feature),
+ "get of unsupported state");
+   /*
+* This assumes the last 'xsave*' instruction to
+* have requested that 'xstate_feature' be saved.
+* If it did not, we might be seeing and old value
+* of the field in the buffer.
+*
+* This can happen because the last 'xsave' did not
+* request that this feature be saved (unlikely)
+* or because the "init optimization" caused it
+* to not be saved.
+*/
+   if (!(xsave->header.xfeatures & xstate_feature))
return NULL;
 
-   return (void *)xsave + xstate_comp_offsets[feature];
+   return (void *)xsave + xstate_comp_offsets[feature_nr];
 }
 EXPORT_SYMBOL_GPL(get_xsave_addr);
_
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 01/19] x86, mpx, xsave: Fix up bad get_xsave_addr() assumptions

2015-05-18 Thread Dave Hansen

From: Dave Hansen 

get_xsave_addr() assumes that if an xsave bit is present in the
hardware (pcntxt_mask) that it is present in a given xsave
buffer.  Due to an bug in the xsave code on all of the systems
that have MPX (and thus all the users of this code), that has
been a true assumption.

But, the bug is getting fixed, so our assumption is not going
to hold any more.

It's quite possible (and normal) for an enabled state to be
present on 'pcntxt_mask', but *not* in 'xstate_bv'.  We need
to consult 'xstate_bv'.

Signed-off-by: Dave Hansen 
Reviewed-by: Thomas Gleixner 
---

 b/arch/x86/kernel/xsave.c |   44 
 1 file changed, 36 insertions(+), 8 deletions(-)

diff -puN arch/x86/kernel/xsave.c~consullt-xstate_bv arch/x86/kernel/xsave.c
--- a/arch/x86/kernel/xsave.c~consullt-xstate_bv2015-05-18 
17:48:57.839373364 -0700
+++ b/arch/x86/kernel/xsave.c   2015-05-18 17:48:57.843373544 -0700
@@ -706,19 +706,47 @@ void __init_refok eager_fpu_init(void)
  * This is the API that is called to get xstate address in either
  * standard format or compacted format of xsave area.
  *
+ * Note that if there is no data for the field in the xsave buffer
+ * this will return NULL.
+ *
  * Inputs:
- * xsave: base address of the xsave area;
- * xstate: state which is defined in xsave.h (e.g. XSTATE_FP, XSTATE_SSE,
- * etc.)
+ * xstate: the thread's storage area for all FPU data
+ * xstate_field: state which is defined in xsave.h (e.g. XSTATE_FP,
+ * XSTATE_SSE, etc...)
  * Output:
- * address of the state in the xsave area.
+ * address of the state in the xsave area, or NULL if the
+ * field is not present in the xsave buffer.
  */
-void *get_xsave_addr(struct xsave_struct *xsave, int xstate)
+void *get_xsave_addr(struct xsave_struct *xsave, int xstate_field)
 {
-   int feature = fls64(xstate) - 1;
-   if (!test_bit(feature, (unsigned long *)&pcntxt_mask))
+   int feature_nr = fls64(xstate_field) - 1;
+   /*
+* Do we even *have* xsave state?
+*/
+   if (!boot_cpu_has(X86_FEATURE_XSAVE))
+   return NULL;
+
+   xsave = ¤t->thread.fpu.state->xsave;
+   /*
+* We should not ever be requesting fields that we
+* have not enabled.  Remember that pcntxt_mask is
+* what we write to the XCR0 register.
+*/
+   WARN_ONCE(!(pcntxt_mask & xstate_field), "get of unsupported state");
+   /*
+* This assumes the last 'xsave*' instruction to
+* have requested that 'xstate_field' be saved.
+* If it did not, we might be seeing and old value
+* of the field in the buffer.
+*
+* This can happen because the last 'xsave' did not
+* request that this feature be saved (unlikely)
+* or because the "init optimization" caused it
+* to not be saved.
+*/
+   if (!(xsave->xsave_hdr.xstate_bv & xstate_field))
return NULL;
 
-   return (void *)xsave + xstate_comp_offsets[feature];
+   return (void *)xsave + xstate_comp_offsets[feature_nr];
 }
 EXPORT_SYMBOL_GPL(get_xsave_addr);
_
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 01/19] x86, mpx, xsave: fix up bad get_xsave_addr() assumptions

2015-05-18 Thread Thomas Gleixner
On Fri, 8 May 2015, Dave Hansen wrote:
> From: Dave Hansen 
> 
> get_xsave_addr() assumes that if an xsave bit is present in the
> hardware (pcntxt_mask) that it is present in a given xsave
> buffer.  Due to an bug in the xsave code on all of the systems
> that have MPX (and thus all the users of this code), that has
> been a true assumption.
> 
> But, the bug is getting fixed, so our assumption is not going
> to hold any more.
> 
> It's quite possible (and normal) for an enabled state to be
> present on 'pcntxt_mask', but *not* in 'xstate_bv'.  We need
> to consult 'xstate_bv'.
> 
> Signed-off-by: Dave Hansen 

Reviewed-by: Thomas Gleixner 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 01/19] x86, mpx, xsave: fix up bad get_xsave_addr() assumptions

2015-05-08 Thread Dave Hansen

From: Dave Hansen 

get_xsave_addr() assumes that if an xsave bit is present in the
hardware (pcntxt_mask) that it is present in a given xsave
buffer.  Due to an bug in the xsave code on all of the systems
that have MPX (and thus all the users of this code), that has
been a true assumption.

But, the bug is getting fixed, so our assumption is not going
to hold any more.

It's quite possible (and normal) for an enabled state to be
present on 'pcntxt_mask', but *not* in 'xstate_bv'.  We need
to consult 'xstate_bv'.

Signed-off-by: Dave Hansen 
---

 b/arch/x86/kernel/xsave.c |   41 ++---
 1 file changed, 34 insertions(+), 7 deletions(-)

diff -puN arch/x86/kernel/xsave.c~consullt-xstate_bv arch/x86/kernel/xsave.c
--- a/arch/x86/kernel/xsave.c~consullt-xstate_bv2015-05-08 
11:46:10.595563814 -0700
+++ b/arch/x86/kernel/xsave.c   2015-05-08 11:46:10.598563949 -0700
@@ -706,19 +706,46 @@ void __init_refok eager_fpu_init(void)
  * This is the API that is called to get xstate address in either
  * standard format or compacted format of xsave area.
  *
+ * Note that if there is no data for the field in the xsave buffer
+ * this will return NULL.
+ *
  * Inputs:
- * xsave: base address of the xsave area;
- * xstate: state which is defined in xsave.h (e.g. XSTATE_FP, XSTATE_SSE,
- * etc.)
+ * xstate: the thread's storage area for all FPU data
+ * xstate_field: state which is defined in xsave.h (e.g. XSTATE_FP,
+ * XSTATE_SSE, etc...)
  * Output:
  * address of the state in the xsave area.
  */
-void *get_xsave_addr(struct xsave_struct *xsave, int xstate)
+void *get_xsave_addr(struct xsave_struct *xsave, int xstate_field)
 {
-   int feature = fls64(xstate) - 1;
-   if (!test_bit(feature, (unsigned long *)&pcntxt_mask))
+   int feature_nr = fls64(xstate_field) - 1;
+   /*
+* Do we even *have* xsave state?
+*/
+   if (!boot_cpu_has(X86_FEATURE_XSAVE))
+   return NULL;
+
+   xsave = ¤t->thread.fpu.state->xsave;
+   /*
+* We should not ever be requesting fields that we
+* have not enabled.  Remember that pcntxt_mask is
+* what we write to the XCR0 register.
+*/
+   WARN_ONCE(!(pcntxt_mask & xstate_field), "get of unsupported state");
+   /*
+* This assumes the last 'xsave*' instruction to
+* have requested that 'xstate_field' be saved.
+* If it did not, we might be seeing and old value
+* of the field in the buffer.
+*
+* This can happen because the last 'xsave' did not
+* request that this feature be saved (unlikely)
+* or because the "init optimization" caused it
+* to not be saved.
+*/
+   if (!(xsave->xsave_hdr.xstate_bv & xstate_field))
return NULL;
 
-   return (void *)xsave + xstate_comp_offsets[feature];
+   return (void *)xsave + xstate_comp_offsets[feature_nr];
 }
 EXPORT_SYMBOL_GPL(get_xsave_addr);
_
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/