On 15 June 2011 15:32, Richard Henderson <rth@xxxxxxxxxxx> wrote:
> On 06/15/2011 01:35 AM, Alexander Graf wrote:
>>> - Â Â Â Â Â Â Â Âabi_ulong arg5 = 0, arg6 = 0, arg7 = 0, arg8 = 0;
>>> + Â Â Â Â Â Â Â Âabi_ulong arg5 = 0, arg6 = 0;
>>>
>>> Â Â Â Â Â Â Â Â nb_args = mips_syscall_args[syscall_num];
>>> Â Â Â Â Â Â Â Â sp_reg = env->active_tc.gpr[29];
>>> Â Â Â Â Â Â Â Â switch (nb_args) {
>>> Â Â Â Â Â Â Â Â /* these arguments are taken from the stack */
>>> Â Â Â Â Â Â Â Â /* FIXME - what to do if get_user() fails? */
>>> - Â Â Â Â Â Â Â Âcase 8: get_user_ual(arg8, sp_reg + 28);
>>> - Â Â Â Â Â Â Â Âcase 7: get_user_ual(arg7, sp_reg + 24);
>>> + Â Â Â Â Â Â Â Âcase 8: /* get_user_ual(arg8, sp_reg + 28); */
>>> + Â Â Â Â Â Â Â Âcase 7: /* get_user_ual(arg7, sp_reg + 24); */
>>
>> I'd prefer to see these and the respective variable definitions #if
>> 0'd with a comment, stating that they're currently unused.
>
> I'd prefer not to see if 0 code. ÂBetter, I think, to mark the
> variables as __attribute__((unused)) with that same comment.
Ideally what we ought to be doing here is actually plumbing arg7
and arg8 through to do_syscall().
Two syscalls in the table, sys_fadvise64_64 and sys_sync_file_range,
take 7 arguments (and we can deduce that they're misimplemented in
do_syscall() for 32 bit MIPS targets, probably by not gluing
together two 32 bit arguments into a 64 bit value to pass to
the libc function); arg8 is needed in order to be able to invoke
these 7 argument syscalls via sys_syscall. (sys_syscall's entry
in the mips_syscall_args[] array says it takes zero arguments,
which I think is just broken.)
Actually fixing the 7-argument syscalls is a bit out of scope,
but I think widening do_syscall() to take 8 arguments is
straightforward enough.
(Incidentally this whole code looks a bit 32 bit specific,
do we support 64 bit mips targets in linux-user mode ?)
-- PMM
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
