[PATCH 2/4] lib: sbi: Move PMP encoding into a new file

[Joel Stanley]

On Tue, 10 Mar 2026 at 11:19, Nicholas Piggin <npiggin at gmail.com> wrote:
>
> The Tenstorrent RISC-V IOMMU PMP MMRs use the same encoding as PMP CSRs.
> In preparation to support it, move the non hart-specific PMP operations
> into their own file where they will also be used to build the IOMMU
> PMPs.
>
> Signed-off-by: Nicholas Piggin <npiggin at gmail.com>

> --- a/lib/sbi/riscv_asm.c
> +++ b/lib/sbi/riscv_asm.c

> +int is_pmp_entry_mapped(unsigned int entry)
> +{
> +       pmp_t pmp;

> +       if (hart_pmp_read(entry, &pmp))
> +               return pmp_enabled(&pmp);

The diff is a bit all over the place so I may have confused myself.
The patch keeps is_pmp_entry_mapped but changes the body from

    if (pmp_get(entry, &prot, &addr, &log2len) != 0)
        return false;

to

    if (hart_pmp_read(entry, &pmp))
        return pmp_enabled(&pmp);

but hart_pmp_read returns SBI_OK (0) on success. So we should keep the !=0?

> +int pmp_get(unsigned int n, unsigned long *prot_out, unsigned long *addr_out,
> +           unsigned long *log2len)
> +{
> +       pmp_t pmp;
> +       int rc;
>
> -       /* return details */
> -       *prot_out    = prot;

It looks like prot_out is no longer written to after your patch. This
hunk never made it into the new pmp_decode:

       /* decode PMP config */
       cfgmask = (0xffUL << pmpcfg_shift);
       pmpcfg  = csr_read_num(pmpcfg_csr) & cfgmask;
       prot    = pmpcfg >> pmpcfg_shift;


> --- /dev/null
> +++ b/lib/sbi/sbi_pmp.c
> @@ -0,0 +1,91 @@

> +int pmp_create(pmp_t *pmp, unsigned long prot, unsigned long addr,
> +           unsigned long log2len)
> +{
> +       unsigned long addrmask, pmpaddr;
> +
> +       /* check parameters */
> +       if (log2len > __riscv_xlen || log2len < PMP_SHIFT)
> +               return SBI_EINVAL;

Here we check that log2len <=__riscv_xlen, but in pmp_decode below we
set log2len =  __riscv_xlen + 3. Is that just how it works, or should
pmp_decode be able to decode what pmp_create does?

> +
> +       /* encode PMP config */
> +       prot &= ~PMP_A;
> +       prot |= (log2len == PMP_SHIFT) ? PMP_A_NA4 : PMP_A_NAPOT;
> +
> +       /* encode PMP address */
> +       if (log2len == PMP_SHIFT) {
> +               pmpaddr = (addr >> PMP_SHIFT);
> +       } else {
> +               if (log2len == __riscv_xlen) {
> +                       pmpaddr = -1UL;
> +               } else {
> +                       addrmask = (1UL << (log2len - PMP_SHIFT)) - 1;
> +                       pmpaddr  = ((addr >> PMP_SHIFT) & ~addrmask);
> +                       pmpaddr |= (addrmask >> 1);
> +               }
> +       }
> +
> +       pmp->cfg = prot;
> +       pmp->addr = pmpaddr;
> +
> +       return SBI_OK;
> +}
> +
> +int pmp_decode(pmp_t *pmp, unsigned long *prot_out, unsigned long *addr_out,
> +           unsigned long *log2len)
> +{
> +       /* check parameters */
> +       if (!pmp || !prot_out || !addr_out || !log2len)
> +               return SBI_EINVAL;
> +
> +       if (!pmp_enabled(pmp))
> +               return SBI_EINVAL;
> +
> +       /* decode PMP address */
> +       if ((pmp->cfg & PMP_A) == PMP_A_NAPOT) {
> +               if (pmp->addr == -1UL) {
> +                       *addr_out = 0;
> +                       *log2len = __riscv_xlen + 3;
> +               } else {
> +                       unsigned long mask = ~((1UL << cto(pmp->addr)) - 1);
> +                       *addr_out = (pmp->addr & mask) << PMP_SHIFT;
> +                       *log2len = (cto(pmp->addr) + PMP_SHIFT + 1);
> +               }
> +       } else {
> +               *addr_out = pmp->addr << PMP_SHIFT;
> +               *log2len = PMP_SHIFT;
> +       }
> +
> +       return SBI_OK;
> +}