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

[Marc Zyngier <maz@kernel.org>]

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.

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

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

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

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

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

Thanks,

	M.

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