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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 12.02.2026 12:57, Oleksii Kurochko wrote:
> On 2/12/26 11:16 AM, Jan Beulich wrote:
>> On 10.02.2026 17:36, Oleksii Kurochko wrote:
>>> --- a/xen/arch/riscv/p2m.c
>>> +++ b/xen/arch/riscv/p2m.c
>>> @@ -1434,3 +1434,126 @@ struct page_info *p2m_get_page_from_gfn(struct 
>>> p2m_domain *p2m, gfn_t gfn,
>>>   
>>>       return get_page(page, p2m->domain) ? page : NULL;
>>>   }
>>> +
>>> +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 is constructed each time when
>>> +     * p2m_handle_vmenter() is called.
>>> +     */
>>> +}
>>> +
>>> +void p2m_ctxt_switch_to(struct vcpu *n)
>>> +{
>>> +    if ( is_idle_vcpu(n) )
>>> +        return;
>>> +
>>> +    n->domain->arch.p2m.is_ctxt_switch_finished = false;
>> How can the context switch of a vCPU affect domain-wide state?
> 
> It is wrong to have is_ctxt_switch_finished per domain, it should be
> vCPU field.
> 
>>
>>> +    /*
>>> +     * Nothing to do with HGATP or VSATP, they will be set in
>>> +     * p2_handle_vmenter()
>>> +     */
>> Why can this not be done here?
> 
> As VMID should be calculated on VM enter.

And I didn't suggest to calculate a new one here.

> We can update HGATP and VSATP here with VMID stored before in 
> p2m_ctxt_switch_from(),
> but then it is possible when vmid_handle_vmenter() will be called before VM 
> entry
> VMID could be changed and it will be needed again to update HGATP and VSATP 
> what
> will lead to flushing of VS TLB twice (one in p2m_ctxt_switch_to() and 
> another one
> in p2m_handle_vmenter()).

Is this a concern resulting from particular logic you expect to appear
in the window between context switch and entering the guest, or is this
merely an abstract concern?

> This is also an answer to ...
> 
>>
>>> +}
>>> +
>>> +void p2m_handle_vmenter(void)
>>> +{
>>> +    struct p2m_domain *p2m = &current->domain->arch.p2m;
>> To save yourself (or others) future work, please never open-code 
>> p2m_get_hostp2m()
>> (applies further up as well, as I notice only now).
>>
>>> +    struct vcpu_vmid *p_vmid = &current->arch.vmid;
>>> +    uint16_t old_vmid, new_vmid;
>>> +    bool need_flush;
>>> +    register_t vsatp_old = 0;
>>> +
>>> +    BUG_ON(is_idle_vcpu(current));
>> This is the 3rd use of current - latch into a local variable?
>>
>>> +    /*
>>> +     * 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
>>> +     *
>>> +     * CSR_VSATP is already set to 0 in p2m_ctxt_switch_from() in the
>>> +     * case when n->arch.is_p2m_switch_finished = false. Also, there is
>>> +     * BUG_ON() below to verify that.
>>> +     */
>>> +    if ( p2m->is_ctxt_switch_finished )
>>> +        vsatp_old = csr_swap(CSR_VSATP, 0);
>> This shouldn't be needed when ...
>>
>>> +    old_vmid = p_vmid->vmid;
>>> +    need_flush = vmid_handle_vmenter(p_vmid);
>>> +    new_vmid = p_vmid->vmid;
>> ... the VMID doesn't change. Imo you want to drop is_ctxt_switch_finished
>> again, handle things normally in p2m_ctxt_switch_to(), and deal with merely
>> a changing VMID here.
> 
> ... (check the answer above)
> 
> If it is okay to have potential double VS TLB flush and double update of
> HGATP and VSATP when old_vmid != new_vmid then we can do in this way.

I think the simpler, straightforward approach should be used initially,
with improvements made once a performance issue was actually determined, or
once a less ugly (sorry) approach was found. For example, assuming CSR
reads aren't overly expensive, it looks to me as if during VM entry
- vsatp only needs writing when vsatp.MODE is zero,
- hgatp only needs writing when vsatp.MODE is zero or when the VMID needs
  updating.

Jan