Skip to contentSkip to conversation

um: add back support for FXSAVE registers

PATCH In review Benjamin Berg 2 participants 3 messages Dec 4, 2024 – Feb 11, 2025

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 um: add back support for FXSAVE registers

From: Benjamin Berg <[email protected]>

It was reported that qemu may not enable the XSTATE CPU extension, which is a requirement after commit 3f17fed21491 ("um: switch to regset API and depend on XSTATE"). Add a fallback to use FXSAVE (FP registers on x86_64 and XFP on i386) which is just a shorter version of the same data. The only difference is that the XSTATE magic should not be set in the signal frame.

Note that this still drops support for the older i386 FP register layout as supporting this would require more backward compatibility to build a correct signal frame.

Fixes 3f17fed21491 ("um: switch to regset API and depend on XSTATE")
---
 arch/x86/um/os-Linux/registers.c | 21 ++++++++++++++++++---
 arch/x86/um/signal.c             |  5 +++++
 2 files changed, 23 insertions(+), 3 deletions(-)
arch/x86/um/os-Linux/registers.c +18 -3
index 76eaeb93928c..eb1cdadc8a61 100644
@@ -18,6 +18,7 @@
 #include <registers.h>
 #include <sys/mman.h>
 
+static unsigned long ptrace_regset;
 unsigned long host_fp_size;
 
 int get_fp_registers(int pid, unsigned long *regs)
@@ -27,7 +28,7 @@ int get_fp_registers(int pid, unsigned long *regs)
 		.iov_len = host_fp_size,
 	};
 
-	if (ptrace(PTRACE_GETREGSET, pid, NT_X86_XSTATE, &iov) < 0)
+	if (ptrace(PTRACE_GETREGSET, pid, ptrace_regset, &iov) < 0)
 		return -errno;
 	return 0;
 }
@@ -39,7 +40,7 @@ int put_fp_registers(int pid, unsigned long *regs)
 		.iov_len = host_fp_size,
 	};
 
-	if (ptrace(PTRACE_SETREGSET, pid, NT_X86_XSTATE, &iov) < 0)
+	if (ptrace(PTRACE_SETREGSET, pid, ptrace_regset, &iov) < 0)
 		return -errno;
 	return 0;
 }
@@ -58,9 +59,23 @@ int arch_init_registers(int pid)
 		return -ENOMEM;
 
 	/* GDB has x86_xsave_length, which uses x86_cpuid_count */
-	ret = ptrace(PTRACE_GETREGSET, pid, NT_X86_XSTATE, &iov);
+	ptrace_regset = NT_X86_XSTATE;
+	ret = ptrace(PTRACE_GETREGSET, pid, ptrace_regset, &iov);
 	if (ret)
 		ret = -errno;
+
+	if (ret == -ENODEV) {
+#ifdef CONFIG_X86_32
+		ptrace_regset = NT_PRXFPREG;
+#else
+		ptrace_regset = NT_PRFPREG;
+#endif
+		iov.iov_len = 2 * 1024 * 1024;
+		ret = ptrace(PTRACE_GETREGSET, pid, ptrace_regset, &iov);
+		if (ret)
+			ret = -errno;
+	}
+
 	munmap(iov.iov_base, 2 * 1024 * 1024);
 
 	host_fp_size = iov.iov_len;
arch/x86/um/signal.c +5 -0
index 75087e85b6fd..ea5b3bcc4245 100644
@@ -187,7 +187,12 @@ static int copy_sc_to_user(struct sigcontext __user *to,
 	 * Put magic/size values for userspace. We do not bother to verify them
 	 * later on, however, userspace needs them should it try to read the
 	 * XSTATE data. And ptrace does not fill in these parts.
+	 *
+	 * Skip this if we do not have an XSTATE frame.
 	 */
+	if (host_fp_size <= sizeof(to_fp64->fpstate))
+		return 0;
+
 	BUILD_BUG_ON(sizeof(int) != FP_XSTATE_MAGIC2_SIZE);
 #ifdef CONFIG_X86_32
 	__put_user(offsetof(struct _fpstate_32, _fxsr_env) +
Signature · 1 line
2.47.1
#1 lore raw

Hello,

Quoted 13 lines · On Wed, 4 Dec 2024 08:48:27 +0100 Benjamin Berg <[email protected]>…

From: Benjamin Berg <[email protected]>

It was reported that qemu may not enable the XSTATE CPU extension, which is a requirement after commit 3f17fed21491 ("um: switch to regset API and depend on XSTATE"). Add a fallback to use FXSAVE (FP registers on x86_64 and XFP on i386) which is just a shorter version of the same data. The only difference is that the XSTATE magic should not be set in the signal frame.

Note that this still drops support for the older i386 FP register layout as supporting this would require more backward compatibility to build a correct signal frame.

Quoted 5 lines
Fixes 3f17fed21491 ("um: switch to regset API and depend on XSTATE")

Seems this patch is not yet merged into uml/next or uml/fixes. May I ask the timeline or blockers for this patch?

Thanks,
SJ

[...]

#2 lore raw
Quoted 17 lines · On Tue, 7 Jan 2025 13:07:58 -0800 SeongJae Park <[email protected]> wrote:

Hello,

Quoted 13 lines · On Wed, 4 Dec 2024 08:48:27 +0100 Benjamin Berg <[email protected]>…

From: Benjamin Berg <[email protected]>

It was reported that qemu may not enable the XSTATE CPU extension, which is a requirement after commit 3f17fed21491 ("um: switch to regset API and depend on XSTATE"). Add a fallback to use FXSAVE (FP registers on x86_64 and XFP on i386) which is just a shorter version of the same data. The only difference is that the XSTATE magic should not be set in the signal frame.

Note that this still drops support for the older i386 FP register layout as supporting this would require more backward compatibility to build a correct signal frame.

Quoted 8 lines
Quoted 5 lines
Fixes 3f17fed21491 ("um: switch to regset API and depend on XSTATE")

Seems this patch is not yet merged into uml/next or uml/fixes. May I ask the timeline or blockers for this patch?

A gentle reminder. :)

Thanks,
SJ

[...]