On Tue, Aug 26, 2025 at 03:11:58PM +0000, CLEMENT MATHIEU--DRIF wrote: > Hi all, > > Kindly ping, > do you think we could have a look at this issue after the release of 10.1? > > Thanks > \>cmd
indeed. > On Wed, 2025-08-20 at 08:41 +0000, CLEMENT MATHIEU--DRIF wrote: > > From: Damien Bergamini > > <[damien.bergam...@eviden.com](mailto: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](mailto:damien.bergam...@eviden.com)> > > Signed-off-by: Clement Mathieu--Drif > > <[clement.mathieu--d...@eviden.com](mailto: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
