Re: [Xen-devel] [PATCH v2 09/11] pci: add vpci hooks for device addition/removal

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 17.07.18 at 11:48, <roger.pau@xxxxxxxxxx> wrote:
> --- a/xen/drivers/passthrough/pci.c
> +++ b/xen/drivers/passthrough/pci.c
> @@ -768,6 +768,13 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn,
>              goto out;
>          }
>  
> +        ret = vpci_add_handlers(pdev);
> +        if ( ret )
> +        {
> +            pdev->domain = NULL;
> +            goto out;
> +        }
> +
>          list_add(&pdev->domain_list, &hardware_domain->arch.pdev_list);
>      }
>      else
> @@ -812,6 +819,7 @@ int pci_remove_device(u16 seg, u8 bus, u8 devfn)
>      list_for_each_entry ( pdev, &pseg->alldevs_list, alldevs_list )
>          if ( pdev->bus == bus && pdev->devfn == devfn )
>          {
> +            vpci_remove_device(pdev);
>              ret = iommu_remove_device(pdev);
>              if ( pdev->domain )
>                  list_del(&pdev->domain_list);

These should have been here even without SR-IOV in mind, as we don't
guarantee to find all devices during our boot time scan. Does this have
any dependencies on the earlier patches in the series? If not, I think it
should go in as soon as it's ready, independent of the rest of this series.

However, error handling in the "add" case looks wrong:
iommu_add_device() either needs undoing, or your call should be done
earlier (and its effects undone in case iommu_add_device() fails).

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel