Re: git: 6f8b3be1fbd6 - main - pci: Add SR-IOV status reporting
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Mon, 10 Aug 2026 23:32:07 UTC
On Tuesday, August 11, 2026, Kevin Bowling <kevin.bowling@kev009.com> wrote:
> 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?
>
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
> >>
>