Kernel KVM virtualization development
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: JinRui <jinrui@haiwei.tech>
Cc: kvm@vger.kernel.org
Subject: Re: [PATCH v10] KVM: selftests: riscv: Add lazy V extension enablement for guests
Date: Thu, 13 Aug 2026 09:13:04 +0000	[thread overview]
Message-ID: <20260813091305.235891F000E9@smtp.kernel.org> (raw)
In-Reply-To: <733FC2542EECADE1+20260813090306.3756434-1-jinrui@haiwei.tech>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Lazy V extension enablement fails on hardware that does not populate stval for illegal instructions.
- [Medium] Storing a per-vCPU V-extension availability flag in a VM-wide shared structure causes test failures in mixed-vCPU VMs.
--

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

This commit adds lazy V extension enablement for guests in KVM RISC-V selftests
to handle auto-vectorized instructions when sstatus.VS is off. It registers a
full exception vector table, advertises V to KVM, and lazily enables V in
route_exception().

> diff --git a/tools/testing/selftests/kvm/lib/riscv/processor.c b/tools/testing/selftests/kvm/lib/riscv/processor.c
> index ded5429f34483..ffd84212c9b0d 100644
> --- a/tools/testing/selftests/kvm/lib/riscv/processor.c
> +++ b/tools/testing/selftests/kvm/lib/riscv/processor.c
[ ... ]
> @@ -348,8 +346,22 @@ struct kvm_vcpu *vm_arch_vcpu_add(struct kvm_vm *vm, u32 vcpu_id)
>  	/* Setup sscratch for guest_get_vcpuid() */
>  	vcpu_set_reg(vcpu, RISCV_GENERAL_CSR_REG(sscratch), vcpu_id);
>  
> -	/* Setup default exception vector of guest */
> -	vcpu_set_reg(vcpu, RISCV_GENERAL_CSR_REG(stvec), (unsigned long)guest_unexp_trap);
> +	/*
> +	 * Advertise V to KVM so -O2 auto-vectorization in guest code is valid;
> +	 * ignore errors since the tests work without V too. Use the full
> +	 * exception vector table (which lazily enables V in route_exception())
> +	 * as the default handler; vm_init_vector_tables() is idempotent.
> +	 */
> +	__vcpu_set_reg(vcpu, RISCV_ISA_EXT_REG(KVM_RISCV_ISA_EXT_V), 1);
> +	vm_init_vector_tables(vm);
> +	vcpu_init_vector_tables(vcpu);
> +
> +	/* Record V availability for route_exception(), which runs in guest context. */
> +	{
> +		struct handlers *h = addr_gva2hva(vm, vm->handlers);
> +
> +		h->v_available = __vcpu_has_isa_ext(vcpu, KVM_RISCV_ISA_EXT_V);

[Severity: Medium]
Does this overwrite a shared VM-wide state with a per-vCPU capability?

The handlers structure is allocated once per VM in vm_init_vector_tables().
If a VM is created with a mix of vCPUs, and the last added vCPU has the V
extension, h->v_available will be true globally for all vCPUs.

If a non-V vCPU encounters an illegal instruction that happens to decode as a
vector instruction, could route_exception() see h->v_available as true and
continually set SR_VS_INITIAL, causing an infinite exception loop?

> +	}
>  
>  	return vcpu;
>  }
[ ... ]
> @@ -432,6 +468,20 @@ void route_exception(struct pt_regs *regs)
>  		ec = 0;
>  	}
>  
> +	/*
> +	 * Lazily enable V on the first vector instruction: if the faulting
> +	 * instruction (stval) 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 fall
> +	 * through to the unexpected-exception path.
> +	 */
> +	if (!(regs->cause & CAUSE_IRQ_FLAG) && ec == EXC_INST_ILLEGAL &&
> +	    handlers && handlers->v_available && !(regs->status & SR_VS) &&
> +	    insn_is_vector((u32)regs->badaddr)) {

[Severity: Medium]
Does this assume hardware always populates stval (which is mapped to
regs->badaddr) for illegal instruction exceptions?

If a test executes on hardware that does not populate stval and leaves it as 0,
insn_is_vector() would evaluate to false.

Would this cause genuinely auto-vectorized instructions to unexpectedly fall
through to kvm_exit_unexpected_exception() instead of being lazily enabled?

Should this fall back to reading the instruction directly from guest memory at
regs->epc when badaddr is 0?

> +		regs->status |= SR_VS_INITIAL;
> +		return;
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/733FC2542EECADE1+20260813090306.3756434-1-jinrui@haiwei.tech?part=1

  reply	other threads:[~2026-08-13  9:13 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20260722073344.771230-1-jinrui@haiwei.tech>
2026-08-11  8:50 ` [PATCH v9] KVM: selftests: riscv: Add lazy V extension enablement for guests JinRui
2026-08-12 17:26   ` Anup Patel
2026-08-13  9:03   ` [PATCH v10] " JinRui
2026-08-13  9:13     ` sashiko-bot [this message]
2026-08-13  9:37     ` [PATCH v11] " JinRui
2026-08-13  9:46       ` sashiko-bot
2026-08-13  9:56       ` [PATCH v12] " JinRui
2026-08-13 10:04         ` sashiko-bot
2026-08-13 10:29         ` [PATCH v13] " JinRui

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260813091305.235891F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=jinrui@haiwei.tech \
    --cc=kvm@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox