| Message ID | 20210719202835.23081-3-xypron.glpk@gmx.de |
|---|---|
| State | Superseded, archived |
| Delegated to: | Andes |
| Headers | show |
| Series | cmd/sbi: add missing SBI information | expand |
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 >
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 >>
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
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
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);
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(-) -- 2.30.2