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

[Jan Beulich]

>>> On 17.07.18 at 11:48,  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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

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

[Roger Pau Monne]

So that pci_{add/remove}_device work correctly with vpci. Note that
this requires moving vpci_add_handlers out of the init section.

Signed-off-by: Roger Pau Monné 
Reviewed-by: Wei Liu 
---
Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Jan Beulich 
Cc: Julien Grall 
Cc: Konrad Rzeszutek Wilk 
Cc: Stefano Stabellini 
Cc: Tim Deegan 
Cc: Wei Liu 
---
 xen/drivers/passthrough/pci.c | 8 
 xen/drivers/vpci/vpci.c   | 2 +-
 xen/include/xen/vpci.h| 2 ++
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index a5d59b83b7..a712db0294 100644
--- 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);
diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
index 0569459699..732d3525ae 100644
--- a/xen/drivers/vpci/vpci.c
+++ b/xen/drivers/vpci/vpci.c
@@ -72,7 +72,7 @@ void vpci_remove_device(struct pci_dev *pdev)
 spin_unlock(&pdev->vpci_lock);
 }
 
-int __hwdom_init vpci_add_handlers(struct pci_dev *pdev)
+int vpci_add_handlers(struct pci_dev *pdev)
 {
 unsigned int i;
 int rc = 0;
diff --git a/xen/include/xen/vpci.h b/xen/include/xen/vpci.h
index 208667227e..e629224088 100644
--- a/xen/include/xen/vpci.h
+++ b/xen/include/xen/vpci.h
@@ -224,6 +224,8 @@ static inline int vpci_add_handlers(struct pci_dev *pdev)
 return 0;
 }
 
+static inline void vpci_remove_device(struct pci_dev *pdev) { }
+
 static inline void vpci_dump_msi(void) { }
 
 static inline uint32_t vpci_read(pci_sbdf_t sbdf, unsigned int reg,
-- 
2.17.1


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