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

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 <[email protected]>

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?

> +
> +	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");

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=42