linux-kernel - Re: [PATCH v10 02/28] x86/fpu/xstate: Modify the initialization helper to handle both static and dynamic buffers

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: <87k0iw6hi5.ffs@tglx>
Date:   Fri, 01 Oct 2021 14:45:22 +0200
From:   Thomas Gleixner <tglx@...utronix.de>
To:     "Chang S. Bae" <chang.seok.bae@...el.com>, bp@...e.de,
        luto@...nel.org, mingo@...nel.org, x86@...nel.org
Cc:     len.brown@...el.com, lenb@...nel.org, dave.hansen@...el.com,
        thiago.macieira@...el.com, jing2.liu@...el.com,
        ravi.v.shankar@...el.com, linux-kernel@...r.kernel.org,
        chang.seok.bae@...el.com, kvm@...r.kernel.org
Subject: Re: [PATCH v10 02/28] x86/fpu/xstate: Modify the initialization
 helper to handle both static and dynamic buffers

Chang,

On Wed, Aug 25 2021 at 08:53, Chang S. Bae wrote:
> Have the function initializing the XSTATE buffer take a struct fpu *
> pointer in preparation for dynamic state buffer support.
>
> init_fpstate is a special case, which is indicated by a null pointer
> parameter to fpstate_init().
>
> Also, fpstate_init_xstate() now accepts the state component bitmap to
> customize the compacted format.

That's not a changelog. Changelogs have to explain the WHY not the WHAT.

I can see the WHY when I look at the later changes, but that's not how
it works.

Also the subject of this patch is just wrong. It does not make the
functions handle dynamic buffers, it prepares them to add support for
that later.

> +static inline void fpstate_init_xstate(struct xregs_state *xsave, u64 mask)
> +{
> +	/*
> +	 * XRSTORS requires these bits set in xcomp_bv, or it will
> +	 * trigger #GP:
> +	 */
> +	xsave->header.xcomp_bv = XCOMP_BV_COMPACTED_FORMAT | mask;
> +}

This wants to be a separate cleanup patch which replaces the open coded
variant here:

> diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
> index fc1d529547e6..0fed7fbcf2e8 100644
> --- a/arch/x86/kernel/fpu/xstate.c
> +++ b/arch/x86/kernel/fpu/xstate.c
> @@ -395,8 +395,7 @@ static void __init setup_init_fpu_buf(void)
>  	print_xstate_features();
>  
>  	if (boot_cpu_has(X86_FEATURE_XSAVES))
> -		init_fpstate.xsave.header.xcomp_bv = XCOMP_BV_COMPACTED_FORMAT |
> -						     xfeatures_mask_all;
> +		fpstate_init_xstate(&init_fpstate.xsave, xfeatures_mask_all);

Thanks,

        tglx