Hi Kevin,
On 4/27/2023 11:33 PM, Tian, Kevin wrote:
>> From: Chatre, Reinette <reinette.chatre@xxxxxxxxx>
>> Sent: Friday, April 28, 2023 1:36 AM
>>
>> @@ -55,17 +80,28 @@ static void vfio_send_intx_eventfd(void *opaque,
>> void *unused)
>> {
>> struct vfio_pci_core_device *vdev = opaque;
>>
>> - if (likely(is_intx(vdev) && !vdev->virq_disabled))
>> - eventfd_signal(vdev->ctx[0].trigger, 1);
>> + if (likely(is_intx(vdev) && !vdev->virq_disabled)) {
>> + struct vfio_pci_irq_ctx *ctx;
>> +
>> + ctx = vfio_irq_ctx_get(vdev, 0);
>> + if (!ctx)
>> + return;
>
> if this error happens it implies a kernel bug since the same check
> has been done in vfio_intx_enable(). Then should be a WARN_ON().
Sure. Considering that if these are triggered it may result
in many instances, so perhaps WARN_ON_ONCE()?
> ditto for other intx functions which can be called only after intx
> is enabled.
It seems the instances in this category can be identified as the places
where the array contents is currently used without any checks.
I am planning on the following changes:
diff --git a/drivers/vfio/pci/vfio_pci_intrs.c b/drivers/vfio/pci/vfio_pci_intrs.c
index d8dae99cf6d9..b549f5c97cb8 100644
--- a/drivers/vfio/pci/vfio_pci_intrs.c
+++ b/drivers/vfio/pci/vfio_pci_intrs.c
@@ -92,7 +92,7 @@ static void vfio_send_intx_eventfd(void *opaque, void *unused)
struct vfio_pci_irq_ctx *ctx;
ctx = vfio_irq_ctx_get(vdev, 0);
- if (!ctx)
+ if (WARN_ON_ONCE(!ctx))
return;
eventfd_signal(ctx->trigger, 1);
}
@@ -107,7 +107,7 @@ bool vfio_pci_intx_mask(struct vfio_pci_core_device *vdev)
bool masked_changed = false;
ctx = vfio_irq_ctx_get(vdev, 0);
- if (!ctx)
+ if (WARN_ON_ONCE(!ctx))
return masked_changed;
spin_lock_irqsave(&vdev->irqlock, flags);
@@ -154,7 +154,7 @@ static int vfio_pci_intx_unmask_handler(void *opaque, void *unused)
int ret = 0;
ctx = vfio_irq_ctx_get(vdev, 0);
- if (!ctx)
+ if (WARN_ON_ONCE(!ctx))
return ret;
spin_lock_irqsave(&vdev->irqlock, flags);
@@ -200,7 +200,7 @@ static irqreturn_t vfio_intx_handler(int irq, void *dev_id)
int ret = IRQ_NONE;
ctx = vfio_irq_ctx_get(vdev, 0);
- if (!ctx)
+ if (WARN_ON_ONCE(!ctx))
return ret;
spin_lock_irqsave(&vdev->irqlock, flags);
@@ -264,7 +264,7 @@ static int vfio_intx_set_signal(struct vfio_pci_core_device *vdev, int fd)
int ret;
ctx = vfio_irq_ctx_get(vdev, 0);
- if (!ctx)
+ if (WARN_ON_ONCE(!ctx))
return -EINVAL;
if (ctx->trigger) {
@@ -320,6 +320,7 @@ static void vfio_intx_disable(struct vfio_pci_core_device *vdev)
dev_dbg(&vdev->pdev->dev, "%s:%d Disabling INTx\n", __func__, __LINE__);
ctx = vfio_irq_ctx_get(vdev, 0);
+ WARN_ON_ONCE(!ctx);
if (ctx) {
vfio_virqfd_disable(&ctx->unmask);
vfio_virqfd_disable(&ctx->mask);
@@ -586,7 +587,7 @@ static int vfio_pci_set_intx_unmask(struct vfio_pci_core_device *vdev,
struct vfio_pci_irq_ctx *ctx = vfio_irq_ctx_get(vdev, 0);
int32_t fd = *(int32_t *)data;
- if (!ctx)
+ if (WARN_ON_ONCE(!ctx))
return -EINVAL;
if (fd >= 0)
return vfio_virqfd_enable((void *) vdev,
