On Tue, 9 Dec 2025 14:32:50 +0000 Manojlo Pekovic <[email protected]> wrote:
> Previously, PCIe Atomic Ops support was only enabled for single > function devices due to potential conflicting capabilities between > functions. This patch enables atomic ops for multifunction devices > by finding the common subset of atomic capabilities supported by > all functions. > > The implementation checks all functions on the same slot and > advertises only the atomic operations supported by all of them, > ensuring compatibility across the multifunction device. > > Signed-off-by: Manojlo Pekovic <[email protected]> > --- > hw/vfio/pci.c | 104 +++++++++++++++++++++++++++++++++++++------------- > 1 file changed, 77 insertions(+), 27 deletions(-) > > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c > index 14bcc725c3..9d1faeabff 100644 > --- a/hw/vfio/pci.c > +++ b/hw/vfio/pci.c > @@ -1900,28 +1900,88 @@ static void vfio_add_emulated_long(VFIOPCIDevice > *vdev, int pos, > vfio_set_long_bits(vdev->emulated_config_bits + pos, mask, mask); > } > > -static void vfio_pci_enable_rp_atomics(VFIOPCIDevice *vdev) > +static uint32_t vfio_get_atomic_cap(VFIOPCIDevice *vdev) > { > - struct vfio_device_info_cap_pci_atomic_comp *cap; > g_autofree struct vfio_device_info *info = NULL; > + struct vfio_info_cap_header *hdr; > + struct vfio_device_info_cap_pci_atomic_comp *cap; > + uint32_t mask = 0; > + > + info = vfio_get_device_info(vdev->vbasedev.fd); > + if (!info) { > + return mask; > + } > + > + hdr = vfio_get_device_info_cap(info, > VFIO_DEVICE_INFO_CAP_PCI_ATOMIC_COMP); > + if (!hdr) { > + return mask; > + } > + > + cap = (void *)hdr; > + if (cap->flags & VFIO_PCI_ATOMIC_COMP32) { > + mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP32; > + } > + if (cap->flags & VFIO_PCI_ATOMIC_COMP64) { > + mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP64; > + } > + if (cap->flags & VFIO_PCI_ATOMIC_COMP128) { > + mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP128; > + } > + > + return mask; > +} > + > +/* Returns biggest subset of supported atomic ops of multifunction device */ > +static uint32_t vfio_get_multifunction_atomic_cap(VFIOPCIDevice *vdev, > + PCIBus *bus) > +{ > + PCIDevice *func_dev; > + VFIOPCIDevice *func_vdev; > + int slot = PCI_SLOT(vdev->pdev.devfn); > + int target_devfn; > + uint32_t common_mask = PCI_EXP_DEVCAP2_ATOMIC_COMP32 | > + PCI_EXP_DEVCAP2_ATOMIC_COMP64 | > + PCI_EXP_DEVCAP2_ATOMIC_COMP128; > + > + for (int fn = 0; fn < PCI_FUNC_MAX; fn++) { > + target_devfn = PCI_DEVFN(slot, fn); > + func_dev = pci_find_device(bus, pci_bus_num(bus), target_devfn); > + uint32_t func_mask = 0; > + > + if (!func_dev) { > + continue; > + } > + > + func_vdev = (VFIOPCIDevice *)object_dynamic_cast(OBJECT(func_dev), > + TYPE_VFIO_PCI); > + if (!func_vdev) { > + continue; > + } Why is it justified to continue here? It at least seems worthy of a comment why we can ignore non-vfio-pci devices relative to the advertised atomic op support. > + > + func_mask = vfio_get_atomic_cap(func_vdev); > + > + common_mask &= func_mask; Factor out func_mask. > + } > + > + return common_mask; > +} > + > +static void vfio_pci_enable_rp_atomics(VFIOPCIDevice *vdev) > +{ > PCIBus *bus = pci_get_bus(&vdev->pdev); > PCIDevice *parent = bus->parent_dev; > - struct vfio_info_cap_header *hdr; > uint32_t mask = 0; > uint8_t *pos; > > /* > * PCIe Atomic Ops completer support is only added automatically for > single > * function devices downstream of a root port supporting DEVCAP2. > Support > - * is added during realize and, if added, removed during device exit. > The > - * single function requirement avoids conflicting requirements should a > - * slot be composed of multiple devices with differing capabilities. > + * is added during realize and, if added, removed during device exit. > */ > if (pci_bus_is_root(bus) || !parent || !parent->exp.exp_cap || > pcie_cap_get_type(parent) != PCI_EXP_TYPE_ROOT_PORT || > pcie_cap_get_version(parent) != PCI_EXP_FLAGS_VER2 || > - vdev->pdev.devfn || > - vdev->pdev.cap_present & QEMU_PCI_CAP_MULTIFUNCTION) { > + vdev->pdev.devfn) { > return; > } > > @@ -1934,25 +1994,15 @@ static void vfio_pci_enable_rp_atomics(VFIOPCIDevice > *vdev) > return; > } > > - info = vfio_get_device_info(vdev->vbasedev.fd); > - if (!info) { > - return; > - } > - > - hdr = vfio_get_device_info_cap(info, > VFIO_DEVICE_INFO_CAP_PCI_ATOMIC_COMP); > - if (!hdr) { > - return; > - } > - > - cap = (void *)hdr; > - if (cap->flags & VFIO_PCI_ATOMIC_COMP32) { > - mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP32; > - } > - if (cap->flags & VFIO_PCI_ATOMIC_COMP64) { > - mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP64; > - } > - if (cap->flags & VFIO_PCI_ATOMIC_COMP128) { > - mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP128; Nit, it would have been cleaner to factor out this helper as a precursor to multifunction support. Thanks, Alex > + if (vdev->pdev.cap_present & QEMU_PCI_CAP_MULTIFUNCTION) { > + /* > + * Only process the through the root function > + * We get through here only on function 0, > + * through check vdev->pdev.devfn > + */ > + mask = vfio_get_multifunction_atomic_cap(vdev, bus); > + } else { > + mask = vfio_get_atomic_cap(vdev); > } > > if (!mask) {
