On Tue, Nov 26, 2019 at 05:01:47PM +1100, David Gibson wrote:
> VFIO PCI devices already respond to the pci intx routing notifier, in order
> to update kernel irqchip mappings when routing is updated. However this
> won't handle the case where the irqchip itself is replaced by a different
> model while retaining the same routing. This case can happen on
> the pseries machine type due to PAPR feature negotiation.
>
> To handle that case, add a handler for the irqchip change notifier, which
> does much the same thing as the routing notifier, but is unconditional,
> rather than being a no-op when the routing hasn't changed.
>
> Cc: Alex Williamson <alex.william...@redhat.com>
> Cc: Alexey Kardashevskiy <a...@ozlabs.ru>
>
> Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
> Tested-by: Alex Williamson <alex.william...@redhat.com>
> Reviewed-by: Alex Williamson <alex.william...@redhat.com>
> Reviewed-by: Greg Kurz <gr...@kaod.org>
> Acked-by: Alex Williamson <alex.william...@redhat.com>
> ---
[...]
> @@ -2973,30 +2981,32 @@ static void vfio_realize(PCIDevice *pdev, Error
> **errp)
> vfio_intx_mmap_enable,
> vdev);
> pci_device_set_intx_routing_notifier(&vdev->pdev,
> vfio_intx_routing_notifier);
> + vdev->irqchip_change_notifier.notify = vfio_irqchip_change;
> + kvm_irqchip_add_change_notifier(&vdev->irqchip_change_notifier);
This code is conditional on
(vfio_pci_read_config(&vdev->pdev, PCI_INTERRUPT_PIN, 1)).
However:
[...]
> -out_teardown:
> +out_deregister:
> pci_device_set_intx_routing_notifier(&vdev->pdev, NULL);
> + kvm_irqchip_remove_change_notifier(&vdev->irqchip_change_notifier);
> +out_teardown:
> vfio_teardown_msi(vdev);
> vfio_bars_exit(vdev);
> error:
> @@ -3064,6 +3076,7 @@ static void vfio_exitfn(PCIDevice *pdev)
> vfio_unregister_req_notifier(vdev);
> vfio_unregister_err_notifier(vdev);
> pci_device_set_intx_routing_notifier(&vdev->pdev, NULL);
> + kvm_irqchip_remove_change_notifier(&vdev->irqchip_change_notifier);
This is unconditional. This doesn't look safe, and might be the
cause of the crash reported at
https://bugzilla.redhat.com/show_bug.cgi?id=1782678
--
Eduardo
