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

[Jan Beulich <jbeulich@xxxxxxxx>]

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