[PATCH v10 1/2] selftests/vfio: Add NO_SEND_MSI feature flag and MSI helper macros

Rubin Du posted 2 patches 1 week, 1 day ago
There is a newer version of this series
[PATCH v10 1/2] selftests/vfio: Add NO_SEND_MSI feature flag and MSI helper macros
Posted by Rubin Du 1 week, 1 day ago
Add a features bitmask to struct vfio_pci_driver and define
VFIO_PCI_DRIVER_F_NO_SEND_MSI for drivers that cannot trigger MSI
interrupts via send_msi(). Drivers that lack MSI support opt-in by
setting this flag in their init() callback.

Change vfio_pci_driver_send_msi() to return int, checking the flag
internally and returning -EOPNOTSUPP when set.

Add fcntl_set_msi_nonblock(self) and update ASSERT_NO_MSI(self)
to check the flag internally.

Signed-off-by: Rubin Du <rubind@nvidia.com>
---
 .../lib/include/libvfio/vfio_pci_driver.h     |  5 ++-
 .../selftests/vfio/lib/vfio_pci_driver.c      |  6 ++-
 .../selftests/vfio/vfio_pci_driver_test.c     | 41 +++++++++++--------
 3 files changed, 33 insertions(+), 19 deletions(-)

diff --git a/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_driver.h b/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_driver.h
index e5ada209b1d1..f2cf2608f757 100644
--- a/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_driver.h
+++ b/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_driver.h
@@ -6,6 +6,8 @@
 
 struct vfio_pci_device;
 
+#define VFIO_PCI_DRIVER_F_NO_SEND_MSI	(1UL << 0) /* Device cannot trigger MSI */
+
 struct vfio_pci_driver_ops {
 	const char *name;
 
@@ -69,6 +71,7 @@ struct vfio_pci_driver {
 	const struct vfio_pci_driver_ops *ops;
 	bool initialized;
 	bool memcpy_in_progress;
+	unsigned long features;
 
 	/* Region to be used by the driver (e.g. for in-memory descriptors) */
 	struct dma_region region;
@@ -92,6 +95,6 @@ void vfio_pci_driver_memcpy_start(struct vfio_pci_device *device,
 				  iova_t src, iova_t dst, u64 size,
 				  u64 count);
 int vfio_pci_driver_memcpy_wait(struct vfio_pci_device *device);
-void vfio_pci_driver_send_msi(struct vfio_pci_device *device);
+int vfio_pci_driver_send_msi(struct vfio_pci_device *device);
 
 #endif /* SELFTESTS_VFIO_LIB_INCLUDE_LIBVFIO_VFIO_PCI_DRIVER_H */
diff --git a/tools/testing/selftests/vfio/lib/vfio_pci_driver.c b/tools/testing/selftests/vfio/lib/vfio_pci_driver.c
index 6827f4a6febe..b72df578717a 100644
--- a/tools/testing/selftests/vfio/lib/vfio_pci_driver.c
+++ b/tools/testing/selftests/vfio/lib/vfio_pci_driver.c
@@ -67,13 +67,17 @@ void vfio_pci_driver_remove(struct vfio_pci_device *device)
 	driver->initialized = false;
 }
 
-void vfio_pci_driver_send_msi(struct vfio_pci_device *device)
+int vfio_pci_driver_send_msi(struct vfio_pci_device *device)
 {
 	struct vfio_pci_driver *driver = &device->driver;
 
+	if (driver->features & VFIO_PCI_DRIVER_F_NO_SEND_MSI)
+		return -EOPNOTSUPP;
+
 	VFIO_CHECK_DRIVER_OP(driver, send_msi);
 
 	driver->ops->send_msi(device);
+	return 0;
 }
 
 void vfio_pci_driver_memcpy_start(struct vfio_pci_device *device,
diff --git a/tools/testing/selftests/vfio/vfio_pci_driver_test.c b/tools/testing/selftests/vfio/vfio_pci_driver_test.c
index afa0480ddd9b..491ac631b108 100644
--- a/tools/testing/selftests/vfio/vfio_pci_driver_test.c
+++ b/tools/testing/selftests/vfio/vfio_pci_driver_test.c
@@ -11,11 +11,18 @@
 
 static const char *device_bdf;
 
-#define ASSERT_NO_MSI(_eventfd) do {			\
-	u64 __value;					\
-							\
-	ASSERT_EQ(-1, read(_eventfd, &__value, 8));	\
-	ASSERT_EQ(EAGAIN, errno);			\
+#define fcntl_set_msi_nonblock(_self) do {				\
+	if (!(_self->device->driver.features & VFIO_PCI_DRIVER_F_NO_SEND_MSI)) \
+		fcntl_set_nonblock(_self->msi_fd);			\
+} while (0)
+
+#define ASSERT_NO_MSI(_self) do {					\
+	u64 __value;							\
+									\
+	if (_self->device->driver.features & VFIO_PCI_DRIVER_F_NO_SEND_MSI) \
+		break;							\
+	ASSERT_EQ(-1, read(_self->msi_fd, &__value, 8));		\
+	ASSERT_EQ(EAGAIN, errno);					\
 } while (0)
 
 static void region_setup(struct iommu *iommu,
@@ -129,7 +136,7 @@ TEST_F(vfio_pci_driver_test, init_remove)
 
 TEST_F(vfio_pci_driver_test, memcpy_success)
 {
-	fcntl_set_nonblock(self->msi_fd);
+	fcntl_set_msi_nonblock(self);
 
 	memset(self->src, 'x', self->size);
 	memset(self->dst, 'y', self->size);
@@ -140,12 +147,12 @@ TEST_F(vfio_pci_driver_test, memcpy_success)
 					    self->size));
 
 	ASSERT_EQ(0, memcmp(self->src, self->dst, self->size));
-	ASSERT_NO_MSI(self->msi_fd);
+	ASSERT_NO_MSI(self);
 }
 
 TEST_F(vfio_pci_driver_test, memcpy_from_unmapped_iova)
 {
-	fcntl_set_nonblock(self->msi_fd);
+	fcntl_set_msi_nonblock(self);
 
 	/*
 	 * Ignore the return value since not all devices will detect and report
@@ -153,13 +160,12 @@ TEST_F(vfio_pci_driver_test, memcpy_from_unmapped_iova)
 	 */
 	vfio_pci_driver_memcpy(self->device, self->unmapped_iova,
 			       self->dst_iova, self->size);
-
-	ASSERT_NO_MSI(self->msi_fd);
+	ASSERT_NO_MSI(self);
 }
 
 TEST_F(vfio_pci_driver_test, memcpy_to_unmapped_iova)
 {
-	fcntl_set_nonblock(self->msi_fd);
+	fcntl_set_msi_nonblock(self);
 
 	/*
 	 * Ignore the return value since not all devices will detect and report
@@ -167,15 +173,15 @@ TEST_F(vfio_pci_driver_test, memcpy_to_unmapped_iova)
 	 */
 	vfio_pci_driver_memcpy(self->device, self->src_iova,
 			       self->unmapped_iova, self->size);
-
-	ASSERT_NO_MSI(self->msi_fd);
+	ASSERT_NO_MSI(self);
 }
 
 TEST_F(vfio_pci_driver_test, send_msi)
 {
 	u64 value;
 
-	vfio_pci_driver_send_msi(self->device);
+	if (vfio_pci_driver_send_msi(self->device))
+		SKIP(return, "Device does not support MSI\n");
 	ASSERT_EQ(8, read(self->msi_fd, &value, 8));
 	ASSERT_EQ(1, value);
 }
@@ -201,7 +207,8 @@ TEST_F(vfio_pci_driver_test, mix_and_match)
 				       self->dst_iova,
 				       self->size);
 
-		vfio_pci_driver_send_msi(self->device);
+		if (vfio_pci_driver_send_msi(self->device))
+			continue;
 		ASSERT_EQ(8, read(self->msi_fd, &value, 8));
 		ASSERT_EQ(1, value);
 	}
@@ -213,7 +220,7 @@ TEST_F_TIMEOUT(vfio_pci_driver_test, memcpy_storm, 60)
 	u64 total_size;
 	u64 count;
 
-	fcntl_set_nonblock(self->msi_fd);
+	fcntl_set_msi_nonblock(self);
 
 	/*
 	 * Perform up to 250GiB worth of DMA reads and writes across several
@@ -230,7 +237,7 @@ TEST_F_TIMEOUT(vfio_pci_driver_test, memcpy_storm, 60)
 				     self->size, count);
 
 	ASSERT_EQ(0, vfio_pci_driver_memcpy_wait(self->device));
-	ASSERT_NO_MSI(self->msi_fd);
+	ASSERT_NO_MSI(self);
 }
 
 static bool device_has_selftests_driver(const char *bdf)
-- 
2.43.0
Re: [PATCH v10 1/2] selftests/vfio: Add NO_SEND_MSI feature flag and MSI helper macros
Posted by David Matlack 6 days, 1 hour ago
On 2026-03-25 02:43 PM, Rubin Du wrote:

> diff --git a/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_driver.h b/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_driver.h
> index e5ada209b1d1..f2cf2608f757 100644
> --- a/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_driver.h
> +++ b/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_driver.h
> @@ -6,6 +6,8 @@
>  
>  struct vfio_pci_device;
>  
> +#define VFIO_PCI_DRIVER_F_NO_SEND_MSI	(1UL << 0) /* Device cannot trigger MSI */

I don't see any particular reason to add this flag. All of the checks
for...

  device->driver.feature & VFIO_PCI_DRIVER_F_NO_SEND_MSI

can just be rewritten as...

  !device->driver.ops->send_msi

to accomplish the same thing. And the latter is less characters while
still being readable.

> -void vfio_pci_driver_send_msi(struct vfio_pci_device *device)
> +int vfio_pci_driver_send_msi(struct vfio_pci_device *device)
>  {
>  	struct vfio_pci_driver *driver = &device->driver;
>  
> +	if (driver->features & VFIO_PCI_DRIVER_F_NO_SEND_MSI)
> +		return -EOPNOTSUPP;
> +

I think it would be better to just let vfio_pci_driver_send_msi() fail
if send_msi() is NULL.

Tests should not be calling vfio_pci_driver_send_msi() if the driver
does not support send_msi(). Tests that require send_msi() should
explicitly check for it, rather than detect it by trying to send an MSI.

It's also more readable to explicitly check for send_msi() support
rather than silently skipping or exiting if send_msi() returns an error.
The latter reads like it could be due to some problem on the device,
rather than just a lack of support in the driver.

> -#define ASSERT_NO_MSI(_eventfd) do {			\
> -	u64 __value;					\
> -							\
> -	ASSERT_EQ(-1, read(_eventfd, &__value, 8));	\
> -	ASSERT_EQ(EAGAIN, errno);			\
> +#define fcntl_set_msi_nonblock(_self) do {				\
> +	if (!(_self->device->driver.features & VFIO_PCI_DRIVER_F_NO_SEND_MSI)) \
> +		fcntl_set_nonblock(_self->msi_fd);			\
> +} while (0)

I think it may be worthwhile to have the nv_falcon driver still enable
at least one MSI or MSI-x vector on the device and set driver->msi. This
will reduce divergence from other drivers, and simplify this commit down
to just:

diff --git a/tools/testing/selftests/vfio/vfio_pci_driver_test.c b/tools/testing/selftests/vfio/vfio_pci_driver_test.c
index afa0480ddd9b..ebd66d677abb 100644
--- a/tools/testing/selftests/vfio/vfio_pci_driver_test.c
+++ b/tools/testing/selftests/vfio/vfio_pci_driver_test.c
@@ -175,6 +175,9 @@ TEST_F(vfio_pci_driver_test, send_msi)
 {
        u64 value;

+       if (!self->device->driver.ops->send_msi)
+               SKIP(return, "Driver does not support send_msi()");
+
        vfio_pci_driver_send_msi(self->device);
        ASSERT_EQ(8, read(self->msi_fd, &value, 8));
        ASSERT_EQ(1, value);
@@ -201,6 +204,9 @@ TEST_F(vfio_pci_driver_test, mix_and_match)
                                       self->dst_iova,
                                       self->size);

+               if (!self->device->driver.ops->send_msi)
+                       continue;
+
                vfio_pci_driver_send_msi(self->device);
                ASSERT_EQ(8, read(self->msi_fd, &value, 8));
                ASSERT_EQ(1, value);

>  TEST_F(vfio_pci_driver_test, send_msi)
>  {
>  	u64 value;
>  
> -	vfio_pci_driver_send_msi(self->device);
> +	if (vfio_pci_driver_send_msi(self->device))
> +		SKIP(return, "Device does not support MSI\n");

NV GPUs do support MSI/x though, right? This should probably be:

  SKIP(return, "Driver does not support send_msi()\n");