| Message ID | 20240826105737.106879-1-alexghiti@rivosinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [-fixes] riscv: Fix RISCV_ALTERNATIVE_EARLY | expand |
Hi Alex, On Mon, 26 Aug 2024 at 18:58, Alexandre Ghiti <alexghiti@rivosinc.com> wrote: > > RISCV_ALTERNATIVE_EARLY will issue sbi_ecall() very early in the boot > process, before the first memory mapping is setup so we can't have any > instrumentation happening here. I also found that when CONFIG_KASAN is enabled, and either RISCV_ALTERNATIVE_EARLY or CONFIG_DT_IDLE_GENPD is set, the kernel cannot boot. This patch fixed the issue. Tested-by: Chunyan Zhang <zhangchunyan@iscas.ac.cn> > > In addition, when the kernel is relocatable, we must also not issue any > relocation this early since they would have been patched virtually only. > > So, instead of disabling instrumentation for the whole kernel/sbi.c file > and compiling it with -fno-pie, simply move __sbi_ecall() and > __sbi_base_ecall() into their own file where this is fixed. > > Fixes: 1745cfafebdf ("riscv: don't use global static vars to store alternative data") > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com> > --- > arch/riscv/include/asm/sbi.h | 2 ++ > arch/riscv/kernel/Makefile | 6 ++++- > arch/riscv/kernel/sbi.c | 44 -------------------------------- > arch/riscv/kernel/sbi_ecall.c | 48 +++++++++++++++++++++++++++++++++++ > 4 files changed, 55 insertions(+), 45 deletions(-) > create mode 100644 arch/riscv/kernel/sbi_ecall.c > > diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h > index 7cffd4ffecd0..5843a10b380e 100644 > --- a/arch/riscv/include/asm/sbi.h > +++ b/arch/riscv/include/asm/sbi.h > @@ -9,6 +9,7 @@ > > #include <linux/types.h> > #include <linux/cpumask.h> > +#include <linux/jump_label.h> > > #ifdef CONFIG_RISCV_SBI > enum sbi_ext_id { > @@ -304,6 +305,7 @@ struct sbiret { > }; > > void sbi_init(void); > +long __sbi_base_ecall(int fid); > struct sbiret __sbi_ecall(unsigned long arg0, unsigned long arg1, > unsigned long arg2, unsigned long arg3, > unsigned long arg4, unsigned long arg5, > diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile > index 06d407f1b30b..7f88cc4931f5 100644 > --- a/arch/riscv/kernel/Makefile > +++ b/arch/riscv/kernel/Makefile > @@ -20,17 +20,21 @@ endif > ifdef CONFIG_RISCV_ALTERNATIVE_EARLY > CFLAGS_alternative.o := -mcmodel=medany > CFLAGS_cpufeature.o := -mcmodel=medany > +CFLAGS_sbi_ecall.o := -mcmodel=medany > ifdef CONFIG_FTRACE > CFLAGS_REMOVE_alternative.o = $(CC_FLAGS_FTRACE) > CFLAGS_REMOVE_cpufeature.o = $(CC_FLAGS_FTRACE) > +CFLAGS_REMOVE_sbi_ecall.o = $(CC_FLAGS_FTRACE) > endif > ifdef CONFIG_RELOCATABLE > CFLAGS_alternative.o += -fno-pie > CFLAGS_cpufeature.o += -fno-pie > +CFLAGS_sbi_ecall.o += -fno-pie > endif > ifdef CONFIG_KASAN > KASAN_SANITIZE_alternative.o := n > KASAN_SANITIZE_cpufeature.o := n > +KASAN_SANITIZE_sbi_ecall.o := n > endif > endif > > @@ -88,7 +92,7 @@ obj-$(CONFIG_DYNAMIC_FTRACE) += mcount-dyn.o > > obj-$(CONFIG_PERF_EVENTS) += perf_callchain.o > obj-$(CONFIG_HAVE_PERF_REGS) += perf_regs.o > -obj-$(CONFIG_RISCV_SBI) += sbi.o > +obj-$(CONFIG_RISCV_SBI) += sbi.o sbi_ecall.o > ifeq ($(CONFIG_RISCV_SBI), y) > obj-$(CONFIG_SMP) += sbi-ipi.o > obj-$(CONFIG_SMP) += cpu_ops_sbi.o > diff --git a/arch/riscv/kernel/sbi.c b/arch/riscv/kernel/sbi.c > index 837bdab2601b..ace9e2f59c41 100644 > --- a/arch/riscv/kernel/sbi.c > +++ b/arch/riscv/kernel/sbi.c > @@ -14,9 +14,6 @@ > #include <asm/smp.h> > #include <asm/tlbflush.h> > > -#define CREATE_TRACE_POINTS > -#include <asm/trace.h> > - > /* default SBI version is 0.1 */ > unsigned long sbi_spec_version __ro_after_init = SBI_SPEC_VERSION_DEFAULT; > EXPORT_SYMBOL(sbi_spec_version); > @@ -27,36 +24,6 @@ static int (*__sbi_rfence)(int fid, const struct cpumask *cpu_mask, > unsigned long start, unsigned long size, > unsigned long arg4, unsigned long arg5) __ro_after_init; > > -struct sbiret __sbi_ecall(unsigned long arg0, unsigned long arg1, > - unsigned long arg2, unsigned long arg3, > - unsigned long arg4, unsigned long arg5, > - int fid, int ext) > -{ > - struct sbiret ret; > - > - trace_sbi_call(ext, fid); > - > - register uintptr_t a0 asm ("a0") = (uintptr_t)(arg0); > - register uintptr_t a1 asm ("a1") = (uintptr_t)(arg1); > - register uintptr_t a2 asm ("a2") = (uintptr_t)(arg2); > - register uintptr_t a3 asm ("a3") = (uintptr_t)(arg3); > - register uintptr_t a4 asm ("a4") = (uintptr_t)(arg4); > - register uintptr_t a5 asm ("a5") = (uintptr_t)(arg5); > - register uintptr_t a6 asm ("a6") = (uintptr_t)(fid); > - register uintptr_t a7 asm ("a7") = (uintptr_t)(ext); > - asm volatile ("ecall" > - : "+r" (a0), "+r" (a1) > - : "r" (a2), "r" (a3), "r" (a4), "r" (a5), "r" (a6), "r" (a7) > - : "memory"); > - ret.error = a0; > - ret.value = a1; > - > - trace_sbi_return(ext, ret.error, ret.value); > - > - return ret; > -} > -EXPORT_SYMBOL(__sbi_ecall); > - > int sbi_err_map_linux_errno(int err) > { > switch (err) { > @@ -535,17 +502,6 @@ long sbi_probe_extension(int extid) > } > EXPORT_SYMBOL(sbi_probe_extension); > > -static long __sbi_base_ecall(int fid) > -{ > - struct sbiret ret; > - > - ret = sbi_ecall(SBI_EXT_BASE, fid, 0, 0, 0, 0, 0, 0); > - if (!ret.error) > - return ret.value; > - else > - return sbi_err_map_linux_errno(ret.error); > -} > - > static inline long sbi_get_spec_version(void) > { > return __sbi_base_ecall(SBI_EXT_BASE_GET_SPEC_VERSION); > diff --git a/arch/riscv/kernel/sbi_ecall.c b/arch/riscv/kernel/sbi_ecall.c > new file mode 100644 > index 000000000000..24aabb4fbde3 > --- /dev/null > +++ b/arch/riscv/kernel/sbi_ecall.c > @@ -0,0 +1,48 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* Copyright (c) 2024 Rivos Inc. */ > + > +#include <asm/sbi.h> > +#define CREATE_TRACE_POINTS > +#include <asm/trace.h> > + > +long __sbi_base_ecall(int fid) > +{ > + struct sbiret ret; > + > + ret = sbi_ecall(SBI_EXT_BASE, fid, 0, 0, 0, 0, 0, 0); > + if (!ret.error) > + return ret.value; > + else > + return sbi_err_map_linux_errno(ret.error); > +} > +EXPORT_SYMBOL(__sbi_base_ecall); > + > +struct sbiret __sbi_ecall(unsigned long arg0, unsigned long arg1, > + unsigned long arg2, unsigned long arg3, > + unsigned long arg4, unsigned long arg5, > + int fid, int ext) > +{ > + struct sbiret ret; > + > + trace_sbi_call(ext, fid); > + > + register uintptr_t a0 asm ("a0") = (uintptr_t)(arg0); > + register uintptr_t a1 asm ("a1") = (uintptr_t)(arg1); > + register uintptr_t a2 asm ("a2") = (uintptr_t)(arg2); > + register uintptr_t a3 asm ("a3") = (uintptr_t)(arg3); > + register uintptr_t a4 asm ("a4") = (uintptr_t)(arg4); > + register uintptr_t a5 asm ("a5") = (uintptr_t)(arg5); > + register uintptr_t a6 asm ("a6") = (uintptr_t)(fid); > + register uintptr_t a7 asm ("a7") = (uintptr_t)(ext); > + asm volatile ("ecall" > + : "+r" (a0), "+r" (a1) > + : "r" (a2), "r" (a3), "r" (a4), "r" (a5), "r" (a6), "r" (a7) > + : "memory"); > + ret.error = a0; > + ret.value = a1; > + > + trace_sbi_return(ext, ret.error, ret.value); > + > + return ret; > +} > +EXPORT_SYMBOL(__sbi_ecall); > -- > 2.39.2 > >
Hi Chunyan, On 27/08/2024 06:57, Chunyan Zhang wrote: > Hi Alex, > > On Mon, 26 Aug 2024 at 18:58, Alexandre Ghiti <alexghiti@rivosinc.com> wrote: >> RISCV_ALTERNATIVE_EARLY will issue sbi_ecall() very early in the boot >> process, before the first memory mapping is setup so we can't have any >> instrumentation happening here. > I also found that when CONFIG_KASAN is enabled, and either > RISCV_ALTERNATIVE_EARLY or CONFIG_DT_IDLE_GENPD is set, the kernel > cannot boot. Yes, I was initially fixing this report: https://lore.kernel.org/linux-riscv/00000000000065062c061fcec37b@google.com/ And that makes me think I forgot the syzbot tag: Reported-by: syzbot+cfbcb82adf6d7279fd35@syzkaller.appspotmail.com > > This patch fixed the issue. > > Tested-by: Chunyan Zhang <zhangchunyan@iscas.ac.cn> Thanks! Alex > >> In addition, when the kernel is relocatable, we must also not issue any >> relocation this early since they would have been patched virtually only. >> >> So, instead of disabling instrumentation for the whole kernel/sbi.c file >> and compiling it with -fno-pie, simply move __sbi_ecall() and >> __sbi_base_ecall() into their own file where this is fixed. >> >> Fixes: 1745cfafebdf ("riscv: don't use global static vars to store alternative data") >> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com> >> --- >> arch/riscv/include/asm/sbi.h | 2 ++ >> arch/riscv/kernel/Makefile | 6 ++++- >> arch/riscv/kernel/sbi.c | 44 -------------------------------- >> arch/riscv/kernel/sbi_ecall.c | 48 +++++++++++++++++++++++++++++++++++ >> 4 files changed, 55 insertions(+), 45 deletions(-) >> create mode 100644 arch/riscv/kernel/sbi_ecall.c >> >> diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h >> index 7cffd4ffecd0..5843a10b380e 100644 >> --- a/arch/riscv/include/asm/sbi.h >> +++ b/arch/riscv/include/asm/sbi.h >> @@ -9,6 +9,7 @@ >> >> #include <linux/types.h> >> #include <linux/cpumask.h> >> +#include <linux/jump_label.h> >> >> #ifdef CONFIG_RISCV_SBI >> enum sbi_ext_id { >> @@ -304,6 +305,7 @@ struct sbiret { >> }; >> >> void sbi_init(void); >> +long __sbi_base_ecall(int fid); >> struct sbiret __sbi_ecall(unsigned long arg0, unsigned long arg1, >> unsigned long arg2, unsigned long arg3, >> unsigned long arg4, unsigned long arg5, >> diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile >> index 06d407f1b30b..7f88cc4931f5 100644 >> --- a/arch/riscv/kernel/Makefile >> +++ b/arch/riscv/kernel/Makefile >> @@ -20,17 +20,21 @@ endif >> ifdef CONFIG_RISCV_ALTERNATIVE_EARLY >> CFLAGS_alternative.o := -mcmodel=medany >> CFLAGS_cpufeature.o := -mcmodel=medany >> +CFLAGS_sbi_ecall.o := -mcmodel=medany >> ifdef CONFIG_FTRACE >> CFLAGS_REMOVE_alternative.o = $(CC_FLAGS_FTRACE) >> CFLAGS_REMOVE_cpufeature.o = $(CC_FLAGS_FTRACE) >> +CFLAGS_REMOVE_sbi_ecall.o = $(CC_FLAGS_FTRACE) >> endif >> ifdef CONFIG_RELOCATABLE >> CFLAGS_alternative.o += -fno-pie >> CFLAGS_cpufeature.o += -fno-pie >> +CFLAGS_sbi_ecall.o += -fno-pie >> endif >> ifdef CONFIG_KASAN >> KASAN_SANITIZE_alternative.o := n >> KASAN_SANITIZE_cpufeature.o := n >> +KASAN_SANITIZE_sbi_ecall.o := n >> endif >> endif >> >> @@ -88,7 +92,7 @@ obj-$(CONFIG_DYNAMIC_FTRACE) += mcount-dyn.o >> >> obj-$(CONFIG_PERF_EVENTS) += perf_callchain.o >> obj-$(CONFIG_HAVE_PERF_REGS) += perf_regs.o >> -obj-$(CONFIG_RISCV_SBI) += sbi.o >> +obj-$(CONFIG_RISCV_SBI) += sbi.o sbi_ecall.o >> ifeq ($(CONFIG_RISCV_SBI), y) >> obj-$(CONFIG_SMP) += sbi-ipi.o >> obj-$(CONFIG_SMP) += cpu_ops_sbi.o >> diff --git a/arch/riscv/kernel/sbi.c b/arch/riscv/kernel/sbi.c >> index 837bdab2601b..ace9e2f59c41 100644 >> --- a/arch/riscv/kernel/sbi.c >> +++ b/arch/riscv/kernel/sbi.c >> @@ -14,9 +14,6 @@ >> #include <asm/smp.h> >> #include <asm/tlbflush.h> >> >> -#define CREATE_TRACE_POINTS >> -#include <asm/trace.h> >> - >> /* default SBI version is 0.1 */ >> unsigned long sbi_spec_version __ro_after_init = SBI_SPEC_VERSION_DEFAULT; >> EXPORT_SYMBOL(sbi_spec_version); >> @@ -27,36 +24,6 @@ static int (*__sbi_rfence)(int fid, const struct cpumask *cpu_mask, >> unsigned long start, unsigned long size, >> unsigned long arg4, unsigned long arg5) __ro_after_init; >> >> -struct sbiret __sbi_ecall(unsigned long arg0, unsigned long arg1, >> - unsigned long arg2, unsigned long arg3, >> - unsigned long arg4, unsigned long arg5, >> - int fid, int ext) >> -{ >> - struct sbiret ret; >> - >> - trace_sbi_call(ext, fid); >> - >> - register uintptr_t a0 asm ("a0") = (uintptr_t)(arg0); >> - register uintptr_t a1 asm ("a1") = (uintptr_t)(arg1); >> - register uintptr_t a2 asm ("a2") = (uintptr_t)(arg2); >> - register uintptr_t a3 asm ("a3") = (uintptr_t)(arg3); >> - register uintptr_t a4 asm ("a4") = (uintptr_t)(arg4); >> - register uintptr_t a5 asm ("a5") = (uintptr_t)(arg5); >> - register uintptr_t a6 asm ("a6") = (uintptr_t)(fid); >> - register uintptr_t a7 asm ("a7") = (uintptr_t)(ext); >> - asm volatile ("ecall" >> - : "+r" (a0), "+r" (a1) >> - : "r" (a2), "r" (a3), "r" (a4), "r" (a5), "r" (a6), "r" (a7) >> - : "memory"); >> - ret.error = a0; >> - ret.value = a1; >> - >> - trace_sbi_return(ext, ret.error, ret.value); >> - >> - return ret; >> -} >> -EXPORT_SYMBOL(__sbi_ecall); >> - >> int sbi_err_map_linux_errno(int err) >> { >> switch (err) { >> @@ -535,17 +502,6 @@ long sbi_probe_extension(int extid) >> } >> EXPORT_SYMBOL(sbi_probe_extension); >> >> -static long __sbi_base_ecall(int fid) >> -{ >> - struct sbiret ret; >> - >> - ret = sbi_ecall(SBI_EXT_BASE, fid, 0, 0, 0, 0, 0, 0); >> - if (!ret.error) >> - return ret.value; >> - else >> - return sbi_err_map_linux_errno(ret.error); >> -} >> - >> static inline long sbi_get_spec_version(void) >> { >> return __sbi_base_ecall(SBI_EXT_BASE_GET_SPEC_VERSION); >> diff --git a/arch/riscv/kernel/sbi_ecall.c b/arch/riscv/kernel/sbi_ecall.c >> new file mode 100644 >> index 000000000000..24aabb4fbde3 >> --- /dev/null >> +++ b/arch/riscv/kernel/sbi_ecall.c >> @@ -0,0 +1,48 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* Copyright (c) 2024 Rivos Inc. */ >> + >> +#include <asm/sbi.h> >> +#define CREATE_TRACE_POINTS >> +#include <asm/trace.h> >> + >> +long __sbi_base_ecall(int fid) >> +{ >> + struct sbiret ret; >> + >> + ret = sbi_ecall(SBI_EXT_BASE, fid, 0, 0, 0, 0, 0, 0); >> + if (!ret.error) >> + return ret.value; >> + else >> + return sbi_err_map_linux_errno(ret.error); >> +} >> +EXPORT_SYMBOL(__sbi_base_ecall); >> + >> +struct sbiret __sbi_ecall(unsigned long arg0, unsigned long arg1, >> + unsigned long arg2, unsigned long arg3, >> + unsigned long arg4, unsigned long arg5, >> + int fid, int ext) >> +{ >> + struct sbiret ret; >> + >> + trace_sbi_call(ext, fid); >> + >> + register uintptr_t a0 asm ("a0") = (uintptr_t)(arg0); >> + register uintptr_t a1 asm ("a1") = (uintptr_t)(arg1); >> + register uintptr_t a2 asm ("a2") = (uintptr_t)(arg2); >> + register uintptr_t a3 asm ("a3") = (uintptr_t)(arg3); >> + register uintptr_t a4 asm ("a4") = (uintptr_t)(arg4); >> + register uintptr_t a5 asm ("a5") = (uintptr_t)(arg5); >> + register uintptr_t a6 asm ("a6") = (uintptr_t)(fid); >> + register uintptr_t a7 asm ("a7") = (uintptr_t)(ext); >> + asm volatile ("ecall" >> + : "+r" (a0), "+r" (a1) >> + : "r" (a2), "r" (a3), "r" (a4), "r" (a5), "r" (a6), "r" (a7) >> + : "memory"); >> + ret.error = a0; >> + ret.value = a1; >> + >> + trace_sbi_return(ext, ret.error, ret.value); >> + >> + return ret; >> +} >> +EXPORT_SYMBOL(__sbi_ecall); >> -- >> 2.39.2 >> >> > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
On Mon, Aug 26, 2024 at 12:57:37PM +0200, Alexandre Ghiti wrote: > RISCV_ALTERNATIVE_EARLY will issue sbi_ecall() very early in the boot > process, before the first memory mapping is setup so we can't have any > instrumentation happening here. > > In addition, when the kernel is relocatable, we must also not issue any > relocation this early since they would have been patched virtually only. > > So, instead of disabling instrumentation for the whole kernel/sbi.c file > and compiling it with -fno-pie, simply move __sbi_ecall() and > __sbi_base_ecall() into their own file where this is fixed. IOW, this should fix the issue that we discussed here https://lore.kernel.org/linux-riscv/abec162e-f3f2-488c-83d9-be17257a5df8@ghiti.fr/ also? I'm sorry I didn't get to test that yet, we had some pretty bad IT issues in the office the last weeks and I have been avoiding going there. I'll try to test this one instead..
Hi Conor, On 27/08/2024 10:38, Conor Dooley wrote: > On Mon, Aug 26, 2024 at 12:57:37PM +0200, Alexandre Ghiti wrote: >> RISCV_ALTERNATIVE_EARLY will issue sbi_ecall() very early in the boot >> process, before the first memory mapping is setup so we can't have any >> instrumentation happening here. >> >> In addition, when the kernel is relocatable, we must also not issue any >> relocation this early since they would have been patched virtually only. >> >> So, instead of disabling instrumentation for the whole kernel/sbi.c file >> and compiling it with -fno-pie, simply move __sbi_ecall() and >> __sbi_base_ecall() into their own file where this is fixed. > IOW, this should fix the issue that we discussed here > https://lore.kernel.org/linux-riscv/abec162e-f3f2-488c-83d9-be17257a5df8@ghiti.fr/ > also? Yes, as a side effect it also fixes TRACEPOINTS + [KASAN|RELOCATABLE] as I reported in this thread since I moved the tracepoints in a file where instrumentation is disabled and compiled with no-pie. > I'm sorry I didn't get to test that yet, we had some pretty bad IT > issues in the office the last weeks and I have been avoiding going > there. I'll try to test this one instead.. > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
On Tue, Aug 27, 2024 at 11:13:16AM +0200, Alexandre Ghiti wrote: > Hi Conor, > > On 27/08/2024 10:38, Conor Dooley wrote: > > On Mon, Aug 26, 2024 at 12:57:37PM +0200, Alexandre Ghiti wrote: > > > RISCV_ALTERNATIVE_EARLY will issue sbi_ecall() very early in the boot > > > process, before the first memory mapping is setup so we can't have any > > > instrumentation happening here. > > > > > > In addition, when the kernel is relocatable, we must also not issue any > > > relocation this early since they would have been patched virtually only. > > > > > > So, instead of disabling instrumentation for the whole kernel/sbi.c file > > > and compiling it with -fno-pie, simply move __sbi_ecall() and > > > __sbi_base_ecall() into their own file where this is fixed. > > IOW, this should fix the issue that we discussed here > > https://lore.kernel.org/linux-riscv/abec162e-f3f2-488c-83d9-be17257a5df8@ghiti.fr/ > > also? > > > Yes, as a side effect it also fixes TRACEPOINTS + [KASAN|RELOCATABLE] as I > reported in this thread since I moved the tracepoints in a file where > instrumentation is disabled and compiled with no-pie. And it does, so Closes: https://lore.kernel.org/linux-riscv/20240813-pony-truck-3e7a83e9759e@spud/ Reported-by: Conor Dooley <conor.dooley@microchip.com> Tested-by: Conor Dooley <conor.dooley@microchip.com> Cheers, Conor.
Hi Alex, On 2024-08-26 5:57 AM, Alexandre Ghiti wrote: > RISCV_ALTERNATIVE_EARLY will issue sbi_ecall() very early in the boot > process, before the first memory mapping is setup so we can't have any > instrumentation happening here. > > In addition, when the kernel is relocatable, we must also not issue any > relocation this early since they would have been patched virtually only. > > So, instead of disabling instrumentation for the whole kernel/sbi.c file > and compiling it with -fno-pie, simply move __sbi_ecall() and > __sbi_base_ecall() into their own file where this is fixed. Looking at the baseline disassembly from both LLVM 19 and GCC 13.2.0 with RISCV_ALTERNATIVE_EARLY + KASAN + TRACEPOINTS, all of the instrumentation in __sbi_ecall() itself is out of line and only executed when the tracepoint static branches are enabled. However, there is instrumentation in sbi_get_m*id() from the switch table inlined from sbi_err_map_linux_errno(), and some of those memory accesses are done unconditionally. This change will force sbi_err_map_linux_errno() to be out of line (unless LTO is enabled), so it also forces that particular bit of instrumentation to be executed only in the error path. But we could still crash in the error path. So I think sbi_err_map_linux_errno() needs to be moved to sbi_ecall.c as well. Alternatively, sbi_get_m*id() do not have any errors defined (and we don't really handle that possibility), so we could drop to the call to sbi_err_map_linux_errno() from __sbi_base_ecall(). Regards, Samuel > Fixes: 1745cfafebdf ("riscv: don't use global static vars to store alternative data") > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com> > --- > arch/riscv/include/asm/sbi.h | 2 ++ > arch/riscv/kernel/Makefile | 6 ++++- > arch/riscv/kernel/sbi.c | 44 -------------------------------- > arch/riscv/kernel/sbi_ecall.c | 48 +++++++++++++++++++++++++++++++++++ > 4 files changed, 55 insertions(+), 45 deletions(-) > create mode 100644 arch/riscv/kernel/sbi_ecall.c > > diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h > index 7cffd4ffecd0..5843a10b380e 100644 > --- a/arch/riscv/include/asm/sbi.h > +++ b/arch/riscv/include/asm/sbi.h > @@ -9,6 +9,7 @@ > > #include <linux/types.h> > #include <linux/cpumask.h> > +#include <linux/jump_label.h> > > #ifdef CONFIG_RISCV_SBI > enum sbi_ext_id { > @@ -304,6 +305,7 @@ struct sbiret { > }; > > void sbi_init(void); > +long __sbi_base_ecall(int fid); > struct sbiret __sbi_ecall(unsigned long arg0, unsigned long arg1, > unsigned long arg2, unsigned long arg3, > unsigned long arg4, unsigned long arg5, > diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile > index 06d407f1b30b..7f88cc4931f5 100644 > --- a/arch/riscv/kernel/Makefile > +++ b/arch/riscv/kernel/Makefile > @@ -20,17 +20,21 @@ endif > ifdef CONFIG_RISCV_ALTERNATIVE_EARLY > CFLAGS_alternative.o := -mcmodel=medany > CFLAGS_cpufeature.o := -mcmodel=medany > +CFLAGS_sbi_ecall.o := -mcmodel=medany > ifdef CONFIG_FTRACE > CFLAGS_REMOVE_alternative.o = $(CC_FLAGS_FTRACE) > CFLAGS_REMOVE_cpufeature.o = $(CC_FLAGS_FTRACE) > +CFLAGS_REMOVE_sbi_ecall.o = $(CC_FLAGS_FTRACE) > endif > ifdef CONFIG_RELOCATABLE > CFLAGS_alternative.o += -fno-pie > CFLAGS_cpufeature.o += -fno-pie > +CFLAGS_sbi_ecall.o += -fno-pie > endif > ifdef CONFIG_KASAN > KASAN_SANITIZE_alternative.o := n > KASAN_SANITIZE_cpufeature.o := n > +KASAN_SANITIZE_sbi_ecall.o := n > endif > endif > > @@ -88,7 +92,7 @@ obj-$(CONFIG_DYNAMIC_FTRACE) += mcount-dyn.o > > obj-$(CONFIG_PERF_EVENTS) += perf_callchain.o > obj-$(CONFIG_HAVE_PERF_REGS) += perf_regs.o > -obj-$(CONFIG_RISCV_SBI) += sbi.o > +obj-$(CONFIG_RISCV_SBI) += sbi.o sbi_ecall.o > ifeq ($(CONFIG_RISCV_SBI), y) > obj-$(CONFIG_SMP) += sbi-ipi.o > obj-$(CONFIG_SMP) += cpu_ops_sbi.o > diff --git a/arch/riscv/kernel/sbi.c b/arch/riscv/kernel/sbi.c > index 837bdab2601b..ace9e2f59c41 100644 > --- a/arch/riscv/kernel/sbi.c > +++ b/arch/riscv/kernel/sbi.c > @@ -14,9 +14,6 @@ > #include <asm/smp.h> > #include <asm/tlbflush.h> > > -#define CREATE_TRACE_POINTS > -#include <asm/trace.h> > - > /* default SBI version is 0.1 */ > unsigned long sbi_spec_version __ro_after_init = SBI_SPEC_VERSION_DEFAULT; > EXPORT_SYMBOL(sbi_spec_version); > @@ -27,36 +24,6 @@ static int (*__sbi_rfence)(int fid, const struct cpumask *cpu_mask, > unsigned long start, unsigned long size, > unsigned long arg4, unsigned long arg5) __ro_after_init; > > -struct sbiret __sbi_ecall(unsigned long arg0, unsigned long arg1, > - unsigned long arg2, unsigned long arg3, > - unsigned long arg4, unsigned long arg5, > - int fid, int ext) > -{ > - struct sbiret ret; > - > - trace_sbi_call(ext, fid); > - > - register uintptr_t a0 asm ("a0") = (uintptr_t)(arg0); > - register uintptr_t a1 asm ("a1") = (uintptr_t)(arg1); > - register uintptr_t a2 asm ("a2") = (uintptr_t)(arg2); > - register uintptr_t a3 asm ("a3") = (uintptr_t)(arg3); > - register uintptr_t a4 asm ("a4") = (uintptr_t)(arg4); > - register uintptr_t a5 asm ("a5") = (uintptr_t)(arg5); > - register uintptr_t a6 asm ("a6") = (uintptr_t)(fid); > - register uintptr_t a7 asm ("a7") = (uintptr_t)(ext); > - asm volatile ("ecall" > - : "+r" (a0), "+r" (a1) > - : "r" (a2), "r" (a3), "r" (a4), "r" (a5), "r" (a6), "r" (a7) > - : "memory"); > - ret.error = a0; > - ret.value = a1; > - > - trace_sbi_return(ext, ret.error, ret.value); > - > - return ret; > -} > -EXPORT_SYMBOL(__sbi_ecall); > - > int sbi_err_map_linux_errno(int err) > { > switch (err) { > @@ -535,17 +502,6 @@ long sbi_probe_extension(int extid) > } > EXPORT_SYMBOL(sbi_probe_extension); > > -static long __sbi_base_ecall(int fid) > -{ > - struct sbiret ret; > - > - ret = sbi_ecall(SBI_EXT_BASE, fid, 0, 0, 0, 0, 0, 0); > - if (!ret.error) > - return ret.value; > - else > - return sbi_err_map_linux_errno(ret.error); > -} > - > static inline long sbi_get_spec_version(void) > { > return __sbi_base_ecall(SBI_EXT_BASE_GET_SPEC_VERSION); > diff --git a/arch/riscv/kernel/sbi_ecall.c b/arch/riscv/kernel/sbi_ecall.c > new file mode 100644 > index 000000000000..24aabb4fbde3 > --- /dev/null > +++ b/arch/riscv/kernel/sbi_ecall.c > @@ -0,0 +1,48 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* Copyright (c) 2024 Rivos Inc. */ > + > +#include <asm/sbi.h> > +#define CREATE_TRACE_POINTS > +#include <asm/trace.h> > + > +long __sbi_base_ecall(int fid) > +{ > + struct sbiret ret; > + > + ret = sbi_ecall(SBI_EXT_BASE, fid, 0, 0, 0, 0, 0, 0); > + if (!ret.error) > + return ret.value; > + else > + return sbi_err_map_linux_errno(ret.error); > +} > +EXPORT_SYMBOL(__sbi_base_ecall); > + > +struct sbiret __sbi_ecall(unsigned long arg0, unsigned long arg1, > + unsigned long arg2, unsigned long arg3, > + unsigned long arg4, unsigned long arg5, > + int fid, int ext) > +{ > + struct sbiret ret; > + > + trace_sbi_call(ext, fid); > + > + register uintptr_t a0 asm ("a0") = (uintptr_t)(arg0); > + register uintptr_t a1 asm ("a1") = (uintptr_t)(arg1); > + register uintptr_t a2 asm ("a2") = (uintptr_t)(arg2); > + register uintptr_t a3 asm ("a3") = (uintptr_t)(arg3); > + register uintptr_t a4 asm ("a4") = (uintptr_t)(arg4); > + register uintptr_t a5 asm ("a5") = (uintptr_t)(arg5); > + register uintptr_t a6 asm ("a6") = (uintptr_t)(fid); > + register uintptr_t a7 asm ("a7") = (uintptr_t)(ext); > + asm volatile ("ecall" > + : "+r" (a0), "+r" (a1) > + : "r" (a2), "r" (a3), "r" (a4), "r" (a5), "r" (a6), "r" (a7) > + : "memory"); > + ret.error = a0; > + ret.value = a1; > + > + trace_sbi_return(ext, ret.error, ret.value); > + > + return ret; > +} > +EXPORT_SYMBOL(__sbi_ecall);
Hi Samuel, On Wed, Aug 28, 2024 at 3:08 PM Samuel Holland <samuel.holland@sifive.com> wrote: > > Hi Alex, > > On 2024-08-26 5:57 AM, Alexandre Ghiti wrote: > > RISCV_ALTERNATIVE_EARLY will issue sbi_ecall() very early in the boot > > process, before the first memory mapping is setup so we can't have any > > instrumentation happening here. > > > > In addition, when the kernel is relocatable, we must also not issue any > > relocation this early since they would have been patched virtually only. > > > > So, instead of disabling instrumentation for the whole kernel/sbi.c file > > and compiling it with -fno-pie, simply move __sbi_ecall() and > > __sbi_base_ecall() into their own file where this is fixed. > > Looking at the baseline disassembly from both LLVM 19 and GCC 13.2.0 with > RISCV_ALTERNATIVE_EARLY + KASAN + TRACEPOINTS, all of the instrumentation in > __sbi_ecall() itself is out of line and only executed when the tracepoint static > branches are enabled. However, there is instrumentation in sbi_get_m*id() from > the switch table inlined from sbi_err_map_linux_errno(), and some of those > memory accesses are done unconditionally. > > This change will force sbi_err_map_linux_errno() to be out of line (unless LTO > is enabled), so it also forces that particular bit of instrumentation to be > executed only in the error path. But we could still crash in the error path. So > I think sbi_err_map_linux_errno() needs to be moved to sbi_ecall.c as well. > > Alternatively, sbi_get_m*id() do not have any errors defined (and we don't > really handle that possibility), so we could drop to the call to > sbi_err_map_linux_errno() from __sbi_base_ecall(). Damn, I should have checked but I was sure it was a static inline in sbi.h, which would be another solution. I prefer this solution, WDYT? Thanks, Alex > > Regards, > Samuel > > > Fixes: 1745cfafebdf ("riscv: don't use global static vars to store alternative data") > > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com> > > --- > > arch/riscv/include/asm/sbi.h | 2 ++ > > arch/riscv/kernel/Makefile | 6 ++++- > > arch/riscv/kernel/sbi.c | 44 -------------------------------- > > arch/riscv/kernel/sbi_ecall.c | 48 +++++++++++++++++++++++++++++++++++ > > 4 files changed, 55 insertions(+), 45 deletions(-) > > create mode 100644 arch/riscv/kernel/sbi_ecall.c > > > > diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h > > index 7cffd4ffecd0..5843a10b380e 100644 > > --- a/arch/riscv/include/asm/sbi.h > > +++ b/arch/riscv/include/asm/sbi.h > > @@ -9,6 +9,7 @@ > > > > #include <linux/types.h> > > #include <linux/cpumask.h> > > +#include <linux/jump_label.h> > > > > #ifdef CONFIG_RISCV_SBI > > enum sbi_ext_id { > > @@ -304,6 +305,7 @@ struct sbiret { > > }; > > > > void sbi_init(void); > > +long __sbi_base_ecall(int fid); > > struct sbiret __sbi_ecall(unsigned long arg0, unsigned long arg1, > > unsigned long arg2, unsigned long arg3, > > unsigned long arg4, unsigned long arg5, > > diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile > > index 06d407f1b30b..7f88cc4931f5 100644 > > --- a/arch/riscv/kernel/Makefile > > +++ b/arch/riscv/kernel/Makefile > > @@ -20,17 +20,21 @@ endif > > ifdef CONFIG_RISCV_ALTERNATIVE_EARLY > > CFLAGS_alternative.o := -mcmodel=medany > > CFLAGS_cpufeature.o := -mcmodel=medany > > +CFLAGS_sbi_ecall.o := -mcmodel=medany > > ifdef CONFIG_FTRACE > > CFLAGS_REMOVE_alternative.o = $(CC_FLAGS_FTRACE) > > CFLAGS_REMOVE_cpufeature.o = $(CC_FLAGS_FTRACE) > > +CFLAGS_REMOVE_sbi_ecall.o = $(CC_FLAGS_FTRACE) > > endif > > ifdef CONFIG_RELOCATABLE > > CFLAGS_alternative.o += -fno-pie > > CFLAGS_cpufeature.o += -fno-pie > > +CFLAGS_sbi_ecall.o += -fno-pie > > endif > > ifdef CONFIG_KASAN > > KASAN_SANITIZE_alternative.o := n > > KASAN_SANITIZE_cpufeature.o := n > > +KASAN_SANITIZE_sbi_ecall.o := n > > endif > > endif > > > > @@ -88,7 +92,7 @@ obj-$(CONFIG_DYNAMIC_FTRACE) += mcount-dyn.o > > > > obj-$(CONFIG_PERF_EVENTS) += perf_callchain.o > > obj-$(CONFIG_HAVE_PERF_REGS) += perf_regs.o > > -obj-$(CONFIG_RISCV_SBI) += sbi.o > > +obj-$(CONFIG_RISCV_SBI) += sbi.o sbi_ecall.o > > ifeq ($(CONFIG_RISCV_SBI), y) > > obj-$(CONFIG_SMP) += sbi-ipi.o > > obj-$(CONFIG_SMP) += cpu_ops_sbi.o > > diff --git a/arch/riscv/kernel/sbi.c b/arch/riscv/kernel/sbi.c > > index 837bdab2601b..ace9e2f59c41 100644 > > --- a/arch/riscv/kernel/sbi.c > > +++ b/arch/riscv/kernel/sbi.c > > @@ -14,9 +14,6 @@ > > #include <asm/smp.h> > > #include <asm/tlbflush.h> > > > > -#define CREATE_TRACE_POINTS > > -#include <asm/trace.h> > > - > > /* default SBI version is 0.1 */ > > unsigned long sbi_spec_version __ro_after_init = SBI_SPEC_VERSION_DEFAULT; > > EXPORT_SYMBOL(sbi_spec_version); > > @@ -27,36 +24,6 @@ static int (*__sbi_rfence)(int fid, const struct cpumask *cpu_mask, > > unsigned long start, unsigned long size, > > unsigned long arg4, unsigned long arg5) __ro_after_init; > > > > -struct sbiret __sbi_ecall(unsigned long arg0, unsigned long arg1, > > - unsigned long arg2, unsigned long arg3, > > - unsigned long arg4, unsigned long arg5, > > - int fid, int ext) > > -{ > > - struct sbiret ret; > > - > > - trace_sbi_call(ext, fid); > > - > > - register uintptr_t a0 asm ("a0") = (uintptr_t)(arg0); > > - register uintptr_t a1 asm ("a1") = (uintptr_t)(arg1); > > - register uintptr_t a2 asm ("a2") = (uintptr_t)(arg2); > > - register uintptr_t a3 asm ("a3") = (uintptr_t)(arg3); > > - register uintptr_t a4 asm ("a4") = (uintptr_t)(arg4); > > - register uintptr_t a5 asm ("a5") = (uintptr_t)(arg5); > > - register uintptr_t a6 asm ("a6") = (uintptr_t)(fid); > > - register uintptr_t a7 asm ("a7") = (uintptr_t)(ext); > > - asm volatile ("ecall" > > - : "+r" (a0), "+r" (a1) > > - : "r" (a2), "r" (a3), "r" (a4), "r" (a5), "r" (a6), "r" (a7) > > - : "memory"); > > - ret.error = a0; > > - ret.value = a1; > > - > > - trace_sbi_return(ext, ret.error, ret.value); > > - > > - return ret; > > -} > > -EXPORT_SYMBOL(__sbi_ecall); > > - > > int sbi_err_map_linux_errno(int err) > > { > > switch (err) { > > @@ -535,17 +502,6 @@ long sbi_probe_extension(int extid) > > } > > EXPORT_SYMBOL(sbi_probe_extension); > > > > -static long __sbi_base_ecall(int fid) > > -{ > > - struct sbiret ret; > > - > > - ret = sbi_ecall(SBI_EXT_BASE, fid, 0, 0, 0, 0, 0, 0); > > - if (!ret.error) > > - return ret.value; > > - else > > - return sbi_err_map_linux_errno(ret.error); > > -} > > - > > static inline long sbi_get_spec_version(void) > > { > > return __sbi_base_ecall(SBI_EXT_BASE_GET_SPEC_VERSION); > > diff --git a/arch/riscv/kernel/sbi_ecall.c b/arch/riscv/kernel/sbi_ecall.c > > new file mode 100644 > > index 000000000000..24aabb4fbde3 > > --- /dev/null > > +++ b/arch/riscv/kernel/sbi_ecall.c > > @@ -0,0 +1,48 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* Copyright (c) 2024 Rivos Inc. */ > > + > > +#include <asm/sbi.h> > > +#define CREATE_TRACE_POINTS > > +#include <asm/trace.h> > > + > > +long __sbi_base_ecall(int fid) > > +{ > > + struct sbiret ret; > > + > > + ret = sbi_ecall(SBI_EXT_BASE, fid, 0, 0, 0, 0, 0, 0); > > + if (!ret.error) > > + return ret.value; > > + else > > + return sbi_err_map_linux_errno(ret.error); > > +} > > +EXPORT_SYMBOL(__sbi_base_ecall); > > + > > +struct sbiret __sbi_ecall(unsigned long arg0, unsigned long arg1, > > + unsigned long arg2, unsigned long arg3, > > + unsigned long arg4, unsigned long arg5, > > + int fid, int ext) > > +{ > > + struct sbiret ret; > > + > > + trace_sbi_call(ext, fid); > > + > > + register uintptr_t a0 asm ("a0") = (uintptr_t)(arg0); > > + register uintptr_t a1 asm ("a1") = (uintptr_t)(arg1); > > + register uintptr_t a2 asm ("a2") = (uintptr_t)(arg2); > > + register uintptr_t a3 asm ("a3") = (uintptr_t)(arg3); > > + register uintptr_t a4 asm ("a4") = (uintptr_t)(arg4); > > + register uintptr_t a5 asm ("a5") = (uintptr_t)(arg5); > > + register uintptr_t a6 asm ("a6") = (uintptr_t)(fid); > > + register uintptr_t a7 asm ("a7") = (uintptr_t)(ext); > > + asm volatile ("ecall" > > + : "+r" (a0), "+r" (a1) > > + : "r" (a2), "r" (a3), "r" (a4), "r" (a5), "r" (a6), "r" (a7) > > + : "memory"); > > + ret.error = a0; > > + ret.value = a1; > > + > > + trace_sbi_return(ext, ret.error, ret.value); > > + > > + return ret; > > +} > > +EXPORT_SYMBOL(__sbi_ecall); >
diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h index 7cffd4ffecd0..5843a10b380e 100644 --- a/arch/riscv/include/asm/sbi.h +++ b/arch/riscv/include/asm/sbi.h @@ -9,6 +9,7 @@ #include <linux/types.h> #include <linux/cpumask.h> +#include <linux/jump_label.h> #ifdef CONFIG_RISCV_SBI enum sbi_ext_id { @@ -304,6 +305,7 @@ struct sbiret { }; void sbi_init(void); +long __sbi_base_ecall(int fid); struct sbiret __sbi_ecall(unsigned long arg0, unsigned long arg1, unsigned long arg2, unsigned long arg3, unsigned long arg4, unsigned long arg5, diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile index 06d407f1b30b..7f88cc4931f5 100644 --- a/arch/riscv/kernel/Makefile +++ b/arch/riscv/kernel/Makefile @@ -20,17 +20,21 @@ endif ifdef CONFIG_RISCV_ALTERNATIVE_EARLY CFLAGS_alternative.o := -mcmodel=medany CFLAGS_cpufeature.o := -mcmodel=medany +CFLAGS_sbi_ecall.o := -mcmodel=medany ifdef CONFIG_FTRACE CFLAGS_REMOVE_alternative.o = $(CC_FLAGS_FTRACE) CFLAGS_REMOVE_cpufeature.o = $(CC_FLAGS_FTRACE) +CFLAGS_REMOVE_sbi_ecall.o = $(CC_FLAGS_FTRACE) endif ifdef CONFIG_RELOCATABLE CFLAGS_alternative.o += -fno-pie CFLAGS_cpufeature.o += -fno-pie +CFLAGS_sbi_ecall.o += -fno-pie endif ifdef CONFIG_KASAN KASAN_SANITIZE_alternative.o := n KASAN_SANITIZE_cpufeature.o := n +KASAN_SANITIZE_sbi_ecall.o := n endif endif @@ -88,7 +92,7 @@ obj-$(CONFIG_DYNAMIC_FTRACE) += mcount-dyn.o obj-$(CONFIG_PERF_EVENTS) += perf_callchain.o obj-$(CONFIG_HAVE_PERF_REGS) += perf_regs.o -obj-$(CONFIG_RISCV_SBI) += sbi.o +obj-$(CONFIG_RISCV_SBI) += sbi.o sbi_ecall.o ifeq ($(CONFIG_RISCV_SBI), y) obj-$(CONFIG_SMP) += sbi-ipi.o obj-$(CONFIG_SMP) += cpu_ops_sbi.o diff --git a/arch/riscv/kernel/sbi.c b/arch/riscv/kernel/sbi.c index 837bdab2601b..ace9e2f59c41 100644 --- a/arch/riscv/kernel/sbi.c +++ b/arch/riscv/kernel/sbi.c @@ -14,9 +14,6 @@ #include <asm/smp.h> #include <asm/tlbflush.h> -#define CREATE_TRACE_POINTS -#include <asm/trace.h> - /* default SBI version is 0.1 */ unsigned long sbi_spec_version __ro_after_init = SBI_SPEC_VERSION_DEFAULT; EXPORT_SYMBOL(sbi_spec_version); @@ -27,36 +24,6 @@ static int (*__sbi_rfence)(int fid, const struct cpumask *cpu_mask, unsigned long start, unsigned long size, unsigned long arg4, unsigned long arg5) __ro_after_init; -struct sbiret __sbi_ecall(unsigned long arg0, unsigned long arg1, - unsigned long arg2, unsigned long arg3, - unsigned long arg4, unsigned long arg5, - int fid, int ext) -{ - struct sbiret ret; - - trace_sbi_call(ext, fid); - - register uintptr_t a0 asm ("a0") = (uintptr_t)(arg0); - register uintptr_t a1 asm ("a1") = (uintptr_t)(arg1); - register uintptr_t a2 asm ("a2") = (uintptr_t)(arg2); - register uintptr_t a3 asm ("a3") = (uintptr_t)(arg3); - register uintptr_t a4 asm ("a4") = (uintptr_t)(arg4); - register uintptr_t a5 asm ("a5") = (uintptr_t)(arg5); - register uintptr_t a6 asm ("a6") = (uintptr_t)(fid); - register uintptr_t a7 asm ("a7") = (uintptr_t)(ext); - asm volatile ("ecall" - : "+r" (a0), "+r" (a1) - : "r" (a2), "r" (a3), "r" (a4), "r" (a5), "r" (a6), "r" (a7) - : "memory"); - ret.error = a0; - ret.value = a1; - - trace_sbi_return(ext, ret.error, ret.value); - - return ret; -} -EXPORT_SYMBOL(__sbi_ecall); - int sbi_err_map_linux_errno(int err) { switch (err) { @@ -535,17 +502,6 @@ long sbi_probe_extension(int extid) } EXPORT_SYMBOL(sbi_probe_extension); -static long __sbi_base_ecall(int fid) -{ - struct sbiret ret; - - ret = sbi_ecall(SBI_EXT_BASE, fid, 0, 0, 0, 0, 0, 0); - if (!ret.error) - return ret.value; - else - return sbi_err_map_linux_errno(ret.error); -} - static inline long sbi_get_spec_version(void) { return __sbi_base_ecall(SBI_EXT_BASE_GET_SPEC_VERSION); diff --git a/arch/riscv/kernel/sbi_ecall.c b/arch/riscv/kernel/sbi_ecall.c new file mode 100644 index 000000000000..24aabb4fbde3 --- /dev/null +++ b/arch/riscv/kernel/sbi_ecall.c @@ -0,0 +1,48 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2024 Rivos Inc. */ + +#include <asm/sbi.h> +#define CREATE_TRACE_POINTS +#include <asm/trace.h> + +long __sbi_base_ecall(int fid) +{ + struct sbiret ret; + + ret = sbi_ecall(SBI_EXT_BASE, fid, 0, 0, 0, 0, 0, 0); + if (!ret.error) + return ret.value; + else + return sbi_err_map_linux_errno(ret.error); +} +EXPORT_SYMBOL(__sbi_base_ecall); + +struct sbiret __sbi_ecall(unsigned long arg0, unsigned long arg1, + unsigned long arg2, unsigned long arg3, + unsigned long arg4, unsigned long arg5, + int fid, int ext) +{ + struct sbiret ret; + + trace_sbi_call(ext, fid); + + register uintptr_t a0 asm ("a0") = (uintptr_t)(arg0); + register uintptr_t a1 asm ("a1") = (uintptr_t)(arg1); + register uintptr_t a2 asm ("a2") = (uintptr_t)(arg2); + register uintptr_t a3 asm ("a3") = (uintptr_t)(arg3); + register uintptr_t a4 asm ("a4") = (uintptr_t)(arg4); + register uintptr_t a5 asm ("a5") = (uintptr_t)(arg5); + register uintptr_t a6 asm ("a6") = (uintptr_t)(fid); + register uintptr_t a7 asm ("a7") = (uintptr_t)(ext); + asm volatile ("ecall" + : "+r" (a0), "+r" (a1) + : "r" (a2), "r" (a3), "r" (a4), "r" (a5), "r" (a6), "r" (a7) + : "memory"); + ret.error = a0; + ret.value = a1; + + trace_sbi_return(ext, ret.error, ret.value); + + return ret; +} +EXPORT_SYMBOL(__sbi_ecall);
RISCV_ALTERNATIVE_EARLY will issue sbi_ecall() very early in the boot process, before the first memory mapping is setup so we can't have any instrumentation happening here. In addition, when the kernel is relocatable, we must also not issue any relocation this early since they would have been patched virtually only. So, instead of disabling instrumentation for the whole kernel/sbi.c file and compiling it with -fno-pie, simply move __sbi_ecall() and __sbi_base_ecall() into their own file where this is fixed. Fixes: 1745cfafebdf ("riscv: don't use global static vars to store alternative data") Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com> --- arch/riscv/include/asm/sbi.h | 2 ++ arch/riscv/kernel/Makefile | 6 ++++- arch/riscv/kernel/sbi.c | 44 -------------------------------- arch/riscv/kernel/sbi_ecall.c | 48 +++++++++++++++++++++++++++++++++++ 4 files changed, 55 insertions(+), 45 deletions(-) create mode 100644 arch/riscv/kernel/sbi_ecall.c