Re: [PATCH v7 8/8] vfio: selftests: Add tests to validate SR-IOV UAPI

[Raghavendra Rao Ananta]

On Mon, Apr 6, 2026 at 3:24 PM David Matlack <dmatlack@xxxxxxxxxx> wrote:
>
> On 2026-04-02 05:30 PM, Raghavendra Rao Ananta wrote:
>
> > +/* clang-format off */
> > +FIXTURE(vfio_pci_sriov_uapi_test) {};
> > +/* clang-format on */
> > +
> > +FIXTURE_SETUP(vfio_pci_sriov_uapi_test)
> > +{
> > +}
> > +
> > +FIXTURE_TEARDOWN(vfio_pci_sriov_uapi_test)
> > +{
> > +}
>
> Please do iommu_init() iommu_cleanup() here to reduce code duplication
> and as an added bonus you can drop the clang-format comments above.
>
I think even Vipin suggested this. Last time I tried this, IIRC got a
build error that 'variant' was inaccessible. But I guess I just had to
re-org my code to make it work.
I'll do this in v8.

> > +static void vf_teardown(void)
> > +{
> > +     /* Destroy the VF only when the main/parent process exits. */
> > +     if (getpid() != main_pid)
> > +             return;
>
> Is this because the child processes created by test_harness_run() to run
> the TEST_F()s inherit the atexit() call? If so please clarify that in
> the comment in more detail.
>
Yes, that's the reason, and I assumed the comment made that clear.
I'll re-write it if required.

> > +
> > +     free(vf_bdf);
> > +     sysfs_sriov_numvfs_set(pf_bdf, 0);
> > +}
> > +
> > +static void vf_setup(void)
> > +{
> > +     char *vf_driver;
> > +     int nr_vfs;
> > +
> > +     nr_vfs = sysfs_sriov_totalvfs_get(pf_bdf);
> > +     if (nr_vfs <= 0)
> > +             ksft_exit_skip("SR-IOV may not be supported by the PF: %s\n", pf_bdf);
> > +
> > +     nr_vfs = sysfs_sriov_numvfs_get(pf_bdf);
> > +     if (nr_vfs != 0)
> > +             ksft_exit_skip("SR-IOV already configured for the PF: %s\n", pf_bdf);
> > +
> > +     /* Create only one VF for testing */
> > +     sysfs_sriov_numvfs_set(pf_bdf, 1);
> > +
> > +     /*
> > +      * Setup an exit handler to destroy the VF in case of failures
> > +      * during further setup at the end of the test run.
> > +      */
> > +     main_pid = getpid();
> > +     VFIO_ASSERT_EQ(atexit(vf_teardown), 0);
> > +
> > +     vf_bdf = sysfs_sriov_vf_bdf_get(pf_bdf, 0);
> > +
> > +     /*
> > +      * The VF inherits the driver from the PF.
> > +      * Ensure this is 'vfio-pci' before proceeding.
> > +      */
> > +     vf_driver = sysfs_driver_get(vf_bdf);
> > +     VFIO_ASSERT_NE(vf_driver, NULL);
> > +     VFIO_ASSERT_EQ(strcmp(vf_driver, "vfio-pci"), 0);
> > +     free(vf_driver);
> > +
> > +     printf("Created 1 VF (%s) under the PF: %s\n", vf_bdf, pf_bdf);
>
> Do we actually need atexit()? This code can go into main and we can do
> the VF cleanup before returning from main after test_harness_run()
> returns. I don't think you even need to check the VF driver. If the VF
> is not bound to vfio-pci then that will be caught by the test cases when
> they call device_init(vf_bdf, ...).
>
Other than the asserts on 'vf_driver', the calls to
`sysfs_sriov_vf_bdf_get()` and `sysfs_driver_get()` themselves have a
potential chance of triggering asserts, although this is unlikely.
But the bigger reason is that test_harness_run() unconditionally calls
ksft_exit() at the end, forcing it to be the last function called from
main(), regardless of the tests' outcome. Hence, the exit handler.

I see atexit() used in some selftests, so I thought it was normal and
acceptable.

Thank you.
Raghavendra