[AMD Official Use Only - AMD Internal Distribution Only] + others, on accident
Manojlo Pekovic Software Development Engineer 2 Cloud Software Team -----Original Message----- From: Pekovic, Manojlo Sent: Tuesday, January 20, 2026 4:59 PM To: Alex Williamson <[email protected]> Subject: RE: [PATCH] vfio/pci: Enable atomic ops for multifunction devices Hi Alex, Hope you are good. Sorry for such a late response. Appreciate the comments I have split up the fix into two patches as you said, and will be sending them into two emails so it's easier for you to check them In this mail, I am sending the helper patch and in the next one the multifunction Extract the code that reads and converts VFIO atomic capabilities into a separate helper function. This is a preparatory refactor with no functional change, making the code easier to extend for multifunction device support. Signed-off-by: Manojlo Pekovic <[email protected]> --- hw/vfio/pci.c | 53 ++++++++++++++++++++++++++++++--------------------- 1 file changed, 31 insertions(+), 22 deletions(-) diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index 14bcc725c3..6a6c8f1807 100644 --- a/hw/vfio/pci.c +++ b/hw/vfio/pci.c @@ -1900,13 +1900,41 @@ 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; + 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; +} + +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; @@ -1934,26 +1962,7 @@ 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; - } + mask = vfio_get_atomic_cap(vdev); if (!mask) { return; -- 2.43.0 Manojlo Pekovic Software Development Engineer 2 Cloud Software Team -----Original Message----- From: Alex Williamson <[email protected]> Sent: Monday, January 12, 2026 5:17 PM To: Pekovic, Manojlo <[email protected]> Cc: [email protected]; [email protected]; [email protected]; Prica, Nikola <[email protected]>; Andjelkovic, Dejan <[email protected]> Subject: Re: [PATCH] vfio/pci: Enable atomic ops for multifunction devices 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) {
