[PATCH 0/3] Add LoongArch support to nolibc

[PATCH 0/3] Add LoongArch support to nolibc

[chris.chenfeiyang]

From: Feiyang Chen <chenfeiyang@loongson.cn>

Add statx() support to implement sys_stat().
Add support for LoongArch (32 and 64 bit) to nolibc.
It was tested on LoongArch64 only.

The latest QEMU support full-system emulation of a LoongArch64
machine, but for the moment we need to specify the firmware manually.
https://github.com/loongson/Firmware/tree/main/LoongArchVirtMachine

QEMU_ARGS_loongarch  = -M virt -bios edk2-loongarch64-code.fd ...

Feiyang Chen (3):
  nolibc: Add statx() support to implement sys_stat()
  nolibc: Add support for LoongArch
  selftests/nolibc: Add support for LoongArch

 tools/include/nolibc/arch-loongarch.h   | 223 ++++++++++++++++++++++++
 tools/include/nolibc/arch.h             |   2 +
 tools/include/nolibc/sys.h              |  36 +++-
 tools/include/nolibc/types.h            |  46 +++++
 tools/testing/selftests/nolibc/Makefile |  78 +++++----
 5 files changed, 345 insertions(+), 40 deletions(-)
 create mode 100644 tools/include/nolibc/arch-loongarch.h

-- 
2.39.0

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

[chris.chenfeiyang]

From: Feiyang Chen <chenfeiyang@loongson.cn>

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 <chenfeiyang@loongson.cn>
---
 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

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

[Arnd Bergmann]

On Tue, Feb 7, 2023, at 03:09, chris.chenfeiyang@gmail.com wrote:
> From: Feiyang Chen <chenfeiyang@loongson.cn>
>
> 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 <chenfeiyang@loongson.cn>

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

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

[Feiyang Chen]

On Tue, 7 Feb 2023 at 22:31, Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Tue, Feb 7, 2023, at 03:09, chris.chenfeiyang@gmail.com wrote:
> > From: Feiyang Chen <chenfeiyang@loongson.cn>
> >
> > 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 <chenfeiyang@loongson.cn>
>
> 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

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

[Willy Tarreau]

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 <arnd@arndb.de> 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

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

[Arnd Bergmann]

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 <arnd@arndb.de> 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

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

[Feiyang Chen]

On Wed, 8 Feb 2023 at 11:31, Willy Tarreau <w@1wt.eu> 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 <arnd@arndb.de> 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

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

[Arnd Bergmann]

On Wed, Feb 8, 2023, at 08:42, Feiyang Chen wrote:
> On Wed, 8 Feb 2023 at 11:31, Willy Tarreau <w@1wt.eu> 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

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

[Willy Tarreau]

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 <w@1wt.eu> 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

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

[Feiyang Chen]

On Wed, 8 Feb 2023 at 16:19, Willy Tarreau <w@1wt.eu> 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 <w@1wt.eu> 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

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

[Willy Tarreau]

On Wed, Feb 08, 2023 at 01:01:48PM +0000, David Laight wrote:
> From: Willy Tarreau
> > Sent: 08 February 2023 08:20
> ....
> > 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!
> 
> Does that really matter?
> Isn't 'nolibc' really just used for programs built in the
> kernel source tree to be release/run with the current kernel?

Not just that even if mostly related, and even in such a case
we'd rather maintain a low level of breakage when it doesn't
require any effort (and moving the ifdef __NR_statx up 20 lines
is perfectly within what I consider low effort).

> I also wonder if building a 'mini_libc.a' that the programs
> can be linked against might be easier than having to
> generate inline versions of everything?

It's another option but a different approach. There are pros and cons
to different approaches. For this, better not reinvent the wheel,
there's already klibc that does this well. A few of us do value the
simplicity of not having to pre-build anything before the test program,
especially when running tests that cover multiple architectures and/or
versions. I'm not saying it's a solution to everything at all, far from
this (and the level of coverage of that code should be self-explanatory
to remind anyone that it's not the goal). But when working on some small
test programs it's quite handy like this.

Regards,
Willy

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

[Willy Tarreau]

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

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

[chris.chenfeiyang]

From: Feiyang Chen <chenfeiyang@loongson.cn>

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

Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn>
---
 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

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

[Arnd Bergmann]

On Tue, Feb 7, 2023, at 03:09, chris.chenfeiyang@gmail.com wrote:
> From: Feiyang Chen <chenfeiyang@loongson.cn>
>
> Add support for LoongArch (32 and 64 bit) to nolibc.
>
> Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn>
> ---
> +
> +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

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

[chris.chenfeiyang]

From: Feiyang Chen <chenfeiyang@loongson.cn>

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

Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn>
---
 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

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

[Willy Tarreau]

On Tue, Feb 07, 2023 at 10:09:42AM +0800, chris.chenfeiyang@gmail.com wrote:
> From: Feiyang Chen <chenfeiyang@loongson.cn>
> 
> 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

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

[Paul E. McKenney]

On Wed, Feb 08, 2023 at 04:54:07AM +0100, Willy Tarreau wrote:
> On Tue, Feb 07, 2023 at 10:09:42AM +0800, chris.chenfeiyang@gmail.com wrote:
> > From: Feiyang Chen <chenfeiyang@loongson.cn>
> > 
> > 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

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

[Feiyang Chen]

On Wed, 8 Feb 2023 at 12:34, Paul E. McKenney <paulmck@kernel.org> wrote:
>
> On Wed, Feb 08, 2023 at 04:54:07AM +0100, Willy Tarreau wrote:
> > On Tue, Feb 07, 2023 at 10:09:42AM +0800, chris.chenfeiyang@gmail.com wrote:
> > > From: Feiyang Chen <chenfeiyang@loongson.cn>
> > >
> > > 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