On 2024/9/13 16:22, Benjamin Berg wrote:
> From: Benjamin Berg <benjamin.b...@intel.com>
>
> When switching from userspace to the kernel, all registers including the
> FP registers are copied into the kernel and restored later on. As such,
> the true source for the FP register state is actually already in the
> kernel and they should never be grabbed from the userspace process.
>
> Change the various places to simply copy the data from the internal FP
> register storage area.
Cool! I had similar thoughts, but I haven't tried to make it happen.
> Note that on i386 the format of PTRACE_GETFPREGS
> and PTRACE_GETFPXREGS is different enough that conversion would be
> needed. With this patch, -EINVAL is returned if the non-native format is
> requested.
>
> The upside is, that this patchset fixes setting registers via ptrace
> (which simply did not work before) as well as fixing setting floating
> point registers using the mcontext on signal return on i386.
>
> Signed-off-by: Benjamin Berg <benjamin.b...@intel.com>
>
> ---
>
> I think that ideally more changs should be done in this space. For a
> start, one can remove support for hosts without PTRACE_GETFPXREGS. But,
> more importantly, UML should probably implement the regset API, which
> would considerably improve both ptrace and coredump generation and
> should also make it easy to fix the cases where this patch returns an
> error.
> ---
> arch/um/include/shared/registers.h | 6 ---
> arch/um/kernel/process.c | 11 ++++-
> arch/x86/um/os-Linux/registers.c | 12 +++---
> arch/x86/um/ptrace_32.c | 50 ++++++++++++-----------
> arch/x86/um/ptrace_64.c | 20 ++++-----
> arch/x86/um/signal.c | 65 ++++++++++++++----------------
> 6 files changed, 80 insertions(+), 84 deletions(-)
>
> diff --git a/arch/um/include/shared/registers.h
> b/arch/um/include/shared/registers.h
> index a0450326521c..7d81b2339a48 100644
> --- a/arch/um/include/shared/registers.h
> +++ b/arch/um/include/shared/registers.h
> @@ -8,12 +8,6 @@
>
> #include <sysdep/ptrace.h>
>
> -extern int save_i387_registers(int pid, unsigned long *fp_regs);
> -extern int restore_i387_registers(int pid, unsigned long *fp_regs);
> -extern int save_fp_registers(int pid, unsigned long *fp_regs);
> -extern int restore_fp_registers(int pid, unsigned long *fp_regs);
> -extern int save_fpx_registers(int pid, unsigned long *fp_regs);
> -extern int restore_fpx_registers(int pid, unsigned long *fp_regs);
> extern int init_pid_registers(int pid);
> extern void get_safe_registers(unsigned long *regs, unsigned long *fp_regs);
> extern int get_fp_registers(int pid, unsigned long *regs);
> diff --git a/arch/um/kernel/process.c b/arch/um/kernel/process.c
> index be2856af6d4c..ad798d40f8a4 100644
> --- a/arch/um/kernel/process.c
> +++ b/arch/um/kernel/process.c
> @@ -290,8 +290,15 @@ unsigned long __get_wchan(struct task_struct *p)
>
> int elf_core_copy_task_fpregs(struct task_struct *t, elf_fpregset_t *fpu)
> {
> - int cpu = current_thread_info()->cpu;
> +#ifdef CONFIG_X86_32
> + extern int have_fpx_regs;
>
> - return save_i387_registers(userspace_pid[cpu], (unsigned long *) fpu);
> + /* FIXME: A plain copy does not work on i386 with have_fpx_regs */
> + if (have_fpx_regs)
> + return -1;
> +#endif
> + memcpy(fpu, &t->thread.regs.regs.fp, sizeof(*fpu));
> +
> + return 0;
> }
This function is expected to return a boolean value which should be
true on success.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/binfmt_elf.c?h=v6.11-rc7#n1816
Regards,
Tiwei
