Re: [PATCH v4 08/48] KVM: arm64: gic-v5: Add VPE doorbell domain

[Marc Zyngier <maz@kernel.org>]

On Fri, 24 Jul 2026 11:50:28 +0100,
Sascha Bischoff <Sascha.Bischoff@arm.com> wrote:
> 
> VPE doorbells allow the GICv5 hardware to notify KVM when an SPI or LPI
> can be signalled to a non-resident VPE. This provides the mechanism used
> to wake blocked vcpus once the hardware determines that the interrupt is
> eligible to be delivered.
> 
> Add support for a per-VM VPE doorbell irq domain. The domain is created
> under the GICv5 LPI domain, with one doorbell allocated per VPE. Store
> the allocated doorbell base in the VM's GICv5 state so that later
> patches can request per-vcpu doorbell IRQs and use them for IRS
> commands and wakeups.
> 
> Add the per-VPE doorbell state to the GICv5 CPU interface state. The
> doorbell IRQ number is populated when the IRQs are requested, and the
> db_fired state is used by later patches once doorbell delivery is wired
> up.
> 
> Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
> ---
>  arch/arm64/kvm/vgic/vgic-init.c    |  18 ++--
>  arch/arm64/kvm/vgic/vgic-v5.c      | 139 +++++++++++++++++++++++++++++
>  arch/arm64/kvm/vgic/vgic.h         |   1 +
>  include/kvm/arm_vgic.h             |   4 +
>  include/linux/irqchip/arm-gic-v5.h |   2 +
>  5 files changed, 158 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
> index 907057881b26a..625d352756fcf 100644
> --- a/arch/arm64/kvm/vgic/vgic-init.c
> +++ b/arch/arm64/kvm/vgic/vgic-init.c
> @@ -492,16 +492,22 @@ static void kvm_vgic_dist_destroy(struct kvm *kvm)
>  	dist->nr_spis = 0;
>  	dist->vgic_dist_base = VGIC_ADDR_UNDEF;
>  
> -	if (dist->vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3) {
> +	switch (dist->vgic_model) {
> +	case KVM_DEV_TYPE_ARM_VGIC_V2:
> +		dist->vgic_cpu_base = VGIC_ADDR_UNDEF;
> +		break;
> +	case KVM_DEV_TYPE_ARM_VGIC_V3:
>  		list_for_each_entry_safe(rdreg, next, &dist->rd_regions, list)
>  			vgic_v3_free_redist_region(kvm, rdreg);
>  		INIT_LIST_HEAD(&dist->rd_regions);
> -	} else {
> -		dist->vgic_cpu_base = VGIC_ADDR_UNDEF;
> -	}
>  
> -	if (vgic_supports_direct_irqs(kvm))
> -		vgic_v4_teardown(kvm);
> +		if (vgic_supports_direct_irqs(kvm))
> +			vgic_v4_teardown(kvm);
> +		break;
> +	case KVM_DEV_TYPE_ARM_VGIC_V5:
> +		vgic_v5_teardown(kvm);
> +		break;
> +	}
>  
>  	xa_destroy(&dist->lpi_xa);
>  }
> diff --git a/arch/arm64/kvm/vgic/vgic-v5.c b/arch/arm64/kvm/vgic/vgic-v5.c
> index 3f7b132110114..ebd7c3ede975f 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"
>  
> @@ -152,6 +153,134 @@ int vgic_v5_probe(const struct gic_kvm_info *info)
>  	return 0;
>  }
>  
> +/*
> + * This set of irq_chip functions is specific for doorbells.
> + */
> +static const struct irq_chip vgic_v5_db_irq_chip = {
> +	.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 void vgic_v5_irq_db_domain_free(struct irq_domain *domain,
> +				       unsigned int virq, unsigned int nr_irqs)
> +{
> +	int i;
> +
> +	for (i = 0; i < nr_irqs; i++) {
> +		struct irq_data *d = irq_domain_get_irq_data(domain, virq + i);
> +
> +		irq_set_handler(virq + i, NULL);
> +		irq_domain_reset_irq_data(d);
> +	}
> +
> +	irq_domain_free_irqs_parent(domain, virq, nr_irqs);
> +}
> +
> +static int vgic_v5_irq_db_domain_alloc(struct irq_domain *domain,
> +				       unsigned int virq, unsigned int nr_irqs,
> +				       void *arg)
> +{
> +	const struct irq_chip *chip = &vgic_v5_db_irq_chip;
> +	struct vgic_v5_vm *vm = arg;
> +	struct irq_data *irqd;
> +	int ret;
> +
> +	if (!vm) {
> +		kvm_err("invalid parameter for doorbell irq allocation\n");
> +		return -EINVAL;
> +	}

How can this happen?

> +
> +	ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, NULL);
> +	if (ret)
> +		return ret;
> +
> +	for (int i = 0; i < nr_irqs; i++) {
> +		irq_domain_set_hwirq_and_chip(domain, virq + i, i, chip,
> +					      domain->host_data);
> +		irqd = irq_desc_get_irq_data(irq_to_desc(virq + i));
> +		irqd_set_single_target(irqd);
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct irq_domain_ops vgic_v5_irq_db_domain_ops = {
> +	.alloc = vgic_v5_irq_db_domain_alloc,
> +	.free = vgic_v5_irq_db_domain_free,
> +};
> +
> +static int vgic_v5_create_per_vm_domain(struct kvm *kvm)
> +{
> +	struct vgic_v5_vm *vm = &kvm->arch.vgic.gicv5_vm;
> +	int nr_vcpus = atomic_read(&kvm->online_vcpus);
> +	int id = task_pid_nr(current);
> +	int ret, db_virq = 0;
> +
> +	if (!gicv5_global_data.lpi_domain) {
> +		kvm_err("LPI domain uninitialized, can't set up KVM Doorbells\n");
> +		return -ENODEV;
> +	}

Surely we should fail KVM's initialisation if we don't have the LPI
domain, rather than checking this at the point of creating the
doorbells. I can't imagine how we'd run on GICv5 without LPIs
anyway...

> +
> +	vm->fwnode = irq_domain_alloc_named_id_fwnode("GICv5-vpe-db", id);
> +	if (!vm->fwnode)
> +		return -ENOMEM;
> +
> +	/*
> +	 * KVM per-VM VPE DB domain; child of LPI domain; only ever handles
> +	 * doorbells. We know how many doorbells we have, and therefore we
> +	 * create a linear domain.

The linear domain thing is an irqdomain implementation detail. We
don't need to capture this here.

> +	 */
> +	vm->domain = irq_domain_create_hierarchy(gicv5_global_data.lpi_domain,
> +						 0, nr_vcpus, vm->fwnode,
> +						 &vgic_v5_irq_db_domain_ops, vm);
> +	if (!vm->domain) {
> +		ret = -ENOMEM;
> +		goto err;
> +	}
> +
> +	db_virq = irq_domain_alloc_irqs(vm->domain, nr_vcpus, NUMA_NO_NODE, vm);
> +	if (db_virq <= 0) {
> +		ret = db_virq;
> +		goto err;
> +	}
> +
> +	kvm->arch.vgic.gicv5_vm.vpe_db_base = db_virq;
> +
> +	return 0;
> +
> +err:
> +	if (db_virq > 0)
> +		irq_domain_free_irqs(db_virq, nr_vcpus);
> +	if (vm->domain)
> +		irq_domain_remove(vm->domain);
> +	if (vm->fwnode)
> +		irq_domain_free_fwnode(vm->fwnode);
> +
> +	kvm->arch.vgic.gicv5_vm.vpe_db_base = 0;
> +	vm->domain = NULL;
> +	vm->fwnode = NULL;
> +
> +	return ret;
> +}
> +
> +static void vgic_v5_teardown_per_vm_domain(struct vgic_v5_vm *vm)
> +{
> +	if (!vm->domain)
> +		return;
> +
> +	irq_domain_remove(vm->domain);
> +	irq_domain_free_fwnode(vm->fwnode);
> +	vm->domain = NULL;
> +	vm->fwnode = NULL;
> +}
> +
>  void vgic_v5_reset(struct kvm_vcpu *vcpu)
>  {
>  	/*
> @@ -167,10 +296,16 @@ void vgic_v5_reset(struct kvm_vcpu *vcpu)
>  	vcpu->arch.vgic_cpu.num_pri_bits = 5;
>  }
>  
> +void vgic_v5_teardown(struct kvm *kvm)
> +{
> +	vgic_v5_teardown_per_vm_domain(&kvm->arch.vgic.gicv5_vm);
> +}

Since this is a global symbol, I'd like to make sure this is never
called concurrently with anything related. Could you please add

	lockdep_assert_held(&kvm->arch.config_lock);

here?

> +
>  int vgic_v5_init(struct kvm *kvm)
>  {
>  	struct kvm_vcpu *vcpu;
>  	unsigned long idx;
> +	int ret;

and here?

>  
>  	if (vgic_initialized(kvm))
>  		return 0;
> @@ -182,6 +317,10 @@ int vgic_v5_init(struct kvm *kvm)
>  		}
>  	}
>  
> +	ret = vgic_v5_create_per_vm_domain(kvm);
> +	if (ret)
> +		return ret;
> +
>  	/* We only allow userspace to drive the SW_PPI, if it is implemented. */
>  	bitmap_zero(kvm->arch.vgic.gicv5_vm.userspace_ppis,
>  		    VGIC_V5_NR_PRIVATE_IRQS);
> diff --git a/arch/arm64/kvm/vgic/vgic.h b/arch/arm64/kvm/vgic/vgic.h
> index f45f7e3ec4d6e..f2f5fdc3211d7 100644
> --- a/arch/arm64/kvm/vgic/vgic.h
> +++ b/arch/arm64/kvm/vgic/vgic.h
> @@ -366,6 +366,7 @@ void vgic_debug_destroy(struct kvm *kvm);
>  int vgic_v5_probe(const struct gic_kvm_info *info);
>  void vgic_v5_reset(struct kvm_vcpu *vcpu);
>  int vgic_v5_init(struct kvm *kvm);
> +void vgic_v5_teardown(struct kvm *kvm);
>  int vgic_v5_map_resources(struct kvm *kvm);
>  void vgic_v5_set_ppi_ops(struct kvm_vcpu *vcpu, u32 vintid);
>  bool vgic_v5_has_pending_ppi(struct kvm_vcpu *vcpu);
> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> index 8d65a18fefb80..bff2b7c896d55 100644
> --- a/include/kvm/arm_vgic.h
> +++ b/include/kvm/arm_vgic.h
> @@ -392,6 +392,10 @@ struct vgic_v5_vm {
>  	 * convenient way to do that).
>  	 */
>  	DECLARE_BITMAP(vgic_ppi_hmr, VGIC_V5_NR_PRIVATE_IRQS);
> +
> +	struct fwnode_handle	*fwnode;
> +	struct irq_domain	*domain;
> +	int			vpe_db_base;
>  };
>  
>  struct vgic_dist {
> diff --git a/include/linux/irqchip/arm-gic-v5.h b/include/linux/irqchip/arm-gic-v5.h
> index 83cfcd056b250..21c8a69f99bb6 100644
> --- a/include/linux/irqchip/arm-gic-v5.h
> +++ b/include/linux/irqchip/arm-gic-v5.h
> @@ -613,6 +613,8 @@ void gicv5_irs_syncr(void);
>  
>  /* Embedded in kvm.arch */
>  struct gicv5_vpe {
> +	int			db;
> +	bool			db_fired;
>  	bool			resident;
>  };

Thanks,

	M.

-- 
Jazz isn't dead. It just smells funny.