Re: [PATCH v8 06/28] vfio: refactor out vfio_interrupt_setup()

["Cédric Le Goater" <clg@redhat.com>]

On 2/19/25 15:48, John Levon wrote:
> Refactor the interrupt setup code out of vfio_realize(), as we will
> later need this for vfio-user too.
> 
> Signed-off-by: John Levon <john.levon@nutanix.com>
> ---
>   hw/vfio/pci.c | 54 +++++++++++++++++++++++++++++++--------------------
>   1 file changed, 33 insertions(+), 21 deletions(-)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 89d900e9cf..5fb6c4c4c6 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -2957,6 +2957,37 @@ static void vfio_unregister_req_notifier(VFIOPCIDevice *vdev)
>       vdev->req_enabled = false;
>   }
>   
> +static bool vfio_interrupt_setup(VFIOPCIDevice *vdev, Error **errp)
> +{
> +    PCIDevice *pdev = &vdev->pdev;
> +
> +    /* QEMU emulates all of MSI & MSIX */
> +    if (pdev->cap_present & QEMU_PCI_CAP_MSIX) {
> +        memset(vdev->emulated_config_bits + pdev->msix_cap, 0xff,
> +               MSIX_CAP_LENGTH);
> +    }
> +
> +    if (pdev->cap_present & QEMU_PCI_CAP_MSI) {
> +        memset(vdev->emulated_config_bits + pdev->msi_cap, 0xff,
> +               vdev->msi_cap_size);
> +    }
> +
> +    if (vfio_pci_read_config(&vdev->pdev, PCI_INTERRUPT_PIN, 1)) {
> +        vdev->intx.mmap_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL,
> +                                                  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);
> +        if (!vfio_intx_enable(vdev, errp)) {
> +            pci_device_set_intx_routing_notifier(&vdev->pdev, NULL);
> +            kvm_irqchip_remove_change_notifier(&vdev->irqchip_change_notifier);
> +            return false;
> +        }
> +    }
> +    return true;
> +}
> +
>   static void vfio_realize(PCIDevice *pdev, Error **errp)
>   {
>       ERRP_GUARD();
> @@ -3157,27 +3188,8 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
>           }
>       }
>   
> -    /* QEMU emulates all of MSI & MSIX */
> -    if (pdev->cap_present & QEMU_PCI_CAP_MSIX) {
> -        memset(vdev->emulated_config_bits + pdev->msix_cap, 0xff,
> -               MSIX_CAP_LENGTH);
> -    }
> -
> -    if (pdev->cap_present & QEMU_PCI_CAP_MSI) {
> -        memset(vdev->emulated_config_bits + pdev->msi_cap, 0xff,
> -               vdev->msi_cap_size);
> -    }
> -
> -    if (vfio_pci_read_config(&vdev->pdev, PCI_INTERRUPT_PIN, 1)) {
> -        vdev->intx.mmap_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL,
> -                                                  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);
> -        if (!vfio_intx_enable(vdev, errp)) {
> -            goto out_deregister;
> -        }
> +    if (!vfio_interrupt_setup(vdev, errp)) {
> +        goto out_teardown;

is that the correct exit label ?


Thanks,

C.


>       }
>   
>       if (vdev->display != ON_OFF_AUTO_OFF) {