Re: [PATCH v5 42/49] KVM: selftests: Add VGICv5 IRS_REGS attribute tests

[Sascha Bischoff <Sascha.Bischoff@arm.com>]

On Fri, 2026-08-07 at 17:17 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
> - [Medium] The selftest for validating `GICV5_IRS_IDR0` domain
> rejection assigns a completely zeroed value to `val` instead of
> modifying the existing register value, making the test ineffective.
> --
> 
> --- Patch [42]: [PATCH v5 42/49] KVM: selftests: Add VGICv5 IRS_REGS
> attribute tests ---
> commit 11fcbef9e79956f20030f1a93c0eeaba44375ad8
> Author: Sascha Bischoff <Sascha.Bischoff@arm.com>
> 
> KVM: selftests: Add VGICv5 IRS_REGS attribute tests
>     
> This commit adds test coverage for the VGICv5 IRS_REGS attribute
> group.
> It verifies that KVM properly exposes valid registers and rejects
> invalid inputs, bad offsets, or incorrect alignment.
> 
> > diff --git a/tools/testing/selftests/kvm/arm64/vgic_v5.c
> > b/tools/testing/selftests/kvm/arm64/vgic_v5.c
> > index f4c0d2c003bb4..54fa13906990b 100644
> > --- a/tools/testing/selftests/kvm/arm64/vgic_v5.c
> > +++ b/tools/testing/selftests/kvm/arm64/vgic_v5.c
> 
> [ ... ]
> 
> > @@ -272,6 +340,189 @@ static void test_vgic_v5_nr_irqs_attrs(void)
> >  
> >  }
> >  
> > +static void test_vgic_v5_irs_regs_attrs(void)
> > +{
> 
> [ ... ]
> 
> > +	/* ID restore validation rejects unsupported values. */
> > +	val = FIELD_PREP(GICV5_IRS_IDR0_INT_DOM,
> > GICV5_IRS_IDR0_INT_DOM_SECURE);
> > +	ret = __kvm_device_attr_set(v.gic_fd,
> > KVM_DEV_ARM_VGIC_GRP_IRS_REGS,
> > +				    GICV5_IRS_IDR0, &val);
> > +	TEST_ASSERT(ret && errno == EINVAL, "GICv5 IRS_REGS
> > accepted bad IDR0 domain");
> 
> [Severity: Medium]
> Will this test effectively check the INT_DOM field rejection?
> 
> Since val is assigned directly rather than modifying a valid read
> value (like
> the SETLPI test below it), and since GICV5_IRS_IDR0_INT_DOM_SECURE is
> 0,
> val is evaluated as 0. 
> 
> The kernel will likely reject IDR0 = 0 due to other mismatched fields
> (such
> as version numbers or feature bits), making this test pass even if
> the kernel
> fails to validate the INT_DOM field specifically.
> 
> Could this be updated to read the existing IDR0 value and modify its
> INT_DOM
> field before calling __kvm_device_attr_set?

Good point. A zero value currently is rejected specifically because it
selects the Secure domain, but preserving the other IDR0 fields makes
the test’s intent clearer and should be more future-proof. I’ve changed
the test to read IDR0 and modify only the INT_DOM field itself.

> 
> > +
> > +	ret = __kvm_device_attr_get(v.gic_fd,
> > KVM_DEV_ARM_VGIC_GRP_IRS_REGS,
> > +				    GICV5_IRS_IDR0, &val);
> > +	TEST_ASSERT(!ret, "GICv5 IRS_REGS get IDR0 failed");
> > +	val |= GICV5_IRS_IDR0_SETLPI;
> > +	ret = __kvm_device_attr_set(v.gic_fd,
> > KVM_DEV_ARM_VGIC_GRP_IRS_REGS,
> > +				    GICV5_IRS_IDR0, &val);
> > +	TEST_ASSERT(ret && errno == EINVAL, "GICv5 IRS_REGS
> > accepted unsupported IDR0");
> 

Thanks,
Sascha