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

[Oleksii Kurochko <oleksii.kurochko@gmail.com>]

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