Re: [PATCH v1 15/17] xen/riscv: implement trap redirection to a guest

[Jan Beulich <jbeulich@xxxxxxxx>]

On 20.07.2026 18:02, Oleksii Kurochko wrote:
> Some traps taken by Xen on behalf of a guest can't or shouldn't be
> handled by the hypervisor and must be forwarded to the guest's own
> S-mode exception handler instead: e.g. when riscv_vcpu_unpriv_read()
> faults while accessing guest memory, or when emulation hits a condition
> only the guest kernel can resolve.

Is the plan to use riscv_vcpu_unpriv_read() also for reading hypercall
buffers? In that case trap redirection shouldn't come into play.

> Introduce riscv_vcpu_trap_redirect() for that purpose. It makes the
> trap appear to the guest as if it had been taken directly in VS-mode:
> the trap information is transferred to the guest's virtual supervisor
> CSRs and the vCPU is resumed at its exception vector in supervisor
> mode, following the trap entry rules of the RISC-V privileged
> specification.
> 
> The implementation is based on kvm_riscv_vcpu_trap_redirect() from
> Linux, with a few deviations:
>  - The function reads and writes physical VS-mode CSRs, so it is only
>    meaningful for the currently running vCPU. Instead of taking a
>    struct vcpu argument, it always operates on current.
>  - The MODE field of vstvec is masked off explicitly when computing the
>    exception target PC (exceptions always vector to BASE), rather than
>    relying on the hardwired zero bit of sepc to drop it on VM entry.
>  - Assertions document the preconditions: the trap must have been taken
>    from virtualized mode (hstatus.SPV set), and only synchronous
>    exceptions may be redirected - interrupts must be injected via hvip
>    instead, so that the hardware performs VS-mode trap entry itself,
>    respecting vsstatus.SIE and vectored vstvec dispatch.

For this last bullet point - how is a reviewer supposed to validate the
assertions added when no caller of the new function exists?

> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> ---
>  xen/arch/riscv/guestcopy.c                | 54 +++++++++++++++++++++++
>  xen/arch/riscv/include/asm/guest_access.h |  2 +
>  2 files changed, 56 insertions(+)

I don't understand this placement - trap redirection has nothing
(directly) to do with accessing guest memory.

> --- a/xen/arch/riscv/guestcopy.c
> +++ b/xen/arch/riscv/guestcopy.c
> @@ -205,3 +205,57 @@ unsigned long riscv_vcpu_unpriv_read(bool read_insn,
>  
>      return val;
>  }
> +
> +/* Redirect trap to Guest. */
> +void riscv_vcpu_trap_redirect(const struct trap_info *trap)
> +{
> +    struct cpu_user_regs *regs = vcpu_guest_cpu_user_regs(current);
> +    unsigned long vsstatus = csr_read(CSR_VSSTATUS);
> +
> +    /*
> +     * Redirecting a trap makes sense only if the trap was taken from
> +     * virtualized mode, i.e. sret is going to return to VS-mode.
> +     */
> +    ASSERT(regs->hstatus & HSTATUS_SPV);
> +
> +    /*
> +     * Only synchronous exceptions can be redirected. Interrupts must be
> +     * injected via hvip instead, so that the hardware itself performs
> +     * VS-mode trap entry, respecting vsstatus.SIE and the vectored
> +     * dispatch (BASE + 4 * cause) if vstvec is configured so.
> +     */
> +    ASSERT(!(trap->scause & CAUSE_IRQ_FLAG));
> +
> +    /* Change Guest SSTATUS.SPP bit */
> +    vsstatus &= ~SSTATUS_SPP;
> +    if ( regs->sstatus & SSTATUS_SPP )
> +        vsstatus |= SSTATUS_SPP;
> +
> +    /* Change Guest SSTATUS.SPIE bit */
> +    vsstatus &= ~SSTATUS_SPIE;
> +    if ( vsstatus & SSTATUS_SIE )
> +        vsstatus |= SSTATUS_SPIE;
> +
> +    /* Clear Guest SSTATUS.SIE bit */
> +    vsstatus &= ~SSTATUS_SIE;
> +
> +    /* Update Guest SSTATUS */
> +    csr_write(CSR_VSSTATUS, vsstatus);
> +
> +    /* Update Guest SCAUSE, STVAL, and SEPC */
> +    csr_write(CSR_VSCAUSE, trap->scause);
> +    csr_write(CSR_VSTVAL, trap->stval);
> +    csr_write(CSR_VSEPC, trap->sepc);
> +
> +    /*
> +     * Set Guest PC to Guest exception vector.
> +     *
> +     * vstvec[1:0] is the vector MODE, not part of the address. Exceptions
> +     * always target BASE regardless of MODE, so mask it off explicitly
> +     * instead of relying on the hardwired zero bit of sepc to drop it.
> +     */
> +    regs->sepc = csr_read(CSR_VSTVEC) & ~0x3UL;

Can there be a proper constant please for this mask?

Jan