Re: [PATCH v5 41/49] KVM: selftests: Add VGICv5 NR_IRQS attribute tests

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The file-level macro `VGIC_V5_MAX_NR_SPIS` contradicts the actual maximum SPI count hardcoded in the test logic and KVM implementation.
--

--- Patch [41]: [PATCH v5 41/49] KVM: selftests: Add VGICv5 NR_IRQS attribute tests ---
commit da8da00920607df68a148cd61772ea241d9a820f
Author: Sascha Bischoff <[email protected]>

KVM: selftests: Add VGICv5 NR_IRQS attribute tests

This commit adds coverage for the VGICv5 NR_IRQS attribute. It verifies
that KVM rejects values outside the supported range, non-32-aligned counts,
and attempts to change the count after initialization.

> diff --git a/tools/testing/selftests/kvm/arm64/vgic_v5.c b/tools/testing/selftests/kvm/arm64/vgic_v5.c
> index 51ea2fe7141ad..f4c0d2c003bb4 100644
> --- a/tools/testing/selftests/kvm/arm64/vgic_v5.c
> +++ b/tools/testing/selftests/kvm/arm64/vgic_v5.c
> @@ -13,6 +13,8 @@
>  #include "vgic.h"
>  
>  #define NR_VCPUS		1
> +#define VGIC_V5_DEFAULT_NR_SPIS	32
> +#define VGIC_V5_MAX_NR_SPIS	BIT(16)

[Severity: Low]
This isn't a bug, but this macro defines the maximum as 65536, whereas the
commit message correctly mentions a "1024-SPI maximum".

[ ... ]

> @@ -125,60 +184,92 @@ static void test_vgic_v5_create(void)
>  	vm_gic_destroy(&v);
>  }
>  
> -static void guest_irq_handler(struct ex_regs *regs)
> +static void test_vgic_v5_nr_irqs_attrs(void)
>  {
> -	bool valid;
> -	u32 hwirq;
> -	u64 ia;
> -	static int count;
> -
> -	/*
> -	 * We have pending interrupts. Should never actually enter WFI
> -	 * here!
> -	 */
> -	wfi();
> -	GUEST_SYNC(GUEST_CMD_IS_AWAKE);
> -
> -	ia = gicr_insn(CDIA);
> -	valid = GICV5_GICR_CDIA_VALID(ia);
> -
> -	GUEST_SYNC(GUEST_CMD_IRQ_CDIA);
> -
> -	if (!valid)
> -		return;
> -
> -	gsb_ack();
> -	isb();
> -
> -	hwirq = FIELD_GET(GICV5_GICR_CDIA_INTID, ia);
> -
> -	gic_insn(hwirq, CDDI);
> -	gic_insn(0, CDEOI);
> +	const u32 max_nr_spis = BIT(10);

[Severity: Low]
Does VGIC_V5_MAX_NR_SPIS go completely unused in favor of this local
max_nr_spis variable?

Should VGIC_V5_MAX_NR_SPIS be defined as BIT(10) and used here instead to
avoid having two contradictory limits?

> +	struct kvm_vcpu *vcpu;
> +	struct vm_gic v;
> +	u32 nr_irqs;
> +	int ret;

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