Thread (17 messages) 17 messages, 4 authors, 2020-07-20

Re: [PATCH V2 4/6] vhost_vdpa: implement IRQ offloading in vhost_vdpa

From: Jason Wang <jasowang@redhat.com>
Date: 2020-07-17 05:30:32
Also in: kvm, virtualization

On 2020/7/16 下午7:23, Zhu Lingshan wrote:
quoted hunk ↗ jump to hunk
This patch introduce a set of functions for setup/unsetup
and update irq offloading respectively by register/unregister
and re-register the irq_bypass_producer.

Signed-off-by: Zhu Lingshan <redacted>
Suggested-by: Jason Wang <jasowang@redhat.com>
---
  drivers/vhost/Kconfig |  1 +
  drivers/vhost/vdpa.c  | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
  2 files changed, 49 insertions(+)
diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig
index d3688c6..587fbae 100644
--- a/drivers/vhost/Kconfig
+++ b/drivers/vhost/Kconfig
@@ -65,6 +65,7 @@ config VHOST_VDPA
  	tristate "Vhost driver for vDPA-based backend"
  	depends on EVENTFD
  	select VHOST
+	select IRQ_BYPASS_MANAGER
  	depends on VDPA
  	help
  	  This kernel module can be loaded in host kernel to accelerate
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index 2fcc422..b9078d4 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -115,6 +115,43 @@ static irqreturn_t vhost_vdpa_config_cb(void *private)
  	return IRQ_HANDLED;
  }
  
+static void vhost_vdpa_setup_vq_irq(struct vdpa_device *dev, int qid, int irq)
+{
+	struct vhost_vdpa *v = vdpa_get_drvdata(dev);
+	struct vhost_virtqueue *vq = &v->vqs[qid];
+	int ret;
+
+	spin_lock(&vq->call_ctx.ctx_lock);
+	if (!vq->call_ctx.ctx) {
+		spin_unlock(&vq->call_ctx.ctx_lock);
+		return;
+	}

I think we can simply remove this check as what is done in 
vhost_vdpq_update_vq_irq().

quoted hunk ↗ jump to hunk
+
+	vq->call_ctx.producer.token = vq->call_ctx.ctx;
+	vq->call_ctx.producer.irq = irq;
+	ret = irq_bypass_register_producer(&vq->call_ctx.producer);
+	spin_unlock(&vq->call_ctx.ctx_lock);
+}
+
+static void vhost_vdpa_unsetup_vq_irq(struct vdpa_device *dev, int qid)
+{
+	struct vhost_vdpa *v = vdpa_get_drvdata(dev);
+	struct vhost_virtqueue *vq = &v->vqs[qid];
+
+	spin_lock(&vq->call_ctx.ctx_lock);
+	irq_bypass_unregister_producer(&vq->call_ctx.producer);
+	spin_unlock(&vq->call_ctx.ctx_lock);
+}
+
+static void vhost_vdpa_update_vq_irq(struct vhost_virtqueue *vq)
+{
+	spin_lock(&vq->call_ctx.ctx_lock);
+	irq_bypass_unregister_producer(&vq->call_ctx.producer);
+	vq->call_ctx.producer.token = vq->call_ctx.ctx;
+	irq_bypass_register_producer(&vq->call_ctx.producer);
+	spin_unlock(&vq->call_ctx.ctx_lock);
+}
+
  static void vhost_vdpa_reset(struct vhost_vdpa *v)
  {
  	struct vdpa_device *vdpa = v->vdpa;
@@ -332,6 +369,7 @@ static long vhost_vdpa_set_config_call(struct vhost_vdpa *v, u32 __user *argp)
  
  	return 0;
  }
+

If you really want to fix coding style issue, it's better to have 
another patch.

quoted hunk ↗ jump to hunk
  static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v, unsigned int cmd,
  				   void __user *argp)
  {
@@ -390,6 +428,14 @@ static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v, unsigned int cmd,
  			cb.private = NULL;
  		}
  		ops->set_vq_cb(vdpa, idx, &cb);
+		/*
+		 * if it has a non-zero irq, means there is a
+		 * previsouly registered irq_bypass_producer,
+		 * we should update it when ctx (its token)
+		 * changes.
+		 */
+		if (vq->call_ctx.producer.irq)
+			vhost_vdpa_update_vq_irq(vq);

Is this safe to check producer.irq outside spinlock?

Thanks

quoted hunk ↗ jump to hunk
  		break;
  
  	case VHOST_SET_VRING_NUM:
@@ -951,6 +997,8 @@ static void vhost_vdpa_remove(struct vdpa_device *vdpa)
  	},
  	.probe	= vhost_vdpa_probe,
  	.remove	= vhost_vdpa_remove,
+	.setup_vq_irq = vhost_vdpa_setup_vq_irq,
+	.unsetup_vq_irq = vhost_vdpa_unsetup_vq_irq,
  };
  
  static int __init vhost_vdpa_init(void)
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help