On Tue, Oct 12, 2021 at 02:00:04AM +0200, Thomas Gleixner wrote:
> These interfaces are really only valid for features which are independently
> managed and not part of the task context state for various reasons.
>
> Tighten the checks and adjust the misleading comments.
>
> Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> ---
> arch/x86/kernel/fpu/xstate.c | 14 ++++----------
> 1 file changed, 4 insertions(+), 10 deletions(-)
>
> --- a/arch/x86/kernel/fpu/xstate.c
> +++ b/arch/x86/kernel/fpu/xstate.c
> @@ -1182,13 +1182,9 @@ static bool validate_xsaves_xrstors(u64
I guess then change the name too, to:
validate_indep_xstate_components()
or so?
Then you don't need the comment below.
> if (WARN_ON_FPU(!cpu_feature_enabled(X86_FEATURE_XSAVES)))
> return false;
> /*
> - * Validate that this is either a task->fpstate related component
> - * subset or an independent one.
> + * Validate that this is a independent compoment.
WARNING: 'compoment' may be misspelled - perhaps 'component'?
#78: FILE: arch/x86/kernel/fpu/xstate.c:1185:
+ * Validate that this is a independent compoment.
^^^^^^^^^
> */
> - if (mask & xfeatures_mask_independent())
> - xchk = ~xfeatures_mask_independent();
> - else
> - xchk = ~xfeatures_mask_all;
> + xchk = ~xfeatures_mask_independent();
>
> if (WARN_ON_ONCE(!mask || mask & xchk))
> return false;
> @@ -1206,8 +1202,7 @@ static bool validate_xsaves_xrstors(u64
> * buffer should be zeroed otherwise a consecutive XRSTORS from that buffer
> * can #GP.
> *
> - * The feature mask must either be a subset of the independent features or
> - * a subset of the task->fpstate related features.
> + * The feature mask must be a subset of the independent features
End with a fullstop.
> */
> void xsaves(struct xregs_state *xstate, u64 mask)
> {
> @@ -1231,8 +1226,7 @@ void xsaves(struct xregs_state *xstate,
> * Proper usage is to restore the state which was saved with
> * xsaves() into @xstate.
> *
> - * The feature mask must either be a subset of the independent features or
> - * a subset of the task->fpstate related features.
> + * The feature mask must be a subset of the independent features
Ditto.
> */
> void xrstors(struct xregs_state *xstate, u64 mask)
> {
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
