Re: [PATCH 3/8] x86/fpu: Split __fpu_restore_sig to extract compat path

[Chang S. Bae]

On 8/16/2026 9:20 PM, Andrei Vagin wrote:
> Split __fpu_restore_sig to move the restore part for the legacy/compat
> FPU state (when buf_f is present) to a separate helper function.

Not sure where the justification for this refactoring is. Is this 
primarily preparation for the next change?

> The legacy 32-bit FP frame duplicates the FP state portion of the
> FX/XSAVE frame. For backward compatibility, the legacy FP frame is
> treated as the source of truth, and its state is folded into the
> FX/XSAVE state before restoring the registers.

This reads more like a description of what the refactored code has 
already been doing than a motivation of the change itself.

> diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
> index 42c3d78bd849..6a14b528ac7f 100644
> --- a/arch/x86/kernel/fpu/signal.c
> +++ b/arch/x86/kernel/fpu/signal.c
> @@ -264,6 +264,9 @@ static int __restore_fpregs_from_user(void __user *buf, u64 task_xfeatures,
>   	}
>   }
>   
> +static bool restore_fpregs_from_user_compat(void __user *buf_f, void __user *buf_fx,
> +					    u64 xrestore_mask, bool fx_only);
> +
>   /*
>    * Attempt to restore the FPU registers directly from user memory.
>    * Pagefaults are handled and any errors returned are fatal.
> @@ -324,14 +327,9 @@ static bool restore_fpregs_from_user(void __user *buf, u64 xrestore_mask, bool f
>   	return true;
>   }
>   
> -static bool __fpu_restore_sig(void __user *buf_f, void __user *buf_fx,
> -			      bool ia32_fxstate)
> +static bool __fpu_restore_sig(void __user *buf_f, void __user *buf_fx)
>   {
> -	struct task_struct *tsk = current;
> -	struct fpu *fpu = x86_task_fpu(tsk);
> -	struct user_i387_ia32_struct env;
> -	bool success, fx_only = false;
> -	union fpregs_state *fpregs;
> +	bool fx_only = false;
>   	u64 xrestore_mask = 0;
>   
>   	if (use_xsave()) {
> @@ -346,11 +344,33 @@ static bool __fpu_restore_sig(void __user *buf_f, void __user *buf_fx,
>   		xrestore_mask = XFEATURE_MASK_FPSSE;
>   	}
>   
> -	if (likely(!ia32_fxstate)) {
> +	if (likely(!buf_f)) {
>   		/* Restore the FPU registers directly from user memory. */
>   		return restore_fpregs_from_user(buf_fx, xrestore_mask, fx_only);
>   	}
>   
> +	return restore_fpregs_from_user_compat(buf_f, buf_fx, xrestore_mask, fx_only);
> +}

I don't think replacing `ia32_fxstate` with `buf_f` is a clear win for 
description itself.

Furthermore, __fpu_restore_sig() now looks thin enough that this may not 
need to remain. It could be folded into fpu_restore_sig() rather than 
tweaking the argument naming, then. I assume the resulting 
fpu_restore_sig() does not become too messy.

Thanks,
Chang