Re: [PATCH v5 20/36] KVM: arm64: gic-v5: Init Private IRQs (PPIs) for GICv5

[Marc Zyngier <maz@kernel.org>]

On Thu, 26 Feb 2026 16:00:36 +0000,
Sascha Bischoff <Sascha.Bischoff@arm.com> wrote:
> 
> Initialise the private interrupts (PPIs, only) for GICv5. This means
> that a GICv5-style intid is generated (which encodes the PPI type in
> the top bits) instead of the 0-based index that is used for older
> GICs.
> 
> Additionally, set all of the GICv5 PPIs to use Level for the handling
> mode, with the exception of the SW_PPI which uses Edge. This matches
> the architecturally-defined set in the GICv5 specification (the CTIIRQ
> handling mode is IMPDEF, so Level has been picked for that).
> 
> Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> ---
>  arch/arm64/kvm/vgic/vgic-init.c | 39 +++++++++++++++++++++++++--------
>  1 file changed, 30 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
> index d1db384698238..e4a230c3857ff 100644
> --- a/arch/arm64/kvm/vgic/vgic-init.c
> +++ b/arch/arm64/kvm/vgic/vgic-init.c
> @@ -254,14 +254,20 @@ static int vgic_allocate_private_irqs_locked(struct kvm_vcpu *vcpu, u32 type)
>  {
>  	struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
>  	int i;
> +	u32 num_private_irqs;

uber-nit: things look better like this:

	struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
	u32 num_private_irqs;
	int i;

I know, that's silly. I'll take my pills shortly.

>  
>  	lockdep_assert_held(&vcpu->kvm->arch.config_lock);
>  
>  	if (vgic_cpu->private_irqs)
>  		return 0;
>  
> +	if (vgic_is_v5(vcpu->kvm))
> +		num_private_irqs = VGIC_V5_NR_PRIVATE_IRQS;
> +	else
> +		num_private_irqs = VGIC_NR_PRIVATE_IRQS;
> +
>  	vgic_cpu->private_irqs = kzalloc_objs(struct vgic_irq,
> -					      VGIC_NR_PRIVATE_IRQS,
> +					      num_private_irqs,
>  					      GFP_KERNEL_ACCOUNT);
>  
>  	if (!vgic_cpu->private_irqs)
> @@ -271,22 +277,37 @@ static int vgic_allocate_private_irqs_locked(struct kvm_vcpu *vcpu, u32 type)
>  	 * Enable and configure all SGIs to be edge-triggered and
>  	 * configure all PPIs as level-triggered.
>  	 */
> -	for (i = 0; i < VGIC_NR_PRIVATE_IRQS; i++) {
> +	for (i = 0; i < num_private_irqs; i++) {
>  		struct vgic_irq *irq = &vgic_cpu->private_irqs[i];
>  
>  		INIT_LIST_HEAD(&irq->ap_list);
>  		raw_spin_lock_init(&irq->irq_lock);
> -		irq->intid = i;
>  		irq->vcpu = NULL;
>  		irq->target_vcpu = vcpu;
>  		refcount_set(&irq->refcount, 0);
> -		if (vgic_irq_is_sgi(i)) {
> -			/* SGIs */
> -			irq->enabled = 1;
> -			irq->config = VGIC_CONFIG_EDGE;
> +		if (!vgic_is_v5(vcpu->kvm)) {
> +			irq->intid = i;
> +			if (vgic_irq_is_sgi(i)) {
> +				/* SGIs */
> +				irq->enabled = 1;
> +				irq->config = VGIC_CONFIG_EDGE;
> +			} else {
> +				/* PPIs */
> +				irq->config = VGIC_CONFIG_LEVEL;
> +			}
>  		} else {
> -			/* PPIs */
> -			irq->config = VGIC_CONFIG_LEVEL;
> +			irq->intid = FIELD_PREP(GICV5_HWIRQ_ID, i) |
> +				     FIELD_PREP(GICV5_HWIRQ_TYPE,
> +						GICV5_HWIRQ_TYPE_PPI);
> +
> +			/* The only Edge architected PPI is the SW_PPI */
> +			if (i == GICV5_ARCH_PPI_SW_PPI)
> +				irq->config = VGIC_CONFIG_EDGE;
> +			else
> +				irq->config = VGIC_CONFIG_LEVEL;
> +
> +			/* Register the GICv5-specific PPI ops */
> +			vgic_v5_set_ppi_ops(irq);
>  		}
>  
>  		switch (type) {

That's another point where I'd rather have structural changes to the
code, moving the SGI/PPI init to their own helper:

diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
index 7df7b8aa77a69..0a2468fef86c6 100644
--- a/arch/arm64/kvm/vgic/vgic-init.c
+++ b/arch/arm64/kvm/vgic/vgic-init.c
@@ -262,6 +262,66 @@ int kvm_vgic_vcpu_nv_init(struct kvm_vcpu *vcpu)
 	return ret;
 }
 
+static void vgic_init_private_irq(struct kvm_vcpu *vcpu, int i, u32 type)
+{
+	struct vgic_irq *irq = &vcpu->arch.vgic_cpu.private_irqs[i];
+
+	INIT_LIST_HEAD(&irq->ap_list);
+	raw_spin_lock_init(&irq->irq_lock);
+	irq->vcpu = NULL;
+	irq->target_vcpu = vcpu;
+	refcount_set(&irq->refcount, 0);
+	irq->intid = i;
+
+	/*
+	 * Enable and configure all SGIs to be edge-triggered and
+	 * configure all PPIs as level-triggered.
+	 */
+	if (vgic_irq_is_sgi(i)) {
+		/* SGIs */
+		irq->enabled = 1;
+		irq->config = VGIC_CONFIG_EDGE;
+	} else {
+		/* PPIs */
+		irq->config = VGIC_CONFIG_LEVEL;
+	}
+
+	switch (type) {
+	case KVM_DEV_TYPE_ARM_VGIC_V3:
+		irq->group = 1;
+		irq->mpidr = kvm_vcpu_get_mpidr_aff(vcpu);
+		break;
+	case KVM_DEV_TYPE_ARM_VGIC_V2:
+		irq->group = 0;
+		irq->targets = BIT(vcpu->vcpu_id);
+		break;
+	}
+}
+
+static void vgic_v5_init_private_irq(struct kvm_vcpu *vcpu, int i, u32 type)
+{
+	struct vgic_irq *irq = &vcpu->arch.vgic_cpu.private_irqs[i];
+
+	INIT_LIST_HEAD(&irq->ap_list);
+	raw_spin_lock_init(&irq->irq_lock);
+	irq->vcpu = NULL;
+	irq->target_vcpu = vcpu;
+	refcount_set(&irq->refcount, 0);
+
+	irq->intid = FIELD_PREP(GICV5_HWIRQ_ID, i) |
+		FIELD_PREP(GICV5_HWIRQ_TYPE,
+			   GICV5_HWIRQ_TYPE_PPI);
+
+	/* The only Edge architected PPI is the SW_PPI */
+	if (i == GICV5_ARCH_PPI_SW_PPI)
+		irq->config = VGIC_CONFIG_EDGE;
+	else
+		irq->config = VGIC_CONFIG_LEVEL;
+
+	/* Register the GICv5-specific PPI ops */
+	vgic_v5_set_ppi_ops(irq);
+}
+
 static int vgic_allocate_private_irqs_locked(struct kvm_vcpu *vcpu, u32 type)
 {
 	struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
@@ -285,53 +345,11 @@ static int vgic_allocate_private_irqs_locked(struct kvm_vcpu *vcpu, u32 type)
 	if (!vgic_cpu->private_irqs)
 		return -ENOMEM;
 
-	/*
-	 * Enable and configure all SGIs to be edge-triggered and
-	 * configure all PPIs as level-triggered.
-	 */
 	for (i = 0; i < num_private_irqs; i++) {
-		struct vgic_irq *irq = &vgic_cpu->private_irqs[i];
-
-		INIT_LIST_HEAD(&irq->ap_list);
-		raw_spin_lock_init(&irq->irq_lock);
-		irq->vcpu = NULL;
-		irq->target_vcpu = vcpu;
-		refcount_set(&irq->refcount, 0);
-		if (!vgic_is_v5(vcpu->kvm)) {
-			irq->intid = i;
-			if (vgic_irq_is_sgi(i)) {
-				/* SGIs */
-				irq->enabled = 1;
-				irq->config = VGIC_CONFIG_EDGE;
-			} else {
-				/* PPIs */
-				irq->config = VGIC_CONFIG_LEVEL;
-			}
-		} else {
-			irq->intid = FIELD_PREP(GICV5_HWIRQ_ID, i) |
-				     FIELD_PREP(GICV5_HWIRQ_TYPE,
-						GICV5_HWIRQ_TYPE_PPI);
-
-			/* The only Edge architected PPI is the SW_PPI */
-			if (i == GICV5_ARCH_PPI_SW_PPI)
-				irq->config = VGIC_CONFIG_EDGE;
-			else
-				irq->config = VGIC_CONFIG_LEVEL;
-
-			/* Register the GICv5-specific PPI ops */
-			vgic_v5_set_ppi_ops(irq);
-		}
-
-		switch (type) {
-		case KVM_DEV_TYPE_ARM_VGIC_V3:
-			irq->group = 1;
-			irq->mpidr = kvm_vcpu_get_mpidr_aff(vcpu);
-			break;
-		case KVM_DEV_TYPE_ARM_VGIC_V2:
-			irq->group = 0;
-			irq->targets = BIT(vcpu->vcpu_id);
-			break;
-		}
+		if (type == KVM_DEV_TYPE_ARM_VGIC_V5)
+			vgic_v5_init_private_irq(vcpu, i, type);
+		else
+			vgic_init_private_irq(vcpu, i, type);
 	}
 
 	return 0;

Thanks,

	M.

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