On Sat, Feb 01, 2014 at 12:09:06PM +0000, Peter Maydell wrote: > On 1 February 2014 08:41, Stefan Weil <s...@weilnetz.de> 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).
Hi, I've tested the patch, it works. CRIS insn stream is always 16bit aligned, I think Stefans patch is OK. Cheers, Edgar
