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

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 2/12/26 4:42 PM, Jan Beulich wrote:
> On 12.02.2026 15:47, Oleksii Kurochko wrote:
> > On 2/12/26 1:56 PM, Jan Beulich wrote:
> > > 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?
> > If we will have VS TLB flush unconditionally in VM entry then it is merely an
> > abstract concern.
> Why would we want to flush unconditionally?
To guarantee that a guest sees a clean switch with no possibilities of 
using a stale entry. For example, if VMID changed between context switch 
and VM entry we want to have flush, but considering your reply here ...
> > Otherwise, considering that speculation could happen between
> > context switch and VM entry what could lead to that some entries were added to
> > VS TLB flush with old VMID in the case if then in VM entry vCPU might receive 
> > new
> > VMID.
> I don't understand: Context switch leaves vsatp.MODE at zero. Nothing can end
> up in the VS TLB in that case, aiui
... we just have different implementation in mind for p2m_ctxt_switch_to().

I thought that your suggestion is to set both HGATP and VSATP in 
p2m_ctx_switch_to()
while calculate VMID in p2m_handle_vmenter() (with potential update of HGATP 
and VSATP
if needed) and with such approach VSATP won't be zero after p2m_ctx_switch_to() 
and
speculation could happen between context switch and VM entry.

So just to clarify your expectations are:
1. p2m_ctxt_switch_from(p):
      p.vsatp = VSATP
      VSATP = 0

2. p2m_ctxt_swith_to(n):
      HGATP = construct_hgatp(...)

3. p2m_handle_vmenter(n):
      update VMID if necessary
      
      recalculate HGATP if necessary
      
      (c) update VSATP with n.VSATP if we here from context switch
          or with hardware VSATP if it wasn't context switch.
       
      do necessary flushes
And at step (c) we can't base on that if VSATP is zero or not to understand that
it is from context switch as it could that guest at the moment of trap (lets say
some SBI call was requested by guest and Xen just handles it and return back
to guest) also had VSTAP = 0.
So it is needed to distinguish if context switch happened or not to properly 
deal
with VSATP (and it was one of the reson to have introduced in this patch
is_ctxt_switch_finished).

~ Oleksii