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

Oleksii Kurochko posted 2 patches 5 months ago
[PATCH v2 2/2] xen/riscv: add p2m context switch handling for VSATP and HGATP
Posted by Oleksii Kurochko 5 months ago
Add helpers to safely update VS-stage and G-stage translation registers
during vCPU context switches.

Because VSATP and HGATP cannot be updated atomically, VSATP is cleared on
switch-out to prevent speculative VS-stage translations being cached under
an incorrect VMID. On VM entry, HGATP is reconstructed, VMID handling is
performed, and VSATP is restored.

This provides the necessary infrastructure for correct p2m context
switching on RISC-V.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in v2:
 - Add vsatp field declaration to arch_vcpu.
 - s/p2m_ctx_switch_{from,to}/p2m_ctxt_switch_{from,to}.
 - Introduce p2m_handle_vmenter() for proper handling of VMID,
   hgatp and vsatp updates.
 - Introduce is_p2m_switch_finished and init it inisde
   p2m_ctx_switch_to() for furhter handling in p2m_handle_vmenter().
 - Code style fixes.
 - Add is_idle_vcpu() check in p2m_ctxt_switch_from().
 - use csr_swap() in p2m_ctxt_switch_from().
 - move flush_tlb_guest_local() to the end if p2m_handle_vmenter() and
   drop unnessary anymore comments.
 - Correct printk()'s arguments in p2m_handle_vmenter().
---
 xen/arch/riscv/include/asm/domain.h |   1 +
 xen/arch/riscv/include/asm/p2m.h    |  11 +++
 xen/arch/riscv/p2m.c                | 123 ++++++++++++++++++++++++++++
 xen/arch/riscv/traps.c              |   2 +
 4 files changed, 137 insertions(+)

diff --git a/xen/arch/riscv/include/asm/domain.h b/xen/arch/riscv/include/asm/domain.h
index b5a8a9f711ac..c029e50b96fe 100644
--- a/xen/arch/riscv/include/asm/domain.h
+++ b/xen/arch/riscv/include/asm/domain.h
@@ -59,6 +59,7 @@ struct arch_vcpu {
     register_t hstateen0;
     register_t hvip;
 
+    register_t vsatp;
     register_t vsie;
 
     /*
diff --git a/xen/arch/riscv/include/asm/p2m.h b/xen/arch/riscv/include/asm/p2m.h
index f63b5dec99b1..0cdd3dc44683 100644
--- a/xen/arch/riscv/include/asm/p2m.h
+++ b/xen/arch/riscv/include/asm/p2m.h
@@ -90,6 +90,13 @@ struct p2m_domain {
      */
     bool clean_dcache;
 
+    /*
+     * Inidicate that context switch is fully finished. It is needed to
+     * detect in p2m_handle_vmenter() to undestand what to write to
+     * CSR_VSATP register.
+     */
+    bool is_ctxt_switch_finished;
+
     /* Highest guest frame that's ever been mapped in the p2m */
     gfn_t max_mapped_gfn;
 
@@ -255,6 +262,10 @@ static inline bool p2m_is_locked(const struct p2m_domain *p2m)
 struct page_info *p2m_get_page_from_gfn(struct p2m_domain *p2m, gfn_t gfn,
                                         p2m_type_t *t);
 
+void p2m_ctxt_switch_from(struct vcpu *p);
+void p2m_ctxt_switch_to(struct vcpu *n);
+void p2m_handle_vmenter(void);
+
 #endif /* ASM__RISCV__P2M_H */
 
 /*
diff --git a/xen/arch/riscv/p2m.c b/xen/arch/riscv/p2m.c
index 0abeb374c110..275c38020ae2 100644
--- 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;
+
+    /*
+     * Nothing to do with HGATP or VSATP, they will be set in
+     * p2_handle_vmenter()
+     */
+}
+
+void p2m_handle_vmenter(void)
+{
+    struct p2m_domain *p2m = &current->domain->arch.p2m;
+    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));
+
+    /*
+     * 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);
+
+    old_vmid = p_vmid->vmid;
+    need_flush = vmid_handle_vmenter(p_vmid);
+    new_vmid = p_vmid->vmid;
+
+#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));
+
+    if ( unlikely(need_flush) )
+        local_hfence_gvma_all();
+
+    if ( p2m->is_ctxt_switch_finished )
+        csr_swap(CSR_VSATP, vsatp_old);
+        /*
+         * We are not coming from a context switch here, so the VSATP value is
+         * the same as it was before csr_swap() was executed at the start of
+         * this function. Since VSATP was set to 0, no speculation could occur,
+         * and the VS-stage TLB cannot be polluted.
+         * Therefore, no additional VS TLB flush is required.
+         */
+    else
+    {
+        vsatp_old = csr_swap(CSR_VSATP, current->arch.vsatp);
+
+        /*
+         * vsatp_old should be zero as in the case of context switch it was
+         * set to 0 in p2m_ctxt_switch_from().
+         */
+        BUG_ON(vsatp_old);
+
+        p2m->is_ctxt_switch_finished = true;
+
+        /*
+         * TODO: further investigation is needed here.
+         *
+         *       In my opinion, a VS-stage TLB flush is not always strictly
+         *       necessary.
+         *       If a context switch occurs and VSATP is set to 0 before any
+         *       context-switch-related operations begin, no speculation can
+         *       occur. Therefore, at the time this function executes, the
+         *       VS-stage TLB should not be polluted with incorrect entries
+         *       belonging to a previously running vCPU. Another one reason is
+         *       that about SATP register is mentioned the following in RISC-V
+         *       spec:
+         *         Changing satp.MODE from Bare to other modes and vice versa
+         *         also takes effect immediately, without the need to execute
+         *         an SFENCE.VMA instruction. Likewise, changes to satp.ASID
+         *         take effect immediately.
+         *       I expect the same for VSATP (as it is VS copy of SATP) but it
+         *       isn't mentioned explicitly in the spec.
+         *
+         *       The only case where a VS-stage TLB flush seems necessary is
+         *       when the VASID remains unchanged but VSATP is updated to point
+         *       to a different VS page table. In that case, flushing
+         *       guarantees that the guest observes a clean context switch
+         *       without any possibility of using stale TLB entries.
+         */
+        flush_tlb_guest_local();
+    }
+}
diff --git a/xen/arch/riscv/traps.c b/xen/arch/riscv/traps.c
index e8d9ca902d9c..d6543eac1390 100644
--- a/xen/arch/riscv/traps.c
+++ b/xen/arch/riscv/traps.c
@@ -175,6 +175,8 @@ static void check_for_pcpu_work(void)
     vcpu_sync_interrupts(current);
 
     vcpu_flush_interrupts(current);
+
+    p2m_handle_vmenter();
 }
 
 static void timer_interrupt(void)
-- 
2.52.0


Re: [PATCH v2 2/2] xen/riscv: add p2m context switch handling for VSATP and HGATP
Posted by Jan Beulich 5 months ago
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 = &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.

> +#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

Re: [PATCH v2 2/2] xen/riscv: add p2m context switch handling for VSATP and HGATP
Posted by Oleksii Kurochko 5 months ago
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.

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

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.

>
>> +#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().

Oh, sure, I will update that.

Thanks.

~ Oleksii


Re: [PATCH v2 2/2] xen/riscv: add p2m context switch handling for VSATP and HGATP
Posted by Jan Beulich 5 months ago
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

Re: [PATCH v2 2/2] xen/riscv: add p2m context switch handling for VSATP and HGATP
Posted by Oleksii Kurochko 5 months ago
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. 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.

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

Do you mean to do that only in p2m_handle_vmenter() and having
p2m_ctxt_switch_to() does nothing (or just dropped)?
If yes then it could be an option.

I thought about such approach but decided to go with an introduction
of is_ctxt_switch_finished as it looked to me that it is better for
potential TLB flushing optimization.
But I am okay to start with suggested way and re-work it once a
performance issue will be actually determined.

~ Oleksii


Re: [PATCH v2 2/2] xen/riscv: add p2m context switch handling for VSATP and HGATP
Posted by Jan Beulich 5 months ago
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?

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

Jan

Re: [PATCH v2 2/2] xen/riscv: add p2m context switch handling for VSATP and HGATP
Posted by Oleksii Kurochko 5 months ago
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


Re: [PATCH v2 2/2] xen/riscv: add p2m context switch handling for VSATP and HGATP
Posted by Jan Beulich 5 months ago
On 12.02.2026 17:16, Oleksii Kurochko wrote:
> 
> 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:

So just to clarify from my side that I don't have any expectations; I
merely want to suggest to start with as simple a model as possible,
for its correctness to be easy to prove.

Jan

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


Re: [PATCH v2 2/2] xen/riscv: add p2m context switch handling for VSATP and HGATP
Posted by Oleksii Kurochko 5 months ago
On 2/12/26 5:20 PM, Jan Beulich wrote:
> On 12.02.2026 17:16, Oleksii Kurochko wrote:
>> 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:
> So just to clarify from my side that I don't have any expectations; I
> merely want to suggest to start with as simple a model as possible,
> for its correctness to be easy to prove.

Got you. Then it seems like the easiest approach is to follow what is in this
patch until ...

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

... we could find a better way to detect if context switch happened before entering
VM entry path or not without introduction of its ugly bool variable
is_ctxt_switch_finished as I mentioned above it seems to me it is not enough just to
check if VSATP is zero or not as guest could had VSATP equal to 0 before trap.

That is why the approach suggested earlier:
   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.

as it isn't clear with what should be updated vsatp (vsatp only needs writing when vsatp.MODE is zero)
with what it was written before we were in a guest or with what ve saved in vcpu->arch.vsatp during
context switch.

Am I missing something obvious?

Thanks.

~ Oleksii