Re: git: 6f8b3be1fbd6 - main - pci: Add SR-IOV status reporting
Date: Mon, 10 Aug 2026 23:15:35 UTC
On Mon, Aug 10, 2026 at 3:52 PM Oliver Pinter <oliver.pntr@gmail.com> wrote:
>
>
>
> On Sunday, August 9, 2026, Kevin Bowling <kbowling@freebsd.org> wrote:
>>
>> The branch main has been updated by kbowling:
>>
>> URL: https://cgit.FreeBSD.org/src/commit/?id=6f8b3be1fbd661bfa11c55081851c36ee1d5d2c1
>>
>> commit 6f8b3be1fbd661bfa11c55081851c36ee1d5d2c1
>> Author: Kevin Bowling <kbowling@FreeBSD.org>
>> AuthorDate: 2026-08-09 02:03:49 +0000
>> Commit: Kevin Bowling <kbowling@FreeBSD.org>
>> 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
>>