public inbox for [email protected]
 help / color / mirror / Atom feed
* [PATCH liburing 0/3] Add x86 32-bit support for the nolibc build
@ 2022-04-14 22:41 Ammar Faizi
  2022-04-14 22:41 ` [PATCH liburing 1/3] arch/syscall-defs: Use `__NR_mmap2` instead of `__NR_mmap` for x86 32-bit Ammar Faizi
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Ammar Faizi @ 2022-04-14 22:41 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Ammar Faizi, io-uring Mailing List, GNU/Weeb Mailing List,
	Pavel Begunkov, Alviro Iskandar Setiawan

Hi,

This series adds nolibc support for x86 32-bit. There are 3 patches in
this series:

1) Use `__NR_mmap2` instead of `__NR_mmap` for x86 32-bit.
2) Provide `get_page_size()` function for x86 32-bit.
3) Add x86 32-bit native syscall support.

The most noticeable changes is the patch 3. Unlike x86-64, only
use native syscall from the __do_syscall macros when CONFIG_NOLIBC is
enabled for 32-bit build. The reason is because the libc syscall
wrapper can do better in 32-bit. The libc syscall wrapper can dispatch
the best syscall instruction that the environment is supported, there
are at least two variants syscall instruction for x86 32-bit, they are:
`int $0x80` and `sysenter`. The `int $0x80` instruction is always
available, but `sysenter` is not, it relies on VDSO. liburing always
uses `int $0x80` for syscall if it's compiled with CONFIG_NOLIBC,
otherwise it uses whatever the libc provides.

Extra notes for __do_syscall6() macro:
On i386, the 6th argument of syscall goes in %ebp. However, both Clang
and GCC cannot use %ebp in the clobber list and in the "r" constraint
without using -fomit-frame-pointer. To make it always available for
any kind of compilation, the below workaround is implemented:

  1) Push the 6-th argument.
  2) Push %ebp.
  3) Load the 6-th argument from 4(%esp) to %ebp.
  4) Do the syscall (int $0x80).
  5) Pop %ebp (restore the old value of %ebp).
  6) Add %esp by 4 (undo the stack pointer).

WARNING:
  Don't use register variables for __do_syscall6(), there is a known
  GCC bug that results in an endless loop.

BugLink: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105032


===== How is this tested? =====

This has been tested on x86-64 Linux (compiled with 32-bit bin support)
with the following commands:

  sudo apt-get install gcc-i686-linux-gnu g++-i686-linux-gnu -y;
  ./configure --cc=i686-linux-gnu-gcc --cxx=i686-linux-gnu-g++ --nolibc;
  sudo make -j8 runtests;


Signed-off-by: Alviro Iskandar Setiawan <[email protected]>
Signed-off-by: Ammar Faizi <[email protected]>
---

Ammar Faizi (3):
  arch/syscall-defs: Use `__NR_mmap2` instead of `__NR_mmap` for x86 32-bit
  arch/x86/lib: Provide `get_page_size()` function for x86 32-bit
  arch/x86/syscall: Add x86 32-bit native syscall support

 src/arch/syscall-defs.h |  11 ++-
 src/arch/x86/lib.h      |  17 -----
 src/arch/x86/syscall.h  | 150 +++++++++++++++++++++++++++++++++++++++-
 3 files changed, 157 insertions(+), 21 deletions(-)


base-commit: c47b44afb686d794f8ed62feb77e9c42431c8444
-- 
Ammar Faizi


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH liburing 1/3] arch/syscall-defs: Use `__NR_mmap2` instead of `__NR_mmap` for x86 32-bit
  2022-04-14 22:41 [PATCH liburing 0/3] Add x86 32-bit support for the nolibc build Ammar Faizi
@ 2022-04-14 22:41 ` Ammar Faizi
  2022-04-14 22:41 ` [PATCH liburing 2/3] arch/x86/lib: Provide `get_page_size()` function " Ammar Faizi
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Ammar Faizi @ 2022-04-14 22:41 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Ammar Faizi, io-uring Mailing List, GNU/Weeb Mailing List,
	Pavel Begunkov, Alviro Iskandar Setiawan

A preparation to add x86 32-bit native syscall support for the nolibc
build. On x86 32-bit, the __NR_mmap maps to sys_old_mmap:

  long sys_old_mmap(struct mmap_arg_struct __user *arg);

which only receives one argument and is not suitable with the canonical
mmap() definition we use. As such, use __NR_mmap2 that maps to
sys_mmap_pgoff:

  long sys_mmap_pgoff(unsigned long addr, unsigned long len,
                      unsigned long prot, unsigned long flags,
                      unsigned long fd, unsigned long pgoff);

Note: For __NR_mmap2, the offset must be shifted-right by 12-bit.

Co-authored-by: Alviro Iskandar Setiawan <[email protected]>
Signed-off-by: Alviro Iskandar Setiawan <[email protected]>
Signed-off-by: Ammar Faizi <[email protected]>
---
 src/arch/syscall-defs.h | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/arch/syscall-defs.h b/src/arch/syscall-defs.h
index f79f56a..1e8ae1b 100644
--- a/src/arch/syscall-defs.h
+++ b/src/arch/syscall-defs.h
@@ -6,8 +6,15 @@
 static inline void *__sys_mmap(void *addr, size_t length, int prot, int flags,
 			       int fd, off_t offset)
 {
-	return (void *) __do_syscall6(__NR_mmap, addr, length, prot, flags, fd,
-				      offset);
+	int nr;
+
+#if defined(__i386__)
+	nr = __NR_mmap2;
+	offset >>= 12;
+#else
+	nr = __NR_mmap;
+#endif
+	return (void *) __do_syscall6(nr, addr, length, prot, flags, fd, offset);
 }
 
 static inline int __sys_munmap(void *addr, size_t length)
-- 
Ammar Faizi


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH liburing 2/3] arch/x86/lib: Provide `get_page_size()` function for x86 32-bit
  2022-04-14 22:41 [PATCH liburing 0/3] Add x86 32-bit support for the nolibc build Ammar Faizi
  2022-04-14 22:41 ` [PATCH liburing 1/3] arch/syscall-defs: Use `__NR_mmap2` instead of `__NR_mmap` for x86 32-bit Ammar Faizi
@ 2022-04-14 22:41 ` Ammar Faizi
  2022-04-14 22:41 ` [PATCH liburing 3/3] arch/x86/syscall: Add x86 32-bit native syscall support Ammar Faizi
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Ammar Faizi @ 2022-04-14 22:41 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Ammar Faizi, io-uring Mailing List, GNU/Weeb Mailing List,
	Pavel Begunkov, Alviro Iskandar Setiawan

A preparation to add nolibc support for x86 32-bit. Provide
get_page_size() function for x86 32-bit.

x86 32-bit and x86-64, both have the same page size 4K, and they can
share the same function definition. Just remove the #ifdef here.

Co-authored-by: Alviro Iskandar Setiawan <[email protected]>
Signed-off-by: Alviro Iskandar Setiawan <[email protected]>
Signed-off-by: Ammar Faizi <[email protected]>
---
 src/arch/x86/lib.h | 17 -----------------
 1 file changed, 17 deletions(-)

diff --git a/src/arch/x86/lib.h b/src/arch/x86/lib.h
index bacf74e..e6a74f3 100644
--- a/src/arch/x86/lib.h
+++ b/src/arch/x86/lib.h
@@ -7,26 +7,9 @@
 #ifndef LIBURING_ARCH_X86_LIB_H
 #define LIBURING_ARCH_X86_LIB_H
 
-#if defined(__x86_64__)
-
 static inline long get_page_size(void)
 {
 	return 4096;
 }
 
-#else /* #if defined(__x86_64__) */
-
-/*
- * For x86 (32-bit), fallback to libc wrapper.
- * We can't use CONFIG_NOLIBC for x86 (32-bit) at the moment.
- *
- * TODO: Add x86 (32-bit) nolibc support.
- */
-#ifdef CONFIG_NOLIBC
-	#error "x86 (32-bit) is currently not supported for nolibc builds"
-#endif
-#include "../generic/lib.h"
-
-#endif /* #if defined(__x86_64__) */
-
 #endif /* #ifndef LIBURING_ARCH_X86_LIB_H */
-- 
Ammar Faizi


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH liburing 3/3] arch/x86/syscall: Add x86 32-bit native syscall support
  2022-04-14 22:41 [PATCH liburing 0/3] Add x86 32-bit support for the nolibc build Ammar Faizi
  2022-04-14 22:41 ` [PATCH liburing 1/3] arch/syscall-defs: Use `__NR_mmap2` instead of `__NR_mmap` for x86 32-bit Ammar Faizi
  2022-04-14 22:41 ` [PATCH liburing 2/3] arch/x86/lib: Provide `get_page_size()` function " Ammar Faizi
@ 2022-04-14 22:41 ` Ammar Faizi
  2022-04-18  2:01 ` [PATCH liburing 0/3] Add x86 32-bit support for the nolibc build Jens Axboe
  2022-04-18 15:24 ` Jens Axboe
  4 siblings, 0 replies; 8+ messages in thread
From: Ammar Faizi @ 2022-04-14 22:41 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Ammar Faizi, io-uring Mailing List, GNU/Weeb Mailing List,
	Pavel Begunkov, David Laight, Alviro Iskandar Setiawan

Create __do_syscall{0..6} macros for x86 32-bit. Unlike x86-64, only
use these macros when CONFIG_NOLIBC is enabled for a 32-bit build. The
reason is that the libc syscall wrapper can do better in 32-bit.

libc syscall wrapper can dispatch the best syscall instruction that the
environment is supported, there are at least two variants of syscall
instruction for x86 32-bit, they are: `int $0x80` and `sysenter`. The
`int $0x80` instruction is always available, but `sysenter` is not, it
relies on VDSO. liburing always uses `int $0x80` for syscall if it's
compiled with CONFIG_NOLIBC, otherwise, it uses whatever the libc
provides.

Extra notes for __do_syscall6() macro:
On i386, the 6th argument of syscall goes in %ebp. However, both Clang
and GCC cannot use %ebp in the clobber list and the "r" constraint
without using -fomit-frame-pointer. To make it always available for any
kind of compilation, the below workaround is implemented:

  1) Push the 6-th argument.
  2) Push %ebp.
  3) Load the 6-th argument from 4(%esp) to %ebp.
  4) Do the syscall (int $0x80).
  5) Pop %ebp (restore the old value of %ebp).
  6) Add %esp by 4 (undo the stack pointer).

WARNING:
  Don't use register variables for __do_syscall6(), there is a known
  GCC bug that results in an endless loop.

BugLink: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105032

Link: https://lore.kernel.org/lkml/[email protected]
Suggested-by: David Laight <[email protected]>
Co-authored-by: Alviro Iskandar Setiawan <[email protected]>
Signed-off-by: Alviro Iskandar Setiawan <[email protected]>
Signed-off-by: Ammar Faizi <[email protected]>
---
 src/arch/x86/syscall.h | 150 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 148 insertions(+), 2 deletions(-)

diff --git a/src/arch/x86/syscall.h b/src/arch/x86/syscall.h
index 89a68f6..8cd24dd 100644
--- a/src/arch/x86/syscall.h
+++ b/src/arch/x86/syscall.h
@@ -151,10 +151,156 @@
  * TODO: Add x86 (32-bit) nolibc support.
  */
 #ifdef CONFIG_NOLIBC
-	#error "x86 (32-bit) is currently not supported for nolibc builds"
-#endif
+/**
+ * Note for syscall registers usage (x86, 32-bit):
+ *   - %eax is the syscall number.
+ *   - %eax is also the return value.
+ *   - %ebx is the 1st argument.
+ *   - %ecx is the 2nd argument.
+ *   - %edx is the 3rd argument.
+ *   - %esi is the 4th argument.
+ *   - %edi is the 5th argument.
+ *   - %ebp is the 6th argument.
+ */
+
+#define __do_syscall0(NUM) ({			\
+	intptr_t eax;				\
+						\
+	__asm__ volatile(			\
+		"int	$0x80"			\
+		: "=a"(eax)	/* %eax */	\
+		: "a"(NUM)	/* %eax */	\
+		: "memory"			\
+	);					\
+	eax;					\
+})
+
+#define __do_syscall1(NUM, ARG1) ({		\
+	intptr_t eax;				\
+						\
+	__asm__ volatile(			\
+		"int	$0x80"			\
+		: "=a"(eax)	/* %eax */	\
+		: "a"(NUM),	/* %eax */	\
+		  "b"((ARG1))	/* %ebx */	\
+		: "memory"			\
+	);					\
+	eax;					\
+})
+
+#define __do_syscall2(NUM, ARG1, ARG2) ({	\
+	intptr_t eax;				\
+						\
+	__asm__ volatile(			\
+		"int	$0x80"			\
+		: "=a" (eax)	/* %eax */	\
+		: "a"(NUM),	/* %eax */	\
+		  "b"((ARG1)),	/* %ebx */	\
+		  "c"((ARG2))	/* %ecx */	\
+		: "memory"			\
+	);					\
+	eax;					\
+})
+
+#define __do_syscall3(NUM, ARG1, ARG2, ARG3) ({	\
+	intptr_t eax;				\
+						\
+	__asm__ volatile(			\
+		"int	$0x80"			\
+		: "=a" (eax)	/* %eax */	\
+		: "a"(NUM),	/* %eax */	\
+		  "b"((ARG1)),	/* %ebx */	\
+		  "c"((ARG2)),	/* %ecx */	\
+		  "d"((ARG3))	/* %edx */	\
+		: "memory"			\
+	);					\
+	eax;					\
+})
+
+#define __do_syscall4(NUM, ARG1, ARG2, ARG3, ARG4) ({	\
+	intptr_t eax;					\
+							\
+	__asm__ volatile(				\
+		"int	$0x80"				\
+		: "=a" (eax)	/* %eax */		\
+		: "a"(NUM),	/* %eax */		\
+		  "b"((ARG1)),	/* %ebx */		\
+		  "c"((ARG2)),	/* %ecx */		\
+		  "d"((ARG3)),	/* %edx */		\
+		  "S"((ARG4))	/* %esi */		\
+		: "memory"				\
+	);						\
+	eax;						\
+})
+
+#define __do_syscall5(NUM, ARG1, ARG2, ARG3, ARG4, ARG5) ({	\
+	intptr_t eax;						\
+								\
+	__asm__ volatile(					\
+		"int	$0x80"					\
+		: "=a" (eax)	/* %eax */			\
+		: "a"(NUM),	/* %eax */			\
+		  "b"((ARG1)),	/* %ebx */			\
+		  "c"((ARG2)),	/* %ecx */			\
+		  "d"((ARG3)),	/* %edx */			\
+		  "S"((ARG4)),	/* %esi */			\
+		  "D"((ARG5))	/* %edi */			\
+		: "memory"					\
+	);							\
+	eax;							\
+})
+
+
+/*
+ * On i386, the 6th argument of syscall goes in %ebp. However, both Clang
+ * and GCC cannot use %ebp in the clobber list and in the "r" constraint
+ * without using -fomit-frame-pointer. To make it always available for
+ * any kind of compilation, the below workaround is implemented:
+ *
+ *  1) Push the 6-th argument.
+ *  2) Push %ebp.
+ *  3) Load the 6-th argument from 4(%esp) to %ebp.
+ *  4) Do the syscall (int $0x80).
+ *  5) Pop %ebp (restore the old value of %ebp).
+ *  6) Add %esp by 4 (undo the stack pointer).
+ *
+ * WARNING:
+ *   Don't use register variables for __do_syscall6(), there is a known
+ *   GCC bug that results in an endless loop.
+ *
+ * BugLink: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105032
+ *
+ */
+#define __do_syscall6(NUM, ARG1, ARG2, ARG3, ARG4, ARG5, ARG6) ({	\
+	intptr_t eax  = (intptr_t)(NUM);				\
+	intptr_t arg6 = (intptr_t)(ARG6); /* Always in memory */	\
+	__asm__ volatile (						\
+		"pushl	%[_arg6]\n\t"					\
+		"pushl	%%ebp\n\t"					\
+		"movl	4(%%esp),%%ebp\n\t"				\
+		"int	$0x80\n\t"					\
+		"popl	%%ebp\n\t"					\
+		"addl	$4,%%esp"					\
+		: "+a"(eax)		/* %eax */			\
+		: "b"(ARG1),		/* %ebx */			\
+		  "c"(ARG2),		/* %ecx */			\
+		  "d"(ARG3),		/* %edx */			\
+		  "S"(ARG4),		/* %esi */			\
+		  "D"(ARG5),		/* %edi */			\
+		  [_arg6]"m"(arg6)	/* memory */			\
+		: "memory", "cc"					\
+	);								\
+	eax;								\
+})
+
+#include "../syscall-defs.h"
+
+#else /* #ifdef CONFIG_NOLIBC */
+
 #include "../generic/syscall.h"
 
+#endif /* #ifdef CONFIG_NOLIBC */
+
 #endif /* #if defined(__x86_64__) */
 
 #endif /* #ifndef LIBURING_ARCH_X86_SYSCALL_H */
-- 
Ammar Faizi


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH liburing 0/3] Add x86 32-bit support for the nolibc build
  2022-04-14 22:41 [PATCH liburing 0/3] Add x86 32-bit support for the nolibc build Ammar Faizi
                   ` (2 preceding siblings ...)
  2022-04-14 22:41 ` [PATCH liburing 3/3] arch/x86/syscall: Add x86 32-bit native syscall support Ammar Faizi
@ 2022-04-18  2:01 ` Jens Axboe
  2022-04-18 14:14   ` Ammar Faizi
  2022-04-18 15:24 ` Jens Axboe
  4 siblings, 1 reply; 8+ messages in thread
From: Jens Axboe @ 2022-04-18  2:01 UTC (permalink / raw)
  To: Ammar Faizi
  Cc: io-uring Mailing List, GNU/Weeb Mailing List, Pavel Begunkov,
	Alviro Iskandar Setiawan

On 4/14/22 4:41 PM, Ammar Faizi wrote:
> Hi,
> 
> This series adds nolibc support for x86 32-bit. There are 3 patches in
> this series:
> 
> 1) Use `__NR_mmap2` instead of `__NR_mmap` for x86 32-bit.
> 2) Provide `get_page_size()` function for x86 32-bit.
> 3) Add x86 32-bit native syscall support.
> 
> The most noticeable changes is the patch 3. Unlike x86-64, only
> use native syscall from the __do_syscall macros when CONFIG_NOLIBC is
> enabled for 32-bit build. The reason is because the libc syscall
> wrapper can do better in 32-bit. The libc syscall wrapper can dispatch
> the best syscall instruction that the environment is supported, there
> are at least two variants syscall instruction for x86 32-bit, they are:
> `int $0x80` and `sysenter`. The `int $0x80` instruction is always
> available, but `sysenter` is not, it relies on VDSO. liburing always
> uses `int $0x80` for syscall if it's compiled with CONFIG_NOLIBC,
> otherwise it uses whatever the libc provides.
> 
> Extra notes for __do_syscall6() macro:
> On i386, the 6th argument of syscall goes in %ebp. However, both Clang
> and GCC cannot use %ebp in the clobber list and in the "r" constraint
> without using -fomit-frame-pointer. To make it always available for
> any kind of compilation, the below workaround is implemented:
> 
>   1) Push the 6-th argument.
>   2) Push %ebp.
>   3) Load the 6-th argument from 4(%esp) to %ebp.
>   4) Do the syscall (int $0x80).
>   5) Pop %ebp (restore the old value of %ebp).
>   6) Add %esp by 4 (undo the stack pointer).
> 
> WARNING:
>   Don't use register variables for __do_syscall6(), there is a known
>   GCC bug that results in an endless loop.
> 
> BugLink: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105032
> 
> 
> ===== How is this tested? =====
> 
> This has been tested on x86-64 Linux (compiled with 32-bit bin support)
> with the following commands:
> 
>   sudo apt-get install gcc-i686-linux-gnu g++-i686-linux-gnu -y;
>   ./configure --cc=i686-linux-gnu-gcc --cxx=i686-linux-gnu-g++ --nolibc;
>   sudo make -j8 runtests;

Looks reasonable to me, even with the warts. I keep threatening to do a
2.2 release, and I do want to do that soon, so question is if we defer
this patchset until after that has happened?

I'm looking for a gauge of confidence on the series.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH liburing 0/3] Add x86 32-bit support for the nolibc build
  2022-04-18  2:01 ` [PATCH liburing 0/3] Add x86 32-bit support for the nolibc build Jens Axboe
@ 2022-04-18 14:14   ` Ammar Faizi
  2022-04-18 15:23     ` Jens Axboe
  0 siblings, 1 reply; 8+ messages in thread
From: Ammar Faizi @ 2022-04-18 14:14 UTC (permalink / raw)
  To: Jens Axboe
  Cc: io-uring Mailing List, GNU/Weeb Mailing List, Pavel Begunkov,
	Alviro Iskandar Setiawan

On 4/18/22 9:01 AM, Jens Axboe wrote:
> On 4/14/22 4:41 PM, Ammar Faizi wrote:
>> Hi,
>>
>> This series adds nolibc support for x86 32-bit. There are 3 patches in
>> this series:
>>
>> 1) Use `__NR_mmap2` instead of `__NR_mmap` for x86 32-bit.
>> 2) Provide `get_page_size()` function for x86 32-bit.
>> 3) Add x86 32-bit native syscall support.
>>
>> The most noticeable changes is the patch 3. Unlike x86-64, only
>> use native syscall from the __do_syscall macros when CONFIG_NOLIBC is
>> enabled for 32-bit build. The reason is because the libc syscall
>> wrapper can do better in 32-bit. The libc syscall wrapper can dispatch
>> the best syscall instruction that the environment is supported, there
>> are at least two variants syscall instruction for x86 32-bit, they are:
>> `int $0x80` and `sysenter`. The `int $0x80` instruction is always
>> available, but `sysenter` is not, it relies on VDSO. liburing always
>> uses `int $0x80` for syscall if it's compiled with CONFIG_NOLIBC,
>> otherwise it uses whatever the libc provides.
>>
>> Extra notes for __do_syscall6() macro:
>> On i386, the 6th argument of syscall goes in %ebp. However, both Clang
>> and GCC cannot use %ebp in the clobber list and in the "r" constraint
>> without using -fomit-frame-pointer. To make it always available for
>> any kind of compilation, the below workaround is implemented:
>>
>>    1) Push the 6-th argument.
>>    2) Push %ebp.
>>    3) Load the 6-th argument from 4(%esp) to %ebp.
>>    4) Do the syscall (int $0x80).
>>    5) Pop %ebp (restore the old value of %ebp).
>>    6) Add %esp by 4 (undo the stack pointer).
>>
>> WARNING:
>>    Don't use register variables for __do_syscall6(), there is a known
>>    GCC bug that results in an endless loop.
>>
>> BugLink: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105032
>>
>>
>> ===== How is this tested? =====
>>
>> This has been tested on x86-64 Linux (compiled with 32-bit bin support)
>> with the following commands:
>>
>>    sudo apt-get install gcc-i686-linux-gnu g++-i686-linux-gnu -y;
>>    ./configure --cc=i686-linux-gnu-gcc --cxx=i686-linux-gnu-g++ --nolibc;
>>    sudo make -j8 runtests;
> 
> Looks reasonable to me, even with the warts. I keep threatening to do a
> 2.2 release, and I do want to do that soon, so question is if we defer
> this patchset until after that has happened?
> 
> I'm looking for a gauge of confidence on the series.

I personally love not to defer this patchset. I understand that if we
were adding something like this to the Linux kernel, it's pretty sure
that it is not acceptable time. But liburing.

Several things that you may want to consider:

1) Previously, `--nolibc` build on x86 32-bit will throw a compile
    error, "Arch doesn't support building liburing without libc".
    After this patchset, it compiles just fine.

2) This series doesn't have any effect for x86 32-bit that uses libc,
    and that is what we do by default.

3) I believe x86 32-bit users are not that many. So having this one
    earlier gives sometime to get it mature without much chaos (if
    we ever found a bug).

    Not to say it's buggy. But younger code tend to be buggier. If we
    ever hit that bug due to this patchset, some of them may fallback
    to the libc build while waiting for the next stable liburing.

But anyway, I don't think it's that urgent seeing that we don't have
visible users that are actively using nolibc x86 32-bit. So if you
prefer to defer this, please defer it. What do you think?

-- 
Ammar Faizi

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH liburing 0/3] Add x86 32-bit support for the nolibc build
  2022-04-18 14:14   ` Ammar Faizi
@ 2022-04-18 15:23     ` Jens Axboe
  0 siblings, 0 replies; 8+ messages in thread
From: Jens Axboe @ 2022-04-18 15:23 UTC (permalink / raw)
  To: Ammar Faizi
  Cc: io-uring Mailing List, GNU/Weeb Mailing List, Pavel Begunkov,
	Alviro Iskandar Setiawan

On 4/18/22 8:14 AM, Ammar Faizi wrote:
> On 4/18/22 9:01 AM, Jens Axboe wrote:
>> On 4/14/22 4:41 PM, Ammar Faizi wrote:
>>> Hi,
>>>
>>> This series adds nolibc support for x86 32-bit. There are 3 patches in
>>> this series:
>>>
>>> 1) Use `__NR_mmap2` instead of `__NR_mmap` for x86 32-bit.
>>> 2) Provide `get_page_size()` function for x86 32-bit.
>>> 3) Add x86 32-bit native syscall support.
>>>
>>> The most noticeable changes is the patch 3. Unlike x86-64, only
>>> use native syscall from the __do_syscall macros when CONFIG_NOLIBC is
>>> enabled for 32-bit build. The reason is because the libc syscall
>>> wrapper can do better in 32-bit. The libc syscall wrapper can dispatch
>>> the best syscall instruction that the environment is supported, there
>>> are at least two variants syscall instruction for x86 32-bit, they are:
>>> `int $0x80` and `sysenter`. The `int $0x80` instruction is always
>>> available, but `sysenter` is not, it relies on VDSO. liburing always
>>> uses `int $0x80` for syscall if it's compiled with CONFIG_NOLIBC,
>>> otherwise it uses whatever the libc provides.
>>>
>>> Extra notes for __do_syscall6() macro:
>>> On i386, the 6th argument of syscall goes in %ebp. However, both Clang
>>> and GCC cannot use %ebp in the clobber list and in the "r" constraint
>>> without using -fomit-frame-pointer. To make it always available for
>>> any kind of compilation, the below workaround is implemented:
>>>
>>>    1) Push the 6-th argument.
>>>    2) Push %ebp.
>>>    3) Load the 6-th argument from 4(%esp) to %ebp.
>>>    4) Do the syscall (int $0x80).
>>>    5) Pop %ebp (restore the old value of %ebp).
>>>    6) Add %esp by 4 (undo the stack pointer).
>>>
>>> WARNING:
>>>    Don't use register variables for __do_syscall6(), there is a known
>>>    GCC bug that results in an endless loop.
>>>
>>> BugLink: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105032
>>>
>>>
>>> ===== How is this tested? =====
>>>
>>> This has been tested on x86-64 Linux (compiled with 32-bit bin support)
>>> with the following commands:
>>>
>>>    sudo apt-get install gcc-i686-linux-gnu g++-i686-linux-gnu -y;
>>>    ./configure --cc=i686-linux-gnu-gcc --cxx=i686-linux-gnu-g++ --nolibc;
>>>    sudo make -j8 runtests;
>>
>> Looks reasonable to me, even with the warts. I keep threatening to do a
>> 2.2 release, and I do want to do that soon, so question is if we defer
>> this patchset until after that has happened?
>>
>> I'm looking for a gauge of confidence on the series.
> 
> I personally love not to defer this patchset. I understand that if we
> were adding something like this to the Linux kernel, it's pretty sure
> that it is not acceptable time. But liburing.
> 
> Several things that you may want to consider:
> 
> 1) Previously, `--nolibc` build on x86 32-bit will throw a compile
>    error, "Arch doesn't support building liburing without libc".
>    After this patchset, it compiles just fine.
> 
> 2) This series doesn't have any effect for x86 32-bit that uses libc,
>    and that is what we do by default.
> 
> 3) I believe x86 32-bit users are not that many. So having this one
>    earlier gives sometime to get it mature without much chaos (if
>    we ever found a bug).
> 
>    Not to say it's buggy. But younger code tend to be buggier. If we
>    ever hit that bug due to this patchset, some of them may fallback
>    to the libc build while waiting for the next stable liburing.
> 
> But anyway, I don't think it's that urgent seeing that we don't have
> visible users that are actively using nolibc x86 32-bit. So if you
> prefer to defer this, please defer it. What do you think?

All good points, let's just get it done.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH liburing 0/3] Add x86 32-bit support for the nolibc build
  2022-04-14 22:41 [PATCH liburing 0/3] Add x86 32-bit support for the nolibc build Ammar Faizi
                   ` (3 preceding siblings ...)
  2022-04-18  2:01 ` [PATCH liburing 0/3] Add x86 32-bit support for the nolibc build Jens Axboe
@ 2022-04-18 15:24 ` Jens Axboe
  4 siblings, 0 replies; 8+ messages in thread
From: Jens Axboe @ 2022-04-18 15:24 UTC (permalink / raw)
  To: ammarfaizi2; +Cc: gwml, alviro.iskandar, asml.silence, io-uring

On Fri, 15 Apr 2022 05:41:37 +0700, Ammar Faizi wrote:
> This series adds nolibc support for x86 32-bit. There are 3 patches in
> this series:
> 
> 1) Use `__NR_mmap2` instead of `__NR_mmap` for x86 32-bit.
> 2) Provide `get_page_size()` function for x86 32-bit.
> 3) Add x86 32-bit native syscall support.
> 
> [...]

Applied, thanks!

[1/3] arch/syscall-defs: Use `__NR_mmap2` instead of `__NR_mmap` for x86 32-bit
      commit: 2afb268164ba99ebe720add3632b4051651296b6
[2/3] arch/x86/lib: Provide `get_page_size()` function for x86 32-bit
      commit: 48342e4c482349718eeecd34b3026d3d6aa78794
[3/3] arch/x86/syscall: Add x86 32-bit native syscall support
      commit: b7d8dd8bbf5b8550c8a0c1ed70431cd8050709f0

Best regards,
-- 
Jens Axboe



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-04-18 15:47 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-04-14 22:41 [PATCH liburing 0/3] Add x86 32-bit support for the nolibc build Ammar Faizi
2022-04-14 22:41 ` [PATCH liburing 1/3] arch/syscall-defs: Use `__NR_mmap2` instead of `__NR_mmap` for x86 32-bit Ammar Faizi
2022-04-14 22:41 ` [PATCH liburing 2/3] arch/x86/lib: Provide `get_page_size()` function " Ammar Faizi
2022-04-14 22:41 ` [PATCH liburing 3/3] arch/x86/syscall: Add x86 32-bit native syscall support Ammar Faizi
2022-04-18  2:01 ` [PATCH liburing 0/3] Add x86 32-bit support for the nolibc build Jens Axboe
2022-04-18 14:14   ` Ammar Faizi
2022-04-18 15:23     ` Jens Axboe
2022-04-18 15:24 ` Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox