Re: git: 6f8b3be1fbd6 - main - pci: Add SR-IOV status reporting

On Tuesday, August 11, 2026, Kevin Bowling <[email protected]> wrote:

> On Mon, Aug 10, 2026 at 3:52 PM Oliver Pinter <[email protected]>
> wrote:
> >
> >
> >
> > On Sunday, August 9, 2026, Kevin Bowling <[email protected]> wrote:
> >>
> >> The branch main has been updated by kbowling:
> >>
> >> URL: https://cgit.FreeBSD.org/src/commit/?id=
> 6f8b3be1fbd661bfa11c55081851c36ee1d5d2c1
> >>
> >> commit 6f8b3be1fbd661bfa11c55081851c36ee1d5d2c1
> >> Author:     Kevin Bowling <[email protected]>
> >> AuthorDate: 2026-08-09 02:03:49 +0000
> >> Commit:     Kevin Bowling <[email protected]>
> >> CommitDate: 2026-08-09 06:46:41 +0000
> >>
> >>     pci: Add SR-IOV status reporting
> >>
> >>     Add a generic packed-nvlist status query to each /dev/iov/<PF>
> >>     control device.  Report the live VF Enable state, configured and
> total
> >>     VF counts, and one record for each configured VF.
> >>
> >>     Each VF record contains its PF-local index, computed PCI location,
> >>     newbus attachment state, attached driver, and ppt binding.
> Construct
> >>     records for hardware VFs whose newbus child is absent so attachment
> >>     failures remain visible.
> >>
> >>     Version the extensible schema in sys/iov.h.  Use fixed-width request
> >>     fields so the ioctl command and layout are identical for 32-bit
> callers.
> >>     Serialize the topology snapshot with Giant, then pack and copy it
> after
> >>     releasing Giant.
> >
> >
> > Hi!
> >
> > Just curiosity, why introducing Giant lock usage in FreeBSD in 2026?
> Wasn't there some very heavy efforts to kill it with fire from the kernel
> before?
>
> Did you look at
> https://cgit.freebsd.org/src/tree/sys/dev/pci/pci_iov.c?


>
Not yet, but thanks for the pointer!


> It's the required topology lock.


>
I thought SR-IOV is a relatively new framework in the kernel, I was wrong.
Seems like the Giant-reaping doesn't reached it yet.


>
> Take another look and see if you can help.
>
> >>
> >> ---
> >>  sys/dev/pci/pci_iov.c | 174 ++++++++++++++++++++++++++++++
> +++++++++++++++++++-
> >>  sys/sys/iov.h         |  46 +++++++++++++
> >>  2 files changed, 219 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/sys/dev/pci/pci_iov.c b/sys/dev/pci/pci_iov.c
> >> index 643f0e59b9b8..00a9c8e8be72 100644
> >> --- a/sys/dev/pci/pci_iov.c
> >> +++ b/sys/dev/pci/pci_iov.c
> >> @@ -27,6 +27,7 @@
> >>  #include <sys/cdefs.h>
> >>  #include "opt_bus.h"
> >>
> >> +#include <sys/abi_compat.h>
> >>  #include <sys/param.h>
> >>  #include <sys/conf.h>
> >>  #include <sys/kernel.h>
> >> @@ -875,6 +876,129 @@ pci_iov_is_child_vf(struct pcicfg_iov *pf,
> device_t child)
> >>         return (pf == vfinfo->cfg.iov);
> >>  }
> >>
> >> +static int
> >> +pci_iov_build_status(struct pci_devinfo *dinfo, nvlist_t **statusp)
> >> +{
> >> +       const char *driver;
> >> +       device_t bus, child, dev, pcib, *devlist, *vfdevs;
> >> +       nvlist_t *pf, *status, **vfs;
> >> +       struct pcicfg_iov *iov;
> >> +       struct pci_devinfo *vfinfo;
> >> +       bool attached, passthrough;
> >> +       int busno, devcount, error, func, i, slot;
> >> +       uint16_t rid_off, rid_stride, vf_rid;
> >> +
> >> +       mtx_assert(&Giant, MA_OWNED);
> >> +
> >> +       iov = dinfo->cfg.iov;
> >> +       dev = dinfo->cfg.dev;
> >> +       bus = device_get_parent(dev);
> >> +       pcib = device_get_parent(bus);
> >> +       devlist = NULL;
> >> +       vfdevs = NULL;
> >> +       vfs = NULL;
> >> +       status = NULL;
> >> +       pf = NULL;
> >> +       error = 0;
> >> +
> >> +       if (iov->iov_num_vfs != 0) {
> >> +               vfdevs = mallocarray(iov->iov_num_vfs, sizeof(*vfdevs),
> >> +                   M_SRIOV, M_WAITOK | M_ZERO);
> >> +               error = device_get_children(bus, &devlist, &devcount);
> >> +               if (error != 0)
> >> +                       goto out;
> >> +               for (i = 0; i < devcount; i++) {
> >> +                       child = devlist[i];
> >> +                       if (!pci_iov_is_child_vf(iov, child))
> >> +                               continue;
> >> +                       vfinfo = device_get_ivars(child);
> >> +                       if (vfinfo->cfg.vf.index < iov->iov_num_vfs)
> >> +                               vfdevs[vfinfo->cfg.vf.index] = child;
> >> +               }
> >> +       }
> >> +
> >> +       status = nvlist_create(0);
> >> +       pf = nvlist_create(0);
> >> +       if (status == NULL || pf == NULL) {
> >> +               error = ENOMEM;
> >> +               goto out;
> >> +       }
> >> +       nvlist_add_number(status, IOV_STATUS_VERSION_NAME,
> IOV_STATUS_VERSION);
> >> +       nvlist_add_string(pf, IOV_STATUS_DEVICE_NAME,
> device_get_nameunit(dev));
> >> +       nvlist_add_stringf(pf, IOV_STATUS_PCI_LOCATION_NAME,
> "pci%u:%u:%u:%u",
> >> +           (u_int)pci_get_domain(dev), (u_int)pci_get_bus(dev),
> >> +           (u_int)pci_get_slot(dev), (u_int)pci_get_function(dev));
> >> +       nvlist_add_bool(pf, IOV_STATUS_ENABLED_NAME,
> >> +           (IOV_READ(dinfo, PCIR_SRIOV_CTL, 2) & PCIM_SRIOV_VF_EN) !=
> 0);
> >> +       nvlist_add_number(pf, IOV_STATUS_NUM_VFS_NAME,
> iov->iov_num_vfs);
> >> +       nvlist_add_number(pf, IOV_STATUS_TOTAL_VFS_NAME,
> >> +           IOV_READ(dinfo, PCIR_SRIOV_TOTAL_VFS, 2));
> >> +       error = nvlist_error(pf);
> >> +       if (error != 0)
> >> +               goto out;
> >> +       nvlist_move_nvlist(status, IOV_STATUS_PF_NAME, pf);
> >> +       pf = NULL;
> >> +
> >> +       if (iov->iov_num_vfs != 0)
> >> +               vfs = mallocarray(iov->iov_num_vfs, sizeof(*vfs),
> M_SRIOV,
> >> +                   M_WAITOK | M_ZERO);
> >> +       rid_off = IOV_READ(dinfo, PCIR_SRIOV_VF_OFF, 2);
> >> +       rid_stride = IOV_READ(dinfo, PCIR_SRIOV_VF_STRIDE, 2);
> >> +       vf_rid = pci_get_rid(dev) + rid_off;
> >> +       for (i = 0; i < iov->iov_num_vfs; i++, vf_rid += rid_stride) {
> >> +               vfs[i] = nvlist_create(0);
> >> +               if (vfs[i] == NULL) {
> >> +                       error = ENOMEM;
> >> +                       goto out;
> >> +               }
> >> +               nvlist_add_number(vfs[i], IOV_STATUS_VF_INDEX_NAME, i);
> >> +               child = vfdevs[i];
> >> +               if (child != NULL) {
> >> +                       busno = pci_get_bus(child);
> >> +                       slot = pci_get_slot(child);
> >> +                       func = pci_get_function(child);
> >> +               } else
> >> +                       PCIB_DECODE_RID(pcib, vf_rid, &busno, &slot,
> &func);
> >> +               nvlist_add_stringf(vfs[i], IOV_STATUS_PCI_LOCATION_NAME,
> >> +                   "pci%u:%u:%u:%u", (u_int)pci_get_domain(dev),
> >> +                   (u_int)busno, (u_int)slot, (u_int)func);
> >> +               attached = child != NULL && device_is_attached(child);
> >> +               passthrough = child != NULL && device_get_name(child)
> != NULL &&
> >> +                   strcmp(device_get_name(child), "ppt") == 0;
> >> +               nvlist_add_bool(vfs[i], IOV_STATUS_ATTACHED_NAME,
> attached);
> >> +               nvlist_add_bool(vfs[i], IOV_STATUS_PASSTHROUGH_NAME,
> >> +                   passthrough);
> >> +               if (attached) {
> >> +                       driver = device_get_nameunit(child);
> >> +                       if (driver != NULL)
> >> +                               nvlist_add_string(vfs[i],
> >> +                                   IOV_STATUS_BOUND_DRIVER_NAME,
> driver);
> >> +               }
> >> +               error = nvlist_error(vfs[i]);
> >> +               if (error != 0)
> >> +                       goto out;
> >> +       }
> >> +       if (iov->iov_num_vfs != 0)
> >> +               nvlist_add_nvlist_array(status, IOV_STATUS_VFS_NAME,
> >> +                   (const nvlist_t * const *)vfs, iov->iov_num_vfs);
> >> +       error = nvlist_error(status);
> >> +       if (error != 0)
> >> +               goto out;
> >> +       *statusp = status;
> >> +       status = NULL;
> >> +out:
> >> +       if (vfs != NULL) {
> >> +               for (i = 0; i < iov->iov_num_vfs; i++)
> >> +                       nvlist_destroy(vfs[i]);
> >> +               free(vfs, M_SRIOV);
> >> +       }
> >> +       nvlist_destroy(pf);
> >> +       nvlist_destroy(status);
> >> +       free(vfdevs, M_SRIOV);
> >> +       free(devlist, M_TEMP);
> >> +       return (error);
> >> +}
> >> +
> >>  static int
> >>  pci_iov_delete_iov_children(struct pci_devinfo *dinfo)
> >>  {
> >> @@ -985,7 +1109,8 @@ pci_iov_get_schema_ioctl(struct cdev *cdev,
> struct pci_iov_schema *output)
> >>  {
> >>         struct pci_devinfo *dinfo;
> >>         void *packed;
> >> -       size_t output_len, size;
> >> +       size_t size;
> >> +       uint64_t output_len;
> >>         int error;
> >>
> >>         packed = NULL;
> >> @@ -1025,6 +1150,50 @@ fail:
> >>         return (error);
> >>  }
> >>
> >> +static int
> >> +pci_iov_get_status_ioctl(struct cdev *cdev, struct pci_iov_status
> *output)
> >> +{
> >> +       struct pci_devinfo *dinfo;
> >> +       nvlist_t *status;
> >> +       void *packed;
> >> +       size_t output_len, size;
> >> +       int error;
> >> +
> >> +       status = NULL;
> >> +       packed = NULL;
> >> +       if (output->reserved != 0)
> >> +               return (EINVAL);
> >> +       mtx_lock(&Giant);
> >> +       dinfo = cdev->si_drv1;
> >> +       error = pci_iov_build_status(dinfo, &status);
> >> +       mtx_unlock(&Giant);
> >> +       if (error != 0)
> >> +               goto out;
> >> +
> >> +       packed = nvlist_pack(status, &size);
> >> +       if (packed == NULL) {
> >> +               error = ENOMEM;
> >> +               goto out;
> >> +       }
> >> +
> >> +       output_len = output->len;
> >> +       output->len = size;
> >> +       if (size <= output_len) {
> >> +               error = copyout(packed, PTRIN(output->status), size);
> >> +               if (error != 0)
> >> +                       goto out;
> >> +               output->error = 0;
> >> +       } else {
> >> +               /* Keep the ioctl successful so the required size is
> copied out. */
> >> +               output->error = EMSGSIZE;
> >> +       }
> >> +       error = 0;
> >> +out:
> >> +       free(packed, M_NVLIST);
> >> +       nvlist_destroy(status);
> >> +       return (error);
> >> +}
> >> +
> >>  static int
> >>  pci_iov_ioctl(struct cdev *dev, u_long cmd, caddr_t data, int fflag,
> >>      struct thread *td)
> >> @@ -1038,6 +1207,9 @@ pci_iov_ioctl(struct cdev *dev, u_long cmd,
> caddr_t data, int fflag,
> >>         case IOV_GET_SCHEMA:
> >>                 return (pci_iov_get_schema_ioctl(dev,
> >>                     (struct pci_iov_schema *)data));
> >> +       case IOV_GET_STATUS:
> >> +               return (pci_iov_get_status_ioctl(dev,
> >> +                   (struct pci_iov_status *)data));
> >>         default:
> >>                 return (EINVAL);
> >>         }
> >> diff --git a/sys/sys/iov.h b/sys/sys/iov.h
> >> index 2ae7e5ac6767..67a890bba66f 100644
> >> --- a/sys/sys/iov.h
> >> +++ b/sys/sys/iov.h
> >> @@ -164,6 +164,51 @@ struct pci_iov_schema
> >>         int error;
> >>  };
> >>
> >> +/*
> >> + * IOV_GET_STATUS schema contract.
> >> + *
> >> + * The top-level nvlist contains a version number, a PF nvlist, and,
> when VFs
> >> + * are configured, an array of per-VF nvlists.  The "vfs" key is
> omitted when
> >> + * "num-vfs" is zero; its absence therefore means no VFs are
> configured.  The
> >> + * PF record identifies the device, reports the live SR-IOV VF Enable
> state,
> >> + * and gives the number of VFs configured by the PCI IOV framework and
> the
> >> + * hardware limit.  When present, the array contains one VF record for
> each
> >> + * configured VF, even when its newbus child could not be attached.
> >> + *
> >> + * PCI locations use FreeBSD's native decimal pciD:B:S:F notation (for
> >> + * example, pci0:2:16:2).  "attached" means that newbus successfully
> attached
> >> + * a driver.  "bound-driver" is present only for an attached VF and
> contains
> >> + * the driver's nameunit.  "passthrough" means that the VF has the ppt
> host
> >> + * devclass; it does not imply that a running virtual machine
> currently owns
> >> + * the VF.
> >> + *
> >> + * Consumers must ignore unknown keys.  Additive optional keys retain
> the
> >> + * status version; incompatible type or structural changes require a
> new
> >> + * version.
> >> + */
> >> +#define        IOV_STATUS_VERSION              1
> >> +#define        IOV_STATUS_VERSION_NAME         "version"
> >> +#define        IOV_STATUS_PF_NAME              "pf"
> >> +#define        IOV_STATUS_VFS_NAME             "vfs"
> >> +#define        IOV_STATUS_DEVICE_NAME          "device"
> >> +#define        IOV_STATUS_PCI_LOCATION_NAME    "pci-location"
> >> +#define        IOV_STATUS_ENABLED_NAME         "enabled"
> >> +#define        IOV_STATUS_NUM_VFS_NAME         "num-vfs"
> >> +#define        IOV_STATUS_TOTAL_VFS_NAME       "total-vfs"
> >> +#define        IOV_STATUS_VF_INDEX_NAME        "index"
> >> +#define        IOV_STATUS_ATTACHED_NAME        "attached"
> >> +#define        IOV_STATUS_BOUND_DRIVER_NAME    "bound-driver"
> >> +#define        IOV_STATUS_PASSTHROUGH_NAME     "passthrough"
> >> +
> >> +/* Fixed-width fields keep the ioctl ABI identical for 32-bit callers.
> */
> >> +struct pci_iov_status
> >> +{
> >> +       uint64_t status;        /* User pointer to the packed nvlist. */
> >> +       uint64_t len;
> >> +       int32_t error;
> >> +       uint32_t reserved;      /* Must be zero. */
> >> +};
> >> +
> >>  /*
> >>   * SR-IOV configuration is passed to the kernel as a packed nvlist.
> See nv(3)
> >>   * for the details of the nvlist API.  The expected format of the
> nvlist is:
> >> @@ -254,5 +299,6 @@ struct pci_iov_arg
> >>  #define        IOV_CONFIG      _IOW('p', 10, struct pci_iov_arg)
> >>  #define        IOV_DELETE      _IO('p', 11)
> >>  #define        IOV_GET_SCHEMA  _IOWR('p', 12, struct pci_iov_schema)
> >> +#define        IOV_GET_STATUS  _IOWR('p', 13, struct pci_iov_status)
> >>
> >>  #endif
> >>
>