[PATCH v4 7/7] iommufd/selftest: Add coverage for vdevice tombstone

Xu Yilun posted 7 patches 3 months ago
There is a newer version of this series
[PATCH v4 7/7] iommufd/selftest: Add coverage for vdevice tombstone
Posted by Xu Yilun 3 months ago
This tests the flow to tombstone vdevice when idevice is to be unbound
before vdevice destruction. The expected result is:

 - idevice unbinding tombstones vdevice ID, the ID can't be repurposed
   anymore.
 - Even ioctl(IOMMU_DESTROY) can't free the tombstoned ID.
 - iommufd_fops_release() can still free everything.

Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
---
 tools/testing/selftests/iommu/iommufd.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/tools/testing/selftests/iommu/iommufd.c b/tools/testing/selftests/iommu/iommufd.c
index 4a9b6e3b37fa..e1470a7a42cd 100644
--- a/tools/testing/selftests/iommu/iommufd.c
+++ b/tools/testing/selftests/iommu/iommufd.c
@@ -3016,6 +3016,20 @@ TEST_F(iommufd_viommu, vdevice_cache)
 	test_ioctl_destroy(vdev_id);
 }
 
+TEST_F(iommufd_viommu, vdevice_tombstone)
+{
+	uint32_t viommu_id = self->viommu_id;
+	uint32_t dev_id = self->device_id;
+	uint32_t vdev_id = 0;
+
+	if (!dev_id)
+		SKIP(return, "Skipping test for variant no_viommu");
+
+	test_cmd_vdevice_alloc(viommu_id, dev_id, 0x99, &vdev_id);
+	test_ioctl_destroy(self->stdev_id);
+	EXPECT_ERRNO(ENOENT, _test_ioctl_destroy(self->fd, vdev_id));
+}
+
 FIXTURE(iommufd_device_pasid)
 {
 	int fd;
-- 
2.25.1
RE: [PATCH v4 7/7] iommufd/selftest: Add coverage for vdevice tombstone
Posted by Tian, Kevin 2 months, 4 weeks ago
> From: Xu Yilun <yilun.xu@linux.intel.com>
> Sent: Wednesday, July 9, 2025 12:03 PM
> 
> This tests the flow to tombstone vdevice when idevice is to be unbound
> before vdevice destruction. The expected result is:
> 
>  - idevice unbinding tombstones vdevice ID, the ID can't be repurposed
>    anymore.
>  - Even ioctl(IOMMU_DESTROY) can't free the tombstoned ID.
>  - iommufd_fops_release() can still free everything.

but the test only checks 2) instead of all?
Re: [PATCH v4 7/7] iommufd/selftest: Add coverage for vdevice tombstone
Posted by Xu Yilun 2 months, 4 weeks ago
On Thu, Jul 10, 2025 at 08:10:34AM +0000, Tian, Kevin wrote:
> > From: Xu Yilun <yilun.xu@linux.intel.com>
> > Sent: Wednesday, July 9, 2025 12:03 PM
> > 
> > This tests the flow to tombstone vdevice when idevice is to be unbound
> > before vdevice destruction. The expected result is:
> > 
> >  - idevice unbinding tombstones vdevice ID, the ID can't be repurposed
> >    anymore.
> >  - Even ioctl(IOMMU_DESTROY) can't free the tombstoned ID.
> >  - iommufd_fops_release() can still free everything.
> 
> but the test only checks 2) instead of all?

It tests 2) & 3), 3) will be executed on FIXTURE_TEARDOWN(iommufd_viommu)

For 1) "the ID can't be repurposed anymore", I don't have a good idea how to
verify it. Allocate 2^32 objects and verify no tombstoned ID is
reused? That's too crazy...   Maybe just mark it untested in the commit
message?

Thanks,
Yilun
RE: [PATCH v4 7/7] iommufd/selftest: Add coverage for vdevice tombstone
Posted by Tian, Kevin 2 months, 4 weeks ago
> From: Xu Yilun <yilun.xu@linux.intel.com>
> Sent: Thursday, July 10, 2025 5:25 PM
> 
> On Thu, Jul 10, 2025 at 08:10:34AM +0000, Tian, Kevin wrote:
> > > From: Xu Yilun <yilun.xu@linux.intel.com>
> > > Sent: Wednesday, July 9, 2025 12:03 PM
> > >
> > > This tests the flow to tombstone vdevice when idevice is to be unbound
> > > before vdevice destruction. The expected result is:
> > >
> > >  - idevice unbinding tombstones vdevice ID, the ID can't be repurposed
> > >    anymore.
> > >  - Even ioctl(IOMMU_DESTROY) can't free the tombstoned ID.
> > >  - iommufd_fops_release() can still free everything.
> >
> > but the test only checks 2) instead of all?
> 
> It tests 2) & 3), 3) will be executed on
> FIXTURE_TEARDOWN(iommufd_viommu)
> 
> For 1) "the ID can't be repurposed anymore", I don't have a good idea how
> to
> verify it. Allocate 2^32 objects and verify no tombstoned ID is
> reused? That's too crazy...   Maybe just mark it untested in the commit
> message?
> 

not required. Just make the commit msg clear.