[PATCH v9 0/4] iommu: add rmrr Xen command line option

[PATCH v9 0/4] iommu: add rmrr Xen command line option

[elena.ufimtseva]

From: Elena Ufimtseva <elena.ufimtseva@oracle.com>

v9 of rmrr command line patches.

Add Xen command line option rmrr to specify RMRR
regions for devices that are not defined in ACPI thus
causing IO Page Fault while booting dom0 in PVH mode.
These additional regions will be added to the list of
RMRR regions parsed from ACPI.

Changes in v9:
 - skip to next RMRR region if current overlaps with any in acpi_rmrr_units;
 - fix typos in commit messages;
 - remove clean up chages introduced by mistake in v8;

Changes in v8:
 - removed bogus debug in patch 1 with non-functional changes;
 - changed PRI_RMRRL macro for formatting to reflect the fact that two arguments
   are used, so make it PRI_RMRR(s,e) for formatting inclusive RMRR range;
   'L' is also removed from macro name, which meant to server as a type of arguments (%lx);
 - added overlapping check with RMRRs from ACPI;
 - added check based on paddr_bits for pfn's in extra RMRR range (not sure if
   its redundant with mfn_valid);
 - addressed while loop exit condition in extra RMRRs parser;

Changes in v7:
 - make sure RMRRs ranges are being checked correctly;
 - dont interrupt RMRRs checking if some of checks fails, instead
 continue to next RMRR;
 - make rmrr variable names more obvious;
 - fix debug output formatting to match type of rmrr range;
 - fix typos in rmrr command line document and in comments;

Elena Ufimtseva (4):
  pci: add PCI_SBDF and PCI_SEG macros
  iommu VT-d: separate rmrr addition function
  pci: add wrapper for parse_pci
  iommu: add rmrr Xen command line option for extra rmrrs

 docs/misc/xen-command-line.markdown |  13 ++
 xen/drivers/passthrough/vtd/dmar.c  | 334 +++++++++++++++++++++++++++++-------
 xen/drivers/pci/pci.c               |  11 ++
 xen/include/xen/pci.h               |   5 +
 4 files changed, 301 insertions(+), 62 deletions(-)

-- 
2.1.3

[PATCH v9 1/4] pci: add PCI_SBDF and PCI_SEG macros

[elena.ufimtseva]

From: Elena Ufimtseva <elena.ufimtseva@oracle.com>

In preparation for patch "iommu: add rmrr Xen command line option for
extra rmrrs" which will use it.

Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com>
---
 xen/include/xen/pci.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
index 3908146..414106a 100644
--- a/xen/include/xen/pci.h
+++ b/xen/include/xen/pci.h
@@ -33,6 +33,8 @@
 #define PCI_DEVFN2(bdf) ((bdf) & 0xff)
 #define PCI_BDF(b,d,f)  ((((b) & 0xff) << 8) | PCI_DEVFN(d,f))
 #define PCI_BDF2(b,df)  ((((b) & 0xff) << 8) | ((df) & 0xff))
+#define PCI_SBDF(s,b,d,f) ((((s) & 0xffff) << 16) | PCI_BDF(b,d,f))
+#define PCI_SEG(sbdf) (((sbdf) >> 16) & 0xffff)
 
 struct pci_dev_info {
     bool_t is_extfn;
-- 
2.1.3

[PATCH v9 2/4] iommu VT-d: separate rmrr addition function

[elena.ufimtseva]

From: Elena Ufimtseva <elena.ufimtseva@oracle.com>

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 <elena.ufimtseva@oracle.com>
---
 xen/drivers/passthrough/vtd/dmar.c | 126 +++++++++++++++++++------------------
 1 file changed, 65 insertions(+), 61 deletions(-)

diff --git a/xen/drivers/passthrough/vtd/dmar.c b/xen/drivers/passthrough/vtd/dmar.c
index 77ef708..a8e1e5d 100644
--- a/xen/drivers/passthrough/vtd/dmar.c
+++ b/xen/drivers/passthrough/vtd/dmar.c
@@ -585,6 +585,68 @@ out:
     return ret;
 }
 
+static int register_one_rmrr(struct acpi_rmrr_unit *rmrru)
+{
+    bool_t ignore = 0;
+    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 = 1;
+        }
+        else
+        {
+            ignore = 0;
+            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_address %"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)
 {
@@ -635,68 +697,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) )
+        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) )
-            {
-                dprintk(XENLOG_WARNING VTDPREFIX,
-                        " Non-existent device (%04x:%02x:%02x.%u) is 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 )
-        {
-            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 ( base_addr > end_addr )
-        {
-            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_address %"PRIx64"\n",
-                        rmrru->base_address, rmrru->end_address);
-            acpi_register_rmrr_unit(rmrru);
-        }
-    }
+        xfree(rmrru);
 
     return ret;
 }
-- 
2.1.3

[PATCH v9 3/4] pci: add wrapper for parse_pci

[elena.ufimtseva]

From: Elena Ufimtseva <elena.ufimtseva@oracle.com>

For sbdf's parsing in RMRR command line add __parse_pci with additional
parameter def_seg. __parse_pci will help to identify if segment was
found in string being parsed or default segment was used.
Make a wrapper parse_pci so the rest of the callers are not affected.

Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
 xen/drivers/pci/pci.c | 11 +++++++++++
 xen/include/xen/pci.h |  3 +++
 2 files changed, 14 insertions(+)

diff --git a/xen/drivers/pci/pci.c b/xen/drivers/pci/pci.c
index ca07ed0..788a356 100644
--- a/xen/drivers/pci/pci.c
+++ b/xen/drivers/pci/pci.c
@@ -119,11 +119,21 @@ const char *__init parse_pci(const char *s, unsigned int *seg_p,
                              unsigned int *bus_p, unsigned int *dev_p,
                              unsigned int *func_p)
 {
+    bool_t def_seg;
+
+    return __parse_pci(s, seg_p, bus_p, dev_p, func_p, &def_seg);
+}
+
+const char *__init __parse_pci(const char *s, unsigned int *seg_p,
+                             unsigned int *bus_p, unsigned int *dev_p,
+                             unsigned int *func_p, bool_t *def_seg)
+{
     unsigned long seg = simple_strtoul(s, &s, 16), bus, dev, func;
 
     if ( *s != ':' )
         return NULL;
     bus = simple_strtoul(s + 1, &s, 16);
+    *def_seg = 0;
     if ( *s == ':' )
         dev = simple_strtoul(s + 1, &s, 16);
     else
@@ -131,6 +141,7 @@ const char *__init parse_pci(const char *s, unsigned int *seg_p,
         dev = bus;
         bus = seg;
         seg = 0;
+        *def_seg = 1;
     }
     if ( func_p )
     {
diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
index 414106a..d66ecab 100644
--- a/xen/include/xen/pci.h
+++ b/xen/include/xen/pci.h
@@ -150,6 +150,9 @@ int pci_find_ext_capability(int seg, int bus, int devfn, int cap);
 int pci_find_next_ext_capability(int seg, int bus, int devfn, int pos, int cap);
 const char *parse_pci(const char *, unsigned int *seg, unsigned int *bus,
                       unsigned int *dev, unsigned int *func);
+const char *__parse_pci(const char *, unsigned int *seg, unsigned int *bus,
+                      unsigned int *dev, unsigned int *func, bool_t *def_seg);
+
 
 bool_t pcie_aer_get_firmware_first(const struct pci_dev *);
 
-- 
2.1.3

[PATCH v9 4/4] iommu: add rmrr Xen command line option for extra rmrrs

[elena.ufimtseva]

From: Elena Ufimtseva <elena.ufimtseva@oracle.com>

On some platforms RMRR regions may be not specified
in ACPI and thus will not be mapped 1:1 in dom0. This
causes IO Page Faults and prevents dom0 from booting
in PVH mode.
New Xen command line option rmrr allows to specify
such devices and memory regions. These regions are added
to the list of RMRR defined in ACPI if the device
is present in system. As a result, additional RMRRs will
be mapped 1:1 in dom0 with correct permissions.

Mentioned above problems were discovered during PVH work with
ThinkCentre M and Dell 5600T. No official documentation
was found so far in regards to what devices and why cause this.
Experiments show that ThinkCentre M USB devices with enabled
debug port generate DMA read transactions to the regions of
memory marked reserved in host e820 map.
For Dell 5600T the device and faulting addresses are not found yet.

For detailed history of the discussion please check following threads:
http://lists.Xen.org/archives/html/xen-devel/2015-02/msg01724.html
http://lists.Xen.org/archives/html/xen-devel/2015-01/msg02513.html

Format for rmrr Xen command line option:
rmrr=start<-end>=[s1]bdf1[,[s1]bdf2[,...]];start<-end>=[s2]bdf1[,[s2]bdf2[,...]]
If grub2 used and multiple ranges are specified, ';' should be
quoted/escaped, refer to grub2 manual for more information.

Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com>
---
 docs/misc/xen-command-line.markdown |  13 +++
 xen/drivers/passthrough/vtd/dmar.c  | 209 +++++++++++++++++++++++++++++++++++-
 2 files changed, 221 insertions(+), 1 deletion(-)

diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index aa684c0..f307f3d 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -1197,6 +1197,19 @@ Specify the host reboot method.
 'efi' instructs Xen to reboot using the EFI reboot call (in EFI mode by
  default it will use that method first).
 
+### rmrr
+> '= start<-end>=[s1]bdf1[,[s1]bdf2[,...]];start<-end>=[s2]bdf1[,[s2]bdf2[,...]]
+
+Define RMRR units that are missing from ACPI table along with device they
+belong to and use them for 1:1 mapping. End addresses can be omitted and one
+page will be mapped. The ranges are inclusive when start and end are specified.
+If segment of the first device is not specified, segment zero will be used.
+If other segments are not specified, first device segment will be used.
+If a segment is specified for other than the first device and it does not match
+the one specified for the first one, an error will be reported.
+Note: grub2 requires to escape or use quotations if special characters are used,
+namely ';', refer to the grub2 documentation if multiple ranges are specified.
+
 ### ro-hpet
 > `= <boolean>`
 
diff --git a/xen/drivers/passthrough/vtd/dmar.c b/xen/drivers/passthrough/vtd/dmar.c
index a8e1e5d..f62fb02 100644
--- a/xen/drivers/passthrough/vtd/dmar.c
+++ b/xen/drivers/passthrough/vtd/dmar.c
@@ -869,6 +869,145 @@ out:
     return ret;
 }
 
+#define MAX_EXTRA_RMRR_PAGES 16
+#define MAX_EXTRA_RMRR 10
+
+/* RMRR units derived from command line rmrr option. */
+#define MAX_EXTRA_RMRR_DEV 20
+struct extra_rmrr_unit {
+    struct list_head list;
+    unsigned long base_pfn, end_pfn;
+    unsigned int dev_count;
+    u32    sbdf[MAX_EXTRA_RMRR_DEV];
+};
+static __initdata unsigned int nr_rmrr;
+static struct __initdata extra_rmrr_unit extra_rmrr_units[MAX_EXTRA_RMRR];
+
+/* Macro for RMRR inclusive range formatting. */
+#define PRI_RMRR(s,e) "[%lx-%lx]"
+
+static void __init add_extra_rmrr(void)
+{
+    struct acpi_rmrr_unit *acpi_rmrr;
+    struct acpi_rmrr_unit *rmrru;
+    unsigned int dev, seg, i, j;
+    unsigned long pfn;
+    bool_t overlap;
+
+    for ( i = 0; i < nr_rmrr; i++ )
+    {
+        if ( extra_rmrr_units[i].base_pfn > extra_rmrr_units[i].end_pfn )
+        {
+            printk(XENLOG_ERR VTDPREFIX
+                   "Invalid RMRR Range "PRI_RMRR(s,e)"\n",
+                   extra_rmrr_units[i].base_pfn, extra_rmrr_units[i].end_pfn);
+            continue;
+        }
+
+        if ( extra_rmrr_units[i].end_pfn - extra_rmrr_units[i].base_pfn >=
+             MAX_EXTRA_RMRR_PAGES )
+        {
+            printk(XENLOG_ERR VTDPREFIX
+                   "RMRR range "PRI_RMRR(s,e)" exceeds "__stringify(MAX_EXTRA_RMRR_PAGES)" pages\n",
+                   extra_rmrr_units[i].base_pfn, extra_rmrr_units[i].end_pfn);
+            continue;
+        }
+
+        for ( j = 0; j < nr_rmrr; j++ )
+        {
+            if ( i != j &&
+                 extra_rmrr_units[i].base_pfn <= extra_rmrr_units[j].end_pfn &&
+                 extra_rmrr_units[j].base_pfn <= extra_rmrr_units[i].end_pfn )
+            {
+                printk(XENLOG_ERR VTDPREFIX
+                      "Overlapping RMRRs "PRI_RMRR(s,e)" and "PRI_RMRR(s,e)"\n",
+                      extra_rmrr_units[i].base_pfn, extra_rmrr_units[i].end_pfn,
+                      extra_rmrr_units[j].base_pfn, extra_rmrr_units[j].end_pfn);
+                break;
+            }
+        }
+        /* Broke out of the overlap loop check, continue with next rmrr. */
+        if ( j < nr_rmrr )
+            continue;
+        overlap = 0;
+        list_for_each_entry(rmrru, &acpi_rmrr_units, list)
+        {
+            if ( pfn_to_paddr(extra_rmrr_units[i].base_pfn <= rmrru->end_address) &&
+                 rmrru->base_address <= pfn_to_paddr(extra_rmrr_units[i].end_pfn) )
+            {
+                printk(XENLOG_ERR VTDPREFIX
+                       "Overlapping extra RMRRs "PRI_RMRR(s,e)" and ACPI RMRRs "PRI_RMRR(s,e)"\n",
+                       extra_rmrr_units[i].base_pfn,
+                       extra_rmrr_units[i].end_pfn,
+                       paddr_to_pfn(rmrru->base_address),
+                       paddr_to_pfn(rmrru->end_address));
+                overlap = 1;
+                break;
+            }
+        }
+        /* Continue to next RMRR is this one overlaps with one from ACPI. */
+        if ( overlap )
+            continue;
+
+        pfn = extra_rmrr_units[i].base_pfn;
+        do
+        {
+            if ( !mfn_valid(pfn) || (pfn >> (paddr_bits - PAGE_SHIFT)) )
+            {
+                printk(XENLOG_ERR VTDPREFIX
+                       "Invalid pfn in RMRR range "PRI_RMRR(s,e)"\n",
+                       extra_rmrr_units[i].base_pfn, extra_rmrr_units[i].end_pfn);
+                break;
+            }
+        } while ( pfn++ <= extra_rmrr_units[i].end_pfn );
+        /* The range had invalid pfn as the loop was broken out before reaching end_pfn. */
+        if ( pfn <= extra_rmrr_units[i].end_pfn )
+            continue;
+
+        acpi_rmrr = xzalloc(struct acpi_rmrr_unit);
+        if ( !acpi_rmrr )
+            return;
+
+        acpi_rmrr->scope.devices = xmalloc_array(u16,
+                                                 extra_rmrr_units[i].dev_count);
+        if ( !acpi_rmrr->scope.devices )
+        {
+            xfree(acpi_rmrr);
+            return;
+        }
+
+        seg = 0;
+        for ( dev = 0; dev < extra_rmrr_units[i].dev_count; dev++ )
+        {
+            acpi_rmrr->scope.devices[dev] = extra_rmrr_units[i].sbdf[dev];
+            seg = seg | PCI_SEG(extra_rmrr_units[i].sbdf[dev]);
+        }
+        if ( seg != PCI_SEG(extra_rmrr_units[i].sbdf[0]) )
+        {
+            printk(XENLOG_ERR VTDPREFIX
+                   "Segments are not equal for RMRR range "PRI_RMRR(s,e)"\n",
+                   extra_rmrr_units[i].base_pfn, extra_rmrr_units[i].end_pfn);
+            scope_devices_free(&acpi_rmrr->scope);
+            xfree(acpi_rmrr);
+            continue;
+        }
+
+        acpi_rmrr->segment = seg;
+        acpi_rmrr->base_address = pfn_to_paddr(extra_rmrr_units[i].base_pfn);
+        acpi_rmrr->end_address = pfn_to_paddr(extra_rmrr_units[i].end_pfn + 1);
+        acpi_rmrr->scope.devices_cnt = extra_rmrr_units[i].dev_count;
+
+        if ( register_one_rmrr(acpi_rmrr) )
+        {
+            printk(XENLOG_ERR VTDPREFIX
+                   "Could not register RMMR range "PRI_RMRR(s,e)"\n",
+                   extra_rmrr_units[i].base_pfn, extra_rmrr_units[i].end_pfn);
+            scope_devices_free(&acpi_rmrr->scope);
+            xfree(acpi_rmrr);
+        }
+    }
+}
+
 #include <asm/tboot.h>
 /* ACPI tables may not be DMA protected by tboot, so use DMAR copy */
 /* SINIT saved in SinitMleData in TXT heap (which is DMA protected) */
@@ -878,6 +1017,7 @@ int __init acpi_dmar_init(void)
 {
     acpi_physical_address dmar_addr;
     acpi_native_uint dmar_len;
+    int ret;
 
     if ( ACPI_SUCCESS(acpi_get_table_phys(ACPI_SIG_DMAR, 0,
                                           &dmar_addr, &dmar_len)) )
@@ -888,7 +1028,10 @@ int __init acpi_dmar_init(void)
         dmar_table = __va(dmar_addr);
     }
 
-    return parse_dmar_table(acpi_parse_dmar);
+    ret = parse_dmar_table(acpi_parse_dmar);
+    add_extra_rmrr();
+
+    return ret;
 }
 
 void acpi_dmar_reinstate(void)
@@ -919,3 +1062,67 @@ int platform_supports_x2apic(void)
     unsigned int mask = ACPI_DMAR_INTR_REMAP | ACPI_DMAR_X2APIC_OPT_OUT;
     return cpu_has_x2apic && ((dmar_flags & mask) == ACPI_DMAR_INTR_REMAP);
 }
+
+/*
+ * Parse rmrr Xen command line options and add parsed device and region into
+ * acpi_rmrr_unit list to mapped as RMRRs parsed from ACPI.
+ * Format:
+ * rmrr=start<-end>=[s1]bdf1[,[s1]bdf2[,...]];start<-end>=[s2]bdf1[,[s2]bdf2[,...]]
+ * If the segment of the first device is not specified, segment zero will be used.
+ * If other segments are not specified, first device segment will be used.
+ * If a segment is specified for other than the first device and it does not match
+ * the one specified for the first one, an error will be reported.
+ */
+static void __init parse_rmrr_param(const char *str)
+{
+    const char *s = str, *cur, *stmp;
+    unsigned int seg, bus, dev, func;
+    unsigned long start, end;
+
+    do {
+        start = simple_strtoul(cur = s, &s, 0);
+        if ( cur == s )
+            break;
+
+        if ( *s == '-' )
+        {
+            end = simple_strtoul(cur = s + 1, &s, 0);
+            if ( cur == s )
+                break;
+        }
+        else
+            end = start;
+
+        extra_rmrr_units[nr_rmrr].base_pfn = start;
+        extra_rmrr_units[nr_rmrr].end_pfn = end;
+        extra_rmrr_units[nr_rmrr].dev_count = 0;
+
+        if ( *s != '=' )
+            continue;
+
+        do {
+            bool_t default_segment = 0;
+
+            if ( *s == ';' )
+                break;
+            stmp = __parse_pci(s + 1, &seg, &bus, &dev, &func, &default_segment);
+            if ( !stmp )
+                break;
+
+            /* Not specified segment will be replaced with one from first device. */
+            if ( extra_rmrr_units[nr_rmrr].dev_count && default_segment )
+                seg = PCI_SEG(extra_rmrr_units[nr_rmrr].sbdf[0]);
+
+            /* Keep sbdf's even if they differ and later report an error. */
+            extra_rmrr_units[nr_rmrr].sbdf[extra_rmrr_units[nr_rmrr].dev_count] = PCI_SBDF(seg, bus, dev, func);
+            extra_rmrr_units[nr_rmrr].dev_count++;
+            s = stmp;
+        } while ( (*s == ',' || *s ) &&
+                  extra_rmrr_units[nr_rmrr].dev_count < MAX_EXTRA_RMRR_DEV );
+
+        if ( extra_rmrr_units[nr_rmrr].dev_count )
+            nr_rmrr++;
+
+    } while ( *s++ == ';' && nr_rmrr < MAX_EXTRA_RMRR );
+}
+custom_param("rmrr", parse_rmrr_param);
-- 
2.1.3

Re: [PATCH v9 2/4] iommu VT-d: separate rmrr addition function

[Konrad Rzeszutek Wilk]

On Wed, Jul 08, 2015 at 09:50:24PM -0400, elena.ufimtseva@oracle.com wrote:
> From: Elena Ufimtseva <elena.ufimtseva@oracle.com>
> 
> 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. 

s/memery/memory/
> 
> Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com>

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  xen/drivers/passthrough/vtd/dmar.c | 126 +++++++++++++++++++------------------
>  1 file changed, 65 insertions(+), 61 deletions(-)
> 
> diff --git a/xen/drivers/passthrough/vtd/dmar.c b/xen/drivers/passthrough/vtd/dmar.c
> index 77ef708..a8e1e5d 100644
> --- a/xen/drivers/passthrough/vtd/dmar.c
> +++ b/xen/drivers/passthrough/vtd/dmar.c
> @@ -585,6 +585,68 @@ out:
>      return ret;
>  }
>  
> +static int register_one_rmrr(struct acpi_rmrr_unit *rmrru)
> +{
> +    bool_t ignore = 0;
> +    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 = 1;
> +        }
> +        else
> +        {
> +            ignore = 0;
> +            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_address %"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)
>  {
> @@ -635,68 +697,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) )
> +        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) )
> -            {
> -                dprintk(XENLOG_WARNING VTDPREFIX,
> -                        " Non-existent device (%04x:%02x:%02x.%u) is 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 )
> -        {
> -            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 ( base_addr > end_addr )
> -        {
> -            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_address %"PRIx64"\n",
> -                        rmrru->base_address, rmrru->end_address);
> -            acpi_register_rmrr_unit(rmrru);
> -        }
> -    }
> +        xfree(rmrru);
>  
>      return ret;
>  }
> -- 
> 2.1.3
> 

Re: [PATCH v9 4/4] iommu: add rmrr Xen command line option for extra rmrrs

[Konrad Rzeszutek Wilk]

On Wed, Jul 08, 2015 at 09:50:26PM -0400, elena.ufimtseva@oracle.com wrote:
> From: Elena Ufimtseva <elena.ufimtseva@oracle.com>
> 
> On some platforms RMRR regions may be not specified
> in ACPI and thus will not be mapped 1:1 in dom0. This
> causes IO Page Faults and prevents dom0 from booting
> in PVH mode.
> New Xen command line option rmrr allows to specify
> such devices and memory regions. These regions are added
> to the list of RMRR defined in ACPI if the device
> is present in system. As a result, additional RMRRs will
> be mapped 1:1 in dom0 with correct permissions.
> 
> Mentioned above problems were discovered during PVH work with
> ThinkCentre M and Dell 5600T. No official documentation
> was found so far in regards to what devices and why cause this.
> Experiments show that ThinkCentre M USB devices with enabled
> debug port generate DMA read transactions to the regions of
> memory marked reserved in host e820 map.
> For Dell 5600T the device and faulting addresses are not found yet.
> 
> For detailed history of the discussion please check following threads:
> http://lists.Xen.org/archives/html/xen-devel/2015-02/msg01724.html
> http://lists.Xen.org/archives/html/xen-devel/2015-01/msg02513.html
> 
> Format for rmrr Xen command line option:
> rmrr=start<-end>=[s1]bdf1[,[s1]bdf2[,...]];start<-end>=[s2]bdf1[,[s2]bdf2[,...]]
> If grub2 used and multiple ranges are specified, ';' should be
> quoted/escaped, refer to grub2 manual for more information.
> 

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Thanks!
> Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com>
> ---
>  docs/misc/xen-command-line.markdown |  13 +++
>  xen/drivers/passthrough/vtd/dmar.c  | 209 +++++++++++++++++++++++++++++++++++-
>  2 files changed, 221 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
> index aa684c0..f307f3d 100644
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -1197,6 +1197,19 @@ Specify the host reboot method.
>  'efi' instructs Xen to reboot using the EFI reboot call (in EFI mode by
>   default it will use that method first).
>  
> +### rmrr
> +> '= start<-end>=[s1]bdf1[,[s1]bdf2[,...]];start<-end>=[s2]bdf1[,[s2]bdf2[,...]]
> +
> +Define RMRR units that are missing from ACPI table along with device they
> +belong to and use them for 1:1 mapping. End addresses can be omitted and one
> +page will be mapped. The ranges are inclusive when start and end are specified.
> +If segment of the first device is not specified, segment zero will be used.
> +If other segments are not specified, first device segment will be used.
> +If a segment is specified for other than the first device and it does not match
> +the one specified for the first one, an error will be reported.
> +Note: grub2 requires to escape or use quotations if special characters are used,
> +namely ';', refer to the grub2 documentation if multiple ranges are specified.
> +
>  ### ro-hpet
>  > `= <boolean>`
>  
> diff --git a/xen/drivers/passthrough/vtd/dmar.c b/xen/drivers/passthrough/vtd/dmar.c
> index a8e1e5d..f62fb02 100644
> --- a/xen/drivers/passthrough/vtd/dmar.c
> +++ b/xen/drivers/passthrough/vtd/dmar.c
> @@ -869,6 +869,145 @@ out:
>      return ret;
>  }
>  
> +#define MAX_EXTRA_RMRR_PAGES 16
> +#define MAX_EXTRA_RMRR 10
> +
> +/* RMRR units derived from command line rmrr option. */
> +#define MAX_EXTRA_RMRR_DEV 20
> +struct extra_rmrr_unit {
> +    struct list_head list;
> +    unsigned long base_pfn, end_pfn;
> +    unsigned int dev_count;
> +    u32    sbdf[MAX_EXTRA_RMRR_DEV];
> +};
> +static __initdata unsigned int nr_rmrr;
> +static struct __initdata extra_rmrr_unit extra_rmrr_units[MAX_EXTRA_RMRR];
> +
> +/* Macro for RMRR inclusive range formatting. */
> +#define PRI_RMRR(s,e) "[%lx-%lx]"
> +
> +static void __init add_extra_rmrr(void)
> +{
> +    struct acpi_rmrr_unit *acpi_rmrr;
> +    struct acpi_rmrr_unit *rmrru;
> +    unsigned int dev, seg, i, j;
> +    unsigned long pfn;
> +    bool_t overlap;
> +
> +    for ( i = 0; i < nr_rmrr; i++ )
> +    {
> +        if ( extra_rmrr_units[i].base_pfn > extra_rmrr_units[i].end_pfn )
> +        {
> +            printk(XENLOG_ERR VTDPREFIX
> +                   "Invalid RMRR Range "PRI_RMRR(s,e)"\n",
> +                   extra_rmrr_units[i].base_pfn, extra_rmrr_units[i].end_pfn);
> +            continue;
> +        }
> +
> +        if ( extra_rmrr_units[i].end_pfn - extra_rmrr_units[i].base_pfn >=
> +             MAX_EXTRA_RMRR_PAGES )
> +        {
> +            printk(XENLOG_ERR VTDPREFIX
> +                   "RMRR range "PRI_RMRR(s,e)" exceeds "__stringify(MAX_EXTRA_RMRR_PAGES)" pages\n",
> +                   extra_rmrr_units[i].base_pfn, extra_rmrr_units[i].end_pfn);
> +            continue;
> +        }
> +
> +        for ( j = 0; j < nr_rmrr; j++ )
> +        {
> +            if ( i != j &&
> +                 extra_rmrr_units[i].base_pfn <= extra_rmrr_units[j].end_pfn &&
> +                 extra_rmrr_units[j].base_pfn <= extra_rmrr_units[i].end_pfn )
> +            {
> +                printk(XENLOG_ERR VTDPREFIX
> +                      "Overlapping RMRRs "PRI_RMRR(s,e)" and "PRI_RMRR(s,e)"\n",
> +                      extra_rmrr_units[i].base_pfn, extra_rmrr_units[i].end_pfn,
> +                      extra_rmrr_units[j].base_pfn, extra_rmrr_units[j].end_pfn);
> +                break;
> +            }
> +        }
> +        /* Broke out of the overlap loop check, continue with next rmrr. */
> +        if ( j < nr_rmrr )
> +            continue;
> +        overlap = 0;
> +        list_for_each_entry(rmrru, &acpi_rmrr_units, list)
> +        {
> +            if ( pfn_to_paddr(extra_rmrr_units[i].base_pfn <= rmrru->end_address) &&
> +                 rmrru->base_address <= pfn_to_paddr(extra_rmrr_units[i].end_pfn) )
> +            {
> +                printk(XENLOG_ERR VTDPREFIX
> +                       "Overlapping extra RMRRs "PRI_RMRR(s,e)" and ACPI RMRRs "PRI_RMRR(s,e)"\n",
> +                       extra_rmrr_units[i].base_pfn,
> +                       extra_rmrr_units[i].end_pfn,
> +                       paddr_to_pfn(rmrru->base_address),
> +                       paddr_to_pfn(rmrru->end_address));
> +                overlap = 1;
> +                break;
> +            }
> +        }
> +        /* Continue to next RMRR is this one overlaps with one from ACPI. */
> +        if ( overlap )
> +            continue;
> +
> +        pfn = extra_rmrr_units[i].base_pfn;
> +        do
> +        {
> +            if ( !mfn_valid(pfn) || (pfn >> (paddr_bits - PAGE_SHIFT)) )
> +            {
> +                printk(XENLOG_ERR VTDPREFIX
> +                       "Invalid pfn in RMRR range "PRI_RMRR(s,e)"\n",
> +                       extra_rmrr_units[i].base_pfn, extra_rmrr_units[i].end_pfn);
> +                break;
> +            }
> +        } while ( pfn++ <= extra_rmrr_units[i].end_pfn );
> +        /* The range had invalid pfn as the loop was broken out before reaching end_pfn. */
> +        if ( pfn <= extra_rmrr_units[i].end_pfn )
> +            continue;
> +
> +        acpi_rmrr = xzalloc(struct acpi_rmrr_unit);
> +        if ( !acpi_rmrr )
> +            return;
> +
> +        acpi_rmrr->scope.devices = xmalloc_array(u16,
> +                                                 extra_rmrr_units[i].dev_count);
> +        if ( !acpi_rmrr->scope.devices )
> +        {
> +            xfree(acpi_rmrr);
> +            return;
> +        }
> +
> +        seg = 0;
> +        for ( dev = 0; dev < extra_rmrr_units[i].dev_count; dev++ )
> +        {
> +            acpi_rmrr->scope.devices[dev] = extra_rmrr_units[i].sbdf[dev];
> +            seg = seg | PCI_SEG(extra_rmrr_units[i].sbdf[dev]);
> +        }
> +        if ( seg != PCI_SEG(extra_rmrr_units[i].sbdf[0]) )
> +        {
> +            printk(XENLOG_ERR VTDPREFIX
> +                   "Segments are not equal for RMRR range "PRI_RMRR(s,e)"\n",
> +                   extra_rmrr_units[i].base_pfn, extra_rmrr_units[i].end_pfn);
> +            scope_devices_free(&acpi_rmrr->scope);
> +            xfree(acpi_rmrr);
> +            continue;
> +        }
> +
> +        acpi_rmrr->segment = seg;
> +        acpi_rmrr->base_address = pfn_to_paddr(extra_rmrr_units[i].base_pfn);
> +        acpi_rmrr->end_address = pfn_to_paddr(extra_rmrr_units[i].end_pfn + 1);
> +        acpi_rmrr->scope.devices_cnt = extra_rmrr_units[i].dev_count;
> +
> +        if ( register_one_rmrr(acpi_rmrr) )
> +        {
> +            printk(XENLOG_ERR VTDPREFIX
> +                   "Could not register RMMR range "PRI_RMRR(s,e)"\n",
> +                   extra_rmrr_units[i].base_pfn, extra_rmrr_units[i].end_pfn);
> +            scope_devices_free(&acpi_rmrr->scope);
> +            xfree(acpi_rmrr);
> +        }
> +    }
> +}
> +
>  #include <asm/tboot.h>
>  /* ACPI tables may not be DMA protected by tboot, so use DMAR copy */
>  /* SINIT saved in SinitMleData in TXT heap (which is DMA protected) */
> @@ -878,6 +1017,7 @@ int __init acpi_dmar_init(void)
>  {
>      acpi_physical_address dmar_addr;
>      acpi_native_uint dmar_len;
> +    int ret;
>  
>      if ( ACPI_SUCCESS(acpi_get_table_phys(ACPI_SIG_DMAR, 0,
>                                            &dmar_addr, &dmar_len)) )
> @@ -888,7 +1028,10 @@ int __init acpi_dmar_init(void)
>          dmar_table = __va(dmar_addr);
>      }
>  
> -    return parse_dmar_table(acpi_parse_dmar);
> +    ret = parse_dmar_table(acpi_parse_dmar);
> +    add_extra_rmrr();
> +
> +    return ret;
>  }
>  
>  void acpi_dmar_reinstate(void)
> @@ -919,3 +1062,67 @@ int platform_supports_x2apic(void)
>      unsigned int mask = ACPI_DMAR_INTR_REMAP | ACPI_DMAR_X2APIC_OPT_OUT;
>      return cpu_has_x2apic && ((dmar_flags & mask) == ACPI_DMAR_INTR_REMAP);
>  }
> +
> +/*
> + * Parse rmrr Xen command line options and add parsed device and region into
> + * acpi_rmrr_unit list to mapped as RMRRs parsed from ACPI.
> + * Format:
> + * rmrr=start<-end>=[s1]bdf1[,[s1]bdf2[,...]];start<-end>=[s2]bdf1[,[s2]bdf2[,...]]
> + * If the segment of the first device is not specified, segment zero will be used.
> + * If other segments are not specified, first device segment will be used.
> + * If a segment is specified for other than the first device and it does not match
> + * the one specified for the first one, an error will be reported.
> + */
> +static void __init parse_rmrr_param(const char *str)
> +{
> +    const char *s = str, *cur, *stmp;
> +    unsigned int seg, bus, dev, func;
> +    unsigned long start, end;
> +
> +    do {
> +        start = simple_strtoul(cur = s, &s, 0);
> +        if ( cur == s )
> +            break;
> +
> +        if ( *s == '-' )
> +        {
> +            end = simple_strtoul(cur = s + 1, &s, 0);
> +            if ( cur == s )
> +                break;
> +        }
> +        else
> +            end = start;
> +
> +        extra_rmrr_units[nr_rmrr].base_pfn = start;
> +        extra_rmrr_units[nr_rmrr].end_pfn = end;
> +        extra_rmrr_units[nr_rmrr].dev_count = 0;
> +
> +        if ( *s != '=' )
> +            continue;
> +
> +        do {
> +            bool_t default_segment = 0;
> +
> +            if ( *s == ';' )
> +                break;
> +            stmp = __parse_pci(s + 1, &seg, &bus, &dev, &func, &default_segment);
> +            if ( !stmp )
> +                break;
> +
> +            /* Not specified segment will be replaced with one from first device. */
> +            if ( extra_rmrr_units[nr_rmrr].dev_count && default_segment )
> +                seg = PCI_SEG(extra_rmrr_units[nr_rmrr].sbdf[0]);
> +
> +            /* Keep sbdf's even if they differ and later report an error. */
> +            extra_rmrr_units[nr_rmrr].sbdf[extra_rmrr_units[nr_rmrr].dev_count] = PCI_SBDF(seg, bus, dev, func);
> +            extra_rmrr_units[nr_rmrr].dev_count++;
> +            s = stmp;
> +        } while ( (*s == ',' || *s ) &&
> +                  extra_rmrr_units[nr_rmrr].dev_count < MAX_EXTRA_RMRR_DEV );
> +
> +        if ( extra_rmrr_units[nr_rmrr].dev_count )
> +            nr_rmrr++;
> +
> +    } while ( *s++ == ';' && nr_rmrr < MAX_EXTRA_RMRR );
> +}
> +custom_param("rmrr", parse_rmrr_param);
> -- 
> 2.1.3
>