Re: [PATCH v5 07/49] KVM: arm64: gic-v5: Cache host IRS ID registers

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The hardware-based limit for maximum vCPUs is silently clobbered by a hardcoded maximum in the GICv3 legacy fallback path.
- [Medium] The return type of `vgic_v5_irs_vped_size()` is `u16`, which will silently truncate descriptor sizes of 64KB or larger to 0.
--

--- Patch [7]: [PATCH v5 07/49] KVM: arm64: gic-v5: Cache host IRS ID registers ---
commit d81802dd59faf96c570bb509e00ac335dada7904
Author: Sascha Bischoff <[email protected]>

KVM: arm64: gic-v5: Cache host IRS ID registers
    
This commit caches the GICv5 IRS ID registers to configure virtual-machine
resources based on hardware capabilities rather than extracting individual
fields. It limits the maximum number of vCPUs using the VPE capacity from
the cached IRS_IDR4 register while retaining KVM's upper bound.

> diff --git a/arch/arm64/kvm/vgic/vgic-v5.c b/arch/arm64/kvm/vgic/vgic-v5.c
> index d4789ff3e7402..41ca2098ecb73 100644
> --- a/arch/arm64/kvm/vgic/vgic-v5.c
> +++ b/arch/arm64/kvm/vgic/vgic-v5.c

[ ... ]

> @@ -61,10 +77,12 @@ int vgic_v5_probe(const struct gic_kvm_info *info)
>  		goto skip_v5;
>  	}
>  
> -	kvm_vgic_global_state.max_gic_vcpus = VGIC_V5_MAX_CPUS;
> -
> +	vgic_v5_irs_cache_id_regs(info);
>  	vgic_v5_get_implemented_ppis();
>  
> +	kvm_vgic_global_state.max_gic_vcpus = min(vgic_v5_irs_max_vpes(&irs_caps),
> +						  VGIC_V5_MAX_CPUS);
> +

[Severity: High]
Does the GICv3 legacy fallback path under the skip_v5 label in vgic_v5_probe()
overwrite this dynamically computed hardware limit?

If the max_gic_vcpus limit is later expanded by a static value (like
VGIC_V3_MAX_CPUS via a max() assignment), could it permit userspace to create
more vCPUs for a GICv5 VM than the hardware IRS actually supports (for example,
allowing 512 vCPUs even if the hardware restricts it to 16 VPEs)?

>  	ret = kvm_register_vgic_device(KVM_DEV_TYPE_ARM_VGIC_V5);
>  	if (ret) {
>  		kvm_err("Cannot register GICv5 KVM device.\n");

> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> index fe49fb56dc3c9..c6f0ab69f7a2e 100644
> --- a/include/kvm/arm_vgic.h
> +++ b/include/kvm/arm_vgic.h

[ ... ]

> @@ -630,6 +646,65 @@ int vgic_v4_load(struct kvm_vcpu *vcpu);
>  void vgic_v4_commit(struct kvm_vcpu *vcpu);
>  int vgic_v4_put(struct kvm_vcpu *vcpu);
>  

[ ... ]

> +static inline u16 vgic_v5_irs_vped_size(const struct vgic_v5_irs_caps *caps)
> +{
> +	return BIT(FIELD_GET(GICV5_IRS_IDR4_VPED_SZ, caps->idr4));
> +}
> +

[Severity: Medium]
If a hardware implementation reports a size parameter of 16 or greater
(indicating 64KB or more), will the BIT() operation in vgic_v5_irs_vped_size()
result in 65536 and silently truncate to 0 when cast to the u16 return type?

Could this zero-byte size calculation lead to 0-byte allocations or infinite
loops in dependent resource sizing?

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