sign extension for 32bit syscalls on ppc64

sign extension for 32bit syscalls on ppc64

[Christoph Hellwig]

For 32bit syscalls implemented in arch/powerpc/ we're doing our own
sign-extension where an int argument is passed as u32 in the prototype
and then casted to int later on:

asmlinkage long compat_sys_sendfile(u32 out_fd, u32 in_fd, compat_off_t
__user * offset, u32 count)

{
	...
	ret = sys_sendfile((int)out_fd, (int)in_fd, up, count);
	..
}

OTOH various syscalls in the generic code don't do that and it seems to
still work fine.  I have patches for various new generic compat
routines, and they all seem to work fine without this sign extension.

What's the exact sign extention rules for ppc64?

Re: sign extension for 32bit syscalls on ppc64

[Paul Mackerras]

Christoph Hellwig writes:

> OTOH various syscalls in the generic code don't do that and it seems to
> still work fine.  I have patches for various new generic compat
> routines, and they all seem to work fine without this sign extension.
> 
> What's the exact sign extention rules for ppc64?

As far as the ABI and the compiler are concerned, 32-bit values stored
in registers are always correctly extended to 64 bits according to
their type.  That is, the top 32 bits of the register will be either
all zeroes or all ones, and will only be all ones for negative values
of a signed type.

Syscall arguments are zero-extended in the syscall entry path for
32-bit processes (see lines 131-136 of arch/powerpc/kernel/entry_64.S)
so if the argument is an int, we do an (int) cast in the compat
wrapper, which makes the compiler emit an extsw instruction to get the
value correctly sign-extended.

However, it's debatable whether file descriptor arguments really need
to be sign-extended.  Although they are typed as int, negative values
are not generally valid (although there are one or two cases where
they are), and an fd value of 4294967295 will generate an EBADF error
just as well as -1 will.

Paul.

Re: sign extension for 32bit syscalls on ppc64

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 658 bytes --]

On Sat, 29 Apr 2006 09:32:17 +1000 Paul Mackerras <paulus@samba.org> wrote:
>
> However, it's debatable whether file descriptor arguments really need
> to be sign-extended.  Although they are typed as int, negative values
> are not generally valid (although there are one or two cases where
> they are), and an fd value of 4294967295 will generate an EBADF error
> just as well as -1 will.

They do need to be sign extended for the new *at syscalls where you can pass
a sepecial value AT_FDCWD (-10) to represent the current directory ...

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: sign extension for 32bit syscalls on ppc64

[Paul Mackerras]

Stephen Rothwell writes:

> They do need to be sign extended for the new *at syscalls where you can pass
> a sepecial value AT_FDCWD (-10) to represent the current directory ...

So, compat_sys_openat() in fs/compat.c looks wrong to me then, if
AT_FDCWD is part of the ABI and not just an internal thing.  And I'm
now not sure whether some of the other *at syscalls do in fact need
compat wrappers...

Paul.

Re: sign extension for 32bit syscalls on ppc64

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1189 bytes --]

On Sat, 29 Apr 2006 12:33:37 +1000 Paul Mackerras <paulus@samba.org> wrote:
>
> Stephen Rothwell writes:
> 
> > They do need to be sign extended for the new *at syscalls where you can pass
> > a sepecial value AT_FDCWD (-10) to represent the current directory ...
> 
> So, compat_sys_openat() in fs/compat.c looks wrong to me then, if
> AT_FDCWD is part of the ABI and not just an internal thing.  And I'm
> now not sure whether some of the other *at syscalls do in fact need
> compat wrappers...

AT_FDCWD (actually -100, sorry) is not protected by __KERNEL__ in
include/linux/fcntl.h and sys_openat does not stop it being passed, so I
assume it is part of the user ABI.

The existence of AT_FDCWD is what stopped me from wiring up all the *at
syscalls earlier.  It also sparked another discussion to try to formulate
some generic wrappers for the compat routines.

Has any testing been done on these interfaces that involves 32 bit
processes passing AT_FDCWD on 64 bit kernels (I realise that it will work
for some architectures but I suspect not ppc64).

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: sign extension for 32bit syscalls on ppc64

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 451 bytes --]

On Sat, 29 Apr 2006 13:16:40 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> AT_FDCWD (actually -100, sorry) is not protected by __KERNEL__ in
> include/linux/fcntl.h and sys_openat does not stop it being passed, so I
> assume it is part of the user ABI.

And AT_FDCWD is mentioned in the Solaris documentation for openat(2).

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: sign extension for 32bit syscalls on ppc64

[Andreas Schwab]

Stephen Rothwell <sfr@canb.auug.org.au> writes:

> Has any testing been done on these interfaces that involves 32 bit
> processes passing AT_FDCWD on 64 bit kernels (I realise that it will work
> for some architectures but I suspect not ppc64).

Appears to work fine for me (tested with openat).

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: sign extension for 32bit syscalls on ppc64

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 725 bytes --]

Hi Andreas,

On Sat, 29 Apr 2006 15:46:11 +0200 Andreas Schwab <schwab@suse.de> wrote:
>
> Stephen Rothwell <sfr@canb.auug.org.au> writes:
> 
> > Has any testing been done on these interfaces that involves 32 bit
> > processes passing AT_FDCWD on 64 bit kernels (I realise that it will work
> > for some architectures but I suspect not ppc64).
> 
> Appears to work fine for me (tested with openat).

Try mkdirat. openat has a compat wrapper that has the dfd paramater
declared as a unsigned int and passes it to sys_openat, whose first
paramter is decalred to be int, so the sign extension gets done.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: sign extension for 32bit syscalls on ppc64

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 666 bytes --]

On Mon, 1 May 2006 10:05:18 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Try mkdirat. openat has a compat wrapper that has the dfd paramater
> declared as a unsigned int and passes it to sys_openat, whose first
> paramter is decalred to be int, so the sign extension gets done.

OK, I actually tried this and it works! :-(

I traced the code path and it turns out that the place we check for the
-100 (in do_path_lookup), the compiler has used a cmpwi instruction and so
ignores the top 32 bits.  Thus we get away with the ABI abuse!

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]