Re: [PATCH v2 24/36] KVM: arm64: gic-v5: Create, init vgic_v5

[Jonathan Cameron <jonathan.cameron@huawei.com>]

On Fri, 19 Dec 2025 15:52:44 +0000
Sascha Bischoff <Sascha.Bischoff@arm.com> wrote:

> Update kvm_vgic_create to create a vgic_v5 device. When creating a
> vgic, FEAT_GCIE in the ID_AA64PFR2 is only exposed to vgic_v5-based
> guests, and is hidden otherwise. GIC in ~ID_AA64PFR0_EL1 is never
> exposed for a vgic_v5 guest.
> 
> When initialising a vgic_v5, skip kvm_vgic_dist_init as GICv5 doesn't
> support one. The current vgic_v5 implementation only supports PPIs, so
> no SPIs are initialised either.
> 
> The current vgic_v5 support doesn't extend to nested

Odd early wrapping of message.

> guests. Therefore, the init of vgic_v5 for a nested guest is failed in
> vgic_v5_init.
> 
> As the current vgic_v5 doesn't require any resources to be mapped,
> vgic_v5_map_resources is simply used to check that the vgic has indeed
> been initialised. Again, this will change as more GICv5 support is
> merged in.
> 
> Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
Comments mostly on existing code, so
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>

> ---
>  arch/arm64/kvm/vgic/vgic-init.c | 51 ++++++++++++++++++++++-----------
>  arch/arm64/kvm/vgic/vgic-v5.c   | 26 +++++++++++++++++
>  arch/arm64/kvm/vgic/vgic.h      |  2 ++
>  include/kvm/arm_vgic.h          |  1 +
>  4 files changed, 63 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
> index 03f45816464b0..afb5888cd8219 100644
> --- a/arch/arm64/kvm/vgic/vgic-init.c
> +++ b/arch/arm64/kvm/vgic/vgic-init.c

>  	if (type == KVM_DEV_TYPE_ARM_VGIC_V3)
> @@ -420,20 +427,26 @@ int vgic_init(struct kvm *kvm)
>  	if (kvm->created_vcpus != atomic_read(&kvm->online_vcpus))
>  		return -EBUSY;
>  
> -	/* freeze the number of spis */
> -	if (!dist->nr_spis)
> -		dist->nr_spis = VGIC_NR_IRQS_LEGACY - VGIC_NR_PRIVATE_IRQS;
> +	if (!vgic_is_v5(kvm)) {
> +		/* freeze the number of spis */
> +		if (!dist->nr_spis)
> +			dist->nr_spis = VGIC_NR_IRQS_LEGACY - VGIC_NR_PRIVATE_IRQS;
>  
> -	ret = kvm_vgic_dist_init(kvm, dist->nr_spis);
> -	if (ret)
> -		goto out;
> +		ret = kvm_vgic_dist_init(kvm, dist->nr_spis);
> +		if (ret)
> +			goto out;

Not really related to this patch, but I have no idea why this function
doesn't just do early returns on error in all paths (rather than just some of them).
It might be worth changing that to improve readability.


>  
> -	/*
> -	 * Ensure vPEs are allocated if direct IRQ injection (e.g. vSGIs,
> -	 * vLPIs) is supported.
> -	 */
> -	if (vgic_supports_direct_irqs(kvm)) {
> -		ret = vgic_v4_init(kvm);
> +		/*
> +		 * Ensure vPEs are allocated if direct IRQ injection (e.g. vSGIs,
> +		 * vLPIs) is supported.
> +		 */
> +		if (vgic_supports_direct_irqs(kvm)) {
> +			ret = vgic_v4_init(kvm);
> +			if (ret)
> +				goto out;
> +		}
> +	} else {
> +		ret = vgic_v5_init(kvm);
>  		if (ret)
>  			goto out;
>  	}
> @@ -610,9 +623,13 @@ int kvm_vgic_map_resources(struct kvm *kvm)
>  	if (dist->vgic_model == KVM_DEV_TYPE_ARM_VGIC_V2) {
>  		ret = vgic_v2_map_resources(kvm);
>  		type = VGIC_V2;
> -	} else {
> +	} else if (dist->vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3) {
>  		ret = vgic_v3_map_resources(kvm);
>  		type = VGIC_V3;
> +	} else {
> +		ret = vgic_v5_map_resources(kvm);
> +		type = VGIC_V5;
> +		goto out;
This skips over the checking of ret which is fine (given it's just goto out)
but I'd add a comment to say why the next bit is skipped or a more complex
flow (maybe a flag to say dist is relevant that gates the next bit.

>  	}
>  
>  	if (ret)