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

[Sascha Bischoff <Sascha.Bischoff@arm.com>]

On Wed, 2026-01-07 at 15:49 +0000, Jonathan Cameron wrote:
> 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.

Fixed. Interestingly, emacs is very insistent that this is the correct
way to wrap here.

> 
> > 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.

Yeah, I've done that.

> 
> 
> >  
> > -	/*
> > -	 * 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.

I've moved to using a flag as it makes things more readable.

Sascha

> 
> >  	}
> >  
> >  	if (ret)
> 
>