Re: [PATCH v10 06/28] x86/fpu/xstate: Add new variables to indicate dynamic XSTATE buffer size

[Thomas Gleixner]

On Wed, Aug 25 2021 at 08:53, Chang S. Bae wrote:
> +/**
> + * struct fpu_xstate_buffer_config - xstate buffer configuration
> + * @max_size:			The CPUID-enumerated all-feature "maximum" size
> + *				for xstate per-task buffer.
> + * @min_size:			The size to fit into the statically-allocated
> + *				buffer. With dynamic states, this buffer no longer
> + *				contains all the enabled state components.
> + * @user_size:			The size of user-space buffer for signal and
> + *				ptrace frames, in the non-compacted format.
> + */

>  void fpstate_init(struct fpu *fpu)
>  {
>  	union fpregs_state *state;
> +	unsigned int size;
> +	u64 mask;
>  
> -	if (likely(fpu))
> +	if (likely(fpu)) {
>  		state = &fpu->state;
> -	else
> +		/* The dynamic user states are not prepared yet. */
> +		mask = xfeatures_mask_all & ~xfeatures_mask_user_dynamic;

The patch ordering is really odd. Why aren't you adding

     fpu->state_mask
and
     fpu->state_size

first and initialize state_mask and state_size to the fixed mode and
then add the dynamic sizing on top?

>  	/*
>  	 * If the target FPU state is not resident in the CPU registers, just
>  	 * memcpy() from current, else save CPU state directly to the target.
> +	 *
> +	 * KVM does not support dynamic user states yet. Assume the buffer
> +	 * always has the minimum size.
>  	 */
>  	if (test_thread_flag(TIF_NEED_FPU_LOAD))
>  		memcpy(&fpu->state, &current->thread.fpu.state,
> -		       fpu_kernel_xstate_size);
> +		       fpu_buf_cfg.min_size);

Which completely avoids the export of fpu_buf_cfg for KVM because the
information is just available via struc fpu. As a bonus the export of
fpu_kernel_xstate_size can be removed as well.

Hmm?

Thanks,

        tglx