linux-kernel - Re: [PATCH 1/2] x86/fpu: Rename "dynamic" XSTATEs to "independent"

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: <YMMYUGW9f7CUJ+oz@hirez.programming.kicks-ass.net>
Date:   Fri, 11 Jun 2021 10:01:20 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Andy Lutomirski <luto@...nel.org>
Cc:     x86@...nel.org, Dave Hansen <dave.hansen@...el.com>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] x86/fpu: Rename "dynamic" XSTATEs to "independent"

On Thu, Jun 10, 2021 at 10:13:36PM -0700, Andy Lutomirski wrote:
> The salient feature of "dynamic" XSTATEs is that they are not part of the
> main task XSTATE buffer.  The fact that they are dynamically allocated is
> irrelevant and will become quite confusing when user math XSTATEs start
> being dynamically allocated.  Rename them to "independent" because they
> are independent of the main XSTATE code.
> 
> This is just a search-and-replace with some whitespace updates to keep
> things aligned.
> 
> Signed-off-by: Andy Lutomirski <luto@...nel.org>
> ---
>  arch/x86/events/intel/lbr.c       |  6 +--
>  arch/x86/include/asm/fpu/xstate.h | 14 +++----
>  arch/x86/kernel/fpu/xstate.c      | 62 +++++++++++++++----------------
>  3 files changed, 41 insertions(+), 41 deletions(-)
> 
> diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
> index 76dbab6ac9fb..0189807fc3c1 100644
> --- a/arch/x86/events/intel/lbr.c
> +++ b/arch/x86/events/intel/lbr.c
> @@ -491,7 +491,7 @@ static void intel_pmu_arch_lbr_xrstors(void *ctx)
>  {
>  	struct x86_perf_task_context_arch_lbr_xsave *task_ctx = ctx;
>  
> -	copy_kernel_to_dynamic_supervisor(&task_ctx->xsave, XFEATURE_MASK_LBR);
> +	copy_kernel_to_independent_supervisor(&task_ctx->xsave, XFEATURE_MASK_LBR);
>  }
>  
>  static __always_inline bool lbr_is_reset_in_cstate(void *ctx)
> @@ -576,7 +576,7 @@ static void intel_pmu_arch_lbr_xsaves(void *ctx)
>  {
>  	struct x86_perf_task_context_arch_lbr_xsave *task_ctx = ctx;
>  
> -	copy_dynamic_supervisor_to_kernel(&task_ctx->xsave, XFEATURE_MASK_LBR);
> +	copy_independent_supervisor_to_kernel(&task_ctx->xsave, XFEATURE_MASK_LBR);
>  }
>  
>  static void __intel_pmu_lbr_save(void *ctx)
> @@ -978,7 +978,7 @@ static void intel_pmu_arch_lbr_read_xsave(struct cpu_hw_events *cpuc)
>  		intel_pmu_store_lbr(cpuc, NULL);
>  		return;
>  	}
> -	copy_dynamic_supervisor_to_kernel(&xsave->xsave, XFEATURE_MASK_LBR);
> +	copy_independent_supervisor_to_kernel(&xsave->xsave, XFEATURE_MASK_LBR);
>  
>  	intel_pmu_store_lbr(cpuc, xsave->lbr.entries);
>  }

Yesterday tglx proposed the *save*_to_{user,kernel}() and
*rstor*_from_{user,kernel}() namespace for pretty much every other such
function.

And while I agree that independent_supervisor beats dynamic_supervisor
for a name, they're both stupid long :-(

I don't suppose we can simply use xsaves_to_kernel()
xstrors_from_kernel() and add some magic to their respective mask
handling to ensure that a mask belongs to only 1 (of 3) types.

	int types = 0;

	if (mask & xfeatures_mask_user())
		type++;
	if (mask & xfeatures_mask_supervisor())
		types++;
	if (mask & xfeatures_mask_independent())
		types++;

	if (WARN_ON_ONCE(type != 1))
		return;

?