On 18.02.2026 17:58, Oleksii Kurochko wrote: > > On 2/13/26 5:29 PM, Oleksii Kurochko wrote: >> Introduce helpers to manage VS-stage and G-stage translation state during >> vCPU context switches. >> >> As VSATP and HGATP cannot be updated atomically, clear VSATP on context >> switch-out to prevent speculative VS-stage translations from being associated >> with an incorrect VMID. On context switch-in, restore HGATP and VSATP in the >> required order. >> >> Add p2m_handle_vmenter() to perform VMID management and issue TLB flushes >> only when required (e.g. on VMID reuse or generation change). >> >> This provides the necessary infrastructure for correct p2m context switching >> on RISC-V. >> >> Signed-off-by: Oleksii Kurochko <[email protected]> >> --- >> Changes in v3: >> - Add comment above p2m_ctxt_switch_{to, from}(). >> - Code style fixes. >> - Refactor p2m_ctxt_switch_to(). >> - Update the comment at the end of p2m_ctxt_switch_from(). >> - Refactor the code of p2m_handle_vmenter(). >> --- >> Changes in v2: >> - Add vsatp field declaration to arch_vcpu. >> - s/p2m_ctx_switch_{from,to}/p2m_ctxt_switch_{from,to}. >> - Introduce p2m_handle_vmenter() for proper handling of VMID, >> hgatp and vsatp updates. >> - Introduce is_p2m_switch_finished and init it inisde >> p2m_ctx_switch_to() for furhter handling in p2m_handle_vmenter(). >> - Code style fixes. >> - Add is_idle_vcpu() check in p2m_ctxt_switch_from(). >> - use csr_swap() in p2m_ctxt_switch_from(). >> - move flush_tlb_guest_local() to the end if p2m_handle_vmenter() and >> drop unnessary anymore comments. >> - Correct printk()'s arguments in p2m_handle_vmenter(). >> --- >> xen/arch/riscv/include/asm/domain.h | 1 + >> xen/arch/riscv/include/asm/p2m.h | 4 ++ >> xen/arch/riscv/p2m.c | 79 +++++++++++++++++++++++++++++ >> xen/arch/riscv/traps.c | 2 + >> 4 files changed, 86 insertions(+) >> >> diff --git a/xen/arch/riscv/include/asm/domain.h >> b/xen/arch/riscv/include/asm/domain.h >> index 3da2387cb197..42bb678fcbf9 100644 >> --- a/xen/arch/riscv/include/asm/domain.h >> +++ b/xen/arch/riscv/include/asm/domain.h >> @@ -59,6 +59,7 @@ struct arch_vcpu { >> register_t hstateen0; >> register_t hvip; >> + register_t vsatp; >> register_t vsie; >> /* >> diff --git a/xen/arch/riscv/include/asm/p2m.h >> b/xen/arch/riscv/include/asm/p2m.h >> index f63b5dec99b1..60f27f9b347e 100644 >> --- a/xen/arch/riscv/include/asm/p2m.h >> +++ b/xen/arch/riscv/include/asm/p2m.h >> @@ -255,6 +255,10 @@ static inline bool p2m_is_locked(const struct >> p2m_domain *p2m) >> struct page_info *p2m_get_page_from_gfn(struct p2m_domain *p2m, gfn_t gfn, >> p2m_type_t *t); >> +void p2m_ctxt_switch_from(struct vcpu *p); >> +void p2m_ctxt_switch_to(struct vcpu *n); >> +void p2m_handle_vmenter(void); >> + >> #endif /* ASM__RISCV__P2M_H */ >> /* >> diff --git a/xen/arch/riscv/p2m.c b/xen/arch/riscv/p2m.c >> index 0abeb374c110..7ae854707174 100644 >> --- a/xen/arch/riscv/p2m.c >> +++ b/xen/arch/riscv/p2m.c >> @@ -1434,3 +1434,82 @@ struct page_info *p2m_get_page_from_gfn(struct >> p2m_domain *p2m, gfn_t gfn, >> return get_page(page, p2m->domain) ? page : NULL; >> } >> + >> +/* Should be called before other CSRs are stored to avoid speculation */ >> +void p2m_ctxt_switch_from(struct vcpu *p) >> +{ >> + if ( is_idle_vcpu(p) ) >> + return; >> + >> + /* >> + * No mechanism is provided to atomically change vsatp and hgatp >> + * together. Hence, to prevent speculative execution causing one >> + * guest’s VS-stage translations to be cached under another guest’s >> + * VMID, world-switch code should zero vsatp, then swap hgatp, then >> + * finally write the new vsatp value what will be done in >> + * p2m_handle_vmenter(). >> + */ >> + p->arch.vsatp = csr_swap(CSR_VSATP, 0); >> + >> + /* >> + * Nothing to do with HGATP as it will be update in p2m_ctxt_switch_to() >> + * or/and in p2m_handle_vmenter(). >> + */ >> +} >> + >> +/* Should be called after other CSRs are restored to avoid speculation */ >> +void p2m_ctxt_switch_to(struct vcpu *n) >> +{ >> + struct p2m_domain *p2m = p2m_get_hostp2m(n->domain); >> + >> + if ( is_idle_vcpu(n) ) >> + return; >> + >> + csr_write(CSR_HGATP, construct_hgatp(p2m, n->arch.vmid.vmid)); >> + /* >> + * As VMID is unique per vCPU and just re-used here thereby there is no >> + * need for G-stage TLB flush here. >> + */ >> + >> + csr_write(CSR_VSATP, n->arch.vsatp); >> + /* >> + * As at the start of context switch VSATP were set to 0, so no >> speculation >> + * could happen thereby there is no need for VS TLB flush here. >> + */ >> +} > > I think we need to flush the VS-stage TLB unconditionally here. It is possible > that `p->arch.vsatp.ASID == n->arch.vsatp.ASID`, in which case the new vCPU > could reuse stale VS-stage TLB entries that do not belong to it. > > I considered performing the flush conditionally, but that would require > checking > not only the ASID, but also whether the PPN differs. In addition, we would > need > to verify that the old and new vCPUs do not belong to different domains, since > the same VSATP PPN value could appear in different domains. > > This starts to look like overcomplication for a marginal optimization, so an > unconditional VS-stage TLB flush seems simpler and safer. > > Do you think this optimization is worth pursuing at this stage?
Let's start simple and optimize later. Jan
