| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAJHc60wHXkZm_QU=SUtCGHRrMWfBhBdy209wmdQqnox8Z0-mQg@mail.gmail.com>
Date: Fri, 9 Jan 2026 11:05:41 -0800
From: Raghavendra Rao Ananta <rananta@...gle.com>
To: David Matlack <dmatlack@...gle.com>
Cc: Alex Williamson <alex@...zbot.org>, Alex Williamson <alex.williamson@...hat.com>,
Josh Hilke <jrhilke@...gle.com>, kvm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 6/6] vfio: selftests: Add tests to validate SR-IOV UAPI
On Wed, Jan 7, 2026 at 3:22 PM David Matlack <dmatlack@...gle.com> wrote:
>
> On 2025-12-10 06:14 PM, Raghavendra Rao Ananta wrote:
> > Add a selfttest, vfio_pci_sriov_uapi_test.c, to validate the
> > SR-IOV UAPI, including the following cases, iterating over
> > all the IOMMU modes currently supported:
> > - Setting correct/incorrect/NULL tokens during device init.
> > - Close the PF device immediately after setting the token.
> > - Change/override the PF's token after device init.
> >
> > Signed-off-by: Raghavendra Rao Ananta <rananta@...gle.com>
> > ---
> > tools/testing/selftests/vfio/Makefile | 1 +
> > .../selftests/vfio/vfio_pci_sriov_uapi_test.c | 215 ++++++++++++++++++
> > 2 files changed, 216 insertions(+)
> > create mode 100644 tools/testing/selftests/vfio/vfio_pci_sriov_uapi_test.c
> >
> > diff --git a/tools/testing/selftests/vfio/Makefile b/tools/testing/selftests/vfio/Makefile
> > index 3c796ca99a509..f00a63902fbfb 100644
> > --- a/tools/testing/selftests/vfio/Makefile
> > +++ b/tools/testing/selftests/vfio/Makefile
> > @@ -4,6 +4,7 @@ TEST_GEN_PROGS += vfio_iommufd_setup_test
> > TEST_GEN_PROGS += vfio_pci_device_test
> > TEST_GEN_PROGS += vfio_pci_device_init_perf_test
> > TEST_GEN_PROGS += vfio_pci_driver_test
> > +TEST_GEN_PROGS += vfio_pci_sriov_uapi_test
> >
> > TEST_FILES += scripts/cleanup.sh
> > TEST_FILES += scripts/lib.sh
> > diff --git a/tools/testing/selftests/vfio/vfio_pci_sriov_uapi_test.c b/tools/testing/selftests/vfio/vfio_pci_sriov_uapi_test.c
> > new file mode 100644
> > index 0000000000000..4c2951d6e049c
> > --- /dev/null
> > +++ b/tools/testing/selftests/vfio/vfio_pci_sriov_uapi_test.c
> > @@ -0,0 +1,215 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +#include <fcntl.h>
> > +#include <unistd.h>
> > +#include <stdlib.h>
> > +#include <sys/ioctl.h>
> > +#include <linux/limits.h>
> > +
> > +#include <libvfio.h>
> > +
> > +#include "../kselftest_harness.h"
> > +
> > +#define UUID_1 "52ac9bff-3a88-4fbd-901a-0d767c3b6c97"
> > +#define UUID_2 "88594674-90a0-47a9-aea8-9d9b352ac08a"
> > +
> > +static const char *pf_dev_bdf;
>
> nit: I think you could simplify some of the names in this file. This
> code isn't in a library so the names dont' have to be globally unique
> and quite so long.
>
> s/pf_dev_bdf/pf_bdf/
> s/vf_dev_bdf/vf_bdf/
> s/pf_device/pf/
> s/vf_device/vf/
> s/test_vfio_pci_container_setup/container_setup/
> s/test_vfio_pci_iommufd_setup/iommufd_setup/
> s/test_vfio_pci_device_init/device_init/
> s/test_vfio_pci_device_cleanup/device_cleanup/
>
> Feel free to ignore this though if you think it makes the names too
> terse.
>
No, I think the short versions are fine. I can change in the next version.
> > +
> > +static int test_vfio_pci_container_setup(struct vfio_pci_device *device,
> > + const char *bdf,
> > + const char *vf_token)
> > +{
> > + vfio_pci_group_setup(device, bdf);
> > + vfio_container_set_iommu(device);
> > + __vfio_pci_group_get_device_fd(device, bdf, vf_token);
> > +
> > + /* The device fd will be -1 in case of mismatched tokens */
> > + return (device->fd < 0);
> > +}
> > +
> > +static int test_vfio_pci_iommufd_setup(struct vfio_pci_device *device,
> > + const char *bdf, const char *vf_token)
> > +{
> > + vfio_pci_iommufd_cdev_open(device, bdf);
> > + return __vfio_device_bind_iommufd(device->fd,
> > + device->iommu->iommufd, vf_token);
> > +}
> > +
> > +static struct vfio_pci_device *test_vfio_pci_device_init(const char *bdf,
> > + struct iommu *iommu,
> > + const char *vf_token,
> > + int *out_ret)
> > +{
> > + struct vfio_pci_device *device;
> > +
> > + device = calloc(1, sizeof(*device));
> > + VFIO_ASSERT_NOT_NULL(device);
> > +
> > + device->iommu = iommu;
> > + device->bdf = bdf;
>
> Can you put this in a helper exposed by vfio_pci_device.h? e.g.
> vfio_pci_device_alloc()
>
Is that just to wrap the ASSERT() within? Or were you thinking of
initializing the members as well in there?
> > +
> > + if (iommu->mode->container_path)
> > + *out_ret = test_vfio_pci_container_setup(device, bdf, vf_token);
> > + else
> > + *out_ret = test_vfio_pci_iommufd_setup(device, bdf, vf_token);
> > +
> > + return device;
> > +}
> > +
> > +static void test_vfio_pci_device_cleanup(struct vfio_pci_device *device)
> > +{
> > + if (device->fd > 0)
> > + VFIO_ASSERT_EQ(close(device->fd), 0);
> > +
> > + if (device->group_fd)
> > + VFIO_ASSERT_EQ(close(device->group_fd), 0);
> > +
> > + free(device);
> > +}
> > +
> > +FIXTURE(vfio_pci_sriov_uapi_test) {
> > + char vf_dev_bdf[16];
> > + char vf_driver[32];
> > + bool sriov_drivers_autoprobe;
> > +};
> > +
> > +FIXTURE_SETUP(vfio_pci_sriov_uapi_test)
> > +{
> > + int nr_vfs;
> > + int ret;
> > +
> > + nr_vfs = sysfs_get_sriov_totalvfs(pf_dev_bdf);
> > + if (nr_vfs < 0)
> > + SKIP(return, "SR-IOV may not be supported by the device\n");
>
> Should this be <= 0?
>
Yes, <= 0 should be better. I was only aiming for the case where
"Device doesn't support SR-IOV if the file is absent." Looking at the
pci code, I think there's a potential for returning 0, say for a VF or
an error in the PCI config.
I'll update this in v3.
> And replace "the device" with the BDF.
>
Sure
> > +
> > + nr_vfs = sysfs_get_sriov_numvfs(pf_dev_bdf);
> > + if (nr_vfs != 0)
> > + SKIP(return, "SR-IOV already configured for the PF\n");
>
> Let's print the BDF and nr_vfs for the user.
>
Sure
> > +
> > + self->sriov_drivers_autoprobe =
> > + sysfs_get_sriov_drivers_autoprobe(pf_dev_bdf);
> > + if (self->sriov_drivers_autoprobe)
> > + sysfs_set_sriov_drivers_autoprobe(pf_dev_bdf, 0);
> > +
> > + /* Export only one VF for testing */
>
> s/Export/Create/
>
Sure
> > + sysfs_set_sriov_numvfs(pf_dev_bdf, 1);
> > +
> > + sysfs_get_sriov_vf_bdf(pf_dev_bdf, 0, self->vf_dev_bdf);
> > + if (sysfs_get_driver(self->vf_dev_bdf, self->vf_driver) == 0)
> > + sysfs_unbind_driver(self->vf_dev_bdf, self->vf_driver);
>
> This should be impossible since we disabled autoprobing.
>
> > + sysfs_bind_driver(self->vf_dev_bdf, "vfio-pci");
>
> Some devices also require setting driver_override to "vfio-pci" as well
> so the device can be bound to vfio-pci. Let's just do that
> unconditionally.
>
Sure, I'll include that in v3.
Thank you.
Raghavendra
Powered by blists - more mailing lists