Commit 30471982 authored by Alex Williamson's avatar Alex Williamson Committed by Alex Williamson
Browse files

vfio/cdx: Consolidate MSI configured state onto cdx_irqs



struct vfio_cdx_device carries three fields that track whether MSI has
been configured: vdev->cdx_irqs (the allocated vector array), vdev->
msi_count (the array length), and vdev->config_msi (a boolean flag).
The three are set together when vfio_cdx_msi_enable() succeeds and
cleared together by vfio_cdx_msi_disable().  However, the error paths
in vfio_cdx_msi_enable() free the cdx_irqs allocation on failure
without resetting the pointer, leaving it stale and skewed from the
other two fields until the next enable call overwrites it.

Clear vdev->cdx_irqs to NULL alongside the kfree() in both error paths
so the pointer consistently reflects the configured state.  With that
invariant restored and access to the MSI state serialized by
cdx_irqs_lock, vdev->config_msi is fully redundant with
(vdev->cdx_irqs != NULL).  Drop the config_msi field and switch all
readers to test cdx_irqs directly.

Assisted-by: Claude:claude-opus-4-7
Signed-off-by: default avatarAlex Williamson <alex.williamson@nvidia.com>
Acked-by: default avatarNikhil Agarwal <nikhil.agarwal@amd.com>
Link: https://lore.kernel.org/r/20260417202800.88287-4-alex.williamson@nvidia.com


Signed-off-by: default avatarAlex Williamson <alex@shazbot.org>
parent 670e8864
Loading
Loading
Loading
Loading
+14 −15
Original line number Diff line number Diff line
@@ -32,26 +32,27 @@ static int vfio_cdx_msi_enable(struct vfio_cdx_device *vdev, int nvec)
		return -ENOMEM;

	ret = cdx_enable_msi(cdx_dev);
	if (ret) {
		kfree(vdev->cdx_irqs);
		return ret;
	}
	if (ret)
		goto err_free;

	/* Allocate cdx MSIs */
	ret = msi_domain_alloc_irqs(dev, MSI_DEFAULT_DOMAIN, nvec);
	if (ret) {
		cdx_disable_msi(cdx_dev);
		kfree(vdev->cdx_irqs);
		return ret;
	}
	if (ret)
		goto err_disable;

	for (msi_idx = 0; msi_idx < nvec; msi_idx++)
		vdev->cdx_irqs[msi_idx].irq_no = msi_get_virq(dev, msi_idx);

	vdev->msi_count = nvec;
	vdev->config_msi = 1;

	return 0;

err_disable:
	cdx_disable_msi(cdx_dev);
err_free:
	kfree(vdev->cdx_irqs);
	vdev->cdx_irqs = NULL;
	return ret;
}

static int vfio_cdx_msi_set_vector_signal(struct vfio_cdx_device *vdev,
@@ -129,7 +130,7 @@ static void vfio_cdx_msi_disable(struct vfio_cdx_device *vdev)

	vfio_cdx_msi_set_block(vdev, 0, vdev->msi_count, NULL);

	if (!vdev->config_msi)
	if (!vdev->cdx_irqs)
		return;

	msi_domain_free_irqs_all(dev, MSI_DEFAULT_DOMAIN);
@@ -138,7 +139,6 @@ static void vfio_cdx_msi_disable(struct vfio_cdx_device *vdev)

	vdev->cdx_irqs = NULL;
	vdev->msi_count = 0;
	vdev->config_msi = 0;
}

static int vfio_cdx_set_msi_trigger(struct vfio_cdx_device *vdev,
@@ -163,7 +163,7 @@ static int vfio_cdx_set_msi_trigger(struct vfio_cdx_device *vdev,
		s32 *fds = data;
		int ret;

		if (vdev->config_msi)
		if (vdev->cdx_irqs)
			return vfio_cdx_msi_set_block(vdev, start, count,
						  fds);
		ret = vfio_cdx_msi_enable(vdev, cdx_dev->num_msi);
@@ -177,8 +177,7 @@ static int vfio_cdx_set_msi_trigger(struct vfio_cdx_device *vdev,
		return ret;
	}

	/* Ensure MSI is configured before accessing cdx_irqs */
	if (!vdev->config_msi)
	if (!vdev->cdx_irqs)
		return -EINVAL;

	for (i = start; i < start + count; i++) {
+0 −1
Original line number Diff line number Diff line
@@ -38,7 +38,6 @@ struct vfio_cdx_device {
	u32			flags;
#define BME_SUPPORT BIT(0)
	u32			msi_count;
	u8			config_msi;
};

#ifdef CONFIG_GENERIC_MSI_IRQ