[Xen-devel] [PATCH v3 1/2] VT-d: re-phrase logic in vtd_set_hwdom_mapping() for clarity

[Paul Durrant <paul.durrant@xxxxxxxxxx>]

It is hard to reconcile the comment at the top of the loop in
vtd_set_hwdom_mapping() with the if statement following it. This patch
re-phrases the logic, preserving the semantics, but making it easier
to read.

The patch also modifies the Xen command line documentation to make it
clear that iommu_inclusive_mapping only applies to pages up to the 4GB
boundary.

NOTE: This patch also corrects the indentation of the printk() towards
      the end of vtd_set_hwdom_mapping().

Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
Reviewed-by: Roger Pau Monne <roger.pau@xxxxxxxxxx>
---
Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Cc: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Cc: Jan Beulich <jbeulich@xxxxxxxx>
Cc: Julien Grall <julien.grall@xxxxxxx>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
Cc: Tim Deegan <tim@xxxxxxx>
Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
Cc: Kevin Tian <kevin.tian@xxxxxxxxx>

v3:
 - Fix top calculation by introducing max_pfn.
 - Move comment.

v2:
 - Compare against GB(4) rather than 0xffffffff.
---
 docs/misc/xen-command-line.markdown   |  4 ++--
 xen/drivers/passthrough/vtd/x86/vtd.c | 34 +++++++++++++++++++---------------
 2 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/docs/misc/xen-command-line.markdown 
b/docs/misc/xen-command-line.markdown
index 8712a833a2..b75471b51a 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -1212,8 +1212,8 @@ wait descriptor timed out', try increasing this value.
 
 Use this to work around firmware issues providing incorrect RMRR entries.
 Rather than only mapping RAM pages for IOMMU accesses for Dom0, with this
-option all pages not marked as unusable in the E820 table will get a mapping
-established.
+option all pages up to 4GB, not marked as unusable in the E820 table, will
+get a mapping established.
 
 ### irq\_ratelimit (x86)
 > `= <integer>`
diff --git a/xen/drivers/passthrough/vtd/x86/vtd.c 
b/xen/drivers/passthrough/vtd/x86/vtd.c
index 88a60b3307..6551f01e31 100644
--- a/xen/drivers/passthrough/vtd/x86/vtd.c
+++ b/xen/drivers/passthrough/vtd/x86/vtd.c
@@ -110,30 +110,34 @@ void hvm_dpci_isairq_eoi(struct domain *d, unsigned int 
isairq)
 
 void __hwdom_init vtd_set_hwdom_mapping(struct domain *d)
 {
-    unsigned long i, j, tmp, top;
+    unsigned long i, j, tmp, top, max_pfn;
 
     BUG_ON(!is_hardware_domain(d));
 
-    top = max(max_pdx, pfn_to_pdx(0xffffffffUL >> PAGE_SHIFT) + 1);
+    max_pfn = (GB(4) >> PAGE_SHIFT) - 1;
+    top = max(max_pdx, pfn_to_pdx(max_pfn) + 1);
 
     for ( i = 0; i < top; i++ )
     {
+        unsigned long pfn = pdx_to_pfn(i);
+        bool map;
         int rc = 0;
 
         /*
-         * Set up 1:1 mapping for dom0. Default to use only conventional RAM
-         * areas and let RMRRs include needed reserved regions. When set, the
-         * inclusive mapping maps in everything below 4GB except unusable
-         * ranges.
+         * Set up 1:1 mapping for dom0. Default to include only
+         * conventional RAM areas and let RMRRs include needed reserved
+         * regions. When set, the inclusive mapping maps in every pfn up
+         * to 4GB except those that fall in unusable ranges.
          */
-        unsigned long pfn = pdx_to_pfn(i);
+        if ( pfn > max_pfn && !mfn_valid(_mfn(pfn)) )
+            continue;
+
+        if ( iommu_inclusive_mapping && pfn <= max_pfn )
+            map = !page_is_ram_type(pfn, RAM_TYPE_UNUSABLE);
+        else
+            map = page_is_ram_type(pfn, RAM_TYPE_CONVENTIONAL);
 
-        if ( pfn > (0xffffffffUL >> PAGE_SHIFT) ?
-             (!mfn_valid(_mfn(pfn)) ||
-              !page_is_ram_type(pfn, RAM_TYPE_CONVENTIONAL)) :
-             iommu_inclusive_mapping ?
-             page_is_ram_type(pfn, RAM_TYPE_UNUSABLE) :
-             !page_is_ram_type(pfn, RAM_TYPE_CONVENTIONAL) )
+        if ( !map )
             continue;
 
         /* Exclude Xen bits */
@@ -151,8 +155,8 @@ void __hwdom_init vtd_set_hwdom_mapping(struct domain *d)
         }
 
         if ( rc )
-           printk(XENLOG_WARNING VTDPREFIX " d%d: IOMMU mapping failed: %d\n",
-                  d->domain_id, rc);
+            printk(XENLOG_WARNING VTDPREFIX " d%d: IOMMU mapping failed: %d\n",
+                   d->domain_id, rc);
 
         if (!(i & (0xfffff >> (PAGE_SHIFT - PAGE_SHIFT_4K))))
             process_pending_softirqs();
-- 
2.11.0


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