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?
> + /*
> + * Nothing to do with HGATP or VSATP, they will be set in
> + * p2_handle_vmenter()
> + */
Why can this not be done here?
> +}
> +
> +void p2m_handle_vmenter(void)
> +{
> + struct p2m_domain *p2m = ¤t->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 = ¤t->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.
> +#ifdef P2M_DEBUG
> + printk("%pv: oldvmid(%d) new_vmid(%d), need_flush(%d)\n",
> + current, old_vmid, new_vmid, need_flush);
> +#endif
> +
> + csr_write(CSR_HGATP, construct_hgatp(p2m_get_hostp2m(current->domain),
> + new_vmid));
Bad indentation - new_vmid isn't an argument to csr_write().
Jan
