From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>

Reset the command register when passing through a PCI device:
it is possible that when passing through a PCI device its memory
decoding bits in the command register are already set. Thus, a
guest OS may not write to the command register to update memory
decoding, so guest mappings (guest's view of the BARs) are
left not updated.

Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
---
Since v1:
 - do not write 0 to the command register, but respect host settings.
---
 xen/drivers/vpci/header.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
index 2e44055946b0..41dda3c43d56 100644
--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -491,8 +491,7 @@ static void cmd_write(const struct pci_dev *pdev, unsigned int reg,
         pci_conf_write16(pdev->sbdf, reg, cmd);
 }
 
-static void guest_cmd_write(const struct pci_dev *pdev, unsigned int reg,
-                            uint32_t cmd, void *data)
+static uint32_t emulate_cmd_reg(const struct pci_dev *pdev, uint32_t cmd)
 {
     /* TODO: Add proper emulation for all bits of the command register. */
 
@@ -504,7 +503,13 @@ static void guest_cmd_write(const struct pci_dev *pdev, unsigned int reg,
     }
 #endif
 
-    cmd_write(pdev, reg, cmd, data);
+    return cmd;
+}
+
+static void guest_cmd_write(const struct pci_dev *pdev, unsigned int reg,
+                            uint32_t cmd, void *data)
+{
+    cmd_write(pdev, reg, emulate_cmd_reg(pdev, cmd), data);
 }
 
 static void bar_write(const struct pci_dev *pdev, unsigned int reg,
@@ -678,6 +683,10 @@ static int init_bars(struct pci_dev *pdev)
         return -EOPNOTSUPP;
     }
 
+    /* Reset the command register for the guest. */
+    if ( !is_hwdom )
+        pci_conf_write16(pdev->sbdf, PCI_COMMAND, emulate_cmd_reg(pdev, 0));
+
     /* Setup a handler for the command register. */
     rc = vpci_add_register(pdev->vpci, vpci_hw_read16,
                            is_hwdom ? cmd_write : guest_cmd_write,
-- 
2.25.1

On Thu, Nov 25, 2021 at 01:02:47PM +0200, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> 
> Reset the command register when passing through a PCI device:
> it is possible that when passing through a PCI device its memory
> decoding bits in the command register are already set. Thus, a
> guest OS may not write to the command register to update memory
> decoding, so guest mappings (guest's view of the BARs) are
> left not updated.
> 
> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> ---
> Since v1:
>  - do not write 0 to the command register, but respect host settings.

There's not much respect of host setting here, are you are basically
writing 0 except for the INTX_DISABLE which will be set if MSI(X) is
enabled.

I wonder whether you really need this anyway. I would expect that a
device that's being assigned to a guest has just been reset globally,
so there should be no need to reset the command register explicitly.

Thanks, Roger.

Hi, Roger!

On 13.01.22 13:07, Roger Pau Monné wrote:
> On Thu, Nov 25, 2021 at 01:02:47PM +0200, Oleksandr Andrushchenko wrote:
>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>>
>> Reset the command register when passing through a PCI device:
>> it is possible that when passing through a PCI device its memory
>> decoding bits in the command register are already set. Thus, a
>> guest OS may not write to the command register to update memory
>> decoding, so guest mappings (guest's view of the BARs) are
>> left not updated.
>>
>> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>> ---
>> Since v1:
>>   - do not write 0 to the command register, but respect host settings.
> There's not much respect of host setting here, are you are basically
> writing 0 except for the INTX_DISABLE which will be set if MSI(X) is
> enabled.
Yes, and this is because we only support INTX emulation at the
moment
>
> I wonder whether you really need this anyway. I would expect that a
> device that's being assigned to a guest has just been reset globally,
> so there should be no need to reset the command register explicitly.
 From my experience it was a real case when the device was not
reset making troubles. I'll remove this patch for now and see if
I can still run without it relying on the device reset which must
be in place while assigning a PCI device (here we rely on the
toolstack, right?).
>
> Thanks, Roger.
Thank you,
Oleksandr