Re: [PATCH v5 06/14] vpci/header: implement guest BAR register handlers

[Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx>]

On 03.02.22 14:54, Jan Beulich wrote:
> On 03.02.2022 13:45, Oleksandr Andrushchenko wrote:
>>>> Also memory decoding needs to be initially disabled when used by
>>>> guests, in order to prevent the BAR being placed on top of a RAM
>>>> region. The guest physmap will be different from the host one, so it's
>>>> possible for BARs to end up placed on top of RAM regions initially
>>>> until the firmware or OS places them at a suitable address.
>>> Agree, memory decoding must be disabled
>> Isn't it already achieved by the toolstack resetting the PCI device
>> while assigning  it to a guest?
> Iirc the tool stack would reset a device only after getting it back from
> a DomU. When coming straight from Dom0 or DomIO, no reset would be
> performed. Furthermore, (again iirc) there are cases where there's no
> known (standard) way to reset a device. Assigning such to a guest when
> it previously was owned by another one is risky (and hence needs an
> admin knowing what they're doing), but may be acceptable in particular
> when e.g. simply rebooting a guest.
>
> IOW - I don't think you can rely on the bit being in a particular state.
So, you mean something like:

diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
index 7695158e6445..9ebd57472da8 100644
--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -808,6 +808,14 @@ static int init_bars(struct pci_dev *pdev)
              return rc;
      }

+    /*
+     * Memory decoding needs to be initially disabled when used by
+     * guests, in order to prevent the BAR being placed on top of a RAM
+     * region.
+     */
+    if ( !is_hwdom )
+        pci_conf_write16(pdev->sbdf, PCI_COMMAND, cmd & ~PCI_COMMAND_MEMORY);
+
      return (cmd & PCI_COMMAND_MEMORY) ? modify_bars(pdev, cmd, false) : 0;
  }
  REGISTER_VPCI_INIT(init_bars, VPCI_PRIORITY_MIDDLE);

> Jan
>
Thank you,
Oleksandr