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

Sascha Bischoff
2026-03-05T13:36:30Z

On Wed, 2026-03-04 at 14:21 +0000, Marc Zyngier wrote:
> 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.

Done! And I can't say I disagree.

>
> >
> >   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_P
> > PI);
> > +
> > + /* 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;

Moved things into separate handlers as you suggested.

Thanks,
Sascha

>
> Thanks,
>
> M.
>