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.
>
> if (dev->exp.sriov_pf.vf_user_created) {
> uint16_t ven_id = pci_get_word(dev->config + PCI_VENDOR_ID);
> - uint16_t total_vfs = pci_get_word(dev->config + PCI_SRIOV_TOTAL_VF);
> - uint16_t vf_dev_id = pci_get_word(dev->config + PCI_SRIOV_VF_DID);
> + uint16_t total_vfs = pci_get_word(cfg + PCI_SRIOV_TOTAL_VF);
> + uint16_t vf_dev_id = pci_get_word(cfg + PCI_SRIOV_VF_DID);
>
> unregister_vfs(dev);
>
> @@ -213,8 +216,6 @@ void pcie_sriov_pf_exit(PCIDevice *dev)
> pci_config_set_device_id(dev->exp.sriov_pf.vf[i]->config,
> vf_dev_id);
> }
> } else {
> - uint8_t *cfg = dev->config + dev->exp.sriov_cap;
> -
> unparent_vfs(dev, pci_get_word(cfg + PCI_SRIOV_TOTAL_VF));
> }
> }
> --
> 2.52.0
