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

[Anup Patel]

On Tue, Jun 17, 2025 at 8:16 PM Chao-ying Fu <cfu at mips.com> wrote:
>
> Hi Anup,
>
> > Hi Chao-ying Fu,
> >
> > On Mon, Jun 16, 2025 at 9:47 PM Chao-ying Fu <cfu at mips.com> wrote:
> > >
> > > Hi Anup,
> > >
> > > > 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
> > >
> > >   Yes, this patch works functionally the same as what the old version does on FPGA and QEMU.
>
> > Thanks, I will apply this patch to the riscv/opensbi repo.
>
> > > Note: I added two more patches from Vladimir to deal with MMIO regions on top of this patc h.
>
> > With this patch, we don't need the PMP_MMIO flag added by Vladimir's patches.
> > Is there any other patch ?
>
>   Sorry, I didn't notice that there is a parameter: flags.
> With "flags", we can check SBI_DOMAIN_MEMREGION_MMIO in mips_p8700_pmp_set().
> We don't need Vladirmir's patch that creates PMP_MMIO and propagates to the parameter: prop.
>
>   However, we may still need Vladimir's patch for the fdt fixup code that tries to disable the aplic node.
> With this patch, we can have the MMIO type for the region that covers the aplic node, and
> mips_p8700_pmp_set() can set up correct cacheability for the MMIO or non-MMIO region.
> (This patch may affect the existing dts files that don't have the MMIO type for the region that covers the aplic node.)

OpenSBI is already disabling APLIC root domain which is meant
for M-mode.

This can be easily verified on QEMU virt,aia=aplic-imsic machine
from Linux user-space using following command:
$ cat /proc/device-tree/soc/interrupt-controller at c000000/status

> --
> From: Vladimir Kondratiev <vladimir.kondratiev at mobileye.com>
>
> When applying FDT fixups, request device memory to be of
> MMIO type
>
> Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev at mobileye.com>
> ---
>  lib/utils/fdt/fdt_fixup.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/utils/fdt/fdt_fixup.c b/lib/utils/fdt/fdt_fixup.c
> index f3fe8af..b0ed20c 100644
> --- a/lib/utils/fdt/fdt_fixup.c
> +++ b/lib/utils/fdt/fdt_fixup.c
> @@ -185,7 +185,7 @@ static void fdt_domain_based_fixup_one(void *fdt, int nodeoff)
>                 return;
>
>         if (!sbi_domain_check_addr(dom, reg_addr, dom->next_mode,
> -                                   SBI_DOMAIN_READ | SBI_DOMAIN_WRITE)) {
> +                                   SBI_DOMAIN_READ | SBI_DOMAIN_WRITE | SBI_DOMAIN_MMIO)) {
>                 rc = fdt_open_into(fdt, fdt, fdt_totalsize(fdt) + 32);
>                 if (rc < 0)
>                         return;
> --
> 2.47.1
>

Regards,
Anup