[PATCH v5 4/9] vpci/msix: move MSI-X hole punching as a result of memory decoding enable

[Stewart Hildebrand <stewart.hildebrand@xxxxxxx>]

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

Deferring the p2m unpopulate of the MSI-X table region to after the BARs are
mapped in the p2m is not needed.  The aim of vpci_make_msix_hole() is to
cope with the BAR containing the MSI-X table being positioned over a
reserved e820 region, by making sure the MSI-X table region is
unpopulated on the p2m. Note that reserved e820 regions are identity mapped
in the hardware domain p2m by default.

Doing it before or after the BAR p2m changes are done is irrelevant, hence
do it ahead of the p2m changes, as that's simpler.

Note that vpci_make_msix_hole() should be invoked strictly after setting
bar->enabled. The movement of vpci_make_msix_hole() is OK since
bar->enabled is set in modify_bars().

Take the opportunity to store the memory decoding enabled bit in a local
variable.

Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
Signed-off-by: Stewart Hildebrand <stewart.hildebrand@xxxxxxx>
---
Note that this should not be committed until after the bar->enabled
setting movement in ("xen/vpci: only check BAR validity once")

I tested this with hot-plug as described in 7c24d58a6e64 ("vpci/msix:
check for BARs enabled in vpci_make_msix_hole").

The return at the end of the function will gain a purpose again after a
later code movement patch in the series.

The new_enabled variable will gain 2 additional uses later in the series.

v1->v5:
* order after ("xen/vpci: only check BAR validity once")
* preserve behavior of only invoking vpci_make_msix_hole() when memory
  decoding toggles from 0 to 1
* add note about vpci_make_msix_hole() invocation after setting bar->enabled
* use local variable
* Jan offered an R-b tag in [1], though I hesitated to pick it up due to
  changes, and it was offered prior to 7c24d58a6e64 going upstream
[1] 
https://lore.kernel.org/xen-devel/e2c4944d-d089-490c-879d-b6baec5e4cae@xxxxxxxx/
---
 xen/drivers/vpci/header.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
index 1fd4832033e9..cf9d0bac8876 100644
--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -134,19 +134,6 @@ static void modify_decoding(const struct pci_dev *pdev, 
uint16_t cmd,
         }
     }
 
-    /*
-     * Make sure there are no mappings in the MSIX MMIO areas, so that accesses
-     * can be trapped (and emulated) by Xen when the memory decoding bit is
-     * enabled.
-     *
-     * FIXME: punching holes after the p2m has been set up might be racy for
-     * DomU usage, needs to be revisited.
-     */
-#ifdef CONFIG_HAS_PCI_MSI
-    if ( map && !rom_only && vpci_make_msix_hole(pdev) )
-        return;
-#endif
-
     if ( !rom_only )
     {
         pci_conf_write16(pdev->sbdf, PCI_COMMAND, cmd);
@@ -529,6 +516,7 @@ static void cf_check cmd_write(
     const struct pci_dev *pdev, unsigned int reg, uint32_t cmd, void *data)
 {
     struct vpci_header *header = data;
+    bool new_enabled = cmd & PCI_COMMAND_MEMORY;
 
     if ( !is_hardware_domain(pdev->domain) )
     {
@@ -546,7 +534,7 @@ static void cf_check cmd_write(
      * decoding one. Bits that are not allowed for DomU are already
      * handled above and by the rsvdp_mask.
      */
-    if ( header->bars_mapped != !!(cmd & PCI_COMMAND_MEMORY) )
+    if ( header->bars_mapped != new_enabled )
         /*
          * Ignore the error. No memory has been added or removed from the p2m
          * (because the actual p2m changes are deferred in defer_map) and the
@@ -556,6 +544,12 @@ static void cf_check cmd_write(
         modify_bars(pdev, cmd, false);
     else
         pci_conf_write16(pdev->sbdf, reg, cmd);
+
+#ifdef CONFIG_HAS_PCI_MSI
+    /* Unpopulate MSI-X table region, so accesses trap into Xen. */
+    if ( !header->bars_mapped && new_enabled && vpci_make_msix_hole(pdev) )
+        return;
+#endif
 }
 
 static uint32_t cf_check guest_cmd_read(
-- 
2.54.0