Re: [PATCH 1/2] riscv: sbi: Remove sbi_spec_version

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

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

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

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

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
participants (2)
-
Bin Meng
-
Rick Chen