diff mbox series

[v2,5/8] vdpa/ifc: only configure enabled queue

Message ID 1662616458-164613-6-git-send-email-andy.pei@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers
Series vdpa/ifc: add multi queue support |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Pei, Andy Sept. 8, 2022, 5:54 a.m. UTC 
  when configure the hardware queue, we only configure queues which
have been enabled by vhost.

Signed-off-by: Andy Pei <andy.pei@intel.com>
Signed-off-by: Huang Wei <wei_huang@intel.com>
---
 drivers/vdpa/ifc/base/ifcvf.c |  6 +++++-
 drivers/vdpa/ifc/ifcvf_vdpa.c | 16 ++++++++++++++--
 2 files changed, 19 insertions(+), 3 deletions(-)

Comments

Chenbo Xia Sept. 14, 2022, 1:59 a.m. UTC | #1 
> -----Original Message-----
> From: Pei, Andy <andy.pei@intel.com>
> Sent: Thursday, September 8, 2022 1:54 PM
> To: dev@dpdk.org
> Cc: Xia, Chenbo <chenbo.xia@intel.com>; Xu, Rosen <rosen.xu@intel.com>;
> Huang, Wei <wei.huang@intel.com>; Cao, Gang <gang.cao@intel.com>;
> maxime.coquelin@redhat.com; Huang Wei <wei_huang@intel.com>
> Subject: [PATCH v2 5/8] vdpa/ifc: only configure enabled queue
> 
> when configure the hardware queue, we only configure queues which
> have been enabled by vhost.
> 
> Signed-off-by: Andy Pei <andy.pei@intel.com>
> Signed-off-by: Huang Wei <wei_huang@intel.com>
> ---
>  drivers/vdpa/ifc/base/ifcvf.c |  6 +++++-
>  drivers/vdpa/ifc/ifcvf_vdpa.c | 16 ++++++++++++++--
>  2 files changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/vdpa/ifc/base/ifcvf.c b/drivers/vdpa/ifc/base/ifcvf.c
> index 0444d74..4875ea1 100644
> --- a/drivers/vdpa/ifc/base/ifcvf.c
> +++ b/drivers/vdpa/ifc/base/ifcvf.c
> @@ -249,6 +249,9 @@
> 
>  	ifcvf_enable_multiqueue(hw);
>  	for (i = 0; i < hw->nr_vring; i++) {
> +		if (!hw->vring[i].enable)
> +			continue;
> +
>  		IFCVF_WRITE_REG16(i, &cfg->queue_select);
>  		io_write64_twopart(hw->vring[i].desc, &cfg->queue_desc_lo,
>  				&cfg->queue_desc_hi);
> @@ -283,7 +286,8 @@
>  		notify_off = IFCVF_READ_REG16(&cfg->queue_notify_off);
>  		hw->notify_addr[i] = (void *)((u8 *)hw->notify_base +
>  				notify_off * hw->notify_off_multiplier);
> -		IFCVF_WRITE_REG16(1, &cfg->queue_enable);
> +		if (hw->vring[i].enable)

Seems useless check as it already checked when the for loop starts

Thanks,
Chenbo

> +			IFCVF_WRITE_REG16(1, &cfg->queue_enable);
>  	}
> 
>  	return 0;
> diff --git a/drivers/vdpa/ifc/ifcvf_vdpa.c b/drivers/vdpa/ifc/ifcvf_vdpa.c
> index 2b42850..48f1a89 100644
> --- a/drivers/vdpa/ifc/ifcvf_vdpa.c
> +++ b/drivers/vdpa/ifc/ifcvf_vdpa.c
> @@ -290,6 +290,8 @@ struct rte_vdpa_dev_info {
>  	rte_vhost_get_negotiated_features(vid, &hw->req_features);
> 
>  	for (i = 0; i < nr_vring; i++) {
> +		if (!hw->vring[i].enable)
> +			continue;
>  		rte_vhost_get_vhost_vring(vid, i, &vq);
>  		gpa = hva_to_gpa(vid, (uint64_t)(uintptr_t)vq.desc);
>  		if (gpa == 0) {
> @@ -505,6 +507,8 @@ struct rte_vdpa_dev_info {
> 
>  	vring.kickfd = -1;
>  	for (qid = 0; qid < q_num; qid++) {
> +		if (!hw->vring[qid].enable)
> +			continue;
>  		ev.events = EPOLLIN | EPOLLPRI;
>  		rte_vhost_get_vhost_vring(internal->vid, qid, &vring);
>  		ev.data.u64 = qid | (uint64_t)vring.kickfd << 32;
> @@ -1064,6 +1068,8 @@ struct rte_vdpa_dev_info {
>  	struct rte_vdpa_device *vdev;
>  	struct internal_list *list;
>  	struct ifcvf_internal *internal;
> +	struct ifcvf_hw *hw;
> +	uint16_t i;
> 
>  	vdev = rte_vhost_get_vdpa_device(vid);
>  	list = find_internal_resource_by_vdev(vdev);
> @@ -1077,11 +1083,17 @@ struct rte_vdpa_dev_info {
>  	rte_atomic32_set(&internal->dev_attached, 1);
>  	update_datapath(internal);
> 
> -	if (rte_vhost_host_notifier_ctrl(vid, RTE_VHOST_QUEUE_ALL, true) !=
> 0)
> -		DRV_LOG(NOTICE, "vDPA (%s): software relay is used.",
> +	hw = &internal->hw;
> +	for (i = 0; i < hw->nr_vring; i++) {
> +		if (!hw->vring[i].enable)
> +			continue;
> +		if (rte_vhost_host_notifier_ctrl(vid, i, true) != 0)
> +			DRV_LOG(NOTICE, "vDPA (%s): software relay is used.",
>  				vdev->device->name);
> +	}
> 
>  	internal->configured = 1;
> +	DRV_LOG(INFO, "vDPA device %s is configured", vdev->device->name);
>  	return 0;
>  }
> 
> --
> 1.8.3.1
Wei Huang Sept. 14, 2022, 2:57 a.m. UTC | #2 
> -----Original Message-----
> From: Xia, Chenbo <chenbo.xia@intel.com>
> Sent: Wednesday, September 14, 2022 09:59
> To: Pei, Andy <andy.pei@intel.com>; dev@dpdk.org
> Cc: Xu, Rosen <rosen.xu@intel.com>; Huang, Wei <wei.huang@intel.com>; Cao,
> Gang <gang.cao@intel.com>; maxime.coquelin@redhat.com; Huang Wei
> <wei_huang@intel.com>
> Subject: RE: [PATCH v2 5/8] vdpa/ifc: only configure enabled queue
> 
> > -----Original Message-----
> > From: Pei, Andy <andy.pei@intel.com>
> > Sent: Thursday, September 8, 2022 1:54 PM
> > To: dev@dpdk.org
> > Cc: Xia, Chenbo <chenbo.xia@intel.com>; Xu, Rosen
> > <rosen.xu@intel.com>; Huang, Wei <wei.huang@intel.com>; Cao, Gang
> > <gang.cao@intel.com>; maxime.coquelin@redhat.com; Huang Wei
> > <wei_huang@intel.com>
> > Subject: [PATCH v2 5/8] vdpa/ifc: only configure enabled queue
> >
> > when configure the hardware queue, we only configure queues which have
> > been enabled by vhost.
> >
> > Signed-off-by: Andy Pei <andy.pei@intel.com>
> > Signed-off-by: Huang Wei <wei_huang@intel.com>
> > ---
> >  drivers/vdpa/ifc/base/ifcvf.c |  6 +++++-
> > drivers/vdpa/ifc/ifcvf_vdpa.c | 16 ++++++++++++++--
> >  2 files changed, 19 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/vdpa/ifc/base/ifcvf.c
> > b/drivers/vdpa/ifc/base/ifcvf.c index 0444d74..4875ea1 100644
> > --- a/drivers/vdpa/ifc/base/ifcvf.c
> > +++ b/drivers/vdpa/ifc/base/ifcvf.c
> > @@ -249,6 +249,9 @@
> >
> >  	ifcvf_enable_multiqueue(hw);
> >  	for (i = 0; i < hw->nr_vring; i++) {
> > +		if (!hw->vring[i].enable)
> > +			continue;
> > +
> >  		IFCVF_WRITE_REG16(i, &cfg->queue_select);
> >  		io_write64_twopart(hw->vring[i].desc, &cfg->queue_desc_lo,
> >  				&cfg->queue_desc_hi);
> > @@ -283,7 +286,8 @@
> >  		notify_off = IFCVF_READ_REG16(&cfg->queue_notify_off);
> >  		hw->notify_addr[i] = (void *)((u8 *)hw->notify_base +
> >  				notify_off * hw->notify_off_multiplier);
> > -		IFCVF_WRITE_REG16(1, &cfg->queue_enable);
> > +		if (hw->vring[i].enable)
> 
> Seems useless check as it already checked when the for loop starts
Yes, it should be removed.
> 
> Thanks,
> Chenbo
> 
> > +			IFCVF_WRITE_REG16(1, &cfg->queue_enable);
> >  	}
> >
> >  	return 0;
> > diff --git a/drivers/vdpa/ifc/ifcvf_vdpa.c
> > b/drivers/vdpa/ifc/ifcvf_vdpa.c index 2b42850..48f1a89 100644
> > --- a/drivers/vdpa/ifc/ifcvf_vdpa.c
> > +++ b/drivers/vdpa/ifc/ifcvf_vdpa.c
> > @@ -290,6 +290,8 @@ struct rte_vdpa_dev_info {
> >  	rte_vhost_get_negotiated_features(vid, &hw->req_features);
> >
> >  	for (i = 0; i < nr_vring; i++) {
> > +		if (!hw->vring[i].enable)
> > +			continue;
> >  		rte_vhost_get_vhost_vring(vid, i, &vq);
> >  		gpa = hva_to_gpa(vid, (uint64_t)(uintptr_t)vq.desc);
> >  		if (gpa == 0) {
> > @@ -505,6 +507,8 @@ struct rte_vdpa_dev_info {
> >
> >  	vring.kickfd = -1;
> >  	for (qid = 0; qid < q_num; qid++) {
> > +		if (!hw->vring[qid].enable)
> > +			continue;
> >  		ev.events = EPOLLIN | EPOLLPRI;
> >  		rte_vhost_get_vhost_vring(internal->vid, qid, &vring);
> >  		ev.data.u64 = qid | (uint64_t)vring.kickfd << 32; @@ -1064,6
> > +1068,8 @@ struct rte_vdpa_dev_info {
> >  	struct rte_vdpa_device *vdev;
> >  	struct internal_list *list;
> >  	struct ifcvf_internal *internal;
> > +	struct ifcvf_hw *hw;
> > +	uint16_t i;
> >
> >  	vdev = rte_vhost_get_vdpa_device(vid);
> >  	list = find_internal_resource_by_vdev(vdev);
> > @@ -1077,11 +1083,17 @@ struct rte_vdpa_dev_info {
> >  	rte_atomic32_set(&internal->dev_attached, 1);
> >  	update_datapath(internal);
> >
> > -	if (rte_vhost_host_notifier_ctrl(vid, RTE_VHOST_QUEUE_ALL, true) !=
> > 0)
> > -		DRV_LOG(NOTICE, "vDPA (%s): software relay is used.",
> > +	hw = &internal->hw;
> > +	for (i = 0; i < hw->nr_vring; i++) {
> > +		if (!hw->vring[i].enable)
> > +			continue;
> > +		if (rte_vhost_host_notifier_ctrl(vid, i, true) != 0)
> > +			DRV_LOG(NOTICE, "vDPA (%s): software relay is used.",
> >  				vdev->device->name);
> > +	}
> >
> >  	internal->configured = 1;
> > +	DRV_LOG(INFO, "vDPA device %s is configured", vdev->device->name);
> >  	return 0;
> >  }
> >
> > --
> > 1.8.3.1
diff mbox series

Patch

diff --git a/drivers/vdpa/ifc/base/ifcvf.c b/drivers/vdpa/ifc/base/ifcvf.c
index 0444d74..4875ea1 100644
--- a/drivers/vdpa/ifc/base/ifcvf.c
+++ b/drivers/vdpa/ifc/base/ifcvf.c
@@ -249,6 +249,9 @@ 
 
 	ifcvf_enable_multiqueue(hw);
 	for (i = 0; i < hw->nr_vring; i++) {
+		if (!hw->vring[i].enable)
+			continue;
+
 		IFCVF_WRITE_REG16(i, &cfg->queue_select);
 		io_write64_twopart(hw->vring[i].desc, &cfg->queue_desc_lo,
 				&cfg->queue_desc_hi);
@@ -283,7 +286,8 @@ 
 		notify_off = IFCVF_READ_REG16(&cfg->queue_notify_off);
 		hw->notify_addr[i] = (void *)((u8 *)hw->notify_base +
 				notify_off * hw->notify_off_multiplier);
-		IFCVF_WRITE_REG16(1, &cfg->queue_enable);
+		if (hw->vring[i].enable)
+			IFCVF_WRITE_REG16(1, &cfg->queue_enable);
 	}
 
 	return 0;
diff --git a/drivers/vdpa/ifc/ifcvf_vdpa.c b/drivers/vdpa/ifc/ifcvf_vdpa.c
index 2b42850..48f1a89 100644
--- a/drivers/vdpa/ifc/ifcvf_vdpa.c
+++ b/drivers/vdpa/ifc/ifcvf_vdpa.c
@@ -290,6 +290,8 @@  struct rte_vdpa_dev_info {
 	rte_vhost_get_negotiated_features(vid, &hw->req_features);
 
 	for (i = 0; i < nr_vring; i++) {
+		if (!hw->vring[i].enable)
+			continue;
 		rte_vhost_get_vhost_vring(vid, i, &vq);
 		gpa = hva_to_gpa(vid, (uint64_t)(uintptr_t)vq.desc);
 		if (gpa == 0) {
@@ -505,6 +507,8 @@  struct rte_vdpa_dev_info {
 
 	vring.kickfd = -1;
 	for (qid = 0; qid < q_num; qid++) {
+		if (!hw->vring[qid].enable)
+			continue;
 		ev.events = EPOLLIN | EPOLLPRI;
 		rte_vhost_get_vhost_vring(internal->vid, qid, &vring);
 		ev.data.u64 = qid | (uint64_t)vring.kickfd << 32;
@@ -1064,6 +1068,8 @@  struct rte_vdpa_dev_info {
 	struct rte_vdpa_device *vdev;
 	struct internal_list *list;
 	struct ifcvf_internal *internal;
+	struct ifcvf_hw *hw;
+	uint16_t i;
 
 	vdev = rte_vhost_get_vdpa_device(vid);
 	list = find_internal_resource_by_vdev(vdev);
@@ -1077,11 +1083,17 @@  struct rte_vdpa_dev_info {
 	rte_atomic32_set(&internal->dev_attached, 1);
 	update_datapath(internal);
 
-	if (rte_vhost_host_notifier_ctrl(vid, RTE_VHOST_QUEUE_ALL, true) != 0)
-		DRV_LOG(NOTICE, "vDPA (%s): software relay is used.",
+	hw = &internal->hw;
+	for (i = 0; i < hw->nr_vring; i++) {
+		if (!hw->vring[i].enable)
+			continue;
+		if (rte_vhost_host_notifier_ctrl(vid, i, true) != 0)
+			DRV_LOG(NOTICE, "vDPA (%s): software relay is used.",
 				vdev->device->name);
+	}
 
 	internal->configured = 1;
+	DRV_LOG(INFO, "vDPA device %s is configured", vdev->device->name);
 	return 0;
 }