Re: [PATCH 5/5] xen/riscv: add SFENCE.VMA after enabling paging

On 2026-08-28 10:11 +0200, Oleksii Kurochko wrote:
> 
> 
> On 8/27/26 6:53 PM, Oleksii Kurochko wrote:
> > 
> > 
> > On 8/27/26 5:33 PM, Baptiste Le Duc wrote:
> >> turn_on_mmu() writes satp to switch on Sv39 paging but never fences
> >> afterwards.
> >>
> >> Xen never allocates a non-zero ASID, so per the Privileged spec, sec.
> >> 12.2.1 "Supervisor Memory-Management Fence Instruction":
> >>
> >>    "If the implementation does not provide ASIDs, or software chooses
> >>    to always use ASID 0, then after every satp write, software should
> >>    execute SFENCE.VMA with rs1=x0."
> >>
> >> The spec text around this rule hedges with "may be necessary", but
> >> RISC-V spec co-author Andrew Waterman confirmed on the ISA manual
> >> issue tracker that the fence after a satp write is not optional in
> >> this case: "The SFENCE after the SATP write is definitely necessary
> >> ... In general, you need to SFENCE after you've recycled an ASID.
> >> Since we don't use ASIDs in the Linux kernel yet, every context
> >> switch is effectively an ASID reuse, hence the full TLB flush." [1]
> >> The same reasoning applies to Xen: with ASID always 0, this satp
> >> write is indistinguishable from an ASID reuse to the hart, so the
> >> fence is required for correctness.
> > 
> > But at the moment of execution of turn_on_mmu() we don't use any ASID, 
> > do we? It was used in check_pgtbl_mode_support() but at the end it is done:
> > 
> >      csr_write(CSR_SATP, 0);
> > 
> >      sfence_vma();
> > 
> > So basically Bare mode + flush all TLBs presented before and then up to
> > 
> > ...
> > 
> >>
> >> Add the missing SFENCE.VMA to order those page-table stores before
> >> the hart's first translation under the new mapping.
> >>
> >> [1] https://github.com/riscv/riscv-isa-manual/issues/226
> >>
> >> Fixes: f5035d480f7a ("xen: add files needed for minimal riscv build")
> >> Assisted-by: Claude:claude-opus-5
> >> Signed-off-by: Baptiste Le Duc <[email protected]>
> >> ---
> >>   xen/arch/riscv/riscv64/head.S | 1 +
> >>   1 file changed, 1 insertion(+)
> >>
> >> diff --git a/xen/arch/riscv/riscv64/head.S b/xen/arch/riscv/riscv64/ 
> >> head.S
> >> index 9c40512e61..7f6edc972f 100644
> >> --- a/xen/arch/riscv/riscv64/head.S
> >> +++ b/xen/arch/riscv/riscv64/head.S
> >> @@ -98,6 +98,7 @@ FUNC(turn_on_mmu)
> >>           srli    t1, t1, PAGE_SHIFT
> >>           or      t1, t1, t0
> >>           csrw    CSR_SATP, t1
> > 
> > ... ASID isn't used as we are in Bare mode.
> > 
> > What am I missing?
> 
> After the conversation with Jan B. in the separate thread I re-read 
> documentaion and found that ASID=0 will be used here too as after 
> check_pgtbl_mode_support() we set Bare mode and ASID 0 will be really 
> used even in Bare mode as to select MODE=Bare as software must write 
> zero to the remaining fields of satp (bits 30–0 when SXLEN=32, or bits 
> 59–0 when SXLEN=64) what automatically includes field ASID (so it will 
> be zero).
> 
> But still the full reason why we need sfence.vma here is that TLB could 
> be polluted with identity mapping (even in Bare mode) and which will be 
> tagged by ASID=0.
> 
> So what about to update commit message with:
> ```
> xen/riscv: flush speculatively cached Bare-mode TLB entries in turn_on_mmu()
> 
> The existing SFENCE.VMA before the satp write only orders the page table
> stores from setup_initial_pagetables() against subsequent implicit reads.
> It does not prevent the CPU from speculatively caching translations 
> after the fence retires.
> 
> According to the RISC-V Privileged specification, implementations are
> permitted to speculatively cache Bare-mode identity mappings. Furthermore,
> selecting MODE=Bare (which happens during check_pgtbl_mode_support())
> requires zeroing the remaining fields of satp, causing ASID=0 to be
> actively used in Bare mode. Consequently, the TLB can be polluted with Bare
> identity mappings tagged with ASID=0.
> 
> Once satp is written to enable Sv39 translation, these cached identity
> mappings (tagged with ASID=0) can shadow the true Sv39 translations.
> This would lead to translation failures since turn_on_mmu() jumps to
> a non-identity-mapped linker address.
> 
> Fix this by adding a post-satp-write SFENCE.VMA to invalidate any stale
> translations (including Bare-mode identity mappings under ASID=0) before
> jumping to the virtual address space.
> ```
> 
> ~ Oleksii
> 
I read the thread and I'm ok with this suggestion.
Thanks.
> 
>