| Message ID | 20200210192038.23588-8-adhemerval.zanella@linaro.org |
|---|---|
| State | Dropped |
| Headers |
Received: (qmail 100128 invoked by alias); 10 Feb 2020 19:20:56 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 100000 invoked by uid 89); 10 Feb 2020 19:20:55 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-22.1 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy=nios X-HELO: mail-qk1-f193.google.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:subject:date:message-id:in-reply-to:references; bh=gcRlSIUfDZNzDXLhQXEXcrQpCXiBWs/Mt0pzBbJG87Y=; b=qzJpZrbrfPCcI97bq1Yw+GTMUtCbxBq/zx7g3ogF7AzS+4/+R7o4XE7Mf3RLpzD6ui n5q3//nnKjCg/JNkMNdCYkfFEs9Bomo2jtH01KtbKkgQtPF8X6fwKJe00rHv6b9BzpzN JlNy6iuhVFt61OR/35LvnxRbKt8OVSek8FQg1qS23R13L0lgwtp3kCljEsSeyg2auOF2 lv0mgjwDiq4FlD27Bgd5gG/QxninP7aLfsRYcyHm0zlLJK328hezSLWltLwgh81T3y/m P0OhfFk5AIrhyVibOHUV+UPv7UmhbhgjnJ/jDnN5+AYuhp59bu4WRhUHKUHKyz5iIrIz cbUw== Return-Path: <adhemerval.zanella@linaro.org> From: Adhemerval Zanella <adhemerval.zanella@linaro.org> To: libc-alpha@sourceware.org Subject: [PATCH 08/15] nios2: Use Linux kABI for syscall return Date: Mon, 10 Feb 2020 16:20:31 -0300 Message-Id: <20200210192038.23588-8-adhemerval.zanella@linaro.org> In-Reply-To: <20200210192038.23588-1-adhemerval.zanella@linaro.org> References: <20200210192038.23588-1-adhemerval.zanella@linaro.org> |
Commit Message
Adhemerval Zanella
Feb. 10, 2020, 7:20 p.m. UTC
It changes the nios INTERNAL_SYSCALL_RAW macro to return a negative value instead of 'r2' register value on 'err' macro argument. The macro INTERNAL_SYSCALL_DECL is no longer required, and the INTERNAL_SYSCALL_ERROR_P follows the other Linux kABIS. Checked with a build against nios2-linux-gnu. --- sysdeps/unix/sysv/linux/nios2/sysdep.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
Comments
* Adhemerval Zanella: > +#define INTERNAL_SYSCALL_ERROR_P(val, err) \ > + ((unsigned long) (val) >= (unsigned long) -4095) This should use unsigned long int. Rest looks okay. Thanks, Florian
On Feb 10 2020, Adhemerval Zanella wrote: > diff --git a/sysdeps/unix/sysv/linux/nios2/sysdep.h b/sysdeps/unix/sysv/linux/nios2/sysdep.h > index b02730bd23..eab888df32 100644 > --- a/sysdeps/unix/sysv/linux/nios2/sysdep.h > +++ b/sysdeps/unix/sysv/linux/nios2/sysdep.h > @@ -157,13 +157,14 @@ > (int) result_var; }) > > #undef INTERNAL_SYSCALL_DECL > -#define INTERNAL_SYSCALL_DECL(err) unsigned int err __attribute__((unused)) > +#define INTERNAL_SYSCALL_DECL(err) do { } while (0) > > #undef INTERNAL_SYSCALL_ERROR_P > -#define INTERNAL_SYSCALL_ERROR_P(val, err) ((void) (val), (unsigned int) (err)) > +#define INTERNAL_SYSCALL_ERROR_P(val, err) \ > + ((unsigned long) (val) >= (unsigned long) -4095) Perhaps -4095UL instead? Andreas.
On 11/02/2020 08:20, Florian Weimer wrote: > * Adhemerval Zanella: > >> +#define INTERNAL_SYSCALL_ERROR_P(val, err) \ >> + ((unsigned long) (val) >= (unsigned long) -4095) > > This should use unsigned long int. Rest looks okay. > > Thanks, > Florian > I didn't bother because this snippet will be removed by the 'linux: Consolidate INLINE_SYSCALL' patch in this set, but you are correct. I have changed to: #define INTERNAL_SYSCALL_ERROR_P(val, err) \ ((unsigned long) (val) > -4096UL) (which is the one I used on consolidation).
On 2/10/20 11:20 AM, Adhemerval Zanella wrote: > It changes the nios INTERNAL_SYSCALL_RAW macro to return a negative > value instead of 'r2' register value on 'err' macro argument. > > The macro INTERNAL_SYSCALL_DECL is no longer required, and the > INTERNAL_SYSCALL_ERROR_P follows the other Linux kABIS. > > Checked with a build against nios2-linux-gnu. > --- > sysdeps/unix/sysv/linux/nios2/sysdep.h | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/sysdeps/unix/sysv/linux/nios2/sysdep.h b/sysdeps/unix/sysv/linux/nios2/sysdep.h > index b02730bd23..eab888df32 100644 > --- a/sysdeps/unix/sysv/linux/nios2/sysdep.h > +++ b/sysdeps/unix/sysv/linux/nios2/sysdep.h > @@ -157,13 +157,14 @@ > (int) result_var; }) > > #undef INTERNAL_SYSCALL_DECL > -#define INTERNAL_SYSCALL_DECL(err) unsigned int err __attribute__((unused)) > +#define INTERNAL_SYSCALL_DECL(err) do { } while (0) > > #undef INTERNAL_SYSCALL_ERROR_P > -#define INTERNAL_SYSCALL_ERROR_P(val, err) ((void) (val), (unsigned int) (err)) > +#define INTERNAL_SYSCALL_ERROR_P(val, err) \ > + ((unsigned long) (val) >= (unsigned long) -4095) > > #undef INTERNAL_SYSCALL_ERRNO > -#define INTERNAL_SYSCALL_ERRNO(val, err) ((void) (err), val) > +#define INTERNAL_SYSCALL_ERRNO(val, err) (-(val)) > > #undef INTERNAL_SYSCALL_RAW > #define INTERNAL_SYSCALL_RAW(name, err, nr, args...) \ > @@ -180,8 +181,7 @@ > : "+r" (_r2), "=r" (_err) \ > : ASM_ARGS_##nr \ > : __SYSCALL_CLOBBERS); \ > - _sys_result = _r2; \ > - err = _err; \ > + _sys_result = _err != 0 ? -_r2 : -_r2; \ Is there a typo here ? both cases seem to be -ve > } \ > (int) _sys_result; }) > >
On 19/02/2020 18:40, Vineet Gupta wrote: > On 2/10/20 11:20 AM, Adhemerval Zanella wrote: >> It changes the nios INTERNAL_SYSCALL_RAW macro to return a negative >> value instead of 'r2' register value on 'err' macro argument. >> >> The macro INTERNAL_SYSCALL_DECL is no longer required, and the >> INTERNAL_SYSCALL_ERROR_P follows the other Linux kABIS. >> >> Checked with a build against nios2-linux-gnu. >> --- >> sysdeps/unix/sysv/linux/nios2/sysdep.h | 10 +++++----- >> 1 file changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/sysdeps/unix/sysv/linux/nios2/sysdep.h b/sysdeps/unix/sysv/linux/nios2/sysdep.h >> index b02730bd23..eab888df32 100644 >> --- a/sysdeps/unix/sysv/linux/nios2/sysdep.h >> +++ b/sysdeps/unix/sysv/linux/nios2/sysdep.h >> @@ -157,13 +157,14 @@ >> (int) result_var; }) >> >> #undef INTERNAL_SYSCALL_DECL >> -#define INTERNAL_SYSCALL_DECL(err) unsigned int err __attribute__((unused)) >> +#define INTERNAL_SYSCALL_DECL(err) do { } while (0) >> >> #undef INTERNAL_SYSCALL_ERROR_P >> -#define INTERNAL_SYSCALL_ERROR_P(val, err) ((void) (val), (unsigned int) (err)) >> +#define INTERNAL_SYSCALL_ERROR_P(val, err) \ >> + ((unsigned long) (val) >= (unsigned long) -4095) >> >> #undef INTERNAL_SYSCALL_ERRNO >> -#define INTERNAL_SYSCALL_ERRNO(val, err) ((void) (err), val) >> +#define INTERNAL_SYSCALL_ERRNO(val, err) (-(val)) >> >> #undef INTERNAL_SYSCALL_RAW >> #define INTERNAL_SYSCALL_RAW(name, err, nr, args...) \ >> @@ -180,8 +181,7 @@ >> : "+r" (_r2), "=r" (_err) \ >> : ASM_ARGS_##nr \ >> : __SYSCALL_CLOBBERS); \ >> - _sys_result = _r2; \ >> - err = _err; \ >> + _sys_result = _err != 0 ? -_r2 : -_r2; \ > > Is there a typo here ? both cases seem to be -ve It is, thanks for catching it. I have pushed b790c8c2ed to fix and double checked nios2 syscall handling (arch/nios2/kernel/entry.S:205) to certify that the modification does follow nios2 kABI.
Hi Adhemerval, On 2/20/20 5:14 AM, Adhemerval Zanella wrote: > > > On 19/02/2020 18:40, Vineet Gupta wrote: >> On 2/10/20 11:20 AM, Adhemerval Zanella wrote: >>> It changes the nios INTERNAL_SYSCALL_RAW macro to return a negative >>> value instead of 'r2' register value on 'err' macro argument. >>> >>> The macro INTERNAL_SYSCALL_DECL is no longer required, and the >>> INTERNAL_SYSCALL_ERROR_P follows the other Linux kABIS. >>> >>> Checked with a build against nios2-linux-gnu. >>> --- >>> sysdeps/unix/sysv/linux/nios2/sysdep.h | 10 +++++----- >>> 1 file changed, 5 insertions(+), 5 deletions(-) >>> >>> diff --git a/sysdeps/unix/sysv/linux/nios2/sysdep.h b/sysdeps/unix/sysv/linux/nios2/sysdep.h >>> index b02730bd23..eab888df32 100644 >>> --- a/sysdeps/unix/sysv/linux/nios2/sysdep.h >>> +++ b/sysdeps/unix/sysv/linux/nios2/sysdep.h >>> @@ -157,13 +157,14 @@ >>> (int) result_var; }) >>> >>> #undef INTERNAL_SYSCALL_DECL >>> -#define INTERNAL_SYSCALL_DECL(err) unsigned int err __attribute__((unused)) >>> +#define INTERNAL_SYSCALL_DECL(err) do { } while (0) >>> >>> #undef INTERNAL_SYSCALL_ERROR_P >>> -#define INTERNAL_SYSCALL_ERROR_P(val, err) ((void) (val), (unsigned int) (err)) >>> +#define INTERNAL_SYSCALL_ERROR_P(val, err) \ >>> + ((unsigned long) (val) >= (unsigned long) -4095) >>> >>> #undef INTERNAL_SYSCALL_ERRNO >>> -#define INTERNAL_SYSCALL_ERRNO(val, err) ((void) (err), val) >>> +#define INTERNAL_SYSCALL_ERRNO(val, err) (-(val)) >>> >>> #undef INTERNAL_SYSCALL_RAW >>> #define INTERNAL_SYSCALL_RAW(name, err, nr, args...) \ >>> @@ -180,8 +181,7 @@ >>> : "+r" (_r2), "=r" (_err) \ >>> : ASM_ARGS_##nr \ >>> : __SYSCALL_CLOBBERS); \ >>> - _sys_result = _r2; \ >>> - err = _err; \ >>> + _sys_result = _err != 0 ? -_r2 : -_r2; \ >> >> Is there a typo here ? both cases seem to be -ve > > It is, thanks for catching it. I have pushed b790c8c2ed to fix and > double checked nios2 syscall handling (arch/nios2/kernel/entry.S:205) > to certify that the modification does follow nios2 kABI. Actually the reason I spotted it was trying to replicate similar changes in ARC port and it seems to be hosed now. It is quite likely a snaufu at my end, but I don't quite understand the new logic. Consider brk syscall which does __curbrk = (void *) INTERNAL_SYSCALL_CALL (brk, addr); Through a maze of defines this ends up calling INTERNAL_SYSCALL_RAW which seems be unconditionally converting !0 value (success) into -ve and returning it. So won't it convert a legit brk address return into a -ve and save in __curbrk. Am I not following this correctly ? Thx, -Vineet
On 2/20/20 12:39 PM, Vineet Gupta wrote:
> Am I not following this correctly ?
Oh never mind, they use 2 seperate regs to convey syscall result and error, so
your code is right.
On 20/02/2020 18:04, Vineet Gupta wrote: > > Through a maze of defines this ends up calling INTERNAL_SYSCALL_RAW which seems be > unconditionally converting !0 value (success) into -ve and returning it. So won't > it convert a legit brk address return into a -ve and save in __curbrk. > On 2/20/20 12:39 PM, Vineet Gupta wrote: >> Am I not following this correctly ? > > Oh never mind, they use 2 seperate regs to convey syscall result and error, so > your code is right. > One of my goals is to disentangle the {INTERNAL,INLINE}_SYSCALL macros to consolidate their definitions and move the arch-specific bits to inline functions instead of macros. Another one is to remove the requirement to define similar assembly macros to be used by the auto-generated one. The idea is an architecture will just need to define a set of inline_syscall{0-6} functions.
diff --git a/sysdeps/unix/sysv/linux/nios2/sysdep.h b/sysdeps/unix/sysv/linux/nios2/sysdep.h index b02730bd23..eab888df32 100644 --- a/sysdeps/unix/sysv/linux/nios2/sysdep.h +++ b/sysdeps/unix/sysv/linux/nios2/sysdep.h @@ -157,13 +157,14 @@ (int) result_var; }) #undef INTERNAL_SYSCALL_DECL -#define INTERNAL_SYSCALL_DECL(err) unsigned int err __attribute__((unused)) +#define INTERNAL_SYSCALL_DECL(err) do { } while (0) #undef INTERNAL_SYSCALL_ERROR_P -#define INTERNAL_SYSCALL_ERROR_P(val, err) ((void) (val), (unsigned int) (err)) +#define INTERNAL_SYSCALL_ERROR_P(val, err) \ + ((unsigned long) (val) >= (unsigned long) -4095) #undef INTERNAL_SYSCALL_ERRNO -#define INTERNAL_SYSCALL_ERRNO(val, err) ((void) (err), val) +#define INTERNAL_SYSCALL_ERRNO(val, err) (-(val)) #undef INTERNAL_SYSCALL_RAW #define INTERNAL_SYSCALL_RAW(name, err, nr, args...) \ @@ -180,8 +181,7 @@ : "+r" (_r2), "=r" (_err) \ : ASM_ARGS_##nr \ : __SYSCALL_CLOBBERS); \ - _sys_result = _r2; \ - err = _err; \ + _sys_result = _err != 0 ? -_r2 : -_r2; \ } \ (int) _sys_result; })