Linux-RISC-V Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] riscv: add asm/unistd.h UAPI header
@ 2018-11-05 14:26 david.abdurachmanov
  2018-11-05 14:26 ` David Abdurachmanov
                   ` (3 more replies)
  0 siblings, 4 replies; 24+ messages in thread
From: david.abdurachmanov @ 2018-11-05 14:26 UTC (permalink / raw)
  To: linux-riscv

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 <david.abdurachmanov@gmail.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
Cc: Guenter Roeck <linux@roeck-us.net>
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 <david.abdurachmanov@gmail.com>
+ *
+ * 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

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

* [PATCH] riscv: add asm/unistd.h UAPI header
  2018-11-05 14:26 [PATCH] riscv: add asm/unistd.h UAPI header david.abdurachmanov
@ 2018-11-05 14:26 ` David Abdurachmanov
  2018-11-05 14:43 ` david.abdurachmanov
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 24+ messages in thread
From: David Abdurachmanov @ 2018-11-05 14:26 UTC (permalink / raw)
  To: palmer, aou, linux-riscv, linux-kernel
  Cc: Marcin Juszkiewicz, Guenter Roeck, Arnd Bergmann, David Abdurachmanov

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 <david.abdurachmanov@gmail.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
Cc: Guenter Roeck <linux@roeck-us.net>
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 <david.abdurachmanov@gmail.com>
+ *
+ * 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


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* [PATCH] riscv: add asm/unistd.h UAPI header
  2018-11-05 14:26 [PATCH] riscv: add asm/unistd.h UAPI header david.abdurachmanov
  2018-11-05 14:26 ` David Abdurachmanov
@ 2018-11-05 14:43 ` david.abdurachmanov
  2018-11-05 14:43   ` David Abdurachmanov
  2018-11-05 16:02 ` marcin.juszkiewicz
  2018-11-05 20:56 ` arnd
  3 siblings, 1 reply; 24+ messages in thread
From: david.abdurachmanov @ 2018-11-05 14:43 UTC (permalink / raw)
  To: linux-riscv

On Mon, Nov 5, 2018 at 3:26 PM David Abdurachmanov
<david.abdurachmanov@gmail.com> 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

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

* Re: [PATCH] riscv: add asm/unistd.h UAPI header
  2018-11-05 14:43 ` david.abdurachmanov
@ 2018-11-05 14:43   ` David Abdurachmanov
  0 siblings, 0 replies; 24+ messages in thread
From: David Abdurachmanov @ 2018-11-05 14:43 UTC (permalink / raw)
  To: Palmer Dabbelt, aou, linux-riscv, linux-kernel
  Cc: Marcin Juszkiewicz, Guenter Roeck, Arnd Bergmann

On Mon, Nov 5, 2018 at 3:26 PM David Abdurachmanov
<david.abdurachmanov@gmail.com> 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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* [PATCH] riscv: add asm/unistd.h UAPI header
  2018-11-05 14:26 [PATCH] riscv: add asm/unistd.h UAPI header david.abdurachmanov
  2018-11-05 14:26 ` David Abdurachmanov
  2018-11-05 14:43 ` david.abdurachmanov
@ 2018-11-05 16:02 ` marcin.juszkiewicz
  2018-11-05 16:02   ` Marcin Juszkiewicz
  2018-11-05 20:56 ` arnd
  3 siblings, 1 reply; 24+ messages in thread
From: marcin.juszkiewicz @ 2018-11-05 16:02 UTC (permalink / raw)
  To: linux-riscv

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 <marcin.juszkiewicz@linaro.org>

With patch applied I got proper syscall data:

fstat  80
newfstatat     79
riscv_flush_icache     259

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

* Re: [PATCH] riscv: add asm/unistd.h UAPI header
  2018-11-05 16:02 ` marcin.juszkiewicz
@ 2018-11-05 16:02   ` Marcin Juszkiewicz
  0 siblings, 0 replies; 24+ messages in thread
From: Marcin Juszkiewicz @ 2018-11-05 16:02 UTC (permalink / raw)
  To: David Abdurachmanov, palmer, aou, linux-riscv, linux-kernel
  Cc: Guenter Roeck, Arnd Bergmann

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 <marcin.juszkiewicz@linaro.org>

With patch applied I got proper syscall data:

fstat  80
newfstatat     79
riscv_flush_icache     259

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* [PATCH] riscv: add asm/unistd.h UAPI header
  2018-11-05 14:26 [PATCH] riscv: add asm/unistd.h UAPI header david.abdurachmanov
                   ` (2 preceding siblings ...)
  2018-11-05 16:02 ` marcin.juszkiewicz
@ 2018-11-05 20:56 ` arnd
  2018-11-05 20:56   ` Arnd Bergmann
  2018-11-07  0:08   ` palmer
  3 siblings, 2 replies; 24+ messages in thread
From: arnd @ 2018-11-05 20:56 UTC (permalink / raw)
  To: linux-riscv

On 11/5/18, David Abdurachmanov <david.abdurachmanov@gmail.com> 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 <david.abdurachmanov@gmail.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
> Cc: Guenter Roeck <linux@roeck-us.net>

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")

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

* Re: [PATCH] riscv: add asm/unistd.h UAPI header
  2018-11-05 20:56 ` arnd
@ 2018-11-05 20:56   ` Arnd Bergmann
  2018-11-07  0:08   ` palmer
  1 sibling, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2018-11-05 20:56 UTC (permalink / raw)
  To: David Abdurachmanov
  Cc: aou, palmer, linux-kernel, Marcin Juszkiewicz, linux-riscv,
	Guenter Roeck

On 11/5/18, David Abdurachmanov <david.abdurachmanov@gmail.com> 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 <david.abdurachmanov@gmail.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
> Cc: Guenter Roeck <linux@roeck-us.net>

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")

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* [PATCH] riscv: add asm/unistd.h UAPI header
  2018-11-05 20:56 ` arnd
  2018-11-05 20:56   ` Arnd Bergmann
@ 2018-11-07  0:08   ` palmer
  2018-11-07  0:08     ` Palmer Dabbelt
  2018-11-07 18:30     ` david.abdurachmanov
  1 sibling, 2 replies; 24+ messages in thread
From: palmer @ 2018-11-07  0:08 UTC (permalink / raw)
  To: linux-riscv

On Mon, 05 Nov 2018 12:56:15 PST (-0800), Arnd Bergmann wrote:
> On 11/5/18, David Abdurachmanov <david.abdurachmanov@gmail.com> 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 <david.abdurachmanov@gmail.com>
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> Cc: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
>> Cc: Guenter Roeck <linux@roeck-us.net>
>
> 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 :).

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

* Re: [PATCH] riscv: add asm/unistd.h UAPI header
  2018-11-07  0:08   ` palmer
@ 2018-11-07  0:08     ` Palmer Dabbelt
  2018-11-07 18:30     ` david.abdurachmanov
  1 sibling, 0 replies; 24+ messages in thread
From: Palmer Dabbelt @ 2018-11-07  0:08 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: aou, david.abdurachmanov, linux-kernel, marcin.juszkiewicz,
	linux-riscv, linux

On Mon, 05 Nov 2018 12:56:15 PST (-0800), Arnd Bergmann wrote:
> On 11/5/18, David Abdurachmanov <david.abdurachmanov@gmail.com> 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 <david.abdurachmanov@gmail.com>
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> Cc: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
>> Cc: Guenter Roeck <linux@roeck-us.net>
>
> 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 :).

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* [PATCH] riscv: add asm/unistd.h UAPI header
  2018-11-07  0:08   ` palmer
  2018-11-07  0:08     ` Palmer Dabbelt
@ 2018-11-07 18:30     ` david.abdurachmanov
  2018-11-07 18:30       ` David Abdurachmanov
  2018-11-07 21:09       ` arnd
  1 sibling, 2 replies; 24+ messages in thread
From: david.abdurachmanov @ 2018-11-07 18:30 UTC (permalink / raw)
  To: linux-riscv

On Wed, Nov 7, 2018 at 1:08 AM Palmer Dabbelt <palmer@sifive.com> wrote:
>
> On Mon, 05 Nov 2018 12:56:15 PST (-0800), Arnd Bergmann wrote:
> > On 11/5/18, David Abdurachmanov <david.abdurachmanov@gmail.com> 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 <david.abdurachmanov@gmail.com>
> >> Cc: Arnd Bergmann <arnd@arndb.de>
> >> Cc: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
> >> Cc: Guenter Roeck <linux@roeck-us.net>
> >
> > 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

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

* Re: [PATCH] riscv: add asm/unistd.h UAPI header
  2018-11-07 18:30     ` david.abdurachmanov
@ 2018-11-07 18:30       ` David Abdurachmanov
  2018-11-07 21:09       ` arnd
  1 sibling, 0 replies; 24+ messages in thread
From: David Abdurachmanov @ 2018-11-07 18:30 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: aou, Arnd Bergmann, linux-kernel, Marcin Juszkiewicz,
	linux-riscv, Guenter Roeck

On Wed, Nov 7, 2018 at 1:08 AM Palmer Dabbelt <palmer@sifive.com> wrote:
>
> On Mon, 05 Nov 2018 12:56:15 PST (-0800), Arnd Bergmann wrote:
> > On 11/5/18, David Abdurachmanov <david.abdurachmanov@gmail.com> 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 <david.abdurachmanov@gmail.com>
> >> Cc: Arnd Bergmann <arnd@arndb.de>
> >> Cc: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
> >> Cc: Guenter Roeck <linux@roeck-us.net>
> >
> > 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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* [PATCH] riscv: add asm/unistd.h UAPI header
  2018-11-07 18:30     ` david.abdurachmanov
  2018-11-07 18:30       ` David Abdurachmanov
@ 2018-11-07 21:09       ` arnd
  2018-11-07 21:09         ` Arnd Bergmann
  2018-11-08  2:10         ` palmer
  1 sibling, 2 replies; 24+ messages in thread
From: arnd @ 2018-11-07 21:09 UTC (permalink / raw)
  To: linux-riscv

On Wed, Nov 7, 2018 at 7:30 PM David Abdurachmanov
<david.abdurachmanov@gmail.com> wrote:
> On Wed, Nov 7, 2018 at 1:08 AM Palmer Dabbelt <palmer@sifive.com> 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

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

* Re: [PATCH] riscv: add asm/unistd.h UAPI header
  2018-11-07 21:09       ` arnd
@ 2018-11-07 21:09         ` Arnd Bergmann
  2018-11-08  2:10         ` palmer
  1 sibling, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2018-11-07 21:09 UTC (permalink / raw)
  To: David Abdurachmanov
  Cc: Albert Ou, Palmer Dabbelt, Linux Kernel Mailing List,
	Marcin Juszkiewicz, linux-riscv, Guenter Roeck

On Wed, Nov 7, 2018 at 7:30 PM David Abdurachmanov
<david.abdurachmanov@gmail.com> wrote:
> On Wed, Nov 7, 2018 at 1:08 AM Palmer Dabbelt <palmer@sifive.com> 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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* [PATCH] riscv: add asm/unistd.h UAPI header
  2018-11-07 21:09       ` arnd
  2018-11-07 21:09         ` Arnd Bergmann
@ 2018-11-08  2:10         ` palmer
  2018-11-08  2:10           ` Palmer Dabbelt
                             ` (2 more replies)
  1 sibling, 3 replies; 24+ messages in thread
From: palmer @ 2018-11-08  2:10 UTC (permalink / raw)
  To: linux-riscv

On Wed, 07 Nov 2018 13:09:39 PST (-0800), Arnd Bergmann wrote:
> On Wed, Nov 7, 2018 at 7:30 PM David Abdurachmanov
> <david.abdurachmanov@gmail.com> wrote:
>> On Wed, Nov 7, 2018 at 1:08 AM Palmer Dabbelt <palmer@sifive.com> 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 :)

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

* Re: [PATCH] riscv: add asm/unistd.h UAPI header
  2018-11-08  2:10         ` palmer
@ 2018-11-08  2:10           ` Palmer Dabbelt
  2018-11-08 10:30           ` arnd
  2018-11-08 10:38           ` david.abdurachmanov
  2 siblings, 0 replies; 24+ messages in thread
From: Palmer Dabbelt @ 2018-11-08  2:10 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: aou, david.abdurachmanov, linux-kernel, marcin.juszkiewicz,
	linux-riscv, linux

On Wed, 07 Nov 2018 13:09:39 PST (-0800), Arnd Bergmann wrote:
> On Wed, Nov 7, 2018 at 7:30 PM David Abdurachmanov
> <david.abdurachmanov@gmail.com> wrote:
>> On Wed, Nov 7, 2018 at 1:08 AM Palmer Dabbelt <palmer@sifive.com> 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 :)

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* [PATCH] riscv: add asm/unistd.h UAPI header
  2018-11-08  2:10         ` palmer
  2018-11-08  2:10           ` Palmer Dabbelt
@ 2018-11-08 10:30           ` arnd
  2018-11-08 10:30             ` Arnd Bergmann
  2018-11-08 16:57             ` palmer
  2018-11-08 10:38           ` david.abdurachmanov
  2 siblings, 2 replies; 24+ messages in thread
From: arnd @ 2018-11-08 10:30 UTC (permalink / raw)
  To: linux-riscv

On Thu, Nov 8, 2018 at 3:10 AM Palmer Dabbelt <palmer@sifive.com> 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
> > <david.abdurachmanov@gmail.com> wrote:
> >> On Wed, Nov 7, 2018 at 1:08 AM Palmer Dabbelt <palmer@sifive.com> 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

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

* Re: [PATCH] riscv: add asm/unistd.h UAPI header
  2018-11-08 10:30           ` arnd
@ 2018-11-08 10:30             ` Arnd Bergmann
  2018-11-08 16:57             ` palmer
  1 sibling, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2018-11-08 10:30 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: Albert Ou, David Abdurachmanov, Linux Kernel Mailing List,
	Marcin Juszkiewicz, linux-riscv, Guenter Roeck

On Thu, Nov 8, 2018 at 3:10 AM Palmer Dabbelt <palmer@sifive.com> 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
> > <david.abdurachmanov@gmail.com> wrote:
> >> On Wed, Nov 7, 2018 at 1:08 AM Palmer Dabbelt <palmer@sifive.com> 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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* [PATCH] riscv: add asm/unistd.h UAPI header
  2018-11-08  2:10         ` palmer
  2018-11-08  2:10           ` Palmer Dabbelt
  2018-11-08 10:30           ` arnd
@ 2018-11-08 10:38           ` david.abdurachmanov
  2018-11-08 10:38             ` David Abdurachmanov
  2018-11-08 16:57             ` palmer
  2 siblings, 2 replies; 24+ messages in thread
From: david.abdurachmanov @ 2018-11-08 10:38 UTC (permalink / raw)
  To: linux-riscv

On Thu, Nov 8, 2018 at 3:10 AM Palmer Dabbelt <palmer@sifive.com> 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
> > <david.abdurachmanov@gmail.com> wrote:
> >> On Wed, Nov 7, 2018 at 1:08 AM Palmer Dabbelt <palmer@sifive.com> 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.

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

* Re: [PATCH] riscv: add asm/unistd.h UAPI header
  2018-11-08 10:38           ` david.abdurachmanov
@ 2018-11-08 10:38             ` David Abdurachmanov
  2018-11-08 16:57             ` palmer
  1 sibling, 0 replies; 24+ messages in thread
From: David Abdurachmanov @ 2018-11-08 10:38 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: aou, Arnd Bergmann, linux-kernel, Marcin Juszkiewicz,
	linux-riscv, Guenter Roeck

On Thu, Nov 8, 2018 at 3:10 AM Palmer Dabbelt <palmer@sifive.com> 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
> > <david.abdurachmanov@gmail.com> wrote:
> >> On Wed, Nov 7, 2018 at 1:08 AM Palmer Dabbelt <palmer@sifive.com> 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.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* [PATCH] riscv: add asm/unistd.h UAPI header
  2018-11-08 10:30           ` arnd
  2018-11-08 10:30             ` Arnd Bergmann
@ 2018-11-08 16:57             ` palmer
  2018-11-08 16:57               ` Palmer Dabbelt
  1 sibling, 1 reply; 24+ messages in thread
From: palmer @ 2018-11-08 16:57 UTC (permalink / raw)
  To: linux-riscv

On Thu, 08 Nov 2018 02:30:02 PST (-0800), Arnd Bergmann wrote:
> On Thu, Nov 8, 2018 at 3:10 AM Palmer Dabbelt <palmer@sifive.com> 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
>> > <david.abdurachmanov@gmail.com> wrote:
>> >> On Wed, Nov 7, 2018 at 1:08 AM Palmer Dabbelt <palmer@sifive.com> 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!

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

* Re: [PATCH] riscv: add asm/unistd.h UAPI header
  2018-11-08 16:57             ` palmer
@ 2018-11-08 16:57               ` Palmer Dabbelt
  0 siblings, 0 replies; 24+ messages in thread
From: Palmer Dabbelt @ 2018-11-08 16:57 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: aou, david.abdurachmanov, linux-kernel, marcin.juszkiewicz,
	linux-riscv, linux

On Thu, 08 Nov 2018 02:30:02 PST (-0800), Arnd Bergmann wrote:
> On Thu, Nov 8, 2018 at 3:10 AM Palmer Dabbelt <palmer@sifive.com> 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
>> > <david.abdurachmanov@gmail.com> wrote:
>> >> On Wed, Nov 7, 2018 at 1:08 AM Palmer Dabbelt <palmer@sifive.com> 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!

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* [PATCH] riscv: add asm/unistd.h UAPI header
  2018-11-08 10:38           ` david.abdurachmanov
  2018-11-08 10:38             ` David Abdurachmanov
@ 2018-11-08 16:57             ` palmer
  2018-11-08 16:57               ` Palmer Dabbelt
  1 sibling, 1 reply; 24+ messages in thread
From: palmer @ 2018-11-08 16:57 UTC (permalink / raw)
  To: linux-riscv

On Thu, 08 Nov 2018 02:38:22 PST (-0800), david.abdurachmanov at gmail.com wrote:
> On Thu, Nov 8, 2018 at 3:10 AM Palmer Dabbelt <palmer@sifive.com> 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
>> > <david.abdurachmanov@gmail.com> wrote:
>> >> On Wed, Nov 7, 2018 at 1:08 AM Palmer Dabbelt <palmer@sifive.com> 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.

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

* Re: [PATCH] riscv: add asm/unistd.h UAPI header
  2018-11-08 16:57             ` palmer
@ 2018-11-08 16:57               ` Palmer Dabbelt
  0 siblings, 0 replies; 24+ messages in thread
From: Palmer Dabbelt @ 2018-11-08 16:57 UTC (permalink / raw)
  To: david.abdurachmanov
  Cc: aou, Arnd Bergmann, linux-kernel, marcin.juszkiewicz, linux-riscv, linux

On Thu, 08 Nov 2018 02:38:22 PST (-0800), david.abdurachmanov@gmail.com wrote:
> On Thu, Nov 8, 2018 at 3:10 AM Palmer Dabbelt <palmer@sifive.com> 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
>> > <david.abdurachmanov@gmail.com> wrote:
>> >> On Wed, Nov 7, 2018 at 1:08 AM Palmer Dabbelt <palmer@sifive.com> 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.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

end of thread, back to index

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-05 14:26 [PATCH] riscv: add asm/unistd.h UAPI header david.abdurachmanov
2018-11-05 14:26 ` David Abdurachmanov
2018-11-05 14:43 ` david.abdurachmanov
2018-11-05 14:43   ` David Abdurachmanov
2018-11-05 16:02 ` marcin.juszkiewicz
2018-11-05 16:02   ` Marcin Juszkiewicz
2018-11-05 20:56 ` arnd
2018-11-05 20:56   ` Arnd Bergmann
2018-11-07  0:08   ` palmer
2018-11-07  0:08     ` Palmer Dabbelt
2018-11-07 18:30     ` david.abdurachmanov
2018-11-07 18:30       ` David Abdurachmanov
2018-11-07 21:09       ` arnd
2018-11-07 21:09         ` Arnd Bergmann
2018-11-08  2:10         ` palmer
2018-11-08  2:10           ` Palmer Dabbelt
2018-11-08 10:30           ` arnd
2018-11-08 10:30             ` Arnd Bergmann
2018-11-08 16:57             ` palmer
2018-11-08 16:57               ` Palmer Dabbelt
2018-11-08 10:38           ` david.abdurachmanov
2018-11-08 10:38             ` David Abdurachmanov
2018-11-08 16:57             ` palmer
2018-11-08 16:57               ` Palmer Dabbelt

Linux-RISC-V Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-riscv/0 linux-riscv/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-riscv linux-riscv/ https://lore.kernel.org/linux-riscv \
		linux-riscv@lists.infradead.org infradead-linux-riscv@archiver.kernel.org
	public-inbox-index linux-riscv


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-riscv


AGPL code for this site: git clone https://public-inbox.org/ public-inbox