[PATCH] lib: sbi_platform: Add platform specific pmp_set() and pmp_disable()

Anup Patel anup at brainfault.org
Mon Jun 16 04:41:05 PDT 2025


Hi Chao-ying Fu,


On Sat, Jun 14, 2025 at 10:58 PM Anup Patel <apatel at ventanamicro.com> wrote:
>
> From: Chao-ying Fu <cfu at wavecomp.com>
>
> Allow platforms to implement platform specific PMP setup and
> PMP disable functions which are called before actual PMP CSRs
> are configured.
>
> Also, implement pmp_set() and pmp_disable() for MIPS P8700.
>
> Signed-off-by: Chao-ying Fu <cfu at mips.com>
> Signed-off-by: Anup Patel <apatel at ventanamicro.com>

Please try this patch at your end on top of the latest OpenSBI
and see if this helps on MIPS P8700.

Regards,
Anup

> ---
>  include/sbi/sbi_platform.h    | 39 +++++++++++++++
>  lib/sbi/sbi_domain_context.c  |  2 +
>  lib/sbi/sbi_hart.c            | 10 ++++
>  platform/generic/mips/p8700.c | 93 +++++++++++++++++++++++++++++++++++
>  4 files changed, 144 insertions(+)
>
> diff --git a/include/sbi/sbi_platform.h b/include/sbi/sbi_platform.h
> index 08ece320..c6d30080 100644
> --- a/include/sbi/sbi_platform.h
> +++ b/include/sbi/sbi_platform.h
> @@ -142,6 +142,13 @@ struct sbi_platform_operations {
>         /** platform specific handler to fixup store fault */
>         int (*emulate_store)(int wlen, unsigned long addr,
>                              union sbi_ldst_data in_val);
> +
> +       /** platform specific pmp setup on current HART */
> +       void (*pmp_set)(unsigned int n, unsigned long flags,
> +                       unsigned long prot, unsigned long addr,
> +                       unsigned long log2len);
> +       /** platform specific pmp disable on current HART */
> +       void (*pmp_disable)(unsigned int n);
>  };
>
>  /** Platform default per-HART stack size for exception/interrupt handling */
> @@ -644,6 +651,38 @@ static inline int sbi_platform_emulate_store(const struct sbi_platform *plat,
>         return SBI_ENOTSUPP;
>  }
>
> +/**
> + * Platform specific PMP setup on current HART
> + *
> + * @param plat pointer to struct sbi_platform
> + * @param n index of the pmp entry
> + * @param flags domain memregion flags
> + * @param prot attribute of the pmp entry
> + * @param addr address of the pmp entry
> + * @param log2len size of the pmp entry as power-of-2
> + */
> +static inline void sbi_platform_pmp_set(const struct sbi_platform *plat,
> +                                       unsigned int n, unsigned long flags,
> +                                       unsigned long prot, unsigned long addr,
> +                                       unsigned long log2len)
> +{
> +       if (plat && sbi_platform_ops(plat)->pmp_set)
> +               sbi_platform_ops(plat)->pmp_set(n, flags, prot, addr, log2len);
> +}
> +
> +/**
> + * Platform specific PMP disable on current HART
> + *
> + * @param plat pointer to struct sbi_platform
> + * @param n index of the pmp entry
> + */
> +static inline void sbi_platform_pmp_disable(const struct sbi_platform *plat,
> +                                           unsigned int n)
> +{
> +       if (plat && sbi_platform_ops(plat)->pmp_disable)
> +               sbi_platform_ops(plat)->pmp_disable(n);
> +}
> +
>  #endif
>
>  #endif
> diff --git a/lib/sbi/sbi_domain_context.c b/lib/sbi/sbi_domain_context.c
> index 2b19fcc7..fb04d81d 100644
> --- a/lib/sbi/sbi_domain_context.c
> +++ b/lib/sbi/sbi_domain_context.c
> @@ -15,6 +15,7 @@
>  #include <sbi/sbi_string.h>
>  #include <sbi/sbi_domain.h>
>  #include <sbi/sbi_domain_context.h>
> +#include <sbi/sbi_platform.h>
>  #include <sbi/sbi_trap.h>
>
>  /** Context representation for a hart within a domain */
> @@ -115,6 +116,7 @@ static void switch_to_next_domain_context(struct hart_context *ctx,
>
>         /* Reconfigure PMP settings for the new domain */
>         for (int i = 0; i < pmp_count; i++) {
> +               sbi_platform_pmp_disable(sbi_platform_thishart_ptr(), i);
>                 pmp_disable(i);
>         }
>         sbi_hart_pmp_configure(scratch);
> diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> index 74ccdd84..054a405c 100644
> --- a/lib/sbi/sbi_hart.c
> +++ b/lib/sbi/sbi_hart.c
> @@ -375,6 +375,9 @@ static void sbi_hart_smepmp_set(struct sbi_scratch *scratch,
>         unsigned long pmp_addr = reg->base >> PMP_SHIFT;
>
>         if (pmp_log2gran <= reg->order && pmp_addr < pmp_addr_max) {
> +               sbi_platform_pmp_set(sbi_platform_ptr(scratch),
> +                                    pmp_idx, reg->flags, pmp_flags,
> +                                    reg->base, reg->order);
>                 pmp_set(pmp_idx, pmp_flags, reg->base, reg->order);
>         } else {
>                 sbi_printf("Can not configure pmp for domain %s because"
> @@ -492,6 +495,9 @@ static int sbi_hart_oldpmp_configure(struct sbi_scratch *scratch,
>
>                 pmp_addr = reg->base >> PMP_SHIFT;
>                 if (pmp_log2gran <= reg->order && pmp_addr < pmp_addr_max) {
> +                       sbi_platform_pmp_set(sbi_platform_ptr(scratch),
> +                                            pmp_idx, reg->flags, pmp_flags,
> +                                            reg->base, reg->order);
>                         pmp_set(pmp_idx++, pmp_flags, reg->base, reg->order);
>                 } else {
>                         sbi_printf("Can not configure pmp for domain %s because"
> @@ -532,6 +538,9 @@ int sbi_hart_map_saddr(unsigned long addr, unsigned long size)
>                 }
>         }
>
> +       sbi_platform_pmp_set(sbi_platform_ptr(scratch), SBI_SMEPMP_RESV_ENTRY,
> +                            SBI_DOMAIN_MEMREGION_SHARED_SURW_MRW,
> +                            pmp_flags, base, order);
>         pmp_set(SBI_SMEPMP_RESV_ENTRY, pmp_flags, base, order);
>
>         return SBI_OK;
> @@ -544,6 +553,7 @@ int sbi_hart_unmap_saddr(void)
>         if (!sbi_hart_has_extension(scratch, SBI_HART_EXT_SMEPMP))
>                 return SBI_OK;
>
> +       sbi_platform_pmp_disable(sbi_platform_ptr(scratch), SBI_SMEPMP_RESV_ENTRY);
>         return pmp_disable(SBI_SMEPMP_RESV_ENTRY);
>  }
>
> diff --git a/platform/generic/mips/p8700.c b/platform/generic/mips/p8700.c
> index 6b687179..888a45c6 100644
> --- a/platform/generic/mips/p8700.c
> +++ b/platform/generic/mips/p8700.c
> @@ -16,6 +16,98 @@
>  #include <mips/p8700.h>
>  #include <mips/mips-cm.h>
>
> +static unsigned long mips_csr_read_num(int csr_num)
> +{
> +#define switchcase_csr_read(__csr_num, __val)          \
> +       case __csr_num:                                 \
> +               __val = csr_read(__csr_num);            \
> +               break;
> +#define switchcase_csr_read_2(__csr_num, __val)                \
> +       switchcase_csr_read(__csr_num + 0, __val)       \
> +       switchcase_csr_read(__csr_num + 1, __val)
> +#define switchcase_csr_read_4(__csr_num, __val)                \
> +       switchcase_csr_read_2(__csr_num + 0, __val)     \
> +       switchcase_csr_read_2(__csr_num + 2, __val)
> +#define switchcase_csr_read_8(__csr_num, __val)                \
> +       switchcase_csr_read_4(__csr_num + 0, __val)     \
> +       switchcase_csr_read_4(__csr_num + 4, __val)
> +#define switchcase_csr_read_16(__csr_num, __val)       \
> +       switchcase_csr_read_8(__csr_num + 0, __val)     \
> +       switchcase_csr_read_8(__csr_num + 8, __val)
> +
> +       unsigned long ret = 0;
> +
> +       switch(csr_num) {
> +       switchcase_csr_read_16(CSR_MIPSPMACFG0, ret)
> +
> +       default:
> +               sbi_panic("%s: Unknown CSR %#x", __func__, csr_num);
> +               break;
> +       }
> +
> +       return ret;
> +
> +#undef switchcase_csr_read_16
> +#undef switchcase_csr_read_8
> +#undef switchcase_csr_read_4
> +#undef switchcase_csr_read_2
> +#undef switchcase_csr_read
> +}
> +
> +static void mips_csr_write_num(int csr_num, unsigned long val)
> +{
> +#define switchcase_csr_write(__csr_num, __val)         \
> +       case __csr_num:                                 \
> +               csr_write(__csr_num, __val);            \
> +               break;
> +#define switchcase_csr_write_2(__csr_num, __val)       \
> +       switchcase_csr_write(__csr_num + 0, __val)      \
> +       switchcase_csr_write(__csr_num + 1, __val)
> +#define switchcase_csr_write_4(__csr_num, __val)       \
> +       switchcase_csr_write_2(__csr_num + 0, __val)    \
> +       switchcase_csr_write_2(__csr_num + 2, __val)
> +#define switchcase_csr_write_8(__csr_num, __val)       \
> +       switchcase_csr_write_4(__csr_num + 0, __val)    \
> +       switchcase_csr_write_4(__csr_num + 4, __val)
> +#define switchcase_csr_write_16(__csr_num, __val)      \
> +       switchcase_csr_write_8(__csr_num + 0, __val)    \
> +       switchcase_csr_write_8(__csr_num + 8, __val)
> +
> +       switch(csr_num) {
> +       switchcase_csr_write_16(CSR_MIPSPMACFG0, val)
> +
> +       default:
> +               sbi_panic("%s: Unknown CSR %#x", __func__, csr_num);
> +               break;
> +       }
> +
> +#undef switchcase_csr_write_16
> +#undef switchcase_csr_write_8
> +#undef switchcase_csr_write_4
> +#undef switchcase_csr_write_2
> +#undef switchcase_csr_write
> +}
> +
> +static void mips_p8700_pmp_set(unsigned int n, unsigned long flags,
> +                              unsigned long prot, unsigned long addr,
> +                              unsigned long log2len)
> +{
> +       int pmacfg_csr, pmacfg_shift;
> +       unsigned long cfgmask;
> +       unsigned long pmacfg, cca;
> +
> +       pmacfg_csr = (CSR_MIPSPMACFG0 + (n >> 2)) & ~1;
> +       pmacfg_shift = (n & 7) << 3;
> +       cfgmask = ~(0xffUL << pmacfg_shift);
> +
> +       /* Read pmacfg to change cacheability */
> +       pmacfg = (mips_csr_read_num(pmacfg_csr) & cfgmask);
> +       cca = (flags & SBI_DOMAIN_MEMREGION_MMIO) ? CCA_CACHE_DISABLE :
> +                                 CCA_CACHE_ENABLE | PMA_SPECULATION;
> +       pmacfg |= ((cca << pmacfg_shift) & ~cfgmask);
> +       mips_csr_write_num(pmacfg_csr, pmacfg);
> +}
> +
>  #if CLUSTERS_IN_PLATFORM > 1
>  static void power_up_other_cluster(u32 hartid)
>  {
> @@ -255,6 +347,7 @@ static int mips_p8700_platform_init(const void *fdt, int nodeoff, const struct f
>         generic_platform_ops.early_init = mips_p8700_early_init;
>         generic_platform_ops.final_init = mips_p8700_final_init;
>         generic_platform_ops.nascent_init = mips_p8700_nascent_init;
> +       generic_platform_ops.pmp_set = mips_p8700_pmp_set;
>
>         return 0;
>  }
> --
> 2.43.0
>



More information about the opensbi mailing list