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

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?

> +        sfence.vma

The one thing which possibly matters here, and could explain why 
sfence.vma is needed, is:
```
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.
```

So the TLB could potentially be populated with identity mappings, and I 
agree that it would be better to flush those.

I’m not entirely convinced, though, that the reason here is the ASID 
itself. Rather, it seems that we want to flush because of potentially 
cached speculative identity mappings.

If this reasoning looks correct to you, could we update the commit 
message to reflect this rationale for why sfence.vma is needed here?

Thanks.

~ Oleksii