[PATCH] xen/vpci: allow BAR write if value is the same

[Stewart Hildebrand <stewart.hildebrand@xxxxxxx>]

During xl pci-assignable-add, pciback may reset the device while memory decoding
(PCI_COMMAND_MEMORY) is enabled. After device reset, memory decoding may be
disabled in hardware, and the BARs may be zeroed/reset in hardware. However, Xen
vPCI still thinks memory decoding is enabled, and BARs will remain mapped in
p2m. In other words, memory decoding may become disabled and BARs reset in
hardware, bypassing the respective vPCI command and BAR register handlers.
Subsequently, when pciback attempts to restore state to the device, including
BARs, it happens to write the BARs before writing the command register.
Restoring/writing the BARs silently fails because Xen vPCI mistakenly thinks
memory decoding is enabled.

Fix the BAR write by allowing it to succeed if the value written is the same as
the Xen vPCI stored value. pciback will subsequently restore the command
register and the state of the BARs and memory decoding bit will then be in sync
between hardware and vPCI again.

While here, remove a nearby stray newline.

Signed-off-by: Stewart Hildebrand <stewart.hildebrand@xxxxxxx>
---
Do we need similar handling in rom_write()?

We may consider additionally checking during bar_write() if the memory decoding
state has become out of sync between hardware and vPCI. During bar_write(), we
would check the device's memory decoding bit, compare it to our vPCI state, and
invoke modify_bars() if needed. Please let me know your thoughts.

I considered additionally checking if the hardware and vPCI memory decoding
state are out of sync in new cmd_read()/bar_read() handlers, and calling
modify_bars() if needed. However, I decided not to do this because it would
impose an unnecessary implication on the in-progress vPCI series with the
rwlock: calling modify_bars() would require holding the lock in write/exclusive
mode, whereas in vPCI read handlers we would only hold the lock in read mode.

I have only observed the inconsistency after device reset when pciback (i.e.
dom0/hardware domain) is restoring the state to the device. Since pciback will
also restore the command register, the state will be back in sync after pciback
is finished restoring the state.
---
 xen/drivers/vpci/header.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
index 767c1ba718d7..446ecf539e89 100644
--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -430,19 +430,17 @@ static void cf_check bar_write(
 
     /*
      * Xen only cares whether the BAR is mapped into the p2m, so allow BAR
-     * writes as long as the BAR is not mapped into the p2m.
+     * writes as long as the BAR is not mapped into the p2m. If the value
+     * written is the current one allow the write regardless to ensure
+     * consistent state between hardware and vPCI.
      */
-    if ( bar->enabled )
+    if ( bar->enabled && val != (uint32_t)(bar->addr >> (hi ? 32 : 0)) )
     {
-        /* If the value written is the current one avoid printing a warning. */
-        if ( val != (uint32_t)(bar->addr >> (hi ? 32 : 0)) )
-            gprintk(XENLOG_WARNING,
-                    "%pp: ignored BAR %zu write while mapped\n",
-                    &pdev->sbdf, bar - pdev->vpci->header.bars + hi);
+        gprintk(XENLOG_WARNING, "%pp: ignored BAR %zu write while mapped\n",
+                &pdev->sbdf, bar - pdev->vpci->header.bars + hi);
         return;
     }
 
-
     /*
      * Update the cached address, so that when memory decoding is enabled
      * Xen can map the BAR into the guest p2m.

base-commit: bad1ac345b1910b820b8a703ad1b9f66412ea844
-- 
2.42.0