Re: [Qemu-devel] [PATCH] linux-user: Fix trampoline code for CRIS

[Peter Maydell]

On 1 February 2014 08:41, Stefan Weil <address@hidden> wrote:
> __put_user can write bytes, words (2 bytes) or longwords (4 bytes).
> Here obviously words should have been written, but bytes were written,
> so values like 0x9c5f were truncated to 0x5f.
>
> Fix this by changing retcode from uint8_t to to uint16_t in
> target_signal_frame and also in the unused rt_signal_frame.

I believe this will do the right thing. The other possible approach
would be to do what the kernel does here (and what some of
the QEMU code for other targets does, eg x86) and put in the cast:

http://lxr.free-electrons.com/source/arch/cris/arch-v10/kernel/signal.c#L261

261                 /* This is movu.w __NR_sigreturn, r9; break 13; */
262                 err |= __put_user(0x9c5f,         (short
__user*)(frame->retcode+0));
263                 err |= __put_user(__NR_sigreturn, (short
__user*)(frame->retcode+2));
264                 err |= __put_user(0xe93d,         (short
__user*)(frame->retcode+4));

(obviously we'd want "(uint16_t *)").

Since CRIS looks (from a scan through its translate.c) like
a variable-width instruction set (in the sense that insns can
have immediate operands which might be 1/2/4 bytes long)
I think there's an argument here for following the kernel and
keeping retcode[] a byte array, for the implausible case where
we want to change the trampoline sequence to include an
insn with a 1 byte immediate value or something.

Either way I believe the endianness handling should be correct
since __put_user does host-to-target swapping for us.

It might be possible to test this by extracting some of the
userspace binaries from the cris system emulation test image
on the QEMU wiki (or it might not).

thanks
-- PMM