[PATCH v5 3/9] vpci: only check BAR validity once

[Stewart Hildebrand <stewart.hildebrand@xxxxxxx>]

From: Roger Pau Monne <roger.pau@xxxxxxxxxx>

The BAR validity checks in modify_decoding() are redundant since they
are already done in modify_bars().

Avoid multiple calls to pci_check_bar() for the same memory decoding
related operation, as each call can possibly print a warning message
about a BAR being in an invalid position.

Further, there's no need to defer setting bar->enabled until p2m
operations are finished. It can safely be set in modify_bars() itself.
This slightly changes the meaning of the bar->enabled flag: when true,
it means a BAR is either mapped or queued to be mapped. Store whether
the BAR is validly positioned in modify_bars(), and use the cached value
for setting bar->enabled.

Reported-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Fixes: 4acab25a9300 ('x86/vpci: fix handling of BAR overlaps with non-hole 
regions')
Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
Signed-off-by: Stewart Hildebrand <stewart.hildebrand@xxxxxxx>
---
v1->v5:
* rebase
* don't defer setting bar->enabled
* use local variable for bar_valid flag

Original report [1]:
[1] 
https://lore.kernel.org/xen-devel/dbc003a2-9202-46ec-bf87-2829d8a63d53@xxxxxxxxxx/
---
 xen/drivers/vpci/header.c | 18 +++++++++---------
 xen/include/xen/vpci.h    |  2 +-
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
index 9f2d0c8a5452..1fd4832033e9 100644
--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -128,19 +128,10 @@ static void modify_decoding(const struct pci_dev *pdev, 
uint16_t cmd,
             uint32_t val = bar->addr |
                            (map ? PCI_ROM_ADDRESS_ENABLE : 0);
 
-            if ( pci_check_bar(pdev, _mfn(PFN_DOWN(bar->addr)),
-                               _mfn(PFN_DOWN(bar->addr + bar->size - 1))) )
-                bar->enabled = map;
             header->rom_enabled = map;
             pci_conf_write32(pdev->sbdf, rom_pos, val);
             return;
         }
-
-        if ( !rom_only &&
-             (bar->type != VPCI_BAR_ROM || header->rom_enabled) &&
-             pci_check_bar(pdev, _mfn(PFN_DOWN(bar->addr)),
-                           _mfn(PFN_DOWN(bar->addr + bar->size - 1))) )
-            bar->enabled = map;
     }
 
     /*
@@ -296,6 +287,7 @@ static int modify_bars(const struct pci_dev *pdev, uint16_t 
cmd, bool rom_only)
     struct pci_dev *tmp;
     const struct domain *d;
     const struct vpci_msix *msix = pdev->vpci->msix;
+    bool bar_valid[ARRAY_SIZE(header->bars)] = { false };
     unsigned int i, j;
     int rc;
 
@@ -341,6 +333,8 @@ static int modify_bars(const struct pci_dev *pdev, uint16_t 
cmd, bool rom_only)
 
         ASSERT(rangeset_is_empty(bar->mem));
 
+        bar_valid[i] = true;
+
         /*
          * Make sure that the guest set address has the same page offset
          * as the physical address on the host or otherwise things won't work 
as
@@ -506,6 +500,12 @@ static int modify_bars(const struct pci_dev *pdev, 
uint16_t cmd, bool rom_only)
         d = dom_xen;
     }
 
+    for ( i = 0; i < ARRAY_SIZE(header->bars); i++ )
+    {
+        if ( bar_valid[i] )
+            header->bars[i].enabled = cmd & PCI_COMMAND_MEMORY;
+    }
+
     if ( system_state < SYS_STATE_active )
     {
         /*
diff --git a/xen/include/xen/vpci.h b/xen/include/xen/vpci.h
index 877aa391d178..ab94eb60d65f 100644
--- a/xen/include/xen/vpci.h
+++ b/xen/include/xen/vpci.h
@@ -64,7 +64,7 @@ struct vpci {
                 VPCI_BAR_ROM,
             } type;
             bool prefetchable : 1;
-            /* Store whether the BAR is mapped into guest p2m. */
+            /* Whether the BAR is mapped or queued for mapping in guest p2m. */
             bool enabled      : 1;
         } bars[PCI_HEADER_NORMAL_NR_BARS + 1];
         /* At most 6 BARS + 1 expansion ROM BAR. */
-- 
2.54.0