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?  It's the
required topology lock.  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
>>

Reply via email to