[PATCH v6 4/7] lib: sbi: give platform choice of using single memregion to cover OpenSBI

[Anup Patel]

On Thu, Dec 18, 2025 at 4:14 PM Bo Gan <ganboing at gmail.com> wrote:
>
> By default the OpenSBI itself is covered by 2 memregions for RX/RW
> sections. This is required by platforms with Smepmp to enforce
> proper permissions in M mode. Note: M-mode only regions can't
> have RWX permissions with Smepmp. Platforms with traditional PMPs
> won't be able to benefit from it, as both regions are effectively
> RWX in M mode, but usually it's harmless to so. Now we provide
> these platforms with an option to disable this logic. It saves 1
> PMP entry. For platforms really in short of PMPs, it does make a
> difference.
>
> Note: Platform requesting single OpenSBI memregion must be using
>       traditional (old) PMP. We expect the platform code to do
>       the right thing.
>
> Signed-off-by: Bo Gan <ganboing at gmail.com>

LGTM.

Reviewed-by: Anup Patel <anup at brainfault.org>

Regards,
Anup

> ---
>  include/sbi/sbi_platform.h | 21 +++++++++++++++++++++
>  lib/sbi/sbi_domain.c       | 36 ++++++++++++++++++++++++------------
>  2 files changed, 45 insertions(+), 12 deletions(-)
>
> diff --git a/include/sbi/sbi_platform.h b/include/sbi/sbi_platform.h
> index d75c12de..e65d9877 100644
> --- a/include/sbi/sbi_platform.h
> +++ b/include/sbi/sbi_platform.h
> @@ -76,6 +76,9 @@ struct sbi_platform_operations {
>         /* Check if specified HART is allowed to do cold boot */
>         bool (*cold_boot_allowed)(u32 hartid);
>
> +       /* Check if platform requires single firmware region */
> +       bool (*single_fw_region)(void);
> +
>         /* Platform nascent initialization */
>         int (*nascent_init)(void);
>
> @@ -347,6 +350,24 @@ static inline bool sbi_platform_cold_boot_allowed(
>         return true;
>  }
>
> +/**
> + * Check whether platform requires single firmware region
> + *
> + * Note: Single firmware region only works with legacy PMP because with
> + * Smepmp M-mode only regions can't have RWX permissions.
> + *
> + * @param plat pointer to struct sbi_platform
> + *
> + * @return true if single firmware region required and false otherwise
> + */
> +static inline bool sbi_platform_single_fw_region(
> +                                       const struct sbi_platform *plat)
> +{
> +       if (plat && sbi_platform_ops(plat)->single_fw_region)
> +               return sbi_platform_ops(plat)->single_fw_region();
> +       return false;
> +}
> +
>  /**
>   * Nascent (very early) initialization for current HART
>   *
> diff --git a/lib/sbi/sbi_domain.c b/lib/sbi/sbi_domain.c
> index a19bf25b..90cbb540 100644
> --- a/lib/sbi/sbi_domain.c
> +++ b/lib/sbi/sbi_domain.c
> @@ -923,18 +923,30 @@ int sbi_domain_init(struct sbi_scratch *scratch, u32 cold_hartid)
>         root.possible_harts = root_hmask;
>
>         /* Root domain firmware memory region */
> -       sbi_domain_memregion_init(scratch->fw_start, scratch->fw_rw_offset,
> -                                 (SBI_DOMAIN_MEMREGION_M_READABLE |
> -                                  SBI_DOMAIN_MEMREGION_M_EXECUTABLE |
> -                                  SBI_DOMAIN_MEMREGION_FW),
> -                                 &root_memregs[root_memregs_count++]);
> -
> -       sbi_domain_memregion_init((scratch->fw_start + scratch->fw_rw_offset),
> -                                 (scratch->fw_size - scratch->fw_rw_offset),
> -                                 (SBI_DOMAIN_MEMREGION_M_READABLE |
> -                                  SBI_DOMAIN_MEMREGION_M_WRITABLE |
> -                                  SBI_DOMAIN_MEMREGION_FW),
> -                                 &root_memregs[root_memregs_count++]);
> +       if (sbi_platform_single_fw_region(sbi_platform_ptr(scratch))) {
> +               sbi_domain_memregion_init(scratch->fw_start, scratch->fw_size,
> +                                         (SBI_DOMAIN_MEMREGION_M_READABLE |
> +                                          SBI_DOMAIN_MEMREGION_M_WRITABLE |
> +                                          SBI_DOMAIN_MEMREGION_M_EXECUTABLE |
> +                                          SBI_DOMAIN_MEMREGION_FW),
> +                                         &root_memregs[root_memregs_count++]);
> +       } else {
> +               sbi_domain_memregion_init(scratch->fw_start,
> +                                         scratch->fw_rw_offset,
> +                                         (SBI_DOMAIN_MEMREGION_M_READABLE |
> +                                          SBI_DOMAIN_MEMREGION_M_EXECUTABLE |
> +                                          SBI_DOMAIN_MEMREGION_FW),
> +                                         &root_memregs[root_memregs_count++]);
> +
> +               sbi_domain_memregion_init((scratch->fw_start +
> +                                          scratch->fw_rw_offset),
> +                                         (scratch->fw_size -
> +                                          scratch->fw_rw_offset),
> +                                         (SBI_DOMAIN_MEMREGION_M_READABLE |
> +                                          SBI_DOMAIN_MEMREGION_M_WRITABLE |
> +                                          SBI_DOMAIN_MEMREGION_FW),
> +                                         &root_memregs[root_memregs_count++]);
> +       }
>
>         root.fw_region_inited = true;
>
> --
> 2.34.1
>
>
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi