[PATCH 1/4] lib: sbi_scratch: Apply bounds check to platform hart_count

[Rahul Pathak]

On Fri, Feb 21, 2025 at 12:13 AM Samuel Holland
<samuel.holland at sifive.com> wrote:
>
> The internal limit on the number of harts is SBI_HARTMASK_MAX_BITS, as
> this value determines the size of various bitmaps and arrays (including
> hartindex_to_hartid_table and hartindex_to_scratch_table). Clamp the
> value provided by the platform, and drop the extra array element.
>
> Update the documentation to indicate that hart_index2id must be sized
> based on hart_count, and that hart indexes must be contiguous. As of
> commit 5e90e54a1a53 ("lib: utils:Check that hartid is valid"), there is
> no restriction on the valid hart ID values.
>
> Signed-off-by: Samuel Holland <samuel.holland at sifive.com>
> ---
>
>  include/sbi/sbi_platform.h | 11 +++--------
>  lib/sbi/sbi_scratch.c      | 14 +++++++++-----
>  2 files changed, 12 insertions(+), 13 deletions(-)
>
> diff --git a/include/sbi/sbi_platform.h b/include/sbi/sbi_platform.h
> index 6d5fbc7d..5e50c997 100644
> --- a/include/sbi/sbi_platform.h
> +++ b/include/sbi/sbi_platform.h
> @@ -169,7 +169,7 @@ struct sbi_platform {
>         char name[64];
>         /** Supported features */
>         u64 features;
> -       /** Total number of HARTs */
> +       /** Total number of HARTs (at most SBI_HARTMASK_MAX_BITS) */
>         u32 hart_count;
>         /** Per-HART stack size for exception/interrupt handling */
>         u32 hart_stack_size;
> @@ -184,17 +184,12 @@ struct sbi_platform {
>         /**
>          * HART index to HART id table
>          *
> -        * For used HART index <abc>:
> +        * If hart_index2id != NULL then the table must contain a mapping
> +        * for each HART index 0 <= <abc> < hart_count:
>          *     hart_index2id[<abc>] = some HART id
> -        * For unused HART index <abc>:
> -        *     hart_index2id[<abc>] = -1U
>          *
>          * If hart_index2id == NULL then we assume identity mapping
>          *     hart_index2id[<abc>] = <abc>
> -        *
> -        * We have only two restrictions:
> -        * 1. HART index < sbi_platform hart_count
> -        * 2. HART id < SBI_HARTMASK_MAX_BITS
>          */
>         const u32 *hart_index2id;
>  };
> diff --git a/lib/sbi/sbi_scratch.c b/lib/sbi/sbi_scratch.c
> index ccbbc68e..5a9e935b 100644
> --- a/lib/sbi/sbi_scratch.c
> +++ b/lib/sbi/sbi_scratch.c
> @@ -15,8 +15,8 @@
>  #include <sbi/sbi_string.h>
>
>  u32 last_hartindex_having_scratch = 0;
> -u32 hartindex_to_hartid_table[SBI_HARTMASK_MAX_BITS + 1] = { -1U };
> -struct sbi_scratch *hartindex_to_scratch_table[SBI_HARTMASK_MAX_BITS + 1] = { 0 };
> +u32 hartindex_to_hartid_table[SBI_HARTMASK_MAX_BITS] = { -1U };
> +struct sbi_scratch *hartindex_to_scratch_table[SBI_HARTMASK_MAX_BITS] = { 0 };
>
>  static spinlock_t extra_lock = SPIN_LOCK_INITIALIZER;
>  static unsigned long extra_offset = SBI_SCRATCH_EXTRA_SPACE_OFFSET;
> @@ -36,17 +36,21 @@ typedef struct sbi_scratch *(*hartid2scratch)(ulong hartid, ulong hartindex);
>
>  int sbi_scratch_init(struct sbi_scratch *scratch)
>  {
> -       u32 i, h;
> +       u32 i, h, hart_count;
>         const struct sbi_platform *plat = sbi_platform_ptr(scratch);
>
> -       for (i = 0; i < plat->hart_count; i++) {
> +       hart_count = plat->hart_count;
> +       if (hart_count > SBI_HARTMASK_MAX_BITS)
> +               hart_count = SBI_HARTMASK_MAX_BITS;
>
>From a platform perspective case where hart_count > SBI_HARTMASK_MAX_BITS
Since its a limitation of SBI to not be able to represent all what
platform expects. Is it a
point of failure and it should not continue from here or it should
silently move forward with
what SBI can accommodate?


>
> +       for (i = 0; i < hart_count; i++) {
>                 h = (plat->hart_index2id) ? plat->hart_index2id[i] : i;
>                 hartindex_to_hartid_table[i] = h;
>                 hartindex_to_scratch_table[i] =
>                         ((hartid2scratch)scratch->hartid_to_scratch)(h, i);
>         }
>
> -       last_hartindex_having_scratch = plat->hart_count - 1;
> +       last_hartindex_having_scratch = hart_count - 1;
>
>         return 0;
>  }
> --
> 2.47.2
>
>
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi