[PATCH] lib: sbi: sbi_system_reset() if not supported

[Heinrich Schuchardt]

On 10/18/21 06:23, Anup Patel wrote:
> On Fri, Oct 15, 2021 at 7:49 PM Heinrich Schuchardt
> <heinrich.schuchardt at canonical.com> wrote:
>>
>> The SBI specification requires that if a reset type or reason is not
>> supported the system reset function returns SBI_ERR_NOT_SUPPORTED.
>>
>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
> 
> This patch is not required because the sbi_ecall_srst_handler() will only
> call sbi_system_reset() if sbi_system_reset_supported() returns true.

sbi_system_reset() has a check dom->system_reset_allowed() which is not 
checked in sbi_ecall_srst_handler(). Code with checks with different 
scopce at multiple call levels is not very transparent. Let's have one 
place where all the checks are done. Instead of dropping this patch we 
can drop the sbi_system_reset_supported() call in sbi_ecall_srst_handler().

Best regards

Heinrich

> 
> Regards,
> Anup
> 
>> ---
>> Yet another patch will be needed to return SBI_ERR_FAILED if the reset
>> did not succeed.
>> ---
>>   include/sbi/sbi_system.h |  2 +-
>>   lib/sbi/sbi_system.c     | 22 ++++++++++++++--------
>>   2 files changed, 15 insertions(+), 9 deletions(-)
>>
>> diff --git a/include/sbi/sbi_system.h b/include/sbi/sbi_system.h
>> index 84c2813..3c98133 100644
>> --- a/include/sbi/sbi_system.h
>> +++ b/include/sbi/sbi_system.h
>> @@ -41,6 +41,6 @@ void sbi_system_reset_add_device(struct sbi_system_reset_device *dev);
>>
>>   bool sbi_system_reset_supported(u32 reset_type, u32 reset_reason);
>>
>> -void __noreturn sbi_system_reset(u32 reset_type, u32 reset_reason);
>> +void sbi_system_reset(u32 reset_type, u32 reset_reason);
>>
>>   #endif
>> diff --git a/lib/sbi/sbi_system.c b/lib/sbi/sbi_system.c
>> index 9cea3c0..5c328fb 100644
>> --- a/lib/sbi/sbi_system.c
>> +++ b/lib/sbi/sbi_system.c
>> @@ -62,12 +62,23 @@ bool sbi_system_reset_supported(u32 reset_type, u32 reset_reason)
>>          return !!sbi_system_reset_get_device(reset_type, reset_reason);
>>   }
>>
>> -void __noreturn sbi_system_reset(u32 reset_type, u32 reset_reason)
>> +void sbi_system_reset(u32 reset_type, u32 reset_reason)
>>   {
>>          ulong hbase = 0, hmask;
>>          u32 cur_hartid = current_hartid();
>>          struct sbi_domain *dom = sbi_domain_thishart_ptr();
>>          struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
>> +       const struct sbi_system_reset_device *dev =
>> +               sbi_system_reset_get_device(reset_type, reset_reason);
>> +
>> +       /* Check if domain allows system reset */
>> +       if (!dom->system_reset_allowed)
>> +               return;
>> +
>> +       /* Check if we have a reset device supporting the reset type */
>> +       dev = sbi_system_reset_get_device(reset_type, reset_reason);
>> +       if (!dev)
>> +               return;
>>
>>          /* Send HALT IPI to every hart other than the current hart */
>>          while (!sbi_hsm_hart_interruptible_mask(dom, hbase, &hmask)) {
>> @@ -81,13 +92,8 @@ void __noreturn sbi_system_reset(u32 reset_type, u32 reset_reason)
>>          /* Stop current HART */
>>          sbi_hsm_hart_stop(scratch, FALSE);
>>
>> -       /* Platform specific reset if domain allowed system reset */
>> -       if (dom->system_reset_allowed) {
>> -               const struct sbi_system_reset_device *dev =
>> -                       sbi_system_reset_get_device(reset_type, reset_reason);
>> -               if (dev)
>> -                       dev->system_reset(reset_type, reset_reason);
>> -       }
>> +       /* Execute platform specific reset */
>> +       dev->system_reset(reset_type, reset_reason);
>>
>>          /* If platform specific reset did not work then do sbi_exit() */
>>          sbi_exit(scratch);
>> --
>> 2.32.0
>>
>>
>> --
>> opensbi mailing list
>> opensbi at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/opensbi