On 10/08/2016 12:39, Christoffer Dall wrote:
> Since we are about to synchronize all accesses to kvm->devices using the
> kvm->lock mutex, we should hold this mutex while iterating over the list
> of devices in the ITS code.
>
> Also move the vgic_register_its_iodev function to where it's called and
> rename it to register_its_iodev to avoid having two almost identially
> named functions.
>
> Signed-off-by: Christoffer Dall <christoffer.dall@xxxxxxxxxx>
> ---
> virt/kvm/arm/vgic/vgic-its.c | 64 +++++++++++++++++++++++---------------------
> 1 file changed, 34 insertions(+), 30 deletions(-)
>
> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
> index 1cf9f59..4e76877 100644
> --- a/virt/kvm/arm/vgic/vgic-its.c
> +++ b/virt/kvm/arm/vgic/vgic-its.c
> @@ -1319,32 +1319,6 @@ void vgic_enable_lpis(struct kvm_vcpu *vcpu)
> its_sync_lpi_pending_table(vcpu);
> }
>
> -static int vgic_register_its_iodev(struct kvm *kvm, struct vgic_its *its)
> -{
> - struct vgic_io_device *iodev = &its->iodev;
> - int ret;
> -
> - if (!its->initialized)
> - return -EBUSY;
> -
> - if (IS_VGIC_ADDR_UNDEF(its->vgic_its_base))
> - return -ENXIO;
> -
> - iodev->regions = its_registers;
> - iodev->nr_regions = ARRAY_SIZE(its_registers);
> - kvm_iodevice_init(&iodev->dev, &kvm_io_gic_ops);
> -
> - iodev->base_addr = its->vgic_its_base;
> - iodev->iodev_type = IODEV_ITS;
> - iodev->its = its;
> - mutex_lock(&kvm->slots_lock);
> - ret = kvm_io_bus_register_dev(kvm, KVM_MMIO_BUS, iodev->base_addr,
> - KVM_VGIC_V3_ITS_SIZE, &iodev->dev);
> - mutex_unlock(&kvm->slots_lock);
> -
> - return ret;
> -}
> -
> #define INITIAL_BASER_VALUE \
> (GIC_BASER_CACHEABILITY(GITS_BASER, INNER, RaWb) | \
> GIC_BASER_CACHEABILITY(GITS_BASER, OUTER, SameAsInner) | \
> @@ -1526,6 +1500,32 @@ int kvm_vgic_register_its_device(void)
> KVM_DEV_TYPE_ARM_VGIC_ITS);
> }
>
> +static int register_its_iodev(struct kvm *kvm, struct vgic_its *its)
> +{
> + struct vgic_io_device *iodev = &its->iodev;
> + int ret;
> +
> + if (!its->initialized)
> + return -EBUSY;
> +
> + if (IS_VGIC_ADDR_UNDEF(its->vgic_its_base))
> + return -ENXIO;
> +
> + iodev->regions = its_registers;
> + iodev->nr_regions = ARRAY_SIZE(its_registers);
> + kvm_iodevice_init(&iodev->dev, &kvm_io_gic_ops);
> +
> + iodev->base_addr = its->vgic_its_base;
> + iodev->iodev_type = IODEV_ITS;
> + iodev->its = its;
> + mutex_lock(&kvm->slots_lock);
> + ret = kvm_io_bus_register_dev(kvm, KVM_MMIO_BUS, iodev->base_addr,
> + KVM_VGIC_V3_ITS_SIZE, &iodev->dev);
> + mutex_unlock(&kvm->slots_lock);
> +
> + return ret;
> +}
> +
> /*
> * Registers all ITSes with the kvm_io_bus framework.
> * To follow the existing VGIC initialization sequence, this has to be
> @@ -1536,19 +1536,23 @@ int vgic_register_its_iodevs(struct kvm *kvm)
> struct kvm_device *dev;
> int ret = 0;
>
> + mutex_lock(&kvm->lock);
> list_for_each_entry(dev, &kvm->devices, vm_node) {
> if (dev->ops != &kvm_arm_vgic_its_ops)
> continue;
>
> - ret = vgic_register_its_iodev(kvm, dev->private);
> + ret = register_its_iodev(kvm, dev->private);
> if (ret)
> - return ret;
> + goto out;
> +
> /*
> * We don't need to care about tearing down previously
> - * registered ITSes, as the kvm_io_bus framework removes
> - * them for us if the VM gets destroyed.
> + * registered ITSes on error, as the kvm_io_bus framework
> + * removes them for us if the VM gets destroyed.
> */
> }
>
> +out:
> + mutex_unlock(&kvm->lock);
> return ret;
> }
>
Reviewed-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
