* [RFC PATCH 0/2] Add support for SBI version to 0.2
@ 2019-08-26 23:32 Atish Patra
2019-08-26 23:32 ` [RFC PATCH 1/2] RISC-V: Mark existing SBI as legacy SBI Atish Patra
` (2 more replies)
0 siblings, 3 replies; 27+ messages in thread
From: Atish Patra @ 2019-08-26 23:32 UTC (permalink / raw)
To: linux-kernel
Cc: Atish Patra, Alan Kao, Albert Ou, Alexios Zavras, Anup Patel,
Gary Guo, Greg Kroah-Hartman, linux-riscv, Mike Rapoport,
Palmer Dabbelt, Paul Walmsley, Thomas Gleixner
This patch series aims to add support for SBI specification version
v0.2. It doesn't break compatibility with any v0.1 implementation.
Internally, all the v0.1 calls are just renamed to legacy to be in
sync with specification [1].
The patches for v0.2 support in OpenSBI are available at
http://lists.infradead.org/pipermail/opensbi/2019-August/000422.html
[1] https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.adoc
Atish Patra (2):
RISC-V: Mark existing SBI as legacy SBI.
RISC-V: Add basic support for SBI v0.2
arch/riscv/include/asm/sbi.h | 109 +++++++++++++++++++++++++----------
arch/riscv/kernel/Makefile | 1 +
arch/riscv/kernel/sbi.c | 50 ++++++++++++++++
arch/riscv/kernel/setup.c | 2 +
4 files changed, 131 insertions(+), 31 deletions(-)
create mode 100644 arch/riscv/kernel/sbi.c
--
2.21.0
^ permalink raw reply [flat|nested] 27+ messages in thread* [RFC PATCH 1/2] RISC-V: Mark existing SBI as legacy SBI. 2019-08-26 23:32 [RFC PATCH 0/2] Add support for SBI version to 0.2 Atish Patra @ 2019-08-26 23:32 ` Atish Patra 2019-08-27 7:51 ` Mike Rapoport 2019-08-27 14:03 ` Christoph Hellwig 2019-08-26 23:32 ` [RFC PATCH 2/2] RISC-V: Add basic support for SBI v0.2 Atish Patra 2019-08-27 14:46 ` [RFC PATCH 0/2] Add support for SBI version to 0.2 Christoph Hellwig 2 siblings, 2 replies; 27+ messages in thread From: Atish Patra @ 2019-08-26 23:32 UTC (permalink / raw) To: linux-kernel Cc: Atish Patra, Alan Kao, Albert Ou, Alexios Zavras, Anup Patel, Gary Guo, Greg Kroah-Hartman, linux-riscv, Mike Rapoport, Palmer Dabbelt, Paul Walmsley, Thomas Gleixner As per the new SBI specification, current SBI implementation is defined as legacy and will be removed/replaced in future. Rename existing implementation to reflect that. This patch is just a preparatory patch for SBI v0.2 and doesn't introduce any functional changes. Signed-off-by: Atish Patra <atish.patra@wdc.com> --- arch/riscv/include/asm/sbi.h | 61 +++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 28 deletions(-) diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h index 21134b3ef404..7f5ecaaaa0d7 100644 --- a/arch/riscv/include/asm/sbi.h +++ b/arch/riscv/include/asm/sbi.h @@ -8,17 +8,18 @@ #include <linux/types.h> -#define SBI_SET_TIMER 0 -#define SBI_CONSOLE_PUTCHAR 1 -#define SBI_CONSOLE_GETCHAR 2 -#define SBI_CLEAR_IPI 3 -#define SBI_SEND_IPI 4 -#define SBI_REMOTE_FENCE_I 5 -#define SBI_REMOTE_SFENCE_VMA 6 -#define SBI_REMOTE_SFENCE_VMA_ASID 7 -#define SBI_SHUTDOWN 8 - -#define SBI_CALL(which, arg0, arg1, arg2, arg3) ({ \ + +#define SBI_EXT_LEGACY_SET_TIMER 0x0 +#define SBI_EXT_LEGACY_CONSOLE_PUTCHAR 0x1 +#define SBI_EXT_LEGACY_CONSOLE_GETCHAR 0x2 +#define SBI_EXT_LEGACY_CLEAR_IPI 0x3 +#define SBI_EXT_LEGACY_SEND_IPI 0x4 +#define SBI_EXT_LEGACY_REMOTE_FENCE_I 0x5 +#define SBI_EXT_LEGACY_REMOTE_SFENCE_VMA 0x6 +#define SBI_EXT_LEGACY_REMOTE_SFENCE_VMA_ASID 0x7 +#define SBI_EXT_LEGACY_SHUTDOWN 0x8 + +#define SBI_CALL_LEGACY(which, 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); \ @@ -32,58 +33,61 @@ }) /* Lazy implementations until SBI is finalized */ -#define SBI_CALL_0(which) SBI_CALL(which, 0, 0, 0, 0) -#define SBI_CALL_1(which, arg0) SBI_CALL(which, arg0, 0, 0, 0) -#define SBI_CALL_2(which, arg0, arg1) SBI_CALL(which, arg0, arg1, 0, 0) -#define SBI_CALL_3(which, arg0, arg1, arg2) \ - SBI_CALL(which, arg0, arg1, arg2, 0) -#define SBI_CALL_4(which, arg0, arg1, arg2, arg3) \ - SBI_CALL(which, arg0, arg1, arg2, arg3) +#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) static inline void sbi_console_putchar(int ch) { - SBI_CALL_1(SBI_CONSOLE_PUTCHAR, ch); + SBI_CALL_LEGACY_1(SBI_EXT_LEGACY_CONSOLE_PUTCHAR, ch); } static inline int sbi_console_getchar(void) { - return SBI_CALL_0(SBI_CONSOLE_GETCHAR); + return SBI_CALL_LEGACY_0(SBI_EXT_LEGACY_CONSOLE_GETCHAR); } static inline void sbi_set_timer(uint64_t stime_value) { #if __riscv_xlen == 32 - SBI_CALL_2(SBI_SET_TIMER, stime_value, stime_value >> 32); + SBI_CALL_LEGACY_2(SBI_EXT_LEGACY_SET_TIMER, stime_value, + stime_value >> 32); #else - SBI_CALL_1(SBI_SET_TIMER, stime_value); + SBI_CALL_LEGACY_1(SBI_EXT_LEGACY_SET_TIMER, stime_value); #endif } static inline void sbi_shutdown(void) { - SBI_CALL_0(SBI_SHUTDOWN); + SBI_CALL_LEGACY_0(SBI_EXT_LEGACY_SHUTDOWN); } static inline void sbi_clear_ipi(void) { - SBI_CALL_0(SBI_CLEAR_IPI); + SBI_CALL_LEGACY_0(SBI_EXT_LEGACY_CLEAR_IPI); } static inline void sbi_send_ipi(const unsigned long *hart_mask) { - SBI_CALL_1(SBI_SEND_IPI, hart_mask); + SBI_CALL_LEGACY_1(SBI_EXT_LEGACY_SEND_IPI, hart_mask); } static inline void sbi_remote_fence_i(const unsigned long *hart_mask) { - SBI_CALL_1(SBI_REMOTE_FENCE_I, hart_mask); + SBI_CALL_LEGACY_1(SBI_EXT_LEGACY_REMOTE_FENCE_I, hart_mask); } static inline void sbi_remote_sfence_vma(const unsigned long *hart_mask, unsigned long start, unsigned long size) { - SBI_CALL_3(SBI_REMOTE_SFENCE_VMA, hart_mask, start, size); + SBI_CALL_LEGACY_3(SBI_EXT_LEGACY_REMOTE_SFENCE_VMA, hart_mask, + start, size); } static inline void sbi_remote_sfence_vma_asid(const unsigned long *hart_mask, @@ -91,7 +95,8 @@ static inline void sbi_remote_sfence_vma_asid(const unsigned long *hart_mask, unsigned long size, unsigned long asid) { - SBI_CALL_4(SBI_REMOTE_SFENCE_VMA_ASID, hart_mask, start, size, asid); + SBI_CALL_LEGACY_4(SBI_EXT_LEGACY_REMOTE_SFENCE_VMA_ASID, hart_mask, + start, size, asid); } #endif -- 2.21.0 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 1/2] RISC-V: Mark existing SBI as legacy SBI. 2019-08-26 23:32 ` [RFC PATCH 1/2] RISC-V: Mark existing SBI as legacy SBI Atish Patra @ 2019-08-27 7:51 ` Mike Rapoport 2019-08-27 8:28 ` Anup Patel 2019-08-27 20:34 ` Atish Patra 2019-08-27 14:03 ` Christoph Hellwig 1 sibling, 2 replies; 27+ messages in thread From: Mike Rapoport @ 2019-08-27 7:51 UTC (permalink / raw) To: Atish Patra Cc: linux-kernel, Albert Ou, Alan Kao, Alexios Zavras, Anup Patel, Palmer Dabbelt, Paul Walmsley, Gary Guo, Greg Kroah-Hartman, linux-riscv, Thomas Gleixner On Mon, Aug 26, 2019 at 04:32:55PM -0700, Atish Patra wrote: > As per the new SBI specification, current SBI implementation is > defined as legacy and will be removed/replaced in future. > > Rename existing implementation to reflect that. This patch is just > a preparatory patch for SBI v0.2 and doesn't introduce any functional > changes. > > Signed-off-by: Atish Patra <atish.patra@wdc.com> > --- > arch/riscv/include/asm/sbi.h | 61 +++++++++++++++++++----------------- > 1 file changed, 33 insertions(+), 28 deletions(-) > > diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h > index 21134b3ef404..7f5ecaaaa0d7 100644 > --- a/arch/riscv/include/asm/sbi.h > +++ b/arch/riscv/include/asm/sbi.h > @@ -8,17 +8,18 @@ > > #include <linux/types.h> > > -#define SBI_SET_TIMER 0 > -#define SBI_CONSOLE_PUTCHAR 1 > -#define SBI_CONSOLE_GETCHAR 2 > -#define SBI_CLEAR_IPI 3 > -#define SBI_SEND_IPI 4 > -#define SBI_REMOTE_FENCE_I 5 > -#define SBI_REMOTE_SFENCE_VMA 6 > -#define SBI_REMOTE_SFENCE_VMA_ASID 7 > -#define SBI_SHUTDOWN 8 > - > -#define SBI_CALL(which, arg0, arg1, arg2, arg3) ({ \ > + > +#define SBI_EXT_LEGACY_SET_TIMER 0x0 > +#define SBI_EXT_LEGACY_CONSOLE_PUTCHAR 0x1 > +#define SBI_EXT_LEGACY_CONSOLE_GETCHAR 0x2 > +#define SBI_EXT_LEGACY_CLEAR_IPI 0x3 > +#define SBI_EXT_LEGACY_SEND_IPI 0x4 > +#define SBI_EXT_LEGACY_REMOTE_FENCE_I 0x5 > +#define SBI_EXT_LEGACY_REMOTE_SFENCE_VMA 0x6 > +#define SBI_EXT_LEGACY_REMOTE_SFENCE_VMA_ASID 0x7 > +#define SBI_EXT_LEGACY_SHUTDOWN 0x8 I can't say I'm closely following RISC-V development, but what will happen when SBI v0.3 will come out and will render v0.2 legacy? Won't we need another similar renaming then? > +#define SBI_CALL_LEGACY(which, 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); \ > @@ -32,58 +33,61 @@ > }) > > /* Lazy implementations until SBI is finalized */ > -#define SBI_CALL_0(which) SBI_CALL(which, 0, 0, 0, 0) > -#define SBI_CALL_1(which, arg0) SBI_CALL(which, arg0, 0, 0, 0) > -#define SBI_CALL_2(which, arg0, arg1) SBI_CALL(which, arg0, arg1, 0, 0) > -#define SBI_CALL_3(which, arg0, arg1, arg2) \ > - SBI_CALL(which, arg0, arg1, arg2, 0) > -#define SBI_CALL_4(which, arg0, arg1, arg2, arg3) \ > - SBI_CALL(which, arg0, arg1, arg2, arg3) > +#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) > > static inline void sbi_console_putchar(int ch) > { > - SBI_CALL_1(SBI_CONSOLE_PUTCHAR, ch); > + SBI_CALL_LEGACY_1(SBI_EXT_LEGACY_CONSOLE_PUTCHAR, ch); > } > > static inline int sbi_console_getchar(void) > { > - return SBI_CALL_0(SBI_CONSOLE_GETCHAR); > + return SBI_CALL_LEGACY_0(SBI_EXT_LEGACY_CONSOLE_GETCHAR); > } > > static inline void sbi_set_timer(uint64_t stime_value) > { > #if __riscv_xlen == 32 > - SBI_CALL_2(SBI_SET_TIMER, stime_value, stime_value >> 32); > + SBI_CALL_LEGACY_2(SBI_EXT_LEGACY_SET_TIMER, stime_value, > + stime_value >> 32); > #else > - SBI_CALL_1(SBI_SET_TIMER, stime_value); > + SBI_CALL_LEGACY_1(SBI_EXT_LEGACY_SET_TIMER, stime_value); > #endif > } > > static inline void sbi_shutdown(void) > { > - SBI_CALL_0(SBI_SHUTDOWN); > + SBI_CALL_LEGACY_0(SBI_EXT_LEGACY_SHUTDOWN); > } > > static inline void sbi_clear_ipi(void) > { > - SBI_CALL_0(SBI_CLEAR_IPI); > + SBI_CALL_LEGACY_0(SBI_EXT_LEGACY_CLEAR_IPI); > } > > static inline void sbi_send_ipi(const unsigned long *hart_mask) > { > - SBI_CALL_1(SBI_SEND_IPI, hart_mask); > + SBI_CALL_LEGACY_1(SBI_EXT_LEGACY_SEND_IPI, hart_mask); > } > > static inline void sbi_remote_fence_i(const unsigned long *hart_mask) > { > - SBI_CALL_1(SBI_REMOTE_FENCE_I, hart_mask); > + SBI_CALL_LEGACY_1(SBI_EXT_LEGACY_REMOTE_FENCE_I, hart_mask); > } > > static inline void sbi_remote_sfence_vma(const unsigned long *hart_mask, > unsigned long start, > unsigned long size) > { > - SBI_CALL_3(SBI_REMOTE_SFENCE_VMA, hart_mask, start, size); > + SBI_CALL_LEGACY_3(SBI_EXT_LEGACY_REMOTE_SFENCE_VMA, hart_mask, > + start, size); > } > > static inline void sbi_remote_sfence_vma_asid(const unsigned long *hart_mask, > @@ -91,7 +95,8 @@ static inline void sbi_remote_sfence_vma_asid(const unsigned long *hart_mask, > unsigned long size, > unsigned long asid) > { > - SBI_CALL_4(SBI_REMOTE_SFENCE_VMA_ASID, hart_mask, start, size, asid); > + SBI_CALL_LEGACY_4(SBI_EXT_LEGACY_REMOTE_SFENCE_VMA_ASID, hart_mask, > + start, size, asid); > } > > #endif > -- > 2.21.0 > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv > -- Sincerely yours, Mike. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 1/2] RISC-V: Mark existing SBI as legacy SBI. 2019-08-27 7:51 ` Mike Rapoport @ 2019-08-27 8:28 ` Anup Patel 2019-08-27 8:37 ` Mike Rapoport 2019-08-27 20:34 ` Atish Patra 1 sibling, 1 reply; 27+ messages in thread From: Anup Patel @ 2019-08-27 8:28 UTC (permalink / raw) To: Mike Rapoport Cc: Atish Patra, Albert Ou, Alan Kao, Greg Kroah-Hartman, Palmer Dabbelt, linux-kernel@vger.kernel.org List, Alexios Zavras, Gary Guo, Paul Walmsley, linux-riscv, Thomas Gleixner On Tue, Aug 27, 2019 at 1:21 PM Mike Rapoport <rppt@linux.ibm.com> wrote: > > On Mon, Aug 26, 2019 at 04:32:55PM -0700, Atish Patra wrote: > > As per the new SBI specification, current SBI implementation is > > defined as legacy and will be removed/replaced in future. > > > > Rename existing implementation to reflect that. This patch is just > > a preparatory patch for SBI v0.2 and doesn't introduce any functional > > changes. > > > > Signed-off-by: Atish Patra <atish.patra@wdc.com> > > --- > > arch/riscv/include/asm/sbi.h | 61 +++++++++++++++++++----------------- > > 1 file changed, 33 insertions(+), 28 deletions(-) > > > > diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h > > index 21134b3ef404..7f5ecaaaa0d7 100644 > > --- a/arch/riscv/include/asm/sbi.h > > +++ b/arch/riscv/include/asm/sbi.h > > @@ -8,17 +8,18 @@ > > > > #include <linux/types.h> > > > > -#define SBI_SET_TIMER 0 > > -#define SBI_CONSOLE_PUTCHAR 1 > > -#define SBI_CONSOLE_GETCHAR 2 > > -#define SBI_CLEAR_IPI 3 > > -#define SBI_SEND_IPI 4 > > -#define SBI_REMOTE_FENCE_I 5 > > -#define SBI_REMOTE_SFENCE_VMA 6 > > -#define SBI_REMOTE_SFENCE_VMA_ASID 7 > > -#define SBI_SHUTDOWN 8 > > - > > -#define SBI_CALL(which, arg0, arg1, arg2, arg3) ({ \ > > + > > +#define SBI_EXT_LEGACY_SET_TIMER 0x0 > > +#define SBI_EXT_LEGACY_CONSOLE_PUTCHAR 0x1 > > +#define SBI_EXT_LEGACY_CONSOLE_GETCHAR 0x2 > > +#define SBI_EXT_LEGACY_CLEAR_IPI 0x3 > > +#define SBI_EXT_LEGACY_SEND_IPI 0x4 > > +#define SBI_EXT_LEGACY_REMOTE_FENCE_I 0x5 > > +#define SBI_EXT_LEGACY_REMOTE_SFENCE_VMA 0x6 > > +#define SBI_EXT_LEGACY_REMOTE_SFENCE_VMA_ASID 0x7 > > +#define SBI_EXT_LEGACY_SHUTDOWN 0x8 > > I can't say I'm closely following RISC-V development, but what will happen > when SBI v0.3 will come out and will render v0.2 legacy? > Won't we need another similar renaming then? Going forward with SBI v0.3 and higher, we won't see any calling convention changes. The SBI spec will be maintained and improved by RISC-V UNIX platform spec working group. My best guess is that, all future SBI releases (v0.3 or higher) will include more optional SBI extensions (hart hotplug, power management, etc). Regards, Anup > > > +#define SBI_CALL_LEGACY(which, 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); \ > > @@ -32,58 +33,61 @@ > > }) > > > > /* Lazy implementations until SBI is finalized */ > > -#define SBI_CALL_0(which) SBI_CALL(which, 0, 0, 0, 0) > > -#define SBI_CALL_1(which, arg0) SBI_CALL(which, arg0, 0, 0, 0) > > -#define SBI_CALL_2(which, arg0, arg1) SBI_CALL(which, arg0, arg1, 0, 0) > > -#define SBI_CALL_3(which, arg0, arg1, arg2) \ > > - SBI_CALL(which, arg0, arg1, arg2, 0) > > -#define SBI_CALL_4(which, arg0, arg1, arg2, arg3) \ > > - SBI_CALL(which, arg0, arg1, arg2, arg3) > > +#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) > > > > static inline void sbi_console_putchar(int ch) > > { > > - SBI_CALL_1(SBI_CONSOLE_PUTCHAR, ch); > > + SBI_CALL_LEGACY_1(SBI_EXT_LEGACY_CONSOLE_PUTCHAR, ch); > > } > > > > static inline int sbi_console_getchar(void) > > { > > - return SBI_CALL_0(SBI_CONSOLE_GETCHAR); > > + return SBI_CALL_LEGACY_0(SBI_EXT_LEGACY_CONSOLE_GETCHAR); > > } > > > > static inline void sbi_set_timer(uint64_t stime_value) > > { > > #if __riscv_xlen == 32 > > - SBI_CALL_2(SBI_SET_TIMER, stime_value, stime_value >> 32); > > + SBI_CALL_LEGACY_2(SBI_EXT_LEGACY_SET_TIMER, stime_value, > > + stime_value >> 32); > > #else > > - SBI_CALL_1(SBI_SET_TIMER, stime_value); > > + SBI_CALL_LEGACY_1(SBI_EXT_LEGACY_SET_TIMER, stime_value); > > #endif > > } > > > > static inline void sbi_shutdown(void) > > { > > - SBI_CALL_0(SBI_SHUTDOWN); > > + SBI_CALL_LEGACY_0(SBI_EXT_LEGACY_SHUTDOWN); > > } > > > > static inline void sbi_clear_ipi(void) > > { > > - SBI_CALL_0(SBI_CLEAR_IPI); > > + SBI_CALL_LEGACY_0(SBI_EXT_LEGACY_CLEAR_IPI); > > } > > > > static inline void sbi_send_ipi(const unsigned long *hart_mask) > > { > > - SBI_CALL_1(SBI_SEND_IPI, hart_mask); > > + SBI_CALL_LEGACY_1(SBI_EXT_LEGACY_SEND_IPI, hart_mask); > > } > > > > static inline void sbi_remote_fence_i(const unsigned long *hart_mask) > > { > > - SBI_CALL_1(SBI_REMOTE_FENCE_I, hart_mask); > > + SBI_CALL_LEGACY_1(SBI_EXT_LEGACY_REMOTE_FENCE_I, hart_mask); > > } > > > > static inline void sbi_remote_sfence_vma(const unsigned long *hart_mask, > > unsigned long start, > > unsigned long size) > > { > > - SBI_CALL_3(SBI_REMOTE_SFENCE_VMA, hart_mask, start, size); > > + SBI_CALL_LEGACY_3(SBI_EXT_LEGACY_REMOTE_SFENCE_VMA, hart_mask, > > + start, size); > > } > > > > static inline void sbi_remote_sfence_vma_asid(const unsigned long *hart_mask, > > @@ -91,7 +95,8 @@ static inline void sbi_remote_sfence_vma_asid(const unsigned long *hart_mask, > > unsigned long size, > > unsigned long asid) > > { > > - SBI_CALL_4(SBI_REMOTE_SFENCE_VMA_ASID, hart_mask, start, size, asid); > > + SBI_CALL_LEGACY_4(SBI_EXT_LEGACY_REMOTE_SFENCE_VMA_ASID, hart_mask, > > + start, size, asid); > > } > > > > #endif > > -- > > 2.21.0 > > > > > > _______________________________________________ > > linux-riscv mailing list > > linux-riscv@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-riscv > > > > -- > Sincerely yours, > Mike. > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 1/2] RISC-V: Mark existing SBI as legacy SBI. 2019-08-27 8:28 ` Anup Patel @ 2019-08-27 8:37 ` Mike Rapoport 2019-08-28 21:37 ` Palmer Dabbelt 0 siblings, 1 reply; 27+ messages in thread From: Mike Rapoport @ 2019-08-27 8:37 UTC (permalink / raw) To: Anup Patel Cc: Albert Ou, Alan Kao, Alexios Zavras, Greg Kroah-Hartman, Palmer Dabbelt, linux-kernel@vger.kernel.org List, Atish Patra, Gary Guo, Paul Walmsley, linux-riscv, Thomas Gleixner On Tue, Aug 27, 2019 at 01:58:03PM +0530, Anup Patel wrote: > On Tue, Aug 27, 2019 at 1:21 PM Mike Rapoport <rppt@linux.ibm.com> wrote: > > > > On Mon, Aug 26, 2019 at 04:32:55PM -0700, Atish Patra wrote: > > > As per the new SBI specification, current SBI implementation is > > > defined as legacy and will be removed/replaced in future. > > > > > > Rename existing implementation to reflect that. This patch is just > > > a preparatory patch for SBI v0.2 and doesn't introduce any functional > > > changes. > > > > > > Signed-off-by: Atish Patra <atish.patra@wdc.com> > > > --- > > > arch/riscv/include/asm/sbi.h | 61 +++++++++++++++++++----------------- > > > 1 file changed, 33 insertions(+), 28 deletions(-) > > > > > > diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h > > > index 21134b3ef404..7f5ecaaaa0d7 100644 > > > --- a/arch/riscv/include/asm/sbi.h > > > +++ b/arch/riscv/include/asm/sbi.h > > > @@ -8,17 +8,18 @@ > > > > > > #include <linux/types.h> > > > > > > -#define SBI_SET_TIMER 0 > > > -#define SBI_CONSOLE_PUTCHAR 1 > > > -#define SBI_CONSOLE_GETCHAR 2 > > > -#define SBI_CLEAR_IPI 3 > > > -#define SBI_SEND_IPI 4 > > > -#define SBI_REMOTE_FENCE_I 5 > > > -#define SBI_REMOTE_SFENCE_VMA 6 > > > -#define SBI_REMOTE_SFENCE_VMA_ASID 7 > > > -#define SBI_SHUTDOWN 8 > > > - > > > -#define SBI_CALL(which, arg0, arg1, arg2, arg3) ({ \ > > > + > > > +#define SBI_EXT_LEGACY_SET_TIMER 0x0 > > > +#define SBI_EXT_LEGACY_CONSOLE_PUTCHAR 0x1 > > > +#define SBI_EXT_LEGACY_CONSOLE_GETCHAR 0x2 > > > +#define SBI_EXT_LEGACY_CLEAR_IPI 0x3 > > > +#define SBI_EXT_LEGACY_SEND_IPI 0x4 > > > +#define SBI_EXT_LEGACY_REMOTE_FENCE_I 0x5 > > > +#define SBI_EXT_LEGACY_REMOTE_SFENCE_VMA 0x6 > > > +#define SBI_EXT_LEGACY_REMOTE_SFENCE_VMA_ASID 0x7 > > > +#define SBI_EXT_LEGACY_SHUTDOWN 0x8 > > > > I can't say I'm closely following RISC-V development, but what will happen > > when SBI v0.3 will come out and will render v0.2 legacy? > > Won't we need another similar renaming then? > > Going forward with SBI v0.3 and higher, we won't see any calling > convention changes. > > The SBI spec will be maintained and improved by RISC-V UNIX > platform spec working group. > > My best guess is that, all future SBI releases (v0.3 or higher) will > include more optional SBI extensions (hart hotplug, power management, etc). Thanks for the clarification! > Regards, > Anup > > > > > > +#define SBI_CALL_LEGACY(which, 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); \ > > > @@ -32,58 +33,61 @@ > > > }) > > > > > > /* Lazy implementations until SBI is finalized */ > > > -#define SBI_CALL_0(which) SBI_CALL(which, 0, 0, 0, 0) > > > -#define SBI_CALL_1(which, arg0) SBI_CALL(which, arg0, 0, 0, 0) > > > -#define SBI_CALL_2(which, arg0, arg1) SBI_CALL(which, arg0, arg1, 0, 0) > > > -#define SBI_CALL_3(which, arg0, arg1, arg2) \ > > > - SBI_CALL(which, arg0, arg1, arg2, 0) > > > -#define SBI_CALL_4(which, arg0, arg1, arg2, arg3) \ > > > - SBI_CALL(which, arg0, arg1, arg2, arg3) > > > +#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) > > > > > > static inline void sbi_console_putchar(int ch) > > > { > > > - SBI_CALL_1(SBI_CONSOLE_PUTCHAR, ch); > > > + SBI_CALL_LEGACY_1(SBI_EXT_LEGACY_CONSOLE_PUTCHAR, ch); > > > } > > > > > > static inline int sbi_console_getchar(void) > > > { > > > - return SBI_CALL_0(SBI_CONSOLE_GETCHAR); > > > + return SBI_CALL_LEGACY_0(SBI_EXT_LEGACY_CONSOLE_GETCHAR); > > > } > > > > > > static inline void sbi_set_timer(uint64_t stime_value) > > > { > > > #if __riscv_xlen == 32 > > > - SBI_CALL_2(SBI_SET_TIMER, stime_value, stime_value >> 32); > > > + SBI_CALL_LEGACY_2(SBI_EXT_LEGACY_SET_TIMER, stime_value, > > > + stime_value >> 32); > > > #else > > > - SBI_CALL_1(SBI_SET_TIMER, stime_value); > > > + SBI_CALL_LEGACY_1(SBI_EXT_LEGACY_SET_TIMER, stime_value); > > > #endif > > > } > > > > > > static inline void sbi_shutdown(void) > > > { > > > - SBI_CALL_0(SBI_SHUTDOWN); > > > + SBI_CALL_LEGACY_0(SBI_EXT_LEGACY_SHUTDOWN); > > > } > > > > > > static inline void sbi_clear_ipi(void) > > > { > > > - SBI_CALL_0(SBI_CLEAR_IPI); > > > + SBI_CALL_LEGACY_0(SBI_EXT_LEGACY_CLEAR_IPI); > > > } > > > > > > static inline void sbi_send_ipi(const unsigned long *hart_mask) > > > { > > > - SBI_CALL_1(SBI_SEND_IPI, hart_mask); > > > + SBI_CALL_LEGACY_1(SBI_EXT_LEGACY_SEND_IPI, hart_mask); > > > } > > > > > > static inline void sbi_remote_fence_i(const unsigned long *hart_mask) > > > { > > > - SBI_CALL_1(SBI_REMOTE_FENCE_I, hart_mask); > > > + SBI_CALL_LEGACY_1(SBI_EXT_LEGACY_REMOTE_FENCE_I, hart_mask); > > > } > > > > > > static inline void sbi_remote_sfence_vma(const unsigned long *hart_mask, > > > unsigned long start, > > > unsigned long size) > > > { > > > - SBI_CALL_3(SBI_REMOTE_SFENCE_VMA, hart_mask, start, size); > > > + SBI_CALL_LEGACY_3(SBI_EXT_LEGACY_REMOTE_SFENCE_VMA, hart_mask, > > > + start, size); > > > } > > > > > > static inline void sbi_remote_sfence_vma_asid(const unsigned long *hart_mask, > > > @@ -91,7 +95,8 @@ static inline void sbi_remote_sfence_vma_asid(const unsigned long *hart_mask, > > > unsigned long size, > > > unsigned long asid) > > > { > > > - SBI_CALL_4(SBI_REMOTE_SFENCE_VMA_ASID, hart_mask, start, size, asid); > > > + SBI_CALL_LEGACY_4(SBI_EXT_LEGACY_REMOTE_SFENCE_VMA_ASID, hart_mask, > > > + start, size, asid); > > > } > > > > > > #endif > > > -- > > > 2.21.0 > > > > > > > > > _______________________________________________ > > > linux-riscv mailing list > > > linux-riscv@lists.infradead.org > > > http://lists.infradead.org/mailman/listinfo/linux-riscv > > > > > > > -- > > Sincerely yours, > > Mike. > > > > > > _______________________________________________ > > linux-riscv mailing list > > linux-riscv@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-riscv > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv -- Sincerely yours, Mike. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 1/2] RISC-V: Mark existing SBI as legacy SBI. 2019-08-27 8:37 ` Mike Rapoport @ 2019-08-28 21:37 ` Palmer Dabbelt 0 siblings, 0 replies; 27+ messages in thread From: Palmer Dabbelt @ 2019-08-28 21:37 UTC (permalink / raw) To: rppt Cc: anup, aou, alankao, alexios.zavras, Greg KH, linux-kernel, Atish Patra, gary, Paul Walmsley, linux-riscv, tglx On Tue, 27 Aug 2019 01:37:00 PDT (-0700), rppt@linux.ibm.com wrote: > On Tue, Aug 27, 2019 at 01:58:03PM +0530, Anup Patel wrote: >> On Tue, Aug 27, 2019 at 1:21 PM Mike Rapoport <rppt@linux.ibm.com> wrote: >> > >> > On Mon, Aug 26, 2019 at 04:32:55PM -0700, Atish Patra wrote: >> > > As per the new SBI specification, current SBI implementation is >> > > defined as legacy and will be removed/replaced in future. >> > > >> > > Rename existing implementation to reflect that. This patch is just >> > > a preparatory patch for SBI v0.2 and doesn't introduce any functional >> > > changes. >> > > >> > > Signed-off-by: Atish Patra <atish.patra@wdc.com> >> > > --- >> > > arch/riscv/include/asm/sbi.h | 61 +++++++++++++++++++----------------- >> > > 1 file changed, 33 insertions(+), 28 deletions(-) >> > > >> > > diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h >> > > index 21134b3ef404..7f5ecaaaa0d7 100644 >> > > --- a/arch/riscv/include/asm/sbi.h >> > > +++ b/arch/riscv/include/asm/sbi.h >> > > @@ -8,17 +8,18 @@ >> > > >> > > #include <linux/types.h> >> > > >> > > -#define SBI_SET_TIMER 0 >> > > -#define SBI_CONSOLE_PUTCHAR 1 >> > > -#define SBI_CONSOLE_GETCHAR 2 >> > > -#define SBI_CLEAR_IPI 3 >> > > -#define SBI_SEND_IPI 4 >> > > -#define SBI_REMOTE_FENCE_I 5 >> > > -#define SBI_REMOTE_SFENCE_VMA 6 >> > > -#define SBI_REMOTE_SFENCE_VMA_ASID 7 >> > > -#define SBI_SHUTDOWN 8 >> > > - >> > > -#define SBI_CALL(which, arg0, arg1, arg2, arg3) ({ \ >> > > + >> > > +#define SBI_EXT_LEGACY_SET_TIMER 0x0 >> > > +#define SBI_EXT_LEGACY_CONSOLE_PUTCHAR 0x1 >> > > +#define SBI_EXT_LEGACY_CONSOLE_GETCHAR 0x2 >> > > +#define SBI_EXT_LEGACY_CLEAR_IPI 0x3 >> > > +#define SBI_EXT_LEGACY_SEND_IPI 0x4 >> > > +#define SBI_EXT_LEGACY_REMOTE_FENCE_I 0x5 >> > > +#define SBI_EXT_LEGACY_REMOTE_SFENCE_VMA 0x6 >> > > +#define SBI_EXT_LEGACY_REMOTE_SFENCE_VMA_ASID 0x7 >> > > +#define SBI_EXT_LEGACY_SHUTDOWN 0x8 >> > >> > I can't say I'm closely following RISC-V development, but what will happen >> > when SBI v0.3 will come out and will render v0.2 legacy? >> > Won't we need another similar renaming then? >> >> Going forward with SBI v0.3 and higher, we won't see any calling >> convention changes. >> >> The SBI spec will be maintained and improved by RISC-V UNIX >> platform spec working group. >> >> My best guess is that, all future SBI releases (v0.3 or higher) will >> include more optional SBI extensions (hart hotplug, power management, etc). > > Thanks for the clarification! Specifically the issue is that 0.1 didn't include any mechanism for probing the existence of a function, but since that has been added in 0.2 it's possible to maintain compatible with future versions. > >> Regards, >> Anup >> >> > >> > > +#define SBI_CALL_LEGACY(which, 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); \ >> > > @@ -32,58 +33,61 @@ >> > > }) >> > > >> > > /* Lazy implementations until SBI is finalized */ >> > > -#define SBI_CALL_0(which) SBI_CALL(which, 0, 0, 0, 0) >> > > -#define SBI_CALL_1(which, arg0) SBI_CALL(which, arg0, 0, 0, 0) >> > > -#define SBI_CALL_2(which, arg0, arg1) SBI_CALL(which, arg0, arg1, 0, 0) >> > > -#define SBI_CALL_3(which, arg0, arg1, arg2) \ >> > > - SBI_CALL(which, arg0, arg1, arg2, 0) >> > > -#define SBI_CALL_4(which, arg0, arg1, arg2, arg3) \ >> > > - SBI_CALL(which, arg0, arg1, arg2, arg3) >> > > +#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) >> > > >> > > static inline void sbi_console_putchar(int ch) >> > > { >> > > - SBI_CALL_1(SBI_CONSOLE_PUTCHAR, ch); >> > > + SBI_CALL_LEGACY_1(SBI_EXT_LEGACY_CONSOLE_PUTCHAR, ch); >> > > } >> > > >> > > static inline int sbi_console_getchar(void) >> > > { >> > > - return SBI_CALL_0(SBI_CONSOLE_GETCHAR); >> > > + return SBI_CALL_LEGACY_0(SBI_EXT_LEGACY_CONSOLE_GETCHAR); >> > > } >> > > >> > > static inline void sbi_set_timer(uint64_t stime_value) >> > > { >> > > #if __riscv_xlen == 32 >> > > - SBI_CALL_2(SBI_SET_TIMER, stime_value, stime_value >> 32); >> > > + SBI_CALL_LEGACY_2(SBI_EXT_LEGACY_SET_TIMER, stime_value, >> > > + stime_value >> 32); >> > > #else >> > > - SBI_CALL_1(SBI_SET_TIMER, stime_value); >> > > + SBI_CALL_LEGACY_1(SBI_EXT_LEGACY_SET_TIMER, stime_value); >> > > #endif >> > > } >> > > >> > > static inline void sbi_shutdown(void) >> > > { >> > > - SBI_CALL_0(SBI_SHUTDOWN); >> > > + SBI_CALL_LEGACY_0(SBI_EXT_LEGACY_SHUTDOWN); >> > > } >> > > >> > > static inline void sbi_clear_ipi(void) >> > > { >> > > - SBI_CALL_0(SBI_CLEAR_IPI); >> > > + SBI_CALL_LEGACY_0(SBI_EXT_LEGACY_CLEAR_IPI); >> > > } >> > > >> > > static inline void sbi_send_ipi(const unsigned long *hart_mask) >> > > { >> > > - SBI_CALL_1(SBI_SEND_IPI, hart_mask); >> > > + SBI_CALL_LEGACY_1(SBI_EXT_LEGACY_SEND_IPI, hart_mask); >> > > } >> > > >> > > static inline void sbi_remote_fence_i(const unsigned long *hart_mask) >> > > { >> > > - SBI_CALL_1(SBI_REMOTE_FENCE_I, hart_mask); >> > > + SBI_CALL_LEGACY_1(SBI_EXT_LEGACY_REMOTE_FENCE_I, hart_mask); >> > > } >> > > >> > > static inline void sbi_remote_sfence_vma(const unsigned long *hart_mask, >> > > unsigned long start, >> > > unsigned long size) >> > > { >> > > - SBI_CALL_3(SBI_REMOTE_SFENCE_VMA, hart_mask, start, size); >> > > + SBI_CALL_LEGACY_3(SBI_EXT_LEGACY_REMOTE_SFENCE_VMA, hart_mask, >> > > + start, size); >> > > } >> > > >> > > static inline void sbi_remote_sfence_vma_asid(const unsigned long *hart_mask, >> > > @@ -91,7 +95,8 @@ static inline void sbi_remote_sfence_vma_asid(const unsigned long *hart_mask, >> > > unsigned long size, >> > > unsigned long asid) >> > > { >> > > - SBI_CALL_4(SBI_REMOTE_SFENCE_VMA_ASID, hart_mask, start, size, asid); >> > > + SBI_CALL_LEGACY_4(SBI_EXT_LEGACY_REMOTE_SFENCE_VMA_ASID, hart_mask, >> > > + start, size, asid); >> > > } >> > > >> > > #endif >> > > -- >> > > 2.21.0 >> > > >> > > >> > > _______________________________________________ >> > > linux-riscv mailing list >> > > linux-riscv@lists.infradead.org >> > > http://lists.infradead.org/mailman/listinfo/linux-riscv >> > > >> > >> > -- >> > Sincerely yours, >> > Mike. >> > >> > >> > _______________________________________________ >> > linux-riscv mailing list >> > linux-riscv@lists.infradead.org >> > http://lists.infradead.org/mailman/listinfo/linux-riscv >> >> _______________________________________________ >> linux-riscv mailing list >> linux-riscv@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 1/2] RISC-V: Mark existing SBI as legacy SBI. 2019-08-27 7:51 ` Mike Rapoport 2019-08-27 8:28 ` Anup Patel @ 2019-08-27 20:34 ` Atish Patra 1 sibling, 0 replies; 27+ messages in thread From: Atish Patra @ 2019-08-27 20:34 UTC (permalink / raw) To: rppt Cc: linux-riscv, alankao, gregkh, gary, paul.walmsley, aou, linux-kernel, tglx, anup, alexios.zavras, palmer On Tue, 2019-08-27 at 10:51 +0300, Mike Rapoport wrote: > On Mon, Aug 26, 2019 at 04:32:55PM -0700, Atish Patra wrote: > > As per the new SBI specification, current SBI implementation is > > defined as legacy and will be removed/replaced in future. > > > > Rename existing implementation to reflect that. This patch is just > > a preparatory patch for SBI v0.2 and doesn't introduce any > > functional > > changes. > > > > Signed-off-by: Atish Patra <atish.patra@wdc.com> > > --- > > arch/riscv/include/asm/sbi.h | 61 +++++++++++++++++++------------- > > ---- > > 1 file changed, 33 insertions(+), 28 deletions(-) > > > > diff --git a/arch/riscv/include/asm/sbi.h > > b/arch/riscv/include/asm/sbi.h > > index 21134b3ef404..7f5ecaaaa0d7 100644 > > --- a/arch/riscv/include/asm/sbi.h > > +++ b/arch/riscv/include/asm/sbi.h > > @@ -8,17 +8,18 @@ > > > > #include <linux/types.h> > > > > -#define SBI_SET_TIMER 0 > > -#define SBI_CONSOLE_PUTCHAR 1 > > -#define SBI_CONSOLE_GETCHAR 2 > > -#define SBI_CLEAR_IPI 3 > > -#define SBI_SEND_IPI 4 > > -#define SBI_REMOTE_FENCE_I 5 > > -#define SBI_REMOTE_SFENCE_VMA 6 > > -#define SBI_REMOTE_SFENCE_VMA_ASID 7 > > -#define SBI_SHUTDOWN 8 > > - > > -#define SBI_CALL(which, arg0, arg1, arg2, arg3) ({ \ > > + > > +#define SBI_EXT_LEGACY_SET_TIMER 0x0 > > +#define SBI_EXT_LEGACY_CONSOLE_PUTCHAR 0x1 > > +#define SBI_EXT_LEGACY_CONSOLE_GETCHAR 0x2 > > +#define SBI_EXT_LEGACY_CLEAR_IPI 0x3 > > +#define SBI_EXT_LEGACY_SEND_IPI 0x4 > > +#define SBI_EXT_LEGACY_REMOTE_FENCE_I 0x5 > > +#define SBI_EXT_LEGACY_REMOTE_SFENCE_VMA 0x6 > > +#define SBI_EXT_LEGACY_REMOTE_SFENCE_VMA_ASID 0x7 > > +#define SBI_EXT_LEGACY_SHUTDOWN 0x8 > > I can't say I'm closely following RISC-V development, but what will > happen > when SBI v0.3 will come out and will render v0.2 legacy? > Won't we need another similar renaming then? > Noope.The idea of renaming 0.1 calls as legacy comes from the fact that these calls will be eventually be replaced or depcreated after a certain time. v0.2 defines base specification which will be mandatory for any newer implementation. Any versions newer than 0.2 will only add new extensions but not necessarily replace the ones in 0.2 > > +#define SBI_CALL_LEGACY(which, 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); \ > > @@ -32,58 +33,61 @@ > > }) > > > > /* Lazy implementations until SBI is finalized */ > > -#define SBI_CALL_0(which) SBI_CALL(which, 0, 0, 0, 0) > > -#define SBI_CALL_1(which, arg0) SBI_CALL(which, arg0, 0, 0, 0) > > -#define SBI_CALL_2(which, arg0, arg1) SBI_CALL(which, arg0, arg1, > > 0, 0) > > -#define SBI_CALL_3(which, arg0, arg1, arg2) \ > > - SBI_CALL(which, arg0, arg1, arg2, 0) > > -#define SBI_CALL_4(which, arg0, arg1, arg2, arg3) \ > > - SBI_CALL(which, arg0, arg1, arg2, arg3) > > +#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) > > > > static inline void sbi_console_putchar(int ch) > > { > > - SBI_CALL_1(SBI_CONSOLE_PUTCHAR, ch); > > + SBI_CALL_LEGACY_1(SBI_EXT_LEGACY_CONSOLE_PUTCHAR, ch); > > } > > > > static inline int sbi_console_getchar(void) > > { > > - return SBI_CALL_0(SBI_CONSOLE_GETCHAR); > > + return SBI_CALL_LEGACY_0(SBI_EXT_LEGACY_CONSOLE_GETCHAR); > > } > > > > static inline void sbi_set_timer(uint64_t stime_value) > > { > > #if __riscv_xlen == 32 > > - SBI_CALL_2(SBI_SET_TIMER, stime_value, stime_value >> 32); > > + SBI_CALL_LEGACY_2(SBI_EXT_LEGACY_SET_TIMER, stime_value, > > + stime_value >> 32); > > #else > > - SBI_CALL_1(SBI_SET_TIMER, stime_value); > > + SBI_CALL_LEGACY_1(SBI_EXT_LEGACY_SET_TIMER, stime_value); > > #endif > > } > > > > static inline void sbi_shutdown(void) > > { > > - SBI_CALL_0(SBI_SHUTDOWN); > > + SBI_CALL_LEGACY_0(SBI_EXT_LEGACY_SHUTDOWN); > > } > > > > static inline void sbi_clear_ipi(void) > > { > > - SBI_CALL_0(SBI_CLEAR_IPI); > > + SBI_CALL_LEGACY_0(SBI_EXT_LEGACY_CLEAR_IPI); > > } > > > > static inline void sbi_send_ipi(const unsigned long *hart_mask) > > { > > - SBI_CALL_1(SBI_SEND_IPI, hart_mask); > > + SBI_CALL_LEGACY_1(SBI_EXT_LEGACY_SEND_IPI, hart_mask); > > } > > > > static inline void sbi_remote_fence_i(const unsigned long > > *hart_mask) > > { > > - SBI_CALL_1(SBI_REMOTE_FENCE_I, hart_mask); > > + SBI_CALL_LEGACY_1(SBI_EXT_LEGACY_REMOTE_FENCE_I, hart_mask); > > } > > > > static inline void sbi_remote_sfence_vma(const unsigned long > > *hart_mask, > > unsigned long start, > > unsigned long size) > > { > > - SBI_CALL_3(SBI_REMOTE_SFENCE_VMA, hart_mask, start, size); > > + SBI_CALL_LEGACY_3(SBI_EXT_LEGACY_REMOTE_SFENCE_VMA, hart_mask, > > + start, size); > > } > > > > static inline void sbi_remote_sfsimilarence_vma_asid(const > > unsigned long *hart_mask, > > @@ -91,7 +95,8 @@ static inline void > > sbi_remote_sfence_vma_asid(const unsigned long *hart_mask, > > unsigned long size, > > unsigned long asid) > > { > > - SBI_CALL_4(SBI_REMOTE_SFENCE_VMA_ASID, hart_mask, start, size, > > asid); > > + SBI_CALL_LEGACY_4(SBI_EXT_LEGACY_REMOTE_SFENCE_VMA_ASID, > > hart_mask, > > + start, size, asid); > > } > > > > #endif > > -- > > 2.21.0 > > > > > > _______________________________________________ > > linux-riscv mailing list > > linux-riscv@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-riscv > > -- Regards, Atish ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 1/2] RISC-V: Mark existing SBI as legacy SBI. 2019-08-26 23:32 ` [RFC PATCH 1/2] RISC-V: Mark existing SBI as legacy SBI Atish Patra 2019-08-27 7:51 ` Mike Rapoport @ 2019-08-27 14:03 ` Christoph Hellwig 2019-08-27 14:04 ` Christoph Hellwig 2019-08-27 20:37 ` Atish Patra 1 sibling, 2 replies; 27+ messages in thread From: Christoph Hellwig @ 2019-08-27 14:03 UTC (permalink / raw) To: Atish Patra Cc: linux-kernel, Albert Ou, Alan Kao, Alexios Zavras, Anup Patel, Palmer Dabbelt, Mike Rapoport, Paul Walmsley, Gary Guo, Greg Kroah-Hartman, linux-riscv, Thomas Gleixner > +#define SBI_EXT_LEGACY_SET_TIMER 0x0 > +#define SBI_EXT_LEGACY_CONSOLE_PUTCHAR 0x1 > +#define SBI_EXT_LEGACY_CONSOLE_GETCHAR 0x2 > +#define SBI_EXT_LEGACY_CLEAR_IPI 0x3 > +#define SBI_EXT_LEGACY_SEND_IPI 0x4 > +#define SBI_EXT_LEGACY_REMOTE_FENCE_I 0x5 > +#define SBI_EXT_LEGACY_REMOTE_SFENCE_VMA 0x6 > +#define SBI_EXT_LEGACY_REMOTE_SFENCE_VMA_ASID 0x7 > +#define SBI_EXT_LEGACY_SHUTDOWN 0x8 As Mike said legacy is a bit of a weird name. I think this should be SBI01_* or so. And pleae align the numeric values and maybe use an enum. > + > +#define SBI_CALL_LEGACY(which, 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); \ Instead of the weird inline assembly with forced register allocation, why not move this to pure assembly? AFAICs this is the whole assembly code we'd need: ENTRY(sbi01_call) ecall END(sbi01_call) > /* Lazy implementations until SBI is finalized */ > -#define SBI_CALL_0(which) SBI_CALL(which, 0, 0, 0, 0) > -#define SBI_CALL_1(which, arg0) SBI_CALL(which, arg0, 0, 0, 0) > -#define SBI_CALL_2(which, arg0, arg1) SBI_CALL(which, arg0, arg1, 0, 0) > -#define SBI_CALL_3(which, arg0, arg1, arg2) \ > - SBI_CALL(which, arg0, arg1, arg2, 0) > -#define SBI_CALL_4(which, arg0, arg1, arg2, arg3) \ > - SBI_CALL(which, arg0, arg1, arg2, arg3) > +#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) When you touch this anyway I'd suggest you kill these rather pointless wrappers as well as the comment above them. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 1/2] RISC-V: Mark existing SBI as legacy SBI. 2019-08-27 14:03 ` Christoph Hellwig @ 2019-08-27 14:04 ` Christoph Hellwig 2019-08-27 20:37 ` Atish Patra 1 sibling, 0 replies; 27+ messages in thread From: Christoph Hellwig @ 2019-08-27 14:04 UTC (permalink / raw) To: Atish Patra Cc: Albert Ou, Alan Kao, Greg Kroah-Hartman, Anup Patel, Palmer Dabbelt, linux-kernel, Mike Rapoport, Alexios Zavras, Gary Guo, Paul Walmsley, linux-riscv, Thomas Gleixner On Tue, Aug 27, 2019 at 07:03:04AM -0700, Christoph Hellwig wrote: > > +#define SBI_EXT_LEGACY_SET_TIMER 0x0 > > +#define SBI_EXT_LEGACY_CONSOLE_PUTCHAR 0x1 > > +#define SBI_EXT_LEGACY_CONSOLE_GETCHAR 0x2 > > +#define SBI_EXT_LEGACY_CLEAR_IPI 0x3 > > +#define SBI_EXT_LEGACY_SEND_IPI 0x4 > > +#define SBI_EXT_LEGACY_REMOTE_FENCE_I 0x5 > > +#define SBI_EXT_LEGACY_REMOTE_SFENCE_VMA 0x6 > > +#define SBI_EXT_LEGACY_REMOTE_SFENCE_VMA_ASID 0x7 > > +#define SBI_EXT_LEGACY_SHUTDOWN 0x8 > > As Mike said legacy is a bit of a weird name. I think this should > be SBI01_* or so. And pleae align the numeric values and maybe use > an enum. > > > + > > +#define SBI_CALL_LEGACY(which, 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); \ > > Instead of the weird inline assembly with forced register allocation, > why not move this to pure assembly? AFAICs this is the whole assembly > code we'd need: > > ENTRY(sbi01_call) > ecall > END(sbi01_call) Actually we'll need a mv to a7 for the function id, but the point still stands. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 1/2] RISC-V: Mark existing SBI as legacy SBI. 2019-08-27 14:03 ` Christoph Hellwig 2019-08-27 14:04 ` Christoph Hellwig @ 2019-08-27 20:37 ` Atish Patra 2019-08-29 10:56 ` hch 1 sibling, 1 reply; 27+ messages in thread From: Atish Patra @ 2019-08-27 20:37 UTC (permalink / raw) To: hch Cc: linux-riscv, alankao, gregkh, gary, rppt, paul.walmsley, aou, linux-kernel, tglx, anup, alexios.zavras, palmer On Tue, 2019-08-27 at 07:03 -0700, Christoph Hellwig wrote: > > +#define SBI_EXT_LEGACY_SET_TIMER 0x0 > > +#define SBI_EXT_LEGACY_CONSOLE_PUTCHAR 0x1 > > +#define SBI_EXT_LEGACY_CONSOLE_GETCHAR 0x2 > > +#define SBI_EXT_LEGACY_CLEAR_IPI 0x3 > > +#define SBI_EXT_LEGACY_SEND_IPI 0x4 > > +#define SBI_EXT_LEGACY_REMOTE_FENCE_I 0x5 > > +#define SBI_EXT_LEGACY_REMOTE_SFENCE_VMA 0x6 > > +#define SBI_EXT_LEGACY_REMOTE_SFENCE_VMA_ASID 0x7 > > +#define SBI_EXT_LEGACY_SHUTDOWN 0x8 > > As Mike said legacy is a bit of a weird name. I think this should > be SBI01_* or so. And pleae align the numeric values and maybe use > an enum. > Will do. > > + > > +#define SBI_CALL_LEGACY(which, 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); \ > > Instead of the weird inline assembly with forced register allocation, > why not move this to pure assembly? AFAICs this is the whole > assembly > code we'd need: > > ENTRY(sbi01_call) > ecall > END(sbi01_call) > That would split the implementation between C file & assembly file for no good reason. How about moving everything in sbi.c and just write everything inline assembly there. > > /* Lazy implementations until SBI is finalized */ > > -#define SBI_CALL_0(which) SBI_CALL(which, 0, 0, 0, 0) > > -#define SBI_CALL_1(which, arg0) SBI_CALL(which, arg0, 0, 0, 0) > > -#define SBI_CALL_2(which, arg0, arg1) SBI_CALL(which, arg0, arg1, > > 0, 0) > > -#define SBI_CALL_3(which, arg0, arg1, arg2) \ > > - SBI_CALL(which, arg0, arg1, arg2, 0) > > -#define SBI_CALL_4(which, arg0, arg1, arg2, arg3) \ > > - SBI_CALL(which, arg0, arg1, arg2, arg3) > > +#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) > > When you touch this anyway I'd suggest you kill these rather > pointless wrappers as well as the comment above them. Sure. I did not want make bigger chagnes in 1st RFC patches. But looking at the comments, I feel it was not a good decission. I will get rid of these wrappers and move necessary ones to sbi.c -- Regards, Atish ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 1/2] RISC-V: Mark existing SBI as legacy SBI. 2019-08-27 20:37 ` Atish Patra @ 2019-08-29 10:56 ` hch 0 siblings, 0 replies; 27+ messages in thread From: hch @ 2019-08-29 10:56 UTC (permalink / raw) To: Atish Patra Cc: hch, aou, alankao, gregkh, anup, palmer, linux-kernel, rppt, alexios.zavras, gary, paul.walmsley, linux-riscv, tglx On Tue, Aug 27, 2019 at 08:37:27PM +0000, Atish Patra wrote: > That would split the implementation between C file & assembly file for > no good reason. > > How about moving everything in sbi.c and just write everything inline > assembly there. Well, if we implement it in pure assembly that would be the entire implementation, wouldn't it? ^ permalink raw reply [flat|nested] 27+ messages in thread
* [RFC PATCH 2/2] RISC-V: Add basic support for SBI v0.2 2019-08-26 23:32 [RFC PATCH 0/2] Add support for SBI version to 0.2 Atish Patra 2019-08-26 23:32 ` [RFC PATCH 1/2] RISC-V: Mark existing SBI as legacy SBI Atish Patra @ 2019-08-26 23:32 ` Atish Patra 2019-08-27 7:58 ` Mike Rapoport ` (2 more replies) 2019-08-27 14:46 ` [RFC PATCH 0/2] Add support for SBI version to 0.2 Christoph Hellwig 2 siblings, 3 replies; 27+ messages in thread From: Atish Patra @ 2019-08-26 23:32 UTC (permalink / raw) To: linux-kernel Cc: Atish Patra, Alan Kao, Albert Ou, Alexios Zavras, Anup Patel, Gary Guo, Greg Kroah-Hartman, linux-riscv, Mike Rapoport, Palmer Dabbelt, Paul Walmsley, Thomas Gleixner 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; \ }) /* 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 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 2/2] RISC-V: Add basic support for SBI v0.2 2019-08-26 23:32 ` [RFC PATCH 2/2] RISC-V: Add basic support for SBI v0.2 Atish Patra @ 2019-08-27 7:58 ` Mike Rapoport 2019-08-27 8:23 ` Anup Patel 2019-08-27 9:36 ` Anup Patel 2019-08-27 14:11 ` Christoph Hellwig 2 siblings, 1 reply; 27+ messages in thread From: Mike Rapoport @ 2019-08-27 7:58 UTC (permalink / raw) To: Atish Patra Cc: linux-kernel, Albert Ou, Alan Kao, Alexios Zavras, Anup Patel, Palmer Dabbelt, Paul Walmsley, Gary Guo, Greg Kroah-Hartman, linux-riscv, Thomas Gleixner 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 -- Sincerely yours, Mike. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 2/2] RISC-V: Add basic support for SBI v0.2 2019-08-27 7:58 ` Mike Rapoport @ 2019-08-27 8:23 ` Anup Patel 2019-08-27 8:39 ` Mike Rapoport 0 siblings, 1 reply; 27+ messages in thread From: Anup Patel @ 2019-08-27 8:23 UTC (permalink / raw) To: Mike Rapoport Cc: Atish Patra, linux-kernel@vger.kernel.org List, Albert Ou, Alan Kao, Alexios Zavras, Palmer Dabbelt, Paul Walmsley, Gary Guo, Greg Kroah-Hartman, linux-riscv, Thomas Gleixner 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. > ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 2/2] RISC-V: Add basic support for SBI v0.2 2019-08-27 8:23 ` Anup Patel @ 2019-08-27 8:39 ` Mike Rapoport 2019-08-27 9:28 ` Anup Patel 2019-08-27 20:30 ` Atish Patra 0 siblings, 2 replies; 27+ messages in thread From: Mike Rapoport @ 2019-08-27 8:39 UTC (permalink / raw) To: Anup Patel Cc: Atish Patra, linux-kernel@vger.kernel.org List, Albert Ou, Alan Kao, Alexios Zavras, Palmer Dabbelt, Paul Walmsley, Gary Guo, Greg Kroah-Hartman, linux-riscv, Thomas Gleixner 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. > > -- Sincerely yours, Mike. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 2/2] RISC-V: Add basic support for SBI v0.2 2019-08-27 8:39 ` Mike Rapoport @ 2019-08-27 9:28 ` Anup Patel 2019-08-27 20:30 ` Atish Patra 1 sibling, 0 replies; 27+ messages in thread From: Anup Patel @ 2019-08-27 9:28 UTC (permalink / raw) To: Mike Rapoport Cc: Atish Patra, linux-kernel@vger.kernel.org List, Albert Ou, Alan Kao, Alexios Zavras, Palmer Dabbelt, Paul Walmsley, Gary Guo, Greg Kroah-Hartman, linux-riscv, Thomas Gleixner 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. > ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 2/2] RISC-V: Add basic support for SBI v0.2 2019-08-27 8:39 ` Mike Rapoport 2019-08-27 9:28 ` Anup Patel @ 2019-08-27 20:30 ` Atish Patra 1 sibling, 0 replies; 27+ messages in thread From: Atish Patra @ 2019-08-27 20:30 UTC (permalink / raw) To: anup, rppt Cc: linux-riscv, alankao, gregkh, gary, paul.walmsley, aou, linux-kernel, tglx, alexios.zavras, palmer 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. > > > -- Regards, Atish ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 2/2] RISC-V: Add basic support for SBI v0.2 2019-08-26 23:32 ` [RFC PATCH 2/2] RISC-V: Add basic support for SBI v0.2 Atish Patra 2019-08-27 7:58 ` Mike Rapoport @ 2019-08-27 9:36 ` Anup Patel 2019-08-27 20:43 ` Atish Patra 2019-08-27 14:11 ` Christoph Hellwig 2 siblings, 1 reply; 27+ messages in thread From: Anup Patel @ 2019-08-27 9:36 UTC (permalink / raw) To: Atish Patra Cc: linux-kernel@vger.kernel.org List, Alan Kao, Albert Ou, Alexios Zavras, Gary Guo, Greg Kroah-Hartman, linux-riscv, Mike Rapoport, Palmer Dabbelt, Paul Walmsley, Thomas Gleixner 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 > ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 2/2] RISC-V: Add basic support for SBI v0.2 2019-08-27 9:36 ` Anup Patel @ 2019-08-27 20:43 ` Atish Patra 0 siblings, 0 replies; 27+ messages in thread From: Atish Patra @ 2019-08-27 20:43 UTC (permalink / raw) To: anup Cc: linux-riscv, alankao, gregkh, gary, rppt, paul.walmsley, linux-kernel, aou, tglx, alexios.zavras, palmer 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 > > -- Regards, Atish ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 2/2] RISC-V: Add basic support for SBI v0.2 2019-08-26 23:32 ` [RFC PATCH 2/2] RISC-V: Add basic support for SBI v0.2 Atish Patra 2019-08-27 7:58 ` Mike Rapoport 2019-08-27 9:36 ` Anup Patel @ 2019-08-27 14:11 ` Christoph Hellwig 2 siblings, 0 replies; 27+ messages in thread From: Christoph Hellwig @ 2019-08-27 14:11 UTC (permalink / raw) To: Atish Patra Cc: linux-kernel, Albert Ou, Alan Kao, Alexios Zavras, Anup Patel, Palmer Dabbelt, Mike Rapoport, Paul Walmsley, Gary Guo, Greg Kroah-Hartman, linux-riscv, Thomas Gleixner > +#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. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 0/2] Add support for SBI version to 0.2 2019-08-26 23:32 [RFC PATCH 0/2] Add support for SBI version to 0.2 Atish Patra 2019-08-26 23:32 ` [RFC PATCH 1/2] RISC-V: Mark existing SBI as legacy SBI Atish Patra 2019-08-26 23:32 ` [RFC PATCH 2/2] RISC-V: Add basic support for SBI v0.2 Atish Patra @ 2019-08-27 14:46 ` Christoph Hellwig 2019-08-27 22:19 ` Atish Patra 2 siblings, 1 reply; 27+ messages in thread From: Christoph Hellwig @ 2019-08-27 14:46 UTC (permalink / raw) To: Atish Patra Cc: linux-kernel, Albert Ou, Alan Kao, Alexios Zavras, Anup Patel, Palmer Dabbelt, Mike Rapoport, Paul Walmsley, Gary Guo, Greg Kroah-Hartman, linux-riscv, Thomas Gleixner On Mon, Aug 26, 2019 at 04:32:54PM -0700, Atish Patra wrote: > This patch series aims to add support for SBI specification version > v0.2. It doesn't break compatibility with any v0.1 implementation. > Internally, all the v0.1 calls are just renamed to legacy to be in > sync with specification [1]. > > The patches for v0.2 support in OpenSBI are available at > http://lists.infradead.org/pipermail/opensbi/2019-August/000422.html > > [1] https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.adoc I really don't like the current design of that SBI 0.2 spec, and don't think implementing it as-is is helpful. For one the way how the extension id is placed creates a compatibilty problem, not allowing your to implement a backwards compatible sbi, which seems bad. Second just blindly moving all the existing calls to a single legacy extension doesn't seem useful. We need to differenciate the existing calls: (1) actually board specific and have not place in a cpu abstraction layer: getchar/putchar, these should just never be advertised in a non-legacy setup, and the drivers using them should not probe on a sbi 0.2+ system (2) useful for currently taped out cpus and in the long run for virtualization to avoid mmio traps: ipis, timers, tlb shootdown. These should stay backwards compatible, but for sbi 0.2 be negotiated individually (3) in theory useful, but given how much of a big hammer sfence.i not useful in theory: SBI_REMOTE_FENCE_I we can decide if we want to either not allow it for sbi 0.2+ or also negotiate it. I'd personally favor not advertising it and just use ipis to implement it. If we want useful acceleration of i-cache synchronization we'll need actual instructions that are much more fine grained in the future. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 0/2] Add support for SBI version to 0.2 2019-08-27 14:46 ` [RFC PATCH 0/2] Add support for SBI version to 0.2 Christoph Hellwig @ 2019-08-27 22:19 ` Atish Patra 2019-08-29 10:59 ` hch 0 siblings, 1 reply; 27+ messages in thread From: Atish Patra @ 2019-08-27 22:19 UTC (permalink / raw) To: hch Cc: linux-riscv, alankao, gregkh, gary, rppt, paul.walmsley, aou, linux-kernel, tglx, anup, alexios.zavras, palmer On Tue, 2019-08-27 at 07:46 -0700, Christoph Hellwig wrote: > On Mon, Aug 26, 2019 at 04:32:54PM -0700, Atish Patra wrote: > > This patch series aims to add support for SBI specification version > > v0.2. It doesn't break compatibility with any v0.1 implementation. > > Internally, all the v0.1 calls are just renamed to legacy to be in > > sync with specification [1]. > > > > The patches for v0.2 support in OpenSBI are available at > > http://lists.infradead.org/pipermail/opensbi/2019-August/000422.html > > > > [1] > > https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.adoc > > I really don't like the current design of that SBI 0.2 spec, > and don't think implementing it as-is is helpful. > > For one the way how the extension id is placed creates a compatibilty > problem, not allowing your to implement a backwards compatible sbi, > which seems bad. > I did not understand this part. All the legacy SBI calls are defined as a separate extension ID not single extension. How did it break the backward compatibility ? Here are the few possible usecases 1. New kernel can use SBI v0.2 calling convention for older legacy calls. It will just set a6 to zero (function id) & not use the return value in a1. a0 is still report error value. 2. New kernel with older SBI implementation (i.e. BBL) will also work as sbi_get_spec_version will return error and spec_version will be set 0.1. BBL never checks a6 or set a1 which works for the legacy calls. 3. Older kernel with new SBI implementation(i.e OpenSBI) will never use v0.2 calling conventions. OpenSBI never use a6 or set a1 for legacy calls anyways. Did I miss any usecase ? > Second just blindly moving all the existing calls to a single legacy > extension doesn't seem useful. We need to differenciate the existing > calls: I think the confusion is because of legacy renaming. They are not single legacy extension. They are all separate extensions. The spec just called all those extensions as collectively as legacy. So I just tried to make the patch sync with the spec. If that's the source of confusion, I can rename it to sbi_0.1_x in stead of legacy. > > (1) actually board specific and have not place in a cpu abstraction > layer: getchar/putchar, these should just never be advertised in > a > non-legacy setup, and the drivers using them should not probe > on a sbi 0.2+ system In that case, we have to update the drivers(earlycon-riscv-sbi & hvc_riscv_sbi) in kernel as well. Once these patches are merged, nobody will be able to use earlycon=sbi feature in mainline kernel. Personally, I am fine with it. But there were some interest during RISC-V workshop in keeping these for now for easy debugging and early bringup. > (2) useful for currently taped out cpus and in the long run for > virtualization to avoid mmio traps: ipis, timers, tlb > shootdown. > These should stay backwards compatible, but for sbi 0.2 be > negotiated individually We still can do that with existing scheme. > (3) in theory useful, but given how much of a big hammer sfence.i > not useful in theory: SBI_REMOTE_FENCE_I we can decide if we > want > to either not allow it for sbi 0.2+ or also negotiate it. I'd > personally favor not advertising it and just use ipis to > implement > it. Once we have native IPIs, sure. Otherwise, sfence.i using IPI via SBI will be even slower compared today. Gary had done the same thing for sfence.vma and it was not enocouraged. https://patchwork.kernel.org/patch/10845959/#22576987 > If we want useful acceleration of i-cache synchronization > we'll need actual instructions that are much more fine grained > in the future. -- Regards, Atish ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 0/2] Add support for SBI version to 0.2 2019-08-27 22:19 ` Atish Patra @ 2019-08-29 10:59 ` hch 2019-08-30 23:13 ` Atish Patra 0 siblings, 1 reply; 27+ messages in thread From: hch @ 2019-08-29 10:59 UTC (permalink / raw) To: Atish Patra Cc: hch, aou, alankao, gregkh, anup, palmer, linux-kernel, rppt, alexios.zavras, gary, paul.walmsley, linux-riscv, tglx On Tue, Aug 27, 2019 at 10:19:42PM +0000, Atish Patra wrote: > I did not understand this part. All the legacy SBI calls are defined as > a separate extension ID not single extension. How did it break the > backward compatibility ? Yes, sorry I mistead this. The way is is defined is rather non-intuitive, but actually backwards compatible. > I think the confusion is because of legacy renaming. They are not > single legacy extension. They are all separate extensions. The spec > just called all those extensions as collectively as legacy. So I just > tried to make the patch sync with the spec. > > If that's the source of confusion, I can rename it to sbi_0.1_x in > stead of legacy. I think we actually need to fix the spec instead, even if it just the naming and not the mechanism. > > (1) actually board specific and have not place in a cpu abstraction > > layer: getchar/putchar, these should just never be advertised in > > a > > non-legacy setup, and the drivers using them should not probe > > on a sbi 0.2+ system > > In that case, we have to update the drivers(earlycon-riscv-sbi & > hvc_riscv_sbi) in kernel as well. Once these patches are merged, nobody > will be able to use earlycon=sbi feature in mainline kernel. > > Personally, I am fine with it. But there were some interest during > RISC-V workshop in keeping these for now for easy debugging and early > bringup. The getchar/putchar calls unfortunately are fundamentally flawed, as they mean the sbi can still access the console after the host has taken it over using its own drivers. Which will lead to bugs sooner or later. And if you can bring up a console driver in opensbi it should be just as trivial to bring up the kernel version. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 0/2] Add support for SBI version to 0.2 2019-08-29 10:59 ` hch @ 2019-08-30 23:13 ` Atish Patra 2019-09-03 7:38 ` hch 0 siblings, 1 reply; 27+ messages in thread From: Atish Patra @ 2019-08-30 23:13 UTC (permalink / raw) To: hch Cc: linux-riscv, alankao, gregkh, gary, rppt, paul.walmsley, linux-kernel, aou, tglx, anup, alexios.zavras, palmer On Thu, 2019-08-29 at 03:59 -0700, hch@infradead.org wrote: > On Tue, Aug 27, 2019 at 10:19:42PM +0000, Atish Patra wrote: > > I did not understand this part. All the legacy SBI calls are > > defined as > > a separate extension ID not single extension. How did it break the > > backward compatibility ? > > Yes, sorry I mistead this. The way is is defined is rather > non-intuitive, but actually backwards compatible. > > > I think the confusion is because of legacy renaming. They are not > > single legacy extension. They are all separate extensions. The spec > > just called all those extensions as collectively as legacy. So I > > just > > tried to make the patch sync with the spec. > > > > If that's the source of confusion, I can rename it to sbi_0.1_x in > > stead of legacy. > > I think we actually need to fix the spec instead, even if it just the > naming and not the mechanism. > If I understood you clearly, you want to call it legacy in the spec and just say v0.1 extensions. The whole idea of marking them as legacy extensions to indicate that it would be obsolete in the future. But I am not too worried about the semantics here. So I am fine with just changing the text to v0.1 if that avoids confusion. > > > (1) actually board specific and have not place in a cpu > > > abstraction > > > layer: getchar/putchar, these should just never be > > > advertised in > > > a > > > non-legacy setup, and the drivers using them should not > > > probe > > > on a sbi 0.2+ system > > > > In that case, we have to update the drivers(earlycon-riscv-sbi & > > hvc_riscv_sbi) in kernel as well. Once these patches are merged, > > nobody > > will be able to use earlycon=sbi feature in mainline kernel. > > > > Personally, I am fine with it. But there were some interest during > > RISC-V workshop in keeping these for now for easy debugging and > > early > > bringup. > > The getchar/putchar calls unfortunately are fundamentally flawed, as > they mean the sbi can still access the console after the host has > taken > it over using its own drivers. Which will lead to bugs sooner or > later. > > And if you can bring up a console driver in opensbi it should be just > as trivial to bring up the kernel version. -- Regards, Atish ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 0/2] Add support for SBI version to 0.2 2019-08-30 23:13 ` Atish Patra @ 2019-09-03 7:38 ` hch [not found] ` <CANs6eMmcbtJ5KTU00LpfTtXszsdi1Jem_5j6GWO+8Yo3JnvTqg@mail.gmail.com> 0 siblings, 1 reply; 27+ messages in thread From: hch @ 2019-09-03 7:38 UTC (permalink / raw) To: Atish Patra Cc: hch, aou, alankao, gregkh, anup, palmer, linux-kernel, rppt, alexios.zavras, gary, paul.walmsley, linux-riscv, tglx On Fri, Aug 30, 2019 at 11:13:25PM +0000, Atish Patra wrote: > If I understood you clearly, you want to call it legacy in the spec and > just say v0.1 extensions. > > The whole idea of marking them as legacy extensions to indicate that it > would be obsolete in the future. > > But I am not too worried about the semantics here. So I am fine with > just changing the text to v0.1 if that avoids confusion. So my main problems is that we are lumping all the "legacy" extensions together. While some of them are simply a bad idea and shouldn't really be implemented for anything new ever, others like the sfence.vma and ipi ones are needed until we have hardware support to avoid them and possibly forever for virtualization. So either we use different markers of legacy for them, or we at least define new extensions that replace them at the same time. What I want to avoid is the possibіly of an implementation using the really legacy bits and new extensions at the same time. ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <CANs6eMmcbtJ5KTU00LpfTtXszsdi1Jem_5j6GWO+8Yo3JnvTqg@mail.gmail.com>]
* Re: [RFC PATCH 0/2] Add support for SBI version to 0.2 [not found] ` <CANs6eMmcbtJ5KTU00LpfTtXszsdi1Jem_5j6GWO+8Yo3JnvTqg@mail.gmail.com> @ 2019-09-16 6:54 ` hch 2019-09-16 16:12 ` Palmer Dabbelt 0 siblings, 1 reply; 27+ messages in thread From: hch @ 2019-09-16 6:54 UTC (permalink / raw) To: Palmer Dabbelt Cc: hch, Atish Patra, aou, alankao, gregkh, anup, linux-kernel, rppt, alexios.zavras, gary, paul.walmsley, linux-riscv, tglx On Fri, Sep 13, 2019 at 08:54:27AM -0700, Palmer Dabbelt wrote: > On Tue, Sep 3, 2019 at 12:38 AM hch@infradead.org <hch@infradead.org> wrote: > > > On Fri, Aug 30, 2019 at 11:13:25PM +0000, Atish Patra wrote: > > > If I understood you clearly, you want to call it legacy in the spec and > > > just say v0.1 extensions. > > > > > > The whole idea of marking them as legacy extensions to indicate that it > > > would be obsolete in the future. > > > > > > But I am not too worried about the semantics here. So I am fine with > > > just changing the text to v0.1 if that avoids confusion. > > > > So my main problems is that we are lumping all the "legacy" extensions > > together. While some of them are simply a bad idea and shouldn't > > really be implemented for anything new ever, others like the sfence.vma > > and ipi ones are needed until we have hardware support to avoid them > > and possibly forever for virtualization. > > > > So either we use different markers of legacy for them, or we at least > > define new extensions that replace them at the same time. What I > > want to avoid is the possibіly of an implementation using the really > > legacy bits and new extensions at the same time. > > > > Nominally we've got to replace these as well because we didn't include > the length of the hart mask. Well, let's do that as part of definining the first real post-0.1 SBI then, and don't bother defining the old ones as legacy at all. Just two different specs that don't interact except that we reserve extension space in the new one for the old one so that one SBI spec can implement both. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 0/2] Add support for SBI version to 0.2 2019-09-16 6:54 ` hch @ 2019-09-16 16:12 ` Palmer Dabbelt 0 siblings, 0 replies; 27+ messages in thread From: Palmer Dabbelt @ 2019-09-16 16:12 UTC (permalink / raw) To: Christoph Hellwig Cc: aou, alankao, alexios.zavras, anup, Paul Walmsley, linux-kernel, rppt, Christoph Hellwig, Atish Patra, gary, Greg KH, linux-riscv, tglx On Sun, 15 Sep 2019 23:54:46 PDT (-0700), Christoph Hellwig wrote: > On Fri, Sep 13, 2019 at 08:54:27AM -0700, Palmer Dabbelt wrote: >> On Tue, Sep 3, 2019 at 12:38 AM hch@infradead.org <hch@infradead.org> wrote: >> >> > On Fri, Aug 30, 2019 at 11:13:25PM +0000, Atish Patra wrote: >> > > If I understood you clearly, you want to call it legacy in the spec and >> > > just say v0.1 extensions. >> > > >> > > The whole idea of marking them as legacy extensions to indicate that it >> > > would be obsolete in the future. >> > > >> > > But I am not too worried about the semantics here. So I am fine with >> > > just changing the text to v0.1 if that avoids confusion. >> > >> > So my main problems is that we are lumping all the "legacy" extensions >> > together. While some of them are simply a bad idea and shouldn't >> > really be implemented for anything new ever, others like the sfence.vma >> > and ipi ones are needed until we have hardware support to avoid them >> > and possibly forever for virtualization. >> > >> > So either we use different markers of legacy for them, or we at least >> > define new extensions that replace them at the same time. What I >> > want to avoid is the possibіly of an implementation using the really >> > legacy bits and new extensions at the same time. >> > >> >> Nominally we've got to replace these as well because we didn't include >> the length of the hart mask. > > Well, let's do that as part of definining the first real post-0.1 > SBI then, and don't bother defining the old ones as legacy at all. > > Just two different specs that don't interact except that we reserve > extension space in the new one for the old one so that one SBI spec > can implement both. Makes sense. We're getting finish with this "just go write everything down" exercise, so we can start actually doing things now :). ^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2019-09-16 16:12 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-26 23:32 [RFC PATCH 0/2] Add support for SBI version to 0.2 Atish Patra
2019-08-26 23:32 ` [RFC PATCH 1/2] RISC-V: Mark existing SBI as legacy SBI Atish Patra
2019-08-27 7:51 ` Mike Rapoport
2019-08-27 8:28 ` Anup Patel
2019-08-27 8:37 ` Mike Rapoport
2019-08-28 21:37 ` Palmer Dabbelt
2019-08-27 20:34 ` Atish Patra
2019-08-27 14:03 ` Christoph Hellwig
2019-08-27 14:04 ` Christoph Hellwig
2019-08-27 20:37 ` Atish Patra
2019-08-29 10:56 ` hch
2019-08-26 23:32 ` [RFC PATCH 2/2] RISC-V: Add basic support for SBI v0.2 Atish Patra
2019-08-27 7:58 ` Mike Rapoport
2019-08-27 8:23 ` Anup Patel
2019-08-27 8:39 ` Mike Rapoport
2019-08-27 9:28 ` Anup Patel
2019-08-27 20:30 ` Atish Patra
2019-08-27 9:36 ` Anup Patel
2019-08-27 20:43 ` Atish Patra
2019-08-27 14:11 ` Christoph Hellwig
2019-08-27 14:46 ` [RFC PATCH 0/2] Add support for SBI version to 0.2 Christoph Hellwig
2019-08-27 22:19 ` Atish Patra
2019-08-29 10:59 ` hch
2019-08-30 23:13 ` Atish Patra
2019-09-03 7:38 ` hch
[not found] ` <CANs6eMmcbtJ5KTU00LpfTtXszsdi1Jem_5j6GWO+8Yo3JnvTqg@mail.gmail.com>
2019-09-16 6:54 ` hch
2019-09-16 16:12 ` Palmer Dabbelt
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).