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

[SeongJae Park <sj@kernel.org>]

On Tue, 03 Dec 2024 09:40:34 +0100 Benjamin Berg <benjamin@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.


Thanks,
SJ

> 
> Benjamin
> 
> On Mon, 2024-12-02 at 23:02 -0800, SeongJae Park wrote:
> > Hello,
> > 
> > 
> > On Wed, 23 Oct 2024 11:41:20 +0200 Benjamin Berg <benjamin@sipsolutions.net> wrote:
> > 
> > > From: Benjamin Berg <benjamin.berg@intel.com>
> > > 
> > > The PTRACE_GETREGSET API has now existed since Linux 2.6.33. The XSAVE
> > > CPU feature should also be sufficiently common to be able to rely on it.
> > > 
> > > With this, define our internal FP state to be the hosts XSAVE data. Add
> > > discovery for the hosts XSAVE size and place the FP registers at the end
> > > of task_struct so that we can adjust the size at runtime.
> > > 
> > > Next we can implement the regset API on top and update the signal
> > > handling as well as ptrace APIs to use them. Also switch coredump
> > > creation to use the regset API and finally set HAVE_ARCH_TRACEHOOK.
> > > 
> > > This considerably improves the signal frames. Previously they might not
> > > have contained all the registers (i386) and also did not have the
> > > sizes and magic values set to the correct values to permit userspace to
> > > decode the frame.
> > > 
> > > As a side effect, this will permit UML to run on hosts with newer CPU
> > > extensions (such as AMX) that need even more register state.
> > 
> > I just found kunit starts failing from the mainline commit of this patch on my
> > qemu-x86 Debian system, as below.
> > 
> >     $ git checkout 3f17fed2149192c7d3b76a45a6a87b4ff22cd586
> >     $ ./tools/testing/kunit/kunit.py run --kunitconfig mm/damon/tests/ --build_dir ../kunit.out/
> >     [...]
> >     [22:48:27] Configuring KUnit Kernel ...
> >     Regenerating .config ...
> >     Populating config with:
> >     $ make ARCH=um O=../kunit.out/ olddefconfig
> >     [22:48:30] Building KUnit Kernel ...
> >     Populating config with:
> >     $ make ARCH=um O=../kunit.out/ olddefconfig
> >     Building with:
> >     $ make all compile_commands.json ARCH=um O=../kunit.out/ --jobs=40
> >     [...]
> >     [22:48:46] Starting KUnit Kernel (1/1)...
> >     [22:48:46] ============================================================
> >     Running tests with:
> >     $ ../kunit.out/linux kunit.enable=1 mem=1G console=tty kunit_shutdown=halt
> >     [22:48:46] [ERROR] Test: <missing>: Could not find any KTAP output. Did any KUnit tests run?
> >     [22:48:46] ============================================================
> >     [22:48:46] Testing complete. Ran 0 tests: errors: 1
> >     [22:48:46] Elapsed time: 19.285s total, 3.805s configuring, 15.475s building, 0.006s running
> > 
> > > 
> > > Signed-off-by: Benjamin Berg <benjamin.berg@intel.com>
> > [...]
> > > -void arch_init_registers(int pid)
> > > -{
> > > -	struct user_fpxregs_struct fpx_regs;
> > > -	int err;
> > > -
> > > -	err = ptrace(PTRACE_GETFPXREGS, pid, 0, &fpx_regs);
> > > -	if (!err)
> > > -		return;
> > > -
> > > -	if (errno != EIO)
> > > -		panic("check_ptrace : PTRACE_GETFPXREGS failed, errno = %d",
> > > -		      errno);
> > > -
> > > -	have_fpx_regs = 0;
> > > -}
> > > -#else
> > > -
> > > -int get_fp_registers(int pid, unsigned long *regs)
> > > +int arch_init_registers(int pid)
> > >  {
> > > -	return save_fp_registers(pid, regs);
> > > +	struct iovec iov = {
> > > +		/* Just use plenty of space, it does not cost us anything */
> > > +		.iov_len = 2 * 1024 * 1024,
> > > +	};
> > > +	int ret;
> > > +
> > > +	iov.iov_base = mmap(NULL, iov.iov_len, PROT_WRITE | PROT_READ,
> > > +			    MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
> > > +	if (iov.iov_base == MAP_FAILED)
> > > +		return -ENOMEM;
> > > +
> > > +	/* GDB has x86_xsave_length, which uses x86_cpuid_count */
> > > +	ret = ptrace(PTRACE_GETREGSET, pid, NT_X86_XSTATE, &iov);
> > > +	if (ret)
> > > +		ret = -errno;
> > > +	munmap(iov.iov_base, 2 * 1024 * 1024);
> > > +
> > > +	host_fp_size = iov.iov_len;
> > > +
> > > +	return ret;
> > >  }
> > 
> > And seems it fails from the registers initialization step:
> > 
> >     $ ../kunit.out/linux
> >     Core dump limits :
> >             soft - 0
> >             hard - NONE
> >     Checking that ptrace can change system call numbers...OK
> >     Checking syscall emulation for ptrace...OK
> >     Checking environment variables for a tempdir...none found
> >     Checking if /dev/shm is on tmpfs...OK
> >     Checking PROT_EXEC mmap in /dev/shm...OK
> >     Failed to initialize default registers
> > 
> > I'm not familiar with uml code, so reporting this issue first.  Any thought, please?
> > 
> > 
> > Thanks,
> > SJ
> > 
> > [...]
> > 
> 
> 
>