Re: [PATCH] KVM: selftests: riscv: Add lazy V extension enablement for guests

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Lazy V extension enablement causes an inescapable infinite exception loop on hosts without the V extension when an illegal instruction is executed.
- [Medium] Reading the live `sstatus` CSR instead of the preserved `regs->status` creates a TOCTOU vulnerability with nested exceptions.
--

--- Patch [1]: [PATCH] KVM: selftests: riscv: Add lazy V extension enablement for guests ---
commit 2d8f4dc253676991c8c0807d4dda2a90b9cca890
Author: jinrui <jinrui@haiwei.tech>

KVM: selftests: riscv: Add lazy V extension enablement for guests

This commit introduces lazy V extension enablement for guests in RISC-V KVM
selftests to handle compiler auto-vectorization. It replaces the default
unexpected trap handler with the full exception vector table and adds a check
in route_exception() to enable V and retry the instruction.

> diff --git a/tools/testing/selftests/kvm/lib/riscv/processor.c b/tools/testing/selftests/kvm/lib/riscv/processor.c
> index ded5429f34483..68577b2f48f4e 100644
> --- a/tools/testing/selftests/kvm/lib/riscv/processor.c
> +++ b/tools/testing/selftests/kvm/lib/riscv/processor.c
[ ... ]
> @@ -432,6 +438,28 @@ void route_exception(struct pt_regs *regs)
>  		ec = 0;
>  	}
>  
> +	/*
> +	 * Handle V (vector) extension lazy enablement before any
> +	 * registered handler. The compiler's default march may include
> +	 * V, and auto-vectorization generates vector instructions that
> +	 * trigger EXC_INST_ILLEGAL when VS (Vector Status) in sstatus
> +	 * is Off. Enable VS to Initial and re-execute the faulting
> +	 * instruction, mimicking what a real OS kernel does.
> +	 *
> +	 * This check runs before any test-registered handler, so tests
> +	 * that install their own EXC_INST_ILLEGAL handler (e.g.
> +	 * sbi_pmu_test) are not affected.
> +	 */
> +	if (!(regs->cause & CAUSE_IRQ_FLAG) && ec == EXC_INST_ILLEGAL) {
> +		unsigned long sstatus;
> +
> +		asm volatile("csrr %0, sstatus" : "=r" (sstatus));

[Severity: Medium]
Does reading the live sstatus CSR here instead of using the preserved
regs->status create an issue with nested exceptions?

If the prologue of route_exception() uses auto-vectorized instructions, it
traps. The nested trap fixes sstatus and returns, leaving the live sstatus.VS
altered to Initial. The outer handler would then read the modified live
sstatus instead of the original exception state in regs->status, skipping V
enablement for the original fault.

> +		if (!(sstatus & SR_VS)) {
> +			regs->status |= SR_VS_INITIAL;
> +			return;

[Severity: Medium]
Can this cause an infinite exception loop on hosts that don't support the V
extension?

If a test deliberately executes an illegal instruction (like sbi_pmu_test) on
hardware without V support, SR_VS is hardwired to 0. The code sets
SR_VS_INITIAL and returns to re-execute, but the instruction faults again with
VS=0, repeating infinitely.

> +		}
> +	}
> +
>  	if (handlers && handlers->exception_handlers[vector][ec])
>  		return handlers->exception_handlers[vector][ec](regs);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/4FAAF34C966898C0+20260721101406.305108-1-jinrui@haiwei.tech?part=1