... using the new iommu_group infrastructure.

Because 'sibling' devices are now members of the same iommu_group,
implement the domctl by looking up the iommu_group of the pdev with the
matching SBDF and then finding all the assigned pdevs that are in the
group.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>

v2:
 - Re-implement in the absence of a per-group devs list.
 - Make use of pci_sbdf_t.
---
 xen/drivers/passthrough/groups.c | 44 ++++++++++++++++++++++++++++++++++
 xen/drivers/passthrough/pci.c    | 51 ++--------------------------------------
 xen/include/xen/iommu.h          |  2 ++
 3 files changed, 48 insertions(+), 49 deletions(-)

diff --git a/xen/drivers/passthrough/groups.c b/xen/drivers/passthrough/groups.c
index 1a2f461c87..6d8064e4f4 100644
--- a/xen/drivers/passthrough/groups.c
+++ b/xen/drivers/passthrough/groups.c
@@ -12,8 +12,12 @@
  * GNU General Public License for more details.
  */
 
+#include <xen/guest_access.h>
 #include <xen/iommu.h>
+#include <xen/pci.h>
 #include <xen/radix-tree.h>
+#include <xen/sched.h>
+#include <xsm/xsm.h>
 
 struct iommu_group {
     unsigned int id;
@@ -81,6 +85,46 @@ int iommu_group_assign(struct pci_dev *pdev, void *arg)
     return 0;
 }
 
+int iommu_get_device_group(struct domain *d, pci_sbdf_t sbdf,
+                           XEN_GUEST_HANDLE_64(uint32) buf, int max_sdevs)
+{
+    struct iommu_group *grp = NULL;
+    struct pci_dev *pdev;
+    unsigned int i = 0;
+
+    pcidevs_lock();
+
+    for_each_pdev ( d, pdev )
+    {
+        if ( pdev->sbdf.sbdf == sbdf.sbdf )
+        {
+            grp = pdev->grp;
+            break;
+        }
+    }
+
+    if ( !grp )
+        goto out;
+
+    for_each_pdev ( d, pdev )
+    {
+        if ( xsm_get_device_group(XSM_HOOK, pdev->sbdf.sbdf) ||
+             pdev->grp != grp )
+            continue;
+
+        if ( unlikely(copy_to_guest_offset(buf, i++, &pdev->sbdf.sbdf, 1)) )
+        {
+            pcidevs_unlock();
+            return -EFAULT;
+        }
+    }
+
+ out:
+    pcidevs_unlock();
+
+    return i;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 179cb7e17e..3a5e90c176 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -1563,53 +1563,6 @@ int deassign_device(struct domain *d, u16 seg, u8 bus, u8 devfn)
     return ret;
 }
 
-static int iommu_get_device_group(
-    struct domain *d, u16 seg, u8 bus, u8 devfn,
-    XEN_GUEST_HANDLE_64(uint32) buf, int max_sdevs)
-{
-    const struct domain_iommu *hd = dom_iommu(d);
-    struct pci_dev *pdev;
-    int group_id, sdev_id;
-    u32 bdf;
-    int i = 0;
-    const struct iommu_ops *ops = hd->platform_ops;
-
-    if ( !iommu_enabled || !ops || !ops->get_device_group_id )
-        return 0;
-
-    group_id = ops->get_device_group_id(seg, bus, devfn);
-
-    pcidevs_lock();
-    for_each_pdev( d, pdev )
-    {
-        if ( (pdev->seg != seg) ||
-             ((pdev->bus == bus) && (pdev->devfn == devfn)) )
-            continue;
-
-        if ( xsm_get_device_group(XSM_HOOK, (seg << 16) | (pdev->bus << 8) | pdev->devfn) )
-            continue;
-
-        sdev_id = ops->get_device_group_id(seg, pdev->bus, pdev->devfn);
-        if ( (sdev_id == group_id) && (i < max_sdevs) )
-        {
-            bdf = 0;
-            bdf |= (pdev->bus & 0xff) << 16;
-            bdf |= (pdev->devfn & 0xff) << 8;
-
-            if ( unlikely(copy_to_guest_offset(buf, i, &bdf, 1)) )
-            {
-                pcidevs_unlock();
-                return -1;
-            }
-            i++;
-        }
-    }
-
-    pcidevs_unlock();
-
-    return i;
-}
-
 void iommu_dev_iotlb_flush_timeout(struct domain *d, struct pci_dev *pdev)
 {
     pcidevs_lock();
@@ -1666,11 +1619,11 @@ int iommu_do_pci_domctl(
         max_sdevs = domctl->u.get_device_group.max_sdevs;
         sdevs = domctl->u.get_device_group.sdev_array;
 
-        ret = iommu_get_device_group(d, seg, bus, devfn, sdevs, max_sdevs);
+        ret = iommu_get_device_group(d, PCI_SBDF3(seg, bus, devfn), sdevs,
+                                     max_sdevs);
         if ( ret < 0 )
         {
             dprintk(XENLOG_ERR, "iommu_get_device_group() failed!\n");
-            ret = -EFAULT;
             domctl->u.get_device_group.num_sdevs = 0;
         }
         else
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index c93f580fdc..ac764b41f9 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -321,6 +321,8 @@ extern struct page_list_head iommu_pt_cleanup_list;
 
 void iommu_groups_init(void);
 int iommu_group_assign(struct pci_dev *pdev, void *arg);
+int iommu_get_device_group(struct domain *d, pci_sbdf_t sbdf,
+                           XEN_GUEST_HANDLE_64(uint32) buf, int max_sdevs);
 
 #endif /* CONFIG_HAS_PCI */
 
-- 
2.11.0


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

On Mon, Jul 15, 2019 at 01:37:10PM +0100, Paul Durrant wrote:
> ... using the new iommu_group infrastructure.
> 
> Because 'sibling' devices are now members of the same iommu_group,
> implement the domctl by looking up the iommu_group of the pdev with the
> matching SBDF and then finding all the assigned pdevs that are in the
> group.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> ---
> Cc: Jan Beulich <jbeulich@suse.com>
> 
> v2:
>  - Re-implement in the absence of a per-group devs list.
>  - Make use of pci_sbdf_t.
> ---
>  xen/drivers/passthrough/groups.c | 44 ++++++++++++++++++++++++++++++++++
>  xen/drivers/passthrough/pci.c    | 51 ++--------------------------------------
>  xen/include/xen/iommu.h          |  2 ++
>  3 files changed, 48 insertions(+), 49 deletions(-)
> 
> diff --git a/xen/drivers/passthrough/groups.c b/xen/drivers/passthrough/groups.c
> index 1a2f461c87..6d8064e4f4 100644
> --- a/xen/drivers/passthrough/groups.c
> +++ b/xen/drivers/passthrough/groups.c
> @@ -12,8 +12,12 @@
>   * GNU General Public License for more details.
>   */
>  
> +#include <xen/guest_access.h>
>  #include <xen/iommu.h>
> +#include <xen/pci.h>
>  #include <xen/radix-tree.h>
> +#include <xen/sched.h>
> +#include <xsm/xsm.h>
>  
>  struct iommu_group {
>      unsigned int id;
> @@ -81,6 +85,46 @@ int iommu_group_assign(struct pci_dev *pdev, void *arg)
>      return 0;
>  }
>  
> +int iommu_get_device_group(struct domain *d, pci_sbdf_t sbdf,
> +                           XEN_GUEST_HANDLE_64(uint32) buf, int max_sdevs)

max_sdevs should be unsigned AFAICT, but it seems to be completely
unused. I think you want to do...

> +{
> +    struct iommu_group *grp = NULL;
> +    struct pci_dev *pdev;
> +    unsigned int i = 0;
> +
> +    pcidevs_lock();
> +
> +    for_each_pdev ( d, pdev )
> +    {
> +        if ( pdev->sbdf.sbdf == sbdf.sbdf )
> +        {
> +            grp = pdev->grp;
> +            break;
> +        }
> +    }
> +
> +    if ( !grp )
> +        goto out;
> +
> +    for_each_pdev ( d, pdev )
> +    {

if ( i == max_sdevs )
{
    pcidevs_unlock();
    return -ENOSPC;
}

Thanks, Roger.

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

> -----Original Message-----
> From: Roger Pau Monne <roger.pau@citrix.com>
> Sent: 15 July 2019 16:46
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: xen-devel@lists.xenproject.org; Jan Beulich <jbeulich@suse.com>
> Subject: Re: [Xen-devel] [PATCH v2 4/4] iommu / pci: re-implement XEN_DOMCTL_get_device_group...
> 
> On Mon, Jul 15, 2019 at 01:37:10PM +0100, Paul Durrant wrote:
> > ... using the new iommu_group infrastructure.
> >
> > Because 'sibling' devices are now members of the same iommu_group,
> > implement the domctl by looking up the iommu_group of the pdev with the
> > matching SBDF and then finding all the assigned pdevs that are in the
> > group.
> >
> > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > ---
> > Cc: Jan Beulich <jbeulich@suse.com>
> >
> > v2:
> >  - Re-implement in the absence of a per-group devs list.
> >  - Make use of pci_sbdf_t.
> > ---
> >  xen/drivers/passthrough/groups.c | 44 ++++++++++++++++++++++++++++++++++
> >  xen/drivers/passthrough/pci.c    | 51 ++--------------------------------------
> >  xen/include/xen/iommu.h          |  2 ++
> >  3 files changed, 48 insertions(+), 49 deletions(-)
> >
> > diff --git a/xen/drivers/passthrough/groups.c b/xen/drivers/passthrough/groups.c
> > index 1a2f461c87..6d8064e4f4 100644
> > --- a/xen/drivers/passthrough/groups.c
> > +++ b/xen/drivers/passthrough/groups.c
> > @@ -12,8 +12,12 @@
> >   * GNU General Public License for more details.
> >   */
> >
> > +#include <xen/guest_access.h>
> >  #include <xen/iommu.h>
> > +#include <xen/pci.h>
> >  #include <xen/radix-tree.h>
> > +#include <xen/sched.h>
> > +#include <xsm/xsm.h>
> >
> >  struct iommu_group {
> >      unsigned int id;
> > @@ -81,6 +85,46 @@ int iommu_group_assign(struct pci_dev *pdev, void *arg)
> >      return 0;
> >  }
> >
> > +int iommu_get_device_group(struct domain *d, pci_sbdf_t sbdf,
> > +                           XEN_GUEST_HANDLE_64(uint32) buf, int max_sdevs)
> 
> max_sdevs should be unsigned AFAICT, but it seems to be completely
> unused. I think you want to do...
> 
> > +{
> > +    struct iommu_group *grp = NULL;
> > +    struct pci_dev *pdev;
> > +    unsigned int i = 0;
> > +
> > +    pcidevs_lock();
> > +
> > +    for_each_pdev ( d, pdev )
> > +    {
> > +        if ( pdev->sbdf.sbdf == sbdf.sbdf )
> > +        {
> > +            grp = pdev->grp;
> > +            break;
> > +        }
> > +    }
> > +
> > +    if ( !grp )
> > +        goto out;
> > +
> > +    for_each_pdev ( d, pdev )
> > +    {
> 
> if ( i == max_sdevs )
> {
>     pcidevs_unlock();
>     return -ENOSPC;
> }

Oh, I'm sure I used to have that... I don't know how it got dropped. It certainly needs to be there.

  Paul

> 
> Thanks, Roger.

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