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

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?


> ---
>  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
>
>