All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v8 0/4] iommu: add rmrr Xen command line option
@ 2015-06-30 23:33 elena.ufimtseva
  2015-06-30 23:33 ` [PATCH v8 1/4] pci: add PCI_SBDF and PCI_SEG macros elena.ufimtseva
                   ` (3 more replies)
  0 siblings, 4 replies; 18+ messages in thread
From: elena.ufimtseva @ 2015-06-30 23:33 UTC (permalink / raw
  To: xen-devel
  Cc: Elena Ufimtseva, kevin.tian, tim, jbeulich, yang.z.zhang,
	boris.ostrovsky

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

v8 of rmrr comman 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 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;                     
                                                                                
Changes in v6:                                                                  
 - make __parse_pci return correct result and error codes;                      
 - move add_extra_rmrr                                                          
 - previous patch was missing RMRR addresses in range check, add it here;       
 - add overlap check and range boundaries check;                                
 - moved extra rmrr structure definition to dmar.c;                             
 - change def_seg in __parse_pci type from int to bool_t;                       
 - change name for extra rmrr range to reflect they hold now pfns;   

Changes in v5:                                                                  
 - make parse_pci a wrapper and add __parse_pci with additional def_seg param   
   to identify if segment was specified;                                        
 - make possible not to define segment for each device within same rmrr;        
 - limit number of pages for one RMRR by 16;                                    
 - run mfn_valid check for every address in RMRR range;                         
 - add PCI_SBDF macro;                                                          
 - remove list for extra rmrrs as they are kept in static array;                
                                              
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  | 360 ++++++++++++++++++++++++++++--------
 xen/drivers/pci/pci.c               |  11 ++
 xen/include/xen/pci.h               |   5 +
 4 files changed, 311 insertions(+), 78 deletions(-)

-- 
2.1.3

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v8 1/4] pci: add PCI_SBDF and PCI_SEG macros
  2015-06-30 23:33 [PATCH v8 0/4] iommu: add rmrr Xen command line option elena.ufimtseva
@ 2015-06-30 23:33 ` elena.ufimtseva
  2015-07-08 17:27   ` Konrad Rzeszutek Wilk
  2015-06-30 23:34 ` [PATCH v8 2/4] iommu VT-d: separate rmrr addition function elena.ufimtseva
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 18+ messages in thread
From: elena.ufimtseva @ 2015-06-30 23:33 UTC (permalink / raw
  To: xen-devel
  Cc: Elena Ufimtseva, kevin.tian, tim, jbeulich, yang.z.zhang,
	boris.ostrovsky

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

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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v8 2/4] iommu VT-d: separate rmrr addition function
  2015-06-30 23:33 [PATCH v8 0/4] iommu: add rmrr Xen command line option elena.ufimtseva
  2015-06-30 23:33 ` [PATCH v8 1/4] pci: add PCI_SBDF and PCI_SEG macros elena.ufimtseva
@ 2015-06-30 23:34 ` elena.ufimtseva
  2015-07-08 17:30   ` Konrad Rzeszutek Wilk
  2015-06-30 23:34 ` [PATCH v8 3/4] pci: add wrapper for parse_pci elena.ufimtseva
  2015-06-30 23:34 ` [PATCH v8 4/4] iommu: add rmrr Xen command line option for extra rmrrs elena.ufimtseva
  3 siblings, 1 reply; 18+ messages in thread
From: elena.ufimtseva @ 2015-06-30 23:34 UTC (permalink / raw
  To: xen-devel
  Cc: Elena Ufimtseva, kevin.tian, tim, jbeulich, yang.z.zhang,
	boris.ostrovsky

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.
I will also post additional patch with cleanups to address                      
other non-functional changes as per Jan's review of v7 of this patch.

Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com>
---
Changes since v7:
 - removed bogus debug introduced in previous version;

 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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v8 3/4] pci: add wrapper for parse_pci
  2015-06-30 23:33 [PATCH v8 0/4] iommu: add rmrr Xen command line option elena.ufimtseva
  2015-06-30 23:33 ` [PATCH v8 1/4] pci: add PCI_SBDF and PCI_SEG macros elena.ufimtseva
  2015-06-30 23:34 ` [PATCH v8 2/4] iommu VT-d: separate rmrr addition function elena.ufimtseva
@ 2015-06-30 23:34 ` elena.ufimtseva
  2015-07-08 17:32   ` Konrad Rzeszutek Wilk
  2015-06-30 23:34 ` [PATCH v8 4/4] iommu: add rmrr Xen command line option for extra rmrrs elena.ufimtseva
  3 siblings, 1 reply; 18+ messages in thread
From: elena.ufimtseva @ 2015-06-30 23:34 UTC (permalink / raw
  To: xen-devel
  Cc: Elena Ufimtseva, kevin.tian, tim, jbeulich, yang.z.zhang,
	boris.ostrovsky

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

For sbdf'si parsing in rmrr command line add __parse_pci with addtional
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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v8 4/4] iommu: add rmrr Xen command line option for extra rmrrs
  2015-06-30 23:33 [PATCH v8 0/4] iommu: add rmrr Xen command line option elena.ufimtseva
                   ` (2 preceding siblings ...)
  2015-06-30 23:34 ` [PATCH v8 3/4] pci: add wrapper for parse_pci elena.ufimtseva
@ 2015-06-30 23:34 ` elena.ufimtseva
  2015-07-08 17:52   ` Konrad Rzeszutek Wilk
  3 siblings, 1 reply; 18+ messages in thread
From: elena.ufimtseva @ 2015-06-30 23:34 UTC (permalink / raw
  To: xen-devel
  Cc: Elena Ufimtseva, kevin.tian, tim, jbeulich, yang.z.zhang,
	boris.ostrovsky

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  | 246 ++++++++++++++++++++++++++++++++----
 2 files changed, 236 insertions(+), 23 deletions(-)

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..fa659a9 100644
--- a/xen/drivers/passthrough/vtd/dmar.c
+++ b/xen/drivers/passthrough/vtd/dmar.c
@@ -42,6 +42,8 @@
 #define MIN_SCOPE_LEN (sizeof(struct acpi_dmar_device_scope) + \
                        sizeof(struct acpi_dmar_pci_path))
 
+#define PRI_RMRR(s,e) "[%lx-%lx]"
+
 LIST_HEAD_READ_MOSTLY(acpi_drhd_units);
 LIST_HEAD_READ_MOSTLY(acpi_rmrr_units);
 static LIST_HEAD_READ_MOSTLY(acpi_atsr_units);
@@ -425,7 +427,7 @@ static int __init acpi_parse_dev_scope(
         default:
             if ( iommu_verbose )
                 printk(XENLOG_WARNING VTDPREFIX "Unknown scope type %#x\n",
-                       acpi_scope->entry_type);
+                acpi_scope->entry_type);
             start += acpi_scope->length;
             continue;
         }
@@ -479,8 +481,7 @@ acpi_parse_one_drhd(struct acpi_dmar_header *header)
     INIT_LIST_HEAD(&dmaru->ioapic_list);
     INIT_LIST_HEAD(&dmaru->hpet_list);
     if ( iommu_verbose )
-        dprintk(VTDPREFIX, "  dmaru->address = %"PRIx64"\n",
-                dmaru->address);
+        dprintk(VTDPREFIX, "  dmaru->address = %"PRIx64"\n", dmaru->address);
 
     ret = iommu_alloc(dmaru);
     if ( ret )
@@ -541,8 +542,8 @@ acpi_parse_one_drhd(struct acpi_dmar_header *header)
             if ( !pci_device_detect(drhd->segment, b, d, f) )
             {
                 dprintk(XENLOG_WARNING VTDPREFIX,
-                        " Non-existent device (%04x:%02x:%02x.%u) is reported"
-                        " in this DRHD's scope!\n", drhd->segment, b, d, f);
+                        " Non-existent device (%04x:%02x:%02x.%u) is reported in this DRHD's scope!\n",
+                        drhd->segment, b, d, f);
                 invalid_cnt++;
             }
         }
@@ -553,8 +554,8 @@ acpi_parse_one_drhd(struct acpi_dmar_header *header)
                  invalid_cnt == dmaru->scope.devices_cnt )
             {
                 dprintk(XENLOG_WARNING VTDPREFIX,
-                    "  Workaround BIOS bug: ignore the DRHD due to all "
-                    "devices under its scope are not PCI discoverable!\n");
+                        "  Workaround BIOS bug: ignore the DRHD due to all "
+                        "devices under its scope are not PCI discoverable!\n");
 
                 scope_devices_free(&dmaru->scope);
                 iommu_free(dmaru);
@@ -563,10 +564,10 @@ acpi_parse_one_drhd(struct acpi_dmar_header *header)
             else
             {
                 dprintk(XENLOG_WARNING VTDPREFIX,
-                    "  The DRHD is invalid due to there are devices under "
-                    "its scope are not PCI discoverable! Pls try option "
-                    "iommu=force or iommu=workaround_bios_bug if you "
-                    "really want VT-d\n");
+                        "  The DRHD is invalid due to there are devices under "
+                        "its scope are not PCI discoverable! Pls try option "
+                        "iommu=force or iommu=workaround_bios_bug if you "
+                        "really want VT-d\n");
                 ret = -EINVAL;
             }
         }
@@ -604,8 +605,7 @@ static int register_one_rmrr(struct acpi_rmrr_unit *rmrru)
         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",
+                    " Non-existent device (%04x:%02x:%02x.%u) is reported in RMRR "PRI_RMRR(s,e)"'s scope!\n",
                     rmrru->segment, b, d, f,
                     rmrru->base_address, rmrru->end_address);
             ignore = 1;
@@ -620,16 +620,16 @@ static int register_one_rmrr(struct acpi_rmrr_unit *rmrru)
     if ( ignore )
     {
         dprintk(XENLOG_WARNING VTDPREFIX,
-                "  Ignore the RMRR (%"PRIx64", %"PRIx64") due to "
+                "  Ignore the RMRR "PRI_RMRR(s,e)" due to "
                 "devices under its scope are not PCI discoverable!\n",
-            rmrru->base_address, rmrru->end_address);
+                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",
+                "  The RMRR "PRI_RMRR(s,e)" is incorrect!\n",
                 rmrru->base_address, rmrru->end_address);
         scope_devices_free(&rmrru->scope);
         xfree(rmrru);
@@ -639,7 +639,7 @@ static int register_one_rmrr(struct acpi_rmrr_unit *rmrru)
     {
         if ( iommu_verbose )
             dprintk(VTDPREFIX,
-                    "  RMRR region: base_addr %"PRIx64" end_address %"PRIx64"\n",
+                    "  RMRR region: "PRI_RMRR(s,e)"\n",
                     rmrru->base_address, rmrru->end_address);
         acpi_register_rmrr_unit(rmrru);
     }
@@ -664,7 +664,7 @@ acpi_parse_one_rmrr(struct acpi_dmar_header *header)
        if ( base_addr <= rmrru->end_address && rmrru->base_address <= end_addr )
        {
            printk(XENLOG_ERR VTDPREFIX
-                  "Overlapping RMRRs [%"PRIx64",%"PRIx64"] and [%"PRIx64",%"PRIx64"]\n",
+                  "Overlapping RMRRs "PRI_RMRR(s,e)" and "PRI_RMRR(s,e)"\n",
                   rmrru->base_address, rmrru->end_address,
                   base_addr, end_addr);
            return -EEXIST;
@@ -678,8 +678,7 @@ acpi_parse_one_rmrr(struct acpi_dmar_header *header)
          (!page_is_ram_type(paddr_to_pfn(end_addr), RAM_TYPE_RESERVED)) )
     {
         dprintk(XENLOG_WARNING VTDPREFIX,
-                "  RMRR address range not in reserved memory "
-                "base = %"PRIx64" end = %"PRIx64"; "
+                "  RMRR address range "PRI_RMRR(s,e)" not in reserved memory; "
                 "iommu_inclusive_mapping=1 parameter may be needed.\n",
                 base_addr, end_addr);
     }
@@ -779,8 +778,7 @@ acpi_parse_one_rhsa(struct acpi_dmar_header *header)
     list_add_tail(&rhsau->list, &acpi_rhsa_units);
     if ( iommu_verbose )
         dprintk(VTDPREFIX,
-                "  rhsau->address: %"PRIx64
-                " rhsau->proximity_domain: %"PRIx32"\n",
+                "  rhsau->address: %"PRIx64" rhsau->proximity_domain: %"PRIx32"\n",
                 rhsau->address, rhsau->proximity_domain);
 
     return ret;
@@ -869,6 +867,140 @@ 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 */
+
+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;
+
+    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;
+        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));
+                break;
+            }
+        }
+
+
+        pfn = extra_rmrr_units[i].base_pfn;
+        do
+        {
+            if ( !mfn_valid(pfn) || (pfn >> (paddr_bits - PAGE_SHIFT)) )
+            {
+                if ( iommu_verbose )
+                    printk(XENLOG_ERR VTDPREFIX
+                           "Invalid mfn 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 mfn 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 +1010,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 +1021,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 +1055,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 == ',' &&
+                  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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v8 1/4] pci: add PCI_SBDF and PCI_SEG macros
  2015-06-30 23:33 ` [PATCH v8 1/4] pci: add PCI_SBDF and PCI_SEG macros elena.ufimtseva
@ 2015-07-08 17:27   ` Konrad Rzeszutek Wilk
  2015-07-09  8:10     ` Jan Beulich
  0 siblings, 1 reply; 18+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-07-08 17:27 UTC (permalink / raw
  To: elena.ufimtseva
  Cc: kevin.tian, tim, xen-devel, jbeulich, yang.z.zhang,
	boris.ostrovsky

On Tue, Jun 30, 2015 at 07:33:59PM -0400, elena.ufimtseva@oracle.com wrote:
> From: Elena Ufimtseva <elena.ufimtseva@oracle.com>
> 

You usually say why you need this patch. Something as simple as:

"In preperation for patch XXXX which will use it" is OK.

> 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
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v8 2/4] iommu VT-d: separate rmrr addition function
  2015-06-30 23:34 ` [PATCH v8 2/4] iommu VT-d: separate rmrr addition function elena.ufimtseva
@ 2015-07-08 17:30   ` Konrad Rzeszutek Wilk
  0 siblings, 0 replies; 18+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-07-08 17:30 UTC (permalink / raw
  To: elena.ufimtseva
  Cc: kevin.tian, tim, xen-devel, jbeulich, yang.z.zhang,
	boris.ostrovsky

On Tue, Jun 30, 2015 at 07:34:00PM -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.
> I will also post additional patch with cleanups to address                      
> other non-functional changes as per Jan's review of v7 of this patch.

Those two linesL "I will also .." should be below the --- as there
is no need for that in the final git tree.

Otherwise looks OK.
> 
> Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com>
> ---
> Changes since v7:
>  - removed bogus debug introduced in previous version;
> 
>  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
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v8 3/4] pci: add wrapper for parse_pci
  2015-06-30 23:34 ` [PATCH v8 3/4] pci: add wrapper for parse_pci elena.ufimtseva
@ 2015-07-08 17:32   ` Konrad Rzeszutek Wilk
  0 siblings, 0 replies; 18+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-07-08 17:32 UTC (permalink / raw
  To: elena.ufimtseva
  Cc: kevin.tian, tim, xen-devel, jbeulich, yang.z.zhang,
	boris.ostrovsky

On Tue, Jun 30, 2015 at 07:34:01PM -0400, elena.ufimtseva@oracle.com wrote:
> From: Elena Ufimtseva <elena.ufimtseva@oracle.com>
> 
> For sbdf'si parsing in rmrr command line add __parse_pci with addtional

sbdf'si ?

s/rmrr/RMRR/

s/addtional/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
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v8 4/4] iommu: add rmrr Xen command line option for extra rmrrs
  2015-06-30 23:34 ` [PATCH v8 4/4] iommu: add rmrr Xen command line option for extra rmrrs elena.ufimtseva
@ 2015-07-08 17:52   ` Konrad Rzeszutek Wilk
  0 siblings, 0 replies; 18+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-07-08 17:52 UTC (permalink / raw
  To: elena.ufimtseva
  Cc: kevin.tian, tim, xen-devel, jbeulich, yang.z.zhang,
	boris.ostrovsky

. snip..
> diff --git a/xen/drivers/passthrough/vtd/dmar.c b/xen/drivers/passthrough/vtd/dmar.c
> index a8e1e5d..fa659a9 100644
> --- a/xen/drivers/passthrough/vtd/dmar.c
> +++ b/xen/drivers/passthrough/vtd/dmar.c
> @@ -42,6 +42,8 @@
>  #define MIN_SCOPE_LEN (sizeof(struct acpi_dmar_device_scope) + \
>                         sizeof(struct acpi_dmar_pci_path))
>  
> +#define PRI_RMRR(s,e) "[%lx-%lx]"
> +
>  LIST_HEAD_READ_MOSTLY(acpi_drhd_units);
>  LIST_HEAD_READ_MOSTLY(acpi_rmrr_units);
>  static LIST_HEAD_READ_MOSTLY(acpi_atsr_units);
> @@ -425,7 +427,7 @@ static int __init acpi_parse_dev_scope(
>          default:
>              if ( iommu_verbose )
>                  printk(XENLOG_WARNING VTDPREFIX "Unknown scope type %#x\n",
> -                       acpi_scope->entry_type);
> +                acpi_scope->entry_type);

Stray change? It should be in a seperate patch I think?

>              start += acpi_scope->length;
>              continue;
>          }
> @@ -479,8 +481,7 @@ acpi_parse_one_drhd(struct acpi_dmar_header *header)
>      INIT_LIST_HEAD(&dmaru->ioapic_list);
>      INIT_LIST_HEAD(&dmaru->hpet_list);
>      if ( iommu_verbose )
> -        dprintk(VTDPREFIX, "  dmaru->address = %"PRIx64"\n",
> -                dmaru->address);
> +        dprintk(VTDPREFIX, "  dmaru->address = %"PRIx64"\n", dmaru->address);

Ditto.
>  
>      ret = iommu_alloc(dmaru);
>      if ( ret )
> @@ -541,8 +542,8 @@ acpi_parse_one_drhd(struct acpi_dmar_header *header)
>              if ( !pci_device_detect(drhd->segment, b, d, f) )
>              {
>                  dprintk(XENLOG_WARNING VTDPREFIX,
> -                        " Non-existent device (%04x:%02x:%02x.%u) is reported"
> -                        " in this DRHD's scope!\n", drhd->segment, b, d, f);
> +                        " Non-existent device (%04x:%02x:%02x.%u) is reported in this DRHD's scope!\n",
> +                        drhd->segment, b, d, f);

As well - an seperate cleanup patch.
>                  invalid_cnt++;
>              }
>          }
> @@ -553,8 +554,8 @@ acpi_parse_one_drhd(struct acpi_dmar_header *header)
>                   invalid_cnt == dmaru->scope.devices_cnt )
>              {
>                  dprintk(XENLOG_WARNING VTDPREFIX,
> -                    "  Workaround BIOS bug: ignore the DRHD due to all "
> -                    "devices under its scope are not PCI discoverable!\n");
> +                        "  Workaround BIOS bug: ignore the DRHD due to all "
> +                        "devices under its scope are not PCI discoverable!\n");

This one too.
>  
>                  scope_devices_free(&dmaru->scope);
>                  iommu_free(dmaru);
> @@ -563,10 +564,10 @@ acpi_parse_one_drhd(struct acpi_dmar_header *header)
>              else
>              {
>                  dprintk(XENLOG_WARNING VTDPREFIX,
> -                    "  The DRHD is invalid due to there are devices under "
> -                    "its scope are not PCI discoverable! Pls try option "
> -                    "iommu=force or iommu=workaround_bios_bug if you "
> -                    "really want VT-d\n");
> +                        "  The DRHD is invalid due to there are devices under "
> +                        "its scope are not PCI discoverable! Pls try option "
> +                        "iommu=force or iommu=workaround_bios_bug if you "
> +                        "really want VT-d\n");

And that as well.
>                  ret = -EINVAL;
>              }
>          }
> @@ -604,8 +605,7 @@ static int register_one_rmrr(struct acpi_rmrr_unit *rmrru)
>          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",
> +                    " Non-existent device (%04x:%02x:%02x.%u) is reported in RMRR "PRI_RMRR(s,e)"'s scope!\n",
>                      rmrru->segment, b, d, f,
>                      rmrru->base_address, rmrru->end_address);
>              ignore = 1;
> @@ -620,16 +620,16 @@ static int register_one_rmrr(struct acpi_rmrr_unit *rmrru)
>      if ( ignore )
>      {
>          dprintk(XENLOG_WARNING VTDPREFIX,
> -                "  Ignore the RMRR (%"PRIx64", %"PRIx64") due to "
> +                "  Ignore the RMRR "PRI_RMRR(s,e)" due to "
>                  "devices under its scope are not PCI discoverable!\n",
> -            rmrru->base_address, rmrru->end_address);
> +                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",
> +                "  The RMRR "PRI_RMRR(s,e)" is incorrect!\n",
>                  rmrru->base_address, rmrru->end_address);
>          scope_devices_free(&rmrru->scope);
>          xfree(rmrru);
> @@ -639,7 +639,7 @@ static int register_one_rmrr(struct acpi_rmrr_unit *rmrru)
>      {
>          if ( iommu_verbose )
>              dprintk(VTDPREFIX,
> -                    "  RMRR region: base_addr %"PRIx64" end_address %"PRIx64"\n",
> +                    "  RMRR region: "PRI_RMRR(s,e)"\n",
>                      rmrru->base_address, rmrru->end_address);
>          acpi_register_rmrr_unit(rmrru);
>      }
> @@ -664,7 +664,7 @@ acpi_parse_one_rmrr(struct acpi_dmar_header *header)
>         if ( base_addr <= rmrru->end_address && rmrru->base_address <= end_addr )
>         {
>             printk(XENLOG_ERR VTDPREFIX
> -                  "Overlapping RMRRs [%"PRIx64",%"PRIx64"] and [%"PRIx64",%"PRIx64"]\n",
> +                  "Overlapping RMRRs "PRI_RMRR(s,e)" and "PRI_RMRR(s,e)"\n",
>                    rmrru->base_address, rmrru->end_address,
>                    base_addr, end_addr);
>             return -EEXIST;
> @@ -678,8 +678,7 @@ acpi_parse_one_rmrr(struct acpi_dmar_header *header)
>           (!page_is_ram_type(paddr_to_pfn(end_addr), RAM_TYPE_RESERVED)) )
>      {
>          dprintk(XENLOG_WARNING VTDPREFIX,
> -                "  RMRR address range not in reserved memory "
> -                "base = %"PRIx64" end = %"PRIx64"; "
> +                "  RMRR address range "PRI_RMRR(s,e)" not in reserved memory; "
>                  "iommu_inclusive_mapping=1 parameter may be needed.\n",
>                  base_addr, end_addr);
>      }
> @@ -779,8 +778,7 @@ acpi_parse_one_rhsa(struct acpi_dmar_header *header)
>      list_add_tail(&rhsau->list, &acpi_rhsa_units);
>      if ( iommu_verbose )
>          dprintk(VTDPREFIX,
> -                "  rhsau->address: %"PRIx64
> -                " rhsau->proximity_domain: %"PRIx32"\n",
> +                "  rhsau->address: %"PRIx64" rhsau->proximity_domain: %"PRIx32"\n",

All of those should go in a seperate cleanup patch please.

>                  rhsau->address, rhsau->proximity_domain);
>  
>      return ret;
> @@ -869,6 +867,140 @@ out:
>      return ret;
>  }
>  
> +#define MAX_EXTRA_RMRR_PAGES 16
> +#define MAX_EXTRA_RMRR 10
> +
> +/* RMRR units derived from command line rmrr option */

Missing full stop.

> +#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 */

Macro? Ah I think you meant the:
#define PRI_RMRR(s,e) "[%lx-%lx]"

which is defined somewhere at the top of the file with your patch. Please
move it here.

Also, missing full stop at the end of the line.
> +
> +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;
> +
> +    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;
> +        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));
> +                break;

You break out of the for loop (list_for_each_entry) - did ..
> +            }
> +        }

... you also want to continue with the next rmrr? Right now the code
will continue on with this 'i'.

> +
> +
> +        pfn = extra_rmrr_units[i].base_pfn;
> +        do
> +        {
> +            if ( !mfn_valid(pfn) || (pfn >> (paddr_bits - PAGE_SHIFT)) )
> +            {
> +                if ( iommu_verbose )

Why the 'iommu_verbose' ? Why not just print it out as all the other errors
are printed?

> +                    printk(XENLOG_ERR VTDPREFIX
> +                           "Invalid mfn 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 mfn 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 +1010,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 +1021,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 +1055,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 == ',' &&
> +                  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);


Otherwise looks fine!
> -- 
> 2.1.3
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v8 1/4] pci: add PCI_SBDF and PCI_SEG macros
  2015-07-08 17:27   ` Konrad Rzeszutek Wilk
@ 2015-07-09  8:10     ` Jan Beulich
  2015-07-09 11:13       ` Elena Ufimtseva
  0 siblings, 1 reply; 18+ messages in thread
From: Jan Beulich @ 2015-07-09  8:10 UTC (permalink / raw
  To: elena.ufimtseva, Konrad Rzeszutek Wilk
  Cc: yang.z.zhang, kevin.tian, tim, boris.ostrovsky, xen-devel

>>> On 08.07.15 at 19:27, <konrad.wilk@oracle.com> wrote:
> On Tue, Jun 30, 2015 at 07:33:59PM -0400, elena.ufimtseva@oracle.com wrote:
>> From: Elena Ufimtseva <elena.ufimtseva@oracle.com>
>> 
> 
> You usually say why you need this patch. Something as simple as:
> 
> "In preperation for patch XXXX which will use it" is OK.

Or, even better, add such macros when the first user appears. Iirc
I said so before...

Jan

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v8 1/4] pci: add PCI_SBDF and PCI_SEG macros
  2015-07-09  8:10     ` Jan Beulich
@ 2015-07-09 11:13       ` Elena Ufimtseva
  2015-07-09 12:03         ` Jan Beulich
  0 siblings, 1 reply; 18+ messages in thread
From: Elena Ufimtseva @ 2015-07-09 11:13 UTC (permalink / raw
  To: Jan Beulich; +Cc: kevin.tian, tim, xen-devel, yang.z.zhang, boris.ostrovsky

On Thu, Jul 09, 2015 at 09:10:06AM +0100, Jan Beulich wrote:
> >>> On 08.07.15 at 19:27, <konrad.wilk@oracle.com> wrote:
> > On Tue, Jun 30, 2015 at 07:33:59PM -0400, elena.ufimtseva@oracle.com wrote:
> >> From: Elena Ufimtseva <elena.ufimtseva@oracle.com>
> >> 
> > 
> > You usually say why you need this patch. Something as simple as:
> > 
> > "In preperation for patch XXXX which will use it" is OK.
> 
> Or, even better, add such macros when the first user appears. Iirc
> I said so before...
>
Yes, I realized this late. Will move over in the next version if needed. 
> Jan
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v8 1/4] pci: add PCI_SBDF and PCI_SEG macros
  2015-07-09 11:13       ` Elena Ufimtseva
@ 2015-07-09 12:03         ` Jan Beulich
  0 siblings, 0 replies; 18+ messages in thread
From: Jan Beulich @ 2015-07-09 12:03 UTC (permalink / raw
  To: Elena Ufimtseva; +Cc: kevin.tian, tim, xen-devel, yang.z.zhang, boris.ostrovsky

>>> On 09.07.15 at 13:13, <elena.ufimtseva@oracle.com> wrote:
> On Thu, Jul 09, 2015 at 09:10:06AM +0100, Jan Beulich wrote:
>> >>> On 08.07.15 at 19:27, <konrad.wilk@oracle.com> wrote:
>> > On Tue, Jun 30, 2015 at 07:33:59PM -0400, elena.ufimtseva@oracle.com wrote:
>> >> From: Elena Ufimtseva <elena.ufimtseva@oracle.com>
>> >> 
>> > 
>> > You usually say why you need this patch. Something as simple as:
>> > 
>> > "In preperation for patch XXXX which will use it" is OK.
>> 
>> Or, even better, add such macros when the first user appears. Iirc
>> I said so before...
>>
> Yes, I realized this late. Will move over in the next version if needed. 

Don't you need to rebase on top of v6 of "dmar: device scope mem
leak fix" anyway? Or does the series not conflict with those changes?

Jan

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v8 1/4] pci: add PCI_SBDF and PCI_SEG macros
@ 2015-07-09 12:07 Elena Ufimtseva
  2015-07-09 12:10 ` Jan Beulich
  0 siblings, 1 reply; 18+ messages in thread
From: Elena Ufimtseva @ 2015-07-09 12:07 UTC (permalink / raw
  To: JBeulich; +Cc: kevin.tian, tim, xen-devel, yang.z.zhang, boris.ostrovsky


----- JBeulich@suse.com wrote:

> >>> On 09.07.15 at 13:13, <elena.ufimtseva@oracle.com> wrote:
> > On Thu, Jul 09, 2015 at 09:10:06AM +0100, Jan Beulich wrote:
> >> >>> On 08.07.15 at 19:27, <konrad.wilk@oracle.com> wrote:
> >> > On Tue, Jun 30, 2015 at 07:33:59PM -0400,
> elena.ufimtseva@oracle.com wrote:
> >> >> From: Elena Ufimtseva <elena.ufimtseva@oracle.com>
> >> >> 
> >> > 
> >> > You usually say why you need this patch. Something as simple as:
> >> > 
> >> > "In preperation for patch XXXX which will use it" is OK.
> >> 
> >> Or, even better, add such macros when the first user appears. Iirc
> >> I said so before...
> >>
> > Yes, I realized this late. Will move over in the next version if
> needed. 
> 
> Don't you need to rebase on top of v6 of "dmar: device scope mem
> leak fix" anyway? Or does the series not conflict with those changes?

You are right, it needs to be rebased. I can post later rebased on memory leak fix version, if you thin its a way to go.
> 
> Jan

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v8 1/4] pci: add PCI_SBDF and PCI_SEG macros
  2015-07-09 12:07 [PATCH v8 1/4] pci: add PCI_SBDF and PCI_SEG macros Elena Ufimtseva
@ 2015-07-09 12:10 ` Jan Beulich
  0 siblings, 0 replies; 18+ messages in thread
From: Jan Beulich @ 2015-07-09 12:10 UTC (permalink / raw
  To: Elena Ufimtseva; +Cc: kevin.tian, tim, xen-devel, yang.z.zhang, boris.ostrovsky

>>> On 09.07.15 at 14:07, <elena.ufimtseva@oracle.com> wrote:
> You are right, it needs to be rebased. I can post later rebased on memory 
> leak fix version, if you thin its a way to go.

I didn't look at v9 yet, and can't predict when I will be able to.

Jan

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v8 1/4] pci: add PCI_SBDF and PCI_SEG macros
@ 2015-07-09 15:53 Elena Ufimtseva
  2015-07-09 16:00 ` Jan Beulich
  0 siblings, 1 reply; 18+ messages in thread
From: Elena Ufimtseva @ 2015-07-09 15:53 UTC (permalink / raw
  To: JBeulich; +Cc: kevin.tian, tim, xen-devel, yang.z.zhang, boris.ostrovsky


----- JBeulich@suse.com wrote:

> >>> On 09.07.15 at 14:07, <elena.ufimtseva@oracle.com> wrote:
> > You are right, it needs to be rebased. I can post later rebased on
> memory 
> > leak fix version, if you thin its a way to go.
> 
> I didn't look at v9 yet, and can't predict when I will be able to.
> 
> Jan

Jan 

Would you like me to post v10 with memory leak patch included in the patchset before you start looking at v9?

Elena

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v8 1/4] pci: add PCI_SBDF and PCI_SEG macros
  2015-07-09 15:53 Elena Ufimtseva
@ 2015-07-09 16:00 ` Jan Beulich
  2015-07-09 16:51   ` Wei Liu
  0 siblings, 1 reply; 18+ messages in thread
From: Jan Beulich @ 2015-07-09 16:00 UTC (permalink / raw
  To: Elena Ufimtseva
  Cc: kevin.tian, Wei Liu, tim, xen-devel, yang.z.zhang,
	boris.ostrovsky

>>> On 09.07.15 at 17:53, <elena.ufimtseva@oracle.com> wrote:
> ----- JBeulich@suse.com wrote:
>> >>> On 09.07.15 at 14:07, <elena.ufimtseva@oracle.com> wrote:
>> > You are right, it needs to be rebased. I can post later rebased on
>> memory 
>> > leak fix version, if you thin its a way to go.
>> 
>> I didn't look at v9 yet, and can't predict when I will be able to.
> 
> Would you like me to post v10 with memory leak patch included in the 
> patchset before you start looking at v9?

If there is a dependency on the changes in the leak fix v6, then
this would be a good idea. If not, you can keep things as they are
now. I view the entire set more as a bug fix than a feature anyway,
and hence see no reason not to get this in after the freeze. But I'm
adding Wei just in case...

Jan

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v8 1/4] pci: add PCI_SBDF and PCI_SEG macros
  2015-07-09 16:00 ` Jan Beulich
@ 2015-07-09 16:51   ` Wei Liu
  0 siblings, 0 replies; 18+ messages in thread
From: Wei Liu @ 2015-07-09 16:51 UTC (permalink / raw
  To: Jan Beulich
  Cc: Elena Ufimtseva, kevin.tian, Wei Liu, tim, xen-devel,
	yang.z.zhang, boris.ostrovsky

On Thu, Jul 09, 2015 at 05:00:45PM +0100, Jan Beulich wrote:
> >>> On 09.07.15 at 17:53, <elena.ufimtseva@oracle.com> wrote:
> > ----- JBeulich@suse.com wrote:
> >> >>> On 09.07.15 at 14:07, <elena.ufimtseva@oracle.com> wrote:
> >> > You are right, it needs to be rebased. I can post later rebased on
> >> memory 
> >> > leak fix version, if you thin its a way to go.
> >> 
> >> I didn't look at v9 yet, and can't predict when I will be able to.
> > 
> > Would you like me to post v10 with memory leak patch included in the 
> > patchset before you start looking at v9?
> 
> If there is a dependency on the changes in the leak fix v6, then
> this would be a good idea. If not, you can keep things as they are
> now. I view the entire set more as a bug fix than a feature anyway,
> and hence see no reason not to get this in after the freeze. But I'm
> adding Wei just in case...
> 

I just looked at v9. The first three patches are quite mechanical. The
fourth patch is relatively bigger but it's also quite straightforward
(mostly parsing input). All in all, this series itself is
self-contained.

I'm don't think OSSTest is able to test that, so it would not cause
visible regression on our side.

I also agree it's a bug fix. Preferably this series should be applied
before first RC.

Wei.

> Jan

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v8 1/4] pci: add PCI_SBDF and PCI_SEG macros
@ 2015-07-09 18:13 Elena Ufimtseva
  0 siblings, 0 replies; 18+ messages in thread
From: Elena Ufimtseva @ 2015-07-09 18:13 UTC (permalink / raw
  To: wei.liu2
  Cc: kevin.tian, tim, xen-devel, JBeulich, yang.z.zhang,
	boris.ostrovsky


----- wei.liu2@citrix.com wrote:

> On Thu, Jul 09, 2015 at 05:00:45PM +0100, Jan Beulich wrote:
> > >>> On 09.07.15 at 17:53, <elena.ufimtseva@oracle.com> wrote:
> > > ----- JBeulich@suse.com wrote:
> > >> >>> On 09.07.15 at 14:07, <elena.ufimtseva@oracle.com> wrote:
> > >> > You are right, it needs to be rebased. I can post later rebased
> on
> > >> memory 
> > >> > leak fix version, if you thin its a way to go.
> > >> 
> > >> I didn't look at v9 yet, and can't predict when I will be able
> to.
> > > 
> > > Would you like me to post v10 with memory leak patch included in
> the 
> > > patchset before you start looking at v9?
> > 
> > If there is a dependency on the changes in the leak fix v6, then
> > this would be a good idea. If not, you can keep things as they are
> > now. I view the entire set more as a bug fix than a feature anyway,
> > and hence see no reason not to get this in after the freeze. But
> I'm
> > adding Wei just in case...
> > 
> 

Thanks Jan.
The dependency exists on memory leak patch, so I will add it to this series and 
squash the first patch from v9.
 
> I just looked at v9. The first three patches are quite mechanical.
> The
> fourth patch is relatively bigger but it's also quite straightforward
> (mostly parsing input). All in all, this series itself is
> self-contained.
> 
> I'm don't think OSSTest is able to test that, so it would not cause
> visible regression on our side.
> 
> I also agree it's a bug fix. Preferably this series should be applied
> before first RC.
> 
> Wei.

Thank you Wei.
> 
> > Jan

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2015-07-09 18:13 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-06-30 23:33 [PATCH v8 0/4] iommu: add rmrr Xen command line option elena.ufimtseva
2015-06-30 23:33 ` [PATCH v8 1/4] pci: add PCI_SBDF and PCI_SEG macros elena.ufimtseva
2015-07-08 17:27   ` Konrad Rzeszutek Wilk
2015-07-09  8:10     ` Jan Beulich
2015-07-09 11:13       ` Elena Ufimtseva
2015-07-09 12:03         ` Jan Beulich
2015-06-30 23:34 ` [PATCH v8 2/4] iommu VT-d: separate rmrr addition function elena.ufimtseva
2015-07-08 17:30   ` Konrad Rzeszutek Wilk
2015-06-30 23:34 ` [PATCH v8 3/4] pci: add wrapper for parse_pci elena.ufimtseva
2015-07-08 17:32   ` Konrad Rzeszutek Wilk
2015-06-30 23:34 ` [PATCH v8 4/4] iommu: add rmrr Xen command line option for extra rmrrs elena.ufimtseva
2015-07-08 17:52   ` Konrad Rzeszutek Wilk
  -- strict thread matches above, loose matches on Subject: below --
2015-07-09 12:07 [PATCH v8 1/4] pci: add PCI_SBDF and PCI_SEG macros Elena Ufimtseva
2015-07-09 12:10 ` Jan Beulich
2015-07-09 15:53 Elena Ufimtseva
2015-07-09 16:00 ` Jan Beulich
2015-07-09 16:51   ` Wei Liu
2015-07-09 18:13 Elena Ufimtseva

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.