Changeset View
Standalone View
sys/riscv/riscv/sbi.c
Show First 20 Lines • Show All 43 Lines • ▼ Show 20 Lines | |||||||||
u_long sbi_spec_version; | u_long sbi_spec_version; | ||||||||
u_long sbi_impl_id; | u_long sbi_impl_id; | ||||||||
u_long sbi_impl_version; | u_long sbi_impl_version; | ||||||||
static bool has_time_extension = false; | static bool has_time_extension = false; | ||||||||
static bool has_ipi_extension = false; | static bool has_ipi_extension = false; | ||||||||
static bool has_rfnc_extension = false; | static bool has_rfnc_extension = false; | ||||||||
static bool has_srst_extension = false; | |||||||||
static struct sbi_ret | static struct sbi_ret | ||||||||
sbi_get_spec_version(void) | sbi_get_spec_version(void) | ||||||||
{ | { | ||||||||
return (SBI_CALL0(SBI_EXT_ID_BASE, SBI_BASE_GET_SPEC_VERSION)); | return (SBI_CALL0(SBI_EXT_ID_BASE, SBI_BASE_GET_SPEC_VERSION)); | ||||||||
} | } | ||||||||
static struct sbi_ret | static struct sbi_ret | ||||||||
Show All 25 Lines | |||||||||
{ | { | ||||||||
return (SBI_CALL0(SBI_EXT_ID_BASE, SBI_BASE_GET_MIMPID)); | return (SBI_CALL0(SBI_EXT_ID_BASE, SBI_BASE_GET_MIMPID)); | ||||||||
} | } | ||||||||
static void | static void | ||||||||
sbi_shutdown_final(void *dummy __unused, int howto) | sbi_shutdown_final(void *dummy __unused, int howto) | ||||||||
{ | { | ||||||||
if ((howto & RB_POWEROFF) != 0) | if ((howto & RB_POWEROFF) != 0) | ||||||||
sbi_shutdown(); | sbi_system_reset(SBI_SRST_TYPE_SHUTDOWN, SBI_SRST_REASON_NONE); | ||||||||
} | } | ||||||||
void | void | ||||||||
sbi_system_reset(u_long reset_type, u_long reset_reason) | |||||||||
{ | |||||||||
/* Use the SRST extension, if available. */ | |||||||||
if (has_srst_extension) { | |||||||||
(void)SBI_CALL2(SBI_EXT_ID_SRST, SBI_SRST_SYSTEM_RESET, | |||||||||
jrtc27: This line looks too long, if I can count correctly | |||||||||
reset_type, reset_reason); | |||||||||
} | |||||||||
(void)SBI_CALL0(SBI_SHUTDOWN, 0); | |||||||||
Not Done Inline ActionsAs a follow-up change, this function could be extended to handle other reset types. RB_POWERCYCLE implies a warm reboot, and RB_DUMP implies a system failure (panic). The lack of these three flags indicates a regular reboot. mhorne: As a follow-up change, this function could be extended to handle other reset types. | |||||||||
Not Done Inline ActionsGenerally drivers just check RB_POWEROFF and RB_HALT. RB_POWERCYCLE is a power cycle, which if anything would be a cold reboot, but is only used by the IPMI driver at the moment. So normally you want: if ((howto & RB_POWEROFF) != 0) { halt } else if ((howto & RB_HALT) == 0) { reboot } (see the ACPI driver) jrtc27: Generally drivers just check RB_POWEROFF and RB_HALT. RB_POWERCYCLE is a power cycle, which if… | |||||||||
Not Done Inline ActionsHmmm, agreed. My comment about RB_POWERCYCLE is incorrect. It remains to be seen how useful SBI_SRST_REASON_SYSTEM_FAILURE is. mhorne: Hmmm, agreed. My comment about `RB_POWERCYCLE` is incorrect. It remains to be seen how useful… | |||||||||
Not Done Inline ActionsIt's not unheard of for embedded system to care. A reasonably likely use case for something like this is to inform the boot loader (or on RISC-V M-mode) that the current image failed to start correctly so it can try another one. kp: It's not unheard of for embedded system to care. A reasonably likely use case for something… | |||||||||
} | |||||||||
Done Inline Actions
mhorne: | |||||||||
Done Inline Actionsline removed. danq1222_gmail.com: >
line removed. | |||||||||
Done Inline ActionsIt may be worth falling back to SBI_SHUTDOWN if this fails. Happily that should only require moving that call out of the 'else' clause. kp: It may be worth falling back to SBI_SHUTDOWN if this fails.
Happily that should only require… | |||||||||
Done Inline Actions
Like the below? if (ret.error != SBI_SUCCESS) { printf("SBI System Reset Type Error %lu\n", reset_type); (void)SBI_CALL0(SBI_SHUTDOWN, 0); } danq1222_gmail.com: > It may be worth falling back to SBI_SHUTDOWN if this fails.
>
> Happily that should only… | |||||||||
Done Inline ActionsI'd just do: if (has_srst_extension) { (void)SBI_CALL2(SBI_EXT_ID_SRST, SBI_SRST_SYSTEM_RESET, reset_type, reset_reason); } (void)SBI_CALL0(SBI_SHUTDOWN, 0); I would expect SBI_SRST_SYSTEM_RESET to not return, so if it does we can try SBI_SHUTDOWN instead. kp: I'd just do:
```
if (has_srst_extension) {
(void)SBI_CALL2(SBI_EXT_ID_SRST… | |||||||||
Done Inline Actions
I see, thanks! danq1222_gmail.com: > I'd just do:
>
>
> ```
> if (has_srst_extension) {
> (void)SBI_CALL2(SBI_EXT_ID_SRST… | |||||||||
Done Inline ActionsNeeds a newline, and should probably print the error code jrtc27: Needs a newline, and should probably print the error code | |||||||||
Not Done Inline Actions
I am thinking of the reset_type as error code. Could this be OK? printf("SBI System Reset Type Error %lu\n", reset_type); danq1222_gmail.com: > Needs a newline, and should probably print the error code
I am thinking of the reset_type as… | |||||||||
Done Inline Actions
Line removed. danq1222_gmail.com: > > Needs a newline, and should probably print the error code
>
> I am thinking of the… | |||||||||
Done Inline Actions
Line removed. danq1222_gmail.com: > Needs a newline, and should probably print the error code
Line removed. | |||||||||
void | |||||||||
sbi_print_version(void) | sbi_print_version(void) | ||||||||
{ | { | ||||||||
u_int major; | u_int major; | ||||||||
u_int minor; | u_int minor; | ||||||||
/* For legacy SBI implementations. */ | /* For legacy SBI implementations. */ | ||||||||
if (sbi_spec_version == 0) { | if (sbi_spec_version == 0) { | ||||||||
printf("SBI: Unknown (Legacy) Implementation\n"); | printf("SBI: Unknown (Legacy) Implementation\n"); | ||||||||
▲ Show 20 Lines • Show All 167 Lines • ▼ Show 20 Lines | sbi_init(void) | ||||||||
/* Probe for legacy replacement extensions. */ | /* Probe for legacy replacement extensions. */ | ||||||||
if (sbi_probe_extension(SBI_EXT_ID_TIME) != 0) | if (sbi_probe_extension(SBI_EXT_ID_TIME) != 0) | ||||||||
has_time_extension = true; | has_time_extension = true; | ||||||||
if (sbi_probe_extension(SBI_EXT_ID_IPI) != 0) | if (sbi_probe_extension(SBI_EXT_ID_IPI) != 0) | ||||||||
has_ipi_extension = true; | has_ipi_extension = true; | ||||||||
if (sbi_probe_extension(SBI_EXT_ID_RFNC) != 0) | if (sbi_probe_extension(SBI_EXT_ID_RFNC) != 0) | ||||||||
has_rfnc_extension = true; | has_rfnc_extension = true; | ||||||||
if (sbi_probe_extension(SBI_EXT_ID_SRST) != 0) | |||||||||
has_srst_extension = true; | |||||||||
/* | /* | ||||||||
* Probe for legacy extensions. We still rely on many of them to be | * Probe for legacy extensions. We still rely on many of them to be | ||||||||
* implemented, but this is not guaranteed by the spec. | * implemented, but this is not guaranteed by the spec. | ||||||||
*/ | */ | ||||||||
KASSERT(has_time_extension || sbi_probe_extension(SBI_SET_TIMER) != 0, | KASSERT(has_time_extension || sbi_probe_extension(SBI_SET_TIMER) != 0, | ||||||||
("SBI doesn't implement sbi_set_timer()")); | ("SBI doesn't implement sbi_set_timer()")); | ||||||||
KASSERT(sbi_probe_extension(SBI_CONSOLE_PUTCHAR) != 0, | KASSERT(sbi_probe_extension(SBI_CONSOLE_PUTCHAR) != 0, | ||||||||
("SBI doesn't implement sbi_console_putchar()")); | ("SBI doesn't implement sbi_console_putchar()")); | ||||||||
KASSERT(sbi_probe_extension(SBI_CONSOLE_GETCHAR) != 0, | KASSERT(sbi_probe_extension(SBI_CONSOLE_GETCHAR) != 0, | ||||||||
("SBI doesn't implement sbi_console_getchar()")); | ("SBI doesn't implement sbi_console_getchar()")); | ||||||||
KASSERT(has_ipi_extension || sbi_probe_extension(SBI_SEND_IPI) != 0, | KASSERT(has_ipi_extension || sbi_probe_extension(SBI_SEND_IPI) != 0, | ||||||||
("SBI doesn't implement sbi_send_ipi()")); | ("SBI doesn't implement sbi_send_ipi()")); | ||||||||
KASSERT(has_rfnc_extension || | KASSERT(has_rfnc_extension || | ||||||||
sbi_probe_extension(SBI_REMOTE_FENCE_I) != 0, | sbi_probe_extension(SBI_REMOTE_FENCE_I) != 0, | ||||||||
("SBI doesn't implement sbi_remote_fence_i()")); | ("SBI doesn't implement sbi_remote_fence_i()")); | ||||||||
KASSERT(has_rfnc_extension || | KASSERT(has_rfnc_extension || | ||||||||
sbi_probe_extension(SBI_REMOTE_SFENCE_VMA) != 0, | sbi_probe_extension(SBI_REMOTE_SFENCE_VMA) != 0, | ||||||||
("SBI doesn't implement sbi_remote_sfence_vma()")); | ("SBI doesn't implement sbi_remote_sfence_vma()")); | ||||||||
KASSERT(has_rfnc_extension || | KASSERT(has_rfnc_extension || | ||||||||
sbi_probe_extension(SBI_REMOTE_SFENCE_VMA_ASID) != 0, | sbi_probe_extension(SBI_REMOTE_SFENCE_VMA_ASID) != 0, | ||||||||
("SBI doesn't implement sbi_remote_sfence_vma_asid()")); | ("SBI doesn't implement sbi_remote_sfence_vma_asid()")); | ||||||||
KASSERT(sbi_probe_extension(SBI_SHUTDOWN) != 0, | KASSERT(has_srst_extension || sbi_probe_extension(SBI_SHUTDOWN) != 0, | ||||||||
("SBI doesn't implement sbi_shutdown()")); | ("SBI doesn't implement a shutdown or reset extension")); | ||||||||
Not Done Inline ActionsThis is a bit ambiguous jrtc27: This is a bit ambiguous | |||||||||
} | } | ||||||||
static void | static void | ||||||||
sbi_late_init(void *dummy __unused) | sbi_late_init(void *dummy __unused) | ||||||||
{ | { | ||||||||
EVENTHANDLER_REGISTER(shutdown_final, sbi_shutdown_final, NULL, | EVENTHANDLER_REGISTER(shutdown_final, sbi_shutdown_final, NULL, | ||||||||
SHUTDOWN_PRI_LAST); | SHUTDOWN_PRI_LAST); | ||||||||
} | } | ||||||||
SYSINIT(sbi, SI_SUB_KLD, SI_ORDER_ANY, sbi_late_init, NULL); | SYSINIT(sbi, SI_SUB_KLD, SI_ORDER_ANY, sbi_late_init, NULL); |
This line looks too long, if I can count correctly