[PATCH 1/3] lib: sbi_ipi: Do not ignore errors from sbi_ipi_send()

[Anup Patel]

On Sat, Dec 9, 2023 at 2:53 AM Samuel Holland <samuel.holland at sifive.com> wrote:
>
> Currently, failures in sbi_ipi_send() are silently ignored, which makes
> them difficult to debug. Instead, abort sending the IPI and pass back
> the error, but still synchronize any IPIs already sent.
>
> This requires changing the value of SBI_IPI_UPDATE_BREAK to 0, since
> sbi_ipi_send() returning that value should not be treated as an error.
>
> Signed-off-by: Samuel Holland <samuel.holland at sifive.com>
> ---
>
>  include/sbi/sbi_ipi.h | 2 +-
>  lib/sbi/sbi_ipi.c     | 6 ++++--
>  2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/include/sbi/sbi_ipi.h b/include/sbi/sbi_ipi.h
> index d3962334..e81d9352 100644
> --- a/include/sbi/sbi_ipi.h
> +++ b/include/sbi/sbi_ipi.h
> @@ -31,9 +31,9 @@ struct sbi_ipi_device {
>  };
>
>  enum sbi_ipi_update_type {
> -       SBI_IPI_UPDATE_SUCCESS,
>         SBI_IPI_UPDATE_BREAK,
>         SBI_IPI_UPDATE_RETRY,
> +       SBI_IPI_UPDATE_SUCCESS,
>  };

I don't think we need to change the enum order here.
Please see the comment below.

>
>  struct sbi_scratch;
> diff --git a/lib/sbi/sbi_ipi.c b/lib/sbi/sbi_ipi.c
> index 5c33a78b..1fd24228 100644
> --- a/lib/sbi/sbi_ipi.c
> +++ b/lib/sbi/sbi_ipi.c
> @@ -101,7 +101,7 @@ static int sbi_ipi_sync(struct sbi_scratch *scratch, u32 event)
>   */
>  int sbi_ipi_send_many(ulong hmask, ulong hbase, u32 event, void *data)
>  {
> -       int rc;
> +       int rc = 0;
>         bool retry_needed;
>         ulong i, m;
>         struct sbi_hartmask target_mask = {0};
> @@ -137,6 +137,8 @@ int sbi_ipi_send_many(ulong hmask, ulong hbase, u32 event, void *data)
>                         rc = sbi_ipi_send(scratch, i, event, data);
>                         if (rc == SBI_IPI_UPDATE_RETRY)
>                                 retry_needed = true;
> +                       else if (rc)

This needs to be "rc < 0" otherwise SBI_IPI_UPDATE_SUCCESS will also
cause a break here.

> +                               break;
>                         else
>                                 sbi_hartmask_clear_hartindex(i, &target_mask);

We have to ensure that sbi_hartmask_clear_hartindex() is done for both
SBI_IPI_UPDATE_SUCCESS and SBI_IPI_UPDATE_BREAK;

>                 }
> @@ -145,7 +147,7 @@ int sbi_ipi_send_many(ulong hmask, ulong hbase, u32 event, void *data)
>         /* Sync IPIs */
>         sbi_ipi_sync(scratch, event);
>
> -       return 0;
> +       return rc;
>  }
>
>  int sbi_ipi_event_create(const struct sbi_ipi_event_ops *ops)
> --
> 2.42.0
>
>
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi

Regards,
Anup