[PATCH] lib: sbi: sbi_ecall: Improve return SBI error

[Xiang W]

在 2023-02-21星期二的 18:47 +0800，Yu Chien Peter Lin写道：
> We should also check if the return error code is greater than 0
> (SBI_SUCCESS), as this is an invalid error.
> 
> Also rename the variable returned via a0 to 'out_err' and use
> SBI_ERR_NOT_SUPPORTED when the extension does not have a handler
> for consistency with the standard SBI errors defined in Table 1
> of the SBI specification.
> 
> Signed-off-by: Yu Chien Peter Lin <peterlin at andestech.com>
> ---
>  lib/sbi/sbi_ecall.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/sbi/sbi_ecall.c b/lib/sbi/sbi_ecall.c
> index 27ce5d49..8accf675 100644
> --- a/lib/sbi/sbi_ecall.c
> +++ b/lib/sbi/sbi_ecall.c
> @@ -97,34 +97,34 @@ void sbi_ecall_unregister_extension(struct sbi_ecall_extension *ext)
>  
>  int sbi_ecall_handler(struct sbi_trap_regs *regs)
>  {
> -       int ret = 0;
> +       int out_err = 0;
There is no need to rename ret to out_err because a0 is not always an
error code in legacy extensions

Other than the above, it looks good to me.

Reviewed-by: Xiang W <wxjstz at 126.com>
> +       unsigned long out_val = 0;
>         struct sbi_ecall_extension *ext;
>         unsigned long extension_id = regs->a7;
>         unsigned long func_id = regs->a6;
>         struct sbi_trap_info trap = {0};
> -       unsigned long out_val = 0;
>         bool is_0_1_spec = 0;
>  
>         ext = sbi_ecall_find_extension(extension_id);
>         if (ext && ext->handle) {
> -               ret = ext->handle(extension_id, func_id,
> +               out_err = ext->handle(extension_id, func_id,
>                                   regs, &out_val, &trap);
>                 if (extension_id >= SBI_EXT_0_1_SET_TIMER &&
>                     extension_id <= SBI_EXT_0_1_SHUTDOWN)
>                         is_0_1_spec = 1;
>         } else {
> -               ret = SBI_ENOTSUPP;
> +               out_err = SBI_ERR_NOT_SUPPORTED;
>         }
>  
> -       if (ret == SBI_ETRAP) {
> +       if (out_err == SBI_ETRAP) {
>                 trap.epc = regs->mepc;
>                 sbi_trap_redirect(regs, &trap);
>         } else {
> -               if (ret < SBI_LAST_ERR) {
> +               if (out_err < SBI_LAST_ERR || SBI_SUCCESS < out_err) {
>                         sbi_printf("%s: Invalid error %d for ext=0x%lx "
> -                                  "func=0x%lx\n", __func__, ret,
> +                                  "func=0x%lx\n", __func__, out_err,
>                                    extension_id, func_id);
> -                       ret = SBI_ERR_FAILED;
> +                       out_err = SBI_ERR_FAILED;
>                 }
>  
>                 /*
> @@ -136,7 +136,7 @@ int sbi_ecall_handler(struct sbi_trap_regs *regs)
>                  * case should be handled differently.
>                  */
>                 regs->mepc += 4;
> -               regs->a0 = ret;
> +               regs->a0 = out_err;
>                 if (!is_0_1_spec)
>                         regs->a1 = out_val;
>         }
> -- 
> 2.34.1
> 
>