All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrei Vagin <avagin@google.com>
To: Thomas Gleixner <tglx@kernel.org>, Ingo Molnar <mingo@redhat.com>,
	Borislav Petkov <bp@alien8.de>,
	 "Chang S. Bae" <chang.seok.bae@intel.com>
Cc: linux-kernel@vger.kernel.org, criu@lists.linux.dev,
	 Dave Hansen <dave.hansen@linux.intel.com>,
	x86@kernel.org,  Andrei Vagin <avagin@google.com>,
	Alexander Mikhalitsyn <alexander@mihalicyn.com>,
	 "H. Peter Anvin" <hpa@zytor.com>
Subject: [PATCH 3/7] x86/fpu: Extract restore_from_ia32_fxstate() and clean up fpu__restore_sig()
Date: Tue,  8 Sep 2026 04:34:23 +0000	[thread overview]
Message-ID: <20260908043427.1842515-4-avagin@google.com> (raw)
In-Reply-To: <20260908043427.1842515-1-avagin@google.com>

Improve readability of the signal frame restoration code. Previously,
most of __fpu_restore_sig() was dedicated to handling the 32-bit compat
fpstate, while the native direct path lived in restore_fpregs_from_user().
Having the compat handling intermixed with the main flow made it tricky
to quickly see what code was doing what.

Extract the 32-bit legacy/compat FPU restore handling into a separate
helper function, restore_from_ia32_fxstate(), and inline the remainder
of __fpu_restore_sig() directly into fpu__restore_sig().

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.

Reviewed-by: Alexander Mikhalitsyn <alexander@mihalicyn.com>
Signed-off-by: Andrei Vagin <avagin@google.com>
---
 arch/x86/kernel/fpu/signal.c | 67 +++++++++++++++++++++++-------------
 1 file changed, 43 insertions(+), 24 deletions(-)

diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
index cd7db6dc819b..60063a7a44f8 100644
--- a/arch/x86/kernel/fpu/signal.c
+++ b/arch/x86/kernel/fpu/signal.c
@@ -325,32 +325,24 @@ 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)
+#if defined(CONFIG_X86_32) || defined(CONFIG_IA32_EMULATION)
+/*
+ * Restore FPU state from a signal frame when a legacy 32-bit FP frame
+ * (buf_f) is present.
+ *
+ * The legacy FP frame duplicates the FP state portion of the FX/XSAVE
+ * frame (buf_fx). 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.
+ */
+static bool restore_from_ia32_fxstate(void __user *buf_f, void __user *buf_fx,
+				      u64 xrestore_mask, bool fx_only)
 {
 	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;
-	u64 xrestore_mask = 0;
-
-	if (use_xsave()) {
-		struct _fpx_sw_bytes fx_sw_user;
-
-		if (!check_xstate_in_sigframe(buf_fx, &fx_sw_user))
-			return false;
-
-		fx_only = !fx_sw_user.magic1;
-		xrestore_mask = fx_sw_user.xfeatures;
-	} else {
-		xrestore_mask = XFEATURE_MASK_FPSSE;
-	}
-
-	if (likely(!ia32_fxstate)) {
-		/* Restore the FPU registers directly from user memory. */
-		return restore_fpregs_from_user(buf_fx, xrestore_mask, fx_only);
-	}
+	bool success;
 
 	/*
 	 * Copy the legacy state because the FP portion of the FX frame has
@@ -436,6 +428,13 @@ static bool __fpu_restore_sig(void __user *buf_f, void __user *buf_fx,
 	fpregs_unlock();
 	return success;
 }
+#else
+static inline bool restore_from_ia32_fxstate(void __user *buf_f, void __user *buf_fx,
+					     u64 xrestore_mask, bool fx_only)
+{
+	return false;
+}
+#endif
 
 static inline unsigned int xstate_sigframe_size(struct fpstate *fpstate)
 {
@@ -450,10 +449,11 @@ static inline unsigned int xstate_sigframe_size(struct fpstate *fpstate)
 bool fpu__restore_sig(void __user *buf, int ia32_frame)
 {
 	struct fpu *fpu = x86_task_fpu(current);
-	void __user *buf_fx = buf;
+	bool success = false, fx_only = false;
 	bool ia32_fxstate = false;
-	bool success = false;
+	void __user *buf_fx = buf;
 	unsigned int size;
+	u64 xrestore_mask;
 
 	if (unlikely(!buf)) {
 		fpu__clear_user_states(fpu);
@@ -482,10 +482,29 @@ bool fpu__restore_sig(void __user *buf, int ia32_frame)
 		success = !fpregs_soft_set(current, NULL, 0,
 					   sizeof(struct user_i387_ia32_struct),
 					   NULL, buf);
+		goto out;
+	}
+
+	if (use_xsave()) {
+		struct _fpx_sw_bytes fx_sw_user;
+
+		if (!check_xstate_in_sigframe(buf_fx, &fx_sw_user))
+			goto out;
+
+		fx_only = !fx_sw_user.magic1;
+		xrestore_mask = fx_sw_user.xfeatures;
 	} else {
-		success = __fpu_restore_sig(buf, buf_fx, ia32_fxstate);
+		xrestore_mask = XFEATURE_MASK_FPSSE;
+	}
+
+	if (ia32_fxstate) {
+		success = restore_from_ia32_fxstate(buf, buf_fx,
+						    xrestore_mask, fx_only);
+		goto out;
 	}
 
+	/* Restore the FPU registers directly from user memory. */
+	success = restore_fpregs_from_user(buf_fx, xrestore_mask, fx_only);
 out:
 	if (unlikely(!success))
 		fpu__clear_user_states(fpu);
-- 
2.55.0.979.g7e5102b832-goog


  parent reply	other threads:[~2026-09-08  4:34 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-08  4:34 [PATCH v5 0/7] x86/fpu: Restore and reinforce signal frame portability Andrei Vagin
2026-09-08  4:34 ` [PATCH 1/7] x86/fpu: Document signal frame layout and portability Andrei Vagin
2026-09-08  4:34 ` [PATCH 2/7] x86/fpu: Clean up and rename variables in signal frame handling Andrei Vagin
2026-09-08  4:34 ` Andrei Vagin [this message]
2026-09-08  4:34 ` [PATCH 4/7] x86/fpu: Document reasoning of FX-only fallback Andrei Vagin
2026-09-08  4:34 ` [PATCH 5/7] x86/fpu: Fix potential underflow in xstate_calculate_size() Andrei Vagin
2026-09-08  4:34 ` [PATCH 6/7] x86/fpu: Pre-fault only required size of xstate buffer Andrei Vagin
2026-09-08  4:34 ` [PATCH 7/7] selftests/x86: Add tests for signal frame FPU portability Andrei Vagin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260908043427.1842515-4-avagin@google.com \
    --to=avagin@google.com \
    --cc=alexander@mihalicyn.com \
    --cc=bp@alien8.de \
    --cc=chang.seok.bae@intel.com \
    --cc=criu@lists.linux.dev \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=tglx@kernel.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.