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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20191009161759.GF10395@zn.tnic>
Date:   Wed, 9 Oct 2019 18:17:59 +0200
From:   Borislav Petkov <bp@...en8.de>
To:     Yu-cheng Yu <yu-cheng.yu@...el.com>
Cc:     linux-kernel@...r.kernel.org, x86@...nel.org,
        "H. Peter Anvin" <hpa@...or.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Tony Luck <tony.luck@...el.com>,
        Andy Lutomirski <luto@...nel.org>,
        Rik van Riel <riel@...riel.com>,
        "Ravi V. Shankar" <ravi.v.shankar@...el.com>,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        Fenghua Yu <fenghua.yu@...el.com>,
        Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH 2/6] x86/fpu/xstate: Define new macros for supervisor and
 user xstates

On Wed, Sep 25, 2019 at 08:10:18AM -0700, Yu-cheng Yu wrote:
> From: Fenghua Yu <fenghua.yu@...el.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@...el.com>
> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@...el.com>
> Signed-off-by: Fenghua Yu <fenghua.yu@...el.com>

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

Signed-off-by: Fenghua Yu <fenghua.yu@...el.com>
Co-developed-by: Yu-cheng Yu <yu-cheng.yu@...el.com>
Signed-off-by: Yu-cheng Yu <yu-cheng.yu@...el.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@...ux.intel.com>
> Reviewed-by: Tony Luck <tony.luck@...el.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