2018-11-05 14:28:50

by David Abdurachmanov

[permalink] [raw]
Subject: [PATCH] riscv: add asm/unistd.h UAPI header

Marcin Juszkiewicz reported issues while generating syscall table for riscv
using 4.20-rc1. The patch refactors our unistd.h files to match some other
architectures.

- Add asm/unistd.h UAPI header, which has __ARCH_WANT_NEW_STAT
- Remove asm/syscalls.h UAPI header and merge to asm/unistd.h
- Adjust kernel asm/unistd.h

So now asm/unistd.h UAPI header should show all syscalls for riscv.

Before this, Makefile simply put `#include <asm-generic/unistd.h>` into
generated asm/unistd.h UAPI header thus user didn't see:

- __NR_riscv_flush_icache
- __NR_newfstatat
- __NR_fstat

which are supported by riscv kernel.

Signed-off-by: David Abdurachmanov <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Marcin Juszkiewicz <[email protected]>
Cc: Guenter Roeck <[email protected]>
Fixes: 67314ec7b025
---
arch/riscv/include/asm/unistd.h | 5 ++--
.../include/uapi/asm/{syscalls.h => unistd.h} | 24 +++++++++++++------
2 files changed, 19 insertions(+), 10 deletions(-)
rename arch/riscv/include/uapi/asm/{syscalls.h => unistd.h} (54%)

diff --git a/arch/riscv/include/asm/unistd.h b/arch/riscv/include/asm/unistd.h
index eff7aa9aa163..fef96f117b4d 100644
--- a/arch/riscv/include/asm/unistd.h
+++ b/arch/riscv/include/asm/unistd.h
@@ -13,10 +13,9 @@

/*
* There is explicitly no include guard here because this file is expected to
- * be included multiple times. See uapi/asm/syscalls.h for more info.
+ * be included multiple times.
*/

-#define __ARCH_WANT_NEW_STAT
#define __ARCH_WANT_SYS_CLONE
+
#include <uapi/asm/unistd.h>
-#include <uapi/asm/syscalls.h>
diff --git a/arch/riscv/include/uapi/asm/syscalls.h b/arch/riscv/include/uapi/asm/unistd.h
similarity index 54%
rename from arch/riscv/include/uapi/asm/syscalls.h
rename to arch/riscv/include/uapi/asm/unistd.h
index 206dc4b0f6ea..5545f498071d 100644
--- a/arch/riscv/include/uapi/asm/syscalls.h
+++ b/arch/riscv/include/uapi/asm/unistd.h
@@ -1,13 +1,23 @@
-/* SPDX-License-Identifier: GPL-2.0 */
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
/*
- * Copyright (C) 2017-2018 SiFive
+ * Copyright (C) 2018 David Abdurachmanov <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

-/*
- * There is explicitly no include guard here because this file is expected to
- * be included multiple times in order to define the syscall macros via
- * __SYSCALL.
- */
+#define __ARCH_WANT_NEW_STAT
+
+#include <asm-generic/unistd.h>

/*
* Allows the instruction cache to be flushed from userspace. Despite RISC-V
--
2.19.1



2018-11-05 14:45:45

by David Abdurachmanov

[permalink] [raw]
Subject: Re: [PATCH] riscv: add asm/unistd.h UAPI header

On Mon, Nov 5, 2018 at 3:26 PM David Abdurachmanov
<[email protected]> wrote:

[..]
> diff --git a/arch/riscv/include/uapi/asm/syscalls.h b/arch/riscv/include/uapi/asm/unistd.h
> similarity index 54%
> rename from arch/riscv/include/uapi/asm/syscalls.h
> rename to arch/riscv/include/uapi/asm/unistd.h
> index 206dc4b0f6ea..5545f498071d 100644
[..]

I forgot to pass --no-renames to git format-patch.
If you want I can resend it with --no-renames to make it
a bit more readable.

david

2018-11-05 16:05:20

by Marcin Juszkiewicz

[permalink] [raw]
Subject: Re: [PATCH] riscv: add asm/unistd.h UAPI header

W dniu 05.11.2018 o 15:26, David Abdurachmanov pisze:
> Marcin Juszkiewicz reported issues while generating syscall table for riscv
> using 4.20-rc1. The patch refactors our unistd.h files to match some other
> architectures.
>
> - Add asm/unistd.h UAPI header, which has __ARCH_WANT_NEW_STAT
> - Remove asm/syscalls.h UAPI header and merge to asm/unistd.h
> - Adjust kernel asm/unistd.h
>
> So now asm/unistd.h UAPI header should show all syscalls for riscv.
>
> Before this, Makefile simply put `#include <asm-generic/unistd.h>` into
> generated asm/unistd.h UAPI header thus user didn't see:
>
> - __NR_riscv_flush_icache
> - __NR_newfstatat
> - __NR_fstat
>
> which are supported by riscv kernel.

Tested-by: Marcin Juszkiewicz <[email protected]>

With patch applied I got proper syscall data:

fstat 80
newfstatat 79
riscv_flush_icache 259

2018-11-05 20:56:55

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] riscv: add asm/unistd.h UAPI header

On 11/5/18, David Abdurachmanov <[email protected]> wrote:
> Marcin Juszkiewicz reported issues while generating syscall table for riscv
> using 4.20-rc1. The patch refactors our unistd.h files to match some other
> architectures.
>
> - Add asm/unistd.h UAPI header, which has __ARCH_WANT_NEW_STAT
> - Remove asm/syscalls.h UAPI header and merge to asm/unistd.h
> - Adjust kernel asm/unistd.h
>
> So now asm/unistd.h UAPI header should show all syscalls for riscv.
>
> Before this, Makefile simply put `#include <asm-generic/unistd.h>` into
> generated asm/unistd.h UAPI header thus user didn't see:
>
> - __NR_riscv_flush_icache
> - __NR_newfstatat
> - __NR_fstat
>
> which are supported by riscv kernel.
>
> Signed-off-by: David Abdurachmanov <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: Marcin Juszkiewicz <[email protected]>
> Cc: Guenter Roeck <[email protected]>

Thanks for addressing this, your patch correctly fixes riscv64, and
I should have noticed the mistake when I originally merged the
broken patch.

However, looking closer I found another problem with the original
patch that your fix does not address:

__ARCH_WANT_NEW_STAT should only be set on 64-bit
architectures.

For a 32-bit architecture, we only want __ARCH_WANT_STAT64 if
any. For 64-bit architectures with compat mode, we still need to
set __ARCH_WANT_STAT64 from the non-uapi file so we get
the syscall implementation.

If we don't care about the riscv32 ABI changing yet, we can
decide to leave out __ARCH_WANT_STAT64 here, and require
glibc to implement it using statx() like any new architecture.
stat64 is not y2038 safe, and statx replaces it because of that.

> Fixes: 67314ec7b025

That line should be formatted as

Fixes: 67314ec7b025 ("RISC-V: Request newstat syscalls")

2018-11-07 00:20:17

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH] riscv: add asm/unistd.h UAPI header

On Mon, 05 Nov 2018 12:56:15 PST (-0800), Arnd Bergmann wrote:
> On 11/5/18, David Abdurachmanov <[email protected]> wrote:
>> Marcin Juszkiewicz reported issues while generating syscall table for riscv
>> using 4.20-rc1. The patch refactors our unistd.h files to match some other
>> architectures.
>>
>> - Add asm/unistd.h UAPI header, which has __ARCH_WANT_NEW_STAT
>> - Remove asm/syscalls.h UAPI header and merge to asm/unistd.h
>> - Adjust kernel asm/unistd.h
>>
>> So now asm/unistd.h UAPI header should show all syscalls for riscv.
>>
>> Before this, Makefile simply put `#include <asm-generic/unistd.h>` into
>> generated asm/unistd.h UAPI header thus user didn't see:
>>
>> - __NR_riscv_flush_icache
>> - __NR_newfstatat
>> - __NR_fstat
>>
>> which are supported by riscv kernel.
>>
>> Signed-off-by: David Abdurachmanov <[email protected]>
>> Cc: Arnd Bergmann <[email protected]>
>> Cc: Marcin Juszkiewicz <[email protected]>
>> Cc: Guenter Roeck <[email protected]>
>
> Thanks for addressing this, your patch correctly fixes riscv64, and
> I should have noticed the mistake when I originally merged the
> broken patch.
>
> However, looking closer I found another problem with the original
> patch that your fix does not address:
>
> __ARCH_WANT_NEW_STAT should only be set on 64-bit
> architectures.
>
> For a 32-bit architecture, we only want __ARCH_WANT_STAT64 if
> any. For 64-bit architectures with compat mode, we still need to
> set __ARCH_WANT_STAT64 from the non-uapi file so we get
> the syscall implementation.
>
> If we don't care about the riscv32 ABI changing yet, we can
> decide to leave out __ARCH_WANT_STAT64 here, and require
> glibc to implement it using statx() like any new architecture.
> stat64 is not y2038 safe, and statx replaces it because of that.

Thanks for pointing this out. A while ago we decided the rv32 ABI was
"slushy": it can change if it has a good reason to. Right now the only planned
changes are the y2038 changes, which I consider this a part of. For some
reason I thought we'd already done this, but since we haven't then I think it
should go in sooner rather than later -- that will help the glibc guys get
everything lined up.

The target is still the next glibc release (Feb 1st) for a stable RV32I ABI.
That's progressing well, with one last blocking issue related to some of our
floating-point emulation routines before we can submit the port. This should
give us ample time to line up the ABIs correctly so everything works.

So I think the correct answer here is to drop __ARCH_WANT_STAT64 from RISC-V.

>> Fixes: 67314ec7b025
>
> That line should be formatted as
>
> Fixes: 67314ec7b025 ("RISC-V: Request newstat syscalls")

Yep, and I have

[pretty]
fixes = Fixes: %h (\"%s\")

to make that slightly easier for me to remember :).

2018-11-07 18:32:35

by David Abdurachmanov

[permalink] [raw]
Subject: Re: [PATCH] riscv: add asm/unistd.h UAPI header

On Wed, Nov 7, 2018 at 1:08 AM Palmer Dabbelt <[email protected]> wrote:
>
> On Mon, 05 Nov 2018 12:56:15 PST (-0800), Arnd Bergmann wrote:
> > On 11/5/18, David Abdurachmanov <[email protected]> wrote:
> >> Marcin Juszkiewicz reported issues while generating syscall table for riscv
> >> using 4.20-rc1. The patch refactors our unistd.h files to match some other
> >> architectures.
> >>
> >> - Add asm/unistd.h UAPI header, which has __ARCH_WANT_NEW_STAT
> >> - Remove asm/syscalls.h UAPI header and merge to asm/unistd.h
> >> - Adjust kernel asm/unistd.h
> >>
> >> So now asm/unistd.h UAPI header should show all syscalls for riscv.
> >>
> >> Before this, Makefile simply put `#include <asm-generic/unistd.h>` into
> >> generated asm/unistd.h UAPI header thus user didn't see:
> >>
> >> - __NR_riscv_flush_icache
> >> - __NR_newfstatat
> >> - __NR_fstat
> >>
> >> which are supported by riscv kernel.
> >>
> >> Signed-off-by: David Abdurachmanov <[email protected]>
> >> Cc: Arnd Bergmann <[email protected]>
> >> Cc: Marcin Juszkiewicz <[email protected]>
> >> Cc: Guenter Roeck <[email protected]>
> >
> > Thanks for addressing this, your patch correctly fixes riscv64, and
> > I should have noticed the mistake when I originally merged the
> > broken patch.
> >
> > However, looking closer I found another problem with the original
> > patch that your fix does not address:
> >
> > __ARCH_WANT_NEW_STAT should only be set on 64-bit
> > architectures.
> >
> > For a 32-bit architecture, we only want __ARCH_WANT_STAT64 if
> > any. For 64-bit architectures with compat mode, we still need to
> > set __ARCH_WANT_STAT64 from the non-uapi file so we get
> > the syscall implementation.
> >
> > If we don't care about the riscv32 ABI changing yet, we can
> > decide to leave out __ARCH_WANT_STAT64 here, and require
> > glibc to implement it using statx() like any new architecture.
> > stat64 is not y2038 safe, and statx replaces it because of that.
>
> Thanks for pointing this out. A while ago we decided the rv32 ABI was
> "slushy": it can change if it has a good reason to. Right now the only planned
> changes are the y2038 changes, which I consider this a part of. For some
> reason I thought we'd already done this, but since we haven't then I think it
> should go in sooner rather than later -- that will help the glibc guys get
> everything lined up.
>
> The target is still the next glibc release (Feb 1st) for a stable RV32I ABI.
> That's progressing well, with one last blocking issue related to some of our
> floating-point emulation routines before we can submit the port. This should
> give us ample time to line up the ABIs correctly so everything works.
>
> So I think the correct answer here is to drop __ARCH_WANT_STAT64 from RISC-V.
>

Then if you agree I could do and send v2:

+#ifdef __LP64__
+#define __ARCH_WANT_NEW_STAT
+#endif /* __LP64__ */

Cannot use CONFIG_64BIT as in user space nothing defines it.
Alternatively I could
check for __riscv_xlen == 64.

I found _LP64 and __LP64__ being used in kernel, incl. include/uapi/linux/rseq.h

david

2018-11-07 21:12:36

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] riscv: add asm/unistd.h UAPI header

On Wed, Nov 7, 2018 at 7:30 PM David Abdurachmanov
<[email protected]> wrote:
> On Wed, Nov 7, 2018 at 1:08 AM Palmer Dabbelt <[email protected]> wrote:
> > On Mon, 05 Nov 2018 12:56:15 PST (-0800), Arnd Bergmann wrote:

> > The target is still the next glibc release (Feb 1st) for a stable RV32I ABI.
> > That's progressing well, with one last blocking issue related to some of our
> > floating-point emulation routines before we can submit the port. This should
> > give us ample time to line up the ABIs correctly so everything works.
> >
> > So I think the correct answer here is to drop __ARCH_WANT_STAT64 from RISC-V.
> >
>
> Then if you agree I could do and send v2:
>
> +#ifdef __LP64__
> +#define __ARCH_WANT_NEW_STAT
> +#endif /* __LP64__ */

Looks good to me.

> Cannot use CONFIG_64BIT as in user space nothing defines it.
> Alternatively I could
> check for __riscv_xlen == 64.
>
> I found _LP64 and __LP64__ being used in kernel, incl. include/uapi/linux/rseq.h

I think older gcc versions were less consistent about those macros, which
is why I introduced __BITS_PER_LONG a long time ago. We now support
gcc-4.6 as the minimum, so using __LP64__ is reliable, and on RISC-V
we obviously have a much newer compiler anyway.

Arnd

2018-11-08 02:10:47

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH] riscv: add asm/unistd.h UAPI header

On Wed, 07 Nov 2018 13:09:39 PST (-0800), Arnd Bergmann wrote:
> On Wed, Nov 7, 2018 at 7:30 PM David Abdurachmanov
> <[email protected]> wrote:
>> On Wed, Nov 7, 2018 at 1:08 AM Palmer Dabbelt <[email protected]> wrote:
>> > On Mon, 05 Nov 2018 12:56:15 PST (-0800), Arnd Bergmann wrote:
>
>> > The target is still the next glibc release (Feb 1st) for a stable RV32I ABI.
>> > That's progressing well, with one last blocking issue related to some of our
>> > floating-point emulation routines before we can submit the port. This should
>> > give us ample time to line up the ABIs correctly so everything works.
>> >
>> > So I think the correct answer here is to drop __ARCH_WANT_STAT64 from RISC-V.
>> >
>>
>> Then if you agree I could do and send v2:
>>
>> +#ifdef __LP64__
>> +#define __ARCH_WANT_NEW_STAT
>> +#endif /* __LP64__ */
>
> Looks good to me.

This is a bit pedantic, but I'm not sure what the right answer is here:
"-march=rv64gc -mabi=ilp32d" will not define __LP64__, but will define
"__riscv_xlen == 64". I actually don't know enough about how an rv64gc/ilp32d
ABI would work to answer this: would we have "long long" all over our syscalls?

Probably not worth worrying about for now, as we'll have to go audit all of
these if we ever end up with an ilp32 ABI. So just go for it and we'll throw
this on the pile to deal with later :)

2018-11-08 10:31:11

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] riscv: add asm/unistd.h UAPI header

On Thu, Nov 8, 2018 at 3:10 AM Palmer Dabbelt <[email protected]> wrote:
>
> On Wed, 07 Nov 2018 13:09:39 PST (-0800), Arnd Bergmann wrote:
> > On Wed, Nov 7, 2018 at 7:30 PM David Abdurachmanov
> > <[email protected]> wrote:
> >> On Wed, Nov 7, 2018 at 1:08 AM Palmer Dabbelt <[email protected]> wrote:
> >> > On Mon, 05 Nov 2018 12:56:15 PST (-0800), Arnd Bergmann wrote:
> >
> >> > The target is still the next glibc release (Feb 1st) for a stable RV32I ABI.
> >> > That's progressing well, with one last blocking issue related to some of our
> >> > floating-point emulation routines before we can submit the port. This should
> >> > give us ample time to line up the ABIs correctly so everything works.
> >> >
> >> > So I think the correct answer here is to drop __ARCH_WANT_STAT64 from RISC-V.
> >> >
> >>
> >> Then if you agree I could do and send v2:
> >>
> >> +#ifdef __LP64__
> >> +#define __ARCH_WANT_NEW_STAT
> >> +#endif /* __LP64__ */
> >
> > Looks good to me.
>
> This is a bit pedantic, but I'm not sure what the right answer is here:
> "-march=rv64gc -mabi=ilp32d" will not define __LP64__, but will define
> "__riscv_xlen == 64". I actually don't know enough about how an rv64gc/ilp32d
> ABI would work to answer this: would we have "long long" all over our syscalls?
>
> Probably not worth worrying about for now, as we'll have to go audit all of
> these if we ever end up with an ilp32 ABI. So just go for it and we'll throw
> this on the pile to deal with later :)

Short answer: it doesn't matter because an ilp32d ABI would use neither
newstat nor stat64, it would only need statx().

Long answer: We've gone through multiple iterations on the question.
x86 uses long long in syscall interfaces and tries to reuse the native
64-bit syscalls as much as possible. This turned out to cause endless
problems, so for the (never merged but still kept around as a patchset)
arm64 ABI, we went the opposite way, and made the syscalls use the
same ABI as the arm32 mode.

From the experience with both of the above, I'd say if you end up
having to do it, use the same method as arm64, but try to resist
doing it at all. Unlike arm64 and x86-64, there is no inherent benefit
to using the 64-bit instruction set (doubled register number etc),
so compared to the normal lp64 ABI you only gain a little dcache
space for the smaller pointers at the cost of a smaller address
space. For you as a maintainer however, the cost of supporting this
mode is that you are stuck with three user space ABIs instead of
just two (normal 32-bit and 64-bit).
If anyone really wants to run 32-bit code, they need a CPU that
allows switching modes.

Arnd

2018-11-08 10:40:38

by David Abdurachmanov

[permalink] [raw]
Subject: Re: [PATCH] riscv: add asm/unistd.h UAPI header

On Thu, Nov 8, 2018 at 3:10 AM Palmer Dabbelt <[email protected]> wrote:
>
> On Wed, 07 Nov 2018 13:09:39 PST (-0800), Arnd Bergmann wrote:
> > On Wed, Nov 7, 2018 at 7:30 PM David Abdurachmanov
> > <[email protected]> wrote:
> >> On Wed, Nov 7, 2018 at 1:08 AM Palmer Dabbelt <[email protected]> wrote:
> >> > On Mon, 05 Nov 2018 12:56:15 PST (-0800), Arnd Bergmann wrote:
> >
> >> > The target is still the next glibc release (Feb 1st) for a stable RV32I ABI.
> >> > That's progressing well, with one last blocking issue related to some of our
> >> > floating-point emulation routines before we can submit the port. This should
> >> > give us ample time to line up the ABIs correctly so everything works.
> >> >
> >> > So I think the correct answer here is to drop __ARCH_WANT_STAT64 from RISC-V.
> >> >
> >>
> >> Then if you agree I could do and send v2:
> >>
> >> +#ifdef __LP64__
> >> +#define __ARCH_WANT_NEW_STAT
> >> +#endif /* __LP64__ */
> >
> > Looks good to me.
>
> This is a bit pedantic, but I'm not sure what the right answer is here:
> "-march=rv64gc -mabi=ilp32d" will not define __LP64__, but will define
> "__riscv_xlen == 64". I actually don't know enough about how an rv64gc/ilp32d
> ABI would work to answer this: would we have "long long" all over our syscalls?
>
> Probably not worth worrying about for now, as we'll have to go audit all of
> these if we ever end up with an ilp32 ABI. So just go for it and we'll throw
> this on the pile to deal with later :)

GCC will not allow "-march=rv64gc -mabi=ilp32d":

cc1: error: ABI requires -march=rv32

I see that arch/riscv/include/uapi/asm/elf.h already use __riscv_xlen so to be
consistent I will use it too (but I like __LP64__ more as it is well
known macro).

Looking at other UAPI headers I see that include/uapi/linux/rseq.h is using
__LP64__ macro. This header is installed on riscv.

2018-11-08 16:58:19

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH] riscv: add asm/unistd.h UAPI header

On Thu, 08 Nov 2018 02:30:02 PST (-0800), Arnd Bergmann wrote:
> On Thu, Nov 8, 2018 at 3:10 AM Palmer Dabbelt <[email protected]> wrote:
>>
>> On Wed, 07 Nov 2018 13:09:39 PST (-0800), Arnd Bergmann wrote:
>> > On Wed, Nov 7, 2018 at 7:30 PM David Abdurachmanov
>> > <[email protected]> wrote:
>> >> On Wed, Nov 7, 2018 at 1:08 AM Palmer Dabbelt <[email protected]> wrote:
>> >> > On Mon, 05 Nov 2018 12:56:15 PST (-0800), Arnd Bergmann wrote:
>> >
>> >> > The target is still the next glibc release (Feb 1st) for a stable RV32I ABI.
>> >> > That's progressing well, with one last blocking issue related to some of our
>> >> > floating-point emulation routines before we can submit the port. This should
>> >> > give us ample time to line up the ABIs correctly so everything works.
>> >> >
>> >> > So I think the correct answer here is to drop __ARCH_WANT_STAT64 from RISC-V.
>> >> >
>> >>
>> >> Then if you agree I could do and send v2:
>> >>
>> >> +#ifdef __LP64__
>> >> +#define __ARCH_WANT_NEW_STAT
>> >> +#endif /* __LP64__ */
>> >
>> > Looks good to me.
>>
>> This is a bit pedantic, but I'm not sure what the right answer is here:
>> "-march=rv64gc -mabi=ilp32d" will not define __LP64__, but will define
>> "__riscv_xlen == 64". I actually don't know enough about how an rv64gc/ilp32d
>> ABI would work to answer this: would we have "long long" all over our syscalls?
>>
>> Probably not worth worrying about for now, as we'll have to go audit all of
>> these if we ever end up with an ilp32 ABI. So just go for it and we'll throw
>> this on the pile to deal with later :)
>
> Short answer: it doesn't matter because an ilp32d ABI would use neither
> newstat nor stat64, it would only need statx().
>
> Long answer: We've gone through multiple iterations on the question.
> x86 uses long long in syscall interfaces and tries to reuse the native
> 64-bit syscalls as much as possible. This turned out to cause endless
> problems, so for the (never merged but still kept around as a patchset)
> arm64 ABI, we went the opposite way, and made the syscalls use the
> same ABI as the arm32 mode.
>
> From the experience with both of the above, I'd say if you end up
> having to do it, use the same method as arm64, but try to resist
> doing it at all. Unlike arm64 and x86-64, there is no inherent benefit
> to using the 64-bit instruction set (doubled register number etc),
> so compared to the normal lp64 ABI you only gain a little dcache
> space for the smaller pointers at the cost of a smaller address
> space. For you as a maintainer however, the cost of supporting this
> mode is that you are stuck with three user space ABIs instead of
> just two (normal 32-bit and 64-bit).
> If anyone really wants to run 32-bit code, they need a CPU that
> allows switching modes.

Thanks!

2018-11-08 16:59:39

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH] riscv: add asm/unistd.h UAPI header

On Thu, 08 Nov 2018 02:38:22 PST (-0800), [email protected] wrote:
> On Thu, Nov 8, 2018 at 3:10 AM Palmer Dabbelt <[email protected]> wrote:
>>
>> On Wed, 07 Nov 2018 13:09:39 PST (-0800), Arnd Bergmann wrote:
>> > On Wed, Nov 7, 2018 at 7:30 PM David Abdurachmanov
>> > <[email protected]> wrote:
>> >> On Wed, Nov 7, 2018 at 1:08 AM Palmer Dabbelt <[email protected]> wrote:
>> >> > On Mon, 05 Nov 2018 12:56:15 PST (-0800), Arnd Bergmann wrote:
>> >
>> >> > The target is still the next glibc release (Feb 1st) for a stable RV32I ABI.
>> >> > That's progressing well, with one last blocking issue related to some of our
>> >> > floating-point emulation routines before we can submit the port. This should
>> >> > give us ample time to line up the ABIs correctly so everything works.
>> >> >
>> >> > So I think the correct answer here is to drop __ARCH_WANT_STAT64 from RISC-V.
>> >> >
>> >>
>> >> Then if you agree I could do and send v2:
>> >>
>> >> +#ifdef __LP64__
>> >> +#define __ARCH_WANT_NEW_STAT
>> >> +#endif /* __LP64__ */
>> >
>> > Looks good to me.
>>
>> This is a bit pedantic, but I'm not sure what the right answer is here:
>> "-march=rv64gc -mabi=ilp32d" will not define __LP64__, but will define
>> "__riscv_xlen == 64". I actually don't know enough about how an rv64gc/ilp32d
>> ABI would work to answer this: would we have "long long" all over our syscalls?
>>
>> Probably not worth worrying about for now, as we'll have to go audit all of
>> these if we ever end up with an ilp32 ABI. So just go for it and we'll throw
>> this on the pile to deal with later :)
>
> GCC will not allow "-march=rv64gc -mabi=ilp32d":
>
> cc1: error: ABI requires -march=rv32
>
> I see that arch/riscv/include/uapi/asm/elf.h already use __riscv_xlen so to be
> consistent I will use it too (but I like __LP64__ more as it is well
> known macro).
>
> Looking at other UAPI headers I see that include/uapi/linux/rseq.h is using
> __LP64__ macro. This header is installed on riscv.

Yes, it's not currently supported and there are no concrete plans to support
it. Like Arnd mentioned, it's a big headache. It was really more of a
question about how this might work than a concrete review, I'm happy with the
patch as it was suggested.