Re: [PATCH v3 6/9] KVM: arm/arm64: Introduce an allocator for in-kernel irq lines

Marc Zyngier
2017-06-08T14:07:19Z

On Thu, Jun 08 2017 at 3:34:43 pm BST, Christoffer Dall <cdall@linaro.org> wrote:
> Having multiple devices being able to signal the same interrupt line is
> very confusing and almost certainly guarantees a configuration error.
>
> Therefore, introduce a very simple allocator which allows a device to
> claim an interrupt line from the vgic for a given VM.
>
> Signed-off-by: Christoffer Dall <cdall@linaro.org>
> ---
> include/kvm/arm_vgic.h | 5 +++++
> virt/kvm/arm/vgic/vgic.c | 33 +++++++++++++++++++++++++++++++++
> 2 files changed, 38 insertions(+)
>
> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> index dde59fb..5d5b345 100644
> --- a/include/kvm/arm_vgic.h
> +++ b/include/kvm/arm_vgic.h
> @@ -121,6 +121,9 @@ struct vgic_irq {
> u8 source; /* GICv2 SGIs only */
> u8 priority;
> enum vgic_irq_config config; /* Level or edge */
> +
> + void *owner; /* Opaque pointer to reserve an interrupt
> + for in-kernel devices. */
> };
>
> struct vgic_register_region;
> @@ -340,4 +343,6 @@ int kvm_send_userspace_msi(struct kvm *kvm, struct kvm_msi *msi);
> */
> int kvm_vgic_setup_default_irq_routing(struct kvm *kvm);
>
> +int kvm_vgic_set_owner(struct kvm_vcpu *vcpu, unsigned int intid, void *owner);
> +
> #endif /* __KVM_ARM_VGIC_H */
> diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
> index c66feac..9628945 100644
> --- a/virt/kvm/arm/vgic/vgic.c
> +++ b/virt/kvm/arm/vgic/vgic.c
> @@ -435,6 +435,39 @@ int kvm_vgic_unmap_phys_irq(struct kvm_vcpu *vcpu, unsigned int virt_irq)
> }
>
> /**
> + * kvm_vgic_set_owner - Set the owner of an interrupt for a VM
> + *
> + * @vcpu: Pointer to the VCPU (used for PPIs)
> + * @intid: The virtual INTID identifying the interrupt (PPI or SPI)
> + * @owner: Opaque pointer to the owner
> + *
> + * Returns 0 if intid is not already used by another in-kernel device and the
> + * owner is set, otherwise returns an error code.
> + */
> +int kvm_vgic_set_owner(struct kvm_vcpu *vcpu, unsigned int intid, void *owner)
> +{
> + struct vgic_irq *irq;
> + int ret = 0;
> +
> + if (!vgic_initialized(vcpu->kvm))
> + return -EAGAIN;
> +
> + /* SGIs and LPIs cannot be wired up to any device */
> + if (!irq_is_ppi(intid) && !vgic_valid_spi(vcpu->kvm, intid))
> + return -EINVAL;
> +
> + irq = vgic_get_irq(vcpu->kvm, vcpu, intid);
> + spin_lock(&irq->irq_lock);
> + if (irq->owner && irq->owner != owner)
> + ret = -EEXIST;
> + else
> + irq->owner = owner;

Is there any case where you'd end-up setting the owner multiple times?
Or is that just convenient?

> + spin_unlock(&irq->irq_lock);
> +
> + return ret;
> +}
> +
> +/**
> * vgic_prune_ap_list - Remove non-relevant interrupts from the list
> *
> * @vcpu: The VCPU pointer

Otherwise,

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

M.
--
Jazz is not dead, it just smell funny.