[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH v2 4/4] IOMMU/PCI: propagate get_device_group_id() failure


  • To: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Thu, 27 Jan 2022 15:49:47 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=DMhzatDsyvqO50atIdsWIGLVtdsSYBlhlA7D8k9xKVo=; b=F2qQ3HUCbgpyGRc3HGGv+pI7BljmFMJ+MwACJxWa9maIGqRCov7BnwW6yhwmy/1OyjyH+Opjy4yogHRoPGPaXlRNRypHeYdJx8owDW7gwwDr67NfkCl2vT8tG7cuH8600DK5RJIqsl6Qcu+6y7EWe1DJW/Rxcbw/pGyLQ/MI+cyardErKEKWPOsATBuSTMG2UGZ3H0iW9Dux2M5yEkjhEzGyqIF1W4QAYhzEgeaz6YEi+dRHez1N7oGA+4jZaLGcWS9TyhL868AiVrecHlRqFIYXuVVB7YjSamfhdkNB6QtrGl4j3Lqa5WdCl1Gvov4jvVVrJwMaHFrLyyIxhiJqCA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=iYfCO8JDt1Qf5ZdUj6PM2osKZmogIqPbjcvnELZeTH/Wu3XKGLpTUkfRodHb7/tWZEyVO+3/3JgYfgXoHY9XZC8lPBbfddw6/vAJMFD9c5dujmY+aC8rAv9PfYoC80YdBzI3mYldOHpAW+SjOU7ptz67ks9v4zW4/WawP6qtHMrCOsO/8jSfB5oOBNVkaAyJ/gckfzJk324hcX/zEbNbh+lYKr02OGRlmZ8mOcrn/xI0sTokR6FWPuFSceCa0GdG0wyUeQJqjvfAvnvmSdgk2kUzv9FT+2iZgCxfinX4GGFtByVFozzVri71jduYK/z5h8EV5WZBhWw3qRIGhQEcVQ==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Paul Durrant <paul@xxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Kevin Tian <kevin.tian@xxxxxxxxx>
  • Delivery-date: Thu, 27 Jan 2022 14:49:53 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

The VT-d hook can indicate an error, which shouldn't be ignored. Convert
the hook's return value to a proper error code, and let that bubble up.

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
---
I'm not convinced of the XSM related behavior here: It's neither clear
why the check gets performed on the possible further group members
instead of on the passed in device, nor - if the former is indeed
intended behavior - why the loop then simply gets continued instead of
returning an error. After all in such a case the reported "group" is
actually incomplete, which can't result in anything good.

I'm further unconvinced that it is correct for the AMD hook to never
return an error.
---
v2: New.

--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -1463,6 +1463,8 @@ static int iommu_get_device_group(
         return 0;
 
     group_id = iommu_call(ops, get_device_group_id, seg, bus, devfn);
+    if ( group_id < 0 )
+        return group_id;
 
     pcidevs_lock();
     for_each_pdev( d, pdev )
@@ -1477,6 +1479,12 @@ static int iommu_get_device_group(
             continue;
 
         sdev_id = iommu_call(ops, get_device_group_id, seg, b, df);
+        if ( sdev_id < 0 )
+        {
+            pcidevs_unlock();
+            return sdev_id;
+        }
+
         if ( (sdev_id == group_id) && (i < max_sdevs) )
         {
             bdf = (b << 16) | (df << 8);
@@ -1484,7 +1492,7 @@ static int iommu_get_device_group(
             if ( unlikely(copy_to_guest_offset(buf, i, &bdf, 1)) )
             {
                 pcidevs_unlock();
-                return -1;
+                return -EFAULT;
             }
             i++;
         }
@@ -1552,8 +1560,7 @@ int iommu_do_pci_domctl(
         ret = iommu_get_device_group(d, seg, bus, devfn, sdevs, max_sdevs);
         if ( ret < 0 )
         {
-            dprintk(XENLOG_ERR, "iommu_get_device_group() failed!\n");
-            ret = -EFAULT;
+            dprintk(XENLOG_ERR, "iommu_get_device_group() failed: %d\n", ret);
             domctl->u.get_device_group.num_sdevs = 0;
         }
         else
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -2564,10 +2564,11 @@ static int intel_iommu_assign_device(
 static int intel_iommu_group_id(u16 seg, u8 bus, u8 devfn)
 {
     u8 secbus;
+
     if ( find_upstream_bridge(seg, &bus, &devfn, &secbus) < 0 )
-        return -1;
-    else
-        return PCI_BDF2(bus, devfn);
+        return -ENODEV;
+
+    return PCI_BDF2(bus, devfn);
 }
 
 static int __must_check vtd_suspend(void)




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.