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 <oleksii.kurochko@gmail.com>
---
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.
+     */
+}
+
+void p2m_handle_vmenter(void)
+{
+    struct vcpu *c = current;
+    struct p2m_domain *p2m = p2m_get_hostp2m(c->domain);
+    struct vcpu_vmid *p_vmid = &c->arch.vmid;
+    uint16_t old_vmid, new_vmid;
+    bool need_flush;
+
+    BUG_ON(is_idle_vcpu(current));
+
+    old_vmid = p_vmid->vmid;
+    need_flush = vmid_handle_vmenter(p_vmid);
+    new_vmid = p_vmid->vmid;
+
+#ifdef P2M_DEBUG
+    printk("%pv: oldvmid(%d) new_vmid(%d), need_flush(%d)\n",
+           c, old_vmid, new_vmid, need_flush);
+#endif
+
+    if ( old_vmid != new_vmid )
+        csr_write(CSR_HGATP, construct_hgatp(p2m, p_vmid->vmid));
+
+    if ( unlikely(need_flush) )
+    {
+        local_hfence_gvma_all();
+        flush_tlb_guest_local();
+    }
+
+    /*
+     * There is no need to set VSATP to 0 to stop speculation before updating
+     * HGATP, as VSATP is not modified here. There is also no need to flush
+     * the VS-stage TLB: even if speculation occurs, it will use the old VMID,
+     * which will not be reused until need_flush is set to true.
+     */
+}
diff --git a/xen/arch/riscv/traps.c b/xen/arch/riscv/traps.c
index ce8d346a14d2..7ef089809734 100644
--- a/xen/arch/riscv/traps.c
+++ b/xen/arch/riscv/traps.c
@@ -177,6 +177,8 @@ static void check_for_pcpu_work(void)
     vcpu_sync_interrupts(c);
 
     vcpu_flush_interrupts(c);
+
+    p2m_handle_vmenter();
 }
 
 static void timer_interrupt(void)
-- 
2.52.0

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 <oleksii.kurochko@gmail.com>
> ---
> 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?

~ Oleksii

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 <oleksii.kurochko@gmail.com>
>> ---
>> 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

On 13.02.2026 17:29, 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 <oleksii.kurochko@gmail.com>
> ---
> Changes in v3:
>  - Add comment above p2m_ctxt_switch_{to, from}().

I find these and other speculation related comments problematic: You can't
prevent every kind of speculation that way, yet all these comments are
written as if that was the case. What I think you mean in all cases is
speculation using the wrong set of page tables?

> --- 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)

What interaction with the storing of other CSRs would be problematic?

> +{
> +    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)

Same question here.

> +{
> +    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.
> +     */
> +}
> +
> +void p2m_handle_vmenter(void)
> +{
> +    struct vcpu *c = current;

Can you please stick to conventional names, i.e. "curr" here?

> +    struct p2m_domain *p2m = p2m_get_hostp2m(c->domain);
> +    struct vcpu_vmid *p_vmid = &c->arch.vmid;
> +    uint16_t old_vmid, new_vmid;

Nit: No real need for a fixed-width type here?

> +    bool need_flush;
> +
> +    BUG_ON(is_idle_vcpu(current));
> +
> +    old_vmid = p_vmid->vmid;
> +    need_flush = vmid_handle_vmenter(p_vmid);
> +    new_vmid = p_vmid->vmid;
> +
> +#ifdef P2M_DEBUG
> +    printk("%pv: oldvmid(%d) new_vmid(%d), need_flush(%d)\n",
> +           c, old_vmid, new_vmid, need_flush);
> +#endif
> +
> +    if ( old_vmid != new_vmid )
> +        csr_write(CSR_HGATP, construct_hgatp(p2m, p_vmid->vmid));
> +
> +    if ( unlikely(need_flush) )
> +    {
> +        local_hfence_gvma_all();
> +        flush_tlb_guest_local();
> +    }

Why would the latter be needed here at all? And if it was needed, why
would it depend on whether a VMID roll-over occurred?

Jan

On 2/16/26 12:50 PM, Jan Beulich wrote:
> On 13.02.2026 17:29, 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 <oleksii.kurochko@gmail.com>
>> ---
>> Changes in v3:
>>   - Add comment above p2m_ctxt_switch_{to, from}().
> I find these and other speculation related comments problematic: You can't
> prevent every kind of speculation that way, yet all these comments are
> written as if that was the case. What I think you mean in all cases is
> speculation using the wrong set of page tables?

According to the RISC-V spec:
   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

Based on that my understand is that the following code could provide an issue:
(1) csr_write(CSR_SEPC, guest_b->sepc);   ...   (2) csr_write(CSR_VSATP, 
0);   csr_write(CSR_HATP, guest_b->hgatp);   csr_write(CSR_VSATP, 
guest_b->vsatp); As IIUC speculation could happen between (1) and (2) 
and we could have some VS-stage translations connected to SEPC'c of 
guest B but with address from guest A page tables. So just to be sure 
that such isuse won't happen I wrote a comment that first VSATP, then 
others CSRs then setting hgatp and vsatp for new guest.


>
>> --- 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)
> What interaction with the storing of other CSRs would be problematic?

Please, look at the reply above.

>
>> +{
>> +    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)
> Same question here.
>
>> +{
>> +    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.
>> +     */
>> +}
>> +
>> +void p2m_handle_vmenter(void)
>> +{
>> +    struct vcpu *c = current;
> Can you please stick to conventional names, i.e. "curr" here?

Sure, I will do that. Also, then I have update in some other patches the name of similar
variable.

>
>> +    struct p2m_domain *p2m = p2m_get_hostp2m(c->domain);
>> +    struct vcpu_vmid *p_vmid = &c->arch.vmid;
>> +    uint16_t old_vmid, new_vmid;
> Nit: No real need for a fixed-width type here?

It could be just 'unsigned int' or 'unsigned short'.

>
>> +    bool need_flush;
>> +
>> +    BUG_ON(is_idle_vcpu(current));
>> +
>> +    old_vmid = p_vmid->vmid;
>> +    need_flush = vmid_handle_vmenter(p_vmid);
>> +    new_vmid = p_vmid->vmid;
>> +
>> +#ifdef P2M_DEBUG
>> +    printk("%pv: oldvmid(%d) new_vmid(%d), need_flush(%d)\n",
>> +           c, old_vmid, new_vmid, need_flush);
>> +#endif
>> +
>> +    if ( old_vmid != new_vmid )
>> +        csr_write(CSR_HGATP, construct_hgatp(p2m, p_vmid->vmid));
>> +
>> +    if ( unlikely(need_flush) )
>> +    {
>> +        local_hfence_gvma_all();
>> +        flush_tlb_guest_local();
>> +    }
> Why would the latter be needed here at all? And if it was needed, why
> would it depend on whether a VMID roll-over occurred?

I misread a spec for the case when implementation uses two TLBs: one that
maps guest virtual addresses to guest physical addresses, and another that
maps guest physical addresses to supervisor physical addresses, that it should
be HFENCE.VVMA, but it is written that HFENCE.GVMA.

So flush_tlb_guest_local() should be dropped here.

~ Oleksii

On 16.02.2026 16:34, Oleksii Kurochko wrote:
> On 2/16/26 12:50 PM, Jan Beulich wrote:
>> On 13.02.2026 17:29, 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 <oleksii.kurochko@gmail.com>
>>> ---
>>> Changes in v3:
>>>   - Add comment above p2m_ctxt_switch_{to, from}().
>> I find these and other speculation related comments problematic: You can't
>> prevent every kind of speculation that way, yet all these comments are
>> written as if that was the case. What I think you mean in all cases is
>> speculation using the wrong set of page tables?
> 
> According to the RISC-V spec:
>    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
> 
> Based on that my understand is that the following code could provide an issue:
> (1) csr_write(CSR_SEPC, guest_b->sepc);   ...   (2) csr_write(CSR_VSATP, 
> 0);   csr_write(CSR_HATP, guest_b->hgatp);   csr_write(CSR_VSATP, 
> guest_b->vsatp); As IIUC speculation could happen between (1) and (2) 
> and we could have some VS-stage translations connected to SEPC'c of 
> guest B but with address from guest A page tables. So just to be sure 
> that such isuse won't happen I wrote a comment that first VSATP, then 
> others CSRs then setting hgatp and vsatp for new guest.

This reply doesn't address the point raised above, it also ...

>>> --- 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)
>> What interaction with the storing of other CSRs would be problematic?
> 
> Please, look at the reply above.

... doesn't apply here, but ...

>>> +{
>>> +    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)
>> Same question here.

... it addresses this point.

Jan

On 2/16/26 4:45 PM, Jan Beulich wrote:
> On 16.02.2026 16:34, Oleksii Kurochko wrote:
>> On 2/16/26 12:50 PM, Jan Beulich wrote:
>>> On 13.02.2026 17:29, 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 <oleksii.kurochko@gmail.com>
>>>> ---
>>>> Changes in v3:
>>>>    - Add comment above p2m_ctxt_switch_{to, from}().
>>> I find these and other speculation related comments problematic: You can't
>>> prevent every kind of speculation that way, yet all these comments are
>>> written as if that was the case. What I think you mean in all cases is
>>> speculation using the wrong set of page tables?
>> According to the RISC-V spec:
>>     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
>>
>> Based on that my understand is that the following code could provide an issue:
>> (1) csr_write(CSR_SEPC, guest_b->sepc);   ...   (2) csr_write(CSR_VSATP,
>> 0);   csr_write(CSR_HATP, guest_b->hgatp);   csr_write(CSR_VSATP,
>> guest_b->vsatp); As IIUC speculation could happen between (1) and (2)
>> and we could have some VS-stage translations connected to SEPC'c of
>> guest B but with address from guest A page tables. So just to be sure
>> that such isuse won't happen I wrote a comment that first VSATP, then
>> others CSRs then setting hgatp and vsatp for new guest.
> This reply doesn't address the point raised above, it also ...

Okay, then just answering on a question above directly - yes, it is about the using of
the wrong set of page tables. (what I tried to describe in the example above)


>
>>>> --- 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)
>>> What interaction with the storing of other CSRs would be problematic?
>> Please, look at the reply above.
> ... doesn't apply here, but ...

Probably then I misunderstood your question.

Technically, it wouldn't problematic to write CSRs in any order (even we can ignore
setting of VSTAP to 0), but it will (at least, based on example above) lead to that
we will want to have TLBs flush.


~ Oleksii

>
>>>> +{
>>>> +    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)
>>> Same question here.
> ... it addresses this point.
>
> Jan

On 16.02.2026 16:57, Oleksii Kurochko wrote:
> On 2/16/26 4:45 PM, Jan Beulich wrote:
>> On 16.02.2026 16:34, Oleksii Kurochko wrote:
>>> On 2/16/26 12:50 PM, Jan Beulich wrote:
>>>> On 13.02.2026 17:29, 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 <oleksii.kurochko@gmail.com>
>>>>> ---
>>>>> Changes in v3:
>>>>>    - Add comment above p2m_ctxt_switch_{to, from}().
>>>> I find these and other speculation related comments problematic: You can't
>>>> prevent every kind of speculation that way, yet all these comments are
>>>> written as if that was the case. What I think you mean in all cases is
>>>> speculation using the wrong set of page tables?
>>> According to the RISC-V spec:
>>>     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
>>>
>>> Based on that my understand is that the following code could provide an issue:
>>> (1) csr_write(CSR_SEPC, guest_b->sepc);   ...   (2) csr_write(CSR_VSATP,
>>> 0);   csr_write(CSR_HATP, guest_b->hgatp);   csr_write(CSR_VSATP,
>>> guest_b->vsatp); As IIUC speculation could happen between (1) and (2)
>>> and we could have some VS-stage translations connected to SEPC'c of
>>> guest B but with address from guest A page tables. So just to be sure
>>> that such isuse won't happen I wrote a comment that first VSATP, then
>>> others CSRs then setting hgatp and vsatp for new guest.
>> This reply doesn't address the point raised above, it also ...
> 
> Okay, then just answering on a question above directly - yes, it is about the using of
> the wrong set of page tables.

Which is what I'm asking to clarify in those comments.

>>>>> --- 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)
>>>> What interaction with the storing of other CSRs would be problematic?
>>> Please, look at the reply above.
>> ... doesn't apply here, but ...
> 
> Probably then I misunderstood your question.
> 
> Technically, it wouldn't problematic to write CSRs in any order (even we can ignore
> setting of VSTAP to 0), but it will (at least, based on example above) lead to that
> we will want to have TLBs flush.

Why? Storing the CSRs doesn't alter the values held in those registers.
Taking your earlier example with EPC, storing it ahead of clearing VSATP
will leave as wide a window for speculation as will storing it afterwards.
In both cases the window will close when VSATP is cleared. As per my
current understanding, that is.

Jan

On 2/16/26 5:20 PM, Jan Beulich wrote:
> On 16.02.2026 16:57, Oleksii Kurochko wrote:
>> On 2/16/26 4:45 PM, Jan Beulich wrote:
>>> On 16.02.2026 16:34, Oleksii Kurochko wrote:
>>>> On 2/16/26 12:50 PM, Jan Beulich wrote:
>>>>> On 13.02.2026 17:29, 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 <oleksii.kurochko@gmail.com>
>>>>>> ---
>>>>>> Changes in v3:
>>>>>>     - Add comment above p2m_ctxt_switch_{to, from}().
>>>>> I find these and other speculation related comments problematic: You can't
>>>>> prevent every kind of speculation that way, yet all these comments are
>>>>> written as if that was the case. What I think you mean in all cases is
>>>>> speculation using the wrong set of page tables?
>>>> According to the RISC-V spec:
>>>>      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
>>>>
>>>> Based on that my understand is that the following code could provide an issue:
>>>> (1) csr_write(CSR_SEPC, guest_b->sepc);   ...   (2) csr_write(CSR_VSATP,
>>>> 0);   csr_write(CSR_HATP, guest_b->hgatp);   csr_write(CSR_VSATP,
>>>> guest_b->vsatp); As IIUC speculation could happen between (1) and (2)
>>>> and we could have some VS-stage translations connected to SEPC'c of
>>>> guest B but with address from guest A page tables. So just to be sure
>>>> that such isuse won't happen I wrote a comment that first VSATP, then
>>>> others CSRs then setting hgatp and vsatp for new guest.
>>> This reply doesn't address the point raised above, it also ...
>> Okay, then just answering on a question above directly - yes, it is about the using of
>> the wrong set of page tables.
> Which is what I'm asking to clarify in those comments.
>
>>>>>> --- 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)
>>>>> What interaction with the storing of other CSRs would be problematic?
>>>> Please, look at the reply above.
>>> ... doesn't apply here, but ...
>> Probably then I misunderstood your question.
>>
>> Technically, it wouldn't problematic to write CSRs in any order (even we can ignore
>> setting of VSTAP to 0), but it will (at least, based on example above) lead to that
>> we will want to have TLBs flush.
> Why? Storing the CSRs doesn't alter the values held in those registers.
> Taking your earlier example with EPC, storing it ahead of clearing VSATP
> will leave as wide a window for speculation as will storing it afterwards.
> In both cases the window will close when VSATP is cleared. As per my
> current understanding, that is.

Oh, you are right. It doesn't really matter when clearing of VSATP is happening during
CSR's storing procedure as CSRs aren't changed, so it is just necessary to set VSATP
to 0 before CSR's restoring.

I just confused myself with my own example as it is basically about restoring but
I missed that I am commenting CSR's storing procedure...

Thanks.

~ Oleksii