| Message ID | 20190826233256.32383-3-atish.patra@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for SBI version to 0.2 | expand |
On Mon, Aug 26, 2019 at 04:32:56PM -0700, Atish Patra wrote: > The SBI v0.2 introduces a base extension which is backward compatible > with v0.1. Implement all helper functions and minimum required SBI > calls from v0.2 for now. All other base extension function will be > added later as per need. > > Signed-off-by: Atish Patra <atish.patra@wdc.com> > --- > arch/riscv/include/asm/sbi.h | 68 +++++++++++++++++++++++++++++------- > arch/riscv/kernel/Makefile | 1 + > arch/riscv/kernel/sbi.c | 50 ++++++++++++++++++++++++++ > arch/riscv/kernel/setup.c | 2 ++ > 4 files changed, 108 insertions(+), 13 deletions(-) > create mode 100644 arch/riscv/kernel/sbi.c > > diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h > index 7f5ecaaaa0d7..4a4476956693 100644 > --- a/arch/riscv/include/asm/sbi.h > +++ b/arch/riscv/include/asm/sbi.h > @@ -8,7 +8,6 @@ > > #include <linux/types.h> > > - > #define SBI_EXT_LEGACY_SET_TIMER 0x0 > #define SBI_EXT_LEGACY_CONSOLE_PUTCHAR 0x1 > #define SBI_EXT_LEGACY_CONSOLE_GETCHAR 0x2 > @@ -19,28 +18,61 @@ > #define SBI_EXT_LEGACY_REMOTE_SFENCE_VMA_ASID 0x7 > #define SBI_EXT_LEGACY_SHUTDOWN 0x8 > > -#define SBI_CALL_LEGACY(which, arg0, arg1, arg2, arg3) ({ \ > +#define SBI_EXT_BASE 0x10 > + > +enum sbi_ext_base_fid { > + SBI_EXT_BASE_GET_SPEC_VERSION = 0, > + SBI_EXT_BASE_GET_IMP_ID, > + SBI_EXT_BASE_GET_IMP_VERSION, > + SBI_EXT_BASE_PROBE_EXT, > + SBI_EXT_BASE_GET_MVENDORID, > + SBI_EXT_BASE_GET_MARCHID, > + SBI_EXT_BASE_GET_MIMPID, > +}; > + > +#define SBI_CALL_LEGACY(ext, fid, arg0, arg1, arg2, arg3) ({ \ > 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 a7 asm ("a7") = (uintptr_t)(which); \ > + 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" (a7) \ > + : "+r" (a0), "+r" (a1) \ > + : "r" (a2), "r" (a3), "r" (a6), "r" (a7) \ Maybe I'm missing something, but how is this supposed to work on systems with SBI v0.1? Wouldn't this cause a mismatch in the registers? > : "memory"); \ > a0; \ > }) > > /* Lazy implementations until SBI is finalized */ > -#define SBI_CALL_LEGACY_0(which) SBI_CALL_LEGACY(which, 0, 0, 0, 0) > -#define SBI_CALL_LEGACY_1(which, arg0) SBI_CALL_LEGACY(which, arg0, 0, 0, 0) > -#define SBI_CALL_LEGACY_2(which, arg0, arg1) \ > - SBI_CALL_LEGACY(which, arg0, arg1, 0, 0) > -#define SBI_CALL_LEGACY_3(which, arg0, arg1, arg2) \ > - SBI_CALL_LEGACY(which, arg0, arg1, arg2, 0) > -#define SBI_CALL_LEGACY_4(which, arg0, arg1, arg2, arg3) \ > - SBI_CALL_LEGACY(which, arg0, arg1, arg2, arg3) > +#define SBI_CALL_LEGACY_0(ext) SBI_CALL_LEGACY(ext, 0, 0, 0, 0, 0) > +#define SBI_CALL_LEGACY_1(ext, arg0) SBI_CALL_LEGACY(ext, 0, arg0, 0, 0, 0) > +#define SBI_CALL_LEGACY_2(ext, arg0, arg1) \ > + SBI_CALL_LEGACY(ext, 0, arg0, arg1, 0, 0) > +#define SBI_CALL_LEGACY_3(ext, arg0, arg1, arg2) \ > + SBI_CALL_LEGACY(ext, 0, arg0, arg1, arg2, 0) > +#define SBI_CALL_LEGACY_4(ext, arg0, arg1, arg2, arg3) \ > + SBI_CALL_LEGACY(ext, 0, arg0, arg1, arg2, arg3) > + > +extern unsigned long sbi_firmware_version; > +struct sbiret { > + long error; > + long value; > +}; > + > +void riscv_sbi_init(void); > +struct sbiret riscv_sbi_ecall(int ext, int fid, int arg0, int arg1, > + int arg2, int arg3); > + > +#define SBI_CALL_0(ext, fid) riscv_sbi_ecall(ext, fid, 0, 0, 0, 0) > +#define SBI_CALL_1(ext, fid, arg0) riscv_sbi_ecall(ext, fid, arg0, 0, 0, 0) > +#define SBI_CALL_2(ext, fid, arg0, arg1) \ > + riscv_sbi_ecall(ext, fid, arg0, arg1, 0, 0) > +#define SBI_CALL_3(ext, fid, arg0, arg1, arg2) \ > + riscv_sbi_ecall(ext, fid, arg0, arg1, arg2, 0) > +#define SBI_CALL_4(ext, fid, arg0, arg1, arg2, arg3) \ > + riscv_sbi_ecall(ext, fid, arg0, arg1, arg2, arg3) > + > > static inline void sbi_console_putchar(int ch) > { > @@ -99,4 +131,14 @@ static inline void sbi_remote_sfence_vma_asid(const unsigned long *hart_mask, > start, size, asid); > } > > +static inline unsigned long riscv_sbi_major_version(void) > +{ > + return (sbi_firmware_version >> 24) & 0x7f; > +} > + > +static inline unsigned long riscv_sbi_minor_version(void) > +{ > + return sbi_firmware_version & 0xffffff; > +} > + > #endif > diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile > index 2420d37d96de..faf862d26924 100644 > --- a/arch/riscv/kernel/Makefile > +++ b/arch/riscv/kernel/Makefile > @@ -17,6 +17,7 @@ obj-y += irq.o > obj-y += process.o > obj-y += ptrace.o > obj-y += reset.o > +obj-y += sbi.o > obj-y += setup.o > obj-y += signal.o > obj-y += syscall_table.o > diff --git a/arch/riscv/kernel/sbi.c b/arch/riscv/kernel/sbi.c > new file mode 100644 > index 000000000000..457b8cc0e9d9 > --- /dev/null > +++ b/arch/riscv/kernel/sbi.c > @@ -0,0 +1,50 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * SBI initialilization and base extension implementation. > + * > + * Copyright (c) 2019 Western Digital Corporation or its affiliates. > + */ > + > +#include <asm/sbi.h> > +#include <linux/sched.h> > + > +unsigned long sbi_firmware_version; > + > +struct sbiret riscv_sbi_ecall(int ext, int fid, int arg0, int arg1, > + int arg2, int arg3) > +{ > + struct sbiret ret; > + > + 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 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" (a6), "r" (a7) > + : "memory"); > + ret.error = a0; > + ret.value = a1; > + > + return ret; > +} > + > +static struct sbiret sbi_get_spec_version(void) > +{ > + return SBI_CALL_0(SBI_EXT_BASE, SBI_EXT_BASE_GET_SPEC_VERSION); > +} > + > +void riscv_sbi_init(void) > +{ > + struct sbiret ret; > + > + /* legacy SBI version*/ > + sbi_firmware_version = 0x1; > + ret = sbi_get_spec_version(); > + if (!ret.error) > + sbi_firmware_version = ret.value; > + pr_info("SBI version implemented in firmware [%lu:%lu]\n", > + riscv_sbi_major_version(), riscv_sbi_minor_version()); > +} > diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c > index a990a6cb184f..4c324fd398c8 100644 > --- a/arch/riscv/kernel/setup.c > +++ b/arch/riscv/kernel/setup.c > @@ -21,6 +21,7 @@ > #include <asm/sections.h> > #include <asm/pgtable.h> > #include <asm/smp.h> > +#include <asm/sbi.h> > #include <asm/tlbflush.h> > #include <asm/thread_info.h> > > @@ -70,6 +71,7 @@ void __init setup_arch(char **cmdline_p) > swiotlb_init(1); > #endif > > + riscv_sbi_init(); > #ifdef CONFIG_SMP > setup_smp(); > #endif > -- > 2.21.0 > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
On Tue, Aug 27, 2019 at 1:28 PM Mike Rapoport <rppt@linux.ibm.com> wrote: > > On Mon, Aug 26, 2019 at 04:32:56PM -0700, Atish Patra wrote: > > The SBI v0.2 introduces a base extension which is backward compatible > > with v0.1. Implement all helper functions and minimum required SBI > > calls from v0.2 for now. All other base extension function will be > > added later as per need. > > > > Signed-off-by: Atish Patra <atish.patra@wdc.com> > > --- > > arch/riscv/include/asm/sbi.h | 68 +++++++++++++++++++++++++++++------- > > arch/riscv/kernel/Makefile | 1 + > > arch/riscv/kernel/sbi.c | 50 ++++++++++++++++++++++++++ > > arch/riscv/kernel/setup.c | 2 ++ > > 4 files changed, 108 insertions(+), 13 deletions(-) > > create mode 100644 arch/riscv/kernel/sbi.c > > > > diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h > > index 7f5ecaaaa0d7..4a4476956693 100644 > > --- a/arch/riscv/include/asm/sbi.h > > +++ b/arch/riscv/include/asm/sbi.h > > @@ -8,7 +8,6 @@ > > > > #include <linux/types.h> > > > > - > > #define SBI_EXT_LEGACY_SET_TIMER 0x0 > > #define SBI_EXT_LEGACY_CONSOLE_PUTCHAR 0x1 > > #define SBI_EXT_LEGACY_CONSOLE_GETCHAR 0x2 > > @@ -19,28 +18,61 @@ > > #define SBI_EXT_LEGACY_REMOTE_SFENCE_VMA_ASID 0x7 > > #define SBI_EXT_LEGACY_SHUTDOWN 0x8 > > > > -#define SBI_CALL_LEGACY(which, arg0, arg1, arg2, arg3) ({ \ > > +#define SBI_EXT_BASE 0x10 > > + > > +enum sbi_ext_base_fid { > > + SBI_EXT_BASE_GET_SPEC_VERSION = 0, > > + SBI_EXT_BASE_GET_IMP_ID, > > + SBI_EXT_BASE_GET_IMP_VERSION, > > + SBI_EXT_BASE_PROBE_EXT, > > + SBI_EXT_BASE_GET_MVENDORID, > > + SBI_EXT_BASE_GET_MARCHID, > > + SBI_EXT_BASE_GET_MIMPID, > > +}; > > + > > +#define SBI_CALL_LEGACY(ext, fid, arg0, arg1, arg2, arg3) ({ \ > > 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 a7 asm ("a7") = (uintptr_t)(which); \ > > + 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" (a7) \ > > + : "+r" (a0), "+r" (a1) \ > > + : "r" (a2), "r" (a3), "r" (a6), "r" (a7) \ > > Maybe I'm missing something, but how is this supposed to work on systems > with SBI v0.1? Wouldn't this cause a mismatch in the registers? The SBI v0.2 has two major changes: 1. New improved calling convention which is backward compatible with SBI v0.1 so older kernels with SBI v0.1 will continue to work as-is. 2. Base set of mandatory SBI v0.2 calls which can be used to detect SBI version, check supported SBI calls and extentions. Old calling convention in SBI v0.1 was: Parameters: a0 -> arg0 a1 -> arg1 a2 -> arg2 a3 -> arg3 a7 -> function_id Return: a0 -> return value or error code In SBI v0.2, we have extension and function. Each SBI extension is a set of function. The new calling convention in SBI v0.2 is: Parameters: a0 -> arg0 a1 -> arg1 a2 -> arg2 a3 -> arg3 a6 -> function_id a7 -> extension_id Return: a0 -> error code a1 -> return value (optional) All legacy SBI v0.1 functions can be thought of as separate extensions. That's how SBI v0.2 will be backward compatible. Regards, Anup > > > : "memory"); \ > > a0; \ > > }) > > > > /* Lazy implementations until SBI is finalized */ > > -#define SBI_CALL_LEGACY_0(which) SBI_CALL_LEGACY(which, 0, 0, 0, 0) > > -#define SBI_CALL_LEGACY_1(which, arg0) SBI_CALL_LEGACY(which, arg0, 0, 0, 0) > > -#define SBI_CALL_LEGACY_2(which, arg0, arg1) \ > > - SBI_CALL_LEGACY(which, arg0, arg1, 0, 0) > > -#define SBI_CALL_LEGACY_3(which, arg0, arg1, arg2) \ > > - SBI_CALL_LEGACY(which, arg0, arg1, arg2, 0) > > -#define SBI_CALL_LEGACY_4(which, arg0, arg1, arg2, arg3) \ > > - SBI_CALL_LEGACY(which, arg0, arg1, arg2, arg3) > > +#define SBI_CALL_LEGACY_0(ext) SBI_CALL_LEGACY(ext, 0, 0, 0, 0, 0) > > +#define SBI_CALL_LEGACY_1(ext, arg0) SBI_CALL_LEGACY(ext, 0, arg0, 0, 0, 0) > > +#define SBI_CALL_LEGACY_2(ext, arg0, arg1) \ > > + SBI_CALL_LEGACY(ext, 0, arg0, arg1, 0, 0) > > +#define SBI_CALL_LEGACY_3(ext, arg0, arg1, arg2) \ > > + SBI_CALL_LEGACY(ext, 0, arg0, arg1, arg2, 0) > > +#define SBI_CALL_LEGACY_4(ext, arg0, arg1, arg2, arg3) \ > > + SBI_CALL_LEGACY(ext, 0, arg0, arg1, arg2, arg3) > > + > > +extern unsigned long sbi_firmware_version; > > +struct sbiret { > > + long error; > > + long value; > > +}; > > + > > +void riscv_sbi_init(void); > > +struct sbiret riscv_sbi_ecall(int ext, int fid, int arg0, int arg1, > > + int arg2, int arg3); > > + > > +#define SBI_CALL_0(ext, fid) riscv_sbi_ecall(ext, fid, 0, 0, 0, 0) > > +#define SBI_CALL_1(ext, fid, arg0) riscv_sbi_ecall(ext, fid, arg0, 0, 0, 0) > > +#define SBI_CALL_2(ext, fid, arg0, arg1) \ > > + riscv_sbi_ecall(ext, fid, arg0, arg1, 0, 0) > > +#define SBI_CALL_3(ext, fid, arg0, arg1, arg2) \ > > + riscv_sbi_ecall(ext, fid, arg0, arg1, arg2, 0) > > +#define SBI_CALL_4(ext, fid, arg0, arg1, arg2, arg3) \ > > + riscv_sbi_ecall(ext, fid, arg0, arg1, arg2, arg3) > > + > > > > static inline void sbi_console_putchar(int ch) > > { > > @@ -99,4 +131,14 @@ static inline void sbi_remote_sfence_vma_asid(const unsigned long *hart_mask, > > start, size, asid); > > } > > > > +static inline unsigned long riscv_sbi_major_version(void) > > +{ > > + return (sbi_firmware_version >> 24) & 0x7f; > > +} > > + > > +static inline unsigned long riscv_sbi_minor_version(void) > > +{ > > + return sbi_firmware_version & 0xffffff; > > +} > > + > > #endif > > diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile > > index 2420d37d96de..faf862d26924 100644 > > --- a/arch/riscv/kernel/Makefile > > +++ b/arch/riscv/kernel/Makefile > > @@ -17,6 +17,7 @@ obj-y += irq.o > > obj-y += process.o > > obj-y += ptrace.o > > obj-y += reset.o > > +obj-y += sbi.o > > obj-y += setup.o > > obj-y += signal.o > > obj-y += syscall_table.o > > diff --git a/arch/riscv/kernel/sbi.c b/arch/riscv/kernel/sbi.c > > new file mode 100644 > > index 000000000000..457b8cc0e9d9 > > --- /dev/null > > +++ b/arch/riscv/kernel/sbi.c > > @@ -0,0 +1,50 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * SBI initialilization and base extension implementation. > > + * > > + * Copyright (c) 2019 Western Digital Corporation or its affiliates. > > + */ > > + > > +#include <asm/sbi.h> > > +#include <linux/sched.h> > > + > > +unsigned long sbi_firmware_version; > > + > > +struct sbiret riscv_sbi_ecall(int ext, int fid, int arg0, int arg1, > > + int arg2, int arg3) > > +{ > > + struct sbiret ret; > > + > > + 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 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" (a6), "r" (a7) > > + : "memory"); > > + ret.error = a0; > > + ret.value = a1; > > + > > + return ret; > > +} > > + > > +static struct sbiret sbi_get_spec_version(void) > > +{ > > + return SBI_CALL_0(SBI_EXT_BASE, SBI_EXT_BASE_GET_SPEC_VERSION); > > +} > > + > > +void riscv_sbi_init(void) > > +{ > > + struct sbiret ret; > > + > > + /* legacy SBI version*/ > > + sbi_firmware_version = 0x1; > > + ret = sbi_get_spec_version(); > > + if (!ret.error) > > + sbi_firmware_version = ret.value; > > + pr_info("SBI version implemented in firmware [%lu:%lu]\n", > > + riscv_sbi_major_version(), riscv_sbi_minor_version()); > > +} > > diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c > > index a990a6cb184f..4c324fd398c8 100644 > > --- a/arch/riscv/kernel/setup.c > > +++ b/arch/riscv/kernel/setup.c > > @@ -21,6 +21,7 @@ > > #include <asm/sections.h> > > #include <asm/pgtable.h> > > #include <asm/smp.h> > > +#include <asm/sbi.h> > > #include <asm/tlbflush.h> > > #include <asm/thread_info.h> > > > > @@ -70,6 +71,7 @@ void __init setup_arch(char **cmdline_p) > > swiotlb_init(1); > > #endif > > > > + riscv_sbi_init(); > > #ifdef CONFIG_SMP > > setup_smp(); > > #endif > > -- > > 2.21.0 > > > > > > _______________________________________________ > > linux-riscv mailing list > > linux-riscv@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-riscv > > -- > Sincerely yours, > Mike. >
On Tue, Aug 27, 2019 at 01:53:23PM +0530, Anup Patel wrote: > On Tue, Aug 27, 2019 at 1:28 PM Mike Rapoport <rppt@linux.ibm.com> wrote: > > > > On Mon, Aug 26, 2019 at 04:32:56PM -0700, Atish Patra wrote: > > > The SBI v0.2 introduces a base extension which is backward compatible > > > with v0.1. Implement all helper functions and minimum required SBI > > > calls from v0.2 for now. All other base extension function will be > > > added later as per need. > > > > > > Signed-off-by: Atish Patra <atish.patra@wdc.com> > > > --- > > > arch/riscv/include/asm/sbi.h | 68 +++++++++++++++++++++++++++++------- > > > arch/riscv/kernel/Makefile | 1 + > > > arch/riscv/kernel/sbi.c | 50 ++++++++++++++++++++++++++ > > > arch/riscv/kernel/setup.c | 2 ++ > > > 4 files changed, 108 insertions(+), 13 deletions(-) > > > create mode 100644 arch/riscv/kernel/sbi.c > > > > > > diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h > > > index 7f5ecaaaa0d7..4a4476956693 100644 > > > --- a/arch/riscv/include/asm/sbi.h > > > +++ b/arch/riscv/include/asm/sbi.h > > > @@ -8,7 +8,6 @@ > > > > > > #include <linux/types.h> > > > > > > - > > > #define SBI_EXT_LEGACY_SET_TIMER 0x0 > > > #define SBI_EXT_LEGACY_CONSOLE_PUTCHAR 0x1 > > > #define SBI_EXT_LEGACY_CONSOLE_GETCHAR 0x2 > > > @@ -19,28 +18,61 @@ > > > #define SBI_EXT_LEGACY_REMOTE_SFENCE_VMA_ASID 0x7 > > > #define SBI_EXT_LEGACY_SHUTDOWN 0x8 > > > > > > -#define SBI_CALL_LEGACY(which, arg0, arg1, arg2, arg3) ({ \ > > > +#define SBI_EXT_BASE 0x10 > > > + > > > +enum sbi_ext_base_fid { > > > + SBI_EXT_BASE_GET_SPEC_VERSION = 0, > > > + SBI_EXT_BASE_GET_IMP_ID, > > > + SBI_EXT_BASE_GET_IMP_VERSION, > > > + SBI_EXT_BASE_PROBE_EXT, > > > + SBI_EXT_BASE_GET_MVENDORID, > > > + SBI_EXT_BASE_GET_MARCHID, > > > + SBI_EXT_BASE_GET_MIMPID, > > > +}; > > > + > > > +#define SBI_CALL_LEGACY(ext, fid, arg0, arg1, arg2, arg3) ({ \ > > > 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 a7 asm ("a7") = (uintptr_t)(which); \ > > > + 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" (a7) \ > > > + : "+r" (a0), "+r" (a1) \ > > > + : "r" (a2), "r" (a3), "r" (a6), "r" (a7) \ > > > > Maybe I'm missing something, but how is this supposed to work on systems > > with SBI v0.1? Wouldn't this cause a mismatch in the registers? > > The SBI v0.2 has two major changes: > 1. New improved calling convention which is backward compatible > with SBI v0.1 so older kernels with SBI v0.1 will continue to work as-is. > 2. Base set of mandatory SBI v0.2 calls which can be used to detect > SBI version, check supported SBI calls and extentions. > > Old calling convention in SBI v0.1 was: > Parameters: > a0 -> arg0 > a1 -> arg1 > a2 -> arg2 > a3 -> arg3 > a7 -> function_id > Return: > a0 -> return value or error code > > In SBI v0.2, we have extension and function. Each SBI extension > is a set of function. The new calling convention in SBI v0.2 is: > Parameters: > a0 -> arg0 > a1 -> arg1 > a2 -> arg2 > a3 -> arg3 > a6 -> function_id > a7 -> extension_id > Return: > a0 -> error code > a1 -> return value (optional) So with this patch SBI_CALL_LEGACY() uses SBI v0.2 convention, right? Doesn't it mean that you cannot run a new kernel on a system with SBI v0.1? > All legacy SBI v0.1 functions can be thought of as separate > extensions. That's how SBI v0.2 will be backward compatible. > > Regards, > Anup > > > > > > : "memory"); \ > > > a0; \ > > > }) > > > > > > /* Lazy implementations until SBI is finalized */ > > > -#define SBI_CALL_LEGACY_0(which) SBI_CALL_LEGACY(which, 0, 0, 0, 0) > > > -#define SBI_CALL_LEGACY_1(which, arg0) SBI_CALL_LEGACY(which, arg0, 0, 0, 0) > > > -#define SBI_CALL_LEGACY_2(which, arg0, arg1) \ > > > - SBI_CALL_LEGACY(which, arg0, arg1, 0, 0) > > > -#define SBI_CALL_LEGACY_3(which, arg0, arg1, arg2) \ > > > - SBI_CALL_LEGACY(which, arg0, arg1, arg2, 0) > > > -#define SBI_CALL_LEGACY_4(which, arg0, arg1, arg2, arg3) \ > > > - SBI_CALL_LEGACY(which, arg0, arg1, arg2, arg3) > > > +#define SBI_CALL_LEGACY_0(ext) SBI_CALL_LEGACY(ext, 0, 0, 0, 0, 0) > > > +#define SBI_CALL_LEGACY_1(ext, arg0) SBI_CALL_LEGACY(ext, 0, arg0, 0, 0, 0) > > > +#define SBI_CALL_LEGACY_2(ext, arg0, arg1) \ > > > + SBI_CALL_LEGACY(ext, 0, arg0, arg1, 0, 0) > > > +#define SBI_CALL_LEGACY_3(ext, arg0, arg1, arg2) \ > > > + SBI_CALL_LEGACY(ext, 0, arg0, arg1, arg2, 0) > > > +#define SBI_CALL_LEGACY_4(ext, arg0, arg1, arg2, arg3) \ > > > + SBI_CALL_LEGACY(ext, 0, arg0, arg1, arg2, arg3) > > > + > > > +extern unsigned long sbi_firmware_version; > > > +struct sbiret { > > > + long error; > > > + long value; > > > +}; > > > + > > > +void riscv_sbi_init(void); > > > +struct sbiret riscv_sbi_ecall(int ext, int fid, int arg0, int arg1, > > > + int arg2, int arg3); > > > + > > > +#define SBI_CALL_0(ext, fid) riscv_sbi_ecall(ext, fid, 0, 0, 0, 0) > > > +#define SBI_CALL_1(ext, fid, arg0) riscv_sbi_ecall(ext, fid, arg0, 0, 0, 0) > > > +#define SBI_CALL_2(ext, fid, arg0, arg1) \ > > > + riscv_sbi_ecall(ext, fid, arg0, arg1, 0, 0) > > > +#define SBI_CALL_3(ext, fid, arg0, arg1, arg2) \ > > > + riscv_sbi_ecall(ext, fid, arg0, arg1, arg2, 0) > > > +#define SBI_CALL_4(ext, fid, arg0, arg1, arg2, arg3) \ > > > + riscv_sbi_ecall(ext, fid, arg0, arg1, arg2, arg3) > > > + > > > > > > static inline void sbi_console_putchar(int ch) > > > { > > > @@ -99,4 +131,14 @@ static inline void sbi_remote_sfence_vma_asid(const unsigned long *hart_mask, > > > start, size, asid); > > > } > > > > > > +static inline unsigned long riscv_sbi_major_version(void) > > > +{ > > > + return (sbi_firmware_version >> 24) & 0x7f; > > > +} > > > + > > > +static inline unsigned long riscv_sbi_minor_version(void) > > > +{ > > > + return sbi_firmware_version & 0xffffff; > > > +} > > > + > > > #endif > > > diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile > > > index 2420d37d96de..faf862d26924 100644 > > > --- a/arch/riscv/kernel/Makefile > > > +++ b/arch/riscv/kernel/Makefile > > > @@ -17,6 +17,7 @@ obj-y += irq.o > > > obj-y += process.o > > > obj-y += ptrace.o > > > obj-y += reset.o > > > +obj-y += sbi.o > > > obj-y += setup.o > > > obj-y += signal.o > > > obj-y += syscall_table.o > > > diff --git a/arch/riscv/kernel/sbi.c b/arch/riscv/kernel/sbi.c > > > new file mode 100644 > > > index 000000000000..457b8cc0e9d9 > > > --- /dev/null > > > +++ b/arch/riscv/kernel/sbi.c > > > @@ -0,0 +1,50 @@ > > > +// SPDX-License-Identifier: GPL-2.0-only > > > +/* > > > + * SBI initialilization and base extension implementation. > > > + * > > > + * Copyright (c) 2019 Western Digital Corporation or its affiliates. > > > + */ > > > + > > > +#include <asm/sbi.h> > > > +#include <linux/sched.h> > > > + > > > +unsigned long sbi_firmware_version; > > > + > > > +struct sbiret riscv_sbi_ecall(int ext, int fid, int arg0, int arg1, > > > + int arg2, int arg3) > > > +{ > > > + struct sbiret ret; > > > + > > > + 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 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" (a6), "r" (a7) > > > + : "memory"); > > > + ret.error = a0; > > > + ret.value = a1; > > > + > > > + return ret; > > > +} > > > + > > > +static struct sbiret sbi_get_spec_version(void) > > > +{ > > > + return SBI_CALL_0(SBI_EXT_BASE, SBI_EXT_BASE_GET_SPEC_VERSION); > > > +} > > > + > > > +void riscv_sbi_init(void) > > > +{ > > > + struct sbiret ret; > > > + > > > + /* legacy SBI version*/ > > > + sbi_firmware_version = 0x1; > > > + ret = sbi_get_spec_version(); > > > + if (!ret.error) > > > + sbi_firmware_version = ret.value; > > > + pr_info("SBI version implemented in firmware [%lu:%lu]\n", > > > + riscv_sbi_major_version(), riscv_sbi_minor_version()); > > > +} > > > diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c > > > index a990a6cb184f..4c324fd398c8 100644 > > > --- a/arch/riscv/kernel/setup.c > > > +++ b/arch/riscv/kernel/setup.c > > > @@ -21,6 +21,7 @@ > > > #include <asm/sections.h> > > > #include <asm/pgtable.h> > > > #include <asm/smp.h> > > > +#include <asm/sbi.h> > > > #include <asm/tlbflush.h> > > > #include <asm/thread_info.h> > > > > > > @@ -70,6 +71,7 @@ void __init setup_arch(char **cmdline_p) > > > swiotlb_init(1); > > > #endif > > > > > > + riscv_sbi_init(); > > > #ifdef CONFIG_SMP > > > setup_smp(); > > > #endif > > > -- > > > 2.21.0 > > > > > > > > > _______________________________________________ > > > linux-riscv mailing list > > > linux-riscv@lists.infradead.org > > > http://lists.infradead.org/mailman/listinfo/linux-riscv > > > > -- > > Sincerely yours, > > Mike. > >
On Tue, Aug 27, 2019 at 2:09 PM Mike Rapoport <rppt@linux.ibm.com> wrote: > > On Tue, Aug 27, 2019 at 01:53:23PM +0530, Anup Patel wrote: > > On Tue, Aug 27, 2019 at 1:28 PM Mike Rapoport <rppt@linux.ibm.com> wrote: > > > > > > On Mon, Aug 26, 2019 at 04:32:56PM -0700, Atish Patra wrote: > > > > The SBI v0.2 introduces a base extension which is backward compatible > > > > with v0.1. Implement all helper functions and minimum required SBI > > > > calls from v0.2 for now. All other base extension function will be > > > > added later as per need. > > > > > > > > Signed-off-by: Atish Patra <atish.patra@wdc.com> > > > > --- > > > > arch/riscv/include/asm/sbi.h | 68 +++++++++++++++++++++++++++++------- > > > > arch/riscv/kernel/Makefile | 1 + > > > > arch/riscv/kernel/sbi.c | 50 ++++++++++++++++++++++++++ > > > > arch/riscv/kernel/setup.c | 2 ++ > > > > 4 files changed, 108 insertions(+), 13 deletions(-) > > > > create mode 100644 arch/riscv/kernel/sbi.c > > > > > > > > diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h > > > > index 7f5ecaaaa0d7..4a4476956693 100644 > > > > --- a/arch/riscv/include/asm/sbi.h > > > > +++ b/arch/riscv/include/asm/sbi.h > > > > @@ -8,7 +8,6 @@ > > > > > > > > #include <linux/types.h> > > > > > > > > - > > > > #define SBI_EXT_LEGACY_SET_TIMER 0x0 > > > > #define SBI_EXT_LEGACY_CONSOLE_PUTCHAR 0x1 > > > > #define SBI_EXT_LEGACY_CONSOLE_GETCHAR 0x2 > > > > @@ -19,28 +18,61 @@ > > > > #define SBI_EXT_LEGACY_REMOTE_SFENCE_VMA_ASID 0x7 > > > > #define SBI_EXT_LEGACY_SHUTDOWN 0x8 > > > > > > > > -#define SBI_CALL_LEGACY(which, arg0, arg1, arg2, arg3) ({ \ > > > > +#define SBI_EXT_BASE 0x10 > > > > + > > > > +enum sbi_ext_base_fid { > > > > + SBI_EXT_BASE_GET_SPEC_VERSION = 0, > > > > + SBI_EXT_BASE_GET_IMP_ID, > > > > + SBI_EXT_BASE_GET_IMP_VERSION, > > > > + SBI_EXT_BASE_PROBE_EXT, > > > > + SBI_EXT_BASE_GET_MVENDORID, > > > > + SBI_EXT_BASE_GET_MARCHID, > > > > + SBI_EXT_BASE_GET_MIMPID, > > > > +}; > > > > + > > > > +#define SBI_CALL_LEGACY(ext, fid, arg0, arg1, arg2, arg3) ({ \ > > > > 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 a7 asm ("a7") = (uintptr_t)(which); \ > > > > + 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" (a7) \ > > > > + : "+r" (a0), "+r" (a1) \ > > > > + : "r" (a2), "r" (a3), "r" (a6), "r" (a7) \ > > > > > > Maybe I'm missing something, but how is this supposed to work on systems > > > with SBI v0.1? Wouldn't this cause a mismatch in the registers? > > > > The SBI v0.2 has two major changes: > > 1. New improved calling convention which is backward compatible > > with SBI v0.1 so older kernels with SBI v0.1 will continue to work as-is. > > 2. Base set of mandatory SBI v0.2 calls which can be used to detect > > SBI version, check supported SBI calls and extentions. > > > > Old calling convention in SBI v0.1 was: > > Parameters: > > a0 -> arg0 > > a1 -> arg1 > > a2 -> arg2 > > a3 -> arg3 > > a7 -> function_id > > Return: > > a0 -> return value or error code > > > > In SBI v0.2, we have extension and function. Each SBI extension > > is a set of function. The new calling convention in SBI v0.2 is: > > Parameters: > > a0 -> arg0 > > a1 -> arg1 > > a2 -> arg2 > > a3 -> arg3 > > a6 -> function_id > > a7 -> extension_id > > Return: > > a0 -> error code > > a1 -> return value (optional) > > So with this patch SBI_CALL_LEGACY() uses SBI v0.2 convention, right? > Doesn't it mean that you cannot run a new kernel on a system with SBI v0.1? This is certainly possible. It's just that this patch is using SBI v0.2 convention for older firmware as well. Ideally, we should check sbi_version for each legacy SBI calls and use different calling convention based sbi_version. The sbi_version detection will work fine on both old and new firmwares because on old firmware SBI version call will fail which means it is SBI v0.1 interface. I think all legacy calls should be moved to kernel/sbi.c. I have other comments too. Let me post detailed review comments. Regards, Anup > > > All legacy SBI v0.1 functions can be thought of as separate > > extensions. That's how SBI v0.2 will be backward compatible. > > > > Regards, > > Anup > > > > > > > > > : "memory"); \ > > > > a0; \ > > > > }) > > > > > > > > /* Lazy implementations until SBI is finalized */ > > > > -#define SBI_CALL_LEGACY_0(which) SBI_CALL_LEGACY(which, 0, 0, 0, 0) > > > > -#define SBI_CALL_LEGACY_1(which, arg0) SBI_CALL_LEGACY(which, arg0, 0, 0, 0) > > > > -#define SBI_CALL_LEGACY_2(which, arg0, arg1) \ > > > > - SBI_CALL_LEGACY(which, arg0, arg1, 0, 0) > > > > -#define SBI_CALL_LEGACY_3(which, arg0, arg1, arg2) \ > > > > - SBI_CALL_LEGACY(which, arg0, arg1, arg2, 0) > > > > -#define SBI_CALL_LEGACY_4(which, arg0, arg1, arg2, arg3) \ > > > > - SBI_CALL_LEGACY(which, arg0, arg1, arg2, arg3) > > > > +#define SBI_CALL_LEGACY_0(ext) SBI_CALL_LEGACY(ext, 0, 0, 0, 0, 0) > > > > +#define SBI_CALL_LEGACY_1(ext, arg0) SBI_CALL_LEGACY(ext, 0, arg0, 0, 0, 0) > > > > +#define SBI_CALL_LEGACY_2(ext, arg0, arg1) \ > > > > + SBI_CALL_LEGACY(ext, 0, arg0, arg1, 0, 0) > > > > +#define SBI_CALL_LEGACY_3(ext, arg0, arg1, arg2) \ > > > > + SBI_CALL_LEGACY(ext, 0, arg0, arg1, arg2, 0) > > > > +#define SBI_CALL_LEGACY_4(ext, arg0, arg1, arg2, arg3) \ > > > > + SBI_CALL_LEGACY(ext, 0, arg0, arg1, arg2, arg3) > > > > + > > > > +extern unsigned long sbi_firmware_version; > > > > +struct sbiret { > > > > + long error; > > > > + long value; > > > > +}; > > > > + > > > > +void riscv_sbi_init(void); > > > > +struct sbiret riscv_sbi_ecall(int ext, int fid, int arg0, int arg1, > > > > + int arg2, int arg3); > > > > + > > > > +#define SBI_CALL_0(ext, fid) riscv_sbi_ecall(ext, fid, 0, 0, 0, 0) > > > > +#define SBI_CALL_1(ext, fid, arg0) riscv_sbi_ecall(ext, fid, arg0, 0, 0, 0) > > > > +#define SBI_CALL_2(ext, fid, arg0, arg1) \ > > > > + riscv_sbi_ecall(ext, fid, arg0, arg1, 0, 0) > > > > +#define SBI_CALL_3(ext, fid, arg0, arg1, arg2) \ > > > > + riscv_sbi_ecall(ext, fid, arg0, arg1, arg2, 0) > > > > +#define SBI_CALL_4(ext, fid, arg0, arg1, arg2, arg3) \ > > > > + riscv_sbi_ecall(ext, fid, arg0, arg1, arg2, arg3) > > > > + > > > > > > > > static inline void sbi_console_putchar(int ch) > > > > { > > > > @@ -99,4 +131,14 @@ static inline void sbi_remote_sfence_vma_asid(const unsigned long *hart_mask, > > > > start, size, asid); > > > > } > > > > > > > > +static inline unsigned long riscv_sbi_major_version(void) > > > > +{ > > > > + return (sbi_firmware_version >> 24) & 0x7f; > > > > +} > > > > + > > > > +static inline unsigned long riscv_sbi_minor_version(void) > > > > +{ > > > > + return sbi_firmware_version & 0xffffff; > > > > +} > > > > + > > > > #endif > > > > diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile > > > > index 2420d37d96de..faf862d26924 100644 > > > > --- a/arch/riscv/kernel/Makefile > > > > +++ b/arch/riscv/kernel/Makefile > > > > @@ -17,6 +17,7 @@ obj-y += irq.o > > > > obj-y += process.o > > > > obj-y += ptrace.o > > > > obj-y += reset.o > > > > +obj-y += sbi.o > > > > obj-y += setup.o > > > > obj-y += signal.o > > > > obj-y += syscall_table.o > > > > diff --git a/arch/riscv/kernel/sbi.c b/arch/riscv/kernel/sbi.c > > > > new file mode 100644 > > > > index 000000000000..457b8cc0e9d9 > > > > --- /dev/null > > > > +++ b/arch/riscv/kernel/sbi.c > > > > @@ -0,0 +1,50 @@ > > > > +// SPDX-License-Identifier: GPL-2.0-only > > > > +/* > > > > + * SBI initialilization and base extension implementation. > > > > + * > > > > + * Copyright (c) 2019 Western Digital Corporation or its affiliates. > > > > + */ > > > > + > > > > +#include <asm/sbi.h> > > > > +#include <linux/sched.h> > > > > + > > > > +unsigned long sbi_firmware_version; > > > > + > > > > +struct sbiret riscv_sbi_ecall(int ext, int fid, int arg0, int arg1, > > > > + int arg2, int arg3) > > > > +{ > > > > + struct sbiret ret; > > > > + > > > > + 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 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" (a6), "r" (a7) > > > > + : "memory"); > > > > + ret.error = a0; > > > > + ret.value = a1; > > > > + > > > > + return ret; > > > > +} > > > > + > > > > +static struct sbiret sbi_get_spec_version(void) > > > > +{ > > > > + return SBI_CALL_0(SBI_EXT_BASE, SBI_EXT_BASE_GET_SPEC_VERSION); > > > > +} > > > > + > > > > +void riscv_sbi_init(void) > > > > +{ > > > > + struct sbiret ret; > > > > + > > > > + /* legacy SBI version*/ > > > > + sbi_firmware_version = 0x1; > > > > + ret = sbi_get_spec_version(); > > > > + if (!ret.error) > > > > + sbi_firmware_version = ret.value; > > > > + pr_info("SBI version implemented in firmware [%lu:%lu]\n", > > > > + riscv_sbi_major_version(), riscv_sbi_minor_version()); > > > > +} > > > > diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c > > > > index a990a6cb184f..4c324fd398c8 100644 > > > > --- a/arch/riscv/kernel/setup.c > > > > +++ b/arch/riscv/kernel/setup.c > > > > @@ -21,6 +21,7 @@ > > > > #include <asm/sections.h> > > > > #include <asm/pgtable.h> > > > > #include <asm/smp.h> > > > > +#include <asm/sbi.h> > > > > #include <asm/tlbflush.h> > > > > #include <asm/thread_info.h> > > > > > > > > @@ -70,6 +71,7 @@ void __init setup_arch(char **cmdline_p) > > > > swiotlb_init(1); > > > > #endif > > > > > > > > + riscv_sbi_init(); > > > > #ifdef CONFIG_SMP > > > > setup_smp(); > > > > #endif > > > > -- > > > > 2.21.0 > > > > > > > > > > > > _______________________________________________ > > > > linux-riscv mailing list > > > > linux-riscv@lists.infradead.org > > > > http://lists.infradead.org/mailman/listinfo/linux-riscv > > > > > > -- > > > Sincerely yours, > > > Mike. > > > > > -- > Sincerely yours, > Mike. >
On Tue, Aug 27, 2019 at 5:03 AM Atish Patra <atish.patra@wdc.com> wrote: > > The SBI v0.2 introduces a base extension which is backward compatible > with v0.1. Implement all helper functions and minimum required SBI > calls from v0.2 for now. All other base extension function will be > added later as per need. > > Signed-off-by: Atish Patra <atish.patra@wdc.com> > --- > arch/riscv/include/asm/sbi.h | 68 +++++++++++++++++++++++++++++------- > arch/riscv/kernel/Makefile | 1 + > arch/riscv/kernel/sbi.c | 50 ++++++++++++++++++++++++++ > arch/riscv/kernel/setup.c | 2 ++ > 4 files changed, 108 insertions(+), 13 deletions(-) > create mode 100644 arch/riscv/kernel/sbi.c > > diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h > index 7f5ecaaaa0d7..4a4476956693 100644 > --- a/arch/riscv/include/asm/sbi.h > +++ b/arch/riscv/include/asm/sbi.h > @@ -8,7 +8,6 @@ > > #include <linux/types.h> > > - > #define SBI_EXT_LEGACY_SET_TIMER 0x0 > #define SBI_EXT_LEGACY_CONSOLE_PUTCHAR 0x1 > #define SBI_EXT_LEGACY_CONSOLE_GETCHAR 0x2 > @@ -19,28 +18,61 @@ > #define SBI_EXT_LEGACY_REMOTE_SFENCE_VMA_ASID 0x7 > #define SBI_EXT_LEGACY_SHUTDOWN 0x8 > > -#define SBI_CALL_LEGACY(which, arg0, arg1, arg2, arg3) ({ \ > +#define SBI_EXT_BASE 0x10 > + > +enum sbi_ext_base_fid { > + SBI_EXT_BASE_GET_SPEC_VERSION = 0, > + SBI_EXT_BASE_GET_IMP_ID, > + SBI_EXT_BASE_GET_IMP_VERSION, > + SBI_EXT_BASE_PROBE_EXT, > + SBI_EXT_BASE_GET_MVENDORID, > + SBI_EXT_BASE_GET_MARCHID, > + SBI_EXT_BASE_GET_MIMPID, > +}; > + > +#define SBI_CALL_LEGACY(ext, fid, arg0, arg1, arg2, arg3) ({ \ > 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 a7 asm ("a7") = (uintptr_t)(which); \ > + 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" (a7) \ > + : "+r" (a0), "+r" (a1) \ > + : "r" (a2), "r" (a3), "r" (a6), "r" (a7) \ > : "memory"); \ > a0; \ > }) I think instead of removing old convention we should use calling convention based on sbi_version detected at boot-time. > > /* Lazy implementations until SBI is finalized */ > -#define SBI_CALL_LEGACY_0(which) SBI_CALL_LEGACY(which, 0, 0, 0, 0) > -#define SBI_CALL_LEGACY_1(which, arg0) SBI_CALL_LEGACY(which, arg0, 0, 0, 0) > -#define SBI_CALL_LEGACY_2(which, arg0, arg1) \ > - SBI_CALL_LEGACY(which, arg0, arg1, 0, 0) > -#define SBI_CALL_LEGACY_3(which, arg0, arg1, arg2) \ > - SBI_CALL_LEGACY(which, arg0, arg1, arg2, 0) > -#define SBI_CALL_LEGACY_4(which, arg0, arg1, arg2, arg3) \ > - SBI_CALL_LEGACY(which, arg0, arg1, arg2, arg3) > +#define SBI_CALL_LEGACY_0(ext) SBI_CALL_LEGACY(ext, 0, 0, 0, 0, 0) > +#define SBI_CALL_LEGACY_1(ext, arg0) SBI_CALL_LEGACY(ext, 0, arg0, 0, 0, 0) > +#define SBI_CALL_LEGACY_2(ext, arg0, arg1) \ > + SBI_CALL_LEGACY(ext, 0, arg0, arg1, 0, 0) > +#define SBI_CALL_LEGACY_3(ext, arg0, arg1, arg2) \ > + SBI_CALL_LEGACY(ext, 0, arg0, arg1, arg2, 0) > +#define SBI_CALL_LEGACY_4(ext, arg0, arg1, arg2, arg3) \ > + SBI_CALL_LEGACY(ext, 0, arg0, arg1, arg2, arg3) > + > +extern unsigned long sbi_firmware_version; > +struct sbiret { > + long error; > + long value; > +}; > + > +void riscv_sbi_init(void); > +struct sbiret riscv_sbi_ecall(int ext, int fid, int arg0, int arg1, > + int arg2, int arg3); > + > +#define SBI_CALL_0(ext, fid) riscv_sbi_ecall(ext, fid, 0, 0, 0, 0) > +#define SBI_CALL_1(ext, fid, arg0) riscv_sbi_ecall(ext, fid, arg0, 0, 0, 0) > +#define SBI_CALL_2(ext, fid, arg0, arg1) \ > + riscv_sbi_ecall(ext, fid, arg0, arg1, 0, 0) > +#define SBI_CALL_3(ext, fid, arg0, arg1, arg2) \ > + riscv_sbi_ecall(ext, fid, arg0, arg1, arg2, 0) > +#define SBI_CALL_4(ext, fid, arg0, arg1, arg2, arg3) \ > + riscv_sbi_ecall(ext, fid, arg0, arg1, arg2, arg3) > + > > static inline void sbi_console_putchar(int ch) > { > @@ -99,4 +131,14 @@ static inline void sbi_remote_sfence_vma_asid(const unsigned long *hart_mask, > start, size, asid); > } To be sure that new kernels work fine on older kernel, we can be conservative and move all legacy SBI calls to kernel/sbi.c. All legacy SBI calls can check sbi_version and use appropriate SBI calling convention. This might be redundant if we can ensure that newer kernels work fine with older SBI v0.1 firmwares. > > +static inline unsigned long riscv_sbi_major_version(void) > +{ > + return (sbi_firmware_version >> 24) & 0x7f; > +} > + > +static inline unsigned long riscv_sbi_minor_version(void) > +{ > + return sbi_firmware_version & 0xffffff; > +} > + > #endif > diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile > index 2420d37d96de..faf862d26924 100644 > --- a/arch/riscv/kernel/Makefile > +++ b/arch/riscv/kernel/Makefile > @@ -17,6 +17,7 @@ obj-y += irq.o > obj-y += process.o > obj-y += ptrace.o > obj-y += reset.o > +obj-y += sbi.o > obj-y += setup.o > obj-y += signal.o > obj-y += syscall_table.o > diff --git a/arch/riscv/kernel/sbi.c b/arch/riscv/kernel/sbi.c > new file mode 100644 > index 000000000000..457b8cc0e9d9 > --- /dev/null > +++ b/arch/riscv/kernel/sbi.c > @@ -0,0 +1,50 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * SBI initialilization and base extension implementation. > + * > + * Copyright (c) 2019 Western Digital Corporation or its affiliates. > + */ > + > +#include <asm/sbi.h> > +#include <linux/sched.h> > + > +unsigned long sbi_firmware_version; Rename this too sbi_version or sbi_spec_version. The firmware version is different thing. > + > +struct sbiret riscv_sbi_ecall(int ext, int fid, int arg0, int arg1, > + int arg2, int arg3) > +{ > + struct sbiret ret; > + > + 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 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" (a6), "r" (a7) > + : "memory"); > + ret.error = a0; > + ret.value = a1; > + > + return ret; > +} > + > +static struct sbiret sbi_get_spec_version(void) > +{ > + return SBI_CALL_0(SBI_EXT_BASE, SBI_EXT_BASE_GET_SPEC_VERSION); > +} > + > +void riscv_sbi_init(void) > +{ > + struct sbiret ret; > + > + /* legacy SBI version*/ > + sbi_firmware_version = 0x1; > + ret = sbi_get_spec_version(); > + if (!ret.error) > + sbi_firmware_version = ret.value; > + pr_info("SBI version implemented in firmware [%lu:%lu]\n", > + riscv_sbi_major_version(), riscv_sbi_minor_version()); Should we not print SBI implementation ID and SBI firmware version. Regards, Anup > +} > diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c > index a990a6cb184f..4c324fd398c8 100644 > --- a/arch/riscv/kernel/setup.c > +++ b/arch/riscv/kernel/setup.c > @@ -21,6 +21,7 @@ > #include <asm/sections.h> > #include <asm/pgtable.h> > #include <asm/smp.h> > +#include <asm/sbi.h> > #include <asm/tlbflush.h> > #include <asm/thread_info.h> > > @@ -70,6 +71,7 @@ void __init setup_arch(char **cmdline_p) > swiotlb_init(1); > #endif > > + riscv_sbi_init(); > #ifdef CONFIG_SMP > setup_smp(); > #endif > -- > 2.21.0 >
> +#define SBI_EXT_BASE 0x10 I think you want an enum enumerating the extensions. > +#define SBI_CALL_LEGACY(ext, fid, arg0, arg1, arg2, arg3) ({ \ > 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 a7 asm ("a7") = (uintptr_t)(which); \ > + register uintptr_t a6 asm ("a6") = (uintptr_t)(fid); \ > + register uintptr_t a7 asm ("a7") = (uintptr_t)(ext); \ This seems to break the calling convention. I also think we should go back to the Unix platform working group and make the calling convention backwards compatible. There is really no advantage or disadvantag in swapping a6 and a7 in the calling convention itself, but doing so means you can just push the ext field in always and it will be ignored by the old sbi. > +struct sbiret riscv_sbi_ecall(int ext, int fid, int arg0, int arg1, > + int arg2, int arg3); > + > +#define SBI_CALL_0(ext, fid) riscv_sbi_ecall(ext, fid, 0, 0, 0, 0) > +#define SBI_CALL_1(ext, fid, arg0) riscv_sbi_ecall(ext, fid, arg0, 0, 0, 0) > +#define SBI_CALL_2(ext, fid, arg0, arg1) \ > + riscv_sbi_ecall(ext, fid, arg0, arg1, 0, 0) > +#define SBI_CALL_3(ext, fid, arg0, arg1, arg2) \ > + riscv_sbi_ecall(ext, fid, arg0, arg1, arg2, 0) > +#define SBI_CALL_4(ext, fid, arg0, arg1, arg2, arg3) \ > + riscv_sbi_ecall(ext, fid, arg0, arg1, arg2, arg3) Again, no point in having these wrappers. > +struct sbiret riscv_sbi_ecall(int ext, int fid, int arg0, int arg1, > + int arg2, int arg3) > +{ > + struct sbiret ret; > + > + 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 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" (a6), "r" (a7) > + : "memory"); > + ret.error = a0; > + ret.value = a1; > + > + return ret; Again much simpler done in pure asm.. > + /* legacy SBI version*/ > + sbi_firmware_version = 0x1; > + ret = sbi_get_spec_version(); > + if (!ret.error) > + sbi_firmware_version = ret.value; Why not: ret = sbi_get_spec_version(); if (ret.error) sbi_firmware_version = 0x1; /* legacy SBI */ else sbi_firmware_version = ret.value; btw, I'd find a calling convention that returns the value as a pointer much nicer than the return by a struct. Yes, the RISC-V ABI still returns that in registers, but it is a pain in the b**t to use. Without that we could simply pass the variable to fill by reference.
On Tue, 2019-08-27 at 11:39 +0300, Mike Rapoport wrote: > On Tue, Aug 27, 2019 at 01:53:23PM +0530, Anup Patel wrote: > > On Tue, Aug 27, 2019 at 1:28 PM Mike Rapoport <rppt@linux.ibm.com> > > wrote: > > > On Mon, Aug 26, 2019 at 04:32:56PM -0700, Atish Patra wrote: > > > > The SBI v0.2 introduces a base extension which is backward > > > > compatible > > > > with v0.1. Implement all helper functions and minimum required > > > > SBI > > > > calls from v0.2 for now. All other base extension function will > > > > be > > > > added later as per need. > > > > > > > > Signed-off-by: Atish Patra <atish.patra@wdc.com> > > > > --- > > > > arch/riscv/include/asm/sbi.h | 68 > > > > +++++++++++++++++++++++++++++------- > > > > arch/riscv/kernel/Makefile | 1 + > > > > arch/riscv/kernel/sbi.c | 50 ++++++++++++++++++++++++++ > > > > arch/riscv/kernel/setup.c | 2 ++ > > > > 4 files changed, 108 insertions(+), 13 deletions(-) > > > > create mode 100644 arch/riscv/kernel/sbi.c > > > > > > > > diff --git a/arch/riscv/include/asm/sbi.h > > > > b/arch/riscv/include/asm/sbi.h > > > > index 7f5ecaaaa0d7..4a4476956693 100644 > > > > --- a/arch/riscv/include/asm/sbi.h > > > > +++ b/arch/riscv/include/asm/sbi.h > > > > @@ -8,7 +8,6 @@ > > > > > > > > #include <linux/types.h> > > > > > > > > - > > > > #define SBI_EXT_LEGACY_SET_TIMER 0x0 > > > > #define SBI_EXT_LEGACY_CONSOLE_PUTCHAR 0x1 > > > > #define SBI_EXT_LEGACY_CONSOLE_GETCHAR 0x2 > > > > @@ -19,28 +18,61 @@ > > > > #define SBI_EXT_LEGACY_REMOTE_SFENCE_VMA_ASID 0x7 > > > > #define SBI_EXT_LEGACY_SHUTDOWN 0x8 > > > > > > > > -#define SBI_CALL_LEGACY(which, arg0, arg1, arg2, arg3) > > > > ({ \ > > > > +#define SBI_EXT_BASE 0x10 > > > > + > > > > +enum sbi_ext_base_fid { > > > > + SBI_EXT_BASE_GET_SPEC_VERSION = 0, > > > > + SBI_EXT_BASE_GET_IMP_ID, > > > > + SBI_EXT_BASE_GET_IMP_VERSION, > > > > + SBI_EXT_BASE_PROBE_EXT, > > > > + SBI_EXT_BASE_GET_MVENDORID, > > > > + SBI_EXT_BASE_GET_MARCHID, > > > > + SBI_EXT_BASE_GET_MIMPID, > > > > +}; > > > > + > > > > +#define SBI_CALL_LEGACY(ext, fid, arg0, arg1, arg2, arg3) ({ \ > > > > 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 a7 asm ("a7") = (uintptr_t)(which); \ > > > > + 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" (a7) \ > > > > + : "+r" (a0), "+r" (a1) \ > > > > + : "r" (a2), "r" (a3), "r" (a6), "r" (a7) \ > > > > > > Maybe I'm missing something, but how is this supposed to work on > > > systems > > > with SBI v0.1? Wouldn't this cause a mismatch in the registers? > > > > The SBI v0.2 has two major changes: > > 1. New improved calling convention which is backward compatible > > with SBI v0.1 so older kernels with SBI v0.1 will continue to work > > as-is. > > 2. Base set of mandatory SBI v0.2 calls which can be used to detect > > SBI version, check supported SBI calls and extentions. > > > > Old calling convention in SBI v0.1 was: > > Parameters: > > a0 -> arg0 > > a1 -> arg1 > > a2 -> arg2 > > a3 -> arg3 > > a7 -> function_id > > Return: > > a0 -> return value or error code > > > > In SBI v0.2, we have extension and function. Each SBI extension > > is a set of function. The new calling convention in SBI v0.2 is: > > Parameters: > > a0 -> arg0 > > a1 -> arg1 > > a2 -> arg2 > > a3 -> arg3 > > a6 -> function_id > > a7 -> extension_id > > Return: > > a0 -> error code > > a1 -> return value (optional) > > So with this patch SBI_CALL_LEGACY() uses SBI v0.2 convention, right? > Doesn't it mean that you cannot run a new kernel on a system with SBI > v0.1? > Thanks Anup for the earlier explaination. I have used SBI v0.2 convention for legacy calls as well to keep both in sync and more redable. I just set a6 to zero and do not use a1 as a return value. With this, both SBI implementation & kernel are backward compatible in both ways. Any newer kernels using SBI calls defined in 0.2 or later will use new convention(SBI_CALL_*) and older SBI calls will continue to will continue to use SBI_CALL_LEGACY*. I thought this is more readable but probably it is not. I will preserve the legacy convention and use different calling conventions based on the firmware version. > > All legacy SBI v0.1 functions can be thought of as separate > > extensions. That's how SBI v0.2 will be backward compatible. > > > > Regards, > > Anup > > > > > > : "memory"); \ > > > > a0; \ > > > > }) > > > > > > > > /* Lazy implementations until SBI is finalized */ > > > > -#define SBI_CALL_LEGACY_0(which) SBI_CALL_LEGACY(which, 0, 0, > > > > 0, 0) > > > > -#define SBI_CALL_LEGACY_1(which, arg0) SBI_CALL_LEGACY(which, > > > > arg0, 0, 0, 0) > > > > -#define SBI_CALL_LEGACY_2(which, arg0, arg1) \ > > > > - SBI_CALL_LEGACY(which, arg0, arg1, 0, 0) > > > > -#define SBI_CALL_LEGACY_3(which, arg0, arg1, arg2) \ > > > > - SBI_CALL_LEGACY(which, arg0, arg1, arg2, 0) > > > > -#define SBI_CALL_LEGACY_4(which, arg0, arg1, arg2, arg3) \ > > > > - SBI_CALL_LEGACY(which, arg0, arg1, arg2, arg3) > > > > +#define SBI_CALL_LEGACY_0(ext) SBI_CALL_LEGACY(ext, 0, 0, 0, > > > > 0, 0) > > > > +#define SBI_CALL_LEGACY_1(ext, arg0) SBI_CALL_LEGACY(ext, 0, > > > > arg0, 0, 0, 0) > > > > +#define SBI_CALL_LEGACY_2(ext, arg0, arg1) \ > > > > + SBI_CALL_LEGACY(ext, 0, arg0, arg1, 0, 0) > > > > +#define SBI_CALL_LEGACY_3(ext, arg0, arg1, arg2) \ > > > > + SBI_CALL_LEGACY(ext, 0, arg0, arg1, arg2, 0) > > > > +#define SBI_CALL_LEGACY_4(ext, arg0, arg1, arg2, arg3) \ > > > > + SBI_CALL_LEGACY(ext, 0, arg0, arg1, arg2, arg3) > > > > + > > > > +extern unsigned long sbi_firmware_version; > > > > +struct sbiret { > > > > + long error; > > > > + long value; > > > > +}; > > > > + > > > > +void riscv_sbi_init(void); > > > > +struct sbiret riscv_sbi_ecall(int ext, int fid, int arg0, int > > > > arg1, > > > > + int arg2, int arg3); > > > > + > > > > +#define SBI_CALL_0(ext, fid) riscv_sbi_ecall(ext, fid, 0, 0, > > > > 0, 0) > > > > +#define SBI_CALL_1(ext, fid, arg0) riscv_sbi_ecall(ext, fid, > > > > arg0, 0, 0, 0) > > > > +#define SBI_CALL_2(ext, fid, arg0, arg1) \ > > > > + riscv_sbi_ecall(ext, fid, arg0, arg1, 0, 0) > > > > +#define SBI_CALL_3(ext, fid, arg0, arg1, arg2) \ > > > > + riscv_sbi_ecall(ext, fid, arg0, arg1, arg2, 0) > > > > +#define SBI_CALL_4(ext, fid, arg0, arg1, arg2, arg3) \ > > > > + riscv_sbi_ecall(ext, fid, arg0, arg1, arg2, arg3) > > > > + > > > > > > > > static inline void sbi_console_putchar(int ch) > > > > { > > > > @@ -99,4 +131,14 @@ static inline void > > > > sbi_remote_sfence_vma_asid(const unsigned long *hart_mask, > > > > start, size, asid); > > > > } > > > > > > > > +static inline unsigned long riscv_sbi_major_version(void) > > > > +{ > > > > + return (sbi_firmware_version >> 24) & 0x7f; > > > > +} > > > > + > > > > +static inline unsigned long riscv_sbi_minor_version(void) > > > > +{ > > > > + return sbi_firmware_version & 0xffffff; > > > > +} > > > > + > > > > #endif > > > > diff --git a/arch/riscv/kernel/Makefile > > > > b/arch/riscv/kernel/Makefile > > > > index 2420d37d96de..faf862d26924 100644 > > > > --- a/arch/riscv/kernel/Makefile > > > > +++ b/arch/riscv/kernel/Makefile > > > > @@ -17,6 +17,7 @@ obj-y += irq.o > > > > obj-y += process.o > > > > obj-y += ptrace.o > > > > obj-y += reset.o > > > > +obj-y += sbi.o > > > > obj-y += setup.o > > > > obj-y += signal.o > > > > obj-y += syscall_table.o > > > > diff --git a/arch/riscv/kernel/sbi.c b/arch/riscv/kernel/sbi.c > > > > new file mode 100644 > > > > index 000000000000..457b8cc0e9d9 > > > > --- /dev/null > > > > +++ b/arch/riscv/kernel/sbi.c > > > > @@ -0,0 +1,50 @@ > > > > +// SPDX-License-Identifier: GPL-2.0-only > > > > +/* > > > > + * SBI initialilization and base extension implementation. > > > > + * > > > > + * Copyright (c) 2019 Western Digital Corporation or its > > > > affiliates. > > > > + */ > > > > + > > > > +#include <asm/sbi.h> > > > > +#include <linux/sched.h> > > > > + > > > > +unsigned long sbi_firmware_version; > > > > + > > > > +struct sbiret riscv_sbi_ecall(int ext, int fid, int arg0, int > > > > arg1, > > > > + int arg2, int arg3) > > > > +{ > > > > + struct sbiret ret; > > > > + > > > > + 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 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" (a6), "r" (a7) > > > > + : "memory"); > > > > + ret.error = a0; > > > > + ret.value = a1; > > > > + > > > > + return ret; > > > > +} > > > > + > > > > +static struct sbiret sbi_get_spec_version(void) > > > > +{ > > > > + return SBI_CALL_0(SBI_EXT_BASE, > > > > SBI_EXT_BASE_GET_SPEC_VERSION); > > > > +} > > > > + > > > > +void riscv_sbi_init(void) > > > > +{ > > > > + struct sbiret ret; > > > > + > > > > + /* legacy SBI version*/ > > > > + sbi_firmware_version = 0x1; > > > > + ret = sbi_get_spec_version(); > > > > + if (!ret.error) > > > > + sbi_firmware_version = ret.value; > > > > + pr_info("SBI version implemented in firmware > > > > [%lu:%lu]\n", > > > > + riscv_sbi_major_version(), > > > > riscv_sbi_minor_version()); > > > > +} > > > > diff --git a/arch/riscv/kernel/setup.c > > > > b/arch/riscv/kernel/setup.c > > > > index a990a6cb184f..4c324fd398c8 100644 > > > > --- a/arch/riscv/kernel/setup.c > > > > +++ b/arch/riscv/kernel/setup.c > > > > @@ -21,6 +21,7 @@ > > > > #include <asm/sections.h> > > > > #include <asm/pgtable.h> > > > > #include <asm/smp.h> > > > > +#include <asm/sbi.h> > > > > #include <asm/tlbflush.h> > > > > #include <asm/thread_info.h> > > > > > > > > @@ -70,6 +71,7 @@ void __init setup_arch(char **cmdline_p) > > > > swiotlb_init(1); > > > > #endif > > > > > > > > + riscv_sbi_init(); > > > > #ifdef CONFIG_SMP > > > > setup_smp(); > > > > #endif > > > > -- > > > > 2.21.0 > > > > > > > > > > > > _______________________________________________ > > > > linux-riscv mailing list > > > > linux-riscv@lists.infradead.org > > > > http://lists.infradead.org/mailman/listinfo/linux-riscv > > > > > > -- > > > Sincerely yours, > > > Mike. > > >
On Tue, 2019-08-27 at 15:06 +0530, Anup Patel wrote: > On Tue, Aug 27, 2019 at 5:03 AM Atish Patra <atish.patra@wdc.com> > wrote: > > The SBI v0.2 introduces a base extension which is backward > > compatible > > with v0.1. Implement all helper functions and minimum required SBI > > calls from v0.2 for now. All other base extension function will be > > added later as per need. > > > > Signed-off-by: Atish Patra <atish.patra@wdc.com> > > --- > > arch/riscv/include/asm/sbi.h | 68 +++++++++++++++++++++++++++++--- > > ---- > > arch/riscv/kernel/Makefile | 1 + > > arch/riscv/kernel/sbi.c | 50 ++++++++++++++++++++++++++ > > arch/riscv/kernel/setup.c | 2 ++ > > 4 files changed, 108 insertions(+), 13 deletions(-) > > create mode 100644 arch/riscv/kernel/sbi.c > > > > diff --git a/arch/riscv/include/asm/sbi.h > > b/arch/riscv/include/asm/sbi.h > > index 7f5ecaaaa0d7..4a4476956693 100644 > > --- a/arch/riscv/include/asm/sbi.h > > +++ b/arch/riscv/include/asm/sbi.h > > @@ -8,7 +8,6 @@ > > > > #include <linux/types.h> > > > > - > > #define SBI_EXT_LEGACY_SET_TIMER 0x0 > > #define SBI_EXT_LEGACY_CONSOLE_PUTCHAR 0x1 > > #define SBI_EXT_LEGACY_CONSOLE_GETCHAR 0x2 > > @@ -19,28 +18,61 @@ > > #define SBI_EXT_LEGACY_REMOTE_SFENCE_VMA_ASID 0x7 > > #define SBI_EXT_LEGACY_SHUTDOWN 0x8 > > > > -#define SBI_CALL_LEGACY(which, arg0, arg1, arg2, arg3) > > ({ \ > > +#define SBI_EXT_BASE 0x10 > > + > > +enum sbi_ext_base_fid { > > + SBI_EXT_BASE_GET_SPEC_VERSION = 0, > > + SBI_EXT_BASE_GET_IMP_ID, > > + SBI_EXT_BASE_GET_IMP_VERSION, > > + SBI_EXT_BASE_PROBE_EXT, > > + SBI_EXT_BASE_GET_MVENDORID, > > + SBI_EXT_BASE_GET_MARCHID, > > + SBI_EXT_BASE_GET_MIMPID, > > +}; > > + > > +#define SBI_CALL_LEGACY(ext, fid, arg0, arg1, arg2, arg3) ({ \ > > 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 a7 asm ("a7") = (uintptr_t)(which); \ > > + 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" (a7) \ > > + : "+r" (a0), "+r" (a1) \ > > + : "r" (a2), "r" (a3), "r" (a6), "r" (a7) \ > > : "memory"); \ > > a0; \ > > }) > > I think instead of removing old convention we should use > calling convention based on sbi_version detected at boot-time. > > > /* Lazy implementations until SBI is finalized */ > > -#define SBI_CALL_LEGACY_0(which) SBI_CALL_LEGACY(which, 0, 0, 0, > > 0) > > -#define SBI_CALL_LEGACY_1(which, arg0) SBI_CALL_LEGACY(which, > > arg0, 0, 0, 0) > > -#define SBI_CALL_LEGACY_2(which, arg0, arg1) \ > > - SBI_CALL_LEGACY(which, arg0, arg1, 0, 0) > > -#define SBI_CALL_LEGACY_3(which, arg0, arg1, arg2) \ > > - SBI_CALL_LEGACY(which, arg0, arg1, arg2, 0) > > -#define SBI_CALL_LEGACY_4(which, arg0, arg1, arg2, arg3) \ > > - SBI_CALL_LEGACY(which, arg0, arg1, arg2, arg3) > > +#define SBI_CALL_LEGACY_0(ext) SBI_CALL_LEGACY(ext, 0, 0, 0, 0, 0) > > +#define SBI_CALL_LEGACY_1(ext, arg0) SBI_CALL_LEGACY(ext, 0, arg0, > > 0, 0, 0) > > +#define SBI_CALL_LEGACY_2(ext, arg0, arg1) \ > > + SBI_CALL_LEGACY(ext, 0, arg0, arg1, 0, 0) > > +#define SBI_CALL_LEGACY_3(ext, arg0, arg1, arg2) \ > > + SBI_CALL_LEGACY(ext, 0, arg0, arg1, arg2, 0) > > +#define SBI_CALL_LEGACY_4(ext, arg0, arg1, arg2, arg3) \ > > + SBI_CALL_LEGACY(ext, 0, arg0, arg1, arg2, arg3) > > + > > +extern unsigned long sbi_firmware_version; > > +struct sbiret { > > + long error; > > + long value; > > +}; > > + > > +void riscv_sbi_init(void); > > +struct sbiret riscv_sbi_ecall(int ext, int fid, int arg0, int > > arg1, > > + int arg2, int arg3); > > + > > +#define SBI_CALL_0(ext, fid) riscv_sbi_ecall(ext, fid, 0, 0, 0, 0) > > +#define SBI_CALL_1(ext, fid, arg0) riscv_sbi_ecall(ext, fid, arg0, > > 0, 0, 0) > > +#define SBI_CALL_2(ext, fid, arg0, arg1) \ > > + riscv_sbi_ecall(ext, fid, arg0, arg1, 0, 0) > > +#define SBI_CALL_3(ext, fid, arg0, arg1, arg2) \ > > + riscv_sbi_ecall(ext, fid, arg0, arg1, arg2, 0) > > +#define SBI_CALL_4(ext, fid, arg0, arg1, arg2, arg3) \ > > + riscv_sbi_ecall(ext, fid, arg0, arg1, arg2, arg3) > > + > > > > static inline void sbi_console_putchar(int ch) > > { > > @@ -99,4 +131,14 @@ static inline void > > sbi_remote_sfence_vma_asid(const unsigned long *hart_mask, > > start, size, asid); > > } > > To be sure that new kernels work fine on older kernel, we > can be conservative and move all legacy SBI calls to kernel/sbi.c. > All legacy SBI calls can check sbi_version and use appropriate > SBI calling convention. > > This might be redundant if we can ensure that newer kernels > work fine with older SBI v0.1 firmwares. Yes that's why I didnot want to do it first time. How about this ? Use only 0.2 convention only and get rid of the 0.1 convention. As it is anyways backward compatible with 0.1, we don't need a if else clause. The legacy calls will not use any value set in a1 and always set 0 in function id (a6). > > > +static inline unsigned long riscv_sbi_major_version(void) > > +{ > > + return (sbi_firmware_version >> 24) & 0x7f; > > +} > > + > > +static inline unsigned long riscv_sbi_minor_version(void) > > +{ > > + return sbi_firmware_version & 0xffffff; > > +} > > + > > #endif > > diff --git a/arch/riscv/kernel/Makefile > > b/arch/riscv/kernel/Makefile > > index 2420d37d96de..faf862d26924 100644 > > --- a/arch/riscv/kernel/Makefile > > +++ b/arch/riscv/kernel/Makefile > > @@ -17,6 +17,7 @@ obj-y += irq.o > > obj-y += process.o > > obj-y += ptrace.o > > obj-y += reset.o > > +obj-y += sbi.o > > obj-y += setup.o > > obj-y += signal.o > > obj-y += syscall_table.o > > diff --git a/arch/riscv/kernel/sbi.c b/arch/riscv/kernel/sbi.c > > new file mode 100644 > > index 000000000000..457b8cc0e9d9 > > --- /dev/null > > +++ b/arch/riscv/kernel/sbi.c > > @@ -0,0 +1,50 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * SBI initialilization and base extension implementation. > > + * > > + * Copyright (c) 2019 Western Digital Corporation or its > > affiliates. > > + */ > > + > > +#include <asm/sbi.h> > > +#include <linux/sched.h> > > + > > +unsigned long sbi_firmware_version; > > Rename this too sbi_version or sbi_spec_version. > The firmware version is different thing. > ok. > > + > > +struct sbiret riscv_sbi_ecall(int ext, int fid, int arg0, int > > arg1, > > + int arg2, int arg3) > > +{ > > + struct sbiret ret; > > + > > + 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 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" (a6), "r" (a7) > > + : "memory"); > > + ret.error = a0; > > + ret.value = a1; > > + > > + return ret; > > +} > > + > > +static struct sbiret sbi_get_spec_version(void) > > +{ > > + return SBI_CALL_0(SBI_EXT_BASE, > > SBI_EXT_BASE_GET_SPEC_VERSION); > > +} > > + > > +void riscv_sbi_init(void) > > +{ > > + struct sbiret ret; > > + > > + /* legacy SBI version*/ > > + sbi_firmware_version = 0x1; > > + ret = sbi_get_spec_version(); > > + if (!ret.error) > > + sbi_firmware_version = ret.value; > > + pr_info("SBI version implemented in firmware [%lu:%lu]\n", > > + riscv_sbi_major_version(), > > riscv_sbi_minor_version()); > > Should we not print SBI implementation ID and SBI firmware version. > Eventually yes. I wanted to have this series minimal implementation and build upon it. I will go ahead and add those. > Regards, > Anup > > > +} > > diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c > > index a990a6cb184f..4c324fd398c8 100644 > > --- a/arch/riscv/kernel/setup.c > > +++ b/arch/riscv/kernel/setup.c > > @@ -21,6 +21,7 @@ > > #include <asm/sections.h> > > #include <asm/pgtable.h> > > #include <asm/smp.h> > > +#include <asm/sbi.h> > > #include <asm/tlbflush.h> > > #include <asm/thread_info.h> > > > > @@ -70,6 +71,7 @@ void __init setup_arch(char **cmdline_p) > > swiotlb_init(1); > > #endif > > > > + riscv_sbi_init(); > > #ifdef CONFIG_SMP > > setup_smp(); > > #endif > > -- > > 2.21.0 > >
diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h index 7f5ecaaaa0d7..4a4476956693 100644 --- a/arch/riscv/include/asm/sbi.h +++ b/arch/riscv/include/asm/sbi.h @@ -8,7 +8,6 @@ #include <linux/types.h> - #define SBI_EXT_LEGACY_SET_TIMER 0x0 #define SBI_EXT_LEGACY_CONSOLE_PUTCHAR 0x1 #define SBI_EXT_LEGACY_CONSOLE_GETCHAR 0x2 @@ -19,28 +18,61 @@ #define SBI_EXT_LEGACY_REMOTE_SFENCE_VMA_ASID 0x7 #define SBI_EXT_LEGACY_SHUTDOWN 0x8 -#define SBI_CALL_LEGACY(which, arg0, arg1, arg2, arg3) ({ \ +#define SBI_EXT_BASE 0x10 + +enum sbi_ext_base_fid { + SBI_EXT_BASE_GET_SPEC_VERSION = 0, + SBI_EXT_BASE_GET_IMP_ID, + SBI_EXT_BASE_GET_IMP_VERSION, + SBI_EXT_BASE_PROBE_EXT, + SBI_EXT_BASE_GET_MVENDORID, + SBI_EXT_BASE_GET_MARCHID, + SBI_EXT_BASE_GET_MIMPID, +}; + +#define SBI_CALL_LEGACY(ext, fid, arg0, arg1, arg2, arg3) ({ \ 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 a7 asm ("a7") = (uintptr_t)(which); \ + 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" (a7) \ + : "+r" (a0), "+r" (a1) \ + : "r" (a2), "r" (a3), "r" (a6), "r" (a7) \ : "memory"); \ a0; \ }) /* Lazy implementations until SBI is finalized */ -#define SBI_CALL_LEGACY_0(which) SBI_CALL_LEGACY(which, 0, 0, 0, 0) -#define SBI_CALL_LEGACY_1(which, arg0) SBI_CALL_LEGACY(which, arg0, 0, 0, 0) -#define SBI_CALL_LEGACY_2(which, arg0, arg1) \ - SBI_CALL_LEGACY(which, arg0, arg1, 0, 0) -#define SBI_CALL_LEGACY_3(which, arg0, arg1, arg2) \ - SBI_CALL_LEGACY(which, arg0, arg1, arg2, 0) -#define SBI_CALL_LEGACY_4(which, arg0, arg1, arg2, arg3) \ - SBI_CALL_LEGACY(which, arg0, arg1, arg2, arg3) +#define SBI_CALL_LEGACY_0(ext) SBI_CALL_LEGACY(ext, 0, 0, 0, 0, 0) +#define SBI_CALL_LEGACY_1(ext, arg0) SBI_CALL_LEGACY(ext, 0, arg0, 0, 0, 0) +#define SBI_CALL_LEGACY_2(ext, arg0, arg1) \ + SBI_CALL_LEGACY(ext, 0, arg0, arg1, 0, 0) +#define SBI_CALL_LEGACY_3(ext, arg0, arg1, arg2) \ + SBI_CALL_LEGACY(ext, 0, arg0, arg1, arg2, 0) +#define SBI_CALL_LEGACY_4(ext, arg0, arg1, arg2, arg3) \ + SBI_CALL_LEGACY(ext, 0, arg0, arg1, arg2, arg3) + +extern unsigned long sbi_firmware_version; +struct sbiret { + long error; + long value; +}; + +void riscv_sbi_init(void); +struct sbiret riscv_sbi_ecall(int ext, int fid, int arg0, int arg1, + int arg2, int arg3); + +#define SBI_CALL_0(ext, fid) riscv_sbi_ecall(ext, fid, 0, 0, 0, 0) +#define SBI_CALL_1(ext, fid, arg0) riscv_sbi_ecall(ext, fid, arg0, 0, 0, 0) +#define SBI_CALL_2(ext, fid, arg0, arg1) \ + riscv_sbi_ecall(ext, fid, arg0, arg1, 0, 0) +#define SBI_CALL_3(ext, fid, arg0, arg1, arg2) \ + riscv_sbi_ecall(ext, fid, arg0, arg1, arg2, 0) +#define SBI_CALL_4(ext, fid, arg0, arg1, arg2, arg3) \ + riscv_sbi_ecall(ext, fid, arg0, arg1, arg2, arg3) + static inline void sbi_console_putchar(int ch) { @@ -99,4 +131,14 @@ static inline void sbi_remote_sfence_vma_asid(const unsigned long *hart_mask, start, size, asid); } +static inline unsigned long riscv_sbi_major_version(void) +{ + return (sbi_firmware_version >> 24) & 0x7f; +} + +static inline unsigned long riscv_sbi_minor_version(void) +{ + return sbi_firmware_version & 0xffffff; +} + #endif diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile index 2420d37d96de..faf862d26924 100644 --- a/arch/riscv/kernel/Makefile +++ b/arch/riscv/kernel/Makefile @@ -17,6 +17,7 @@ obj-y += irq.o obj-y += process.o obj-y += ptrace.o obj-y += reset.o +obj-y += sbi.o obj-y += setup.o obj-y += signal.o obj-y += syscall_table.o diff --git a/arch/riscv/kernel/sbi.c b/arch/riscv/kernel/sbi.c new file mode 100644 index 000000000000..457b8cc0e9d9 --- /dev/null +++ b/arch/riscv/kernel/sbi.c @@ -0,0 +1,50 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * SBI initialilization and base extension implementation. + * + * Copyright (c) 2019 Western Digital Corporation or its affiliates. + */ + +#include <asm/sbi.h> +#include <linux/sched.h> + +unsigned long sbi_firmware_version; + +struct sbiret riscv_sbi_ecall(int ext, int fid, int arg0, int arg1, + int arg2, int arg3) +{ + struct sbiret ret; + + 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 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" (a6), "r" (a7) + : "memory"); + ret.error = a0; + ret.value = a1; + + return ret; +} + +static struct sbiret sbi_get_spec_version(void) +{ + return SBI_CALL_0(SBI_EXT_BASE, SBI_EXT_BASE_GET_SPEC_VERSION); +} + +void riscv_sbi_init(void) +{ + struct sbiret ret; + + /* legacy SBI version*/ + sbi_firmware_version = 0x1; + ret = sbi_get_spec_version(); + if (!ret.error) + sbi_firmware_version = ret.value; + pr_info("SBI version implemented in firmware [%lu:%lu]\n", + riscv_sbi_major_version(), riscv_sbi_minor_version()); +} diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c index a990a6cb184f..4c324fd398c8 100644 --- a/arch/riscv/kernel/setup.c +++ b/arch/riscv/kernel/setup.c @@ -21,6 +21,7 @@ #include <asm/sections.h> #include <asm/pgtable.h> #include <asm/smp.h> +#include <asm/sbi.h> #include <asm/tlbflush.h> #include <asm/thread_info.h> @@ -70,6 +71,7 @@ void __init setup_arch(char **cmdline_p) swiotlb_init(1); #endif + riscv_sbi_init(); #ifdef CONFIG_SMP setup_smp(); #endif
The SBI v0.2 introduces a base extension which is backward compatible with v0.1. Implement all helper functions and minimum required SBI calls from v0.2 for now. All other base extension function will be added later as per need. Signed-off-by: Atish Patra <atish.patra@wdc.com> --- arch/riscv/include/asm/sbi.h | 68 +++++++++++++++++++++++++++++------- arch/riscv/kernel/Makefile | 1 + arch/riscv/kernel/sbi.c | 50 ++++++++++++++++++++++++++ arch/riscv/kernel/setup.c | 2 ++ 4 files changed, 108 insertions(+), 13 deletions(-) create mode 100644 arch/riscv/kernel/sbi.c