From: Damien Bergamini <damien.bergam...@eviden.com>

Starting with commit cab1398a60eb, SR-IOV VFs are realized as soon as
pcie_sriov_pf_init() is called.  Because pcie_sriov_pf_init() must be
called before pcie_sriov_pf_init_vf_bar(), the VF BARs types won't be
known when the VF realize function calls pcie_sriov_vf_register_bar().

This breaks the memory regions of the VFs (for instance with igbvf):

$ lspci
...
    Region 0: Memory at 281a00000 (64-bit, prefetchable) [virtual] [size=16K]
    Region 3: Memory at 281a20000 (64-bit, prefetchable) [virtual] [size=16K]

$ info mtree
...
address-space: pci_bridge_pci_mem
  0000000000000000-ffffffffffffffff (prio 0, i/o): pci_bridge_pci
    0000000081a00000-0000000081a03fff (prio 1, i/o): igbvf-mmio
    0000000081a20000-0000000081a23fff (prio 1, i/o): igbvf-msix

and causes MMIO accesses to fail:

    Invalid write at addr 0x281A01520, size 4, region '(null)', reason: rejected
    Invalid read at addr 0x281A00C40, size 4, region '(null)', reason: rejected

To fix this, a type parameter is added to pcie_sriov_vf_register_bar()
to indicate the BAR type. It should be set to the same value as in the
pcie_sriov_pf_init_vf_bar() call.

Fixes: cab1398a60eb ("pcie_sriov: Reuse SR-IOV VF device instances")
Signed-off-by: Damien Bergamini <damien.bergam...@eviden.com>
Signed-off-by: Clement Mathieu--Drif <clement.mathieu--d...@eviden.com>
---
 docs/pcie_sriov.txt         | 2 +-
 hw/net/igbvf.c              | 8 ++++++--
 hw/nvme/ctrl.c              | 4 +++-
 hw/pci/pci.c                | 3 ---
 hw/pci/pcie_sriov.c         | 6 ++----
 include/hw/pci/pcie_sriov.h | 2 +-
 6 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/docs/pcie_sriov.txt b/docs/pcie_sriov.txt
index ab2142807f..77d618b36f 100644
--- a/docs/pcie_sriov.txt
+++ b/docs/pcie_sriov.txt
@@ -83,7 +83,7 @@ setting up a BAR for a VF.
       pcie_ari_init(d, 0x100);
       ...
       memory_region_init(mr, ... )
-      pcie_sriov_vf_register_bar(d, bar_nr, mr);
+      pcie_sriov_vf_register_bar(d, bar_nr, bar_type, mr);
       ...
    }
 
diff --git a/hw/net/igbvf.c b/hw/net/igbvf.c
index 31d72c4977..88dd8fb516 100644
--- a/hw/net/igbvf.c
+++ b/hw/net/igbvf.c
@@ -251,10 +251,14 @@ static void igbvf_pci_realize(PCIDevice *dev, Error 
**errp)
 
     memory_region_init_io(&s->mmio, OBJECT(dev), &mmio_ops, s, "igbvf-mmio",
         IGBVF_MMIO_SIZE);
-    pcie_sriov_vf_register_bar(dev, IGBVF_MMIO_BAR_IDX, &s->mmio);
+    pcie_sriov_vf_register_bar(dev, IGBVF_MMIO_BAR_IDX,
+                               PCI_BASE_ADDRESS_MEM_TYPE_64 |
+                               PCI_BASE_ADDRESS_MEM_PREFETCH, &s->mmio);
 
     memory_region_init(&s->msix, OBJECT(dev), "igbvf-msix", IGBVF_MSIX_SIZE);
-    pcie_sriov_vf_register_bar(dev, IGBVF_MSIX_BAR_IDX, &s->msix);
+    pcie_sriov_vf_register_bar(dev, IGBVF_MSIX_BAR_IDX,
+                               PCI_BASE_ADDRESS_MEM_TYPE_64 |
+                               PCI_BASE_ADDRESS_MEM_PREFETCH, &s->msix);
 
     ret = msix_init(dev, IGBVF_MSIX_VEC_NUM, &s->msix, IGBVF_MSIX_BAR_IDX, 0,
         &s->msix, IGBVF_MSIX_BAR_IDX, 0x2000, 0x70, errp);
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index f5ee6bf260..35a82d2037 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -8709,7 +8709,9 @@ static bool nvme_init_pci(NvmeCtrl *n, PCIDevice 
*pci_dev, Error **errp)
         memory_region_add_subregion(&n->bar0, 0, &n->iomem);
 
         if (pci_is_vf(pci_dev)) {
-            pcie_sriov_vf_register_bar(pci_dev, 0, &n->bar0);
+            pcie_sriov_vf_register_bar(pci_dev, 0,
+                                       PCI_BASE_ADDRESS_SPACE_MEMORY |
+                                       PCI_BASE_ADDRESS_MEM_TYPE_64, &n->bar0);
         } else {
             pci_register_bar(pci_dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY |
                              PCI_BASE_ADDRESS_MEM_TYPE_64, &n->bar0);
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index c70b5ceeba..4fe2626f9e 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1490,9 +1490,6 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
                         : pci_get_bus(pci_dev)->address_space_mem;
 
     if (pci_is_vf(pci_dev)) {
-        PCIDevice *pf = pci_dev->exp.sriov_vf.pf;
-        assert(!pf || type == pf->exp.sriov_pf.vf_bar_type[region_num]);
-
         r->addr = pci_bar_address(pci_dev, region_num, r->type, r->size);
         if (r->addr != PCI_BAR_UNMAPPED) {
             memory_region_add_subregion_overlap(r->address_space,
diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c
index 8a4bf0d6f7..eedce6be1d 100644
--- a/hw/pci/pcie_sriov.c
+++ b/hw/pci/pcie_sriov.c
@@ -242,13 +242,11 @@ void pcie_sriov_pf_init_vf_bar(PCIDevice *dev, int 
region_num,
     dev->exp.sriov_pf.vf_bar_type[region_num] = type;
 }
 
+/* `type` must match the type passed to pcie_sriov_pf_init_vf_bar() */
 void pcie_sriov_vf_register_bar(PCIDevice *dev, int region_num,
-                                MemoryRegion *memory)
+                                uint8_t type, MemoryRegion *memory)
 {
-    uint8_t type;
-
     assert(dev->exp.sriov_vf.pf);
-    type = dev->exp.sriov_vf.pf->exp.sriov_pf.vf_bar_type[region_num];
 
     return pci_register_bar(dev, region_num, type, memory);
 }
diff --git a/include/hw/pci/pcie_sriov.h b/include/hw/pci/pcie_sriov.h
index aeaa38cf34..b67449f8ba 100644
--- a/include/hw/pci/pcie_sriov.h
+++ b/include/hw/pci/pcie_sriov.h
@@ -39,7 +39,7 @@ void pcie_sriov_pf_init_vf_bar(PCIDevice *dev, int region_num,
 
 /* Instantiate a bar for a VF */
 void pcie_sriov_vf_register_bar(PCIDevice *dev, int region_num,
-                                MemoryRegion *memory);
+                                uint8_t type, MemoryRegion *memory);
 
 /**
  * pcie_sriov_pf_init_from_user_created_vfs() - Initialize PF with user-created
-- 
2.50.1

Reply via email to