Hi,
Best Regards, Xiao > -----Original Message----- > From: Jason Wang <jasow...@redhat.com> > Sent: Friday, May 15, 2020 5:09 PM > To: Maxime Coquelin <maxime.coque...@redhat.com>; Ye, Xiaolong > <xiaolong...@intel.com>; shah...@mellanox.com; ma...@mellanox.com; > amore...@redhat.com; Wang, Xiao W <xiao.w.w...@intel.com>; > viachesl...@mellanox.com; dev@dpdk.org > Cc: l...@redhat.com > Subject: Re: [PATCH 3/9] vdpa/ifc: add support to vDPA queue enable > > > On 2020/5/14 下午4:02, Maxime Coquelin wrote: > > This patch adds support to enabling and disabling > > vrings on a per-vring granularity. > > > > Signed-off-by: Maxime Coquelin > > <maxime.coque...@redhat.com<mailto:maxime.coque...@redhat.com>> > > > A question here, I see in qemu peer_attach() may try to generate > VHOST_USER_SET_VRING_ENABLE, but just from the name I think it should > behave as queue_enable defined in virtio specification which is > explicitly under the control of guest? > > (Note, in Cindy's vDPA series, we must invent new vhost_ops to differ > from this one). From my view, common_cfg.enable reg is used for registering a queue to hypervisor&vhost, but not ENABLE. The control queue message VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET is for enable/disable queue pairs. Think about when virtio net probes, all queues are selected and "enabled" by init_vqs(), but MQ is not enabled until virtnet_set_channels() by user config with "ethtool". Based on this, below reg writing is not OK to enable MQ. IFC HW supports below registers for VF pass-thru case. Actually, we have specific reg designed to enable MQ in VDPA case. > > + IFCVF_WRITE_REG16(qid, &cfg->queue_select); > > + IFCVF_WRITE_REG16(enable, &cfg->queue_enable); BRs, Xiao > > Thanks > > > > --- > > drivers/vdpa/ifc/base/ifcvf.c | 9 +++++++++ > > drivers/vdpa/ifc/base/ifcvf.h | 4 ++++ > > drivers/vdpa/ifc/ifcvf_vdpa.c | 23 ++++++++++++++++++++++- > > 3 files changed, 35 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/vdpa/ifc/base/ifcvf.c b/drivers/vdpa/ifc/base/ifcvf.c > > index 3c0b2dff66..dd4e7468ae 100644 > > --- a/drivers/vdpa/ifc/base/ifcvf.c > > +++ b/drivers/vdpa/ifc/base/ifcvf.c > > @@ -327,3 +327,12 @@ ifcvf_get_queue_notify_off(struct ifcvf_hw *hw, int > qid) > > return (u8 *)hw->notify_addr[qid] - > > (u8 *)hw->mem_resource[hw->notify_region].addr; > > } > > + > > +void > > +ifcvf_queue_enable(struct ifcvf_hw *hw, u16 qid, u16 enable) > > +{ > > + struct ifcvf_pci_common_cfg *cfg = hw->common_cfg; > > + > > + IFCVF_WRITE_REG16(qid, &cfg->queue_select); > > + IFCVF_WRITE_REG16(enable, &cfg->queue_enable); > > +} > > diff --git a/drivers/vdpa/ifc/base/ifcvf.h b/drivers/vdpa/ifc/base/ifcvf.h > > index eb04a94067..bd85010eff 100644 > > --- a/drivers/vdpa/ifc/base/ifcvf.h > > +++ b/drivers/vdpa/ifc/base/ifcvf.h > > @@ -159,4 +159,8 @@ ifcvf_get_notify_region(struct ifcvf_hw *hw); > > u64 > > ifcvf_get_queue_notify_off(struct ifcvf_hw *hw, int qid); > > > > +void > > +ifcvf_queue_enable(struct ifcvf_hw *hw, u16 qid, u16 enable); > > + > > + > > #endif /* _IFCVF_H_ */ > > diff --git a/drivers/vdpa/ifc/ifcvf_vdpa.c b/drivers/vdpa/ifc/ifcvf_vdpa.c > > index ec97178dcb..55ce0cf13d 100644 > > --- a/drivers/vdpa/ifc/ifcvf_vdpa.c > > +++ b/drivers/vdpa/ifc/ifcvf_vdpa.c > > @@ -937,6 +937,27 @@ ifcvf_dev_close(int vid) > > return 0; > > } > > > > +static int > > +ifcvf_set_vring_state(int vid, int vring, int state) > > +{ > > + int did; > > + struct internal_list *list; > > + struct ifcvf_internal *internal; > > + > > + did = rte_vhost_get_vdpa_device_id(vid); > > + list = find_internal_resource_by_did(did); > > + if (list == NULL) { > > + DRV_LOG(ERR, "Invalid device id: %d", did); > > + return -1; > > + } > > + > > + internal = list->internal; > > + > > + ifcvf_queue_enable(&internal->hw, (uint16_t)vring, (uint16_t) state); > > + > > + return 0; > > +} > > + > > static int > > ifcvf_set_features(int vid) > > { > > @@ -1086,7 +1107,7 @@ static struct rte_vdpa_dev_ops ifcvf_ops = { > > .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,
