diff mbox series

[1/2] riscv: sbi: Remove sbi_spec_version

Message ID 1590570293-14225-1-git-send-email-bmeng.cn@gmail.com
State Accepted
Commit fe9435571612066cc897e8c80921cba01c068c64
Delegated to: Andes
Headers show
Series [1/2] riscv: sbi: Remove sbi_spec_version | expand

Commit Message

Bin Meng May 27, 2020, 9:04 a.m. UTC 
From: Bin Meng <bin.meng@windriver.com>

U-Boot defaults to use SBI v0.2. Howerver there is a global variable
sbi_spec_version that stills refers to v0.1. Since it is not used
anywhere, let's remove it.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
---

 arch/riscv/include/asm/sbi.h | 2 --
 arch/riscv/lib/sbi.c         | 3 ---
 2 files changed, 5 deletions(-)

Comments

Rick Chen June 1, 2020, 8:13 a.m. UTC | #1 
Hi Bin

> From: Bin Meng [mailto:bmeng.cn@gmail.com]
> Sent: Wednesday, May 27, 2020 5:05 PM
> To: Rick Jian-Zhi Chen(陳建志); U-Boot Mailing List
> Cc: Atish Patra; Bin Meng
> Subject: [PATCH 1/2] riscv: sbi: Remove sbi_spec_version
>
> From: Bin Meng <bin.meng@windriver.com>
>
> U-Boot defaults to use SBI v0.2. Howerver there is a global variable sbi_spec_version that stills refers to v0.1. Since it is not used anywhere, let's remove it.
>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
>
>  arch/riscv/include/asm/sbi.h | 2 --
>  arch/riscv/lib/sbi.c         | 3 ---
>  2 files changed, 5 deletions(-)
>
> diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h index 453cb5c..08e1ac0 100644
> --- a/arch/riscv/include/asm/sbi.h
> +++ b/arch/riscv/include/asm/sbi.h
> @@ -77,7 +77,6 @@ enum sbi_ext_rfence_fid {
>  #define SBI_FID_REMOTE_SFENCE_VMA_ASID SBI_EXT_RFENCE_REMOTE_SFENCE_VMA_ASID
>  #endif
>
> -#define SBI_SPEC_VERSION_DEFAULT       0x1
>  #define SBI_SPEC_VERSION_MAJOR_SHIFT   24
>  #define SBI_SPEC_VERSION_MAJOR_MASK    0x7f
>  #define SBI_SPEC_VERSION_MINOR_MASK    0xffffff
> @@ -90,7 +89,6 @@ enum sbi_ext_rfence_fid {
>  #define SBI_ERR_DENIED                 -4
>  #define SBI_ERR_INVALID_ADDRESS                -5
>
> -extern unsigned long sbi_spec_version;
>  struct sbiret {
>         long error;
>         long value;
> diff --git a/arch/riscv/lib/sbi.c b/arch/riscv/lib/sbi.c index 993597e..f298846 100644
> --- a/arch/riscv/lib/sbi.c
> +++ b/arch/riscv/lib/sbi.c
> @@ -11,9 +11,6 @@
>  #include <asm/encoding.h>
>  #include <asm/sbi.h>
>
> -/* default SBI version is 0.1 */
> -unsigned long sbi_spec_version = SBI_SPEC_VERSION_DEFAULT;

Why not keep this variable and get version of openSbi automatically,
then register v01 or v02 callback function just like sbi_init() of
Atish' patch.

Thanks,
Rick

> -
>  struct sbiret sbi_ecall(int ext, int fid, unsigned long arg0,
>                         unsigned long arg1, unsigned long arg2,
>                         unsigned long arg3, unsigned long arg4,
> --
> 2.7.4
>
Bin Meng June 1, 2020, 9:06 a.m. UTC | #2 
Hi Rick,

On Mon, Jun 1, 2020 at 4:14 PM Rick Chen <rickchen36@gmail.com> wrote:
>
> Hi Bin
>
> > From: Bin Meng [mailto:bmeng.cn@gmail.com]
> > Sent: Wednesday, May 27, 2020 5:05 PM
> > To: Rick Jian-Zhi Chen(陳建志); U-Boot Mailing List
> > Cc: Atish Patra; Bin Meng
> > Subject: [PATCH 1/2] riscv: sbi: Remove sbi_spec_version
> >
> > From: Bin Meng <bin.meng@windriver.com>
> >
> > U-Boot defaults to use SBI v0.2. Howerver there is a global variable sbi_spec_version that stills refers to v0.1. Since it is not used anywhere, let's remove it.
> >
> > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > ---
> >
> >  arch/riscv/include/asm/sbi.h | 2 --
> >  arch/riscv/lib/sbi.c         | 3 ---
> >  2 files changed, 5 deletions(-)
> >
> > diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h index 453cb5c..08e1ac0 100644
> > --- a/arch/riscv/include/asm/sbi.h
> > +++ b/arch/riscv/include/asm/sbi.h
> > @@ -77,7 +77,6 @@ enum sbi_ext_rfence_fid {
> >  #define SBI_FID_REMOTE_SFENCE_VMA_ASID SBI_EXT_RFENCE_REMOTE_SFENCE_VMA_ASID
> >  #endif
> >
> > -#define SBI_SPEC_VERSION_DEFAULT       0x1
> >  #define SBI_SPEC_VERSION_MAJOR_SHIFT   24
> >  #define SBI_SPEC_VERSION_MAJOR_MASK    0x7f
> >  #define SBI_SPEC_VERSION_MINOR_MASK    0xffffff
> > @@ -90,7 +89,6 @@ enum sbi_ext_rfence_fid {
> >  #define SBI_ERR_DENIED                 -4
> >  #define SBI_ERR_INVALID_ADDRESS                -5
> >
> > -extern unsigned long sbi_spec_version;
> >  struct sbiret {
> >         long error;
> >         long value;
> > diff --git a/arch/riscv/lib/sbi.c b/arch/riscv/lib/sbi.c index 993597e..f298846 100644
> > --- a/arch/riscv/lib/sbi.c
> > +++ b/arch/riscv/lib/sbi.c
> > @@ -11,9 +11,6 @@
> >  #include <asm/encoding.h>
> >  #include <asm/sbi.h>
> >
> > -/* default SBI version is 0.1 */
> > -unsigned long sbi_spec_version = SBI_SPEC_VERSION_DEFAULT;
>
> Why not keep this variable and get version of openSbi automatically,
> then register v01 or v02 callback function just like sbi_init() of
> Atish' patch.

I feel this is not needed anyway, because we are using Kconfig option
to pass the same information.

Regards,
Bin
Rick Chen June 2, 2020, 9:12 a.m. UTC | #3 
Hi Bin

Bin Meng <bmeng.cn@gmail.com> 於 2020年6月1日 週一 下午5:06寫道：
>
> Hi Rick,
>
> On Mon, Jun 1, 2020 at 4:14 PM Rick Chen <rickchen36@gmail.com> wrote:
> >
> > Hi Bin
> >
> > > From: Bin Meng [mailto:bmeng.cn@gmail.com]
> > > Sent: Wednesday, May 27, 2020 5:05 PM
> > > To: Rick Jian-Zhi Chen(陳建志); U-Boot Mailing List
> > > Cc: Atish Patra; Bin Meng
> > > Subject: [PATCH 1/2] riscv: sbi: Remove sbi_spec_version
> > >
> > > From: Bin Meng <bin.meng@windriver.com>
> > >
> > > U-Boot defaults to use SBI v0.2. Howerver there is a global variable sbi_spec_version that stills refers to v0.1. Since it is not used anywhere, let's remove it.
> > >
> > > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > > ---
> > >
> > >  arch/riscv/include/asm/sbi.h | 2 --
> > >  arch/riscv/lib/sbi.c         | 3 ---
> > >  2 files changed, 5 deletions(-)
> > >
> > > diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h index 453cb5c..08e1ac0 100644
> > > --- a/arch/riscv/include/asm/sbi.h
> > > +++ b/arch/riscv/include/asm/sbi.h
> > > @@ -77,7 +77,6 @@ enum sbi_ext_rfence_fid {
> > >  #define SBI_FID_REMOTE_SFENCE_VMA_ASID SBI_EXT_RFENCE_REMOTE_SFENCE_VMA_ASID
> > >  #endif
> > >
> > > -#define SBI_SPEC_VERSION_DEFAULT       0x1
> > >  #define SBI_SPEC_VERSION_MAJOR_SHIFT   24
> > >  #define SBI_SPEC_VERSION_MAJOR_MASK    0x7f
> > >  #define SBI_SPEC_VERSION_MINOR_MASK    0xffffff
> > > @@ -90,7 +89,6 @@ enum sbi_ext_rfence_fid {
> > >  #define SBI_ERR_DENIED                 -4
> > >  #define SBI_ERR_INVALID_ADDRESS                -5
> > >
> > > -extern unsigned long sbi_spec_version;
> > >  struct sbiret {
> > >         long error;
> > >         long value;
> > > diff --git a/arch/riscv/lib/sbi.c b/arch/riscv/lib/sbi.c index 993597e..f298846 100644
> > > --- a/arch/riscv/lib/sbi.c
> > > +++ b/arch/riscv/lib/sbi.c
> > > @@ -11,9 +11,6 @@
> > >  #include <asm/encoding.h>
> > >  #include <asm/sbi.h>
> > >
> > > -/* default SBI version is 0.1 */
> > > -unsigned long sbi_spec_version = SBI_SPEC_VERSION_DEFAULT;
> >
> > Why not keep this variable and get version of openSbi automatically,
> > then register v01 or v02 callback function just like sbi_init() of
> > Atish' patch.
>
> I feel this is not needed anyway, because we are using Kconfig option
> to pass the same information.

U-Boot proper has been configured as SBI_V02 by default currently.
About backward compatible issue, it is not so friendly for users.
They shall select SBI_VERSION configurations correctly in U-Boot
proper and re-compile between different versions of openSbi.
If U-Boot proper can probe openSbi and switch V01 or V02 mode
automatically, users can run smoothly without awareness of SBI
versions.

Thanks,
Rick



>
> Regards,
> Bin
Bin Meng June 2, 2020, 9:39 a.m. UTC | #4 
Hi Rick,

On Tue, Jun 2, 2020 at 5:13 PM Rick Chen <rickchen36@gmail.com> wrote:
>
> Hi Bin
>
> Bin Meng <bmeng.cn@gmail.com> 於 2020年6月1日 週一 下午5:06寫道：
> >
> > Hi Rick,
> >
> > On Mon, Jun 1, 2020 at 4:14 PM Rick Chen <rickchen36@gmail.com> wrote:
> > >
> > > Hi Bin
> > >
> > > > From: Bin Meng [mailto:bmeng.cn@gmail.com]
> > > > Sent: Wednesday, May 27, 2020 5:05 PM
> > > > To: Rick Jian-Zhi Chen(陳建志); U-Boot Mailing List
> > > > Cc: Atish Patra; Bin Meng
> > > > Subject: [PATCH 1/2] riscv: sbi: Remove sbi_spec_version
> > > >
> > > > From: Bin Meng <bin.meng@windriver.com>
> > > >
> > > > U-Boot defaults to use SBI v0.2. Howerver there is a global variable sbi_spec_version that stills refers to v0.1. Since it is not used anywhere, let's remove it.
> > > >
> > > > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > > > ---
> > > >
> > > >  arch/riscv/include/asm/sbi.h | 2 --
> > > >  arch/riscv/lib/sbi.c         | 3 ---
> > > >  2 files changed, 5 deletions(-)
> > > >
> > > > diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h index 453cb5c..08e1ac0 100644
> > > > --- a/arch/riscv/include/asm/sbi.h
> > > > +++ b/arch/riscv/include/asm/sbi.h
> > > > @@ -77,7 +77,6 @@ enum sbi_ext_rfence_fid {
> > > >  #define SBI_FID_REMOTE_SFENCE_VMA_ASID SBI_EXT_RFENCE_REMOTE_SFENCE_VMA_ASID
> > > >  #endif
> > > >
> > > > -#define SBI_SPEC_VERSION_DEFAULT       0x1
> > > >  #define SBI_SPEC_VERSION_MAJOR_SHIFT   24
> > > >  #define SBI_SPEC_VERSION_MAJOR_MASK    0x7f
> > > >  #define SBI_SPEC_VERSION_MINOR_MASK    0xffffff
> > > > @@ -90,7 +89,6 @@ enum sbi_ext_rfence_fid {
> > > >  #define SBI_ERR_DENIED                 -4
> > > >  #define SBI_ERR_INVALID_ADDRESS                -5
> > > >
> > > > -extern unsigned long sbi_spec_version;
> > > >  struct sbiret {
> > > >         long error;
> > > >         long value;
> > > > diff --git a/arch/riscv/lib/sbi.c b/arch/riscv/lib/sbi.c index 993597e..f298846 100644
> > > > --- a/arch/riscv/lib/sbi.c
> > > > +++ b/arch/riscv/lib/sbi.c
> > > > @@ -11,9 +11,6 @@
> > > >  #include <asm/encoding.h>
> > > >  #include <asm/sbi.h>
> > > >
> > > > -/* default SBI version is 0.1 */
> > > > -unsigned long sbi_spec_version = SBI_SPEC_VERSION_DEFAULT;
> > >
> > > Why not keep this variable and get version of openSbi automatically,
> > > then register v01 or v02 callback function just like sbi_init() of
> > > Atish' patch.
> >
> > I feel this is not needed anyway, because we are using Kconfig option
> > to pass the same information.
>
> U-Boot proper has been configured as SBI_V02 by default currently.
> About backward compatible issue, it is not so friendly for users.
> They shall select SBI_VERSION configurations correctly in U-Boot
> proper and re-compile between different versions of openSbi.
> If U-Boot proper can probe openSbi and switch V01 or V02 mode
> automatically, users can run smoothly without awareness of SBI
> versions.
>

OpenSBI v0.7 is not backward compatible with previous version
regarding the multicore boot. Dynamically detecting SBI and the SBI
implementation version will make U-Boot codes very complicated. In
addition to SBI v0.1 vs. v0.2, we need detect whether SBI HSM
extension is available and if that's not available, U-Boot has to do
the lottery multi-core boot in every stage (SPL and proper). RISC-V is
a new architecture and without a lot of legacy burden to carry on, I
hope we can do the clean room implementation from the beginning.

Regards,
Bin
Rick Chen June 3, 2020, 2:33 a.m. UTC | #5 
Hi Bin

Bin Meng <bmeng.cn@gmail.com> 於 2020年6月2日 週二 下午5:39寫道：
>
> Hi Rick,
>
> On Tue, Jun 2, 2020 at 5:13 PM Rick Chen <rickchen36@gmail.com> wrote:
> >
> > Hi Bin
> >
> > Bin Meng <bmeng.cn@gmail.com> 於 2020年6月1日 週一 下午5:06寫道：
> > >
> > > Hi Rick,
> > >
> > > On Mon, Jun 1, 2020 at 4:14 PM Rick Chen <rickchen36@gmail.com> wrote:
> > > >
> > > > Hi Bin
> > > >
> > > > > From: Bin Meng [mailto:bmeng.cn@gmail.com]
> > > > > Sent: Wednesday, May 27, 2020 5:05 PM
> > > > > To: Rick Jian-Zhi Chen(陳建志); U-Boot Mailing List
> > > > > Cc: Atish Patra; Bin Meng
> > > > > Subject: [PATCH 1/2] riscv: sbi: Remove sbi_spec_version
> > > > >
> > > > > From: Bin Meng <bin.meng@windriver.com>
> > > > >
> > > > > U-Boot defaults to use SBI v0.2. Howerver there is a global variable sbi_spec_version that stills refers to v0.1. Since it is not used anywhere, let's remove it.
> > > > >
> > > > > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > > > > ---
> > > > >
> > > > >  arch/riscv/include/asm/sbi.h | 2 --
> > > > >  arch/riscv/lib/sbi.c         | 3 ---
> > > > >  2 files changed, 5 deletions(-)
> > > > >
> > > > > diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h index 453cb5c..08e1ac0 100644
> > > > > --- a/arch/riscv/include/asm/sbi.h
> > > > > +++ b/arch/riscv/include/asm/sbi.h
> > > > > @@ -77,7 +77,6 @@ enum sbi_ext_rfence_fid {
> > > > >  #define SBI_FID_REMOTE_SFENCE_VMA_ASID SBI_EXT_RFENCE_REMOTE_SFENCE_VMA_ASID
> > > > >  #endif
> > > > >
> > > > > -#define SBI_SPEC_VERSION_DEFAULT       0x1
> > > > >  #define SBI_SPEC_VERSION_MAJOR_SHIFT   24
> > > > >  #define SBI_SPEC_VERSION_MAJOR_MASK    0x7f
> > > > >  #define SBI_SPEC_VERSION_MINOR_MASK    0xffffff
> > > > > @@ -90,7 +89,6 @@ enum sbi_ext_rfence_fid {
> > > > >  #define SBI_ERR_DENIED                 -4
> > > > >  #define SBI_ERR_INVALID_ADDRESS                -5
> > > > >
> > > > > -extern unsigned long sbi_spec_version;
> > > > >  struct sbiret {
> > > > >         long error;
> > > > >         long value;
> > > > > diff --git a/arch/riscv/lib/sbi.c b/arch/riscv/lib/sbi.c index 993597e..f298846 100644
> > > > > --- a/arch/riscv/lib/sbi.c
> > > > > +++ b/arch/riscv/lib/sbi.c
> > > > > @@ -11,9 +11,6 @@
> > > > >  #include <asm/encoding.h>
> > > > >  #include <asm/sbi.h>
> > > > >
> > > > > -/* default SBI version is 0.1 */
> > > > > -unsigned long sbi_spec_version = SBI_SPEC_VERSION_DEFAULT;
> > > >
> > > > Why not keep this variable and get version of openSbi automatically,
> > > > then register v01 or v02 callback function just like sbi_init() of
> > > > Atish' patch.
> > >
> > > I feel this is not needed anyway, because we are using Kconfig option
> > > to pass the same information.
> >
> > U-Boot proper has been configured as SBI_V02 by default currently.
> > About backward compatible issue, it is not so friendly for users.
> > They shall select SBI_VERSION configurations correctly in U-Boot
> > proper and re-compile between different versions of openSbi.
> > If U-Boot proper can probe openSbi and switch V01 or V02 mode
> > automatically, users can run smoothly without awareness of SBI
> > versions.
> >
>
> OpenSBI v0.7 is not backward compatible with previous version
> regarding the multicore boot. Dynamically detecting SBI and the SBI
> implementation version will make U-Boot codes very complicated. In
> addition to SBI v0.1 vs. v0.2, we need detect whether SBI HSM
> extension is available and if that's not available, U-Boot has to do
> the lottery multi-core boot in every stage (SPL and proper). RISC-V is
> a new architecture and without a lot of legacy burden to carry on, I
> hope we can do the clean room implementation from the beginning.
>

OK.
Thanks for explanation.

Reviewed-by: Rick Chen <rick@andestech.com>

> Regards,
> Bin
diff mbox series

Patch

diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
index 453cb5c..08e1ac0 100644
--- a/arch/riscv/include/asm/sbi.h
+++ b/arch/riscv/include/asm/sbi.h
@@ -77,7 +77,6 @@  enum sbi_ext_rfence_fid {
 #define SBI_FID_REMOTE_SFENCE_VMA_ASID	SBI_EXT_RFENCE_REMOTE_SFENCE_VMA_ASID
 #endif
 
-#define SBI_SPEC_VERSION_DEFAULT	0x1
 #define SBI_SPEC_VERSION_MAJOR_SHIFT	24
 #define SBI_SPEC_VERSION_MAJOR_MASK	0x7f
 #define SBI_SPEC_VERSION_MINOR_MASK	0xffffff
@@ -90,7 +89,6 @@  enum sbi_ext_rfence_fid {
 #define SBI_ERR_DENIED			-4
 #define SBI_ERR_INVALID_ADDRESS		-5
 
-extern unsigned long sbi_spec_version;
 struct sbiret {
 	long error;
 	long value;
diff --git a/arch/riscv/lib/sbi.c b/arch/riscv/lib/sbi.c
index 993597e..f298846 100644
--- a/arch/riscv/lib/sbi.c
+++ b/arch/riscv/lib/sbi.c
@@ -11,9 +11,6 @@ 
 #include <asm/encoding.h>
 #include <asm/sbi.h>
 
-/* default SBI version is 0.1 */
-unsigned long sbi_spec_version = SBI_SPEC_VERSION_DEFAULT;
-
 struct sbiret sbi_ecall(int ext, int fid, unsigned long arg0,
 			unsigned long arg1, unsigned long arg2,
 			unsigned long arg3, unsigned long arg4,