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

[Jan Beulich <jbeulich@xxxxxxxx>]

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