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
next prev parent 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