[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAOsKWHCCEAi-G=Ld9GJ2YUrbbV6dEzThXh5rOzYp6kWfUHfNHw@mail.gmail.com>
Date: Tue, 27 Aug 2024 12:57:41 +0800
From: Chunyan Zhang <zhangchunyan@...as.ac.cn>
To: Alexandre Ghiti <alexghiti@...osinc.com>
Cc: Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt <palmer@...belt.com>,
Albert Ou <aou@...s.berkeley.edu>, Guo Ren <guoren@...nel.org>,
Heiko Stuebner <heiko@...ech.de>, Philipp Tomsich <philipp.tomsich@...ll.eu>,
linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH -fixes] riscv: Fix RISCV_ALTERNATIVE_EARLY
Hi Alex,
On Mon, 26 Aug 2024 at 18:58, Alexandre Ghiti <alexghiti@...osinc.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@...as.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@...osinc.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
>
>
Powered by blists - more mailing lists