Re: [PATCH v2 1/2] KVM: arm64: Finalize guest-wide sysregs prior to per-vCPU sysregs
Fuad Tabba <[email protected]> Tue, 4 Aug 2026 09:01:47 +0100
| Newsgroups | dev.linux.lists.kvmarm,dev.linux.lists.sashiko-reviews |
|---|---|
| Message-ID | <CA+EHjTwm7DkwV=TNmA6dKmu3mxrO-C4qsX-FZjfpY-iA3fF1=w@mail.gmail.com> |
On Tue, 4 Aug 2026 at 00:13, <[email protected]> wrote: > > Thank you for your contribution! Sashiko AI review found 1 potential issu= e(s) to consider: > > Pre-existing issues: > - [High] Numeric range comparison over sparse `enum vcpu_sysreg` covers u= nintended registers. False positive. The gaps are real, but the loop body is a value-preserving no-op on them and stays in bounds, so there is nothing to touch. /fuad > -- > > --- Patch [1]: [PATCH v2 1/2] KVM: arm64: Finalize guest-wide sysregs pri= or to per-vCPU sysregs --- > commit d51df44c26211dddcc71c5fe7798bb6cbbc7e67e > Author: Mark Brown <[email protected]> > > KVM: arm64: Finalize guest-wide sysregs prior to per-vCPU sysregs > > This commit splits the sysreg initialization into a per-VM and a per-vCPU > function to ensure that guest-wide settings are finalized before per-vCPU > NV initialization runs. > > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > > index 5d5c579d45790..958d7ef78785f 100644 > > --- a/arch/arm64/kvm/sys_regs.c > > +++ b/arch/arm64/kvm/sys_regs.c > [ ... ] > > @@ -5825,6 +5814,29 @@ int kvm_finalize_sys_regs(struct kvm_vcpu *vcpu) > [ ... ] > > +int kvm_vcpu_finalize_sys_regs(struct kvm_vcpu *vcpu) > > +{ > [ ... ] > > + if (vcpu_has_nv(vcpu)) { > > + ret =3D kvm_init_nv_sysregs(vcpu); > > [Severity: High] > This isn't a bug introduced by this patch, but does the initialization in > kvm_init_nv_sysregs() touch unintended registers by iterating over the > sparse vcpu_sysreg enumeration? > > Looking at the loop at the end of kvm_init_nv_sysregs() in > arch/arm64/kvm/nested.c: > > for (enum vcpu_sysreg sr =3D __SANITISED_REG_START__; sr < NR_SYS_REG= S; sr++) > __vcpu_rmw_sys_reg(vcpu, sr, |=3D, 0); > > Because VNCR-mapped entries are numbered by their VNCR-page byte offset > rather than declaration order, iterating using a numeric range comparison > over the enum covers unassigned gaps and other registers that happen to f= all > in that byte span. > > Could this loop inadvertently corrupt or desync guest state by treating > offsets as sequential enum entries? > > > + if (ret) > > + return ret; > > + } > > + > > + return 0; > > +} > > -- > Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260803-kvm-arm6= [email protected]?part=3D1 >