Re: [PATCH v12] 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] Unaligned 32-bit memory access when decoding 32-bit instructions
--

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

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

This commit introduces lazy enablement of the RISC-V vector (V) extension
for guest tests. It updates the exception routing to decode faulting
instructions and re-execute vector instructions when the VS status is off.

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

[ ... ]

> @@ -432,6 +472,37 @@ 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.
> +		 * Vector instructions are always 32-bit, so read a 16-bit
> +		 * halfword first and only load the full 32-bit instruction when
> +		 * it is not compressed; this avoids an unaligned or cross-page
> +		 * access on a compressed instruction.
> +		 */
> +		if (!insn) {
> +			u16 half = *(u16 *)regs->epc;
> +
> +			if ((half & 0x3) == 0x3)
> +				insn = *(u32 *)regs->epc;

[Severity: Medium]
Will this result in an unaligned 32-bit memory access on certain hardware?

If a 32-bit vector instruction is placed at a 2-byte aligned address (which
is valid when the RISC-V 'C' compressed extension is enabled), regs->epc
will only be 2-byte aligned.

Casting a potentially 2-byte aligned address to a u32 pointer is undefined
behavior in C, and the compiler typically emits a standard 32-bit word load
instruction.

On strict-alignment RISC-V hardware, won't this trap and crash the selftest
guest with a 'Load Address Misaligned' exception?

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

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