Re: [PATCH v3] xen/riscv: add p2m context switch handling for VSATP and HGATP

[Jan Beulich <jbeulich@xxxxxxxx>]

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@xxxxxxxxx>
>> ---
>> 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