[PATCH 0/6] Support XSAVES supervisor states

[PATCH 0/6] Support XSAVES supervisor states

[Yu-cheng Yu]

There are two types of XSAVE-managed states (xstates): user and supervisor.
This series introduces the supervisor xstate support in preparation for new
features that will make use of supervisor xstates.

This series has been separated for ease of review from the series that add
supervisor xstate features [3].

In current and near future generations of Intel processors there are three
classes of objects that can be managed as supervisor xstates:

- Processor Trace (PT):
    Linux already supports PT, but PT xstates are not saved to the FPU
    context and not context-switched by the kernel.  There are no plans to
    integrate PT into XSAVES supervisor states.

- ENQCMD Process Address Space ID (MSR_IA32_PASID):
    ENQCMD is a new instruction and will be introduced shortly in a
    separate series [2].

- Control-flow Enforcement Technology (CET):
    CET is being reviewed on the LKML [2] [3].

Supervisor xstates can be accessed only from the kernel (PL-0) with XSAVES/
XRSTORS instructions.  They cannot be accessed with other XSAVE*/XRSTOR*
instructions.  MSR_IA32_XSS sets enabled supervisor xstates, while XCR0
sets enabled user xstates.

This series separates the two xstate types by declaring new macros for each
type.  The kernel finds all available features during system initialization
and stores them in xfeatures_mask_all.  It then retrieves perspective
xstate type with xfeatures_mask_supervisor()/xfeatures_mask_user() for
handling signals and PTRACE.

[1] Detailed information on supervisor xstates can be found in "Intel 64
    and IA-32 Architectures Software Developer's Manual":

    https://software.intel.com/en-us/download/intel-64-and-ia-32-
    architectures-sdm-combined-volumes-1-2a-2b-2c-2d-3a-3b-3c-3d-and-4

[2] Detailed information on CET, the ENQCMD instruction and MSR_IA32_PASID
    can be found in "Intel Architecture Instruction Set Extensions and
    Future Features Programming Reference":

    https://software.intel.com/sites/default/files/managed/c5/15/
    architecture-instruction-set-extensions-programming-reference.pdf

[3] CET patches:

    https://lkml.kernel.org/r/20190813205225.12032-1-yu-cheng.yu@intel.com/
    https://lkml.kernel.org/r/20190813205359.12196-1-yu-cheng.yu@intel.com/

Fenghua Yu (3):
  x86/fpu/xstate: Define new macros for supervisor and user xstates
  x86/fpu/xstate: Define new functions for clearing fpregs and xstates
  x86/fpu/xstate: Rename validate_xstate_header() to
    validate_xstate_header_from_user()

Yu-cheng Yu (3):
  x86/fpu/xstate: Fix small issues before adding supervisor xstates
  x86/fpu/xstate: Separate user and supervisor xfeatures mask
  x86/fpu/xstate: Introduce XSAVES supervisor states

 arch/x86/include/asm/fpu/internal.h |   5 +-
 arch/x86/include/asm/fpu/xstate.h   |  46 ++++++----
 arch/x86/kernel/fpu/core.c          |  32 ++++---
 arch/x86/kernel/fpu/init.c          |   3 +-
 arch/x86/kernel/fpu/regset.c        |   2 +-
 arch/x86/kernel/fpu/signal.c        |  21 +++--
 arch/x86/kernel/fpu/xstate.c        | 131 ++++++++++++++++------------
 arch/x86/kernel/process.c           |   2 +-
 arch/x86/kernel/signal.c            |   2 +-
 9 files changed, 149 insertions(+), 95 deletions(-)

-- 
2.17.1

[PATCH 1/6] x86/fpu/xstate: Fix small issues before adding supervisor xstates

[Yu-cheng Yu]

In response to earlier comments, fix small issues before introducing XSAVES
supervisor states:
- Add spaces around '*'.
- Fix comments of xfeature_is_supervisor().
- Replace ((u64)1 << 63) with XCOMP_BV_COMPACTED_FORMAT.

No functional changes from this patch.

Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
---
 arch/x86/kernel/fpu/xstate.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index e5cb67d67c03..b793fc2156b9 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -60,7 +60,7 @@ u64 xfeatures_mask __read_mostly;
 
 static unsigned int xstate_offsets[XFEATURE_MAX] = { [ 0 ... XFEATURE_MAX - 1] = -1};
 static unsigned int xstate_sizes[XFEATURE_MAX]   = { [ 0 ... XFEATURE_MAX - 1] = -1};
-static unsigned int xstate_comp_offsets[sizeof(xfeatures_mask)*8];
+static unsigned int xstate_comp_offsets[sizeof(xfeatures_mask) * 8];
 
 /*
  * The XSAVE area of kernel can be in standard or compacted format;
@@ -110,12 +110,8 @@ EXPORT_SYMBOL_GPL(cpu_has_xfeatures);
 static int xfeature_is_supervisor(int xfeature_nr)
 {
 	/*
-	 * We currently do not support supervisor states, but if
-	 * we did, we could find out like this.
-	 *
-	 * SDM says: If state component 'i' is a user state component,
-	 * ECX[0] return 0; if state component i is a supervisor
-	 * state component, ECX[0] returns 1.
+	 * Extended State Enumeration Sub-leaves (EAX = 0DH, ECX = n, n > 1)
+	 * returns ECX[0] set to (1) for a supervisor state.
 	 */
 	u32 eax, ebx, ecx, edx;
 
@@ -342,7 +338,7 @@ static int xfeature_is_aligned(int xfeature_nr)
  */
 static void __init setup_xstate_comp(void)
 {
-	unsigned int xstate_comp_sizes[sizeof(xfeatures_mask)*8];
+	unsigned int xstate_comp_sizes[sizeof(xfeatures_mask) * 8];
 	int i;
 
 	/*
@@ -415,7 +411,8 @@ static void __init setup_init_fpu_buf(void)
 	print_xstate_features();
 
 	if (boot_cpu_has(X86_FEATURE_XSAVES))
-		init_fpstate.xsave.header.xcomp_bv = (u64)1 << 63 | xfeatures_mask;
+		init_fpstate.xsave.header.xcomp_bv = XCOMP_BV_COMPACTED_FORMAT |
+						     xfeatures_mask;
 
 	/*
 	 * Init all the features state with header.xfeatures being 0x0
-- 
2.17.1

Re: [PATCH 1/6] x86/fpu/xstate: Fix small issues before adding supervisor xstates

[Borislav Petkov]

On Wed, Sep 25, 2019 at 08:10:17AM -0700, Yu-cheng Yu wrote:
> In response to earlier comments, fix small issues before introducing XSAVES
> supervisor states:
> - Add spaces around '*'.
> - Fix comments of xfeature_is_supervisor().
> - Replace ((u64)1 << 63) with XCOMP_BV_COMPACTED_FORMAT.
> 
> No functional changes from this patch.
> 
> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>
> Reviewed-by: Tony Luck <tony.luck@intel.com>
> ---
>  arch/x86/kernel/fpu/xstate.c | 15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
> index e5cb67d67c03..b793fc2156b9 100644
> --- a/arch/x86/kernel/fpu/xstate.c
> +++ b/arch/x86/kernel/fpu/xstate.c
> @@ -60,7 +60,7 @@ u64 xfeatures_mask __read_mostly;
>  
>  static unsigned int xstate_offsets[XFEATURE_MAX] = { [ 0 ... XFEATURE_MAX - 1] = -1};
>  static unsigned int xstate_sizes[XFEATURE_MAX]   = { [ 0 ... XFEATURE_MAX - 1] = -1};
> -static unsigned int xstate_comp_offsets[sizeof(xfeatures_mask)*8];
> +static unsigned int xstate_comp_offsets[sizeof(xfeatures_mask) * 8];
>  
>  /*
>   * The XSAVE area of kernel can be in standard or compacted format;
> @@ -110,12 +110,8 @@ EXPORT_SYMBOL_GPL(cpu_has_xfeatures);
>  static int xfeature_is_supervisor(int xfeature_nr)
>  {
>  	/*
> -	 * We currently do not support supervisor states, but if
> -	 * we did, we could find out like this.
> -	 *
> -	 * SDM says: If state component 'i' is a user state component,
> -	 * ECX[0] return 0; if state component i is a supervisor
> -	 * state component, ECX[0] returns 1.
> +	 * Extended State Enumeration Sub-leaves (EAX = 0DH, ECX = n, n > 1)
> +	 * returns ECX[0] set to (1) for a supervisor state.

"... and cleared (0) for a user state."

I believe it is is clearer this way.

>  	 */
>  	u32 eax, ebx, ecx, edx;
>  

Since you're touching this function: make it return bool as it is used
in boolean context only and have it return simply:

	return ecx & 1;

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH 1/6] x86/fpu/xstate: Fix small issues before adding supervisor xstates

[Yu-cheng Yu]

On Wed, 2019-10-09 at 08:58 -0700, Borislav Petkov wrote:
> On Wed, Sep 25, 2019 at 08:10:17AM -0700, Yu-cheng Yu wrote:
> > In response to earlier comments, fix small issues before introducing XSAVES
> > supervisor states:
> > - Add spaces around '*'.
> > - Fix comments of xfeature_is_supervisor().
> > - Replace ((u64)1 << 63) with XCOMP_BV_COMPACTED_FORMAT.
> > 
> > No functional changes from this patch.
> > 
> > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> > Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>
> > Reviewed-by: Tony Luck <tony.luck@intel.com>
> > ---
> >  arch/x86/kernel/fpu/xstate.c | 15 ++++++---------
> >  1 file changed, 6 insertions(+), 9 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
> > index e5cb67d67c03..b793fc2156b9 100644
> > --- a/arch/x86/kernel/fpu/xstate.c
> > +++ b/arch/x86/kernel/fpu/xstate.c
> > @@ -60,7 +60,7 @@ u64 xfeatures_mask __read_mostly;
> > 
> >  static unsigned int xstate_offsets[XFEATURE_MAX] = { [ 0 ... XFEATURE_MAX - 1] = -1};
> >  static unsigned int xstate_sizes[XFEATURE_MAX]   = { [ 0 ... XFEATURE_MAX - 1] = -1};
> > -static unsigned int xstate_comp_offsets[sizeof(xfeatures_mask)*8];
> > +static unsigned int xstate_comp_offsets[sizeof(xfeatures_mask) * 8];
> > 
> >  /*
> >   * The XSAVE area of kernel can be in standard or compacted format;
> > @@ -110,12 +110,8 @@ EXPORT_SYMBOL_GPL(cpu_has_xfeatures);
> >  static int xfeature_is_supervisor(int xfeature_nr)
> >  {
> >       /*
> > -      * We currently do not support supervisor states, but if
> > -      * we did, we could find out like this.
> > -      *
> > -      * SDM says: If state component 'i' is a user state component,
> > -      * ECX[0] return 0; if state component i is a supervisor
> > -      * state component, ECX[0] returns 1.
> > +      * Extended State Enumeration Sub-leaves (EAX = 0DH, ECX = n, n > 1)
> > +      * returns ECX[0] set to (1) for a supervisor state.
> 
> "... and cleared (0) for a user state."
> 
> I believe it is is clearer this way.
> 
> >        */
> >       u32 eax, ebx, ecx, edx;
> > 
> 
> Since you're touching this function: make it return bool as it is used
> in boolean context only and have it return simply:
> 
>         return ecx & 1;

This implicitly converts a u32 to a bool.  By looking at it, I think it should
be OK, but wonder if anything overlooked?  I would be happy to make this a
separate patch.

Yu-cheng

Re: [PATCH 1/6] x86/fpu/xstate: Fix small issues before adding supervisor xstates

[Borislav Petkov]

On Fri, Nov 22, 2019 at 01:22:37PM -0800, Yu-cheng Yu wrote:
> This implicitly converts a u32 to a bool. By looking at it, I think it
> should be OK, but wonder if anything overlooked? I would be happy to
> make this a separate patch.

I don't understand your question: are you asking whether you should
convert the functions to "static bool" or what is your question?

If it is that, then yes pls. Also, change xfeature_is_user() to return
bool too, while at it.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

[PATCH 2/6] x86/fpu/xstate: Define new macros for supervisor and user xstates

[Yu-cheng Yu]

From: Fenghua Yu <fenghua.yu@intel.com>

XCNTXT_MASK is 'all supported xfeatures' before introducing supervisor
xstates.  It is hereby renamed to SUPPORTED_XFEATURES_MASK_USER to make it
clear that these are user xstates.

XFEATURE_MASK_SUPERVISOR is replaced with the following:
- SUPPORTED_XFEATURES_MASK_SUPERVISOR: Currently nothing.  ENQCMD and
  Control-flow Enforcement Technology (CET) will be introduced in separate
  series.
- UNSUPPORTED_XFEATURES_MASK_SUPERVISOR: Currently only Processor Trace.
- ALL_XFEATURES_MASK_SUPERVISOR: the combination of above.

Co-developed-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
---
 arch/x86/include/asm/fpu/xstate.h | 36 ++++++++++++++++++++-----------
 arch/x86/kernel/fpu/init.c        |  3 ++-
 arch/x86/kernel/fpu/xstate.c      | 26 +++++++++++-----------
 3 files changed, 38 insertions(+), 27 deletions(-)

diff --git a/arch/x86/include/asm/fpu/xstate.h b/arch/x86/include/asm/fpu/xstate.h
index c6136d79f8c0..014c386deaa3 100644
--- a/arch/x86/include/asm/fpu/xstate.h
+++ b/arch/x86/include/asm/fpu/xstate.h
@@ -21,19 +21,29 @@
 #define XSAVE_YMM_SIZE	    256
 #define XSAVE_YMM_OFFSET    (XSAVE_HDR_SIZE + XSAVE_HDR_OFFSET)
 
-/* Supervisor features */
-#define XFEATURE_MASK_SUPERVISOR (XFEATURE_MASK_PT)
-
-/* All currently supported features */
-#define XCNTXT_MASK		(XFEATURE_MASK_FP | \
-				 XFEATURE_MASK_SSE | \
-				 XFEATURE_MASK_YMM | \
-				 XFEATURE_MASK_OPMASK | \
-				 XFEATURE_MASK_ZMM_Hi256 | \
-				 XFEATURE_MASK_Hi16_ZMM	 | \
-				 XFEATURE_MASK_PKRU | \
-				 XFEATURE_MASK_BNDREGS | \
-				 XFEATURE_MASK_BNDCSR)
+/* All currently supported user features */
+#define SUPPORTED_XFEATURES_MASK_USER (XFEATURE_MASK_FP | \
+				       XFEATURE_MASK_SSE | \
+				       XFEATURE_MASK_YMM | \
+				       XFEATURE_MASK_OPMASK | \
+				       XFEATURE_MASK_ZMM_Hi256 | \
+				       XFEATURE_MASK_Hi16_ZMM	 | \
+				       XFEATURE_MASK_PKRU | \
+				       XFEATURE_MASK_BNDREGS | \
+				       XFEATURE_MASK_BNDCSR)
+
+/* All currently supported supervisor features */
+#define SUPPORTED_XFEATURES_MASK_SUPERVISOR (0)
+
+/*
+ * Unsupported supervisor features. When a supervisor feature in this mask is
+ * supported in the future, move it to the supported supervisor feature mask.
+ */
+#define UNSUPPORTED_XFEATURES_MASK_SUPERVISOR (XFEATURE_MASK_PT)
+
+/* All supervisor states including supported and unsupported states. */
+#define ALL_XFEATURES_MASK_SUPERVISOR (SUPPORTED_XFEATURES_MASK_SUPERVISOR | \
+				       UNSUPPORTED_XFEATURES_MASK_SUPERVISOR)
 
 #ifdef CONFIG_X86_64
 #define REX_PREFIX	"0x48, "
diff --git a/arch/x86/kernel/fpu/init.c b/arch/x86/kernel/fpu/init.c
index 6ce7e0a23268..ba3705d25162 100644
--- a/arch/x86/kernel/fpu/init.c
+++ b/arch/x86/kernel/fpu/init.c
@@ -224,7 +224,8 @@ static void __init fpu__init_system_xstate_size_legacy(void)
  */
 u64 __init fpu__get_supported_xfeatures_mask(void)
 {
-	return XCNTXT_MASK;
+	return SUPPORTED_XFEATURES_MASK_USER |
+	       SUPPORTED_XFEATURES_MASK_SUPERVISOR;
 }
 
 /* Legacy code to initialize eager fpu mode. */
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index b793fc2156b9..4f99a747b4f1 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -212,14 +212,13 @@ void fpu__init_cpu_xstate(void)
 	if (!boot_cpu_has(X86_FEATURE_XSAVE) || !xfeatures_mask)
 		return;
 	/*
-	 * Make it clear that XSAVES supervisor states are not yet
-	 * implemented should anyone expect it to work by changing
-	 * bits in XFEATURE_MASK_* macros and XCR0.
+	 * Unsupported supervisor xstates should not be found in
+	 * the xfeatures mask.
 	 */
-	WARN_ONCE((xfeatures_mask & XFEATURE_MASK_SUPERVISOR),
-		"x86/fpu: XSAVES supervisor states are not yet implemented.\n");
+	WARN_ONCE((xfeatures_mask & UNSUPPORTED_XFEATURES_MASK_SUPERVISOR),
+		  "x86/fpu: Found unsupported supervisor xstates.\n");
 
-	xfeatures_mask &= ~XFEATURE_MASK_SUPERVISOR;
+	xfeatures_mask &= ~UNSUPPORTED_XFEATURES_MASK_SUPERVISOR;
 
 	cr4_set_bits(X86_CR4_OSXSAVE);
 	xsetbv(XCR_XFEATURE_ENABLED_MASK, xfeatures_mask);
@@ -435,7 +434,7 @@ static int xfeature_uncompacted_offset(int xfeature_nr)
 	 * format. Checking a supervisor state's uncompacted offset is
 	 * an error.
 	 */
-	if (XFEATURE_MASK_SUPERVISOR & BIT_ULL(xfeature_nr)) {
+	if (ALL_XFEATURES_MASK_SUPERVISOR & BIT_ULL(xfeature_nr)) {
 		WARN_ONCE(1, "No fixed offset for xstate %d\n", xfeature_nr);
 		return -1;
 	}
@@ -472,7 +471,7 @@ int using_compacted_format(void)
 int validate_xstate_header(const struct xstate_header *hdr)
 {
 	/* No unknown or supervisor features may be set */
-	if (hdr->xfeatures & (~xfeatures_mask | XFEATURE_MASK_SUPERVISOR))
+	if (hdr->xfeatures & ~(xfeatures_mask & SUPPORTED_XFEATURES_MASK_USER))
 		return -EINVAL;
 
 	/* Userspace must use the uncompacted format */
@@ -765,7 +764,8 @@ void __init fpu__init_system_xstate(void)
 	 * Update info used for ptrace frames; use standard-format size and no
 	 * supervisor xstates:
 	 */
-	update_regset_xstate_info(fpu_user_xstate_size,	xfeatures_mask & ~XFEATURE_MASK_SUPERVISOR);
+	update_regset_xstate_info(fpu_user_xstate_size,
+				  xfeatures_mask & SUPPORTED_XFEATURES_MASK_USER);
 
 	fpu__init_prepare_fx_sw_frame();
 	setup_init_fpu_buf();
@@ -988,7 +988,7 @@ int copy_xstate_to_kernel(void *kbuf, struct xregs_state *xsave, unsigned int of
 	 */
 	memset(&header, 0, sizeof(header));
 	header.xfeatures = xsave->header.xfeatures;
-	header.xfeatures &= ~XFEATURE_MASK_SUPERVISOR;
+	header.xfeatures &= SUPPORTED_XFEATURES_MASK_USER;
 
 	/*
 	 * Copy xregs_state->header:
@@ -1072,7 +1072,7 @@ int copy_xstate_to_user(void __user *ubuf, struct xregs_state *xsave, unsigned i
 	 */
 	memset(&header, 0, sizeof(header));
 	header.xfeatures = xsave->header.xfeatures;
-	header.xfeatures &= ~XFEATURE_MASK_SUPERVISOR;
+	header.xfeatures &= SUPPORTED_XFEATURES_MASK_USER;
 
 	/*
 	 * Copy xregs_state->header:
@@ -1165,7 +1165,7 @@ int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf)
 	 * The state that came in from userspace was user-state only.
 	 * Mask all the user states out of 'xfeatures':
 	 */
-	xsave->header.xfeatures &= XFEATURE_MASK_SUPERVISOR;
+	xsave->header.xfeatures &= ALL_XFEATURES_MASK_SUPERVISOR;
 
 	/*
 	 * Add back in the features that came in from userspace:
@@ -1221,7 +1221,7 @@ int copy_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf)
 	 * The state that came in from userspace was user-state only.
 	 * Mask all the user states out of 'xfeatures':
 	 */
-	xsave->header.xfeatures &= XFEATURE_MASK_SUPERVISOR;
+	xsave->header.xfeatures &= ALL_XFEATURES_MASK_SUPERVISOR;
 
 	/*
 	 * Add back in the features that came in from userspace:
-- 
2.17.1

Re: [PATCH 2/6] x86/fpu/xstate: Define new macros for supervisor and user xstates

[Borislav Petkov]

On Wed, Sep 25, 2019 at 08:10:18AM -0700, Yu-cheng Yu wrote:
> From: Fenghua Yu <fenghua.yu@intel.com>
> 
> XCNTXT_MASK is 'all supported xfeatures' before introducing supervisor
> xstates.  It is hereby renamed to SUPPORTED_XFEATURES_MASK_USER to make it
	    ^^^^^^^^^^^^^^^^^^^^

To quote straight from Documentation/process/submitting-patches.rst:

 "Describe your changes in imperative mood, e.g. "make xyzzy do frotz"
  instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy
  to do frotz", as if you are giving orders to the codebase to change
  its behaviour."

IOW, s/It is hereby renamed/Rename it/ - much simpler. Check all your
commit messages too pls.

> clear that these are user xstates.
> 
> XFEATURE_MASK_SUPERVISOR is replaced with the following:
> - SUPPORTED_XFEATURES_MASK_SUPERVISOR: Currently nothing.  ENQCMD and
>   Control-flow Enforcement Technology (CET) will be introduced in separate
>   series.
> - UNSUPPORTED_XFEATURES_MASK_SUPERVISOR: Currently only Processor Trace.
> - ALL_XFEATURES_MASK_SUPERVISOR: the combination of above.
> 
> Co-developed-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>

Your SOB needs to come after Fenghua's since you're sending the patch:

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
Co-developed-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>

This way, the SOB chain shows the path the patch has taken and who has
handled it along the way.

Check your other SOB chains too because they have the same/similar
issue.

> Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>
> Reviewed-by: Tony Luck <tony.luck@intel.com>
> ---
>  arch/x86/include/asm/fpu/xstate.h | 36 ++++++++++++++++++++-----------
>  arch/x86/kernel/fpu/init.c        |  3 ++-
>  arch/x86/kernel/fpu/xstate.c      | 26 +++++++++++-----------
>  3 files changed, 38 insertions(+), 27 deletions(-)
> 
> diff --git a/arch/x86/include/asm/fpu/xstate.h b/arch/x86/include/asm/fpu/xstate.h
> index c6136d79f8c0..014c386deaa3 100644
> --- a/arch/x86/include/asm/fpu/xstate.h
> +++ b/arch/x86/include/asm/fpu/xstate.h
> @@ -21,19 +21,29 @@
>  #define XSAVE_YMM_SIZE	    256
>  #define XSAVE_YMM_OFFSET    (XSAVE_HDR_SIZE + XSAVE_HDR_OFFSET)
>  
> -/* Supervisor features */
> -#define XFEATURE_MASK_SUPERVISOR (XFEATURE_MASK_PT)
> -
> -/* All currently supported features */
> -#define XCNTXT_MASK		(XFEATURE_MASK_FP | \
> -				 XFEATURE_MASK_SSE | \
> -				 XFEATURE_MASK_YMM | \
> -				 XFEATURE_MASK_OPMASK | \
> -				 XFEATURE_MASK_ZMM_Hi256 | \
> -				 XFEATURE_MASK_Hi16_ZMM	 | \
> -				 XFEATURE_MASK_PKRU | \
> -				 XFEATURE_MASK_BNDREGS | \
> -				 XFEATURE_MASK_BNDCSR)
> +/* All currently supported user features */
> +#define SUPPORTED_XFEATURES_MASK_USER (XFEATURE_MASK_FP | \
> +				       XFEATURE_MASK_SSE | \
> +				       XFEATURE_MASK_YMM | \
> +				       XFEATURE_MASK_OPMASK | \
> +				       XFEATURE_MASK_ZMM_Hi256 | \
> +				       XFEATURE_MASK_Hi16_ZMM	 | \
> +				       XFEATURE_MASK_PKRU | \
> +				       XFEATURE_MASK_BNDREGS | \
> +				       XFEATURE_MASK_BNDCSR)
> +
> +/* All currently supported supervisor features */
> +#define SUPPORTED_XFEATURES_MASK_SUPERVISOR (0)
> +
> +/*
> + * Unsupported supervisor features. When a supervisor feature in this mask is
> + * supported in the future, move it to the supported supervisor feature mask.
> + */
> +#define UNSUPPORTED_XFEATURES_MASK_SUPERVISOR (XFEATURE_MASK_PT)
> +
> +/* All supervisor states including supported and unsupported states. */
> +#define ALL_XFEATURES_MASK_SUPERVISOR (SUPPORTED_XFEATURES_MASK_SUPERVISOR | \
> +				       UNSUPPORTED_XFEATURES_MASK_SUPERVISOR)

Those are kinda too long for my taste but they're at least descriptive... :-)

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

[PATCH 3/6] x86/fpu/xstate: Separate user and supervisor xfeatures mask

[Yu-cheng Yu]

Before the introduction of XSAVES supervisor states, 'xfeatures_mask' is
used at various places to determine XSAVE buffer components and XCR0 bits.
It contains only user xstates.  To support supervisor xstates, it is
necessary to separate user and supervisor xstates:

- First, change 'xfeatures_mask' to 'xfeatures_mask_all', which represents
  the full set of bits that should ever be set in a kernel XSAVE buffer.
- Introduce xfeature_mask_supervisor() and xfeatures_mask_user() to
  extract relevant xfeatures from xfeatures_mask_all.

Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
---
 arch/x86/include/asm/fpu/internal.h |  2 +-
 arch/x86/include/asm/fpu/xstate.h   |  8 ++-
 arch/x86/kernel/fpu/signal.c        | 15 ++++--
 arch/x86/kernel/fpu/xstate.c        | 78 +++++++++++++++++------------
 4 files changed, 65 insertions(+), 38 deletions(-)

diff --git a/arch/x86/include/asm/fpu/internal.h b/arch/x86/include/asm/fpu/internal.h
index 4c95c365058a..f56ad1248b5d 100644
--- a/arch/x86/include/asm/fpu/internal.h
+++ b/arch/x86/include/asm/fpu/internal.h
@@ -92,7 +92,7 @@ static inline void fpstate_init_xstate(struct xregs_state *xsave)
 	 * XRSTORS requires these bits set in xcomp_bv, or it will
 	 * trigger #GP:
 	 */
-	xsave->header.xcomp_bv = XCOMP_BV_COMPACTED_FORMAT | xfeatures_mask;
+	xsave->header.xcomp_bv = XCOMP_BV_COMPACTED_FORMAT | xfeatures_mask_all;
 }
 
 static inline void fpstate_init_fxstate(struct fxregs_state *fx)
diff --git a/arch/x86/include/asm/fpu/xstate.h b/arch/x86/include/asm/fpu/xstate.h
index 014c386deaa3..5954bd97da16 100644
--- a/arch/x86/include/asm/fpu/xstate.h
+++ b/arch/x86/include/asm/fpu/xstate.h
@@ -51,7 +51,13 @@
 #define REX_PREFIX
 #endif
 
-extern u64 xfeatures_mask;
+extern u64 xfeatures_mask_all;
+
+static inline u64 xfeatures_mask_user(void)
+{
+	return xfeatures_mask_all & SUPPORTED_XFEATURES_MASK_USER;
+}
+
 extern u64 xstate_fx_sw_bytes[USER_XSTATE_FX_SW_WORDS];
 
 extern void __init update_regset_xstate_info(unsigned int size,
diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
index 0071b794ed19..c1f0688f35cf 100644
--- a/arch/x86/kernel/fpu/signal.c
+++ b/arch/x86/kernel/fpu/signal.c
@@ -254,11 +254,14 @@ static int copy_user_to_fpregs_zeroing(void __user *buf, u64 xbv, int fx_only)
 {
 	if (use_xsave()) {
 		if (fx_only) {
-			u64 init_bv = xfeatures_mask & ~XFEATURE_MASK_FPSSE;
+			u64 init_bv;
+
+			init_bv = xfeatures_mask_user() & ~XFEATURE_MASK_FPSSE;
 			copy_kernel_to_xregs(&init_fpstate.xsave, init_bv);
 			return copy_user_to_fxregs(buf);
 		} else {
-			u64 init_bv = xfeatures_mask & ~xbv;
+			u64 init_bv = xfeatures_mask_user() & ~xbv;
+
 			if (unlikely(init_bv))
 				copy_kernel_to_xregs(&init_fpstate.xsave, init_bv);
 			return copy_user_to_xregs(buf, xbv);
@@ -357,7 +360,7 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size)
 
 
 	if (use_xsave() && !fx_only) {
-		u64 init_bv = xfeatures_mask & ~xfeatures;
+		u64 init_bv = xfeatures_mask_user() & ~xfeatures;
 
 		if (using_compacted_format()) {
 			ret = copy_user_to_xstate(&fpu->state.xsave, buf_fx);
@@ -388,7 +391,9 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size)
 
 		fpregs_lock();
 		if (use_xsave()) {
-			u64 init_bv = xfeatures_mask & ~XFEATURE_MASK_FPSSE;
+			u64 init_bv;
+
+			init_bv = xfeatures_mask_user() & ~XFEATURE_MASK_FPSSE;
 			copy_kernel_to_xregs(&init_fpstate.xsave, init_bv);
 		}
 
@@ -462,7 +467,7 @@ void fpu__init_prepare_fx_sw_frame(void)
 
 	fx_sw_reserved.magic1 = FP_XSTATE_MAGIC1;
 	fx_sw_reserved.extended_size = size;
-	fx_sw_reserved.xfeatures = xfeatures_mask;
+	fx_sw_reserved.xfeatures = xfeatures_mask_user();
 	fx_sw_reserved.xstate_size = fpu_user_xstate_size;
 
 	if (IS_ENABLED(CONFIG_IA32_EMULATION) ||
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 4f99a747b4f1..b7f2808dd3f4 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -54,13 +54,14 @@ static short xsave_cpuid_features[] __initdata = {
 };
 
 /*
- * Mask of xstate features supported by the CPU and the kernel:
+ * This represents the full set of bits that should ever be set in a kernel
+ * XSAVE buffer, both supervisor and user xstates.
  */
-u64 xfeatures_mask __read_mostly;
+u64 xfeatures_mask_all __read_mostly;
 
 static unsigned int xstate_offsets[XFEATURE_MAX] = { [ 0 ... XFEATURE_MAX - 1] = -1};
 static unsigned int xstate_sizes[XFEATURE_MAX]   = { [ 0 ... XFEATURE_MAX - 1] = -1};
-static unsigned int xstate_comp_offsets[sizeof(xfeatures_mask) * 8];
+static unsigned int xstate_comp_offsets[sizeof(xfeatures_mask_all) * 8];
 
 /*
  * The XSAVE area of kernel can be in standard or compacted format;
@@ -76,7 +77,7 @@ unsigned int fpu_user_xstate_size;
  */
 int cpu_has_xfeatures(u64 xfeatures_needed, const char **feature_name)
 {
-	u64 xfeatures_missing = xfeatures_needed & ~xfeatures_mask;
+	u64 xfeatures_missing = xfeatures_needed & ~xfeatures_mask_all;
 
 	if (unlikely(feature_name)) {
 		long xfeature_idx, max_idx;
@@ -154,7 +155,7 @@ void fpstate_sanitize_xstate(struct fpu *fpu)
 	 * None of the feature bits are in init state. So nothing else
 	 * to do for us, as the memory layout is up to date.
 	 */
-	if ((xfeatures & xfeatures_mask) == xfeatures_mask)
+	if ((xfeatures & xfeatures_mask_all) == xfeatures_mask_all)
 		return;
 
 	/*
@@ -181,7 +182,7 @@ void fpstate_sanitize_xstate(struct fpu *fpu)
 	 * in a special way already:
 	 */
 	feature_bit = 0x2;
-	xfeatures = (xfeatures_mask & ~xfeatures) >> 2;
+	xfeatures = (xfeatures_mask_user() & ~xfeatures) >> 2;
 
 	/*
 	 * Update all the remaining memory layouts according to their
@@ -203,25 +204,35 @@ void fpstate_sanitize_xstate(struct fpu *fpu)
 	}
 }
 
+static inline u64 xfeatures_mask_supervisor(void)
+{
+	return xfeatures_mask_all & SUPPORTED_XFEATURES_MASK_SUPERVISOR;
+}
+
 /*
  * Enable the extended processor state save/restore feature.
  * Called once per CPU onlining.
  */
 void fpu__init_cpu_xstate(void)
 {
-	if (!boot_cpu_has(X86_FEATURE_XSAVE) || !xfeatures_mask)
+	if (!boot_cpu_has(X86_FEATURE_XSAVE) || !xfeatures_mask_all)
 		return;
 	/*
 	 * Unsupported supervisor xstates should not be found in
 	 * the xfeatures mask.
 	 */
-	WARN_ONCE((xfeatures_mask & UNSUPPORTED_XFEATURES_MASK_SUPERVISOR),
+	WARN_ONCE((xfeatures_mask_all & UNSUPPORTED_XFEATURES_MASK_SUPERVISOR),
 		  "x86/fpu: Found unsupported supervisor xstates.\n");
 
-	xfeatures_mask &= ~UNSUPPORTED_XFEATURES_MASK_SUPERVISOR;
+	xfeatures_mask_all &= ~UNSUPPORTED_XFEATURES_MASK_SUPERVISOR;
 
 	cr4_set_bits(X86_CR4_OSXSAVE);
-	xsetbv(XCR_XFEATURE_ENABLED_MASK, xfeatures_mask);
+	/*
+	 * XCR_XFEATURE_ENABLED_MASK (aka. XCR0) sets user features
+	 * managed by XSAVE{C, OPT, S} and XRSTOR{S}.  Only XSAVE user
+	 * states can be set here.
+	 */
+	xsetbv(XCR_XFEATURE_ENABLED_MASK, xfeatures_mask_user());
 }
 
 /*
@@ -231,7 +242,7 @@ void fpu__init_cpu_xstate(void)
  */
 static int xfeature_enabled(enum xfeature xfeature)
 {
-	return !!(xfeatures_mask & (1UL << xfeature));
+	return !!(xfeatures_mask_all & (1UL << xfeature));
 }
 
 /*
@@ -337,7 +348,7 @@ static int xfeature_is_aligned(int xfeature_nr)
  */
 static void __init setup_xstate_comp(void)
 {
-	unsigned int xstate_comp_sizes[sizeof(xfeatures_mask) * 8];
+	unsigned int xstate_comp_sizes[sizeof(xfeatures_mask_all) * 8];
 	int i;
 
 	/*
@@ -411,7 +422,7 @@ static void __init setup_init_fpu_buf(void)
 
 	if (boot_cpu_has(X86_FEATURE_XSAVES))
 		init_fpstate.xsave.header.xcomp_bv = XCOMP_BV_COMPACTED_FORMAT |
-						     xfeatures_mask;
+						     xfeatures_mask_all;
 
 	/*
 	 * Init all the features state with header.xfeatures being 0x0
@@ -471,7 +482,7 @@ int using_compacted_format(void)
 int validate_xstate_header(const struct xstate_header *hdr)
 {
 	/* No unknown or supervisor features may be set */
-	if (hdr->xfeatures & ~(xfeatures_mask & SUPPORTED_XFEATURES_MASK_USER))
+	if (hdr->xfeatures & ~xfeatures_mask_user())
 		return -EINVAL;
 
 	/* Userspace must use the uncompacted format */
@@ -606,7 +617,7 @@ static void do_extra_xstate_size_checks(void)
 
 
 /*
- * Get total size of enabled xstates in XCR0/xfeatures_mask.
+ * Get total size of enabled xstates in XCR0 | IA32_XSS.
  *
  * Note the SDM's wording here.  "sub-function 0" only enumerates
  * the size of the *user* states.  If we use it to size a buffer
@@ -696,7 +707,7 @@ static int __init init_xstate_size(void)
  */
 static void fpu__init_disable_system_xstate(void)
 {
-	xfeatures_mask = 0;
+	xfeatures_mask_all = 0;
 	cr4_clear_bits(X86_CR4_OSXSAVE);
 	setup_clear_cpu_cap(X86_FEATURE_XSAVE);
 }
@@ -731,16 +742,22 @@ void __init fpu__init_system_xstate(void)
 		return;
 	}
 
+	/*
+	 * Find user xstates supported by the processor.
+	 */
 	cpuid_count(XSTATE_CPUID, 0, &eax, &ebx, &ecx, &edx);
-	xfeatures_mask = eax + ((u64)edx << 32);
+	xfeatures_mask_all = eax + ((u64)edx << 32);
+
+	/* Place supervisor features in xfeatures_mask_all here */
 
-	if ((xfeatures_mask & XFEATURE_MASK_FPSSE) != XFEATURE_MASK_FPSSE) {
+	if ((xfeatures_mask_user() & XFEATURE_MASK_FPSSE) != XFEATURE_MASK_FPSSE) {
 		/*
 		 * This indicates that something really unexpected happened
 		 * with the enumeration.  Disable XSAVE and try to continue
 		 * booting without it.  This is too early to BUG().
 		 */
-		pr_err("x86/fpu: FP/SSE not present amongst the CPU's xstate features: 0x%llx.\n", xfeatures_mask);
+		pr_err("x86/fpu: FP/SSE not present amongst the CPU's xstate features: 0x%llx.\n",
+		       xfeatures_mask_all);
 		goto out_disable;
 	}
 
@@ -749,10 +766,10 @@ void __init fpu__init_system_xstate(void)
 	 */
 	for (i = 0; i < ARRAY_SIZE(xsave_cpuid_features); i++) {
 		if (!boot_cpu_has(xsave_cpuid_features[i]))
-			xfeatures_mask &= ~BIT(i);
+			xfeatures_mask_all &= ~BIT_ULL(i);
 	}
 
-	xfeatures_mask &= fpu__get_supported_xfeatures_mask();
+	xfeatures_mask_all &= fpu__get_supported_xfeatures_mask();
 
 	/* Enable xstate instructions to be able to continue with initialization: */
 	fpu__init_cpu_xstate();
@@ -764,16 +781,16 @@ void __init fpu__init_system_xstate(void)
 	 * Update info used for ptrace frames; use standard-format size and no
 	 * supervisor xstates:
 	 */
-	update_regset_xstate_info(fpu_user_xstate_size,
-				  xfeatures_mask & SUPPORTED_XFEATURES_MASK_USER);
+	update_regset_xstate_info(fpu_user_xstate_size, xfeatures_mask_user());
 
 	fpu__init_prepare_fx_sw_frame();
 	setup_init_fpu_buf();
 	setup_xstate_comp();
 	print_xstate_offset_size();
 
-	pr_info("x86/fpu: Enabled xstate features 0x%llx, context size is %d bytes, using '%s' format.\n",
-		xfeatures_mask,
+	pr_info("x86/fpu: Enabled xstate features 0x%llx: user 0x%llx, context size is %d bytes, using '%s' format.\n",
+		xfeatures_mask_all,
+		xfeatures_mask_user(),
 		fpu_kernel_xstate_size,
 		boot_cpu_has(X86_FEATURE_XSAVES) ? "compacted" : "standard");
 	return;
@@ -792,7 +809,7 @@ void fpu__resume_cpu(void)
 	 * Restore XCR0 on xsave capable CPUs:
 	 */
 	if (boot_cpu_has(X86_FEATURE_XSAVE))
-		xsetbv(XCR_XFEATURE_ENABLED_MASK, xfeatures_mask);
+		xsetbv(XCR_XFEATURE_ENABLED_MASK, xfeatures_mask_user());
 }
 
 /*
@@ -837,10 +854,9 @@ void *get_xsave_addr(struct xregs_state *xsave, int xfeature_nr)
 
 	/*
 	 * We should not ever be requesting features that we
-	 * have not enabled.  Remember that pcntxt_mask is
-	 * what we write to the XCR0 register.
+	 * have not enabled.
 	 */
-	WARN_ONCE(!(xfeatures_mask & BIT_ULL(xfeature_nr)),
+	WARN_ONCE(!(xfeatures_mask_all & BIT_ULL(xfeature_nr)),
 		  "get of unsupported state");
 	/*
 	 * This assumes the last 'xsave*' instruction to
@@ -988,7 +1004,7 @@ int copy_xstate_to_kernel(void *kbuf, struct xregs_state *xsave, unsigned int of
 	 */
 	memset(&header, 0, sizeof(header));
 	header.xfeatures = xsave->header.xfeatures;
-	header.xfeatures &= SUPPORTED_XFEATURES_MASK_USER;
+	header.xfeatures &= xfeatures_mask_user();
 
 	/*
 	 * Copy xregs_state->header:
@@ -1072,7 +1088,7 @@ int copy_xstate_to_user(void __user *ubuf, struct xregs_state *xsave, unsigned i
 	 */
 	memset(&header, 0, sizeof(header));
 	header.xfeatures = xsave->header.xfeatures;
-	header.xfeatures &= SUPPORTED_XFEATURES_MASK_USER;
+	header.xfeatures &= xfeatures_mask_user();
 
 	/*
 	 * Copy xregs_state->header:
-- 
2.17.1

Re: [PATCH 3/6] x86/fpu/xstate: Separate user and supervisor xfeatures mask

[Borislav Petkov]

On Wed, Sep 25, 2019 at 08:10:19AM -0700, Yu-cheng Yu wrote:
> Before the introduction of XSAVES supervisor states, 'xfeatures_mask' is
> used at various places to determine XSAVE buffer components and XCR0 bits.
> It contains only user xstates.  To support supervisor xstates, it is
> necessary to separate user and supervisor xstates:
> 
> - First, change 'xfeatures_mask' to 'xfeatures_mask_all', which represents
>   the full set of bits that should ever be set in a kernel XSAVE buffer.
> - Introduce xfeature_mask_supervisor() and xfeatures_mask_user() to

xfeatures_mask_supervisor()
	^

's' is missing.

>   extract relevant xfeatures from xfeatures_mask_all.
>
> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>

Similar problem here. Fenghua's SOB means what exactly?

You're probably trying to say that he co-developed it but then it needs
to state that this way:

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
Co-developed-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>

> Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>
> Reviewed-by: Tony Luck <tony.luck@intel.com>
> ---
>  arch/x86/include/asm/fpu/internal.h |  2 +-
>  arch/x86/include/asm/fpu/xstate.h   |  8 ++-
>  arch/x86/kernel/fpu/signal.c        | 15 ++++--
>  arch/x86/kernel/fpu/xstate.c        | 78 +++++++++++++++++------------
>  4 files changed, 65 insertions(+), 38 deletions(-)
> 
> diff --git a/arch/x86/include/asm/fpu/internal.h b/arch/x86/include/asm/fpu/internal.h
> index 4c95c365058a..f56ad1248b5d 100644
> --- a/arch/x86/include/asm/fpu/internal.h
> +++ b/arch/x86/include/asm/fpu/internal.h
> @@ -92,7 +92,7 @@ static inline void fpstate_init_xstate(struct xregs_state *xsave)
>  	 * XRSTORS requires these bits set in xcomp_bv, or it will
>  	 * trigger #GP:
>  	 */
> -	xsave->header.xcomp_bv = XCOMP_BV_COMPACTED_FORMAT | xfeatures_mask;
> +	xsave->header.xcomp_bv = XCOMP_BV_COMPACTED_FORMAT | xfeatures_mask_all;
>  }
>  
>  static inline void fpstate_init_fxstate(struct fxregs_state *fx)
> diff --git a/arch/x86/include/asm/fpu/xstate.h b/arch/x86/include/asm/fpu/xstate.h
> index 014c386deaa3..5954bd97da16 100644
> --- a/arch/x86/include/asm/fpu/xstate.h
> +++ b/arch/x86/include/asm/fpu/xstate.h
> @@ -51,7 +51,13 @@
>  #define REX_PREFIX
>  #endif
>  
> -extern u64 xfeatures_mask;
> +extern u64 xfeatures_mask_all;
> +
> +static inline u64 xfeatures_mask_user(void)
> +{
> +	return xfeatures_mask_all & SUPPORTED_XFEATURES_MASK_USER;
> +}

Please group this function along with xfeature_is_user(),
xfeature_is_supervisor() and xfeature_mask_supervisor() in the xstate.h
header as they're very similar in what they do.

> +
>  extern u64 xstate_fx_sw_bytes[USER_XSTATE_FX_SW_WORDS];
>  
>  extern void __init update_regset_xstate_info(unsigned int size,

...

> @@ -731,16 +742,22 @@ void __init fpu__init_system_xstate(void)
>  		return;
>  	}
>  
> +	/*
> +	 * Find user xstates supported by the processor.
> +	 */
>  	cpuid_count(XSTATE_CPUID, 0, &eax, &ebx, &ecx, &edx);
> -	xfeatures_mask = eax + ((u64)edx << 32);
> +	xfeatures_mask_all = eax + ((u64)edx << 32);
> +
> +	/* Place supervisor features in xfeatures_mask_all here */
>  

Remove that newline.

> -	if ((xfeatures_mask & XFEATURE_MASK_FPSSE) != XFEATURE_MASK_FPSSE) {
> +	if ((xfeatures_mask_user() & XFEATURE_MASK_FPSSE) != XFEATURE_MASK_FPSSE) {
>  		/*
>  		 * This indicates that something really unexpected happened
>  		 * with the enumeration.  Disable XSAVE and try to continue
>  		 * booting without it.  This is too early to BUG().
>  		 */
> -		pr_err("x86/fpu: FP/SSE not present amongst the CPU's xstate features: 0x%llx.\n", xfeatures_mask);
> +		pr_err("x86/fpu: FP/SSE not present amongst the CPU's xstate features: 0x%llx.\n",
> +		       xfeatures_mask_all);
>  		goto out_disable;
>  	}
>  
> @@ -749,10 +766,10 @@ void __init fpu__init_system_xstate(void)
>  	 */
>  	for (i = 0; i < ARRAY_SIZE(xsave_cpuid_features); i++) {
>  		if (!boot_cpu_has(xsave_cpuid_features[i]))
> -			xfeatures_mask &= ~BIT(i);
> +			xfeatures_mask_all &= ~BIT_ULL(i);
>  	}
>  
> -	xfeatures_mask &= fpu__get_supported_xfeatures_mask();
> +	xfeatures_mask_all &= fpu__get_supported_xfeatures_mask();
>  
>  	/* Enable xstate instructions to be able to continue with initialization: */
>  	fpu__init_cpu_xstate();
> @@ -764,16 +781,16 @@ void __init fpu__init_system_xstate(void)
>  	 * Update info used for ptrace frames; use standard-format size and no
>  	 * supervisor xstates:
>  	 */
> -	update_regset_xstate_info(fpu_user_xstate_size,
> -				  xfeatures_mask & SUPPORTED_XFEATURES_MASK_USER);
> +	update_regset_xstate_info(fpu_user_xstate_size, xfeatures_mask_user());
>  
>  	fpu__init_prepare_fx_sw_frame();
>  	setup_init_fpu_buf();
>  	setup_xstate_comp();
>  	print_xstate_offset_size();
>  
> -	pr_info("x86/fpu: Enabled xstate features 0x%llx, context size is %d bytes, using '%s' format.\n",
> -		xfeatures_mask,
> +	pr_info("x86/fpu: Enabled xstate features 0x%llx: user 0x%llx, context size is %d bytes, using '%s' format.\n",
> +		xfeatures_mask_all,
> +		xfeatures_mask_user(),

Why do we need to dump the user features separately?

>  		fpu_kernel_xstate_size,
>  		boot_cpu_has(X86_FEATURE_XSAVES) ? "compacted" : "standard");
>  	return;

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

[PATCH 4/6] x86/fpu/xstate: Introduce XSAVES supervisor states

[Yu-cheng Yu]

Enable XSAVES supervisor states by setting MSR_IA32_XSS bits according to
CPUID enumeration results.  Also revise comments at various places.

Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
---
 arch/x86/kernel/fpu/xstate.c | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index b7f2808dd3f4..a183c319d808 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -233,13 +233,14 @@ void fpu__init_cpu_xstate(void)
 	 * states can be set here.
 	 */
 	xsetbv(XCR_XFEATURE_ENABLED_MASK, xfeatures_mask_user());
+
+	/*
+	 * MSR_IA32_XSS sets supervisor states managed by XSAVES.
+	 */
+	if (boot_cpu_has(X86_FEATURE_XSAVES))
+		wrmsrl(MSR_IA32_XSS, xfeatures_mask_supervisor());
 }
 
-/*
- * Note that in the future we will likely need a pair of
- * functions here: one for user xstates and the other for
- * system xstates.  For now, they are the same.
- */
 static int xfeature_enabled(enum xfeature xfeature)
 {
 	return !!(xfeatures_mask_all & (1UL << xfeature));
@@ -623,9 +624,6 @@ static void do_extra_xstate_size_checks(void)
  * the size of the *user* states.  If we use it to size a buffer
  * that we use 'XSAVES' on, we could potentially overflow the
  * buffer because 'XSAVES' saves system states too.
- *
- * Note that we do not currently set any bits on IA32_XSS so
- * 'XCR0 | IA32_XSS == XCR0' for now.
  */
 static unsigned int __init get_xsaves_size(void)
 {
@@ -748,7 +746,11 @@ void __init fpu__init_system_xstate(void)
 	cpuid_count(XSTATE_CPUID, 0, &eax, &ebx, &ecx, &edx);
 	xfeatures_mask_all = eax + ((u64)edx << 32);
 
-	/* Place supervisor features in xfeatures_mask_all here */
+	/*
+	 * Find supervisor xstates supported by the processor.
+	 */
+	cpuid_count(XSTATE_CPUID, 1, &eax, &ebx, &ecx, &edx);
+	xfeatures_mask_all |= ecx + ((u64)edx << 32);
 
 	if ((xfeatures_mask_user() & XFEATURE_MASK_FPSSE) != XFEATURE_MASK_FPSSE) {
 		/*
@@ -788,9 +790,10 @@ void __init fpu__init_system_xstate(void)
 	setup_xstate_comp();
 	print_xstate_offset_size();
 
-	pr_info("x86/fpu: Enabled xstate features 0x%llx: user 0x%llx, context size is %d bytes, using '%s' format.\n",
+	pr_info("x86/fpu: Enabled xstate features 0x%llx: user 0x%llx and supervisor %llx, context size is %d bytes, using '%s' format.\n",
 		xfeatures_mask_all,
 		xfeatures_mask_user(),
+		xfeatures_mask_supervisor(),
 		fpu_kernel_xstate_size,
 		boot_cpu_has(X86_FEATURE_XSAVES) ? "compacted" : "standard");
 	return;
@@ -810,6 +813,13 @@ void fpu__resume_cpu(void)
 	 */
 	if (boot_cpu_has(X86_FEATURE_XSAVE))
 		xsetbv(XCR_XFEATURE_ENABLED_MASK, xfeatures_mask_user());
+
+	/*
+	 * Restore IA32_XSS. The same CPUID bit enumerates support
+	 * of XSAVES and MSR_IA32_XSS.
+	 */
+	if (boot_cpu_has(X86_FEATURE_XSAVES))
+		wrmsrl(MSR_IA32_XSS, xfeatures_mask_supervisor());
 }
 
 /*
-- 
2.17.1

Re: [PATCH 4/6] x86/fpu/xstate: Introduce XSAVES supervisor states

[Borislav Petkov]

On Wed, Sep 25, 2019 at 08:10:20AM -0700, Yu-cheng Yu wrote:
> Enable XSAVES supervisor states by setting MSR_IA32_XSS bits according to
> CPUID enumeration results.  Also revise comments at various places.
> 
> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>

Same issue as before.

> Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>
> Reviewed-by: Tony Luck <tony.luck@intel.com>
> ---
>  arch/x86/kernel/fpu/xstate.c | 30 ++++++++++++++++++++----------
>  1 file changed, 20 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
> index b7f2808dd3f4..a183c319d808 100644
> --- a/arch/x86/kernel/fpu/xstate.c
> +++ b/arch/x86/kernel/fpu/xstate.c
> @@ -233,13 +233,14 @@ void fpu__init_cpu_xstate(void)
>  	 * states can be set here.
>  	 */
>  	xsetbv(XCR_XFEATURE_ENABLED_MASK, xfeatures_mask_user());
> +
> +	/*
> +	 * MSR_IA32_XSS sets supervisor states managed by XSAVES.
> +	 */
> +	if (boot_cpu_has(X86_FEATURE_XSAVES))
> +		wrmsrl(MSR_IA32_XSS, xfeatures_mask_supervisor());
>  }
>  
> -/*
> - * Note that in the future we will likely need a pair of
> - * functions here: one for user xstates and the other for
> - * system xstates.  For now, they are the same.
> - */
>  static int xfeature_enabled(enum xfeature xfeature)
>  {
>  	return !!(xfeatures_mask_all & (1UL << xfeature));
> @@ -623,9 +624,6 @@ static void do_extra_xstate_size_checks(void)
>   * the size of the *user* states.  If we use it to size a buffer
>   * that we use 'XSAVES' on, we could potentially overflow the
>   * buffer because 'XSAVES' saves system states too.
> - *
> - * Note that we do not currently set any bits on IA32_XSS so
> - * 'XCR0 | IA32_XSS == XCR0' for now.
>   */
>  static unsigned int __init get_xsaves_size(void)
>  {
> @@ -748,7 +746,11 @@ void __init fpu__init_system_xstate(void)
>  	cpuid_count(XSTATE_CPUID, 0, &eax, &ebx, &ecx, &edx);
>  	xfeatures_mask_all = eax + ((u64)edx << 32);
>  
> -	/* Place supervisor features in xfeatures_mask_all here */
> +	/*
> +	 * Find supervisor xstates supported by the processor.
> +	 */
> +	cpuid_count(XSTATE_CPUID, 1, &eax, &ebx, &ecx, &edx);
> +	xfeatures_mask_all |= ecx + ((u64)edx << 32);
>  
>  	if ((xfeatures_mask_user() & XFEATURE_MASK_FPSSE) != XFEATURE_MASK_FPSSE) {
>  		/*
> @@ -788,9 +790,10 @@ void __init fpu__init_system_xstate(void)
>  	setup_xstate_comp();
>  	print_xstate_offset_size();
>  
> -	pr_info("x86/fpu: Enabled xstate features 0x%llx: user 0x%llx, context size is %d bytes, using '%s' format.\n",
> +	pr_info("x86/fpu: Enabled xstate features 0x%llx: user 0x%llx and supervisor %llx, context size is %d bytes, using '%s' format.\n",
>  		xfeatures_mask_all,
>  		xfeatures_mask_user(),
> +		xfeatures_mask_supervisor(),

So if you're dumping both user and supervisor, then you don't need to
dump _all anymore. Do it this way:

	pr_info("x86/fpu: Enabled xstate features: (U: 0x%llx, S: 0x%llx), context size: %d bytes, using '%s' format.\n",
  		xfeatures_mask_user(),
 		xfeatures_mask_supervisor(),

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

[PATCH 5/6] x86/fpu/xstate: Define new functions for clearing fpregs and xstates

[Yu-cheng Yu]

From: Fenghua Yu <fenghua.yu@intel.com>

Currently, fpu__clear() clears all fpregs and xstates.  Once XSAVES
supervisor states are introduced, supervisor settings must remain active
for signals; it is necessary to have separate functions:

 - Create fpu__clear_user_states(): clear only user settings for signals;
 - Create fpu__clear_all(): clear both user and supervisor settings in
   flush_thread().

Also modify copy_init_fpstate_to_fpregs() to take a mask from above two
functions.

Co-developed-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
---
 arch/x86/include/asm/fpu/internal.h |  3 ++-
 arch/x86/kernel/fpu/core.c          | 32 ++++++++++++++++++-----------
 arch/x86/kernel/fpu/signal.c        |  4 ++--
 arch/x86/kernel/process.c           |  2 +-
 arch/x86/kernel/signal.c            |  2 +-
 5 files changed, 26 insertions(+), 17 deletions(-)

diff --git a/arch/x86/include/asm/fpu/internal.h b/arch/x86/include/asm/fpu/internal.h
index f56ad1248b5d..ec08351fd5b4 100644
--- a/arch/x86/include/asm/fpu/internal.h
+++ b/arch/x86/include/asm/fpu/internal.h
@@ -31,7 +31,8 @@ extern void fpu__save(struct fpu *fpu);
 extern int  fpu__restore_sig(void __user *buf, int ia32_frame);
 extern void fpu__drop(struct fpu *fpu);
 extern int  fpu__copy(struct task_struct *dst, struct task_struct *src);
-extern void fpu__clear(struct fpu *fpu);
+extern void fpu__clear_user_states(struct fpu *fpu);
+extern void fpu__clear_all(struct fpu *fpu);
 extern int  fpu__exception_code(struct fpu *fpu, int trap_nr);
 extern int  dump_fpu(struct pt_regs *ptregs, struct user_i387_struct *fpstate);
 
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index 12c70840980e..2c946ba78b1d 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -294,12 +294,12 @@ void fpu__drop(struct fpu *fpu)
  * Clear FPU registers by setting them up from
  * the init fpstate:
  */
-static inline void copy_init_fpstate_to_fpregs(void)
+static inline void copy_init_fpstate_to_fpregs(u64 features_mask)
 {
 	fpregs_lock();
 
 	if (use_xsave())
-		copy_kernel_to_xregs(&init_fpstate.xsave, -1);
+		copy_kernel_to_xregs(&init_fpstate.xsave, features_mask);
 	else if (static_cpu_has(X86_FEATURE_FXSR))
 		copy_kernel_to_fxregs(&init_fpstate.fxsave);
 	else
@@ -312,24 +312,32 @@ static inline void copy_init_fpstate_to_fpregs(void)
 	fpregs_unlock();
 }
 
+static void fpu__clear_states(struct fpu *fpu, u64 xfeatures_mask)
+{
+	WARN_ON_FPU(fpu != &current->thread.fpu);
+
+	fpu__drop(fpu);
+
+	/* Make sure fpstate is cleared and initialized. */
+	fpu__initialize(fpu);
+	if (static_cpu_has(X86_FEATURE_FPU))
+		copy_init_fpstate_to_fpregs(xfeatures_mask);
+}
+
 /*
  * Clear the FPU state back to init state.
  *
  * Called by sys_execve(), by the signal handler code and by various
  * error paths.
  */
-void fpu__clear(struct fpu *fpu)
+void fpu__clear_user_states(struct fpu *fpu)
 {
-	WARN_ON_FPU(fpu != &current->thread.fpu); /* Almost certainly an anomaly */
-
-	fpu__drop(fpu);
+	fpu__clear_states(fpu, xfeatures_mask_user());
+}
 
-	/*
-	 * Make sure fpstate is cleared and initialized.
-	 */
-	fpu__initialize(fpu);
-	if (static_cpu_has(X86_FEATURE_FPU))
-		copy_init_fpstate_to_fpregs();
+void fpu__clear_all(struct fpu *fpu)
+{
+	fpu__clear_states(fpu, xfeatures_mask_all);
 }
 
 /*
diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
index c1f0688f35cf..25e7dddbc6fd 100644
--- a/arch/x86/kernel/fpu/signal.c
+++ b/arch/x86/kernel/fpu/signal.c
@@ -288,7 +288,7 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size)
 			 IS_ENABLED(CONFIG_IA32_EMULATION));
 
 	if (!buf) {
-		fpu__clear(fpu);
+		fpu__clear_user_states(fpu);
 		return 0;
 	}
 
@@ -412,7 +412,7 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size)
 
 err_out:
 	if (ret)
-		fpu__clear(fpu);
+		fpu__clear_user_states(fpu);
 	return ret;
 }
 
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 75fea0d48c0e..d360bf4d696b 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -139,7 +139,7 @@ void flush_thread(void)
 	flush_ptrace_hw_breakpoint(tsk);
 	memset(tsk->thread.tls_array, 0, sizeof(tsk->thread.tls_array));
 
-	fpu__clear(&tsk->thread.fpu);
+	fpu__clear_all(&tsk->thread.fpu);
 }
 
 void disable_TSC(void)
diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index 8eb7193e158d..ce9421ec285f 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -763,7 +763,7 @@ handle_signal(struct ksignal *ksig, struct pt_regs *regs)
 		/*
 		 * Ensure the signal handler starts with the new fpu state.
 		 */
-		fpu__clear(fpu);
+		fpu__clear_user_states(fpu);
 	}
 	signal_setup_done(failed, ksig, stepping);
 }
-- 
2.17.1

Re: [PATCH 5/6] x86/fpu/xstate: Define new functions for clearing fpregs and xstates

[Borislav Petkov]

On Wed, Sep 25, 2019 at 08:10:21AM -0700, Yu-cheng Yu wrote:
> From: Fenghua Yu <fenghua.yu@intel.com>
> 
> Currently, fpu__clear() clears all fpregs and xstates.  Once XSAVES
> supervisor states are introduced, supervisor settings must remain active
> for signals;

I could very well use an example here: for signal handling supervisor
states, I'm guessing this would be something CET-related so some text
about a usage scenario would be very helpful here.

> it is necessary to have separate functions:
> 
>  - Create fpu__clear_user_states(): clear only user settings for signals;
>  - Create fpu__clear_all(): clear both user and supervisor settings in
>    flush_thread().
> 
> Also modify copy_init_fpstate_to_fpregs() to take a mask from above two
> functions.
> 
> Co-developed-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>

As before...

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

[PATCH 6/6] x86/fpu/xstate: Rename validate_xstate_header() to validate_xstate_header_from_user()

[Yu-cheng Yu]

From: Fenghua Yu <fenghua.yu@intel.com>

The function validate_xstate_header() validates an xstate header coming
from userspace (PTRACE or sigreturn).  To make it clear, rename it to
validate_xstate_header_from_user().

Suggested-by: Dave Hansen <dave.hansen@intel.com>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
---
 arch/x86/include/asm/fpu/xstate.h | 2 +-
 arch/x86/kernel/fpu/regset.c      | 2 +-
 arch/x86/kernel/fpu/signal.c      | 2 +-
 arch/x86/kernel/fpu/xstate.c      | 6 +++---
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/fpu/xstate.h b/arch/x86/include/asm/fpu/xstate.h
index 5954bd97da16..45df38043600 100644
--- a/arch/x86/include/asm/fpu/xstate.h
+++ b/arch/x86/include/asm/fpu/xstate.h
@@ -72,6 +72,6 @@ int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf);
 int copy_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf);
 
 /* Validate an xstate header supplied by userspace (ptrace or sigreturn) */
-extern int validate_xstate_header(const struct xstate_header *hdr);
+int validate_xstate_header_from_user(const struct xstate_header *hdr);
 
 #endif
diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c
index d652b939ccfb..9789f95cfb66 100644
--- a/arch/x86/kernel/fpu/regset.c
+++ b/arch/x86/kernel/fpu/regset.c
@@ -139,7 +139,7 @@ int xstateregs_set(struct task_struct *target, const struct user_regset *regset,
 	} else {
 		ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, xsave, 0, -1);
 		if (!ret)
-			ret = validate_xstate_header(&xsave->header);
+			ret = validate_xstate_header_from_user(&xsave->header);
 	}
 
 	/*
diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
index 25e7dddbc6fd..728b43c34536 100644
--- a/arch/x86/kernel/fpu/signal.c
+++ b/arch/x86/kernel/fpu/signal.c
@@ -368,7 +368,7 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size)
 			ret = __copy_from_user(&fpu->state.xsave, buf_fx, state_size);
 
 			if (!ret && state_size > offsetof(struct xregs_state, header))
-				ret = validate_xstate_header(&fpu->state.xsave.header);
+				ret = validate_xstate_header_from_user(&fpu->state.xsave.header);
 		}
 		if (ret)
 			goto err_out;
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index a183c319d808..3df2e55e860d 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -480,7 +480,7 @@ int using_compacted_format(void)
 }
 
 /* Validate an xstate header supplied by userspace (ptrace or sigreturn) */
-int validate_xstate_header(const struct xstate_header *hdr)
+int validate_xstate_header_from_user(const struct xstate_header *hdr)
 {
 	/* No unknown or supervisor features may be set */
 	if (hdr->xfeatures & ~xfeatures_mask_user())
@@ -1165,7 +1165,7 @@ int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf)
 
 	memcpy(&hdr, kbuf + offset, size);
 
-	if (validate_xstate_header(&hdr))
+	if (validate_xstate_header_from_user(&hdr))
 		return -EINVAL;
 
 	for (i = 0; i < XFEATURE_MAX; i++) {
@@ -1219,7 +1219,7 @@ int copy_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf)
 	if (__copy_from_user(&hdr, ubuf + offset, size))
 		return -EFAULT;
 
-	if (validate_xstate_header(&hdr))
+	if (validate_xstate_header_from_user(&hdr))
 		return -EINVAL;
 
 	for (i = 0; i < XFEATURE_MAX; i++) {
-- 
2.17.1

Re: [PATCH 6/6] x86/fpu/xstate: Rename validate_xstate_header() to validate_xstate_header_from_user()

[Borislav Petkov]

On Wed, Sep 25, 2019 at 08:10:22AM -0700, Yu-cheng Yu wrote:
> From: Fenghua Yu <fenghua.yu@intel.com>
> 
> The function validate_xstate_header() validates an xstate header coming
> from userspace (PTRACE or sigreturn).  To make it clear, rename it to
> validate_xstate_header_from_user().
> 
> Suggested-by: Dave Hansen <dave.hansen@intel.com>
> Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>

This one is correct!

\o/

> Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>
> Reviewed-by: Tony Luck <tony.luck@intel.com>
> ---
>  arch/x86/include/asm/fpu/xstate.h | 2 +-
>  arch/x86/kernel/fpu/regset.c      | 2 +-
>  arch/x86/kernel/fpu/signal.c      | 2 +-
>  arch/x86/kernel/fpu/xstate.c      | 6 +++---
>  4 files changed, 6 insertions(+), 6 deletions(-)

And I like patches like this one! :-)

Reviewed-by: Borislav Petkov <bp@suse.de>

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH 6/6] x86/fpu/xstate: Rename validate_xstate_header() to validate_xstate_header_from_user()

[Yu-cheng Yu]

On Wed, 2019-10-09 at 19:31 +0200, Borislav Petkov wrote:
> On Wed, Sep 25, 2019 at 08:10:22AM -0700, Yu-cheng Yu wrote:
> > From: Fenghua Yu <fenghua.yu@intel.com>
> > 
> > The function validate_xstate_header() validates an xstate header coming
> > from userspace (PTRACE or sigreturn).  To make it clear, rename it to
> > validate_xstate_header_from_user().
> > 
> > Suggested-by: Dave Hansen <dave.hansen@intel.com>
> > Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
> > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> 
> This one is correct!
> 
> \o/
> 
> > Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>
> > Reviewed-by: Tony Luck <tony.luck@intel.com>
> > ---
> >  arch/x86/include/asm/fpu/xstate.h | 2 +-
> >  arch/x86/kernel/fpu/regset.c      | 2 +-
> >  arch/x86/kernel/fpu/signal.c      | 2 +-
> >  arch/x86/kernel/fpu/xstate.c      | 6 +++---
> >  4 files changed, 6 insertions(+), 6 deletions(-)
> 
> And I like patches like this one! :-)
> 
> Reviewed-by: Borislav Petkov <bp@suse.de>

Thanks for reviewing.  I will send out an updated version.

Yu-cheng