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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `v_epc` array for tracking vector extension enablement is sized using `KVM_CAP_MAX_VCPUS` but indexed by `vcpu_id`. Since KVM allows `vcpu_id` up to `KVM_CAP_MAX_VCPU_ID` (which can be significantly larger than `KVM_CAP_MAX_VCPUS` for sparse topologies), tests using large vCPU IDs will silently bypass the lazy V extension enablement and fail with an unhandled exception.
--

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

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

This commit introduces lazy V extension enablement for RISC-V KVM selftests.
It replaces the default guest exception handler with the full vector table
and adds a check in route_exception() to lazily enable the V extension if
an EXC_INST_ILLEGAL is triggered by auto-vectorized instructions.

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

[ ... ]

> @@ -432,6 +454,46 @@ 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) {
> +		/*
> +		 * If KVM supports the V extension for this Guest and VS
> +		 * (Vector Status) is Off in the saved sstatus, set it to
> +		 * Initial and re-execute the faulting instruction.
> +		 *
> +		 * Use regs->status (saved at exception entry) rather than
> +		 * reading the live CSR to avoid a TOCTOU race.
> +		 *
> +		 * Track the epc per-vCPU to avoid an infinite loop when
> +		 * V is disabled or the hardware rejects the VS change.
> +		 * Using a per-vCPU array avoids races between concurrent
> +		 * vCPUs that would occur with a single shared (epc, vcpu)
> +		 * pair.
> +		 */
> +		if (handlers && handlers->v_available && !(regs->status & SR_VS)) {
> +			unsigned int vcpu_id;
> +
> +			asm volatile("csrr %0, sscratch" : "=r" (vcpu_id));
> +			if (vcpu_id < handlers->v_epc_capacity &&
> +			    handlers->v_epc[vcpu_id] != regs->epc) {
> +				handlers->v_epc[vcpu_id] = regs->epc;
> +				regs->status |= SR_VS_INITIAL;
> +				return;
> +			}
> +		}
> +	}
> +

[ ... ]

> @@ -448,9 +510,26 @@ void vcpu_init_vector_tables(struct kvm_vcpu *vcpu)
>  
>  void vm_init_vector_tables(struct kvm_vm *vm)
>  {
> -	vm->handlers = __vm_alloc(vm, sizeof(struct handlers), vm->page_size,
> +	unsigned int max_vcpus;
> +	size_t size;
> +
> +	if (vm->handlers)
> +		return;
> +
> +	max_vcpus = kvm_check_cap(KVM_CAP_MAX_VCPUS);
> +	if (max_vcpus == 0)
> +		max_vcpus = 512;
> +
> +	size = sizeof(struct handlers) + max_vcpus * sizeof(unsigned long);

[Severity: Medium]
Should this allocation use KVM_CAP_MAX_VCPU_ID instead of KVM_CAP_MAX_VCPUS?

The v_epc array is sized here using max_vcpus, but it is indexed by vcpu_id
when accessed in route_exception():

			asm volatile("csrr %0, sscratch" : "=r" (vcpu_id));
			if (vcpu_id < handlers->v_epc_capacity &&
			    handlers->v_epc[vcpu_id] != regs->epc) {
				handlers->v_epc[vcpu_id] = regs->epc;

Since KVM allows vcpu_id to go up to KVM_CAP_MAX_VCPU_ID, which can be
significantly larger than KVM_CAP_MAX_VCPUS for sparse topologies, tests
using large vCPU IDs might exceed this capacity. This would cause the bounds
check in route_exception() to fail, silently bypassing the lazy V extension
enablement and resulting in an unhandled exception for those tests.

> +	vm->handlers = __vm_alloc(vm, size, vm->page_size,
>  				  MEM_REGION_DATA);
>  
> +	{
> +		struct handlers *h = addr_gva2hva(vm, vm->handlers);
> +
> +		h->v_epc_capacity = max_vcpus;
> +	}
> +
>  	*(gva_t *)addr_gva2hva(vm, (gva_t)(&exception_handlers)) = vm->handlers;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/33AEC00DC78613A1+20260722073344.771230-1-jinrui@haiwei.tech?part=1