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

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Directly reading a 32-bit instruction from `regs->epc` can trigger an unhandled Misaligned Load or Load Page Fault exception in the guest.
--

--- Patch [1]: [PATCH v11] KVM: selftests: riscv: Add lazy V extension enablement for guests ---
commit 53e28f775e49a3c6cc2ff1526d5a5089a4c1c737
Author: jinrui <[email protected]>

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

This commit adds lazy enablement of the V (vector) extension in KVM RISC-V
guest selftests to handle compiler auto-vectorization. It updates the
exception routing to decode faulting instructions and enable the V extension
if a vector instruction is executed while VS is off.

> diff --git a/tools/testing/selftests/kvm/lib/riscv/processor.c b/tools/testing/selftests/kvm/lib/riscv/processor.c
> --- a/tools/testing/selftests/kvm/lib/riscv/processor.c
> +++ b/tools/testing/selftests/kvm/lib/riscv/processor.c

[ ... ]

> @@ -432,6 +472,27 @@ void route_exception(struct pt_regs *regs)
>  		ec = 0;
>  	}
>  
> +	/*
> +	 * Lazily enable V on the first vector instruction: if the faulting
> +	 * instruction decodes as vector while VS is off, set VS to Initial
> +	 * and re-execute it, like the kernel's riscv_v_first_use_handler().
> +	 * Genuinely illegal instructions continue to the unexpected-exception
> +	 * path.
> +	 */
> +	if (!(regs->cause & CAUSE_IRQ_FLAG) && ec == EXC_INST_ILLEGAL &&
> +	    handlers && handlers->v_available && !(regs->status & SR_VS)) {
> +		u32 insn = (u32)regs->badaddr;
> +
> +		/* stval is not guaranteed to hold the faulting instruction */
> +		if (!insn)
> +			insn = *(u32 *)regs->epc;

[Severity: Medium]
Can reading a full 32-bit value directly from regs->epc trigger an unhandled
misaligned load or load page fault in the guest?

If the faulting instruction is a compressed 16-bit instruction at a 2-byte
aligned address, this 32-bit read will be unaligned and might trap on
hardware that does not support unaligned accesses.

Additionally, if the 16-bit instruction sits exactly at the end of a mapped
page and the next page is unmapped, reading 32 bits could trigger a page
fault instead of correctly handling the illegal instruction exception.

> +
> +		if (insn_is_vector(insn)) {
> +			regs->status |= SR_VS_INITIAL;
> +			return;
> +		}
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1