On Wed, 27 Nov 2019 16:49:10 +0000,
Jiang Yi <giangyi@xxxxxxxxxx> wrote:
Hi Jiang,
Thanks for spotting this!
> Since irq_bypass_register_producer() is called after request_irq(), we
> should do tear-down in reverse order: irq_bypass_unregister_producer()
> then free_irq().
More importantly, free_irq() is going to releases resources that can
still be required by the del_producer callback. Notably, for arm64 and
GICv4:
free_irq(irq)
__free_irq(irq)
irq_domain_deactivate_irq(irq)
its_irq_domain_deactivate()
[unmap the VLPI from the ITS]
kvm_arch_irq_bypass_del_producer(cons, prod)
kvm_vgic_v4_unset_forwarding(kvm, irq, ...)
its_unmap_vlpi(irq)
[Unmap the VLPI from the ITS (again), remap the original LPI]
which isn't great, and has the potential to wedge the HW. Reversing
the two makes more sense: Unmap the VLPI, remap the LPI, and finally
unmap the LPI. I haven't checked what it does with VT-D.
> Signed-off-by: Jiang Yi <giangyi@xxxxxxxxxx>
> ---
> drivers/vfio/pci/vfio_pci_intrs.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/vfio/pci/vfio_pci_intrs.c b/drivers/vfio/pci/vfio_pci_intrs.c
> index 3fa3f728fb39..2056f3f85f59 100644
> --- a/drivers/vfio/pci/vfio_pci_intrs.c
> +++ b/drivers/vfio/pci/vfio_pci_intrs.c
> @@ -289,18 +289,18 @@ static int vfio_msi_set_vector_signal(struct vfio_pci_device *vdev,
> int irq, ret;
>
> if (vector < 0 || vector >= vdev->num_ctx)
> return -EINVAL;
>
> irq = pci_irq_vector(pdev, vector);
>
> if (vdev->ctx[vector].trigger) {
> - free_irq(irq, vdev->ctx[vector].trigger);
> irq_bypass_unregister_producer(&vdev->ctx[vector].producer);
> + free_irq(irq, vdev->ctx[vector].trigger);
> kfree(vdev->ctx[vector].name);
> eventfd_ctx_put(vdev->ctx[vector].trigger);
> vdev->ctx[vector].trigger = NULL;
> }
>
> if (fd < 0)
> return 0;
>
> --
> 2.17.1
>
>
FWIW:
Cc: stable@xxxxxxxxxxxxxxx # v4.4+
Fixes: 6d7425f109d26 ("vfio: Register/unregister irq_bypass_producer")
Reviewed-by: Marc Zyngier <maz@xxxxxxxxxx>
Thanks again,
M.
--
Jazz is not dead, it just smells funny.
