[PATCH] lib: sbi: Flush cache entries after writing PMP CSRs

[Anup Patel]

On Thu, Oct 30, 2025 at 4:02 AM Samuel Holland
<samuel.holland at sifive.com> wrote:
>
> Hi Anup,
>
> On 2025-10-19 11:36 PM, Anup Patel wrote:
> > On Thu, Sep 4, 2025 at 4:26 PM <cp0613 at linux.alibaba.com> wrote:
> >>
> >> From: Chen Pei <cp0613 at linux.alibaba.com>
> >>
> >> As the privileged specification states, after writing to the PMP CSRs,
> >> a SFENCE.VMA or HFENCE.GVMA instruction should be executed with rs1=x0
> >> and rs2=x0 to flush all address translation cache entries.
> >>
> >> The original implementation does not cover all possible cases. For
> >> example, the sbi_hart_map_saddr function calls pmp_set but does not
> >> execute the SFENCE.VMA instruction. This patch covers sbi_hart_map_saddr
> >> and sbi_hart_unmap_saddr, ensuring that dbtr, sse and other modules can
> >> safely update pmpcfg.
> >
> > The temporary mapping created using sbi_hart_map/unmap_saddr() is
> > only accessed by M-mode and does not impact the S-mode TLB
> > entries in anyway.
>
> That doesn't match my interpretation of this paragraph from the privileged ISA:
>
> Implementations with virtual memory are permitted to perform address
> translations speculatively and earlier than required by an explicit memory
> access, and are permitted to cache them in address translation cache
> structures—including possibly caching the identity mappings from effective
> address to physical address used in Bare translation modes and M-mode. The PMP
> settings for the resulting physical address may be checked (and possibly cached)
> at any point between the address translation and the explicit memory access.
> Hence, when the PMP settings are modified, M-mode software must synchronize the
> PMP settings with the virtual memory system and any PMP or address-translation
> caches. This is accomplished by executing an SFENCE.VMA instruction with rs1=x0
> and rs2=x0, after the PMP CSRs are written. See Section 19.5.3 for additional
> synchronization requirements when the hypervisor extension is implemented.
>
> Specifically, note "including possibly caching the identity mappings from
> effective address to physical address used in Bare translation modes and M-mode".
>
> So hardware is allowed to (speculatively) fill the TLB with entries that apply
> to M-mode, and tag those M-mode PMP entries with the permissions from the PMPs.
> The sfence.vma is required to clear those entries, or else accesses _even from
> M-mode_ may erroneously use the previous PMP permissions.

If hardware is filling TLB entries for M-mode then it must tag these TLB
entries with privilege level so that they don't match for S-mode otherwise
a simple M-mode to S-mode switch at boot-time will allow bootloaders
to access M-mode memory with M-mode permissions.

Regards,
Anup

>
> Regards,
> Samuel
>
> > Further, flushing TLB entries in sbi_hart_map/unmap_saddr() also
> > slows-down many SBI calls which use shared memory.
> >
> > NACK to this patch.
> >
> > Regards,
> > Anup
> >
> >>
> >> Signed-off-by: Chen Pei <cp0613 at linux.alibaba.com>
> >> ---
> >>  lib/sbi/sbi_hart.c | 54 ++++++++++++++++++++++++++++------------------
> >>  1 file changed, 33 insertions(+), 21 deletions(-)
> >>
> >> diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> >> index 6a2d7d6..33bba38 100644
> >> --- a/lib/sbi/sbi_hart.c
> >> +++ b/lib/sbi/sbi_hart.c
> >> @@ -364,6 +364,30 @@ static unsigned int sbi_hart_get_smepmp_flags(struct sbi_scratch *scratch,
> >>         return pmp_flags;
> >>  }
> >>
> >> +static void pmp_flush(void)
> >> +{
> >> +       /*
> >> +        * As per section 3.7.2 of privileged specification v1.12,
> >> +        * virtual address translations can be speculatively performed
> >> +        * (even before actual access). These, along with PMP traslations,
> >> +        * can be cached. This can pose a problem with CPU hotplug
> >> +        * and non-retentive suspend scenario because PMP states are
> >> +        * not preserved.
> >> +        * It is advisable to flush the caching structures under such
> >> +        * conditions.
> >> +        */
> >> +       if (misa_extension('S')) {
> >> +               __asm__ __volatile__("sfence.vma");
> >> +
> >> +               /*
> >> +                * If hypervisor mode is supported, flush caching
> >> +                * structures in guest mode too.
> >> +                */
> >> +               if (misa_extension('H'))
> >> +                       __sbi_hfence_gvma_all();
> >> +       }
> >> +}
> >> +
> >>  static void sbi_hart_smepmp_set(struct sbi_scratch *scratch,
> >>                                 struct sbi_domain *dom,
> >>                                 struct sbi_domain_memregion *reg,
> >> @@ -543,18 +567,25 @@ int sbi_hart_map_saddr(unsigned long addr, unsigned long size)
> >>                              pmp_flags, base, order);
> >>         pmp_set(SBI_SMEPMP_RESV_ENTRY, pmp_flags, base, order);
> >>
> >> +       pmp_flush();
> >> +
> >>         return SBI_OK;
> >>  }
> >>
> >>  int sbi_hart_unmap_saddr(void)
> >>  {
> >> +       int rc;
> >>         struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
> >>
> >>         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);
> >> +       rc = pmp_disable(SBI_SMEPMP_RESV_ENTRY);
> >> +
> >> +       pmp_flush();
> >> +
> >> +       return rc;
> >>  }
> >>
> >>  int sbi_hart_pmp_configure(struct sbi_scratch *scratch)
> >> @@ -578,26 +609,7 @@ int sbi_hart_pmp_configure(struct sbi_scratch *scratch)
> >>                 rc = sbi_hart_oldpmp_configure(scratch, pmp_count,
> >>                                                 pmp_log2gran, pmp_addr_max);
> >>
> >> -       /*
> >> -        * As per section 3.7.2 of privileged specification v1.12,
> >> -        * virtual address translations can be speculatively performed
> >> -        * (even before actual access). These, along with PMP traslations,
> >> -        * can be cached. This can pose a problem with CPU hotplug
> >> -        * and non-retentive suspend scenario because PMP states are
> >> -        * not preserved.
> >> -        * It is advisable to flush the caching structures under such
> >> -        * conditions.
> >> -        */
> >> -       if (misa_extension('S')) {
> >> -               __asm__ __volatile__("sfence.vma");
> >> -
> >> -               /*
> >> -                * If hypervisor mode is supported, flush caching
> >> -                * structures in guest mode too.
> >> -                */
> >> -               if (misa_extension('H'))
> >> -                       __sbi_hfence_gvma_all();
> >> -       }
> >> +       pmp_flush();
> >>
> >>         return rc;
> >>  }
> >> --
> >> 2.49.0
> >>
> >>
> >> --
> >> opensbi mailing list
> >> opensbi at lists.infradead.org
> >> http://lists.infradead.org/mailman/listinfo/opensbi
> >
>