Re: [PATCH v4 02/20] powerpc: Use generic fallocate compatibility syscall

[Christophe Leroy <christophe.leroy@csgroup.eu>]

Le 12/09/2022 à 11:57, Arnd Bergmann a écrit :
> On Mon, Sep 12, 2022, at 10:38 AM, Nicholas Piggin wrote:
>> On Wed Aug 24, 2022 at 12:05 PM AEST, Rohan McLure wrote:
>>> The powerpc fallocate compat syscall handler is identical to the
>>> generic implementation provided by commit 59c10c52f573f ("riscv:
>>> compat: syscall: Add compat_sys_call_table implementation"), and as
>>> such can be removed in favour of the generic implementation.
>>>
>>> A future patch series will replace more architecture-defined syscall
>>> handlers with generic implementations, dependent on introducing generic
>>> implementations that are compatible with powerpc and arm's parameter
>>> reorderings.
>>>
>>> Reported-by: Arnd Bergmann <arnd@arndb.de>
>>> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
>>> ---
>>> V1 -> V2: Remove arch-specific fallocate handler.
>>> V2 -> V3: Remove generic fallocate prototype. Move to beginning of
>>> series.
>>> ---
> 
>>> @@ -16,6 +16,11 @@ typedef u16		compat_ipc_pid_t;
>>>   #include <asm-generic/compat.h>
>>>   
>>>   #ifdef __BIG_ENDIAN__
>>> +#define compat_arg_u64(name)		u32  name##_hi, u32  name##_lo
>>> +#define compat_arg_u64_dual(name)	u32, name##_hi, u32, name##_lo
>>> +#define compat_arg_u64_glue(name)	(((u64)name##_lo & 0xffffffffUL) | \
>>> +					 ((u64)name##_hi << 32))
>>
>> Is there a reason not to put this in asm-generic/compat.h?
>>
>> Possibly you want to put this with the other compat definitions and
>> above the asm-generic include. The generic header expects the arch to
>> include it after defining what it wants to override.
> 
> Yes, makes sense. I think the riscv people added this to asm-generic,
> they tried to do only the minimal parts.
> 
> In theory, any architecture could have its own calling conventions
> for each syscall and have them in the opposite order for one
> endianess. I checked the seven non-generic implementations of the
> sys_fallocate() syscall and all except powerpc have the same
> ABI as the generic one.
> 
> The powerpc difference is that in little-endian mode, only
> the 'len' argument is swapped but the 'offset' argument is
> still high/low:
> 
> long compat_sys_fallocate(int fd, int mode, u32 offset1, u32 offset2,
>                                      u32 len1, u32 len2)
> {
>         return ksys_fallocate(fd, mode, ((loff_t)offset1 << 32) | offset2,
>                              merge_64(len1, len2));
> }
> 
> It's probably best to first fix this by using merge_64(offset1,
> offset2) and allow that patch to be backported to stable kernels,
> before changing it over to the generic code in a separate patch
> within that series.
> 
> A related issue seems to exist in ppc_fadvise64_64(), which
> uses the wrong argument order on ppc32le compat tasks, in addition
> to having at least three different calling conventions across
> architectures.

Do ppc32le exist at all ?

Native ppc32 is be only, and I'm not aware that ppc64 is able to run 
ppc32le compat tasks.

Christophe