Re: [Qemu-devel] [PATCH 4/4] linux-user: Fix fadvise64() syscall support for Mips32

[Aleksandar Markovic]

Yes, we could replace TARGET_NR_fadvise64 with TARGET_NR_fadvise64_64 in Mips32 
syscall numbers definition list, and expect the same effect. I just prefer to 
leave original symbols for syscall numbers, as they are defined in kernel 
headers, unless there is a compelling reason not to do it.

But, there is more. Code that handles TARGET_NR_fadvise64_64 in Qemu utilizes 
host's posix_advise(). It looks to me that this is just plain incorrect. It 
won't work correctly on 32 bit hosts, with respect to  arguments of 
posix_advise() that are 32-bit (while 64-bit value is passed to them) Instead, 
posix_fadvise64() should have been used, IMHO.

Please let me know if you think I should do anything additionally in relation 
to this matter.

Thanks again!
Aleksandar


______________________________________
From: Qemu-devel address@hidden on behalf of Aleksandar Markovic address@hidden
Sent: Tuesday, October 04, 2016 1:58 PM
To: Peter Maydell; Aleksandar Markovic
Cc: Riku Voipio; QEMU Developers; Laurent Vivier; Miodrag Dinic; Aleksandar 
Rikalo; Petar Jovanovic; Yongbok Kim; Leon Alrae; Aurelien Jarno
Subject: Re: [Qemu-devel] [PATCH 4/4] linux-user: Fix fadvise64() syscall 
support for Mips32

Hi, Peter,

The file in question is arch/mips/kernel/scall32-o32.S. I'll correct this in 
next version. Thank for the reviewing this patch. I am going to answer your 
second question tomorrow. The quick answer is actually yes, but there are 
caveats, that I will explain tomorrow.

Respectfully,
Aleksandar


________________________________________
From: Peter Maydell address@hidden
Sent: Tuesday, October 04, 2016 1:42 PM
To: Aleksandar Markovic
Cc: QEMU Developers; Riku Voipio; Laurent Vivier; Aurelien Jarno; Yongbok Kim; 
Leon Alrae; Petar Jovanovic; Miodrag Dinic; Aleksandar Rikalo; Aleksandar 
Markovic
Subject: Re: [PATCH 4/4] linux-user: Fix fadvise64() syscall support for Mips32

On 4 October 2016 at 11:05, Aleksandar Markovic
<address@hidden> wrote:
> From: Aleksandar Markovic <address@hidden>
>
> By looking at the file arch/mips/kernel/sys.S in Linux kernel,

There doesn't appear to be a file by that name:
http://lxr.free-electrons.com/source/arch/mips/kernel/

> it can be deduced that, for Mips32 platform, syscall
> corresponding to number _NR_fadvise64 translates to kernel
> function sys_fadvise64_64, and that argument layout is as
> follows:
>           0             32 0             32
>          +----------------+----------------+
>   (arg1) |       fd       |     __pad      | (arg2)
>          +----------------+----------------+
>   (arg3) |             buffer              | (arg4)
>          +----------------+----------------+
>   (arg5) |               len               | (arg6)
>          +----------------+----------------+
>   (arg7) |     advise     |    not used    | (arg8)
>          +----------------+----------------+
>
> This can be deduced from glibc code as well, and relevant commits
> in linux kernel and glibc.
>
> Mips32 uniqness is that it does not define _NR_fadvise64_64,
> however its fadvise64 implemantation is identical to
> fadvise64_64 impleanentation on most other platforms.
>
> This patch also fixes the failure LTP test posix_fadvise03, if
> executed on Qemu-emulated Mips32 platform (user mode).

We could alternatively fix this by having
#define TARGET_NR_fadvise64_64 (TARGET_NR_Linux + 254)
in linux-user/mips/syscall_nr.h rather than
#define TARGET_NR_fadvise64 (TARGET_NR_Linux + 254)

right?

thanks
-- PMM