Am 05.12.2025 um 16:03 hat Michael S. Tsirkin geschrieben: > On Fri, Dec 05, 2025 at 03:57:18PM +0100, Kevin Wolf wrote: > > PCI_SRIOV_* are offsets into the SR-IOV capability, not into the PCI > > config space. pcie_sriov_pf_exit() erroneously takes them as the latter, > > which makes it read PCI_HEADER_TYPE and PCI_BIST when it tries to read > > PCI_SRIOV_TOTAL_VF. > > > > In many cases we're lucky enough that the PCI config space will be 0 > > there, so we just skip the whole for loop, but this isn't guaranteed. > > For example, setting the multifunction bit on the PF and then doing a > > 'device_del' on it will get a larger number and cause a segfault. > > > > Fix this and access the real PCI_SRIOV_* fields in the capability. > > > > Cc: [email protected] > > Fixes: 19e55471d4e8 ('pcie_sriov: Allow user to create SR-IOV device') > > Signed-off-by: Kevin Wolf <[email protected]> > > Thanks for the patch! something small to improve: > > > --- > > hw/pci/pcie_sriov.c | 9 +++++---- > > 1 file changed, 5 insertions(+), 4 deletions(-) > > > > diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c > > index c4f88f09757..d467284cbda 100644 > > --- a/hw/pci/pcie_sriov.c > > +++ b/hw/pci/pcie_sriov.c > > @@ -195,14 +195,17 @@ bool pcie_sriov_pf_init(PCIDevice *dev, uint16_t > > offset, > > > > void pcie_sriov_pf_exit(PCIDevice *dev) > > { > > + uint8_t *cfg; > > + > > if (dev->exp.sriov_cap == 0) { > > return; > > } > > + cfg = dev->config + dev->exp.sriov_cap; > > initialize cfg at the point of declaration maybe? I think it would > be clearer.
That's what I had first, then changed it to make it clearer that the pointer is only guaranteed to be valid after the dev->exp.sriov_cap check. But either way works for me. Let me know if I should send a v2 that puts it back on the top. Kevin
