diff mbox series

[1/4] sparc: Move __IGNORE* entries to non uapi header

Message ID 1536036087-15260-2-git-send-email-firoz.khan@linaro.org
State Changes Requested
Delegated to: David Miller
Headers show
Series System call table generation support | expand

Commit Message

Firoz Khan Sept. 4, 2018, 4:41 a.m. UTC
All the  __IGNORE* entries are resides in the uapi header
file and it is not used by any user space applications.

One of the patch in this patch series will generate the
uapi header file and system call table file. So if we move
all the __IGNORE* entries to non uapi header, it will simplify
the uapi header and system call table file generation script.

It is correct to keep __IGNORE* entry in non uapi header
asm/unistd.h while uapi/asm/unistd.h must hold information
only useful for user space applications.

Signed-off-by: Firoz Khan <firoz.khan@linaro.org>
---
 arch/sparc/include/asm/unistd.h      | 25 +++++++++++++++++++++++++
 arch/sparc/include/uapi/asm/unistd.h | 25 -------------------------
 2 files changed, 25 insertions(+), 25 deletions(-)

Comments

Arnd Bergmann Sept. 6, 2018, 3:28 p.m. UTC | #1
On Tue, Sep 4, 2018 at 6:42 AM Firoz Khan <firoz.khan@linaro.org> wrote:

> +++ b/arch/sparc/include/uapi/asm/unistd.h
> @@ -15,12 +15,6 @@
>  #ifndef _UAPI_SPARC_UNISTD_H
>  #define _UAPI_SPARC_UNISTD_H
>
> -#ifndef __32bit_syscall_numbers__
> -#ifndef __arch64__
> -#define __32bit_syscall_numbers__
> -#endif
> -#endif

This is certainly required in the uapi header as of this patch,
without it all the numbers are wrong when you include the
file from user space.

I suppose it can be removed later once the header is replaced
with the two generated versions,

> -/* Bitmask values returned from kern_features system call.  */
> -#define KERN_FEATURE_MIXED_MODE_STACK  0x00000001

I'm fairly sure this also needs to remain in the uapi/asm/unistd.h header
as a start, so that user space can call the sys_kern_features() system
call and interpret its result when only the first patch is applied.

> -#ifdef __32bit_syscall_numbers__
> -/* Sparc 32-bit only has the "setresuid32", "getresuid32" variants,
> - * it never had the plain ones and there is no value to adding those
> - * old versions into the syscall table.
> - */
> -#define __IGNORE_setresuid
> -#define __IGNORE_getresuid
> -#define __IGNORE_setresgid
> -#define __IGNORE_getresgid
> -#endif
> -
> -/* Sparc doesn't have protection keys. */
> -#define __IGNORE_pkey_mprotect
> -#define __IGNORE_pkey_alloc
> -#define __IGNORE_pkey_free
> -

This part is fine.

      Arnd
kernel test robot Sept. 6, 2018, 7:09 p.m. UTC | #2
Hi Firoz,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on sparc-next/master]
[also build test WARNING on v4.19-rc2 next-20180906]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Firoz-Khan/sparc-Move-__IGNORE-entries-to-non-uapi-header/20180907-011700
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/sparc-next.git master
config: sparc-defconfig (attached as .config)
compiler: sparc-linux-gcc (GCC) 8.1.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=8.1.0 make.cross ARCH=sparc 

All warnings (new ones prefixed by >>):

   <stdin>:765:2: warning: #warning syscall mmap2 not implemented [-Wcpp]
>> <stdin>:768:2: warning: #warning syscall truncate64 not implemented [-Wcpp]
>> <stdin>:771:2: warning: #warning syscall ftruncate64 not implemented [-Wcpp]
   <stdin>:852:2: warning: #warning syscall fcntl64 not implemented [-Wcpp]
   <stdin>:1332:2: warning: #warning syscall io_pgetevents not implemented [-Wcpp]
   <stdin>:1335:2: warning: #warning syscall rseq not implemented [-Wcpp]

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Firoz Khan Sept. 18, 2018, 11:53 a.m. UTC | #3
On 6 September 2018 at 20:58, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tue, Sep 4, 2018 at 6:42 AM Firoz Khan <firoz.khan@linaro.org> wrote:
>
>> +++ b/arch/sparc/include/uapi/asm/unistd.h
>> @@ -15,12 +15,6 @@
>>  #ifndef _UAPI_SPARC_UNISTD_H
>>  #define _UAPI_SPARC_UNISTD_H
>>
>> -#ifndef __32bit_syscall_numbers__
>> -#ifndef __arch64__
>> -#define __32bit_syscall_numbers__
>> -#endif
>> -#endif
>
> This is certainly required in the uapi header as of this patch,
> without it all the numbers are wrong when you include the
> file from user space.
>
> I suppose it can be removed later once the header is replaced
> with the two generated versions,

The script will generate 2 versions, ie, unistd_32.h and unistd_64.h.
Please give few more pointer here.

>
>> -/* Bitmask values returned from kern_features system call.  */
>> -#define KERN_FEATURE_MIXED_MODE_STACK  0x00000001
>
> I'm fairly sure this also needs to remain in the uapi/asm/unistd.h header
> as a start, so that user space can call the sys_kern_features() system
> call and interpret its result when only the first patch is applied.

Sure. I'll update in the next patch series. Thanks!

>
>> -#ifdef __32bit_syscall_numbers__
>> -/* Sparc 32-bit only has the "setresuid32", "getresuid32" variants,
>> - * it never had the plain ones and there is no value to adding those
>> - * old versions into the syscall table.
>> - */
>> -#define __IGNORE_setresuid
>> -#define __IGNORE_getresuid
>> -#define __IGNORE_setresgid
>> -#define __IGNORE_getresgid
>> -#endif
>> -
>> -/* Sparc doesn't have protection keys. */
>> -#define __IGNORE_pkey_mprotect
>> -#define __IGNORE_pkey_alloc
>> -#define __IGNORE_pkey_free
>> -
>
> This part is fine.
>
>       Arnd
Arnd Bergmann Sept. 24, 2018, 9:06 p.m. UTC | #4
On Tue, Sep 18, 2018 at 1:53 PM Firoz Khan <firoz.khan@linaro.org> wrote:
>
> On 6 September 2018 at 20:58, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Tue, Sep 4, 2018 at 6:42 AM Firoz Khan <firoz.khan@linaro.org> wrote:
> >
> >> +++ b/arch/sparc/include/uapi/asm/unistd.h
> >> @@ -15,12 +15,6 @@
> >>  #ifndef _UAPI_SPARC_UNISTD_H
> >>  #define _UAPI_SPARC_UNISTD_H
> >>
> >> -#ifndef __32bit_syscall_numbers__
> >> -#ifndef __arch64__
> >> -#define __32bit_syscall_numbers__
> >> -#endif
> >> -#endif
> >
> > This is certainly required in the uapi header as of this patch,
> > without it all the numbers are wrong when you include the
> > file from user space.
> >
> > I suppose it can be removed later once the header is replaced
> > with the two generated versions,
>
> The script will generate 2 versions, ie, unistd_32.h and unistd_64.h.
> Please give few more pointer here.

You still need to check at least for __arch64__ in asm/unistd.h in order
to pick which header to include, right? Since the
__32bit_syscall_numbers__ check was already in the public header
file, it may also be possible that there is some user space application
that sets this in order to get the 32-bit definitions. This might e.g.
be needed for strace.

     Arnd
diff mbox series

Patch

diff --git a/arch/sparc/include/asm/unistd.h b/arch/sparc/include/asm/unistd.h
index b2a6a95..f120b6b 100644
--- a/arch/sparc/include/asm/unistd.h
+++ b/arch/sparc/include/asm/unistd.h
@@ -17,6 +17,12 @@ 
 
 #include <uapi/asm/unistd.h>
 
+#ifndef __32bit_syscall_numbers__
+#ifndef __arch64__
+#define __32bit_syscall_numbers__
+#endif
+#endif
+
 #ifdef __32bit_syscall_numbers__
 #else
 #define __NR_time		231 /* Linux sparc32                               */
@@ -45,4 +51,23 @@ 
 #define __ARCH_WANT_COMPAT_SYS_SENDFILE
 #endif
 
+/* Bitmask values returned from kern_features system call.  */
+#define KERN_FEATURE_MIXED_MODE_STACK	0x00000001
+
+#ifdef __32bit_syscall_numbers__
+/* Sparc 32-bit only has the "setresuid32", "getresuid32" variants,
+ * it never had the plain ones and there is no value to adding those
+ * old versions into the syscall table.
+ */
+#define __IGNORE_setresuid
+#define __IGNORE_getresuid
+#define __IGNORE_setresgid
+#define __IGNORE_getresgid
+#endif
+
+/* Sparc doesn't have protection keys. */
+#define __IGNORE_pkey_mprotect
+#define __IGNORE_pkey_alloc
+#define __IGNORE_pkey_free
+
 #endif /* _SPARC_UNISTD_H */
diff --git a/arch/sparc/include/uapi/asm/unistd.h b/arch/sparc/include/uapi/asm/unistd.h
index 09acf0d..ccf4bea 100644
--- a/arch/sparc/include/uapi/asm/unistd.h
+++ b/arch/sparc/include/uapi/asm/unistd.h
@@ -15,12 +15,6 @@ 
 #ifndef _UAPI_SPARC_UNISTD_H
 #define _UAPI_SPARC_UNISTD_H
 
-#ifndef __32bit_syscall_numbers__
-#ifndef __arch64__
-#define __32bit_syscall_numbers__
-#endif
-#endif
-
 #define __NR_restart_syscall      0 /* Linux Specific				   */
 #define __NR_exit                 1 /* Common                                      */
 #define __NR_fork                 2 /* Common                                      */
@@ -430,23 +424,4 @@ 
 
 #define NR_syscalls		361
 
-/* Bitmask values returned from kern_features system call.  */
-#define KERN_FEATURE_MIXED_MODE_STACK	0x00000001
-
-#ifdef __32bit_syscall_numbers__
-/* Sparc 32-bit only has the "setresuid32", "getresuid32" variants,
- * it never had the plain ones and there is no value to adding those
- * old versions into the syscall table.
- */
-#define __IGNORE_setresuid
-#define __IGNORE_getresuid
-#define __IGNORE_setresgid
-#define __IGNORE_getresgid
-#endif
-
-/* Sparc doesn't have protection keys. */
-#define __IGNORE_pkey_mprotect
-#define __IGNORE_pkey_alloc
-#define __IGNORE_pkey_free
-
 #endif /* _UAPI_SPARC_UNISTD_H */