LinuxLists.cc - [PATCH 0/3] Add LoongArch support to nolibc

[permalink] [raw]

Subject: [PATCH 1/3] nolibc: Add statx() support to implement sys_stat()

From: Feiyang Chen <[email protected]>

Neither __NR_newfstatat nor __NR_stat is defined on new architecture
like LoongArch, but we can use statx() to implement sys_stat().

Signed-off-by: Feiyang Chen <[email protected]>
---
tools/include/nolibc/sys.h | 36 +++++++++++++++++++++++++---
tools/include/nolibc/types.h | 46 ++++++++++++++++++++++++++++++++++++
2 files changed, 79 insertions(+), 3 deletions(-)

diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h
index b5f8cd35c03b..7ff9b401008a 100644
--- a/tools/include/nolibc/sys.h
+++ b/tools/include/nolibc/sys.h
@@ -1054,6 +1054,7 @@ pid_t setsid(void)
* Warning: the struct stat's layout is arch-dependent.
*/

+#if defined(__NR_newfstatat) || defined(__NR_stat)
static __attribute__((unused))
int sys_stat(const char *path, struct stat *buf)
{
@@ -1063,10 +1064,8 @@ int sys_stat(const char *path, struct stat *buf)
#ifdef __NR_newfstatat
/* only solution for arm64 */
ret = my_syscall4(__NR_newfstatat, AT_FDCWD, path, &stat, 0);
-#elif defined(__NR_stat)
+#else /* __NR_stat */
ret = my_syscall2(__NR_stat, path, &stat);
-#else
-#error Neither __NR_newfstatat nor __NR_stat defined, cannot implement sys_stat()
#endif
buf->st_dev = stat.st_dev;
buf->st_ino = stat.st_ino;
@@ -1083,6 +1082,37 @@ int sys_stat(const char *path, struct stat *buf)
buf->st_ctime = stat.st_ctime;
return ret;
}
+#elif defined(__NR_statx)
+/* only solution for loongarch */
+static __attribute__((unused))
+int sys_stat(const char *path, struct stat *buf)
+{
+ struct statx statx;
+ long ret;
+
+ ret = my_syscall5(__NR_statx, AT_FDCWD, path, 0, STATX_BASIC_STATS, &statx);
+ buf->st_dev = ((statx.stx_dev_minor & 0xff)
+ | (statx.stx_dev_major << 8)
+ | ((statx.stx_dev_minor & ~0xff) << 12));
+ buf->st_ino = statx.stx_ino;
+ buf->st_mode = statx.stx_mode;
+ buf->st_nlink = statx.stx_nlink;
+ buf->st_uid = statx.stx_uid;
+ buf->st_gid = statx.stx_gid;
+ buf->st_rdev = ((statx.stx_rdev_minor & 0xff)
+ | (statx.stx_rdev_major << 8) |
+ ((statx.stx_rdev_minor & ~0xff) << 12));
+ buf->st_size = statx.stx_size;
+ buf->st_blksize = statx.stx_blksize;
+ buf->st_blocks = statx.stx_blocks;
+ buf->st_atime = statx.stx_atime.tv_sec;
+ buf->st_mtime = statx.stx_mtime.tv_sec;
+ buf->st_ctime = statx.stx_ctime.tv_sec;
+ return ret;
+}
+#else
+#error None of __NR_newfstatat, __NR_stat, nor __NR_statx defined, cannot implement sys_stat()
+#endif

static __attribute__((unused))
int stat(const char *path, struct stat *buf)
diff --git a/tools/include/nolibc/types.h b/tools/include/nolibc/types.h
index fbbc0e68c001..9b244af1ec2c 100644
--- a/tools/include/nolibc/types.h
+++ b/tools/include/nolibc/types.h
@@ -193,6 +193,52 @@ struct stat {
time_t st_ctime; /* time of last status change */
};

+/* for statx() */
+#ifndef STATX_BASIC_STATS
+#define STATX_BASIC_STATS (0x000007ffU)
+#endif
+
+struct statx_timestamp {
+ __s64 tv_sec;
+ __u32 tv_nsec;
+ __s32 __reserved;
+};
+
+struct statx {
+ /* 0x00 */
+ __u32 stx_mask; /* What results were written [uncond] */
+ __u32 stx_blksize; /* Preferred general I/O size [uncond] */
+ __u64 stx_attributes; /* Flags conveying information about the file [uncond] */
+ /* 0x10 */
+ __u32 stx_nlink; /* Number of hard links */
+ __u32 stx_uid; /* User ID of owner */
+ __u32 stx_gid; /* Group ID of owner */
+ __u16 stx_mode; /* File mode */
+ __u16 __spare0[1];
+ /* 0x20 */
+ __u64 stx_ino; /* Inode number */
+ __u64 stx_size; /* File size */
+ __u64 stx_blocks; /* Number of 512-byte blocks allocated */
+ __u64 stx_attributes_mask; /* Mask to show what's supported in stx_attributes */
+ /* 0x40 */
+ struct statx_timestamp stx_atime; /* Last access time */
+ struct statx_timestamp stx_btime; /* File creation time */
+ struct statx_timestamp stx_ctime; /* Last attribute change time */
+ struct statx_timestamp stx_mtime; /* Last data modification time */
+ /* 0x80 */
+ __u32 stx_rdev_major; /* Device ID of special file [if bdev/cdev] */
+ __u32 stx_rdev_minor;
+ __u32 stx_dev_major; /* ID of device containing file [uncond] */
+ __u32 stx_dev_minor;
+ /* 0x90 */
+ __u64 stx_mnt_id;
+ __u32 stx_dio_mem_align; /* Memory buffer alignment for direct I/O */
+ __u32 stx_dio_offset_align; /* File offset alignment for direct I/O */
+ /* 0xa0 */
+ __u64 __spare3[12]; /* Spare space for future expansion */
+ /* 0x100 */
+};
+
/* WARNING, it only deals with the 4096 first majors and 256 first minors */
#define makedev(major, minor) ((dev_t)((((major) & 0xfff) << 8) | ((minor) & 0xff)))
#define major(dev) ((unsigned int)(((dev) >> 8) & 0xfff))
--
2.39.0

2023-02-07 02:10:04

[permalink] [raw]

Subject: [PATCH 2/3] nolibc: Add support for LoongArch

From: Feiyang Chen <[email protected]>

Add support for LoongArch (32 and 64 bit) to nolibc.

Signed-off-by: Feiyang Chen <[email protected]>
---
tools/include/nolibc/arch-loongarch.h | 223 ++++++++++++++++++++++++++
tools/include/nolibc/arch.h | 2 +
2 files changed, 225 insertions(+)
create mode 100644 tools/include/nolibc/arch-loongarch.h

diff --git a/tools/include/nolibc/arch-loongarch.h b/tools/include/nolibc/arch-loongarch.h
new file mode 100644
index 000000000000..610efe00915c
--- /dev/null
+++ b/tools/include/nolibc/arch-loongarch.h
@@ -0,0 +1,223 @@
+/* SPDX-License-Identifier: LGPL-2.1 OR MIT */
+/*
+ * LoongArch specific definitions for NOLIBC
+ * Copyright (C) 2023 Loongson Technology Corporation Limited
+ */
+
+#ifndef _NOLIBC_ARCH_LOONGARCH_H
+#define _NOLIBC_ARCH_LOONGARCH_H
+
+struct sys_stat_struct {
+ unsigned long st_dev; /* Device. */
+ unsigned long st_ino; /* File serial number. */
+ unsigned int st_mode; /* File mode. */
+ unsigned int st_nlink; /* Link count. */
+ unsigned int st_uid; /* User ID of the file's owner. */
+ unsigned int st_gid; /* Group ID of the file's group. */
+ unsigned long st_rdev; /* Device number, if device. */
+ unsigned long __pad1;
+ long st_size; /* Size of file, in bytes. */
+ int st_blksize; /* Optimal block size for I/O. */
+ int __pad2;
+ long st_blocks; /* Number 512-byte blocks allocated. */
+ long st_atime; /* Time of last access. */
+ unsigned long st_atime_nsec;
+ long st_mtime; /* Time of last modification. */
+ unsigned long st_mtime_nsec;
+ long st_ctime; /* Time of last status change. */
+ unsigned long st_ctime_nsec;
+ unsigned int __unused4;
+ unsigned int __unused5;
+};
+
+/* Syscalls for LoongArch :
+ * - stack is 16-byte aligned
+ * - syscall number is passed in a7
+ * - arguments are in a0, a1, a2, a3, a4, a5
+ * - the system call is performed by calling "syscall 0"
+ * - syscall return comes in a0
+ * - the arguments are cast to long and assigned into the target
+ * registers which are then simply passed as registers to the asm code,
+ * so that we don't have to experience issues with register constraints.
+ *
+ * On LoongArch, select() is not implemented so we have to use pselect6().
+ */
+#define __ARCH_WANT_SYS_PSELECT6
+
+#define my_syscall0(num) \
+({ \
+ register long _num __asm__ ("a7") = (num); \
+ register long _arg1 __asm__ ("a0"); \
+ \
+ __asm__ volatile ( \
+ "syscall 0\n" \
+ : "=r"(_arg1) \
+ : "r"(_num) \
+ : "memory", "$t0", "$t1", "$t2", "$t3", \
+ "$t4", "$t5", "$t6", "$t7", "$t8" \
+ ); \
+ _arg1; \
+})
+
+#define my_syscall1(num, arg1) \
+({ \
+ register long _num __asm__ ("a7") = (num); \
+ register long _arg1 __asm__ ("a0") = (long)(arg1); \
+ \
+ __asm__ volatile ( \
+ "syscall 0\n" \
+ : "+r"(_arg1) \
+ : "r"(_num) \
+ : "memory", "$t0", "$t1", "$t2", "$t3", \
+ "$t4", "$t5", "$t6", "$t7", "$t8" \
+ ); \
+ _arg1; \
+})
+
+#define my_syscall2(num, arg1, arg2) \
+({ \
+ register long _num __asm__ ("a7") = (num); \
+ register long _arg1 __asm__ ("a0") = (long)(arg1); \
+ register long _arg2 __asm__ ("a1") = (long)(arg2); \
+ \
+ __asm__ volatile ( \
+ "syscall 0\n" \
+ : "+r"(_arg1) \
+ : "r"(_arg2), \
+ "r"(_num) \
+ : "memory", "$t0", "$t1", "$t2", "$t3", \
+ "$t4", "$t5", "$t6", "$t7", "$t8" \
+ ); \
+ _arg1; \
+})
+
+#define my_syscall3(num, arg1, arg2, arg3) \
+({ \
+ register long _num __asm__ ("a7") = (num); \
+ register long _arg1 __asm__ ("a0") = (long)(arg1); \
+ register long _arg2 __asm__ ("a1") = (long)(arg2); \
+ register long _arg3 __asm__ ("a2") = (long)(arg3); \
+ \
+ __asm__ volatile ( \
+ "syscall 0\n" \
+ : "+r"(_arg1) \
+ : "r"(_arg2), "r"(_arg3), \
+ "r"(_num) \
+ : "memory", "$t0", "$t1", "$t2", "$t3", \
+ "$t4", "$t5", "$t6", "$t7", "$t8" \
+ ); \
+ _arg1; \
+})
+
+#define my_syscall4(num, arg1, arg2, arg3, arg4) \
+({ \
+ register long _num __asm__ ("a7") = (num); \
+ register long _arg1 __asm__ ("a0") = (long)(arg1); \
+ register long _arg2 __asm__ ("a1") = (long)(arg2); \
+ register long _arg3 __asm__ ("a2") = (long)(arg3); \
+ register long _arg4 __asm__ ("a3") = (long)(arg4); \
+ \
+ __asm__ volatile ( \
+ "syscall 0\n" \
+ : "+r"(_arg1) \
+ : "r"(_arg2), "r"(_arg3), "r"(_arg4), \
+ "r"(_num) \
+ : "memory", "$t0", "$t1", "$t2", "$t3", \
+ "$t4", "$t5", "$t6", "$t7", "$t8" \
+ ); \
+ _arg1; \
+})
+
+#define my_syscall5(num, arg1, arg2, arg3, arg4, arg5) \
+({ \
+ register long _num __asm__ ("a7") = (num); \
+ register long _arg1 __asm__ ("a0") = (long)(arg1); \
+ register long _arg2 __asm__ ("a1") = (long)(arg2); \
+ register long _arg3 __asm__ ("a2") = (long)(arg3); \
+ register long _arg4 __asm__ ("a3") = (long)(arg4); \
+ register long _arg5 __asm__ ("a4") = (long)(arg5); \
+ \
+ __asm__ volatile ( \
+ "syscall 0\n" \
+ : "+r"(_arg1) \
+ : "r"(_arg2), "r"(_arg3), "r"(_arg4), "r"(_arg5), \
+ "r"(_num) \
+ : "memory", "$t0", "$t1", "$t2", "$t3", \
+ "$t4", "$t5", "$t6", "$t7", "$t8" \
+ ); \
+ _arg1; \
+})
+
+#define my_syscall6(num, arg1, arg2, arg3, arg4, arg5, arg6) \
+({ \
+ register long _num __asm__ ("a7") = (num); \
+ register long _arg1 __asm__ ("a0") = (long)(arg1); \
+ register long _arg2 __asm__ ("a1") = (long)(arg2); \
+ register long _arg3 __asm__ ("a2") = (long)(arg3); \
+ register long _arg4 __asm__ ("a3") = (long)(arg4); \
+ register long _arg5 __asm__ ("a4") = (long)(arg5); \
+ register long _arg6 __asm__ ("a5") = (long)(arg6); \
+ \
+ __asm__ volatile ( \
+ "syscall 0\n" \
+ : "+r"(_arg1) \
+ : "r"(_arg2), "r"(_arg3), "r"(_arg4), "r"(_arg5), "r"(_arg6), \
+ "r"(_num) \
+ : "memory", "$t0", "$t1", "$t2", "$t3", \
+ "$t4", "$t5", "$t6", "$t7", "$t8" \
+ ); \
+ _arg1; \
+})
+
+char **environ __attribute__((weak));
+const unsigned long *_auxv __attribute__((weak));
+
+#if __loongarch_grlen == 32
+#define LONGLOG "2"
+#define SZREG "4"
+#define REG_L "ld.w"
+#define LONG_S "st.w"
+#define LONG_ADD "add.w"
+#define LONG_ADDI "addi.w"
+#define LONG_SLL "slli.w"
+#define LONG_BSTRINS "bstrins.w"
+#else // __loongarch_grlen == 64
+#define LONGLOG "3"
+#define SZREG "8"
+#define REG_L "ld.d"
+#define LONG_S "st.d"
+#define LONG_ADD "add.d"
+#define LONG_ADDI "addi.d"
+#define LONG_SLL "slli.d"
+#define LONG_BSTRINS "bstrins.d"
+#endif
+
+/* startup code */
+void __attribute__((weak,noreturn,optimize("omit-frame-pointer"))) _start(void)
+{
+ __asm__ volatile (
+ REG_L " $a0, $sp, 0\n" // argc (a0) was in the stack
+ LONG_ADDI " $a1, $sp, "SZREG"\n" // argv (a1) = sp + SZREG
+ LONG_SLL " $a2, $a0, "LONGLOG"\n" // envp (a2) = SZREG*argc ...
+ LONG_ADDI " $a2, $a2, "SZREG"\n" // + SZREG (skip null)
+ LONG_ADD " $a2, $a2, $a1\n" // + argv
+
+ "move $a3, $a2\n" // iterate a3 over envp to find auxv (after NULL)
+ "0:\n" // do {
+ REG_L " $a4, $a3, 0\n" // a4 = *a3;
+ LONG_ADDI " $a3, $a3, "SZREG"\n" // a3 += sizeof(void*);
+ "bne $a4, $zero, 0b\n" // } while (a4);
+ LONG_S " $a3, %[_auxv]\n" // store a3 into _auxv
+
+ LONG_S " $a2, %[environ]\n" // store envp(a2) into environ
+ LONG_BSTRINS " $sp, $zero, 3, 0\n" // sp must be 16-byte aligned
+ "bl main\n" // main() returns the status code, we'll exit with it.
+ "li.w $a7, 93\n" // NR_exit == 93
+ "syscall 0\n"
+ :
+ : [_auxv]"m"(_auxv), [environ]"m"(environ)
+ );
+ __builtin_unreachable();
+}
+
+#endif // _NOLIBC_ARCH_LOONGARCH_H
diff --git a/tools/include/nolibc/arch.h b/tools/include/nolibc/arch.h
index 78b067a4fa47..2d5386a8d6aa 100644
--- a/tools/include/nolibc/arch.h
+++ b/tools/include/nolibc/arch.h
@@ -29,6 +29,8 @@
#include "arch-riscv.h"
#elif defined(__s390x__)
#include "arch-s390.h"
+#elif defined(__loongarch__)
+#include "arch-loongarch.h"
#endif

#endif /* _NOLIBC_ARCH_H */
--
2.39.0

2023-02-07 02:10:07

[permalink] [raw]

Subject: [PATCH 3/3] selftests/nolibc: Add support for LoongArch

From: Feiyang Chen <[email protected]>

Add support for LoongArch (64 bit) to nolibc selftest.

Signed-off-by: Feiyang Chen <[email protected]>
---
tools/testing/selftests/nolibc/Makefile | 78 +++++++++++++------------
1 file changed, 41 insertions(+), 37 deletions(-)

diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
index 8fe61d3e3cce..ea2b82a3cd86 100644
--- a/tools/testing/selftests/nolibc/Makefile
+++ b/tools/testing/selftests/nolibc/Makefile
@@ -13,52 +13,56 @@ ARCH = $(SUBARCH)
endif

# kernel image names by architecture
-IMAGE_i386 = arch/x86/boot/bzImage
-IMAGE_x86_64 = arch/x86/boot/bzImage
-IMAGE_x86 = arch/x86/boot/bzImage
-IMAGE_arm64 = arch/arm64/boot/Image
-IMAGE_arm = arch/arm/boot/zImage
-IMAGE_mips = vmlinuz
-IMAGE_riscv = arch/riscv/boot/Image
-IMAGE_s390 = arch/s390/boot/bzImage
-IMAGE = $(IMAGE_$(ARCH))
-IMAGE_NAME = $(notdir $(IMAGE))
+IMAGE_i386 = arch/x86/boot/bzImage
+IMAGE_x86_64 = arch/x86/boot/bzImage
+IMAGE_x86 = arch/x86/boot/bzImage
+IMAGE_arm64 = arch/arm64/boot/Image
+IMAGE_arm = arch/arm/boot/zImage
+IMAGE_mips = vmlinuz
+IMAGE_riscv = arch/riscv/boot/Image
+IMAGE_s390 = arch/s390/boot/bzImage
+IMAGE_loongarch = arch/loongarch/boot/vmlinuz.efi
+IMAGE = $(IMAGE_$(ARCH))
+IMAGE_NAME = $(notdir $(IMAGE))

# default kernel configurations that appear to be usable
-DEFCONFIG_i386 = defconfig
-DEFCONFIG_x86_64 = defconfig
-DEFCONFIG_x86 = defconfig
-DEFCONFIG_arm64 = defconfig
-DEFCONFIG_arm = multi_v7_defconfig
-DEFCONFIG_mips = malta_defconfig
-DEFCONFIG_riscv = defconfig
-DEFCONFIG_s390 = defconfig
-DEFCONFIG = $(DEFCONFIG_$(ARCH))
+DEFCONFIG_i386 = defconfig
+DEFCONFIG_x86_64 = defconfig
+DEFCONFIG_x86 = defconfig
+DEFCONFIG_arm64 = defconfig
+DEFCONFIG_arm = multi_v7_defconfig
+DEFCONFIG_mips = malta_defconfig
+DEFCONFIG_riscv = defconfig
+DEFCONFIG_s390 = defconfig
+DEFCONFIG_loongarch = defconfig
+DEFCONFIG = $(DEFCONFIG_$(ARCH))

# optional tests to run (default = all)
TEST =

# QEMU_ARCH: arch names used by qemu
-QEMU_ARCH_i386 = i386
-QEMU_ARCH_x86_64 = x86_64
-QEMU_ARCH_x86 = x86_64
-QEMU_ARCH_arm64 = aarch64
-QEMU_ARCH_arm = arm
-QEMU_ARCH_mips = mipsel # works with malta_defconfig
-QEMU_ARCH_riscv = riscv64
-QEMU_ARCH_s390 = s390x
-QEMU_ARCH = $(QEMU_ARCH_$(ARCH))
+QEMU_ARCH_i386 = i386
+QEMU_ARCH_x86_64 = x86_64
+QEMU_ARCH_x86 = x86_64
+QEMU_ARCH_arm64 = aarch64
+QEMU_ARCH_arm = arm
+QEMU_ARCH_mips = mipsel # works with malta_defconfig
+QEMU_ARCH_riscv = riscv64
+QEMU_ARCH_s390 = s390x
+QEMU_ARCH_loongarch = loongarch64
+QEMU_ARCH = $(QEMU_ARCH_$(ARCH))

# QEMU_ARGS : some arch-specific args to pass to qemu
-QEMU_ARGS_i386 = -M pc -append "console=ttyS0,9600 i8042.noaux panic=-1 $(TEST:%=NOLIBC_TEST=%)"
-QEMU_ARGS_x86_64 = -M pc -append "console=ttyS0,9600 i8042.noaux panic=-1 $(TEST:%=NOLIBC_TEST=%)"
-QEMU_ARGS_x86 = -M pc -append "console=ttyS0,9600 i8042.noaux panic=-1 $(TEST:%=NOLIBC_TEST=%)"
-QEMU_ARGS_arm64 = -M virt -cpu cortex-a53 -append "panic=-1 $(TEST:%=NOLIBC_TEST=%)"
-QEMU_ARGS_arm = -M virt -append "panic=-1 $(TEST:%=NOLIBC_TEST=%)"
-QEMU_ARGS_mips = -M malta -append "panic=-1 $(TEST:%=NOLIBC_TEST=%)"
-QEMU_ARGS_riscv = -M virt -append "console=ttyS0 panic=-1 $(TEST:%=NOLIBC_TEST=%)"
-QEMU_ARGS_s390 = -M s390-ccw-virtio -m 1G -append "console=ttyS0 panic=-1 $(TEST:%=NOLIBC_TEST=%)"
-QEMU_ARGS = $(QEMU_ARGS_$(ARCH))
+QEMU_ARGS_i386 = -M pc -append "console=ttyS0,9600 i8042.noaux panic=-1 $(TEST:%=NOLIBC_TEST=%)"
+QEMU_ARGS_x86_64 = -M pc -append "console=ttyS0,9600 i8042.noaux panic=-1 $(TEST:%=NOLIBC_TEST=%)"
+QEMU_ARGS_x86 = -M pc -append "console=ttyS0,9600 i8042.noaux panic=-1 $(TEST:%=NOLIBC_TEST=%)"
+QEMU_ARGS_arm64 = -M virt -cpu cortex-a53 -append "panic=-1 $(TEST:%=NOLIBC_TEST=%)"
+QEMU_ARGS_arm = -M virt -append "panic=-1 $(TEST:%=NOLIBC_TEST=%)"
+QEMU_ARGS_mips = -M malta -append "panic=-1 $(TEST:%=NOLIBC_TEST=%)"
+QEMU_ARGS_riscv = -M virt -append "console=ttyS0 panic=-1 $(TEST:%=NOLIBC_TEST=%)"
+QEMU_ARGS_s390 = -M s390-ccw-virtio -m 1G -append "console=ttyS0 panic=-1 $(TEST:%=NOLIBC_TEST=%)"
+QEMU_ARGS_loongarch = -M virt -append "console=ttyS0,115200 panic=-1 $(TEST:%=NOLIBC_TEST=%)"
+QEMU_ARGS = $(QEMU_ARGS_$(ARCH))

# OUTPUT is only set when run from the main makefile, otherwise
# it defaults to this nolibc directory.
--
2.39.0

2023-02-07 14:26:38

[permalink] [raw]

Subject: Re: [PATCH 2/3] nolibc: Add support for LoongArch

On Tue, Feb 7, 2023, at 03:09, [email protected] wrote:
> From: Feiyang Chen <[email protected]>
>
> Add support for LoongArch (32 and 64 bit) to nolibc.
>
> Signed-off-by: Feiyang Chen <[email protected]>
> ---
> +
> +struct sys_stat_struct {
> + unsigned long st_dev; /* Device. */
> + unsigned long st_ino; /* File serial number. */
> + unsigned int st_mode; /* File mode. */
> + unsigned int st_nlink; /* Link count. */
> + unsigned int st_uid; /* User ID of the file's owner. */
> + unsigned int st_gid; /* Group ID of the file's group. */
> + unsigned long st_rdev; /* Device number, if device. */
> + unsigned long __pad1;
> + long st_size; /* Size of file, in bytes. */
> + int st_blksize; /* Optimal block size for I/O. */
> + int __pad2;
> + long st_blocks; /* Number 512-byte blocks allocated. */
> + long st_atime; /* Time of last access. */
> + unsigned long st_atime_nsec;
> + long st_mtime; /* Time of last modification. */
> + unsigned long st_mtime_nsec;
> + long st_ctime; /* Time of last status change. */
> + unsigned long st_ctime_nsec;
> + unsigned int __unused4;
> + unsigned int __unused5;
> +};

This definition should not be used after patch 1, I think you
should drop it.

Arnd

2023-02-07 14:42:35

[permalink] [raw]

Subject: Re: [PATCH 1/3] nolibc: Add statx() support to implement sys_stat()

On Tue, Feb 7, 2023, at 03:09, [email protected] wrote:
> From: Feiyang Chen <[email protected]>
>
> Neither __NR_newfstatat nor __NR_stat is defined on new architecture
> like LoongArch, but we can use statx() to implement sys_stat().
>
> Signed-off-by: Feiyang Chen <[email protected]>

This looks nice, it should also help on 32-bit architectures that
only have stat64 but not newfstatat or stat.

> +#if defined(__NR_newfstatat) || defined(__NR_stat)
> static __attribute__((unused))
> int sys_stat(const char *path, struct stat *buf)
...
> +#else
> +#error None of __NR_newfstatat, __NR_stat, nor __NR_statx defined,
> cannot implement sys_stat()
> +#endif

Given that all architectures implement statx the same way, I wonder
if we can't just kill off the old function here and always use statx.

That would also allow removing the architecture specific
sys_stat_struct definitions in all arch-*.h files.

> +struct statx_timestamp {
> + __s64 tv_sec;
> + __u32 tv_nsec;
> + __s32 __reserved;
> +};
> +
> +struct statx {
> + /* 0x00 */
> + __u32 stx_mask; /* What results were written [uncond] */
> + __u32 stx_blksize; /* Preferred general I/O size [uncond] */
> + __u64 stx_attributes; /* Flags conveying information about the file
> [uncond] */
> + /* 0x10 */
> + __u32 stx_nlink; /* Number of hard links */
> + __u32 stx_uid; /* User ID of owner */
> + __u32 stx_gid; /* Group ID of owner */
> + __u16 stx_mode; /* File mode */
> + __u16 __spare0[1];
> + /* 0x20 */
> + __u64 stx_ino; /* Inode number */
> + __u64 stx_size; /* File size */
> + __u64 stx_blocks; /* Number of 512-byte blocks allocated */
> + __u64 stx_attributes_mask; /* Mask to show what's supported in
> stx_attributes */
> + /* 0x40 */
> + struct statx_timestamp stx_atime; /* Last access time */
> + struct statx_timestamp stx_btime; /* File creation time */
> + struct statx_timestamp stx_ctime; /* Last attribute change time */
> + struct statx_timestamp stx_mtime; /* Last data modification time */
> + /* 0x80 */
> + __u32 stx_rdev_major; /* Device ID of special file [if bdev/cdev] */
> + __u32 stx_rdev_minor;
> + __u32 stx_dev_major; /* ID of device containing file [uncond] */
> + __u32 stx_dev_minor;
> + /* 0x90 */
> + __u64 stx_mnt_id;
> + __u32 stx_dio_mem_align; /* Memory buffer alignment for direct I/O */
> + __u32 stx_dio_offset_align; /* File offset alignment for direct I/O */
> + /* 0xa0 */
> + __u64 __spare3[12]; /* Spare space for future expansion */
> + /* 0x100 */
> +};

Can't we just #include <linux/stat.h> here to avoid having to maintain
a duplicate copy?

Arnd

2023-02-08 02:10:16

[permalink] [raw]

Subject: Re: [PATCH 1/3] nolibc: Add statx() support to implement sys_stat()

On Tue, 7 Feb 2023 at 22:31, Arnd Bergmann <[email protected]> wrote:
>
> On Tue, Feb 7, 2023, at 03:09, [email protected] wrote:
> > From: Feiyang Chen <[email protected]>
> >
> > Neither __NR_newfstatat nor __NR_stat is defined on new architecture
> > like LoongArch, but we can use statx() to implement sys_stat().
> >
> > Signed-off-by: Feiyang Chen <[email protected]>
>
> This looks nice, it should also help on 32-bit architectures that
> only have stat64 but not newfstatat or stat.
>
> > +#if defined(__NR_newfstatat) || defined(__NR_stat)
> > static __attribute__((unused))
> > int sys_stat(const char *path, struct stat *buf)
> ...
> > +#else
> > +#error None of __NR_newfstatat, __NR_stat, nor __NR_statx defined,
> > cannot implement sys_stat()
> > +#endif
>
> Given that all architectures implement statx the same way, I wonder
> if we can't just kill off the old function here and always use statx.
>
> That would also allow removing the architecture specific
> sys_stat_struct definitions in all arch-*.h files.
>

Hi, Arnd,

I'd really like to make all architectures use sys_statx() instead
of sys_stat(). I just fear we might get dragged into a long discussion.
Can I send a patch series to do this later?

> > +struct statx_timestamp {
> > + __s64 tv_sec;
> > + __u32 tv_nsec;
> > + __s32 __reserved;
> > +};
> > +
> > +struct statx {
> > + /* 0x00 */
> > + __u32 stx_mask; /* What results were written [uncond] */
> > + __u32 stx_blksize; /* Preferred general I/O size [uncond] */
> > + __u64 stx_attributes; /* Flags conveying information about the file
> > [uncond] */
> > + /* 0x10 */
> > + __u32 stx_nlink; /* Number of hard links */
> > + __u32 stx_uid; /* User ID of owner */
> > + __u32 stx_gid; /* Group ID of owner */
> > + __u16 stx_mode; /* File mode */
> > + __u16 __spare0[1];
> > + /* 0x20 */
> > + __u64 stx_ino; /* Inode number */
> > + __u64 stx_size; /* File size */
> > + __u64 stx_blocks; /* Number of 512-byte blocks allocated */
> > + __u64 stx_attributes_mask; /* Mask to show what's supported in
> > stx_attributes */
> > + /* 0x40 */
> > + struct statx_timestamp stx_atime; /* Last access time */
> > + struct statx_timestamp stx_btime; /* File creation time */
> > + struct statx_timestamp stx_ctime; /* Last attribute change time */
> > + struct statx_timestamp stx_mtime; /* Last data modification time */
> > + /* 0x80 */
> > + __u32 stx_rdev_major; /* Device ID of special file [if bdev/cdev] */
> > + __u32 stx_rdev_minor;
> > + __u32 stx_dev_major; /* ID of device containing file [uncond] */
> > + __u32 stx_dev_minor;
> > + /* 0x90 */
> > + __u64 stx_mnt_id;
> > + __u32 stx_dio_mem_align; /* Memory buffer alignment for direct I/O */
> > + __u32 stx_dio_offset_align; /* File offset alignment for direct I/O */
> > + /* 0xa0 */
> > + __u64 __spare3[12]; /* Spare space for future expansion */
> > + /* 0x100 */
> > +};
>
> Can't we just #include <linux/stat.h> here to avoid having to maintain
> a duplicate copy?
>

Yes, I will #include <linux/stat.h> here.

Thanks,
Feiyang

> Arnd

2023-02-08 03:32:28

[permalink] [raw]

Subject: Re: [PATCH 1/3] nolibc: Add statx() support to implement sys_stat()

Hi Feiyang,

Sorry for the delay.

On Wed, Feb 08, 2023 at 10:09:48AM +0800, Feiyang Chen wrote:
> On Tue, 7 Feb 2023 at 22:31, Arnd Bergmann <[email protected]> wrote:
(...)
> > Given that all architectures implement statx the same way, I wonder
> > if we can't just kill off the old function here and always use statx.
> >
> > That would also allow removing the architecture specific
> > sys_stat_struct definitions in all arch-*.h files.
> >
>
> Hi, Arnd,
>
> I'd really like to make all architectures use sys_statx() instead
> of sys_stat(). I just fear we might get dragged into a long discussion.
> Can I send a patch series to do this later?

I generally agree with the Arnd's points overall and I'm fine with the
rest of your series. On this specific point, I'm fine with your proposal,
let's just start with sys_statx() only on this arch, please add a comment
about this possibility in the commit message that brings statx(),
indicating that other archs are likely to benefit from it as well, and
let's see after this if we can migrate all archs to statx.

I'm having another comment below however:

> > > +struct statx_timestamp {
> > > + __s64 tv_sec;
> > > + __u32 tv_nsec;
> > > + __s32 __reserved;
> > > +};
> > > +
> > > +struct statx {
> > > + /* 0x00 */
> > > + __u32 stx_mask; /* What results were written [uncond] */
> > > + __u32 stx_blksize; /* Preferred general I/O size [uncond] */
> > > + __u64 stx_attributes; /* Flags conveying information about the file
(...)

For all these types exposed to userland that you have to define, I'd
prefer if we would avoid using kernel-inherited types like __u32, __u64
etc given that all other archs for now only use regular types. It's not
critical at all but I'd prefer that we stay consistent between all archs.
Also, based on the comments on the fields it seems to me that this file
was just copy-pasted from some uapi header which is not under the same
license, so that's another reason for just defining what is needed here
if you need to define it here. And of course, if including linux/stat.h
also works, that's by far the preferred solution which will also save
us from having to maintain a copy!

Thanks!
Willy

2023-02-08 03:54:35

[permalink] [raw]

Subject: Re: [PATCH 3/3] selftests/nolibc: Add support for LoongArch

On Tue, Feb 07, 2023 at 10:09:42AM +0800, [email protected] wrote:
> From: Feiyang Chen <[email protected]>
>
> Add support for LoongArch (64 bit) to nolibc selftest.

Please also mention in this one that the makefile was reindented with
no other change than adding entries for loongarch, this will save
those reading this patch later from checking that the rest was not
changed.

Thanks!
Willy

2023-02-08 04:34:47

by Paul E. McKenney

[permalink] [raw]

Subject: Re: [PATCH 3/3] selftests/nolibc: Add support for LoongArch

On Wed, Feb 08, 2023 at 04:54:07AM +0100, Willy Tarreau wrote:
> On Tue, Feb 07, 2023 at 10:09:42AM +0800, [email protected] wrote:
> > From: Feiyang Chen <[email protected]>
> >
> > Add support for LoongArch (64 bit) to nolibc selftest.
>
> Please also mention in this one that the makefile was reindented with
> no other change than adding entries for loongarch, this will save
> those reading this patch later from checking that the rest was not
> changed.

I like Willy's suggestion, but even better would be to put the whitespace
changes into one commit, and the code changes into another commit.
This makes it a lot easier for reviewers and future nolibc developers
to quickly and easily see exactly what changed.

And one of those future nolibc maintainers just might be you! ;-)

Thanx, Paul

2023-02-08 06:40:16

[permalink] [raw]

Subject: Re: [PATCH 3/3] selftests/nolibc: Add support for LoongArch

On Wed, 8 Feb 2023 at 12:34, Paul E. McKenney <[email protected]> wrote:
>
> On Wed, Feb 08, 2023 at 04:54:07AM +0100, Willy Tarreau wrote:
> > On Tue, Feb 07, 2023 at 10:09:42AM +0800, [email protected] wrote:
> > > From: Feiyang Chen <[email protected]>
> > >
> > > Add support for LoongArch (64 bit) to nolibc selftest.
> >
> > Please also mention in this one that the makefile was reindented with
> > no other change than adding entries for loongarch, this will save
> > those reading this patch later from checking that the rest was not
> > changed.
>
> I like Willy's suggestion, but even better would be to put the whitespace
> changes into one commit, and the code changes into another commit.
> This makes it a lot easier for reviewers and future nolibc developers
> to quickly and easily see exactly what changed.
>

Hi, Willy, Paul,

I get it now, thank you.

> And one of those future nolibc maintainers just might be you! ;-)
>
> Thanx, Paul

Thanks,
Feiyang

2023-02-08 07:30:14

[permalink] [raw]

Subject: Re: [PATCH 1/3] nolibc: Add statx() support to implement sys_stat()

On Wed, Feb 8, 2023, at 04:31, Willy Tarreau wrote:
> On Wed, Feb 08, 2023 at 10:09:48AM +0800, Feiyang Chen wrote:
>> On Tue, 7 Feb 2023 at 22:31, Arnd Bergmann <[email protected]> wrote:
> (...)
>> > Given that all architectures implement statx the same way, I wonder
>> > if we can't just kill off the old function here and always use statx.
>> >
>> > That would also allow removing the architecture specific
>> > sys_stat_struct definitions in all arch-*.h files.
>> >
>>
>> Hi, Arnd,
>>
>> I'd really like to make all architectures use sys_statx() instead
>> of sys_stat(). I just fear we might get dragged into a long discussion.
>> Can I send a patch series to do this later?
>
> I generally agree with the Arnd's points overall and I'm fine with the
> rest of your series. On this specific point, I'm fine with your proposal,
> let's just start with sys_statx() only on this arch, please add a comment
> about this possibility in the commit message that brings statx(),
> indicating that other archs are likely to benefit from it as well, and
> let's see after this if we can migrate all archs to statx.

Ok, makes sense. Just change the description then to note that this
will also fix riscv32 (which has only statx) as well as arc, hexagon,
nios2 and openrisc (which have statx and stat64 but not stat or newstat).

Arnd

2023-02-08 07:43:13

[permalink] [raw]

Subject: Re: [PATCH 1/3] nolibc: Add statx() support to implement sys_stat()

On Wed, 8 Feb 2023 at 11:31, Willy Tarreau <[email protected]> wrote:
>
> Hi Feiyang,
>
> Sorry for the delay.
>
> On Wed, Feb 08, 2023 at 10:09:48AM +0800, Feiyang Chen wrote:
> > On Tue, 7 Feb 2023 at 22:31, Arnd Bergmann <[email protected]> wrote:
> (...)
> > > Given that all architectures implement statx the same way, I wonder
> > > if we can't just kill off the old function here and always use statx.
> > >
> > > That would also allow removing the architecture specific
> > > sys_stat_struct definitions in all arch-*.h files.
> > >
> >
> > Hi, Arnd,
> >
> > I'd really like to make all architectures use sys_statx() instead
> > of sys_stat(). I just fear we might get dragged into a long discussion.
> > Can I send a patch series to do this later?
>
> I generally agree with the Arnd's points overall and I'm fine with the
> rest of your series. On this specific point, I'm fine with your proposal,
> let's just start with sys_statx() only on this arch, please add a comment
> about this possibility in the commit message that brings statx(),
> indicating that other archs are likely to benefit from it as well, and
> let's see after this if we can migrate all archs to statx.
>

Hi, Arnd, Willy,

We have a problem if we just start with sys_statx() only on this arch.
When struct stat is not defined, what should we do with stat() in the
nolibc selftest?

> I'm having another comment below however:
>
> > > > +struct statx_timestamp {
> > > > + __s64 tv_sec;
> > > > + __u32 tv_nsec;
> > > > + __s32 __reserved;
> > > > +};
> > > > +
> > > > +struct statx {
> > > > + /* 0x00 */
> > > > + __u32 stx_mask; /* What results were written [uncond] */
> > > > + __u32 stx_blksize; /* Preferred general I/O size [uncond] */
> > > > + __u64 stx_attributes; /* Flags conveying information about the file
> (...)
>
> For all these types exposed to userland that you have to define, I'd
> prefer if we would avoid using kernel-inherited types like __u32, __u64
> etc given that all other archs for now only use regular types. It's not
> critical at all but I'd prefer that we stay consistent between all archs.
> Also, based on the comments on the fields it seems to me that this file
> was just copy-pasted from some uapi header which is not under the same
> license, so that's another reason for just defining what is needed here
> if you need to define it here. And of course, if including linux/stat.h
> also works, that's by far the preferred solution which will also save
> us from having to maintain a copy!
>

I try to include linux/stat.h and it works.

Thanks,
Feiyang

> Thanks!
> Willy

2023-02-08 08:06:55

[permalink] [raw]

Subject: Re: [PATCH 1/3] nolibc: Add statx() support to implement sys_stat()

On Wed, Feb 8, 2023, at 08:42, Feiyang Chen wrote:
> On Wed, 8 Feb 2023 at 11:31, Willy Tarreau <[email protected]> wrote:
>>
>> I generally agree with the Arnd's points overall and I'm fine with the
>> rest of your series. On this specific point, I'm fine with your proposal,
>> let's just start with sys_statx() only on this arch, please add a comment
>> about this possibility in the commit message that brings statx(),
>> indicating that other archs are likely to benefit from it as well, and
>> let's see after this if we can migrate all archs to statx.
>>
>
> We have a problem if we just start with sys_statx() only on this arch.
> When struct stat is not defined, what should we do with stat() in the
> nolibc selftest?

To clarify: your proposed implementation of the stat() function that
fills the nolibc 'struct stat' based on information from 'struct statx'
is fine here. Just remove the 'struct sys_stat_struct' definition
loongarch but keep 'struct stat'.

Arnd

2023-02-08 08:08:12

[permalink] [raw]

Subject: Re: [PATCH 1/3] nolibc: Add statx() support to implement sys_stat()

On Wed, Feb 08, 2023 at 03:42:56PM +0800, Feiyang Chen wrote:
> Hi, Arnd, Willy,
>
> We have a problem if we just start with sys_statx() only on this arch.
> When struct stat is not defined, what should we do with stat() in the
> nolibc selftest?

You need to have a struct stat regardless of the syscalls you implement,
because it is what userland will see and use. Even if your implementation
of stat() is based on statx(), you'll need that struct stat that your
stat() function will feed. Right now it is defined in types.h.

As you can see in sys.h where sys_stat() is implemented, each arch's
specific syscall is used with a local sys_stat_struct that is specific
to that syscall, then the fields are stuffed into the userland-compatible
stat struct.

aarch64 is a good example, as its stat() implementation relies on
newfstatat() that uses a different one, but all fields are properly
mapped.

Just let me know if anything is not clear and if you need more help.

Thanks!
Willy

2023-02-08 08:19:56

[permalink] [raw]

Subject: Re: [PATCH 1/3] nolibc: Add statx() support to implement sys_stat()

On Wed, Feb 08, 2023 at 09:06:24AM +0100, Arnd Bergmann wrote:
> On Wed, Feb 8, 2023, at 08:42, Feiyang Chen wrote:
> > On Wed, 8 Feb 2023 at 11:31, Willy Tarreau <[email protected]> wrote:
> >>
> >> I generally agree with the Arnd's points overall and I'm fine with the
> >> rest of your series. On this specific point, I'm fine with your proposal,
> >> let's just start with sys_statx() only on this arch, please add a comment
> >> about this possibility in the commit message that brings statx(),
> >> indicating that other archs are likely to benefit from it as well, and
> >> let's see after this if we can migrate all archs to statx.
> >>
> >
> > We have a problem if we just start with sys_statx() only on this arch.
> > When struct stat is not defined, what should we do with stat() in the
> > nolibc selftest?
>
> To clarify: your proposed implementation of the stat() function that
> fills the nolibc 'struct stat' based on information from 'struct statx'
> is fine here. Just remove the 'struct sys_stat_struct' definition
> loongarch but keep 'struct stat'.

Ah I think now I understand the problem Feiyang is facing. Since "struct
stat" is just between libc and userland, there's the "sys_stat_struct"
that we're using to abstract the syscalls in sys_stat() and that is
compatible with each variant of the stat() syscall on each arch. Here
there's simply no stat() syscall so it's best not to try to abstract
this function at all since types will not match between stat and statx,
so it will be better to just implement it like this:

#if defined(__NR_statx)
static __attribute__((unused))
int sys_stat(const char *path, struct stat *buf)
{
struct statx statx;
long ret;

ret = statx(...);
buf->xxx = statx.xxx;
...
return ret;
}
#else ...
// keep the regular sys_stat() here
#endif

Also looking at the man page I see that statx() only appeared in 4.11,
and here we're targetting userland so I'd rather keep a bit of margin
when it comes to backwards compatibility, thus not dropping stat() and
friends too early when not necessary. However using statx() by default
when available sounds fine to me!

Cheers,
Willy

2023-02-08 09:23:17

[permalink] [raw]

Subject: Re: [PATCH 1/3] nolibc: Add statx() support to implement sys_stat()

On Wed, 8 Feb 2023 at 16:19, Willy Tarreau <[email protected]> wrote:
>
> On Wed, Feb 08, 2023 at 09:06:24AM +0100, Arnd Bergmann wrote:
> > On Wed, Feb 8, 2023, at 08:42, Feiyang Chen wrote:
> > > On Wed, 8 Feb 2023 at 11:31, Willy Tarreau <[email protected]> wrote:
> > >>
> > >> I generally agree with the Arnd's points overall and I'm fine with the
> > >> rest of your series. On this specific point, I'm fine with your proposal,
> > >> let's just start with sys_statx() only on this arch, please add a comment
> > >> about this possibility in the commit message that brings statx(),
> > >> indicating that other archs are likely to benefit from it as well, and
> > >> let's see after this if we can migrate all archs to statx.
> > >>
> > >
> > > We have a problem if we just start with sys_statx() only on this arch.
> > > When struct stat is not defined, what should we do with stat() in the
> > > nolibc selftest?
> >
> > To clarify: your proposed implementation of the stat() function that
> > fills the nolibc 'struct stat' based on information from 'struct statx'
> > is fine here. Just remove the 'struct sys_stat_struct' definition
> > loongarch but keep 'struct stat'.
>
> Ah I think now I understand the problem Feiyang is facing. Since "struct
> stat" is just between libc and userland, there's the "sys_stat_struct"
> that we're using to abstract the syscalls in sys_stat() and that is
> compatible with each variant of the stat() syscall on each arch. Here
> there's simply no stat() syscall so it's best not to try to abstract
> this function at all since types will not match between stat and statx,
> so it will be better to just implement it like this:
>
> #if defined(__NR_statx)
> static __attribute__((unused))
> int sys_stat(const char *path, struct stat *buf)
> {
> struct statx statx;
> long ret;
>
> ret = statx(...);
> buf->xxx = statx.xxx;
> ...
> return ret;
> }
> #else ...
> // keep the regular sys_stat() here
> #endif
>
> Also looking at the man page I see that statx() only appeared in 4.11,
> and here we're targetting userland so I'd rather keep a bit of margin
> when it comes to backwards compatibility, thus not dropping stat() and
> friends too early when not necessary. However using statx() by default
> when available sounds fine to me!
>

Hi, Arnd, Willy,

I think I get it now, thank you!

Thanks,
Feiyang

> Cheers,
> Willy

2023-02-08 14:08:03