Re: [PATCH v5] um: switch to regset API and depend on XSTATE

[Benjamin Berg]

On Tue, 2024-12-03 at 07:56 -0800, SeongJae Park wrote:
> On Tue, 03 Dec 2024 07:01:09 SeongJae Park <s...@kernel.org> wrote:
> 
> > On Tue, 03 Dec 2024 09:40:34 +0100 Benjamin Berg
> > <benja...@sipsolutions.net> wrote:
> > 
> > > Hi,
> > > 
> > > that probably means the size detection for the FPU state (i.e.
> > > PTRACE_GETREGSET for NT_X86_XSTATE is incorrect on a 32bit host
> > > in some
> > > way.
> > > 
> > > Is there anything special about the qemu setup or it is just a
> > > default
> > > qemu-x86?
> > 
> > I use default qemu-system-x86_64 on my system.
> > 
> >     $ qemu-system-x86_64 --version
> >     QEMU emulator version 8.2.2 (qemu-8.2.2-1.1.hs+fb.el9)
> >     Copyright (c) 2003-2023 Fabrice Bellard and the QEMU Project
> > developers
> > 
> > I forgot saying it is not just x86 but x86_64, sorry.
> 
> Oh, and seems my qemu has some downstream changes.  I will try to reproduce 
> the
> issue with upstream versions and report the result again.

I doubt that is the reason. The code tries to detect the size of the
NT_X86_XSTATE register set and something breaks.

Thinking about it a bit, the only good explanation is that the qemu CPU
does not have XSTATE support. This would cause the ptrace syscall to
fetch the NT_X86_XSTATE register set to always fail (with -ENODEV).

Honestly, I just had not expected such an issue. Could you try the
below patch to add a fallback?

Regards,
Benjamin

diff --git a/arch/x86/um/os-Linux/registers.c b/arch/x86/um/os-Linux/registers.c
index 76eaeb93928c..eb1cdadc8a61 100644
--- a/arch/x86/um/os-Linux/registers.c
+++ b/arch/x86/um/os-Linux/registers.c
@@ -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;