RISC-V: Fix the build on LLVM-based toolchains

RISC-V: Fix the build on LLVM-based toolchains

[Palmer Dabbelt]

The RISC-V LLVM port has progressed to the point where it should be able to use
it to compile Linux.  Unfortunately we ended up with a few GNU-isms in our port
so that doesn't work out of the box, but I don't think the code without them is
any uglier than the code with them so I'm happy to support both toolchains.
There are still some issues using the GNU assembler to compile clang's assembly
(at least got_pcrel_hi, but there may be others).  I'm going to call those
binutils bugs, though, and chase them around over there.

While the first one could be considered a bug fix, I think the bug is unlikely
enough to manifst that I'm going to wait for the merge window for these.  I'm
going to preemptively drop them on for-next now, but as I haven't really
started building that branch they'll be rebased (my current plan is to start
taking 5.7 patches on top of rc4, as it seems like things are shaping up to be
fairly solid on our end).  If there are any comments I'll handle them as part
of the rebase, but I'd like the various autobuilders to start chewing on these.

Unfortunately the kernel compiled with LLVM doesn't boot for me.

[PATCH 1/3] RISC-V: Stop relying on GCC's register allocator's hueristics

[Palmer Dabbelt]

From: Palmer Dabbelt <palmerdabbelt@google.com>

GCC allows users to hint to the register allocation that a variable should be
placed in a register by using a syntax along the lines of

    function(...) {
        register long in_REG __asm__("REG");
    }

We've abused this a bit throughout the RISC-V port to access fixed registers
directly as C variables.  In practice it's never going to blow up because GCC
isn't going to allocate these registers, but it's not a well defined syntax so
we really shouldn't be relying upon this.  Luckily there is a very similar but
well defined syntax that allows us to still access these registers directly as
C variables, which is to simply declare the register variables globally.  For
fixed variables this doesn't change the ABI.

LLVM disallows this ambiguous syntax, so this isn't just strictly a formatting
change.

Signed-off-by: Palmer Dabbelt <palmerdabbelt@google.com>
---
 arch/riscv/include/asm/current.h | 5 +++--
 arch/riscv/kernel/process.c      | 5 +++--
 arch/riscv/kernel/stacktrace.c   | 7 ++++---
 3 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/arch/riscv/include/asm/current.h b/arch/riscv/include/asm/current.h
index dd973efe5d7c..1de233d8e8de 100644
--- a/arch/riscv/include/asm/current.h
+++ b/arch/riscv/include/asm/current.h
@@ -17,6 +17,8 @@
 
 struct task_struct;
 
+register struct task_struct *riscv_current_is_tp __asm__("tp");
+
 /*
  * This only works because "struct thread_info" is at offset 0 from "struct
  * task_struct".  This constraint seems to be necessary on other architectures
@@ -26,8 +28,7 @@ struct task_struct;
  */
 static __always_inline struct task_struct *get_current(void)
 {
-	register struct task_struct *tp __asm__("tp");
-	return tp;
+	return riscv_current_is_tp;
 }
 
 #define current get_current()
diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
index 817cf7b0974c..610c11e91606 100644
--- a/arch/riscv/kernel/process.c
+++ b/arch/riscv/kernel/process.c
@@ -22,6 +22,8 @@
 #include <asm/switch_to.h>
 #include <asm/thread_info.h>
 
+unsigned long gp_in_global __asm__("gp");
+
 extern asmlinkage void ret_from_fork(void);
 extern asmlinkage void ret_from_kernel_thread(void);
 
@@ -107,9 +109,8 @@ int copy_thread_tls(unsigned long clone_flags, unsigned long usp,
 	/* p->thread holds context to be restored by __switch_to() */
 	if (unlikely(p->flags & PF_KTHREAD)) {
 		/* Kernel thread */
-		const register unsigned long gp __asm__ ("gp");
 		memset(childregs, 0, sizeof(struct pt_regs));
-		childregs->gp = gp;
+		childregs->gp = gp_in_global;
 		/* Supervisor/Machine, irqs on: */
 		childregs->status = SR_PP | SR_PIE;
 
diff --git a/arch/riscv/kernel/stacktrace.c b/arch/riscv/kernel/stacktrace.c
index 0940681d2f68..02087fe539c6 100644
--- a/arch/riscv/kernel/stacktrace.c
+++ b/arch/riscv/kernel/stacktrace.c
@@ -19,6 +19,8 @@ struct stackframe {
 	unsigned long ra;
 };
 
+register unsigned long sp_in_global __asm__("sp");
+
 void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs,
 			     bool (*fn)(unsigned long, void *), void *arg)
 {
@@ -29,7 +31,7 @@ void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs,
 		sp = user_stack_pointer(regs);
 		pc = instruction_pointer(regs);
 	} else if (task == NULL || task == current) {
-		const register unsigned long current_sp __asm__ ("sp");
+		const register unsigned long current_sp = sp_in_global;
 		fp = (unsigned long)__builtin_frame_address(0);
 		sp = current_sp;
 		pc = (unsigned long)walk_stackframe;
@@ -73,8 +75,7 @@ static void notrace walk_stackframe(struct task_struct *task,
 		sp = user_stack_pointer(regs);
 		pc = instruction_pointer(regs);
 	} else if (task == NULL || task == current) {
-		const register unsigned long current_sp __asm__ ("sp");
-		sp = current_sp;
+		sp = sp_in_global;
 		pc = (unsigned long)walk_stackframe;
 	} else {
 		/* task blocked in __switch_to */
-- 
2.25.1.481.gfbce0eb801-goog

[PATCH 2/3] RISC-V: Inline the assembly register save/restore macros

[Palmer Dabbelt]

From: Palmer Dabbelt <palmerdabbelt@google.com>

These are only used once, and when reading the code I've always found them to
be more of a headache than a benefit.  While they were never worth removing
before, LLVM's integrated assembler doesn't support LOCAL so rather that trying
to figure out how to refactor the macros it seems saner to just inline them.

Signed-off-by: Palmer Dabbelt <palmerdabbelt@google.com>
---
 arch/riscv/kernel/entry.S | 143 ++++++++++++++++----------------------
 1 file changed, 61 insertions(+), 82 deletions(-)

diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
index bad4d85b5e91..f2e8e7c8089d 100644
--- a/arch/riscv/kernel/entry.S
+++ b/arch/riscv/kernel/entry.S
@@ -13,17 +13,11 @@
 #include <asm/thread_info.h>
 #include <asm/asm-offsets.h>
 
-	.text
-	.altmacro
-
-/*
- * Prepares to enter a system call or exception by saving all registers to the
- * stack.
- */
-	.macro SAVE_ALL
-	LOCAL _restore_kernel_tpsp
-	LOCAL _save_context
+#if !IS_ENABLED(CONFIG_PREEMPTION)
+.set resume_kernel, restore_all
+#endif
 
+ENTRY(handle_exception)
 	/*
 	 * If coming from userspace, preserve the user thread pointer and load
 	 * the kernel thread pointer.  If we came from the kernel, the scratch
@@ -90,77 +84,6 @@ _save_context:
 	REG_S s3, PT_BADADDR(sp)
 	REG_S s4, PT_CAUSE(sp)
 	REG_S s5, PT_TP(sp)
-	.endm
-
-/*
- * Prepares to return from a system call or exception by restoring all
- * registers from the stack.
- */
-	.macro RESTORE_ALL
-	REG_L a0, PT_STATUS(sp)
-	/*
-	 * The current load reservation is effectively part of the processor's
-	 * state, in the sense that load reservations cannot be shared between
-	 * different hart contexts.  We can't actually save and restore a load
-	 * reservation, so instead here we clear any existing reservation --
-	 * it's always legal for implementations to clear load reservations at
-	 * any point (as long as the forward progress guarantee is kept, but
-	 * we'll ignore that here).
-	 *
-	 * Dangling load reservations can be the result of taking a trap in the
-	 * middle of an LR/SC sequence, but can also be the result of a taken
-	 * forward branch around an SC -- which is how we implement CAS.  As a
-	 * result we need to clear reservations between the last CAS and the
-	 * jump back to the new context.  While it is unlikely the store
-	 * completes, implementations are allowed to expand reservations to be
-	 * arbitrarily large.
-	 */
-	REG_L  a2, PT_EPC(sp)
-	REG_SC x0, a2, PT_EPC(sp)
-
-	csrw CSR_STATUS, a0
-	csrw CSR_EPC, a2
-
-	REG_L x1,  PT_RA(sp)
-	REG_L x3,  PT_GP(sp)
-	REG_L x4,  PT_TP(sp)
-	REG_L x5,  PT_T0(sp)
-	REG_L x6,  PT_T1(sp)
-	REG_L x7,  PT_T2(sp)
-	REG_L x8,  PT_S0(sp)
-	REG_L x9,  PT_S1(sp)
-	REG_L x10, PT_A0(sp)
-	REG_L x11, PT_A1(sp)
-	REG_L x12, PT_A2(sp)
-	REG_L x13, PT_A3(sp)
-	REG_L x14, PT_A4(sp)
-	REG_L x15, PT_A5(sp)
-	REG_L x16, PT_A6(sp)
-	REG_L x17, PT_A7(sp)
-	REG_L x18, PT_S2(sp)
-	REG_L x19, PT_S3(sp)
-	REG_L x20, PT_S4(sp)
-	REG_L x21, PT_S5(sp)
-	REG_L x22, PT_S6(sp)
-	REG_L x23, PT_S7(sp)
-	REG_L x24, PT_S8(sp)
-	REG_L x25, PT_S9(sp)
-	REG_L x26, PT_S10(sp)
-	REG_L x27, PT_S11(sp)
-	REG_L x28, PT_T3(sp)
-	REG_L x29, PT_T4(sp)
-	REG_L x30, PT_T5(sp)
-	REG_L x31, PT_T6(sp)
-
-	REG_L x2,  PT_SP(sp)
-	.endm
-
-#if !IS_ENABLED(CONFIG_PREEMPTION)
-.set resume_kernel, restore_all
-#endif
-
-ENTRY(handle_exception)
-	SAVE_ALL
 
 	/*
 	 * Set the scratch register to 0, so that if a recursive exception
@@ -298,7 +221,63 @@ resume_userspace:
 	csrw CSR_SCRATCH, tp
 
 restore_all:
-	RESTORE_ALL
+	REG_L a0, PT_STATUS(sp)
+	/*
+	 * The current load reservation is effectively part of the processor's
+	 * state, in the sense that load reservations cannot be shared between
+	 * different hart contexts.  We can't actually save and restore a load
+	 * reservation, so instead here we clear any existing reservation --
+	 * it's always legal for implementations to clear load reservations at
+	 * any point (as long as the forward progress guarantee is kept, but
+	 * we'll ignore that here).
+	 *
+	 * Dangling load reservations can be the result of taking a trap in the
+	 * middle of an LR/SC sequence, but can also be the result of a taken
+	 * forward branch around an SC -- which is how we implement CAS.  As a
+	 * result we need to clear reservations between the last CAS and the
+	 * jump back to the new context.  While it is unlikely the store
+	 * completes, implementations are allowed to expand reservations to be
+	 * arbitrarily large.
+	 */
+	REG_L  a2, PT_EPC(sp)
+	REG_SC x0, a2, PT_EPC(sp)
+
+	csrw CSR_STATUS, a0
+	csrw CSR_EPC, a2
+
+	REG_L x1,  PT_RA(sp)
+	REG_L x3,  PT_GP(sp)
+	REG_L x4,  PT_TP(sp)
+	REG_L x5,  PT_T0(sp)
+	REG_L x6,  PT_T1(sp)
+	REG_L x7,  PT_T2(sp)
+	REG_L x8,  PT_S0(sp)
+	REG_L x9,  PT_S1(sp)
+	REG_L x10, PT_A0(sp)
+	REG_L x11, PT_A1(sp)
+	REG_L x12, PT_A2(sp)
+	REG_L x13, PT_A3(sp)
+	REG_L x14, PT_A4(sp)
+	REG_L x15, PT_A5(sp)
+	REG_L x16, PT_A6(sp)
+	REG_L x17, PT_A7(sp)
+	REG_L x18, PT_S2(sp)
+	REG_L x19, PT_S3(sp)
+	REG_L x20, PT_S4(sp)
+	REG_L x21, PT_S5(sp)
+	REG_L x22, PT_S6(sp)
+	REG_L x23, PT_S7(sp)
+	REG_L x24, PT_S8(sp)
+	REG_L x25, PT_S9(sp)
+	REG_L x26, PT_S10(sp)
+	REG_L x27, PT_S11(sp)
+	REG_L x28, PT_T3(sp)
+	REG_L x29, PT_T4(sp)
+	REG_L x30, PT_T5(sp)
+	REG_L x31, PT_T6(sp)
+
+	REG_L x2,  PT_SP(sp)
+
 #ifdef CONFIG_RISCV_M_MODE
 	mret
 #else
-- 
2.25.1.481.gfbce0eb801-goog

[PATCH 3/3] RISC-V: Stop using LOCAL for the uaccess fixups

[Palmer Dabbelt]

From: Palmer Dabbelt <palmerdabbelt@google.com>

LLVM's integrated assembler doesn't support the LOCAL directive, which we're
using when generating our uaccess fixup tables.  Luckily the table fragment is
small enough that there's only one internal symbol, so using a relative symbol
reference doesn't really complicate anything.

Signed-off-by: Palmer Dabbelt <palmerdabbelt@google.com>
---
 arch/riscv/lib/uaccess.S | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/riscv/lib/uaccess.S b/arch/riscv/lib/uaccess.S
index f29d2ba2c0a6..40bf130073e8 100644
--- a/arch/riscv/lib/uaccess.S
+++ b/arch/riscv/lib/uaccess.S
@@ -5,12 +5,11 @@
 
 	.altmacro
 	.macro fixup op reg addr lbl
-	LOCAL _epc
-_epc:
+100:
 	\op \reg, \addr
 	.section __ex_table,"a"
 	.balign RISCV_SZPTR
-	RISCV_PTR _epc, \lbl
+	RISCV_PTR 100b, \lbl
 	.previous
 	.endm
 
-- 
2.25.1.481.gfbce0eb801-goog

Re: [PATCH 1/3] RISC-V: Stop relying on GCC's register allocator's hueristics

[Nick Desaulniers]

On Thu, Feb 27, 2020 at 1:35 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> From: Palmer Dabbelt <palmerdabbelt@google.com>
>
> GCC allows users to hint to the register allocation that a variable should be
> placed in a register by using a syntax along the lines of
>
>     function(...) {
>         register long in_REG __asm__("REG");
>     }
>
> We've abused this a bit throughout the RISC-V port to access fixed registers
> directly as C variables.  In practice it's never going to blow up because GCC
> isn't going to allocate these registers, but it's not a well defined syntax so
> we really shouldn't be relying upon this.  Luckily there is a very similar but
> well defined syntax that allows us to still access these registers directly as
> C variables, which is to simply declare the register variables globally.  For
> fixed variables this doesn't change the ABI.
>
> LLVM disallows this ambiguous syntax, so this isn't just strictly a formatting
> change.
>
> Signed-off-by: Palmer Dabbelt <palmerdabbelt@google.com>

Thanks for the patches!
Indeed, looks like the local register variables are very much intended
to be used as inputs & outputs to extended inline assembly, which in
these cases are not.
Link: https://gcc.gnu.org/onlinedocs/gcc/Local-Register-Variables.html
Link: https://gcc.gnu.org/onlinedocs/gcc/Global-Register-Variables.html
One fixup, below:

> ---
>  arch/riscv/include/asm/current.h | 5 +++--
>  arch/riscv/kernel/process.c      | 5 +++--
>  arch/riscv/kernel/stacktrace.c   | 7 ++++---
>  3 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/arch/riscv/include/asm/current.h b/arch/riscv/include/asm/current.h
> index dd973efe5d7c..1de233d8e8de 100644
> --- a/arch/riscv/include/asm/current.h
> +++ b/arch/riscv/include/asm/current.h
> @@ -17,6 +17,8 @@
>
>  struct task_struct;
>
> +register struct task_struct *riscv_current_is_tp __asm__("tp");
> +
>  /*
>   * This only works because "struct thread_info" is at offset 0 from "struct
>   * task_struct".  This constraint seems to be necessary on other architectures
> @@ -26,8 +28,7 @@ struct task_struct;
>   */
>  static __always_inline struct task_struct *get_current(void)
>  {
> -       register struct task_struct *tp __asm__("tp");
> -       return tp;
> +       return riscv_current_is_tp;
>  }
>
>  #define current get_current()
> diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
> index 817cf7b0974c..610c11e91606 100644
> --- a/arch/riscv/kernel/process.c
> +++ b/arch/riscv/kernel/process.c
> @@ -22,6 +22,8 @@
>  #include <asm/switch_to.h>
>  #include <asm/thread_info.h>
>
> +unsigned long gp_in_global __asm__("gp");
> +
>  extern asmlinkage void ret_from_fork(void);
>  extern asmlinkage void ret_from_kernel_thread(void);
>
> @@ -107,9 +109,8 @@ int copy_thread_tls(unsigned long clone_flags, unsigned long usp,
>         /* p->thread holds context to be restored by __switch_to() */
>         if (unlikely(p->flags & PF_KTHREAD)) {
>                 /* Kernel thread */
> -               const register unsigned long gp __asm__ ("gp");
>                 memset(childregs, 0, sizeof(struct pt_regs));
> -               childregs->gp = gp;
> +               childregs->gp = gp_in_global;
>                 /* Supervisor/Machine, irqs on: */
>                 childregs->status = SR_PP | SR_PIE;
>
> diff --git a/arch/riscv/kernel/stacktrace.c b/arch/riscv/kernel/stacktrace.c
> index 0940681d2f68..02087fe539c6 100644
> --- a/arch/riscv/kernel/stacktrace.c
> +++ b/arch/riscv/kernel/stacktrace.c
> @@ -19,6 +19,8 @@ struct stackframe {
>         unsigned long ra;
>  };
>
> +register unsigned long sp_in_global __asm__("sp");
> +
>  void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs,
>                              bool (*fn)(unsigned long, void *), void *arg)
>  {
> @@ -29,7 +31,7 @@ void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs,
>                 sp = user_stack_pointer(regs);
>                 pc = instruction_pointer(regs);
>         } else if (task == NULL || task == current) {
> -               const register unsigned long current_sp __asm__ ("sp");
> +               const register unsigned long current_sp = sp_in_global;
>                 fp = (unsigned long)__builtin_frame_address(0);
>                 sp = current_sp;

^ probably this should just be:
sp = sp_in_global;

>                 pc = (unsigned long)walk_stackframe;
> @@ -73,8 +75,7 @@ static void notrace walk_stackframe(struct task_struct *task,
>                 sp = user_stack_pointer(regs);
>                 pc = instruction_pointer(regs);
>         } else if (task == NULL || task == current) {
> -               const register unsigned long current_sp __asm__ ("sp");
> -               sp = current_sp;
> +               sp = sp_in_global;
>                 pc = (unsigned long)walk_stackframe;
>         } else {
>                 /* task blocked in __switch_to */
> --
> 2.25.1.481.gfbce0eb801-goog
>
> --

-- 
Thanks,
~Nick Desaulniers

Re: [PATCH 3/3] RISC-V: Stop using LOCAL for the uaccess fixups

[Nick Desaulniers]

On Thu, Feb 27, 2020 at 1:35 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> From: Palmer Dabbelt <palmerdabbelt@google.com>
>
> LLVM's integrated assembler doesn't support the LOCAL directive, which we're
> using when generating our uaccess fixup tables.  Luckily the table fragment is
> small enough that there's only one internal symbol, so using a relative symbol
> reference doesn't really complicate anything.

Is `LOCAL` a macro for `.local`? (Looks like no). I would think
`.local` is supported, but `LOCAL` isn't something I've seen before.

Ah, looks like it's local to macros:
https://sourceware.org/binutils/docs/as/Macro.html#Macro
"Warning: LOCAL is only available if you select “alternate macro
syntax” with ‘--alternate’ or .altmacro. See .altmacro."
https://sourceware.org/binutils/docs/as/Altmacro.html#Altmacro
Link: https://sourceware.org/binutils/docs/as/Local.html#Local

But these macros are setting .altmacro...
So it looks like Clang's integrated assembler doesn't yet support
`LOCAL`. Filed:
https://bugs.llvm.org/show_bug.cgi?id=45051

If we're no longer using LOCAL, do we still need `.altmacro`?

I also see two usages in:
arch/riscv/kernel/entry.S

Would you mind fixing those up, too?

Otherwise patch LGTM.

>
> Signed-off-by: Palmer Dabbelt <palmerdabbelt@google.com>
> ---
>  arch/riscv/lib/uaccess.S | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/arch/riscv/lib/uaccess.S b/arch/riscv/lib/uaccess.S
> index f29d2ba2c0a6..40bf130073e8 100644
> --- a/arch/riscv/lib/uaccess.S
> +++ b/arch/riscv/lib/uaccess.S
> @@ -5,12 +5,11 @@
>
>         .altmacro
>         .macro fixup op reg addr lbl
> -       LOCAL _epc
> -_epc:
> +100:
>         \op \reg, \addr
>         .section __ex_table,"a"
>         .balign RISCV_SZPTR
> -       RISCV_PTR _epc, \lbl
> +       RISCV_PTR 100b, \lbl
>         .previous
>         .endm
>
> --

-- 
Thanks,
~Nick Desaulniers

Re: [PATCH 3/3] RISC-V: Stop using LOCAL for the uaccess fixups

[Nick Desaulniers]

On Thu, Feb 27, 2020 at 3:03 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
> I also see two usages in:
> arch/riscv/kernel/entry.S
>
> Would you mind fixing those up, too?

Ah, removed by patch 2/3, sorry for reviewing them out of order!
Outstanding question about `.altmacro` though.

-- 
Thanks,
~Nick Desaulniers

Re: [PATCH 2/3] RISC-V: Inline the assembly register save/restore macros

[Nick Desaulniers]

On Thu, Feb 27, 2020 at 1:35 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> From: Palmer Dabbelt <palmerdabbelt@google.com>
>
> These are only used once, and when reading the code I've always found them to

Looks like there's SAVE_ALL/RESTORE_ALL macros in
arch/riscv/kernel/mcount-dyn.S as well that could probably be
refactored to do the saving/restoring of just the GPRs (since the two
similarly named macros share quite a bit of code between the two
source files, but aren't 100% the same), but this patch is fine, too.
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> be more of a headache than a benefit.  While they were never worth removing
> before, LLVM's integrated assembler doesn't support LOCAL so rather that trying
> to figure out how to refactor the macros it seems saner to just inline them.
>
> Signed-off-by: Palmer Dabbelt <palmerdabbelt@google.com>
> ---
>  arch/riscv/kernel/entry.S | 143 ++++++++++++++++----------------------
>  1 file changed, 61 insertions(+), 82 deletions(-)
>
> diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
> index bad4d85b5e91..f2e8e7c8089d 100644
> --- a/arch/riscv/kernel/entry.S
> +++ b/arch/riscv/kernel/entry.S
> @@ -13,17 +13,11 @@
>  #include <asm/thread_info.h>
>  #include <asm/asm-offsets.h>
>
> -       .text
> -       .altmacro
> -
> -/*
> - * Prepares to enter a system call or exception by saving all registers to the
> - * stack.
> - */
> -       .macro SAVE_ALL
> -       LOCAL _restore_kernel_tpsp
> -       LOCAL _save_context
> +#if !IS_ENABLED(CONFIG_PREEMPTION)
> +.set resume_kernel, restore_all
> +#endif
>
> +ENTRY(handle_exception)
>         /*
>          * If coming from userspace, preserve the user thread pointer and load
>          * the kernel thread pointer.  If we came from the kernel, the scratch
> @@ -90,77 +84,6 @@ _save_context:
>         REG_S s3, PT_BADADDR(sp)
>         REG_S s4, PT_CAUSE(sp)
>         REG_S s5, PT_TP(sp)
> -       .endm
> -
> -/*
> - * Prepares to return from a system call or exception by restoring all
> - * registers from the stack.
> - */
> -       .macro RESTORE_ALL
> -       REG_L a0, PT_STATUS(sp)
> -       /*
> -        * The current load reservation is effectively part of the processor's
> -        * state, in the sense that load reservations cannot be shared between
> -        * different hart contexts.  We can't actually save and restore a load
> -        * reservation, so instead here we clear any existing reservation --
> -        * it's always legal for implementations to clear load reservations at
> -        * any point (as long as the forward progress guarantee is kept, but
> -        * we'll ignore that here).
> -        *
> -        * Dangling load reservations can be the result of taking a trap in the
> -        * middle of an LR/SC sequence, but can also be the result of a taken
> -        * forward branch around an SC -- which is how we implement CAS.  As a
> -        * result we need to clear reservations between the last CAS and the
> -        * jump back to the new context.  While it is unlikely the store
> -        * completes, implementations are allowed to expand reservations to be
> -        * arbitrarily large.
> -        */
> -       REG_L  a2, PT_EPC(sp)
> -       REG_SC x0, a2, PT_EPC(sp)
> -
> -       csrw CSR_STATUS, a0
> -       csrw CSR_EPC, a2
> -
> -       REG_L x1,  PT_RA(sp)
> -       REG_L x3,  PT_GP(sp)
> -       REG_L x4,  PT_TP(sp)
> -       REG_L x5,  PT_T0(sp)
> -       REG_L x6,  PT_T1(sp)
> -       REG_L x7,  PT_T2(sp)
> -       REG_L x8,  PT_S0(sp)
> -       REG_L x9,  PT_S1(sp)
> -       REG_L x10, PT_A0(sp)
> -       REG_L x11, PT_A1(sp)
> -       REG_L x12, PT_A2(sp)
> -       REG_L x13, PT_A3(sp)
> -       REG_L x14, PT_A4(sp)
> -       REG_L x15, PT_A5(sp)
> -       REG_L x16, PT_A6(sp)
> -       REG_L x17, PT_A7(sp)
> -       REG_L x18, PT_S2(sp)
> -       REG_L x19, PT_S3(sp)
> -       REG_L x20, PT_S4(sp)
> -       REG_L x21, PT_S5(sp)
> -       REG_L x22, PT_S6(sp)
> -       REG_L x23, PT_S7(sp)
> -       REG_L x24, PT_S8(sp)
> -       REG_L x25, PT_S9(sp)
> -       REG_L x26, PT_S10(sp)
> -       REG_L x27, PT_S11(sp)
> -       REG_L x28, PT_T3(sp)
> -       REG_L x29, PT_T4(sp)
> -       REG_L x30, PT_T5(sp)
> -       REG_L x31, PT_T6(sp)
> -
> -       REG_L x2,  PT_SP(sp)
> -       .endm
> -
> -#if !IS_ENABLED(CONFIG_PREEMPTION)
> -.set resume_kernel, restore_all
> -#endif
> -
> -ENTRY(handle_exception)
> -       SAVE_ALL
>
>         /*
>          * Set the scratch register to 0, so that if a recursive exception
> @@ -298,7 +221,63 @@ resume_userspace:
>         csrw CSR_SCRATCH, tp
>
>  restore_all:
> -       RESTORE_ALL
> +       REG_L a0, PT_STATUS(sp)
> +       /*
> +        * The current load reservation is effectively part of the processor's
> +        * state, in the sense that load reservations cannot be shared between
> +        * different hart contexts.  We can't actually save and restore a load
> +        * reservation, so instead here we clear any existing reservation --
> +        * it's always legal for implementations to clear load reservations at
> +        * any point (as long as the forward progress guarantee is kept, but
> +        * we'll ignore that here).
> +        *
> +        * Dangling load reservations can be the result of taking a trap in the
> +        * middle of an LR/SC sequence, but can also be the result of a taken
> +        * forward branch around an SC -- which is how we implement CAS.  As a
> +        * result we need to clear reservations between the last CAS and the
> +        * jump back to the new context.  While it is unlikely the store
> +        * completes, implementations are allowed to expand reservations to be
> +        * arbitrarily large.
> +        */
> +       REG_L  a2, PT_EPC(sp)
> +       REG_SC x0, a2, PT_EPC(sp)
> +
> +       csrw CSR_STATUS, a0
> +       csrw CSR_EPC, a2
> +
> +       REG_L x1,  PT_RA(sp)
> +       REG_L x3,  PT_GP(sp)
> +       REG_L x4,  PT_TP(sp)
> +       REG_L x5,  PT_T0(sp)
> +       REG_L x6,  PT_T1(sp)
> +       REG_L x7,  PT_T2(sp)
> +       REG_L x8,  PT_S0(sp)
> +       REG_L x9,  PT_S1(sp)
> +       REG_L x10, PT_A0(sp)
> +       REG_L x11, PT_A1(sp)
> +       REG_L x12, PT_A2(sp)
> +       REG_L x13, PT_A3(sp)
> +       REG_L x14, PT_A4(sp)
> +       REG_L x15, PT_A5(sp)
> +       REG_L x16, PT_A6(sp)
> +       REG_L x17, PT_A7(sp)
> +       REG_L x18, PT_S2(sp)
> +       REG_L x19, PT_S3(sp)
> +       REG_L x20, PT_S4(sp)
> +       REG_L x21, PT_S5(sp)
> +       REG_L x22, PT_S6(sp)
> +       REG_L x23, PT_S7(sp)
> +       REG_L x24, PT_S8(sp)
> +       REG_L x25, PT_S9(sp)
> +       REG_L x26, PT_S10(sp)
> +       REG_L x27, PT_S11(sp)
> +       REG_L x28, PT_T3(sp)
> +       REG_L x29, PT_T4(sp)
> +       REG_L x30, PT_T5(sp)
> +       REG_L x31, PT_T6(sp)
> +
> +       REG_L x2,  PT_SP(sp)
> +
>  #ifdef CONFIG_RISCV_M_MODE
>         mret
>  #else
> --
> 2.25.1.481.gfbce0eb801-goog
>
> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20200227213450.87194-3-palmer%40dabbelt.com.



-- 
Thanks,
~Nick Desaulniers

Re: RISC-V: Fix the build on LLVM-based toolchains

[Nick Desaulniers]

On Thu, Feb 27, 2020 at 1:35 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> The RISC-V LLVM port has progressed to the point where it should be able to use
> it to compile Linux.  Unfortunately we ended up with a few GNU-isms in our port
> so that doesn't work out of the box, but I don't think the code without them is
> any uglier than the code with them so I'm happy to support both toolchains.
> There are still some issues using the GNU assembler to compile clang's assembly
> (at least got_pcrel_hi, but there may be others).  I'm going to call those
> binutils bugs, though, and chase them around over there.
>
> While the first one could be considered a bug fix, I think the bug is unlikely
> enough to manifst that I'm going to wait for the merge window for these.  I'm
> going to preemptively drop them on for-next now, but as I haven't really
> started building that branch they'll be rebased (my current plan is to start
> taking 5.7 patches on top of rc4, as it seems like things are shaping up to be
> fairly solid on our end).  If there are any comments I'll handle them as part
> of the rebase, but I'd like the various autobuilders to start chewing on these.
>
> Unfortunately the kernel compiled with LLVM doesn't boot for me.

Thanks for the series! In general, our approach for bringing various
architectures online has been:
1. get it building
2. get it booting
3. get it running well

For most architectures, 1 included 2 (per chance).  Mips was a notable
case of 1 not including 2 due to undefined behavior we found and
removed.  There's always a chance of compiler bugs, too. With the
above series, we should now be able to start digging into 2.

-- 
Thanks,
~Nick Desaulniers

Re: [PATCH 3/3] RISC-V: Stop using LOCAL for the uaccess fixups

[Palmer Dabbelt]

On Thu, 27 Feb 2020 15:03:42 PST (-0800), Nick Desaulniers wrote:
> On Thu, Feb 27, 2020 at 1:35 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>>
>> From: Palmer Dabbelt <palmerdabbelt@google.com>
>>
>> LLVM's integrated assembler doesn't support the LOCAL directive, which we're
>> using when generating our uaccess fixup tables.  Luckily the table fragment is
>> small enough that there's only one internal symbol, so using a relative symbol
>> reference doesn't really complicate anything.
>
> Is `LOCAL` a macro for `.local`? (Looks like no). I would think
> `.local` is supported, but `LOCAL` isn't something I've seen before.
>
> Ah, looks like it's local to macros:
> https://sourceware.org/binutils/docs/as/Macro.html#Macro
> "Warning: LOCAL is only available if you select “alternate macro
> syntax” with ‘--alternate’ or .altmacro. See .altmacro."
> https://sourceware.org/binutils/docs/as/Altmacro.html#Altmacro
> Link: https://sourceware.org/binutils/docs/as/Local.html#Local
>
> But these macros are setting .altmacro...
> So it looks like Clang's integrated assembler doesn't yet support
> `LOCAL`. Filed:
> https://bugs.llvm.org/show_bug.cgi?id=45051
>
> If we're no longer using LOCAL, do we still need `.altmacro`?
>
> I also see two usages in:
> arch/riscv/kernel/entry.S
>
> Would you mind fixing those up, too?

Done.

>
> Otherwise patch LGTM.
>
>>
>> Signed-off-by: Palmer Dabbelt <palmerdabbelt@google.com>
>> ---
>>  arch/riscv/lib/uaccess.S | 5 ++---
>>  1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/riscv/lib/uaccess.S b/arch/riscv/lib/uaccess.S
>> index f29d2ba2c0a6..40bf130073e8 100644
>> --- a/arch/riscv/lib/uaccess.S
>> +++ b/arch/riscv/lib/uaccess.S
>> @@ -5,12 +5,11 @@
>>
>>         .altmacro
>>         .macro fixup op reg addr lbl
>> -       LOCAL _epc
>> -_epc:
>> +100:
>>         \op \reg, \addr
>>         .section __ex_table,"a"
>>         .balign RISCV_SZPTR
>> -       RISCV_PTR _epc, \lbl
>> +       RISCV_PTR 100b, \lbl
>>         .previous
>>         .endm
>>
>> --
>
> -- 
> Thanks,
> ~Nick Desaulniers

Re: RISC-V: Fix the build on LLVM-based toolchains

[Palmer Dabbelt]

On Fri, 28 Feb 2020 15:31:13 PST (-0800), Nick Desaulniers wrote:
> On Thu, Feb 27, 2020 at 1:35 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>>
>> The RISC-V LLVM port has progressed to the point where it should be able to use
>> it to compile Linux.  Unfortunately we ended up with a few GNU-isms in our port
>> so that doesn't work out of the box, but I don't think the code without them is
>> any uglier than the code with them so I'm happy to support both toolchains.
>> There are still some issues using the GNU assembler to compile clang's assembly
>> (at least got_pcrel_hi, but there may be others).  I'm going to call those
>> binutils bugs, though, and chase them around over there.
>>
>> While the first one could be considered a bug fix, I think the bug is unlikely
>> enough to manifst that I'm going to wait for the merge window for these.  I'm
>> going to preemptively drop them on for-next now, but as I haven't really
>> started building that branch they'll be rebased (my current plan is to start
>> taking 5.7 patches on top of rc4, as it seems like things are shaping up to be
>> fairly solid on our end).  If there are any comments I'll handle them as part
>> of the rebase, but I'd like the various autobuilders to start chewing on these.
>>
>> Unfortunately the kernel compiled with LLVM doesn't boot for me.
>
> Thanks for the series! In general, our approach for bringing various
> architectures online has been:
> 1. get it building
> 2. get it booting
> 3. get it running well
>
> For most architectures, 1 included 2 (per chance).  Mips was a notable
> case of 1 not including 2 due to undefined behavior we found and
> removed.  There's always a chance of compiler bugs, too. With the
> above series, we should now be able to start digging into 2.

Thanks!  LMK if you have any issues, but from looking at the bug it should be
fairly straight-forward: init isn't being run, returning some bogus error like
file not found.  I wouldn't be surprised if we had some undefined behavior
(maybe ubsan runs would be a good idea?), but I also wouldn't be surprised if
it's a compiler issue given that LLVM is pretty new and a lot of these bogus
error returns can come from reasonable backend bugs (shortcutting some type
conversion due to some instruction pattern issue, for example).

Hopefully it's just a small issue... :)

Re: [PATCH 1/3] RISC-V: Stop relying on GCC's register allocator's hueristics

[Maciej W. Rozycki]

On Thu, 27 Feb 2020, Nick Desaulniers wrote:

> Indeed, looks like the local register variables are very much intended
> to be used as inputs & outputs to extended inline assembly, which in
> these cases are not.
> Link: https://gcc.gnu.org/onlinedocs/gcc/Local-Register-Variables.html

 There is a further restriction on local register variables causing them 
to be possibly clobbered as a result of a function call, which may happen 
at unusual places due to inlining.

 For an example of this happening see a glibc bug recently reported here: 
<https://sourceware.org/bugzilla/show_bug.cgi?id=25523> and the cascade of 
fixes following, including one for the RISC-V target.

  Maciej