* [PATCH 0/2] cmd/sbi: add missing SBI information
@ 2021-07-19 20:28 Heinrich Schuchardt
2021-07-19 20:28 ` [PATCH 1/2] riscv: provide missing base extension functions Heinrich Schuchardt
2021-07-19 20:28 ` [PATCH 2/2] cmd/sbi: add missing SBI information Heinrich Schuchardt
0 siblings, 2 replies; 9+ messages in thread
From: Heinrich Schuchardt @ 2021-07-19 20:28 UTC (permalink / raw)
To: Rick Chen
Cc: Leo, Bin Meng, Atish Patra, Pragnesh Patel, u-boot,
Dimitri John Ledkov, Heinrich Schuchardt
The series provides library functions to read
* SBI implementation version
* machine vendor ID
* machine architecture ID
* machine implementation ID
and enhances the sbi command to display this information.
Heinrich Schuchardt (2):
riscv: provide missing base extension functions
cmd/sbi: add missing SBI information
arch/riscv/include/asm/sbi.h | 4 ++
arch/riscv/lib/sbi.c | 72 ++++++++++++++++++++++++++++++++++++
cmd/riscv/sbi.c | 19 +++++++++-
3 files changed, 94 insertions(+), 1 deletion(-)
--
2.30.2
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] riscv: provide missing base extension functions
2021-07-19 20:28 [PATCH 0/2] cmd/sbi: add missing SBI information Heinrich Schuchardt
@ 2021-07-19 20:28 ` Heinrich Schuchardt
2021-07-20 1:07 ` Sean Anderson
2021-07-26 6:44 ` Leo Liang
2021-07-19 20:28 ` [PATCH 2/2] cmd/sbi: add missing SBI information Heinrich Schuchardt
1 sibling, 2 replies; 9+ messages in thread
From: Heinrich Schuchardt @ 2021-07-19 20:28 UTC (permalink / raw)
To: Rick Chen
Cc: Leo, Bin Meng, Atish Patra, Pragnesh Patel, u-boot,
Dimitri John Ledkov, Heinrich Schuchardt, Heinrich Schuchardt
Provide library functions to read:
* SBI implementation version
* machine vendor ID
* machine architecture ID
* machine implementation ID
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
arch/riscv/include/asm/sbi.h | 4 ++
arch/riscv/lib/sbi.c | 71 ++++++++++++++++++++++++++++++++++++
2 files changed, 75 insertions(+)
diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
index 53ca316180..262cc9228c 100644
--- a/arch/riscv/include/asm/sbi.h
+++ b/arch/riscv/include/asm/sbi.h
@@ -117,6 +117,10 @@ void sbi_remote_sfence_vma_asid(const unsigned long *hart_mask,
void sbi_set_timer(uint64_t stime_value);
long sbi_get_spec_version(void);
int sbi_get_impl_id(void);
+long sbi_get_impl_version(void);
int sbi_probe_extension(int ext);
+long sbi_get_mvendorid(void);
+long sbi_get_marchid(void);
+long sbi_get_mimpid(void);
#endif
diff --git a/arch/riscv/lib/sbi.c b/arch/riscv/lib/sbi.c
index 77845a73ca..8b8b4b278f 100644
--- a/arch/riscv/lib/sbi.c
+++ b/arch/riscv/lib/sbi.c
@@ -89,6 +89,23 @@ int sbi_get_impl_id(void)
return -ENOTSUPP;
}
+/**
+ * sbi_get_impl_version() - get SBI implementation version
+ *
+ * Return: implementation version
+ */
+long sbi_get_impl_version(void)
+{
+ struct sbiret ret;
+
+ ret = sbi_ecall(SBI_EXT_BASE, SBI_EXT_BASE_GET_IMP_VERSION,
+ 0, 0, 0, 0, 0, 0);
+ if (!ret.error)
+ return ret.value;
+
+ return -ENOTSUPP;
+}
+
/**
* sbi_probe_extension() - Check if an SBI extension ID is supported or not.
* @extid: The extension ID to be probed.
@@ -108,6 +125,60 @@ int sbi_probe_extension(int extid)
return -ENOTSUPP;
}
+/**
+ * sbi_get_mvendorid() - get machine vendor ID
+ *
+ * Return: implementation vendor ID
+ */
+long sbi_get_mvendorid(void)
+{
+ struct sbiret ret;
+
+ ret = sbi_ecall(SBI_EXT_BASE, SBI_EXT_BASE_GET_MVENDORID,
+ 0, 0, 0, 0, 0, 0);
+ if (!ret.error)
+ if (ret.value)
+ return ret.value;
+
+ return -ENOTSUPP;
+}
+
+/**
+ * sbi_get_marchid() - get machine architecture ID
+ *
+ * Return: implementation version
+ */
+long sbi_get_marchid(void)
+{
+ struct sbiret ret;
+
+ ret = sbi_ecall(SBI_EXT_BASE, SBI_EXT_BASE_GET_MARCHID,
+ 0, 0, 0, 0, 0, 0);
+ if (!ret.error)
+ if (ret.value)
+ return ret.value;
+
+ return -ENOTSUPP;
+}
+
+/**
+ * sbi_get_mimpid() - get machine implementation ID
+ *
+ * Return: machine implementation ID
+ */
+long sbi_get_mimpid(void)
+{
+ struct sbiret ret;
+
+ ret = sbi_ecall(SBI_EXT_BASE, SBI_EXT_BASE_GET_MIMPID,
+ 0, 0, 0, 0, 0, 0);
+ if (!ret.error)
+ if (ret.value)
+ return ret.value;
+
+ return -ENOTSUPP;
+}
+
#ifdef CONFIG_SBI_V01
/**
--
2.30.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] cmd/sbi: add missing SBI information
2021-07-19 20:28 [PATCH 0/2] cmd/sbi: add missing SBI information Heinrich Schuchardt
2021-07-19 20:28 ` [PATCH 1/2] riscv: provide missing base extension functions Heinrich Schuchardt
@ 2021-07-19 20:28 ` Heinrich Schuchardt
2021-07-20 1:11 ` Sean Anderson
1 sibling, 1 reply; 9+ messages in thread
From: Heinrich Schuchardt @ 2021-07-19 20:28 UTC (permalink / raw)
To: Rick Chen
Cc: Leo, Bin Meng, Atish Patra, Pragnesh Patel, u-boot,
Dimitri John Ledkov, Heinrich Schuchardt, Heinrich Schuchardt
Let the sbi command display:
* SBI implementation version
* machine vendor ID
* machine architecture ID
* machine implementation ID
With this patch the output for the HiFive Unmatched looks like
=> sbi
SBI 0.3
OpenSBI 0.9
Machine:
Vendor ID 489
Architecture ID 8000000000000007
Implementation ID 20181004
Extensions:
sbi_set_timer
sbi_console_putchar
sbi_console_getchar
sbi_clear_ipi
sbi_send_ipi
sbi_remote_fence_i
sbi_remote_sfence_vma
sbi_remote_sfence_vma_asid
sbi_shutdown
SBI Base Functionality
Timer Extension
IPI Extension
RFENCE Extension
Hart State Management Extension
System Reset Extension
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
cmd/riscv/sbi.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/cmd/riscv/sbi.c b/cmd/riscv/sbi.c
index 90c0811e14..c0db763ba7 100644
--- a/cmd/riscv/sbi.c
+++ b/cmd/riscv/sbi.c
@@ -59,13 +59,30 @@ static int do_sbi(struct cmd_tbl *cmdtp, int flag, int argc,
if (ret >= 0) {
for (i = 0; i < ARRAY_SIZE(implementations); ++i) {
if (ret == implementations[i].id) {
- printf("%s\n", implementations[i].name);
+ printf("%s", implementations[i].name);
+ ret = sbi_get_impl_version();
+ if (ret > 0) {
+ /* OpenSBI specific version encoding */
+ printf(" %ld", ret >> 16);
+ printf(".%ld", ret & 0xffff);
+ }
+ printf("\n");
break;
}
}
if (i == ARRAY_SIZE(implementations))
printf("Unknown implementation ID %ld\n", ret);
}
+ printf("Machine:\n");
+ ret = sbi_get_mvendorid();
+ if (ret != -ENOTSUPP)
+ printf(" Vendor ID %lx\n", ret);
+ ret = sbi_get_marchid();
+ if (ret != -ENOTSUPP)
+ printf(" Architecture ID %lx\n", ret);
+ ret = sbi_get_mimpid();
+ if (ret != -ENOTSUPP)
+ printf(" Implementation ID %lx\n", ret);
printf("Extensions:\n");
for (i = 0; i < ARRAY_SIZE(extensions); ++i) {
ret = sbi_probe_extension(extensions[i].id);
--
2.30.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] riscv: provide missing base extension functions
2021-07-19 20:28 ` [PATCH 1/2] riscv: provide missing base extension functions Heinrich Schuchardt
@ 2021-07-20 1:07 ` Sean Anderson
2021-07-26 6:44 ` Leo Liang
1 sibling, 0 replies; 9+ messages in thread
From: Sean Anderson @ 2021-07-20 1:07 UTC (permalink / raw)
To: Heinrich Schuchardt, Rick Chen
Cc: Leo, Bin Meng, Atish Patra, Pragnesh Patel, u-boot,
Dimitri John Ledkov, Heinrich Schuchardt
On 7/19/21 4:28 PM, Heinrich Schuchardt wrote:
> Provide library functions to read:
>
> * SBI implementation version
> * machine vendor ID
> * machine architecture ID
> * machine implementation ID
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
> arch/riscv/include/asm/sbi.h | 4 ++
> arch/riscv/lib/sbi.c | 71 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 75 insertions(+)
>
> diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
> index 53ca316180..262cc9228c 100644
> --- a/arch/riscv/include/asm/sbi.h
> +++ b/arch/riscv/include/asm/sbi.h
> @@ -117,6 +117,10 @@ void sbi_remote_sfence_vma_asid(const unsigned long *hart_mask,
> void sbi_set_timer(uint64_t stime_value);
> long sbi_get_spec_version(void);
> int sbi_get_impl_id(void);
> +long sbi_get_impl_version(void);
> int sbi_probe_extension(int ext);
> +long sbi_get_mvendorid(void);
> +long sbi_get_marchid(void);
> +long sbi_get_mimpid(void);
>
> #endif
> diff --git a/arch/riscv/lib/sbi.c b/arch/riscv/lib/sbi.c
> index 77845a73ca..8b8b4b278f 100644
> --- a/arch/riscv/lib/sbi.c
> +++ b/arch/riscv/lib/sbi.c
> @@ -89,6 +89,23 @@ int sbi_get_impl_id(void)
> return -ENOTSUPP;
> }
>
> +/**
> + * sbi_get_impl_version() - get SBI implementation version
> + *
> + * Return: implementation version
> + */
> +long sbi_get_impl_version(void)
> +{
> + struct sbiret ret;
> +
> + ret = sbi_ecall(SBI_EXT_BASE, SBI_EXT_BASE_GET_IMP_VERSION,
> + 0, 0, 0, 0, 0, 0);
> + if (!ret.error)
> + return ret.value;
> +
> + return -ENOTSUPP;
> +}
> +
> /**
> * sbi_probe_extension() - Check if an SBI extension ID is supported or not.
> * @extid: The extension ID to be probed.
> @@ -108,6 +125,60 @@ int sbi_probe_extension(int extid)
> return -ENOTSUPP;
> }
>
> +/**
> + * sbi_get_mvendorid() - get machine vendor ID
> + *
> + * Return: implementation vendor ID
> + */
> +long sbi_get_mvendorid(void)
> +{
> + struct sbiret ret;
> +
> + ret = sbi_ecall(SBI_EXT_BASE, SBI_EXT_BASE_GET_MVENDORID,
> + 0, 0, 0, 0, 0, 0);
> + if (!ret.error)
> + if (ret.value)
> + return ret.value;
> +
> + return -ENOTSUPP;
> +}
> +
> +/**
> + * sbi_get_marchid() - get machine architecture ID
> + *
> + * Return: implementation version
> + */
> +long sbi_get_marchid(void)
> +{
> + struct sbiret ret;
> +
> + ret = sbi_ecall(SBI_EXT_BASE, SBI_EXT_BASE_GET_MARCHID,
> + 0, 0, 0, 0, 0, 0);
> + if (!ret.error)
> + if (ret.value)
> + return ret.value;
> +
> + return -ENOTSUPP;
> +}
> +
> +/**
> + * sbi_get_mimpid() - get machine implementation ID
> + *
> + * Return: machine implementation ID
> + */
> +long sbi_get_mimpid(void)
> +{
> + struct sbiret ret;
> +
> + ret = sbi_ecall(SBI_EXT_BASE, SBI_EXT_BASE_GET_MIMPID,
> + 0, 0, 0, 0, 0, 0);
> + if (!ret.error)
> + if (ret.value)
> + return ret.value;
> +
> + return -ENOTSUPP;
> +}
> +
> #ifdef CONFIG_SBI_V01
>
> /**
> --
> 2.30.2
>
Reviewed-by: Sean Anderson <seanga2@gmail.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] cmd/sbi: add missing SBI information
2021-07-19 20:28 ` [PATCH 2/2] cmd/sbi: add missing SBI information Heinrich Schuchardt
@ 2021-07-20 1:11 ` Sean Anderson
2021-07-20 4:59 ` Heinrich Schuchardt
0 siblings, 1 reply; 9+ messages in thread
From: Sean Anderson @ 2021-07-20 1:11 UTC (permalink / raw)
To: Heinrich Schuchardt, Rick Chen
Cc: Leo, Bin Meng, Atish Patra, Pragnesh Patel, u-boot,
Dimitri John Ledkov, Heinrich Schuchardt
On 7/19/21 4:28 PM, Heinrich Schuchardt wrote:
> Let the sbi command display:
>
> * SBI implementation version
> * machine vendor ID
> * machine architecture ID
> * machine implementation ID
>
> With this patch the output for the HiFive Unmatched looks like
>
> => sbi
> SBI 0.3
> OpenSBI 0.9
> Machine:
> Vendor ID 489
> Architecture ID 8000000000000007
> Implementation ID 20181004
> Extensions:
> sbi_set_timer
> sbi_console_putchar
> sbi_console_getchar
> sbi_clear_ipi
> sbi_send_ipi
> sbi_remote_fence_i
> sbi_remote_sfence_vma
> sbi_remote_sfence_vma_asid
> sbi_shutdown
> SBI Base Functionality
> Timer Extension
> IPI Extension
> RFENCE Extension
> Hart State Management Extension
> System Reset Extension
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
> cmd/riscv/sbi.c | 19 ++++++++++++++++++-
> 1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/cmd/riscv/sbi.c b/cmd/riscv/sbi.c
> index 90c0811e14..c0db763ba7 100644
> --- a/cmd/riscv/sbi.c
> +++ b/cmd/riscv/sbi.c
> @@ -59,13 +59,30 @@ static int do_sbi(struct cmd_tbl *cmdtp, int flag, int argc,
> if (ret >= 0) {
> for (i = 0; i < ARRAY_SIZE(implementations); ++i) {
> if (ret == implementations[i].id) {
> - printf("%s\n", implementations[i].name);
> + printf("%s", implementations[i].name);
> + ret = sbi_get_impl_version();
> + if (ret > 0) {
Shouldn't this have to check to ensure that i == 1?
> + /* OpenSBI specific version encoding */
> + printf(" %ld", ret >> 16);
> + printf(".%ld", ret & 0xffff);
> + }
> + printf("\n");
> break;
> }
> }
> if (i == ARRAY_SIZE(implementations))
> printf("Unknown implementation ID %ld\n", ret);
> }
> + printf("Machine:\n");
> + ret = sbi_get_mvendorid();
> + if (ret != -ENOTSUPP)
> + printf(" Vendor ID %lx\n", ret);
perhaps %0.8lx? And should we decode the JEDEC id?
> + ret = sbi_get_marchid();
> + if (ret != -ENOTSUPP)
> + printf(" Architecture ID %lx\n", ret);
> + ret = sbi_get_mimpid();
> + if (ret != -ENOTSUPP)
> + printf(" Implementation ID %lx\n", ret);
> printf("Extensions:\n");
> for (i = 0; i < ARRAY_SIZE(extensions); ++i) {
> ret = sbi_probe_extension(extensions[i].id);
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] cmd/sbi: add missing SBI information
2021-07-20 1:11 ` Sean Anderson
@ 2021-07-20 4:59 ` Heinrich Schuchardt
2021-07-20 5:13 ` Sean Anderson
0 siblings, 1 reply; 9+ messages in thread
From: Heinrich Schuchardt @ 2021-07-20 4:59 UTC (permalink / raw)
To: Sean Anderson, Rick Chen
Cc: Leo, Bin Meng, Atish Patra, Pragnesh Patel, u-boot,
Dimitri John Ledkov, Heinrich Schuchardt
Am 20. Juli 2021 03:11:34 MESZ schrieb Sean Anderson <seanga2@gmail.com>:
>On 7/19/21 4:28 PM, Heinrich Schuchardt wrote:
>> Let the sbi command display:
>>
>> * SBI implementation version
>> * machine vendor ID
>> * machine architecture ID
>> * machine implementation ID
>>
>> With this patch the output for the HiFive Unmatched looks like
>>
>> => sbi
>> SBI 0.3
>> OpenSBI 0.9
>> Machine:
>> Vendor ID 489
>> Architecture ID 8000000000000007
>> Implementation ID 20181004
>> Extensions:
>> sbi_set_timer
>> sbi_console_putchar
>> sbi_console_getchar
>> sbi_clear_ipi
>> sbi_send_ipi
>> sbi_remote_fence_i
>> sbi_remote_sfence_vma
>> sbi_remote_sfence_vma_asid
>> sbi_shutdown
>> SBI Base Functionality
>> Timer Extension
>> IPI Extension
>> RFENCE Extension
>> Hart State Management Extension
>> System Reset Extension
>>
>> Signed-off-by: Heinrich Schuchardt
><heinrich.schuchardt@canonical.com>
>> ---
>> cmd/riscv/sbi.c | 19 ++++++++++++++++++-
>> 1 file changed, 18 insertions(+), 1 deletion(-)
>>
>> diff --git a/cmd/riscv/sbi.c b/cmd/riscv/sbi.c
>> index 90c0811e14..c0db763ba7 100644
>> --- a/cmd/riscv/sbi.c
>> +++ b/cmd/riscv/sbi.c
>> @@ -59,13 +59,30 @@ static int do_sbi(struct cmd_tbl *cmdtp, int
>flag, int argc,
>> if (ret >= 0) {
>> for (i = 0; i < ARRAY_SIZE(implementations); ++i) {
>> if (ret == implementations[i].id) {
>> - printf("%s\n", implementations[i].name);
>> + printf("%s", implementations[i].name);
>> + ret = sbi_get_impl_version();
>> + if (ret > 0) {
>
>Shouldn't this have to check to ensure that i == 1?
Other SBI implementions may also provide a version number. I did not analyze how the should be formatted.
>
>> + /* OpenSBI specific version encoding */
>> + printf(" %ld", ret >> 16);
>> + printf(".%ld", ret & 0xffff);
>> + }
>> + printf("\n");
>> break;
>> }
>> }
>> if (i == ARRAY_SIZE(implementations))
>> printf("Unknown implementation ID %ld\n", ret);
>> }
>> + printf("Machine:\n");
>> + ret = sbi_get_mvendorid();
>> + if (ret != -ENOTSUPP)
Should we use an unsigned int as return value and 0 to indicate a missing implementation? mvendorid is only 32 bits wide.
>> + printf(" Vendor ID %lx\n", ret);
>
>perhaps %0.8lx? And should we decode the JEDEC id?
Leading zeros won't add any meaning here. Splitting into the 25 bit and 7 bit subfields could be reasonable.
Decoding could result in up to 2^26 digits. I don't want to wait for all of these on my serial console.
Best regards
Heinrich
>
>> + ret = sbi_get_marchid();
>> + if (ret != -ENOTSUPP)
>> + printf(" Architecture ID %lx\n", ret);
>> + ret = sbi_get_mimpid();
>> + if (ret != -ENOTSUPP)
>> + printf(" Implementation ID %lx\n", ret);
>> printf("Extensions:\n");
>> for (i = 0; i < ARRAY_SIZE(extensions); ++i) {
>> ret = sbi_probe_extension(extensions[i].id);
>> --
>> 2.30.2
>>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] cmd/sbi: add missing SBI information
2021-07-20 4:59 ` Heinrich Schuchardt
@ 2021-07-20 5:13 ` Sean Anderson
2021-07-26 7:08 ` Leo Liang
0 siblings, 1 reply; 9+ messages in thread
From: Sean Anderson @ 2021-07-20 5:13 UTC (permalink / raw)
To: Heinrich Schuchardt, Rick Chen
Cc: Leo, Bin Meng, Atish Patra, Pragnesh Patel, u-boot,
Dimitri John Ledkov, Heinrich Schuchardt
On 7/20/21 12:59 AM, Heinrich Schuchardt wrote:
> Am 20. Juli 2021 03:11:34 MESZ schrieb Sean Anderson <seanga2@gmail.com>:
>> On 7/19/21 4:28 PM, Heinrich Schuchardt wrote:
>>> Let the sbi command display:
>>>
>>> * SBI implementation version
>>> * machine vendor ID
>>> * machine architecture ID
>>> * machine implementation ID
>>>
>>> With this patch the output for the HiFive Unmatched looks like
>>>
>>> => sbi
>>> SBI 0.3
>>> OpenSBI 0.9
>>> Machine:
>>> Vendor ID 489
>>> Architecture ID 8000000000000007
>>> Implementation ID 20181004
>>> Extensions:
>>> sbi_set_timer
>>> sbi_console_putchar
>>> sbi_console_getchar
>>> sbi_clear_ipi
>>> sbi_send_ipi
>>> sbi_remote_fence_i
>>> sbi_remote_sfence_vma
>>> sbi_remote_sfence_vma_asid
>>> sbi_shutdown
>>> SBI Base Functionality
>>> Timer Extension
>>> IPI Extension
>>> RFENCE Extension
>>> Hart State Management Extension
>>> System Reset Extension
>>>
>>> Signed-off-by: Heinrich Schuchardt
>> <heinrich.schuchardt@canonical.com>
>>> ---
>>> cmd/riscv/sbi.c | 19 ++++++++++++++++++-
>>> 1 file changed, 18 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/cmd/riscv/sbi.c b/cmd/riscv/sbi.c
>>> index 90c0811e14..c0db763ba7 100644
>>> --- a/cmd/riscv/sbi.c
>>> +++ b/cmd/riscv/sbi.c
>>> @@ -59,13 +59,30 @@ static int do_sbi(struct cmd_tbl *cmdtp, int
>> flag, int argc,
>>> if (ret >= 0) {
>>> for (i = 0; i < ARRAY_SIZE(implementations); ++i) {
>>> if (ret == implementations[i].id) {
>>> - printf("%s\n", implementations[i].name);
>>> + printf("%s", implementations[i].name);
>>> + ret = sbi_get_impl_version();
>>> + if (ret > 0) {
>>
>> Shouldn't this have to check to ensure that i == 1?
>
> Other SBI implementions may also provide a version number. I did not analyze how the should be formatted.
Right, so shouldn't we default to the raw hex string?
>
>>
>>> + /* OpenSBI specific version encoding */
>>> + printf(" %ld", ret >> 16);
>>> + printf(".%ld", ret & 0xffff);
>>> + }
>>> + printf("\n");
>>> break;
>>> }
>>> }
>>> if (i == ARRAY_SIZE(implementations))
>>> printf("Unknown implementation ID %ld\n", ret);
>>> }
>>> + printf("Machine:\n");
>>> + ret = sbi_get_mvendorid();
>>> + if (ret != -ENOTSUPP)
>
> Should we use an unsigned int as return value and 0 to indicate a missing implementation? mvendorid is only 32 bits wide.
>
>>> + printf(" Vendor ID %lx\n", ret);
>>
>> perhaps %0.8lx? And should we decode the JEDEC id?
>
> Leading zeros won't add any meaning here. Splitting into the 25 bit and 7 bit subfields could be reasonable.
I think this would be a good option.
> Decoding could result in up to 2^26 digits. I don't want to wait for all of these on my serial console.
Well, they're only up to 12 continuation codes, so imposing a small maximum would likely not be too onerous.
--Sean
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] riscv: provide missing base extension functions
2021-07-19 20:28 ` [PATCH 1/2] riscv: provide missing base extension functions Heinrich Schuchardt
2021-07-20 1:07 ` Sean Anderson
@ 2021-07-26 6:44 ` Leo Liang
1 sibling, 0 replies; 9+ messages in thread
From: Leo Liang @ 2021-07-26 6:44 UTC (permalink / raw)
To: Heinrich Schuchardt; +Cc: u-boot
On Tue, Jul 20, 2021 at 04:28:34AM +0800, Heinrich Schuchardt wrote:
> Provide library functions to read:
>
> * SBI implementation version
> * machine vendor ID
> * machine architecture ID
> * machine implementation ID
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Reviewed-by: Leo Yu-Chi Liang <ycliang@andestech.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] cmd/sbi: add missing SBI information
2021-07-20 5:13 ` Sean Anderson
@ 2021-07-26 7:08 ` Leo Liang
0 siblings, 0 replies; 9+ messages in thread
From: Leo Liang @ 2021-07-26 7:08 UTC (permalink / raw)
To: Sean Anderson
Cc: Heinrich Schuchardt, Rick Jian-Zhi Chen(陳建志),
Bin Meng, Atish Patra, Pragnesh Patel, u-boot,
Dimitri John Ledkov, Heinrich Schuchardt
On Tue, Jul 20, 2021 at 01:13:56PM +0800, Sean Anderson wrote:
> On 7/20/21 12:59 AM, Heinrich Schuchardt wrote:
> > Am 20. Juli 2021 03:11:34 MESZ schrieb Sean Anderson <seanga2@gmail.com>:
> >> On 7/19/21 4:28 PM, Heinrich Schuchardt wrote:
> >>> Let the sbi command display:
> >>>
> >>> * SBI implementation version
> >>> * machine vendor ID
> >>> * machine architecture ID
> >>> * machine implementation ID
> >>>
> >>> With this patch the output for the HiFive Unmatched looks like
> >>>
> >>> => sbi
> >>> SBI 0.3
> >>> OpenSBI 0.9
> >>> Machine:
> >>> Vendor ID 489
> >>> Architecture ID 8000000000000007
> >>> Implementation ID 20181004
> >>> Extensions:
> >>> sbi_set_timer
> >>> sbi_console_putchar
> >>> sbi_console_getchar
> >>> sbi_clear_ipi
> >>> sbi_send_ipi
> >>> sbi_remote_fence_i
> >>> sbi_remote_sfence_vma
> >>> sbi_remote_sfence_vma_asid
> >>> sbi_shutdown
> >>> SBI Base Functionality
> >>> Timer Extension
> >>> IPI Extension
> >>> RFENCE Extension
> >>> Hart State Management Extension
> >>> System Reset Extension
> >>>
> >>> Signed-off-by: Heinrich Schuchardt
> >> <heinrich.schuchardt@canonical.com>
> >>> ---
> >>> cmd/riscv/sbi.c | 19 ++++++++++++++++++-
> >>> 1 file changed, 18 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/cmd/riscv/sbi.c b/cmd/riscv/sbi.c
> >>> index 90c0811e14..c0db763ba7 100644
> >>> --- a/cmd/riscv/sbi.c
> >>> +++ b/cmd/riscv/sbi.c
> >>> @@ -59,13 +59,30 @@ static int do_sbi(struct cmd_tbl *cmdtp, int
> >> flag, int argc,
> >>> if (ret >= 0) {
> >>> for (i = 0; i < ARRAY_SIZE(implementations); ++i) {
> >>> if (ret == implementations[i].id) {
> >>> - printf("%s\n", implementations[i].name);
> >>> + printf("%s", implementations[i].name);
> >>> + ret = sbi_get_impl_version();
> >>> + if (ret > 0) {
> >>
> >> Shouldn't this have to check to ensure that i == 1?
> >
> > Other SBI implementions may also provide a version number. I did not analyze how the should be formatted.
>
> Right, so shouldn't we default to the raw hex string?
+1
>
> >
> >>
> >>> + /* OpenSBI specific version encoding */
> >>> + printf(" %ld", ret >> 16);
> >>> + printf(".%ld", ret & 0xffff);
> >>> + }
> >>> + printf("\n");
> >>> break;
> >>> }
> >>> }
> >>> if (i == ARRAY_SIZE(implementations))
> >>> printf("Unknown implementation ID %ld\n", ret);
> >>> }
> >>> + printf("Machine:\n");
> >>> + ret = sbi_get_mvendorid();
> >>> + if (ret != -ENOTSUPP)
> >
> > Should we use an unsigned int as return value and 0 to indicate a missing implementation? mvendorid is only 32 bits wide.
> >
> >>> + printf(" Vendor ID %lx\n", ret);
> >>
> >> perhaps %0.8lx? And should we decode the JEDEC id?
> >
> > Leading zeros won't add any meaning here. Splitting into the 25 bit and 7 bit subfields could be reasonable.
>
> I think this would be a good option.
+1 with 25, 7 split.
IMHO, perhaps we could show only the "numbers" of the continuation code and the "decoded offset" ?
Something like: vendor ID: 489, continuation code: 9, offset decoded: 0x89.
Best regards,
Leo
>
> > Decoding could result in up to 2^26 digits. I don't want to wait for all of these on my serial console.
>
> Well, they're only up to 12 continuation codes, so imposing a small maximum would likely not be too onerous.
>
> --Sean
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-07-26 7:09 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-19 20:28 [PATCH 0/2] cmd/sbi: add missing SBI information Heinrich Schuchardt
2021-07-19 20:28 ` [PATCH 1/2] riscv: provide missing base extension functions Heinrich Schuchardt
2021-07-20 1:07 ` Sean Anderson
2021-07-26 6:44 ` Leo Liang
2021-07-19 20:28 ` [PATCH 2/2] cmd/sbi: add missing SBI information Heinrich Schuchardt
2021-07-20 1:11 ` Sean Anderson
2021-07-20 4:59 ` Heinrich Schuchardt
2021-07-20 5:13 ` Sean Anderson
2021-07-26 7:08 ` Leo Liang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.