Re: [PATCH v5 06/26] KVM: arm/arm64: vITS: Add MSI translation helpers

From: Auger Eric
Date: Tue Nov 07 2017 - 15:34:22 EST


Hi,

On 27/10/2017 16:28, Marc Zyngier wrote:
> The whole MSI injection process is fairly monolithic. An MSI write
> gets turned into an injected LPI in one swift go. But this is actually
> a more fine-grained process:
>
> - First, a virtual ITS gets selected using the doorbell address
> - Then the DevID/EventID pair gets translated into an LPI
> - Finally the LPI is injected
>
> Since the GICv4 code needs the first two steps in order to match
> an IRQ routing entry to an LPI, let's expose them as helpers,
> and refactor the existing code to use them
>
> Reviewed-by: Christoffer Dall <cdall@xxxxxxxxxx>
> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
Reviewed-by: Eric Auger <eric.auger@xxxxxxxxxx>

Thanks

Eric
> ---
> virt/kvm/arm/vgic/vgic-its.c | 93 +++++++++++++++++++++++++-------------------
> virt/kvm/arm/vgic/vgic.h | 4 ++
> 2 files changed, 57 insertions(+), 40 deletions(-)
>
> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
> index f51c1e1b3f70..475dffd0a44c 100644
> --- a/virt/kvm/arm/vgic/vgic-its.c
> +++ b/virt/kvm/arm/vgic/vgic-its.c
> @@ -503,15 +503,8 @@ static unsigned long vgic_mmio_read_its_idregs(struct kvm *kvm,
> return 0;
> }
>
> -/*
> - * Find the target VCPU and the LPI number for a given devid/eventid pair
> - * and make this IRQ pending, possibly injecting it.
> - * Must be called with the its_lock mutex held.
> - * Returns 0 on success, a positive error value for any ITS mapping
> - * related errors and negative error values for generic errors.
> - */
> -static int vgic_its_trigger_msi(struct kvm *kvm, struct vgic_its *its,
> - u32 devid, u32 eventid)
> +int vgic_its_resolve_lpi(struct kvm *kvm, struct vgic_its *its,
> + u32 devid, u32 eventid, struct vgic_irq **irq)
> {
> struct kvm_vcpu *vcpu;
> struct its_ite *ite;
> @@ -530,26 +523,60 @@ static int vgic_its_trigger_msi(struct kvm *kvm, struct vgic_its *its,
> if (!vcpu->arch.vgic_cpu.lpis_enabled)
> return -EBUSY;
>
> - spin_lock(&ite->irq->irq_lock);
> - ite->irq->pending_latch = true;
> - vgic_queue_irq_unlock(kvm, ite->irq);
> -
> + *irq = ite->irq;
> return 0;
> }
>
> -static struct vgic_io_device *vgic_get_its_iodev(struct kvm_io_device *dev)
> +struct vgic_its *vgic_msi_to_its(struct kvm *kvm, struct kvm_msi *msi)
> {
> + u64 address;
> + struct kvm_io_device *kvm_io_dev;
> struct vgic_io_device *iodev;
>
> - if (dev->ops != &kvm_io_gic_ops)
> - return NULL;
> + if (!vgic_has_its(kvm))
> + return ERR_PTR(-ENODEV);
>
> - iodev = container_of(dev, struct vgic_io_device, dev);
> + if (!(msi->flags & KVM_MSI_VALID_DEVID))
> + return ERR_PTR(-EINVAL);
> +
> + address = (u64)msi->address_hi << 32 | msi->address_lo;
> +
> + kvm_io_dev = kvm_io_bus_get_dev(kvm, KVM_MMIO_BUS, address);
> + if (!kvm_io_dev)
> + return ERR_PTR(-EINVAL);
>
> + if (kvm_io_dev->ops != &kvm_io_gic_ops)
> + return ERR_PTR(-EINVAL);
> +
> + iodev = container_of(kvm_io_dev, struct vgic_io_device, dev);
> if (iodev->iodev_type != IODEV_ITS)
> - return NULL;
> + return ERR_PTR(-EINVAL);
>
> - return iodev;
> + return iodev->its;
> +}
> +
> +/*
> + * Find the target VCPU and the LPI number for a given devid/eventid pair
> + * and make this IRQ pending, possibly injecting it.
> + * Must be called with the its_lock mutex held.
> + * Returns 0 on success, a positive error value for any ITS mapping
> + * related errors and negative error values for generic errors.
> + */
> +static int vgic_its_trigger_msi(struct kvm *kvm, struct vgic_its *its,
> + u32 devid, u32 eventid)
> +{
> + struct vgic_irq *irq = NULL;
> + int err;
> +
> + err = vgic_its_resolve_lpi(kvm, its, devid, eventid, &irq);
> + if (err)
> + return err;
> +
> + spin_lock(&irq->irq_lock);
> + irq->pending_latch = true;
> + vgic_queue_irq_unlock(kvm, irq);
> +
> + return 0;
> }
>
> /*
> @@ -560,30 +587,16 @@ static struct vgic_io_device *vgic_get_its_iodev(struct kvm_io_device *dev)
> */
> int vgic_its_inject_msi(struct kvm *kvm, struct kvm_msi *msi)
> {
> - u64 address;
> - struct kvm_io_device *kvm_io_dev;
> - struct vgic_io_device *iodev;
> + struct vgic_its *its;
> int ret;
>
> - if (!vgic_has_its(kvm))
> - return -ENODEV;
> -
> - if (!(msi->flags & KVM_MSI_VALID_DEVID))
> - return -EINVAL;
> -
> - address = (u64)msi->address_hi << 32 | msi->address_lo;
> -
> - kvm_io_dev = kvm_io_bus_get_dev(kvm, KVM_MMIO_BUS, address);
> - if (!kvm_io_dev)
> - return -EINVAL;
> + its = vgic_msi_to_its(kvm, msi);
> + if (IS_ERR(its))
> + return PTR_ERR(its);
>
> - iodev = vgic_get_its_iodev(kvm_io_dev);
> - if (!iodev)
> - return -EINVAL;
> -
> - mutex_lock(&iodev->its->its_lock);
> - ret = vgic_its_trigger_msi(kvm, iodev->its, msi->devid, msi->data);
> - mutex_unlock(&iodev->its->its_lock);
> + mutex_lock(&its->its_lock);
> + ret = vgic_its_trigger_msi(kvm, its, msi->devid, msi->data);
> + mutex_unlock(&its->its_lock);
>
> if (ret < 0)
> return ret;
> diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h
> index bf9ceab67c77..31b70326b966 100644
> --- a/virt/kvm/arm/vgic/vgic.h
> +++ b/virt/kvm/arm/vgic/vgic.h
> @@ -236,4 +236,8 @@ static inline int vgic_v3_max_apr_idx(struct kvm_vcpu *vcpu)
> }
> }
>
> +int vgic_its_resolve_lpi(struct kvm *kvm, struct vgic_its *its,
> + u32 devid, u32 eventid, struct vgic_irq **irq);
> +struct vgic_its *vgic_msi_to_its(struct kvm *kvm, struct kvm_msi *msi);
> +
> #endif
>