[PATCH RFC v2] vPCI: account for hidden devices

[Jan Beulich <jbeulich@xxxxxxxx>]

Hidden devices (e.g. an add-in PCI serial card used for Xen's serial
console) are associated with DomXEN, not Dom0. This means that while
looking for overlapping BARs such devices cannot be found on Dom0's list
of devices; DomXEN's list also needs to be scanned.

Suppress vPCI init altogether for r/o devices (which constitute a subset
of hidden ones).

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
---
RFC: The modify_bars() change is intentionally mis-formatted, as the
     necessary re-indentation would make the diff difficult to read. At
     this point I'd merely like to gather input towards possible better
     approaches to solve the issue (not the least because quite possibly
     there are further places needing changing).

RFC: Whether to actually suppress vPCI init is up for debate; adding the
     extra logic is following Roger's suggestion (I'm not convinced it is
     useful to have). If we want to keep that, a 2nd question would be
     whether to keep it in vpci_add_handlers(): Both of its callers (can)
     have a struct pci_seg readily available, so checking ->ro_map at the
     call sites would be easier.

RFC: _setup_hwdom_pci_devices()' loop may want splitting: For
     modify_bars() to consistently respect BARs of hidden devices while
     setting up "normal" ones (i.e. to avoid as much as possible the
     "continue" path introduced here), setting up of the former may want
     doing first.

RFC: vpci_write()'s check of the r/o map may want moving out of mainline
     code, into the case dealing with !pdev->vpci.
---
v2: Extend existing comment. Relax assertion. Don't initialize vPCI for
    r/o devices.

--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -218,6 +218,7 @@ static int modify_bars(const struct pci_
     struct vpci_header *header = &pdev->vpci->header;
     struct rangeset *mem = rangeset_new(NULL, NULL, 0);
     struct pci_dev *tmp, *dev = NULL;
+    const struct domain *d;
     const struct vpci_msix *msix = pdev->vpci->msix;
     unsigned int i;
     int rc;
@@ -285,9 +286,11 @@ static int modify_bars(const struct pci_
 
     /*
      * Check for overlaps with other BARs. Note that only BARs that are
-     * currently mapped (enabled) are checked for overlaps.
+     * currently mapped (enabled) are checked for overlaps. Note also that
+     * for Dom0 we also need to include hidden, i.e. DomXEN's, devices.
      */
-    for_each_pdev ( pdev->domain, tmp )
+for ( d = pdev->domain; ; d = dom_xen ) {//todo
+    for_each_pdev ( d, tmp )
     {
         if ( tmp == pdev )
         {
@@ -304,6 +307,7 @@ static int modify_bars(const struct pci_
                  */
                 continue;
         }
+if ( !tmp->vpci ) { ASSERT(d == dom_xen); continue; }//todo
 
         for ( i = 0; i < ARRAY_SIZE(tmp->vpci->header.bars); i++ )
         {
@@ -330,6 +334,7 @@ static int modify_bars(const struct pci_
             }
         }
     }
+if ( !is_hardware_domain(d) ) break; }//todo
 
     ASSERT(dev);
 
--- a/xen/drivers/vpci/vpci.c
+++ b/xen/drivers/vpci/vpci.c
@@ -70,6 +70,7 @@ void vpci_remove_device(struct pci_dev *
 int vpci_add_handlers(struct pci_dev *pdev)
 {
     unsigned int i;
+    const unsigned long *ro_map;
     int rc = 0;
 
     if ( !has_vpci(pdev->domain) )
@@ -78,6 +79,11 @@ int vpci_add_handlers(struct pci_dev *pd
     /* We should not get here twice for the same device. */
     ASSERT(!pdev->vpci);
 
+    /* No vPCI for r/o devices. */
+    ro_map = pci_get_ro_map(pdev->sbdf.seg);
+    if ( ro_map && test_bit(pdev->sbdf.bdf, ro_map) )
+        return 0;
+
     pdev->vpci = xzalloc(struct vpci);
     if ( !pdev->vpci )
         return -ENOMEM;