[v14,1/3] iommu VT-d: separate rmrr addition function.

Commit Message

Venu Busireddy Jan. 23, 2017, 6:20 p.m. UTC

In preparation for auxiliary RMRR data provided on Xen command line,
make RMRR adding a separate function.
Also free memery for rmrr device scope in error path.

Signed-off-by: Elena Ufimtseva <[e-mail]>
Signed-off-by: Venu Busireddy <[e-mail]>
Acked-by: Kevin Tian <[e-mail]>
---
 xen/drivers/passthrough/vtd/dmar.c | 123 ++++++++++++++++++++-----------------
 1 file changed, 65 insertions(+), 58 deletions(-)

Changes in v14:
 - Capture the return value from register_one_rmrr() and use it.

Comments

Jan Beulich Jan. 24, 2017, 8:33 a.m. UTC | #1

>>> On 23.01.17 at 19:20, <[e-mail]> wrote:
> In preparation for auxiliary RMRR data provided on Xen command line,
> make RMRR adding a separate function.
> Also free memery for rmrr device scope in error path.
> 
> Signed-off-by: Elena Ufimtseva <[e-mail]>
> Signed-off-by: Venu Busireddy <[e-mail]>
> Acked-by: Kevin Tian <[e-mail]>

Reviewed-by: Jan Beulich <[e-mail]>

> Changes in v14:
>  - Capture the return value from register_one_rmrr() and use it.

When you fix bugs in areas earlier offered tags apply to, I think
you should strictly drop any such tags. I'm pretty convinced
Kevin would re-offer his one in the case here, but you shouldn't
take this for given.

Jan

Tian, Kevin Jan. 25, 2017, 2:26 a.m. UTC | #2

> From: Jan Beulich [mailto:[e-mail]]
> Sent: Tuesday, January 24, 2017 4:34 PM
> 
> >>> On 23.01.17 at 19:20, <[e-mail]> wrote:
> > In preparation for auxiliary RMRR data provided on Xen command line,
> > make RMRR adding a separate function.
> > Also free memery for rmrr device scope in error path.
> >
> > Signed-off-by: Elena Ufimtseva <[e-mail]>
> > Signed-off-by: Venu Busireddy <[e-mail]>
> > Acked-by: Kevin Tian <[e-mail]>
> 
> Reviewed-by: Jan Beulich <[e-mail]>
> 
> > Changes in v14:
> >  - Capture the return value from register_one_rmrr() and use it.
> 
> When you fix bugs in areas earlier offered tags apply to, I think
> you should strictly drop any such tags. I'm pretty convinced
> Kevin would re-offer his one in the case here, but you shouldn't
> take this for given.
> 
> Jan

Thanks for remind. 

Acked-by: Kevin Tian <[e-mail]>

Patch

diff --git a/xen/drivers/passthrough/vtd/dmar.c b/xen/drivers/passthrough/vtd/dmar.c
index 08c1d2d..3c7c9b2 100644
--- a/xen/drivers/passthrough/vtd/dmar.c
+++ b/xen/drivers/passthrough/vtd/dmar.c
@@ -579,6 +579,68 @@  out:
     return ret;
 }
 
+static int register_one_rmrr(struct acpi_rmrr_unit *rmrru)
+{
+    bool ignore = false;
+    unsigned int i = 0;
+    int ret = 0;
+
+    /* Skip checking if segment is not accessible yet. */
+    if ( !pci_known_segment(rmrru->segment) )
+        i = UINT_MAX;
+
+    for ( ; i < rmrru->scope.devices_cnt; i++ )
+    {
+        u8 b = PCI_BUS(rmrru->scope.devices[i]);
+        u8 d = PCI_SLOT(rmrru->scope.devices[i]);
+        u8 f = PCI_FUNC(rmrru->scope.devices[i]);
+
+        if ( pci_device_detect(rmrru->segment, b, d, f) == 0 )
+        {
+            dprintk(XENLOG_WARNING VTDPREFIX,
+                    " Non-existent device (%04x:%02x:%02x.%u) is reported"
+                    " in RMRR (%"PRIx64", %"PRIx64")'s scope!\n",
+                    rmrru->segment, b, d, f,
+                    rmrru->base_address, rmrru->end_address);
+            ignore = true;
+        }
+        else
+        {
+            ignore = false;
+            break;
+        }
+    }
+
+    if ( ignore )
+    {
+        dprintk(XENLOG_WARNING VTDPREFIX,
+                "  Ignore the RMRR (%"PRIx64", %"PRIx64") due to "
+                "devices under its scope are not PCI discoverable!\n",
+                rmrru->base_address, rmrru->end_address);
+        scope_devices_free(&rmrru->scope);
+        xfree(rmrru);
+    }
+    else if ( rmrru->base_address > rmrru->end_address )
+    {
+        dprintk(XENLOG_WARNING VTDPREFIX,
+                "  The RMRR (%"PRIx64", %"PRIx64") is incorrect!\n",
+                rmrru->base_address, rmrru->end_address);
+        scope_devices_free(&rmrru->scope);
+        xfree(rmrru);
+        ret = -EFAULT;
+    }
+    else
+    {
+        if ( iommu_verbose )
+            dprintk(VTDPREFIX,
+                    "  RMRR region: base_addr %"PRIx64" end_addr %"PRIx64"\n",
+                    rmrru->base_address, rmrru->end_address);
+        acpi_register_rmrr_unit(rmrru);
+    }
+
+    return ret;
+}
+
 static int __init
 acpi_parse_one_rmrr(struct acpi_dmar_header *header)
 {
@@ -628,65 +690,10 @@  acpi_parse_one_rmrr(struct acpi_dmar_header *header)
     ret = acpi_parse_dev_scope(dev_scope_start, dev_scope_end,
                                &rmrru->scope, RMRR_TYPE, rmrr->segment);
 
-    if ( ret || (rmrru->scope.devices_cnt == 0) )
-        xfree(rmrru);
+    if ( !ret && (rmrru->scope.devices_cnt != 0) )
+        ret = register_one_rmrr(rmrru);
     else
-    {
-        u8 b, d, f;
-        bool_t ignore = 0;
-        unsigned int i = 0;
-
-        /* Skip checking if segment is not accessible yet. */
-        if ( !pci_known_segment(rmrr->segment) )
-            i = UINT_MAX;
-
-        for ( ; i < rmrru->scope.devices_cnt; i++ )
-        {
-            b = PCI_BUS(rmrru->scope.devices[i]);
-            d = PCI_SLOT(rmrru->scope.devices[i]);
-            f = PCI_FUNC(rmrru->scope.devices[i]);
-
-            if ( !pci_device_detect(rmrr->segment, b, d, f) )
-            {
-                printk(XENLOG_WARNING VTDPREFIX
-                       " Non-existent device (%04x:%02x:%02x.%u) reported in RMRR (%"PRIx64", %"PRIx64")'s scope!\n",
-                       rmrr->segment, b, d, f,
-                       rmrru->base_address, rmrru->end_address);
-                ignore = 1;
-            }
-            else
-            {
-                ignore = 0;
-                break;
-            }
-        }
-
-        if ( ignore )
-        {
-            printk(XENLOG_WARNING VTDPREFIX
-                   "  Ignore RMRR (%"PRIx64", %"PRIx64") (some devices in its scope are not PCI discoverable)\n",
-                   rmrru->base_address, rmrru->end_address);
-            scope_devices_free(&rmrru->scope);
-            xfree(rmrru);
-        }
-        else if ( base_addr > end_addr )
-        {
-            printk(XENLOG_WARNING VTDPREFIX
-                   "  RMRR (%"PRIx64", %"PRIx64") is incorrect\n",
-                   rmrru->base_address, rmrru->end_address);
-            scope_devices_free(&rmrru->scope);
-            xfree(rmrru);
-            ret = -EFAULT;
-        }
-        else
-        {
-            if ( iommu_verbose )
-                printk(VTDPREFIX
-                       "  RMRR region: base_addr %"PRIx64" end_address %"PRIx64"\n",
-                       rmrru->base_address, rmrru->end_address);
-            acpi_register_rmrr_unit(rmrru);
-        }
-    }
+        xfree(rmrru);
 
     return ret;
 }