Re: [PATCH for-5.2 0/3] linux-user: fix various sparc64 guest bugs - Mark Cave-Ayland

From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
To: Peter Maydell <peter.maydell@linaro.org>, qemu-devel@nongnu.org
Cc: Giuseppe Musacchio <thatlemon@gmail.com>,
	Richard Henderson <richard.henderson@linaro.org>,
	Laurent Vivier <laurent@vivier.eu>
Subject: Re: [PATCH for-5.2 0/3] linux-user: fix various sparc64 guest bugs
Date: Tue, 10 Nov 2020 12:56:24 +0000	[thread overview]
Message-ID: <21cd093c-404b-cd9c-2b61-fe1c0957d7f4@ilande.co.uk> (raw)
In-Reply-To: <20201105212314.9628-1-peter.maydell@linaro.org>

On 05/11/2020 21:23, Peter Maydell wrote:

> This set of patches fixes bugs which were preventing the
> Debian sparc64 /bin/bash from running:
>   * the target_ucontext structure put the registers in the
>     wrong place (missing alignment specifier, mostly)
>   * the set_context and get_context traps weren't saving fp
>     and i7, which meant that guest code that did a longjmp would
>     crash shortly afterwards (SPARC64 uses these traps to
>     implement setjmp/longjmp)
>   * we were trying to stuff a 64-bit PC into a uint32_t in
>     sigreturn, which caused a SEGV on return from a signal handler
> 
> Review very much desired in particular from anybody who understands
> SPARC register windows and how we handle them in linux-user for
> patch 2! The other patches are straightforward.
> 
> This patchset is sufficient that I can at least chroot into
> a Debian sparc64 chroot and run basic commands like 'ls' from
> the shell prompt (together with Giuseppe Musacchio's patch that
> fixes the stack_t struct).
> 
> There are clearly a bunch of other bugs in sparc signal handling
> (starting with the fact that rt_frame support is simply not
> implemented, but there are also some XXX/FIXME comments about TSTATE
> save/restore in set/get_context and about the FPU state in the signal
> frame code). There's also a Coverity issue about accessing off the
> end of the sregs[] array in the target_mc_fpu struct -- the error is
> actually harmless (we're accessing into the space in the union for
> dregs[16..31] which is what we want to be doing) but I'll probably
> put together a patch to make Coverity happier.

Thanks Peter! This has been broken for a very long time indeed. Once this is merged I 
should probably look at getting a test environment set up.

ATB,

Mark.