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

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

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