Introduce functions required to perform a p2m context switch during a vCPU context switch. This patch series originally depended on a single patch (Patch 1) from [1]. To avoid introducing a dependency on the larger patch series [1], that patch was cherry-picked into the current series, since it is the only true dependency. Patch 2 is fully independent. Patch 3 depends on Patch 1. [1] https://lore.kernel.org/xen-devel/cover.1769099883.git.oleksii.kurochko@gmail.com/T/#t Oleksii Kurochko (3): xen/riscv: introduce struct arch_vcpu xen/riscv: add support for local guest TLB flush using HFENCE.VVMA xen/riscv: implement p2m_ctx_switch_{to,from}_state() xen/arch/riscv/include/asm/domain.h | 49 ++++++++++++++++ xen/arch/riscv/include/asm/flushtlb.h | 7 +++ xen/arch/riscv/include/asm/insn-defs.h | 10 ++++ xen/arch/riscv/include/asm/p2m.h | 4 ++ xen/arch/riscv/p2m.c | 81 ++++++++++++++++++++++++++ 5 files changed, 151 insertions(+) create mode 100644 xen/arch/riscv/include/asm/insn-defs.h -- 2.52.0 Introdce struct arch_vcpu to hold RISC-V vCPU-specific state. The structure contains: - Guest-visible CSR state, used to save and restore vCPU execution state across context switches. hstatus isn't added here as it is already part of cpu_user_regs struct. - Callee-saved registers used to preserve Xen’s own execution context when switching between vCPU stacks. It is going to be used in the following way (pseudocode): context_switch(prev_vcpu, next_vcpu): ... /* Switch from previous stack to the next stack. */ __context_switch(prev_vcpu, next_vcpu); ... schedule_tail(prev_vcpu): Save and restore vCPU's CSRs. The Xen-saved context allows __context_switch() to switch execution from the previous vCPU’s stack to the next vCPU’s stack and later resume execution on the original stack when switching back. During vCPU creation, the Xen-saved context is going to be initialized with: - SP pointing to the newly allocated vCPU stack - RA pointing to a helper that performs final vCPU setup before transferring control to the guest After the first execution of __context_switch(), RA naturally points to the instruction following the call site, and the remaining callee-saved registers contain the Xen register state at the time of the switch. Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> --- Changes in V3: - Drop gp from struct {...} xen_saved_context as it ought to be stable accross Xen so there is no need to be saved/restored and, also, it shouldn't be preserved across calls according to RISC-V ABI. - Update the comment above struct {...} xen_saved_context to make it more clear. - Drop CSRs and VCSRS comments in arch_vcpu as it is clear what kind of CSR it is based on the name. - Drop __cacheline_aligned for struct arch_vcpu as proper measurements can't be made now so it is hard to prove that the attribute really boost performance. --- Changes in v2: - Drop hstatus from struct arch_vcpu as it is stored in struct cpu_user_regs which will be stored on top of vCPU's stack. - Drop the comment above ra in xen_saved_context struct as it is potentially misleading. --- xen/arch/riscv/include/asm/domain.h | 49 +++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/xen/arch/riscv/include/asm/domain.h b/xen/arch/riscv/include/asm/domain.h index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/riscv/include/asm/domain.h +++ b/xen/arch/riscv/include/asm/domain.h @@ -XXX,XX +XXX,XX @@ struct arch_vcpu_io { struct arch_vcpu { struct vcpu_vmid vmid; + + /* + * Callee saved registers for Xen's state used to switch from + * prev's stack to the next's stack during context switch. + */ + struct + { + register_t s0; + register_t s1; + register_t s2; + register_t s3; + register_t s4; + register_t s5; + register_t s6; + register_t s7; + register_t s8; + register_t s9; + register_t s10; + register_t s11; + register_t sp; + register_t ra; + } xen_saved_context; + + register_t hedeleg; + register_t hideleg; + register_t hvip; + register_t hip; + register_t hie; + register_t hgeie; + register_t henvcfg; + register_t hcounteren; + register_t htimedelta; + register_t htval; + register_t htinst; + register_t hstateen0; +#ifdef CONFIG_RISCV_32 + register_t henvcfgh; + register_t htimedeltah; +#endif + + register_t vsstatus; + register_t vsip; + register_t vsie; + register_t vstvec; + register_t vsscratch; + register_t vscause; + register_t vstval; + register_t vsatp; + register_t vsepc; }; struct paging_domain { -- 2.52.0 Introduce flush_tlb_guest_local() to perform a local TLB flush of the guest's address space for the current hart. This leverages the RISC-V HFENCE.VVMA instruction, which is used to invalidate translations in the VS-stage of address translation. As for RISC-V binutils >= 2.39 is choosen, we can use hfence.vvma mnemonics instead of defining hfence.vvma using .insn. Although it would be possible to use sbi_remote_hfence_vvma() for this purpose, it is unnecessary in this context since the flush is required only on the local hart. Using the SBI call would introduce additional overhead without benefit, resulting in unnecessary performance loss. Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> --- xen/arch/riscv/include/asm/flushtlb.h | 7 +++++++ xen/arch/riscv/include/asm/insn-defs.h | 10 ++++++++++ 2 files changed, 17 insertions(+) create mode 100644 xen/arch/riscv/include/asm/insn-defs.h diff --git a/xen/arch/riscv/include/asm/flushtlb.h b/xen/arch/riscv/include/asm/flushtlb.h index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/riscv/include/asm/flushtlb.h +++ b/xen/arch/riscv/include/asm/flushtlb.h @@ -XXX,XX +XXX,XX @@ #include <xen/bug.h> #include <xen/cpumask.h> +#include <asm/insn-defs.h> #include <asm/sbi.h> struct page_info; @@ -XXX,XX +XXX,XX @@ static inline void local_hfence_gvma_all(void) asm volatile ( "hfence.gvma zero, zero" ::: "memory" ); } +/* Flush VS-stage TLB for current hart. */ +static inline void flush_tlb_guest_local(void) +{ + HFENCE_VVMA(0, 0); +} + /* Flush TLB of local processor for address va. */ static inline void flush_tlb_one_local(vaddr_t va) { diff --git a/xen/arch/riscv/include/asm/insn-defs.h b/xen/arch/riscv/include/asm/insn-defs.h new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/xen/arch/riscv/include/asm/insn-defs.h @@ -XXX,XX +XXX,XX @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef ASM_RISCV_INSN_DEFS_H +#define ASM_RISCV_INSN_DEFS_H + +#define HFENCE_VVMA(vaddr, asid) \ + asm volatile ("hfence.vvma %0, %1" \ + :: "r"(vaddr), "r"(asid) : "memory") + +#endif /* ASM_RISCV_INSN_DEFS_H */ -- 2.52.0 Introduce functions required to perform a p2m context switch during a vCPU context switch. As 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 in p2m_ctx_swith_from(), then construct new hgatp and write the new vsatp value in p2m_ctx_switch_to(). Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> --- xen/arch/riscv/include/asm/p2m.h | 4 ++ xen/arch/riscv/p2m.c | 81 ++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+) diff --git a/xen/arch/riscv/include/asm/p2m.h b/xen/arch/riscv/include/asm/p2m.h index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/riscv/include/asm/p2m.h +++ b/xen/arch/riscv/include/asm/p2m.h @@ -XXX,XX +XXX,XX @@ 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_ctx_switch_from(struct vcpu *p); +void p2m_ctx_switch_to(struct vcpu *n); + #endif /* ASM__RISCV__P2M_H */ /* diff --git a/xen/arch/riscv/p2m.c b/xen/arch/riscv/p2m.c index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/riscv/p2m.c +++ b/xen/arch/riscv/p2m.c @@ -XXX,XX +XXX,XX @@ 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_ctx_switch_from(struct vcpu *p) +{ + /* + * 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. + */ + p->arch.vsatp = csr_read(CSR_VSATP); + csr_write(CSR_VSATP, 0); + + /* + * No need for VS-stage TLB flush here: + * Changing satp.MODE from Bare to other modes and vice versa also + * takes effect immediately, without the need to execute an + * SFENCE.VMA instruction. + * Note that VSATP is just VS-mode’s version of SATP, so the mentioned + * above should be true for VSATP. + */ + + /* + * Nothing to do with HGATP as it is constructed each time when + * p2m_ctx_switch_to() is called. + */ +} + +void p2m_ctx_switch_to(struct vcpu *n) +{ + struct vcpu_vmid *p_vmid = &n->arch.vmid; + uint16_t old_vmid, new_vmid; + bool need_flush; + + if ( is_idle_vcpu(n) ) + return; + + old_vmid = p_vmid->vmid; + need_flush = vmid_handle_vmenter(p_vmid); + new_vmid = p_vmid->vmid; + +#ifdef P2M_DEBUG + printk(XENLOG_INFO, "%pv: oldvmid(%d) new_vmid(%d), need_flush(%d)\n", + n, 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(); + + /* + * According to the RISC-V specification, speculation can happen + * during an update of hgatp and vsatp: + * 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. Similarly, if henvcfg.PBMTE + * need be world-switched, it should be switched after zeroing vsatp + * but before writing the new vsatp value, obviating the need to + * execute an HFENCE.VVMA instruction. + * So just flush TLBs for VS-Stage and G-stage after both of regs are + * touched. + */ + flush_tlb_guest_local(); + + /* + * The vsatp register is a VSXLEN-bit read/write register that is + * VS-mode’s version of supervisor register satp, so the following is + * true for VSATP registers: + * 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. + * Considering the mentioned above and that VS-stage TLB flush has been + * already done there is no need to flush VS-stage TLB after an update + * of VSATP from Bare mode to what is written in `n->arch.vsatp`. + */ + csr_write(CSR_VSATP, n->arch.vsatp); +} -- 2.52.0 Introduce functions required to perform a p2m context switch during a vCPU context switch. Although this patch series is technically independent of [1], some merge conflicts are still possible depending on which series is applied first. For now, the series has been rebased on top of [1] but can go without [1]. [1] https://lore.kernel.org/xen-devel/cover.1770650552.git.oleksii.kurochko@gmail.com/T/#m5d8de24e5ee0eaf6f35f9f1437a00096e3574684 --- Changes in v2: - Drop patch "xen/riscv: introduce struct arch_vcpu", took only declaration of vsatp and squash it with the patch: xen/riscv: implement p2m_ctx_switch_{to,from}_state() - Other changes please look at specific patch. - CI tests: https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/2317277592 --- Oleksii Kurochko (2): xen/riscv: add support for local guest TLB flush using HFENCE.VVMA xen/riscv: add p2m context switch handling for VSATP and HGATP xen/arch/riscv/include/asm/domain.h | 1 + xen/arch/riscv/include/asm/flushtlb.h | 7 ++ xen/arch/riscv/include/asm/insn-defs.h | 10 ++ xen/arch/riscv/include/asm/p2m.h | 11 +++ xen/arch/riscv/p2m.c | 123 +++++++++++++++++++++++++ xen/arch/riscv/traps.c | 2 + 6 files changed, 154 insertions(+) create mode 100644 xen/arch/riscv/include/asm/insn-defs.h -- 2.52.0 Introduce flush_tlb_guest_local() to perform a local TLB flush of the guest's address space for the current hart. This leverages the RISC-V HFENCE.VVMA instruction, which is used to invalidate translations in the VS-stage of address translation. As for RISC-V binutils >= 2.39 is choosen, we can use hfence.vvma mnemonics instead of defining hfence.vvma using .insn. Although it would be possible to use sbi_remote_hfence_vvma() for this purpose, it is unnecessary in this context since the flush is required only on the local hart. Using the SBI call would introduce additional overhead without benefit, resulting in unnecessary performance loss. Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> --- Changes in v2: - Add missed blanks in asm(). - Add operand modifier "z" and "J" constraint to be sure that zero register will be used when 0 is passed to HFENCE_VVMA(). --- xen/arch/riscv/include/asm/flushtlb.h | 7 +++++++ xen/arch/riscv/include/asm/insn-defs.h | 10 ++++++++++ 2 files changed, 17 insertions(+) create mode 100644 xen/arch/riscv/include/asm/insn-defs.h diff --git a/xen/arch/riscv/include/asm/flushtlb.h b/xen/arch/riscv/include/asm/flushtlb.h index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/riscv/include/asm/flushtlb.h +++ b/xen/arch/riscv/include/asm/flushtlb.h @@ -XXX,XX +XXX,XX @@ #include <xen/bug.h> #include <xen/cpumask.h> +#include <asm/insn-defs.h> #include <asm/sbi.h> struct page_info; @@ -XXX,XX +XXX,XX @@ static inline void local_hfence_gvma_all(void) asm volatile ( "hfence.gvma zero, zero" ::: "memory" ); } +/* Flush VS-stage TLB for current hart. */ +static inline void flush_tlb_guest_local(void) +{ + HFENCE_VVMA(0, 0); +} + /* Flush TLB of local processor for address va. */ static inline void flush_tlb_one_local(vaddr_t va) { diff --git a/xen/arch/riscv/include/asm/insn-defs.h b/xen/arch/riscv/include/asm/insn-defs.h new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/xen/arch/riscv/include/asm/insn-defs.h @@ -XXX,XX +XXX,XX @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef ASM_RISCV_INSN_DEFS_H +#define ASM_RISCV_INSN_DEFS_H + +#define HFENCE_VVMA(vaddr, asid) \ + asm volatile ( "hfence.vvma %z0, %z1" \ + :: "rJ" (vaddr), "rJ" (asid) : "memory" ) + +#endif /* ASM_RISCV_INSN_DEFS_H */ -- 2.52.0 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 XXXXXXX..XXXXXXX 100644 --- a/xen/arch/riscv/include/asm/domain.h +++ b/xen/arch/riscv/include/asm/domain.h @@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644 --- a/xen/arch/riscv/include/asm/p2m.h +++ b/xen/arch/riscv/include/asm/p2m.h @@ -XXX,XX +XXX,XX @@ 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; @@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644 --- a/xen/arch/riscv/p2m.c +++ b/xen/arch/riscv/p2m.c @@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644 --- a/xen/arch/riscv/traps.c +++ b/xen/arch/riscv/traps.c @@ -XXX,XX +XXX,XX @@ 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