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

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 9/8/26 4:16 PM, Jan Beulich wrote:
> 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.
Agree, not too much sense.

> > --- 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.
Agree, it could be dropped. I will update that in v3.

Thanks.

~ Oleksii