* [dpdk-dev] [PATCH v1] vdpa/ifc: fix vring update after device config @ 2020-07-24 17:09 Chenbo Xia 2020-07-26 13:27 ` Xu, Rosen 2020-07-28 14:32 ` [dpdk-dev] [PATCH v2] " Chenbo Xia 0 siblings, 2 replies; 9+ messages in thread From: Chenbo Xia @ 2020-07-24 17:09 UTC (permalink / raw) To: dev; +Cc: maxime.coquelin, ferruh.yigit, xiao.w.wang, rosen.xu The device ready state in vhost lib is now defined as the state that first queue pair is ready. And kick/callfd may be updated by QEMU when ifc device is configured. Although now ifc driver only supports one queue pair, it still has to update callfd when working with QEMU. This patch fixes this vring update problem by implementing the set_vring_state callback. Fixes: a3f8150eac6d (net/ifcvf: add ifcvf vDPA driver) Suggested-by: Maxime Coquelin <maxime.coquelin@redhat.com> Signed-off-by: Chenbo Xia <chenbo.xia@intel.com> Acked-by: Wang Xiao W <xiao.w.wang@intel.com> --- drivers/vdpa/ifc/base/ifcvf.h | 1 + drivers/vdpa/ifc/ifcvf_vdpa.c | 54 ++++++++++++++++++++++++++++++++++- 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/drivers/vdpa/ifc/base/ifcvf.h b/drivers/vdpa/ifc/base/ifcvf.h index eb04a9406..a288ce57d 100644 --- a/drivers/vdpa/ifc/base/ifcvf.h +++ b/drivers/vdpa/ifc/base/ifcvf.h @@ -115,6 +115,7 @@ struct vring_info { u16 size; u16 last_avail_idx; u16 last_used_idx; + bool enable; }; struct ifcvf_hw { diff --git a/drivers/vdpa/ifc/ifcvf_vdpa.c b/drivers/vdpa/ifc/ifcvf_vdpa.c index a757d45ec..9da25f0c7 100644 --- a/drivers/vdpa/ifc/ifcvf_vdpa.c +++ b/drivers/vdpa/ifc/ifcvf_vdpa.c @@ -49,6 +49,7 @@ static const char * const ifcvf_valid_arguments[] = { struct ifcvf_internal { struct rte_pci_device *pdev; struct ifcvf_hw hw; + int configured; int vfio_container_fd; int vfio_group_fd; int vfio_dev_fd; @@ -897,6 +898,7 @@ ifcvf_dev_config(int vid) DRV_LOG(NOTICE, "vDPA (%s): software relay is used.", vdev->device->name); + internal->configured = 1; return 0; } @@ -935,6 +937,7 @@ ifcvf_dev_close(int vid) update_datapath(internal); } + internal->configured = 0; return 0; } @@ -1084,13 +1087,61 @@ ifcvf_get_protocol_features(struct rte_vdpa_device *vdev, uint64_t *features) return 0; } +static int +ifcvf_set_vring_state(int vid, int vring, int state) +{ + struct rte_vdpa_device *vdev; + struct internal_list *list; + struct ifcvf_internal *internal; + struct ifcvf_hw *hw; + struct ifcvf_pci_common_cfg *cfg; + int ret = 0; + + vdev = rte_vhost_get_vdpa_device(vid); + list = find_internal_resource_by_vdev(vdev); + if (list == NULL) { + DRV_LOG(ERR, "Invalid vDPA device: %p", vdev); + return -1; + } + + internal = list->internal; + if (vring < 0 || vring >= internal->max_queues * 2) { + DRV_LOG(ERR, "Vring index %d not correct", vring); + return -1; + } + + hw = &internal->hw; + if (!internal->configured) + goto exit; + + cfg = hw->common_cfg; + IFCVF_WRITE_REG16(vring, &cfg->queue_select); + IFCVF_WRITE_REG16(state, &cfg->queue_enable); + + if (!state && hw->vring[vring].enable) { + ret = vdpa_disable_vfio_intr(internal); + if (ret) + return ret; + } + + if (state && !hw->vring[vring].enable) { + ret = vdpa_enable_vfio_intr(internal, 0); + if (ret) + return ret; + } + +exit: + hw->vring[vring].enable = !!state; + return 0; +} + static struct rte_vdpa_dev_ops ifcvf_ops = { .get_queue_num = ifcvf_get_queue_num, .get_features = ifcvf_get_vdpa_features, .get_protocol_features = ifcvf_get_protocol_features, .dev_conf = ifcvf_dev_config, .dev_close = ifcvf_dev_close, - .set_vring_state = NULL, + .set_vring_state = ifcvf_set_vring_state, .set_features = ifcvf_set_features, .migration_done = NULL, .get_vfio_group_fd = ifcvf_get_vfio_group_fd, @@ -1170,6 +1221,7 @@ ifcvf_pci_probe(struct rte_pci_driver *pci_drv __rte_unused, goto error; } + internal->configured = 0; internal->max_queues = IFCVF_MAX_QUEUES; features = ifcvf_get_features(&internal->hw); internal->features = (features & -- 2.17.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH v1] vdpa/ifc: fix vring update after device config 2020-07-24 17:09 [dpdk-dev] [PATCH v1] vdpa/ifc: fix vring update after device config Chenbo Xia @ 2020-07-26 13:27 ` Xu, Rosen 2020-07-28 2:35 ` Xia, Chenbo 2020-07-28 14:32 ` [dpdk-dev] [PATCH v2] " Chenbo Xia 1 sibling, 1 reply; 9+ messages in thread From: Xu, Rosen @ 2020-07-26 13:27 UTC (permalink / raw) To: Xia, Chenbo, dev; +Cc: maxime.coquelin, Yigit, Ferruh, Wang, Xiao W Hi, > -----Original Message----- > From: Xia, Chenbo <chenbo.xia@intel.com> > Sent: Saturday, July 25, 2020 1:09 > To: dev@dpdk.org > Cc: maxime.coquelin@redhat.com; Yigit, Ferruh <ferruh.yigit@intel.com>; > Wang, Xiao W <xiao.w.wang@intel.com>; Xu, Rosen <rosen.xu@intel.com> > Subject: [PATCH v1] vdpa/ifc: fix vring update after device config > > The device ready state in vhost lib is now defined as the state that first > queue pair is ready. And kick/callfd may be updated by QEMU when ifc > device is configured. > > Although now ifc driver only supports one queue pair, it still has to update > callfd when working with QEMU. This patch fixes this vring update problem > by implementing the set_vring_state callback. > > Fixes: a3f8150eac6d (net/ifcvf: add ifcvf vDPA driver) > > Suggested-by: Maxime Coquelin <maxime.coquelin@redhat.com> > Signed-off-by: Chenbo Xia <chenbo.xia@intel.com> > Acked-by: Wang Xiao W <xiao.w.wang@intel.com> > --- > drivers/vdpa/ifc/base/ifcvf.h | 1 + > drivers/vdpa/ifc/ifcvf_vdpa.c | 54 > ++++++++++++++++++++++++++++++++++- > 2 files changed, 54 insertions(+), 1 deletion(-) > > diff --git a/drivers/vdpa/ifc/base/ifcvf.h b/drivers/vdpa/ifc/base/ifcvf.h > index eb04a9406..a288ce57d 100644 > --- a/drivers/vdpa/ifc/base/ifcvf.h > +++ b/drivers/vdpa/ifc/base/ifcvf.h > @@ -115,6 +115,7 @@ struct vring_info { > u16 size; > u16 last_avail_idx; > u16 last_used_idx; > + bool enable; > }; > > struct ifcvf_hw { > diff --git a/drivers/vdpa/ifc/ifcvf_vdpa.c b/drivers/vdpa/ifc/ifcvf_vdpa.c > index a757d45ec..9da25f0c7 100644 > --- a/drivers/vdpa/ifc/ifcvf_vdpa.c > +++ b/drivers/vdpa/ifc/ifcvf_vdpa.c > @@ -49,6 +49,7 @@ static const char * const ifcvf_valid_arguments[] = > { struct ifcvf_internal { > struct rte_pci_device *pdev; > struct ifcvf_hw hw; > + int configured; > int vfio_container_fd; > int vfio_group_fd; > int vfio_dev_fd; > @@ -897,6 +898,7 @@ ifcvf_dev_config(int vid) > DRV_LOG(NOTICE, "vDPA (%s): software relay is used.", > vdev->device->name); > > + internal->configured = 1; > return 0; > } > > @@ -935,6 +937,7 @@ ifcvf_dev_close(int vid) > update_datapath(internal); > } > > + internal->configured = 0; > return 0; > } > > @@ -1084,13 +1087,61 @@ ifcvf_get_protocol_features(struct > rte_vdpa_device *vdev, uint64_t *features) > return 0; > } > > +static int > +ifcvf_set_vring_state(int vid, int vring, int state) { > + struct rte_vdpa_device *vdev; > + struct internal_list *list; > + struct ifcvf_internal *internal; > + struct ifcvf_hw *hw; > + struct ifcvf_pci_common_cfg *cfg; > + int ret = 0; > + > + vdev = rte_vhost_get_vdpa_device(vid); > + list = find_internal_resource_by_vdev(vdev); > + if (list == NULL) { > + DRV_LOG(ERR, "Invalid vDPA device: %p", vdev); > + return -1; > + } > + > + internal = list->internal; > + if (vring < 0 || vring >= internal->max_queues * 2) { > + DRV_LOG(ERR, "Vring index %d not correct", vring); > + return -1; > + } > + > + hw = &internal->hw; > + if (!internal->configured) > + goto exit; > + > + cfg = hw->common_cfg; > + IFCVF_WRITE_REG16(vring, &cfg->queue_select); > + IFCVF_WRITE_REG16(state, &cfg->queue_enable); > + > + if (!state && hw->vring[vring].enable) { > + ret = vdpa_disable_vfio_intr(internal); > + if (ret) > + return ret; > + } > + > + if (state && !hw->vring[vring].enable) { > + ret = vdpa_enable_vfio_intr(internal, 0); > + if (ret) > + return ret; > + } > + > +exit: > + hw->vring[vring].enable = !!state; Redundant operation on variable "state". > + return 0; > +} > + > static struct rte_vdpa_dev_ops ifcvf_ops = { > .get_queue_num = ifcvf_get_queue_num, > .get_features = ifcvf_get_vdpa_features, > .get_protocol_features = ifcvf_get_protocol_features, > .dev_conf = ifcvf_dev_config, > .dev_close = ifcvf_dev_close, > - .set_vring_state = NULL, > + .set_vring_state = ifcvf_set_vring_state, > .set_features = ifcvf_set_features, > .migration_done = NULL, > .get_vfio_group_fd = ifcvf_get_vfio_group_fd, @@ -1170,6 +1221,7 > @@ ifcvf_pci_probe(struct rte_pci_driver *pci_drv __rte_unused, > goto error; > } > > + internal->configured = 0; > internal->max_queues = IFCVF_MAX_QUEUES; > features = ifcvf_get_features(&internal->hw); > internal->features = (features & > -- > 2.17.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH v1] vdpa/ifc: fix vring update after device config 2020-07-26 13:27 ` Xu, Rosen @ 2020-07-28 2:35 ` Xia, Chenbo 2020-07-28 3:07 ` Xu, Rosen 0 siblings, 1 reply; 9+ messages in thread From: Xia, Chenbo @ 2020-07-28 2:35 UTC (permalink / raw) To: Xu, Rosen, dev; +Cc: maxime.coquelin, Yigit, Ferruh, Wang, Xiao W Hi Rosen, > -----Original Message----- > From: Xu, Rosen <rosen.xu@intel.com> > Sent: Sunday, July 26, 2020 9:27 PM > To: Xia, Chenbo <chenbo.xia@intel.com>; dev@dpdk.org > Cc: maxime.coquelin@redhat.com; Yigit, Ferruh <ferruh.yigit@intel.com>; > Wang, Xiao W <xiao.w.wang@intel.com> > Subject: RE: [PATCH v1] vdpa/ifc: fix vring update after device config > > Hi, > > > -----Original Message----- > > From: Xia, Chenbo <chenbo.xia@intel.com> > > Sent: Saturday, July 25, 2020 1:09 > > To: dev@dpdk.org > > Cc: maxime.coquelin@redhat.com; Yigit, Ferruh > > <ferruh.yigit@intel.com>; Wang, Xiao W <xiao.w.wang@intel.com>; Xu, > > Rosen <rosen.xu@intel.com> > > Subject: [PATCH v1] vdpa/ifc: fix vring update after device config > > > > The device ready state in vhost lib is now defined as the state that > > first queue pair is ready. And kick/callfd may be updated by QEMU when > > ifc device is configured. > > > > Although now ifc driver only supports one queue pair, it still has to > > update callfd when working with QEMU. This patch fixes this vring > > update problem by implementing the set_vring_state callback. > > > > Fixes: a3f8150eac6d (net/ifcvf: add ifcvf vDPA driver) > > > > Suggested-by: Maxime Coquelin <maxime.coquelin@redhat.com> > > Signed-off-by: Chenbo Xia <chenbo.xia@intel.com> > > Acked-by: Wang Xiao W <xiao.w.wang@intel.com> > > --- > > drivers/vdpa/ifc/base/ifcvf.h | 1 + > > drivers/vdpa/ifc/ifcvf_vdpa.c | 54 > > ++++++++++++++++++++++++++++++++++- > > 2 files changed, 54 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/vdpa/ifc/base/ifcvf.h > > b/drivers/vdpa/ifc/base/ifcvf.h index eb04a9406..a288ce57d 100644 > > --- a/drivers/vdpa/ifc/base/ifcvf.h > > +++ b/drivers/vdpa/ifc/base/ifcvf.h > > @@ -115,6 +115,7 @@ struct vring_info { > > u16 size; > > u16 last_avail_idx; > > u16 last_used_idx; > > + bool enable; > > }; > > > > struct ifcvf_hw { > > diff --git a/drivers/vdpa/ifc/ifcvf_vdpa.c > > b/drivers/vdpa/ifc/ifcvf_vdpa.c index a757d45ec..9da25f0c7 100644 > > --- a/drivers/vdpa/ifc/ifcvf_vdpa.c > > +++ b/drivers/vdpa/ifc/ifcvf_vdpa.c > > @@ -49,6 +49,7 @@ static const char * const ifcvf_valid_arguments[] = > > { struct ifcvf_internal { > > struct rte_pci_device *pdev; > > struct ifcvf_hw hw; > > + int configured; > > int vfio_container_fd; > > int vfio_group_fd; > > int vfio_dev_fd; > > @@ -897,6 +898,7 @@ ifcvf_dev_config(int vid) > > DRV_LOG(NOTICE, "vDPA (%s): software relay is used.", > > vdev->device->name); > > > > + internal->configured = 1; > > return 0; > > } > > > > @@ -935,6 +937,7 @@ ifcvf_dev_close(int vid) > > update_datapath(internal); > > } > > > > + internal->configured = 0; > > return 0; > > } > > > > @@ -1084,13 +1087,61 @@ ifcvf_get_protocol_features(struct > > rte_vdpa_device *vdev, uint64_t *features) > > return 0; > > } > > > > +static int > > +ifcvf_set_vring_state(int vid, int vring, int state) { > > + struct rte_vdpa_device *vdev; > > + struct internal_list *list; > > + struct ifcvf_internal *internal; > > + struct ifcvf_hw *hw; > > + struct ifcvf_pci_common_cfg *cfg; > > + int ret = 0; > > + > > + vdev = rte_vhost_get_vdpa_device(vid); > > + list = find_internal_resource_by_vdev(vdev); > > + if (list == NULL) { > > + DRV_LOG(ERR, "Invalid vDPA device: %p", vdev); > > + return -1; > > + } > > + > > + internal = list->internal; > > + if (vring < 0 || vring >= internal->max_queues * 2) { > > + DRV_LOG(ERR, "Vring index %d not correct", vring); > > + return -1; > > + } > > + > > + hw = &internal->hw; > > + if (!internal->configured) > > + goto exit; > > + > > + cfg = hw->common_cfg; > > + IFCVF_WRITE_REG16(vring, &cfg->queue_select); > > + IFCVF_WRITE_REG16(state, &cfg->queue_enable); > > + > > + if (!state && hw->vring[vring].enable) { > > + ret = vdpa_disable_vfio_intr(internal); > > + if (ret) > > + return ret; > > + } > > + > > + if (state && !hw->vring[vring].enable) { > > + ret = vdpa_enable_vfio_intr(internal, 0); > > + if (ret) > > + return ret; > > + } > > + > > +exit: > > + hw->vring[vring].enable = !!state; > > Redundant operation on variable "state". The state should be 0/1 by default, but as the callback definition uses 'int', for other lib passing non-0/1 to the callback, we'd better to make sure it's 0/1 internally. What do you think? And thanks to your comments, I find that the register writing does not make sure it's a 0/1. Thanks! Chenbo > > > + return 0; > > +} > > + > > static struct rte_vdpa_dev_ops ifcvf_ops = { > > .get_queue_num = ifcvf_get_queue_num, > > .get_features = ifcvf_get_vdpa_features, > > .get_protocol_features = ifcvf_get_protocol_features, > > .dev_conf = ifcvf_dev_config, > > .dev_close = ifcvf_dev_close, > > - .set_vring_state = NULL, > > + .set_vring_state = ifcvf_set_vring_state, > > .set_features = ifcvf_set_features, > > .migration_done = NULL, > > .get_vfio_group_fd = ifcvf_get_vfio_group_fd, @@ -1170,6 +1221,7 > @@ > > ifcvf_pci_probe(struct rte_pci_driver *pci_drv __rte_unused, > > goto error; > > } > > > > + internal->configured = 0; > > internal->max_queues = IFCVF_MAX_QUEUES; > > features = ifcvf_get_features(&internal->hw); > > internal->features = (features & > > -- > > 2.17.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH v1] vdpa/ifc: fix vring update after device config 2020-07-28 2:35 ` Xia, Chenbo @ 2020-07-28 3:07 ` Xu, Rosen 0 siblings, 0 replies; 9+ messages in thread From: Xu, Rosen @ 2020-07-28 3:07 UTC (permalink / raw) To: Xia, Chenbo, dev; +Cc: maxime.coquelin, Yigit, Ferruh, Wang, Xiao W Hi Chenbo, > -----Original Message----- > From: Xia, Chenbo <chenbo.xia@intel.com> > Sent: Tuesday, July 28, 2020 10:35 > To: Xu, Rosen <rosen.xu@intel.com>; dev@dpdk.org > Cc: maxime.coquelin@redhat.com; Yigit, Ferruh <ferruh.yigit@intel.com>; > Wang, Xiao W <xiao.w.wang@intel.com> > Subject: RE: [PATCH v1] vdpa/ifc: fix vring update after device config > > Hi Rosen, > > > -----Original Message----- > > From: Xu, Rosen <rosen.xu@intel.com> > > Sent: Sunday, July 26, 2020 9:27 PM > > To: Xia, Chenbo <chenbo.xia@intel.com>; dev@dpdk.org > > Cc: maxime.coquelin@redhat.com; Yigit, Ferruh > > <ferruh.yigit@intel.com>; Wang, Xiao W <xiao.w.wang@intel.com> > > Subject: RE: [PATCH v1] vdpa/ifc: fix vring update after device config > > > > Hi, > > > > > -----Original Message----- > > > From: Xia, Chenbo <chenbo.xia@intel.com> > > > Sent: Saturday, July 25, 2020 1:09 > > > To: dev@dpdk.org > > > Cc: maxime.coquelin@redhat.com; Yigit, Ferruh > > > <ferruh.yigit@intel.com>; Wang, Xiao W <xiao.w.wang@intel.com>; Xu, > > > Rosen <rosen.xu@intel.com> > > > Subject: [PATCH v1] vdpa/ifc: fix vring update after device config > > > > > > The device ready state in vhost lib is now defined as the state that > > > first queue pair is ready. And kick/callfd may be updated by QEMU > > > when ifc device is configured. > > > > > > Although now ifc driver only supports one queue pair, it still has > > > to update callfd when working with QEMU. This patch fixes this vring > > > update problem by implementing the set_vring_state callback. > > > > > > Fixes: a3f8150eac6d (net/ifcvf: add ifcvf vDPA driver) > > > > > > Suggested-by: Maxime Coquelin <maxime.coquelin@redhat.com> > > > Signed-off-by: Chenbo Xia <chenbo.xia@intel.com> > > > Acked-by: Wang Xiao W <xiao.w.wang@intel.com> > > > --- > > > drivers/vdpa/ifc/base/ifcvf.h | 1 + drivers/vdpa/ifc/ifcvf_vdpa.c > > > | 54 > > > ++++++++++++++++++++++++++++++++++- > > > 2 files changed, 54 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/vdpa/ifc/base/ifcvf.h > > > b/drivers/vdpa/ifc/base/ifcvf.h index eb04a9406..a288ce57d 100644 > > > --- a/drivers/vdpa/ifc/base/ifcvf.h > > > +++ b/drivers/vdpa/ifc/base/ifcvf.h > > > @@ -115,6 +115,7 @@ struct vring_info { > > > u16 size; > > > u16 last_avail_idx; > > > u16 last_used_idx; > > > + bool enable; > > > }; > > > > > > struct ifcvf_hw { > > > diff --git a/drivers/vdpa/ifc/ifcvf_vdpa.c > > > b/drivers/vdpa/ifc/ifcvf_vdpa.c index a757d45ec..9da25f0c7 100644 > > > --- a/drivers/vdpa/ifc/ifcvf_vdpa.c > > > +++ b/drivers/vdpa/ifc/ifcvf_vdpa.c > > > @@ -49,6 +49,7 @@ static const char * const ifcvf_valid_arguments[] > > > = { struct ifcvf_internal { > > > struct rte_pci_device *pdev; > > > struct ifcvf_hw hw; > > > + int configured; > > > int vfio_container_fd; > > > int vfio_group_fd; > > > int vfio_dev_fd; > > > @@ -897,6 +898,7 @@ ifcvf_dev_config(int vid) > > > DRV_LOG(NOTICE, "vDPA (%s): software relay is used.", > > > vdev->device->name); > > > > > > + internal->configured = 1; > > > return 0; > > > } > > > > > > @@ -935,6 +937,7 @@ ifcvf_dev_close(int vid) > > > update_datapath(internal); > > > } > > > > > > + internal->configured = 0; > > > return 0; > > > } > > > > > > @@ -1084,13 +1087,61 @@ ifcvf_get_protocol_features(struct > > > rte_vdpa_device *vdev, uint64_t *features) > > > return 0; > > > } > > > > > > +static int > > > +ifcvf_set_vring_state(int vid, int vring, int state) { > > > + struct rte_vdpa_device *vdev; > > > + struct internal_list *list; > > > + struct ifcvf_internal *internal; > > > + struct ifcvf_hw *hw; > > > + struct ifcvf_pci_common_cfg *cfg; > > > + int ret = 0; > > > + > > > + vdev = rte_vhost_get_vdpa_device(vid); > > > + list = find_internal_resource_by_vdev(vdev); > > > + if (list == NULL) { > > > + DRV_LOG(ERR, "Invalid vDPA device: %p", vdev); > > > + return -1; > > > + } > > > + > > > + internal = list->internal; > > > + if (vring < 0 || vring >= internal->max_queues * 2) { > > > + DRV_LOG(ERR, "Vring index %d not correct", vring); > > > + return -1; > > > + } > > > + > > > + hw = &internal->hw; > > > + if (!internal->configured) > > > + goto exit; > > > + > > > + cfg = hw->common_cfg; > > > + IFCVF_WRITE_REG16(vring, &cfg->queue_select); > > > + IFCVF_WRITE_REG16(state, &cfg->queue_enable); > > > + > > > + if (!state && hw->vring[vring].enable) { > > > + ret = vdpa_disable_vfio_intr(internal); > > > + if (ret) > > > + return ret; > > > + } > > > + > > > + if (state && !hw->vring[vring].enable) { > > > + ret = vdpa_enable_vfio_intr(internal, 0); > > > + if (ret) > > > + return ret; > > > + } > > > + > > > +exit: > > > + hw->vring[vring].enable = !!state; > > > > Redundant operation on variable "state". > > The state should be 0/1 by default, but as the callback definition uses 'int', > for other lib passing non-0/1 to the callback, we'd better to make sure it's 0/1 > internally. What do you think? Got, it make sense. > And thanks to your comments, I find that the register writing does not make > sure it's a 0/1. > > Thanks! > Chenbo > > > > > > + return 0; > > > +} > > > + > > > static struct rte_vdpa_dev_ops ifcvf_ops = { > > > .get_queue_num = ifcvf_get_queue_num, > > > .get_features = ifcvf_get_vdpa_features, > > > .get_protocol_features = ifcvf_get_protocol_features, > > > .dev_conf = ifcvf_dev_config, > > > .dev_close = ifcvf_dev_close, > > > - .set_vring_state = NULL, > > > + .set_vring_state = ifcvf_set_vring_state, > > > .set_features = ifcvf_set_features, > > > .migration_done = NULL, > > > .get_vfio_group_fd = ifcvf_get_vfio_group_fd, @@ -1170,6 +1221,7 > > @@ > > > ifcvf_pci_probe(struct rte_pci_driver *pci_drv __rte_unused, > > > goto error; > > > } > > > > > > + internal->configured = 0; > > > internal->max_queues = IFCVF_MAX_QUEUES; > > > features = ifcvf_get_features(&internal->hw); > > > internal->features = (features & > > > -- > > > 2.17.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [dpdk-dev] [PATCH v2] vdpa/ifc: fix vring update after device config 2020-07-24 17:09 [dpdk-dev] [PATCH v1] vdpa/ifc: fix vring update after device config Chenbo Xia 2020-07-26 13:27 ` Xu, Rosen @ 2020-07-28 14:32 ` Chenbo Xia 2020-07-28 8:24 ` Wang, Xiao W ` (3 more replies) 1 sibling, 4 replies; 9+ messages in thread From: Chenbo Xia @ 2020-07-28 14:32 UTC (permalink / raw) To: dev; +Cc: maxime.coquelin, ferruh.yigit, xiao.w.wang, rosen.xu The device ready state in vhost lib is now defined as the state that first queue pair is ready. And kick/callfd may be updated by QEMU when ifc device is configured. Although now ifc driver only supports one queue pair, it still has to update callfd when working with QEMU. This patch fixes this vring update problem by implementing the set_vring_state callback. Fixes: a3f8150eac6d (net/ifcvf: add ifcvf vDPA driver) Suggested-by: Maxime Coquelin <maxime.coquelin@redhat.com> Signed-off-by: Chenbo Xia <chenbo.xia@intel.com> Acked-by: Wang Xiao W <xiao.w.wang@intel.com> --- drivers/vdpa/ifc/base/ifcvf.h | 1 + drivers/vdpa/ifc/ifcvf_vdpa.c | 54 ++++++++++++++++++++++++++++++++++- 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/drivers/vdpa/ifc/base/ifcvf.h b/drivers/vdpa/ifc/base/ifcvf.h index eb04a9406..a288ce57d 100644 --- a/drivers/vdpa/ifc/base/ifcvf.h +++ b/drivers/vdpa/ifc/base/ifcvf.h @@ -115,6 +115,7 @@ struct vring_info { u16 size; u16 last_avail_idx; u16 last_used_idx; + bool enable; }; struct ifcvf_hw { diff --git a/drivers/vdpa/ifc/ifcvf_vdpa.c b/drivers/vdpa/ifc/ifcvf_vdpa.c index a757d45ec..6a1b44bc7 100644 --- a/drivers/vdpa/ifc/ifcvf_vdpa.c +++ b/drivers/vdpa/ifc/ifcvf_vdpa.c @@ -49,6 +49,7 @@ static const char * const ifcvf_valid_arguments[] = { struct ifcvf_internal { struct rte_pci_device *pdev; struct ifcvf_hw hw; + int configured; int vfio_container_fd; int vfio_group_fd; int vfio_dev_fd; @@ -897,6 +898,7 @@ ifcvf_dev_config(int vid) DRV_LOG(NOTICE, "vDPA (%s): software relay is used.", vdev->device->name); + internal->configured = 1; return 0; } @@ -935,6 +937,7 @@ ifcvf_dev_close(int vid) update_datapath(internal); } + internal->configured = 0; return 0; } @@ -1084,13 +1087,61 @@ ifcvf_get_protocol_features(struct rte_vdpa_device *vdev, uint64_t *features) return 0; } +static int +ifcvf_set_vring_state(int vid, int vring, int state) +{ + struct rte_vdpa_device *vdev; + struct internal_list *list; + struct ifcvf_internal *internal; + struct ifcvf_hw *hw; + struct ifcvf_pci_common_cfg *cfg; + int ret = 0; + + vdev = rte_vhost_get_vdpa_device(vid); + list = find_internal_resource_by_vdev(vdev); + if (list == NULL) { + DRV_LOG(ERR, "Invalid vDPA device: %p", vdev); + return -1; + } + + internal = list->internal; + if (vring < 0 || vring >= internal->max_queues * 2) { + DRV_LOG(ERR, "Vring index %d not correct", vring); + return -1; + } + + hw = &internal->hw; + if (!internal->configured) + goto exit; + + cfg = hw->common_cfg; + IFCVF_WRITE_REG16(vring, &cfg->queue_select); + IFCVF_WRITE_REG16(!!state, &cfg->queue_enable); + + if (!state && hw->vring[vring].enable) { + ret = vdpa_disable_vfio_intr(internal); + if (ret) + return ret; + } + + if (state && !hw->vring[vring].enable) { + ret = vdpa_enable_vfio_intr(internal, 0); + if (ret) + return ret; + } + +exit: + hw->vring[vring].enable = !!state; + return 0; +} + static struct rte_vdpa_dev_ops ifcvf_ops = { .get_queue_num = ifcvf_get_queue_num, .get_features = ifcvf_get_vdpa_features, .get_protocol_features = ifcvf_get_protocol_features, .dev_conf = ifcvf_dev_config, .dev_close = ifcvf_dev_close, - .set_vring_state = NULL, + .set_vring_state = ifcvf_set_vring_state, .set_features = ifcvf_set_features, .migration_done = NULL, .get_vfio_group_fd = ifcvf_get_vfio_group_fd, @@ -1170,6 +1221,7 @@ ifcvf_pci_probe(struct rte_pci_driver *pci_drv __rte_unused, goto error; } + internal->configured = 0; internal->max_queues = IFCVF_MAX_QUEUES; features = ifcvf_get_features(&internal->hw); internal->features = (features & -- 2.17.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH v2] vdpa/ifc: fix vring update after device config 2020-07-28 14:32 ` [dpdk-dev] [PATCH v2] " Chenbo Xia @ 2020-07-28 8:24 ` Wang, Xiao W 2020-07-28 9:05 ` Xu, Rosen ` (2 subsequent siblings) 3 siblings, 0 replies; 9+ messages in thread From: Wang, Xiao W @ 2020-07-28 8:24 UTC (permalink / raw) To: Xia, Chenbo, dev; +Cc: maxime.coquelin, Yigit, Ferruh, Xu, Rosen Hi, > -----Original Message----- > From: Xia, Chenbo <chenbo.xia@intel.com> > Sent: Tuesday, July 28, 2020 10:32 PM > To: dev@dpdk.org > Cc: maxime.coquelin@redhat.com; Yigit, Ferruh <ferruh.yigit@intel.com>; > Wang, Xiao W <xiao.w.wang@intel.com>; Xu, Rosen <rosen.xu@intel.com> > Subject: [PATCH v2] vdpa/ifc: fix vring update after device config > > The device ready state in vhost lib is now defined as the state > that first queue pair is ready. And kick/callfd may be updated > by QEMU when ifc device is configured. > > Although now ifc driver only supports one queue pair, it still > has to update callfd when working with QEMU. This patch fixes > this vring update problem by implementing the set_vring_state > callback. > > Fixes: a3f8150eac6d (net/ifcvf: add ifcvf vDPA driver) > > Suggested-by: Maxime Coquelin <maxime.coquelin@redhat.com> > Signed-off-by: Chenbo Xia <chenbo.xia@intel.com> > Acked-by: Wang Xiao W <xiao.w.wang@intel.com> > --- > drivers/vdpa/ifc/base/ifcvf.h | 1 + > drivers/vdpa/ifc/ifcvf_vdpa.c | 54 ++++++++++++++++++++++++++++++++++- > 2 files changed, 54 insertions(+), 1 deletion(-) > > diff --git a/drivers/vdpa/ifc/base/ifcvf.h b/drivers/vdpa/ifc/base/ifcvf.h > index eb04a9406..a288ce57d 100644 > --- a/drivers/vdpa/ifc/base/ifcvf.h > +++ b/drivers/vdpa/ifc/base/ifcvf.h > @@ -115,6 +115,7 @@ struct vring_info { > u16 size; > u16 last_avail_idx; > u16 last_used_idx; > + bool enable; > }; > > struct ifcvf_hw { > diff --git a/drivers/vdpa/ifc/ifcvf_vdpa.c b/drivers/vdpa/ifc/ifcvf_vdpa.c > index a757d45ec..6a1b44bc7 100644 > --- a/drivers/vdpa/ifc/ifcvf_vdpa.c > +++ b/drivers/vdpa/ifc/ifcvf_vdpa.c > @@ -49,6 +49,7 @@ static const char * const ifcvf_valid_arguments[] = { > struct ifcvf_internal { > struct rte_pci_device *pdev; > struct ifcvf_hw hw; > + int configured; > int vfio_container_fd; > int vfio_group_fd; > int vfio_dev_fd; > @@ -897,6 +898,7 @@ ifcvf_dev_config(int vid) > DRV_LOG(NOTICE, "vDPA (%s): software relay is used.", > vdev->device->name); > > + internal->configured = 1; > return 0; > } > > @@ -935,6 +937,7 @@ ifcvf_dev_close(int vid) > update_datapath(internal); > } > > + internal->configured = 0; > return 0; > } > > @@ -1084,13 +1087,61 @@ ifcvf_get_protocol_features(struct > rte_vdpa_device *vdev, uint64_t *features) > return 0; > } > > +static int > +ifcvf_set_vring_state(int vid, int vring, int state) > +{ > + struct rte_vdpa_device *vdev; > + struct internal_list *list; > + struct ifcvf_internal *internal; > + struct ifcvf_hw *hw; > + struct ifcvf_pci_common_cfg *cfg; > + int ret = 0; > + > + vdev = rte_vhost_get_vdpa_device(vid); > + list = find_internal_resource_by_vdev(vdev); > + if (list == NULL) { > + DRV_LOG(ERR, "Invalid vDPA device: %p", vdev); > + return -1; > + } > + > + internal = list->internal; > + if (vring < 0 || vring >= internal->max_queues * 2) { > + DRV_LOG(ERR, "Vring index %d not correct", vring); > + return -1; > + } > + > + hw = &internal->hw; > + if (!internal->configured) > + goto exit; > + > + cfg = hw->common_cfg; > + IFCVF_WRITE_REG16(vring, &cfg->queue_select); > + IFCVF_WRITE_REG16(!!state, &cfg->queue_enable); > + > + if (!state && hw->vring[vring].enable) { > + ret = vdpa_disable_vfio_intr(internal); > + if (ret) > + return ret; > + } > + > + if (state && !hw->vring[vring].enable) { > + ret = vdpa_enable_vfio_intr(internal, 0); > + if (ret) > + return ret; > + } > + > +exit: > + hw->vring[vring].enable = !!state; > + return 0; > +} > + > static struct rte_vdpa_dev_ops ifcvf_ops = { > .get_queue_num = ifcvf_get_queue_num, > .get_features = ifcvf_get_vdpa_features, > .get_protocol_features = ifcvf_get_protocol_features, > .dev_conf = ifcvf_dev_config, > .dev_close = ifcvf_dev_close, > - .set_vring_state = NULL, > + .set_vring_state = ifcvf_set_vring_state, > .set_features = ifcvf_set_features, > .migration_done = NULL, > .get_vfio_group_fd = ifcvf_get_vfio_group_fd, > @@ -1170,6 +1221,7 @@ ifcvf_pci_probe(struct rte_pci_driver *pci_drv > __rte_unused, > goto error; > } > > + internal->configured = 0; > internal->max_queues = IFCVF_MAX_QUEUES; > features = ifcvf_get_features(&internal->hw); > internal->features = (features & > -- > 2.17.1 Acked-by: Xiao Wang <xiao.w.wang@intel.com> BRs, Xiao ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH v2] vdpa/ifc: fix vring update after device config 2020-07-28 14:32 ` [dpdk-dev] [PATCH v2] " Chenbo Xia 2020-07-28 8:24 ` Wang, Xiao W @ 2020-07-28 9:05 ` Xu, Rosen 2020-07-28 14:38 ` Maxime Coquelin 2020-07-28 15:56 ` Maxime Coquelin 3 siblings, 0 replies; 9+ messages in thread From: Xu, Rosen @ 2020-07-28 9:05 UTC (permalink / raw) To: Xia, Chenbo, dev; +Cc: maxime.coquelin, Yigit, Ferruh, Wang, Xiao W Hi, > -----Original Message----- > From: Xia, Chenbo <chenbo.xia@intel.com> > Sent: Tuesday, July 28, 2020 22:32 > To: dev@dpdk.org > Cc: maxime.coquelin@redhat.com; Yigit, Ferruh <ferruh.yigit@intel.com>; > Wang, Xiao W <xiao.w.wang@intel.com>; Xu, Rosen <rosen.xu@intel.com> > Subject: [PATCH v2] vdpa/ifc: fix vring update after device config > > The device ready state in vhost lib is now defined as the state that first queue > pair is ready. And kick/callfd may be updated by QEMU when ifc device is > configured. > > Although now ifc driver only supports one queue pair, it still has to update > callfd when working with QEMU. This patch fixes this vring update problem > by implementing the set_vring_state callback. > > Fixes: a3f8150eac6d (net/ifcvf: add ifcvf vDPA driver) > > Suggested-by: Maxime Coquelin <maxime.coquelin@redhat.com> > Signed-off-by: Chenbo Xia <chenbo.xia@intel.com> > Acked-by: Wang Xiao W <xiao.w.wang@intel.com> > --- > drivers/vdpa/ifc/base/ifcvf.h | 1 + > drivers/vdpa/ifc/ifcvf_vdpa.c | 54 ++++++++++++++++++++++++++++++++++- > 2 files changed, 54 insertions(+), 1 deletion(-) > > diff --git a/drivers/vdpa/ifc/base/ifcvf.h b/drivers/vdpa/ifc/base/ifcvf.h > index eb04a9406..a288ce57d 100644 > --- a/drivers/vdpa/ifc/base/ifcvf.h > +++ b/drivers/vdpa/ifc/base/ifcvf.h > @@ -115,6 +115,7 @@ struct vring_info { > u16 size; > u16 last_avail_idx; > u16 last_used_idx; > + bool enable; > }; > > struct ifcvf_hw { > diff --git a/drivers/vdpa/ifc/ifcvf_vdpa.c b/drivers/vdpa/ifc/ifcvf_vdpa.c > index a757d45ec..6a1b44bc7 100644 > --- a/drivers/vdpa/ifc/ifcvf_vdpa.c > +++ b/drivers/vdpa/ifc/ifcvf_vdpa.c > @@ -49,6 +49,7 @@ static const char * const ifcvf_valid_arguments[] = > { struct ifcvf_internal { > struct rte_pci_device *pdev; > struct ifcvf_hw hw; > + int configured; > int vfio_container_fd; > int vfio_group_fd; > int vfio_dev_fd; > @@ -897,6 +898,7 @@ ifcvf_dev_config(int vid) > DRV_LOG(NOTICE, "vDPA (%s): software relay is used.", > vdev->device->name); > > + internal->configured = 1; > return 0; > } > > @@ -935,6 +937,7 @@ ifcvf_dev_close(int vid) > update_datapath(internal); > } > > + internal->configured = 0; > return 0; > } > > @@ -1084,13 +1087,61 @@ ifcvf_get_protocol_features(struct > rte_vdpa_device *vdev, uint64_t *features) > return 0; > } > > +static int > +ifcvf_set_vring_state(int vid, int vring, int state) { > + struct rte_vdpa_device *vdev; > + struct internal_list *list; > + struct ifcvf_internal *internal; > + struct ifcvf_hw *hw; > + struct ifcvf_pci_common_cfg *cfg; > + int ret = 0; > + > + vdev = rte_vhost_get_vdpa_device(vid); > + list = find_internal_resource_by_vdev(vdev); > + if (list == NULL) { > + DRV_LOG(ERR, "Invalid vDPA device: %p", vdev); > + return -1; > + } > + > + internal = list->internal; > + if (vring < 0 || vring >= internal->max_queues * 2) { > + DRV_LOG(ERR, "Vring index %d not correct", vring); > + return -1; > + } > + > + hw = &internal->hw; > + if (!internal->configured) > + goto exit; > + > + cfg = hw->common_cfg; > + IFCVF_WRITE_REG16(vring, &cfg->queue_select); > + IFCVF_WRITE_REG16(!!state, &cfg->queue_enable); > + > + if (!state && hw->vring[vring].enable) { > + ret = vdpa_disable_vfio_intr(internal); > + if (ret) > + return ret; > + } > + > + if (state && !hw->vring[vring].enable) { > + ret = vdpa_enable_vfio_intr(internal, 0); > + if (ret) > + return ret; > + } > + > +exit: > + hw->vring[vring].enable = !!state; > + return 0; > +} > + > static struct rte_vdpa_dev_ops ifcvf_ops = { > .get_queue_num = ifcvf_get_queue_num, > .get_features = ifcvf_get_vdpa_features, > .get_protocol_features = ifcvf_get_protocol_features, > .dev_conf = ifcvf_dev_config, > .dev_close = ifcvf_dev_close, > - .set_vring_state = NULL, > + .set_vring_state = ifcvf_set_vring_state, > .set_features = ifcvf_set_features, > .migration_done = NULL, > .get_vfio_group_fd = ifcvf_get_vfio_group_fd, @@ -1170,6 +1221,7 > @@ ifcvf_pci_probe(struct rte_pci_driver *pci_drv __rte_unused, > goto error; > } > > + internal->configured = 0; > internal->max_queues = IFCVF_MAX_QUEUES; > features = ifcvf_get_features(&internal->hw); > internal->features = (features & > -- > 2.17.1 Acked-by: Rosen Xu <rosen.xu@intel.com> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH v2] vdpa/ifc: fix vring update after device config 2020-07-28 14:32 ` [dpdk-dev] [PATCH v2] " Chenbo Xia 2020-07-28 8:24 ` Wang, Xiao W 2020-07-28 9:05 ` Xu, Rosen @ 2020-07-28 14:38 ` Maxime Coquelin 2020-07-28 15:56 ` Maxime Coquelin 3 siblings, 0 replies; 9+ messages in thread From: Maxime Coquelin @ 2020-07-28 14:38 UTC (permalink / raw) To: Chenbo Xia, dev; +Cc: ferruh.yigit, xiao.w.wang, rosen.xu On 7/28/20 4:32 PM, Chenbo Xia wrote: > The device ready state in vhost lib is now defined as the state > that first queue pair is ready. And kick/callfd may be updated > by QEMU when ifc device is configured. > > Although now ifc driver only supports one queue pair, it still > has to update callfd when working with QEMU. This patch fixes > this vring update problem by implementing the set_vring_state > callback. > > Fixes: a3f8150eac6d (net/ifcvf: add ifcvf vDPA driver) > > Suggested-by: Maxime Coquelin <maxime.coquelin@redhat.com> > Signed-off-by: Chenbo Xia <chenbo.xia@intel.com> > Acked-by: Wang Xiao W <xiao.w.wang@intel.com> > --- > drivers/vdpa/ifc/base/ifcvf.h | 1 + > drivers/vdpa/ifc/ifcvf_vdpa.c | 54 ++++++++++++++++++++++++++++++++++- > 2 files changed, 54 insertions(+), 1 deletion(-) > Thanks Chenbo for doing the change! Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com> Thanks, Maxime ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH v2] vdpa/ifc: fix vring update after device config 2020-07-28 14:32 ` [dpdk-dev] [PATCH v2] " Chenbo Xia ` (2 preceding siblings ...) 2020-07-28 14:38 ` Maxime Coquelin @ 2020-07-28 15:56 ` Maxime Coquelin 3 siblings, 0 replies; 9+ messages in thread From: Maxime Coquelin @ 2020-07-28 15:56 UTC (permalink / raw) To: Chenbo Xia, dev; +Cc: ferruh.yigit, xiao.w.wang, rosen.xu On 7/28/20 4:32 PM, Chenbo Xia wrote: > The device ready state in vhost lib is now defined as the state > that first queue pair is ready. And kick/callfd may be updated > by QEMU when ifc device is configured. > > Although now ifc driver only supports one queue pair, it still > has to update callfd when working with QEMU. This patch fixes > this vring update problem by implementing the set_vring_state > callback. > > Fixes: a3f8150eac6d (net/ifcvf: add ifcvf vDPA driver) > > Suggested-by: Maxime Coquelin <maxime.coquelin@redhat.com> > Signed-off-by: Chenbo Xia <chenbo.xia@intel.com> > Acked-by: Wang Xiao W <xiao.w.wang@intel.com> > --- > drivers/vdpa/ifc/base/ifcvf.h | 1 + > drivers/vdpa/ifc/ifcvf_vdpa.c | 54 ++++++++++++++++++++++++++++++++++- > 2 files changed, 54 insertions(+), 1 deletion(-) > Applied to dpdk-next-virtio/master Thanks, Maxime ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-07-28 15:56 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-07-24 17:09 [dpdk-dev] [PATCH v1] vdpa/ifc: fix vring update after device config Chenbo Xia 2020-07-26 13:27 ` Xu, Rosen 2020-07-28 2:35 ` Xia, Chenbo 2020-07-28 3:07 ` Xu, Rosen 2020-07-28 14:32 ` [dpdk-dev] [PATCH v2] " Chenbo Xia 2020-07-28 8:24 ` Wang, Xiao W 2020-07-28 9:05 ` Xu, Rosen 2020-07-28 14:38 ` Maxime Coquelin 2020-07-28 15:56 ` Maxime Coquelin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).