Re: git: 6f8b3be1fbd6 - main - pci: Add SR-IOV status reporting
- In reply to: Kevin Bowling : "git: 6f8b3be1fbd6 - main - pci: Add SR-IOV status reporting"
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Mon, 10 Aug 2026 22:52:23 UTC
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?
> ---
> 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
>
>