Re: [PATCH] KVM: selftests: Fix a couple "prio" signedness bugs

[Marc Zyngier <maz@kernel.org>]

Hey Dan,

On Fri, 21 Mar 2025 14:32:53 +0000,
Dan Carpenter <dan.carpenter@linaro.org> wrote:
> 
> There is an assert which relies on "prio" to be signed.
> 
> 	GUEST_ASSERT(prio >= 0);
> 
> Change the type from uint32_t to int.
> 
> Fixes: 728fcc46d2c2 ("KVM: selftests: aarch64: Add test for restoring active IRQs")
> Fixes: 0ad3ff4a6adc ("KVM: selftests: aarch64: Add preemption tests in vgic_irq")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
> ---
>  tools/testing/selftests/kvm/arm64/vgic_irq.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/arm64/vgic_irq.c b/tools/testing/selftests/kvm/arm64/vgic_irq.c
> index f4ac28d53747..e89c0fc5eef3 100644
> --- a/tools/testing/selftests/kvm/arm64/vgic_irq.c
> +++ b/tools/testing/selftests/kvm/arm64/vgic_irq.c
> @@ -294,7 +294,8 @@ static void guest_restore_active(struct test_args *args,
>  		uint32_t first_intid, uint32_t num,
>  		kvm_inject_cmd cmd)
>  {
> -	uint32_t prio, intid, ap1r;
> +	uint32_t intid, ap1r;
> +	int prio;
>  	int i;
>  
>  	/*
> @@ -362,7 +363,8 @@ static void test_inject_preemption(struct test_args *args,
>  		uint32_t first_intid, int num,
>  		kvm_inject_cmd cmd)
>  {
> -	uint32_t intid, prio, step = KVM_PRIO_STEPS;
> +	uint32_t intid, step = KVM_PRIO_STEPS;
> +	int prio;
>  	int i;
>  
>  	/* Set the priorities of the first (KVM_NUM_PRIOS - 1) IRQs

I think this is going in the wrong direction. A GIC priority is an
unsigned 8bit value as per the architecture definition.

So the type used by the test the first place looks wrong (it is too
wide), and the assertion is pointless.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.