Re: [PATCH v2 19/39] xen/riscv: implement trap redirection to a guest

[Jan Beulich <jbeulich@xxxxxxxx>]

On 27.08.2026 17:21, Oleksii Kurochko wrote:
> --- a/xen/arch/riscv/include/asm/riscv_encoding.h
> +++ b/xen/arch/riscv/include/asm/riscv_encoding.h
> @@ -109,6 +109,12 @@
>  #define SIP_SSIP                     MIP_SSIP
>  #define SIP_STIP                     MIP_STIP
>  
> +/* stvec/vstvec: MODE is bits [1:0], BASE is bits [XLEN-1:2] */
> +#define STVEC_MODE_MASK                      _UL(0x3)
> +#define STVEC_MODE_DIRECT            _UL(0x0)
> +#define STVEC_MODE_VECTORED          _UL(0x1)

As on earlier occasions: Do the 0x here actually add any value? There are
none ...

> +#define STVEC_BASE_MASK                      (~STVEC_MODE_MASK)
> +
>  #define PRV_U                                _UL(0)
>  #define PRV_S                                _UL(1)
>  #define PRV_M                                _UL(3)

... here, for example.

> --- a/xen/arch/riscv/traps.c
> +++ b/xen/arch/riscv/traps.c
> @@ -294,5 +294,53 @@ enum mc_disposition arch_do_multicall_call(struct 
> mc_state *state)
>  /* Redirect trap to Guest. */
>  void trap_redirect(const struct trap_info *trap)
>  {
> -    BUG_ON("unimplemented");
> +    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's MODE field is 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) & STVEC_BASE_MASK;

Nit: Given how much the comment talks about MODE, imo using ~STVEC_MODE_MASK
here directly (and dropping STVEC_BASE_MASK) might be better.

Jan