Re: [PATCH v3 1/3] KVM: arm64: Finalize guest-wide sysregs prior to per-vCPU sysregs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Iterating over `enum vcpu_sysreg` using a numeric range (`< NR_SYS_REGS`) treats the enum as a dense array, touching unintended unallocated gaps because VNCR-mapped entries have sparse numbering based on their byte offsets.
--

commit 54e02ec9a7efd1b5c5a028a29a9441e4c02c5f90
Author: Mark Brown <broonie@kernel.org>

KVM: arm64: Finalize guest-wide sysregs prior to per-vCPU sysregs

This commit splits the sysreg initialization into a guest-wide phase and a
per-vCPU phase. It runs the guest-wide setup earlier to prevent mismatches
between ID registers updated by vGIC finalization and the per-vCPU NV
register sanitization.

> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c

[ ... ]

> +/*
> + * Because this can be called once per CPU, changes must be idempotent.
> + */
> +int kvm_vcpu_finalize_sys_regs(struct kvm_vcpu *vcpu)
> +{
> +	struct kvm *kvm = vcpu->kvm;
> +	int ret;
> +
> +	guard(mutex)(&kvm->arch.config_lock);
> +
> +	ret = kvm_vm_finalize_sys_regs(kvm);
> +	if (ret)
> +		return ret;
> +
> +	if (vcpu_has_nv(vcpu)) {
> +		ret = kvm_init_nv_sysregs(vcpu);

[Severity: High]
This isn't a bug introduced by this patch, but this is a pre-existing 
issue in the kvm_init_nv_sysregs() function called here.

Does this loop in arch/arm64/kvm/nested.c:kvm_init_nv_sysregs() drop
guest state?

	for (enum vcpu_sysreg sr = __SANITISED_REG_START__; sr < NR_SYS_REGS; sr++)
		__vcpu_rmw_sys_reg(vcpu, sr, |=, 0);

The enum vcpu_sysreg maps VNCR entries using sparse byte offsets rather
than sequential integers. Iterating over it with a numeric range
comparison (< NR_SYS_REGS) covers unallocated gaps, which can silently
drop guest state and desynchronize the host's GET/SET_ONE_REG view when
a VM is configured with nested virtualization.

Could this loop be replaced with an explicit per-register allowlist?

> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901-kvm-arm64-idreg-final-v3-0-a0ffa06fa872@kernel.org?part=1