Re: [PATCH 06/43] KVM: arm64: gic-v5: Add VPE doorbell domain

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

On Tue, 2026-04-28 at 17:40 +0100, Marc Zyngier wrote:
> On Mon, 27 Apr 2026 17:08:05 +0100,
> Sascha Bischoff <Sascha.Bischoff@arm.com> wrote:
> > 
> > GICv5 supports two types of doorbell - VPE doorbells and VM
> > doorbells. In KVM we only support Targeted interrupts, and do not
> > support 1ofN target selection. This means that we only implement
> > VPE
> > doorbells. These doorbells are implemented as host LPIs which are
> > generated when a non-resident VPE has a pending interrupt of
> > sufficient priority and the doorbell has been requested as part of
> > making the VPE non-resident.
> 
> This is mostly a repeat of the architecture spec. I don't think we
> need to paraphrase it.

Dropped.

> 
> > 
> > VPE doorbells allow KVM to wake VPEs (so, vcpus) as soon as the
> > hardware determines that sufficient conditions for the interrupt to
> > be
> > signalled have been met. This simplifies the wake-up path for vcpus
> > with GICv5 for LPIs and SPIs. NOTE: PPI pending state must still be
> > checked explicitly as the IRS never sees them.
> 
> Drop the note, it serves no purpose here.

Done.

> 
> > 
> > This change introduces support for the vgic_v5 doorbell domain. One
> > doorbell domain is created per GICv5 VM, and all VPEs have their
> > own
> > doorbell within this domain. When the doorbell fires, this is
> > tracked
> > (in gicv5_vpe.db_fired) and the corresponding vcpu is kicked.
> > 
> > Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
> > ---
> >  arch/arm64/kvm/vgic/vgic-init.c    |   5 +-
> >  arch/arm64/kvm/vgic/vgic-v5.c      | 143
> > +++++++++++++++++++++++++++++
> >  arch/arm64/kvm/vgic/vgic.h         |   1 +
> >  include/kvm/arm_vgic.h             |   6 ++
> >  include/linux/irqchip/arm-gic-v5.h |   2 +
> >  5 files changed, 156 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm64/kvm/vgic/vgic-init.c
> > b/arch/arm64/kvm/vgic/vgic-init.c
> > index 907057881b26a..984908a271c8d 100644
> > --- a/arch/arm64/kvm/vgic/vgic-init.c
> > +++ b/arch/arm64/kvm/vgic/vgic-init.c
> > @@ -500,8 +500,11 @@ static void kvm_vgic_dist_destroy(struct kvm
> > *kvm)
> >  		dist->vgic_cpu_base = VGIC_ADDR_UNDEF;
> >  	}
> >  
> > -	if (vgic_supports_direct_irqs(kvm))
> > +	if (dist->vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3 &&
> > +	    vgic_supports_direct_irqs(kvm))
> >  		vgic_v4_teardown(kvm);
> > +	else if (dist->vgic_model == KVM_DEV_TYPE_ARM_VGIC_V5)
> > +		vgic_v5_teardown(kvm);
> 
> nit: switch/case instead?

Ack. Done.

> 
> > 
> >  	xa_destroy(&dist->lpi_xa);
> >  }
> > diff --git a/arch/arm64/kvm/vgic/vgic-v5.c
> > b/arch/arm64/kvm/vgic/vgic-v5.c
> > index fd3d6299a2baa..4e0d52b309628 100644
> > --- a/arch/arm64/kvm/vgic/vgic-v5.c
> > +++ b/arch/arm64/kvm/vgic/vgic-v5.c
> > @@ -7,6 +7,7 @@
> >  
> >  #include <linux/bitops.h>
> >  #include <linux/irqchip/arm-vgic-info.h>
> > +#include <linux/irqdomain.h>
> >  
> >  #include "vgic.h"
> >  #include "vgic-v5-tables.h"
> > @@ -162,6 +163,138 @@ int vgic_v5_probe(const struct gic_kvm_info
> > *info)
> >  	return 0;
> >  }
> >  
> > +/*
> > + * This set of irq_chip functions is specific for doorbells.
> > + */
> > +static struct irq_chip vgic_v5_db_irq_chip = {
> 
> const?

Oops, yes. Done!

> 
> > +	.name = "GICv5-DB",
> > +	.irq_mask = irq_chip_mask_parent,
> > +	.irq_unmask = irq_chip_unmask_parent,
> > +	.irq_eoi = irq_chip_eoi_parent,
> > +	.irq_set_affinity = irq_chip_set_affinity_parent,
> > +	.irq_get_irqchip_state = irq_chip_get_parent_state,
> > +	.irq_set_irqchip_state = irq_chip_set_parent_state,
> > +	.flags = IRQCHIP_SET_TYPE_MASKED | IRQCHIP_SKIP_SET_WAKE |
> > +		 IRQCHIP_MASK_ON_SUSPEND,
> > +};
> > +
> > +static int vgic_v5_irq_db_domain_map(struct irq_domain *d,
> > unsigned int virq,
> > +				     u16 vpe_id)
> > +{
> > +	int ret;
> > +	u32 lpi;
> > +	irq_hw_number_t hwirq;
> > +	struct irq_chip *chip = &vgic_v5_db_irq_chip;
> > +	struct irq_data *irqd =
> > irq_desc_get_irq_data(irq_to_desc(virq));
> > +
> > +	/*
> > +	 * For the DB domain, we don't use the same hwirq as for
> > LPIs.
> > +	 */
> > +	hwirq = vpe_id;
> > +
> > +	ret = gicv5_alloc_lpi();
> 
> NAK. Allocating LPIs is the task of the underlying domain that
> manages
> LPIs, and absolutely not the vgic code.

Yeah, that's rather wonky! Alas that is (hopefully soon was - see
below) how the other domains using the GICv5 LPI domain work today.

> 
> > +	if (ret < 0)
> > +		return ret;
> > +	lpi = ret;
> > +
> > +	ret = irq_domain_alloc_irqs_parent(d, virq, 1, &lpi);
> 
> Why? I'd expect to see an irq_domain_alloc_irqs() for the whole VM,
> and be done with it.
> 
> The whole allocation/freeing of LPIs is upside down. You really
> should
> not have to do this, and I'd strongly suggest you align the way the
> doorbell domain is constructed with the way GICv4 does it.

Following our offline discussion around the LPI wackiness, I've posted
a small series to fix the underlying cause of this mess - the fact that
the LPI allocation for GICv5 was not owned by the LPI domain, but
rather was managed by other domains in the hierarchy (IPI & ITS's MSI
domains).

This series is here:
https://lore.kernel.org/all/20260430153352.3654325-1-sascha.bischoff@arm.com/

Based on these changes, I've re-worked this to do what you suggested,
and now alloc the domain and all DB virqs in one go.

Thanks,
Sascha

> 
> Thanks,
> 
> 	M.
>