[PATCH v2 7/8] lib: sbi: sbi_hart: track firmware PMP entries when configuring SmePMP

[Anup Patel]

On Wed, Oct 8, 2025 at 2:15 PM Yu-Chien Peter Lin <peter.lin at sifive.com> wrote:
>
> Add fw_smepmp_ids bitmap to track PMP entries that protect firmware
> regions. Allow us to preserve these critical entries across domain
> transitions and check inconsistent firmware entry allocation.
>
> Also add sbi_hart_pmp_is_fw_region() helper function to query

s/sbi_hart_pmp_is_fw_region/sbi_hart_smepmp_is_fw_region/

> whether a given PMP entry protects firmware regions.

s/PMP/SmePMP/

>
> Signed-off-by: Yu-Chien Peter Lin <peter.lin at sifive.com>
> ---
>  include/sbi/sbi_hart.h |  1 +
>  lib/sbi/sbi_hart.c     | 33 +++++++++++++++++++++++++++++++++
>  2 files changed, 34 insertions(+)
>
> diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h
> index 82b19dcf..6db4fed7 100644
> --- a/include/sbi/sbi_hart.h
> +++ b/include/sbi/sbi_hart.h
> @@ -142,6 +142,7 @@ unsigned int sbi_hart_pmp_count(struct sbi_scratch *scratch);
>  unsigned int sbi_hart_pmp_log2gran(struct sbi_scratch *scratch);
>  unsigned int sbi_hart_pmp_addrbits(struct sbi_scratch *scratch);
>  unsigned int sbi_hart_mhpm_bits(struct sbi_scratch *scratch);
> +bool sbi_hart_smepmp_is_fw_region(unsigned int pmp_idx);
>  int sbi_hart_pmp_configure(struct sbi_scratch *scratch);
>  int sbi_hart_map_saddr(unsigned long base, unsigned long size);
>  int sbi_hart_unmap_saddr(void);
> diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> index 032f7dc1..abab8b73 100644
> --- a/lib/sbi/sbi_hart.c
> +++ b/lib/sbi/sbi_hart.c
> @@ -30,6 +30,8 @@ extern void __sbi_expected_trap_hext(void);
>  void (*sbi_hart_expected_trap)(void) = &__sbi_expected_trap;
>
>  static unsigned long hart_features_offset;
> +static DECLARE_BITMAP(fw_smepmp_ids, PMP_COUNT);
> +static bool fw_smepmp_ids_inited;
>
>  static void mstatus_init(struct sbi_scratch *scratch)
>  {
> @@ -301,6 +303,17 @@ unsigned int sbi_hart_mhpm_bits(struct sbi_scratch *scratch)
>         return hfeatures->mhpm_bits;
>  }
>
> +bool sbi_hart_smepmp_is_fw_region(unsigned int pmp_idx)
> +{
> +       if (!fw_smepmp_ids_inited) {
> +               sbi_printf("%s: ERR: fw_smepmp_ids uninitialized\n",
> +                          __func__);

This printf() causes unnecessary prints on HW with legacy PMP so
needs to be removed.

> +               return false;
> +       }
> +
> +       return bitmap_test(fw_smepmp_ids, pmp_idx) ? true : false;
> +}
> +
>  static void sbi_hart_smepmp_set(struct sbi_scratch *scratch,
>                                 struct sbi_domain *dom,
>                                 struct sbi_domain_memregion *reg,
> @@ -367,12 +380,32 @@ static int sbi_hart_smepmp_configure(struct sbi_scratch *scratch,
>                         continue;
>                 }
>
> +               /*
> +                * Track firmware PMP entries to preserve them during
> +                * domain switches. Under SmePMP, M-mode requires
> +                * explicit PMP entries to access firmware code/data.
> +                * These entries must remain enabled across domain
> +                * context switches to prevent M-mode access faults.
> +                */
> +               if (SBI_DOMAIN_MEMREGION_IS_FIRMWARE(reg->flags)) {
> +                       if (fw_smepmp_ids_inited) {
> +                               /* Check inconsistent firmware region */
> +                               if (!sbi_hart_smepmp_is_fw_region(pmp_idx))
> +                                       return SBI_EINVAL;
> +                       } else {
> +                               bitmap_set(fw_smepmp_ids, pmp_idx, 1);
> +                       }
> +               }
> +
>                 pmp_flags = sbi_domain_get_smepmp_flags(reg);
>
>                 sbi_hart_smepmp_set(scratch, dom, reg, pmp_idx++, pmp_flags,
>                                     pmp_log2gran, pmp_addr_max);
> +

Redundant newline.

>         }
>
> +       fw_smepmp_ids_inited = true;
> +
>         /* Set the MML to enforce new encoding */
>         csr_set(CSR_MSECCFG, MSECCFG_MML);
>
> --
> 2.48.0
>

Otherwise, it looks good to me. I will take care of the above
comments at the time of merging.

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

Thanks,
Anup