[v3,04/15] vdpa/ifc: add vdpa interrupt for blk device

Message ID 1643425417-215270-5-git-send-email-andy.pei@intel.com (mailing list archive)
State Changes Requested, archived
Delegated to: Maxime Coquelin
Headers
Series add virtio_blk device support to vdpa/ifc |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Pei, Andy Jan. 29, 2022, 3:03 a.m. UTC
  For the blk we need to relay all the cmd of each queue.

Signed-off-by: Andy Pei <andy.pei@intel.com>
---
 drivers/vdpa/ifc/ifcvf_vdpa.c | 46 ++++++++++++++++++++++++++++++++-----------
 1 file changed, 35 insertions(+), 11 deletions(-)
  

Comments

Maxime Coquelin March 22, 2022, 10:04 a.m. UTC | #1
On 1/29/22 04:03, Andy Pei wrote:
> For the blk we need to relay all the cmd of each queue.

The message is not clear to me, do you mean "For the block device type,
we have to relay the commands on all queues."?

> 
> Signed-off-by: Andy Pei <andy.pei@intel.com>
> ---
>   drivers/vdpa/ifc/ifcvf_vdpa.c | 46 ++++++++++++++++++++++++++++++++-----------
>   1 file changed, 35 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/vdpa/ifc/ifcvf_vdpa.c b/drivers/vdpa/ifc/ifcvf_vdpa.c
> index 778e1fd..4f99bb3 100644
> --- a/drivers/vdpa/ifc/ifcvf_vdpa.c
> +++ b/drivers/vdpa/ifc/ifcvf_vdpa.c
> @@ -372,24 +372,48 @@ struct rte_vdpa_dev_info {
>   	irq_set->index = VFIO_PCI_MSIX_IRQ_INDEX;
>   	irq_set->start = 0;
>   	fd_ptr = (int *)&irq_set->data;
> +	/* The first interrupt is for the configure space change notification */
>   	fd_ptr[RTE_INTR_VEC_ZERO_OFFSET] =
>   		rte_intr_fd_get(internal->pdev->intr_handle);
>   
>   	for (i = 0; i < nr_vring; i++)
>   		internal->intr_fd[i] = -1;
>   
> -	for (i = 0; i < nr_vring; i++) {
> -		rte_vhost_get_vhost_vring(internal->vid, i, &vring);
> -		fd_ptr[RTE_INTR_VEC_RXTX_OFFSET + i] = vring.callfd;
> -		if ((i & 1) == 0 && m_rx == true) {
> -			fd = eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC);
> -			if (fd < 0) {
> -				DRV_LOG(ERR, "can't setup eventfd: %s",
> -					strerror(errno));
> -				return -1;
> +	if (internal->device_type == IFCVF_NET) {
> +		for (i = 0; i < nr_vring; i++) {
> +			rte_vhost_get_vhost_vring(internal->vid, i, &vring);
> +			fd_ptr[RTE_INTR_VEC_RXTX_OFFSET + i] = vring.callfd;
> +			if ((i & 1) == 0 && m_rx == true) {
> +				/* For the net we only need to relay rx queue,
> +				 * which will change the mem of VM.
> +				 */
> +				fd = eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC);
> +				if (fd < 0) {
> +					DRV_LOG(ERR, "can't setup eventfd: %s",
> +						strerror(errno));
> +					return -1;
> +				}
> +				internal->intr_fd[i] = fd;
> +				fd_ptr[RTE_INTR_VEC_RXTX_OFFSET + i] = fd;
> +			}
> +		}
> +	} else if (internal->device_type == IFCVF_BLK) {
> +		for (i = 0; i < nr_vring; i++) {
> +			rte_vhost_get_vhost_vring(internal->vid, i, &vring);
> +			fd_ptr[RTE_INTR_VEC_RXTX_OFFSET + i] = vring.callfd;
> +			if (m_rx == true) {
> +				/* For the blk we need to relay all the read cmd
> +				 * of each queue
> +				 */
> +				fd = eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC);
> +				if (fd < 0) {
> +					DRV_LOG(ERR, "can't setup eventfd: %s",
> +						strerror(errno));
> +					return -1;
> +				}
> +				internal->intr_fd[i] = fd;
> +				fd_ptr[RTE_INTR_VEC_RXTX_OFFSET + i] = fd;


>   			}
> -			internal->intr_fd[i] = fd;
> -			fd_ptr[RTE_INTR_VEC_RXTX_OFFSET + i] = fd;
>   		}
>   	}
>
  
Pei, Andy March 23, 2022, 7:07 a.m. UTC | #2
Hi Maxime,

Thanks for your reply and my reply is inline.

-----Original Message-----
From: Maxime Coquelin <maxime.coquelin@redhat.com> 
Sent: Tuesday, March 22, 2022 6:05 PM
To: Pei, Andy <andy.pei@intel.com>; dev@dpdk.org
Cc: Xia, Chenbo <chenbo.xia@intel.com>; Cao, Gang <gang.cao@intel.com>; Liu, Changpeng <changpeng.liu@intel.com>
Subject: Re: [PATCH v3 04/15] vdpa/ifc: add vdpa interrupt for blk device



On 1/29/22 04:03, Andy Pei wrote:
> For the blk we need to relay all the cmd of each queue.

The message is not clear to me, do you mean "For the block device type, we have to relay the commands on all queues."?
Andy: Yes. For BLK device, device can work with single queue, comparing to NET device, NET device use queue pair.

> 
> Signed-off-by: Andy Pei <andy.pei@intel.com>
> ---
>   drivers/vdpa/ifc/ifcvf_vdpa.c | 46 ++++++++++++++++++++++++++++++++-----------
>   1 file changed, 35 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/vdpa/ifc/ifcvf_vdpa.c 
> b/drivers/vdpa/ifc/ifcvf_vdpa.c index 778e1fd..4f99bb3 100644
> --- a/drivers/vdpa/ifc/ifcvf_vdpa.c
> +++ b/drivers/vdpa/ifc/ifcvf_vdpa.c
> @@ -372,24 +372,48 @@ struct rte_vdpa_dev_info {
>   	irq_set->index = VFIO_PCI_MSIX_IRQ_INDEX;
>   	irq_set->start = 0;
>   	fd_ptr = (int *)&irq_set->data;
> +	/* The first interrupt is for the configure space change 
> +notification */
>   	fd_ptr[RTE_INTR_VEC_ZERO_OFFSET] =
>   		rte_intr_fd_get(internal->pdev->intr_handle);
>   
>   	for (i = 0; i < nr_vring; i++)
>   		internal->intr_fd[i] = -1;
>   
> -	for (i = 0; i < nr_vring; i++) {
> -		rte_vhost_get_vhost_vring(internal->vid, i, &vring);
> -		fd_ptr[RTE_INTR_VEC_RXTX_OFFSET + i] = vring.callfd;
> -		if ((i & 1) == 0 && m_rx == true) {
> -			fd = eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC);
> -			if (fd < 0) {
> -				DRV_LOG(ERR, "can't setup eventfd: %s",
> -					strerror(errno));
> -				return -1;
> +	if (internal->device_type == IFCVF_NET) {
> +		for (i = 0; i < nr_vring; i++) {
> +			rte_vhost_get_vhost_vring(internal->vid, i, &vring);
> +			fd_ptr[RTE_INTR_VEC_RXTX_OFFSET + i] = vring.callfd;
> +			if ((i & 1) == 0 && m_rx == true) {
> +				/* For the net we only need to relay rx queue,
> +				 * which will change the mem of VM.
> +				 */
> +				fd = eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC);
> +				if (fd < 0) {
> +					DRV_LOG(ERR, "can't setup eventfd: %s",
> +						strerror(errno));
> +					return -1;
> +				}
> +				internal->intr_fd[i] = fd;
> +				fd_ptr[RTE_INTR_VEC_RXTX_OFFSET + i] = fd;
> +			}
> +		}
> +	} else if (internal->device_type == IFCVF_BLK) {
> +		for (i = 0; i < nr_vring; i++) {
> +			rte_vhost_get_vhost_vring(internal->vid, i, &vring);
> +			fd_ptr[RTE_INTR_VEC_RXTX_OFFSET + i] = vring.callfd;
> +			if (m_rx == true) {
> +				/* For the blk we need to relay all the read cmd
> +				 * of each queue
> +				 */
> +				fd = eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC);
> +				if (fd < 0) {
> +					DRV_LOG(ERR, "can't setup eventfd: %s",
> +						strerror(errno));
> +					return -1;
> +				}
> +				internal->intr_fd[i] = fd;
> +				fd_ptr[RTE_INTR_VEC_RXTX_OFFSET + i] = fd;


>   			}
> -			internal->intr_fd[i] = fd;
> -			fd_ptr[RTE_INTR_VEC_RXTX_OFFSET + i] = fd;
>   		}
>   	}
>
  
Pei, Andy March 23, 2022, 7:42 a.m. UTC | #3
Hi Maxime,

I think it is better to change the commit log to your description.
"For the block device type, we have to relay the commands on all queues."
In the next version of patch set.

-----Original Message-----
From: Pei, Andy <andy.pei@intel.com> 
Sent: Wednesday, March 23, 2022 3:08 PM
To: Maxime Coquelin <maxime.coquelin@redhat.com>; dev@dpdk.org
Cc: Xia, Chenbo <chenbo.xia@intel.com>; Cao, Gang <gang.cao@intel.com>; Liu, Changpeng <changpeng.liu@intel.com>
Subject: RE: [PATCH v3 04/15] vdpa/ifc: add vdpa interrupt for blk device

Hi Maxime,

Thanks for your reply and my reply is inline.

-----Original Message-----
From: Maxime Coquelin <maxime.coquelin@redhat.com> 
Sent: Tuesday, March 22, 2022 6:05 PM
To: Pei, Andy <andy.pei@intel.com>; dev@dpdk.org
Cc: Xia, Chenbo <chenbo.xia@intel.com>; Cao, Gang <gang.cao@intel.com>; Liu, Changpeng <changpeng.liu@intel.com>
Subject: Re: [PATCH v3 04/15] vdpa/ifc: add vdpa interrupt for blk device



On 1/29/22 04:03, Andy Pei wrote:
> For the blk we need to relay all the cmd of each queue.

The message is not clear to me, do you mean "For the block device type, we have to relay the commands on all queues."?
Andy: Yes. For BLK device, device can work with single queue, comparing to NET device, NET device use queue pair.

> 
> Signed-off-by: Andy Pei <andy.pei@intel.com>
> ---
>   drivers/vdpa/ifc/ifcvf_vdpa.c | 46 ++++++++++++++++++++++++++++++++-----------
>   1 file changed, 35 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/vdpa/ifc/ifcvf_vdpa.c 
> b/drivers/vdpa/ifc/ifcvf_vdpa.c index 778e1fd..4f99bb3 100644
> --- a/drivers/vdpa/ifc/ifcvf_vdpa.c
> +++ b/drivers/vdpa/ifc/ifcvf_vdpa.c
> @@ -372,24 +372,48 @@ struct rte_vdpa_dev_info {
>   	irq_set->index = VFIO_PCI_MSIX_IRQ_INDEX;
>   	irq_set->start = 0;
>   	fd_ptr = (int *)&irq_set->data;
> +	/* The first interrupt is for the configure space change 
> +notification */
>   	fd_ptr[RTE_INTR_VEC_ZERO_OFFSET] =
>   		rte_intr_fd_get(internal->pdev->intr_handle);
>   
>   	for (i = 0; i < nr_vring; i++)
>   		internal->intr_fd[i] = -1;
>   
> -	for (i = 0; i < nr_vring; i++) {
> -		rte_vhost_get_vhost_vring(internal->vid, i, &vring);
> -		fd_ptr[RTE_INTR_VEC_RXTX_OFFSET + i] = vring.callfd;
> -		if ((i & 1) == 0 && m_rx == true) {
> -			fd = eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC);
> -			if (fd < 0) {
> -				DRV_LOG(ERR, "can't setup eventfd: %s",
> -					strerror(errno));
> -				return -1;
> +	if (internal->device_type == IFCVF_NET) {
> +		for (i = 0; i < nr_vring; i++) {
> +			rte_vhost_get_vhost_vring(internal->vid, i, &vring);
> +			fd_ptr[RTE_INTR_VEC_RXTX_OFFSET + i] = vring.callfd;
> +			if ((i & 1) == 0 && m_rx == true) {
> +				/* For the net we only need to relay rx queue,
> +				 * which will change the mem of VM.
> +				 */
> +				fd = eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC);
> +				if (fd < 0) {
> +					DRV_LOG(ERR, "can't setup eventfd: %s",
> +						strerror(errno));
> +					return -1;
> +				}
> +				internal->intr_fd[i] = fd;
> +				fd_ptr[RTE_INTR_VEC_RXTX_OFFSET + i] = fd;
> +			}
> +		}
> +	} else if (internal->device_type == IFCVF_BLK) {
> +		for (i = 0; i < nr_vring; i++) {
> +			rte_vhost_get_vhost_vring(internal->vid, i, &vring);
> +			fd_ptr[RTE_INTR_VEC_RXTX_OFFSET + i] = vring.callfd;
> +			if (m_rx == true) {
> +				/* For the blk we need to relay all the read cmd
> +				 * of each queue
> +				 */
> +				fd = eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC);
> +				if (fd < 0) {
> +					DRV_LOG(ERR, "can't setup eventfd: %s",
> +						strerror(errno));
> +					return -1;
> +				}
> +				internal->intr_fd[i] = fd;
> +				fd_ptr[RTE_INTR_VEC_RXTX_OFFSET + i] = fd;


>   			}
> -			internal->intr_fd[i] = fd;
> -			fd_ptr[RTE_INTR_VEC_RXTX_OFFSET + i] = fd;
>   		}
>   	}
>
  

Patch

diff --git a/drivers/vdpa/ifc/ifcvf_vdpa.c b/drivers/vdpa/ifc/ifcvf_vdpa.c
index 778e1fd..4f99bb3 100644
--- a/drivers/vdpa/ifc/ifcvf_vdpa.c
+++ b/drivers/vdpa/ifc/ifcvf_vdpa.c
@@ -372,24 +372,48 @@  struct rte_vdpa_dev_info {
 	irq_set->index = VFIO_PCI_MSIX_IRQ_INDEX;
 	irq_set->start = 0;
 	fd_ptr = (int *)&irq_set->data;
+	/* The first interrupt is for the configure space change notification */
 	fd_ptr[RTE_INTR_VEC_ZERO_OFFSET] =
 		rte_intr_fd_get(internal->pdev->intr_handle);
 
 	for (i = 0; i < nr_vring; i++)
 		internal->intr_fd[i] = -1;
 
-	for (i = 0; i < nr_vring; i++) {
-		rte_vhost_get_vhost_vring(internal->vid, i, &vring);
-		fd_ptr[RTE_INTR_VEC_RXTX_OFFSET + i] = vring.callfd;
-		if ((i & 1) == 0 && m_rx == true) {
-			fd = eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC);
-			if (fd < 0) {
-				DRV_LOG(ERR, "can't setup eventfd: %s",
-					strerror(errno));
-				return -1;
+	if (internal->device_type == IFCVF_NET) {
+		for (i = 0; i < nr_vring; i++) {
+			rte_vhost_get_vhost_vring(internal->vid, i, &vring);
+			fd_ptr[RTE_INTR_VEC_RXTX_OFFSET + i] = vring.callfd;
+			if ((i & 1) == 0 && m_rx == true) {
+				/* For the net we only need to relay rx queue,
+				 * which will change the mem of VM.
+				 */
+				fd = eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC);
+				if (fd < 0) {
+					DRV_LOG(ERR, "can't setup eventfd: %s",
+						strerror(errno));
+					return -1;
+				}
+				internal->intr_fd[i] = fd;
+				fd_ptr[RTE_INTR_VEC_RXTX_OFFSET + i] = fd;
+			}
+		}
+	} else if (internal->device_type == IFCVF_BLK) {
+		for (i = 0; i < nr_vring; i++) {
+			rte_vhost_get_vhost_vring(internal->vid, i, &vring);
+			fd_ptr[RTE_INTR_VEC_RXTX_OFFSET + i] = vring.callfd;
+			if (m_rx == true) {
+				/* For the blk we need to relay all the read cmd
+				 * of each queue
+				 */
+				fd = eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC);
+				if (fd < 0) {
+					DRV_LOG(ERR, "can't setup eventfd: %s",
+						strerror(errno));
+					return -1;
+				}
+				internal->intr_fd[i] = fd;
+				fd_ptr[RTE_INTR_VEC_RXTX_OFFSET + i] = fd;
 			}
-			internal->intr_fd[i] = fd;
-			fd_ptr[RTE_INTR_VEC_RXTX_OFFSET + i] = fd;
 		}
 	}