Re: [PATCH V3 net-next 1/2] virtio-net: convert rx mode setting to use workqueue
On Wed, May 24, 2023 at 5:15 PM Michael S. Tsirkin wrote:
>
> On Wed, May 24, 2023 at 04:18:41PM +0800, Jason Wang wrote:
> > This patch convert rx mode setting to be done in a workqueue, this is
> > a must for allow to sleep when waiting for the cvq command to
> > response since current code is executed under addr spin lock.
> >
> > Signed-off-by: Jason Wang
> > ---
> > Changes since V1:
> > - use RTNL to synchronize rx mode worker
> > ---
> > drivers/net/virtio_net.c | 55 +---
> > 1 file changed, 52 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 56ca1d270304..5d2f1da4eaa0 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -265,6 +265,12 @@ struct virtnet_info {
> > /* Work struct for config space updates */
> > struct work_struct config_work;
> >
> > + /* Work struct for config rx mode */
>
> With a bit less abbreviation maybe? setting rx mode?
That's fine.
>
> > + struct work_struct rx_mode_work;
> > +
> > + /* Is rx mode work enabled? */
>
> Ugh not a great comment.
Any suggestions for this. E.g we had:
/* Is delayed refill enabled? */
>
> > + bool rx_mode_work_enabled;
> > +
>
>
>
> > /* Does the affinity hint is set for virtqueues? */
> > bool affinity_hint_set;
> >
> > @@ -388,6 +394,20 @@ static void disable_delayed_refill(struct virtnet_info
> > *vi)
> > spin_unlock_bh(>refill_lock);
> > }
> >
> > +static void enable_rx_mode_work(struct virtnet_info *vi)
> > +{
> > + rtnl_lock();
> > + vi->rx_mode_work_enabled = true;
> > + rtnl_unlock();
> > +}
> > +
> > +static void disable_rx_mode_work(struct virtnet_info *vi)
> > +{
> > + rtnl_lock();
> > + vi->rx_mode_work_enabled = false;
> > + rtnl_unlock();
> > +}
> > +
> > static void virtqueue_napi_schedule(struct napi_struct *napi,
> > struct virtqueue *vq)
> > {
> > @@ -2341,9 +2361,11 @@ static int virtnet_close(struct net_device *dev)
> > return 0;
> > }
> >
> > -static void virtnet_set_rx_mode(struct net_device *dev)
> > +static void virtnet_rx_mode_work(struct work_struct *work)
> > {
> > - struct virtnet_info *vi = netdev_priv(dev);
> > + struct virtnet_info *vi =
> > + container_of(work, struct virtnet_info, rx_mode_work);
> > + struct net_device *dev = vi->dev;
> > struct scatterlist sg[2];
> > struct virtio_net_ctrl_mac *mac_data;
> > struct netdev_hw_addr *ha;
> > @@ -2356,6 +2378,8 @@ static void virtnet_set_rx_mode(struct net_device
> > *dev)
> > if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_RX))
> > return;
> >
> > + rtnl_lock();
> > +
> > vi->ctrl->promisc = ((dev->flags & IFF_PROMISC) != 0);
> > vi->ctrl->allmulti = ((dev->flags & IFF_ALLMULTI) != 0);
> >
> > @@ -2373,14 +2397,19 @@ static void virtnet_set_rx_mode(struct net_device
> > *dev)
> > dev_warn(>dev, "Failed to %sable allmulti mode.\n",
> >vi->ctrl->allmulti ? "en" : "dis");
> >
> > + netif_addr_lock_bh(dev);
> > +
> > uc_count = netdev_uc_count(dev);
> > mc_count = netdev_mc_count(dev);
> > /* MAC filter - use one buffer for both lists */
> > buf = kzalloc(((uc_count + mc_count) * ETH_ALEN) +
> > (2 * sizeof(mac_data->entries)), GFP_ATOMIC);
> > mac_data = buf;
> > - if (!buf)
> > + if (!buf) {
> > + netif_addr_unlock_bh(dev);
> > + rtnl_unlock();
> > return;
> > + }
> >
> > sg_init_table(sg, 2);
> >
> > @@ -2401,6 +2430,8 @@ static void virtnet_set_rx_mode(struct net_device
> > *dev)
> > netdev_for_each_mc_addr(ha, dev)
> > memcpy(_data->macs[i++][0], ha->addr, ETH_ALEN);
> >
> > + netif_addr_unlock_bh(dev);
> > +
> > sg_set_buf([1], mac_data,
> > sizeof(mac_data->entries) + (mc_count * ETH_ALEN));
> >
> > @@ -2408,9 +2439,19 @@ static void virtnet_set_rx_mode(struct net_device
> > *dev)
> > VIRTIO_NET_CTRL_MAC_TABLE_SET, sg))
> > dev_warn(>dev, "Failed to set MAC filter table.\n");
> >
> > + rtnl_unlock();
> > +
> > kfree(buf);
> > }
> >
> > +static void virtnet_set_rx_mode(struct net_device *dev)
> > +{
> > + struct virtnet_info *vi = netdev_priv(dev);
> > +
> > + if (vi->rx_mode_work_enabled)
> > + schedule_work(>rx_mode_work);
> > +}
> > +
>
> > static int virtnet_vlan_rx_add_vid(struct net_device *dev,
> > __be16 proto, u16 vid)
> > {
> > @@ -3181,6 +3222,8 @@ static void virtnet_freeze_down(struct virtio_device
> > *vdev)
> >
> > /* Make sure no work handler is accessing the device */
> > flush_work(>config_work);
> > + disable_rx_mode_work(vi);
> > + flush_work(>rx_mode_work);
> >
> >
Re: [PATCH V2 4/5] vDPA/ifcvf: synchronize irqs in the reset routine
On Wed, May 24, 2023 at 4:03 PM Jason Wang wrote:
>
> On Mon, May 8, 2023 at 6:05 PM Zhu Lingshan wrote:
> >
> > This commit synchronize irqs of the virtqueues
> > and config space in the reset routine.
> > Thus ifcvf_stop_hw() and reset() are refactored as well.
> >
> > Signed-off-by: Zhu Lingshan
> > ---
> > drivers/vdpa/ifcvf/ifcvf_base.c | 41 +
> > drivers/vdpa/ifcvf/ifcvf_base.h | 1 +
> > drivers/vdpa/ifcvf/ifcvf_main.c | 46 +
> > 3 files changed, 38 insertions(+), 50 deletions(-)
> >
> > diff --git a/drivers/vdpa/ifcvf/ifcvf_base.c
> > b/drivers/vdpa/ifcvf/ifcvf_base.c
> > index 79e313c5e10e..1f39290baa38 100644
> > --- a/drivers/vdpa/ifcvf/ifcvf_base.c
> > +++ b/drivers/vdpa/ifcvf/ifcvf_base.c
> > @@ -170,12 +170,9 @@ void ifcvf_set_status(struct ifcvf_hw *hw, u8 status)
> >
> > void ifcvf_reset(struct ifcvf_hw *hw)
> > {
> > - hw->config_cb.callback = NULL;
> > - hw->config_cb.private = NULL;
> > -
> > ifcvf_set_status(hw, 0);
> > - /* flush set_status, make sure VF is stopped, reset */
> > - ifcvf_get_status(hw);
> > + while (ifcvf_get_status(hw))
> > + msleep(1);
> > }
> >
> > u64 ifcvf_get_hw_features(struct ifcvf_hw *hw)
> > @@ -368,20 +365,42 @@ void ifcvf_set_vq_ready(struct ifcvf_hw *hw, u16 qid,
> > bool ready)
> > vp_iowrite16(ready, >queue_enable);
> > }
> >
> > -static void ifcvf_hw_disable(struct ifcvf_hw *hw)
> > +static void ifcvf_reset_vring(struct ifcvf_hw *hw)
> > {
> > - u32 i;
> > + u16 qid;
> > +
> > + for (qid = 0; qid < hw->nr_vring; qid++) {
> > + hw->vring[qid].cb.callback = NULL;
> > + hw->vring[qid].cb.private = NULL;
> > + ifcvf_set_vq_vector(hw, qid, VIRTIO_MSI_NO_VECTOR);
> > + }
> > +}
> >
> > +static void ifcvf_reset_config_handler(struct ifcvf_hw *hw)
> > +{
> > + hw->config_cb.callback = NULL;
> > + hw->config_cb.private = NULL;
> > ifcvf_set_config_vector(hw, VIRTIO_MSI_NO_VECTOR);
> > - for (i = 0; i < hw->nr_vring; i++) {
> > - ifcvf_set_vq_vector(hw, i, VIRTIO_MSI_NO_VECTOR);
> > +}
> > +
> > +static void ifcvf_synchronize_irq(struct ifcvf_hw *hw)
> > +{
> > + u32 nvectors = hw->num_msix_vectors;
> > + struct pci_dev *pdev = hw->pdev;
> > + int i, irq;
> > +
> > + for (i = 0; i < nvectors; i++) {
> > + irq = pci_irq_vector(pdev, i);
> > + if (irq >= 0)
> > + synchronize_irq(irq);
> > }
> > }
> >
> > void ifcvf_stop_hw(struct ifcvf_hw *hw)
> > {
> > - ifcvf_hw_disable(hw);
> > - ifcvf_reset(hw);
> > + ifcvf_synchronize_irq(hw);
> > + ifcvf_reset_vring(hw);
> > + ifcvf_reset_config_handler(hw);
>
> Nit:
>
> So the name of this function is kind of misleading since irq
> synchronization and virtqueue/config handler are not belong to
> hardware?
>
> Maybe it would be better to call it ifcvf_stop().
I think we can tweak this on top. So
Acked-by: Jason Wang
Thanks
>
> Thanks
>
> > }
> >
> > void ifcvf_notify_queue(struct ifcvf_hw *hw, u16 qid)
> > diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h
> > b/drivers/vdpa/ifcvf/ifcvf_base.h
> > index d34d3bc0dbf4..7430f80779be 100644
> > --- a/drivers/vdpa/ifcvf/ifcvf_base.h
> > +++ b/drivers/vdpa/ifcvf/ifcvf_base.h
> > @@ -82,6 +82,7 @@ struct ifcvf_hw {
> > int vqs_reused_irq;
> > u16 nr_vring;
> > /* VIRTIO_PCI_CAP_DEVICE_CFG size */
> > + u32 num_msix_vectors;
> > u32 cap_dev_config_size;
> > struct pci_dev *pdev;
> > };
> > diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c
> > b/drivers/vdpa/ifcvf/ifcvf_main.c
> > index 968687159e44..3401b9901dd2 100644
> > --- a/drivers/vdpa/ifcvf/ifcvf_main.c
> > +++ b/drivers/vdpa/ifcvf/ifcvf_main.c
> > @@ -125,6 +125,7 @@ static void ifcvf_free_irq(struct ifcvf_hw *vf)
> > ifcvf_free_vq_irq(vf);
> > ifcvf_free_config_irq(vf);
> > ifcvf_free_irq_vectors(pdev);
> > + vf->num_msix_vectors = 0;
> > }
> >
> > /* ifcvf MSIX vectors allocator, this helper tries to allocate
> > @@ -343,36 +344,11 @@ static int ifcvf_request_irq(struct ifcvf_hw *vf)
> > if (ret)
> > return ret;
> >
> > - return 0;
> > -}
> > -
> > -static int ifcvf_stop_datapath(struct ifcvf_adapter *adapter)
> > -{
> > - struct ifcvf_hw *vf = adapter->vf;
> > - int i;
> > -
> > - for (i = 0; i < vf->nr_vring; i++)
> > - vf->vring[i].cb.callback = NULL;
> > -
> > - ifcvf_stop_hw(vf);
> > + vf->num_msix_vectors = nvectors;
> >
> > return 0;
> > }
> >
> > -static void ifcvf_reset_vring(struct ifcvf_adapter *adapter)
> > -{
> > - struct ifcvf_hw *vf = adapter->vf;
> > - int i;
> > -
> > - for (i = 0; i < vf->nr_vring; i++) {
> > - vf->vring[i].last_avail_idx = 0;
> > -
Re: [PATCH] tools/virtio: Add .gitignore to ringtest
On Wed, 24 May 2023 20:36:12 +0800, Rong Tao wrote: > From: Rong Tao > > Ignore executions for ringtest. > > Signed-off-by: Rong Tao Is this the resend mail? Or a new version? I replyed to your last mail. THanks. > --- > tools/virtio/ringtest/.gitignore | 7 +++ > 1 file changed, 7 insertions(+) > create mode 100644 tools/virtio/ringtest/.gitignore > > diff --git a/tools/virtio/ringtest/.gitignore > b/tools/virtio/ringtest/.gitignore > new file mode 100644 > index ..100b9e30c0f4 > --- /dev/null > +++ b/tools/virtio/ringtest/.gitignore > @@ -0,0 +1,7 @@ > +# SPDX-License-Identifier: GPL-2.0-only > +/noring > +/ptr_ring > +/ring > +/virtio_ring_0_9 > +/virtio_ring_inorder > +/virtio_ring_poll > -- > 2.39.1 > > ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 3/3] vhost-scsi: Rename vhost_scsi_iov_to_sgl
Rename vhost_scsi_iov_to_sgl to vhost_scsi_map_iov_to_sgl so it matches
matches the naming style used for vhost_scsi_copy_iov_to_sgl.
Signed-off-by: Mike Christie
---
drivers/vhost/scsi.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 382158b39740..a4d32b96e66a 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -694,8 +694,8 @@ vhost_scsi_calc_sgls(struct iov_iter *iter, size_t bytes,
int max_sgls)
}
static int
-vhost_scsi_iov_to_sgl(struct vhost_scsi_cmd *cmd, struct iov_iter *iter,
- struct scatterlist *sg, int sg_count)
+vhost_scsi_map_iov_to_sgl(struct vhost_scsi_cmd *cmd, struct iov_iter *iter,
+ struct scatterlist *sg, int sg_count)
{
struct scatterlist *p = sg;
int ret;
@@ -778,8 +778,9 @@ vhost_scsi_mapal(struct vhost_scsi_tpg *tpg, struct
vhost_scsi_cmd *cmd,
pr_debug("%s prot_sg %p prot_sgl_count %u\n", __func__,
cmd->tvc_prot_sgl, cmd->tvc_prot_sgl_count);
- ret = vhost_scsi_iov_to_sgl(cmd, prot_iter, cmd->tvc_prot_sgl,
- cmd->tvc_prot_sgl_count);
+ ret = vhost_scsi_map_iov_to_sgl(cmd, prot_iter,
+ cmd->tvc_prot_sgl,
+ cmd->tvc_prot_sgl_count);
if (ret < 0)
goto map_fail;
}
@@ -808,8 +809,8 @@ vhost_scsi_mapal(struct vhost_scsi_tpg *tpg, struct
vhost_scsi_cmd *cmd,
ret = vhost_scsi_copy_iov_to_sgl(cmd, data_iter, cmd->tvc_sgl,
cmd->tvc_sgl_count);
else
- ret = vhost_scsi_iov_to_sgl(cmd, data_iter, cmd->tvc_sgl,
- cmd->tvc_sgl_count);
+ ret = vhost_scsi_map_iov_to_sgl(cmd, data_iter, cmd->tvc_sgl,
+ cmd->tvc_sgl_count);
if (ret)
goto map_fail;
--
2.25.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 1/3] vhost-scsi: Fix alignment handling with windows
The linux block layer requires bios/requests to have lengths with a 512
byte alignment. Some drivers/layers like dm-crypt and the directi IO code
will test for it and just fail. Other drivers like SCSI just assume the
requirement is met and will end up in infinte retry loops. The problem
for drivers like SCSI is that it uses functions like blk_rq_cur_sectors
and blk_rq_sectors which divide the request's length by 512. If there's
lefovers then it just gets dropped. But other code in the block/scsi
layer may use blk_rq_bytes/blk_rq_cur_bytes and end up thinking there is
still data left and try to retry the cmd. We can then end up getting
stuck in retry loops where part of the block/scsi thinks there is data
left, but other parts think we want to do IOs of zero length.
Linux will always check for alignment, but windows will not. When
vhost-scsi then translates the iovec it gets from a windows guest to a
scatterlist, we can end up with sg items where the sg->length is not
divisible by 512 due to the misaligned offset:
sg[0].offset = 255;
sg[0].length = 3841;
sg...
sg[N].offset = 0;
sg[N].length = 255;
When the lio backends then convert the SG to bios or other iovecs, we
end up sending them with the same misaligned values and can hit the
issues above.
This just has us drop down to allocating a temp page and copying the data
when this happens. Because this adds a check that needs to loop over the
iovec in the main IO path, this patch adds an attribute that can be turned
on for just windows.
Signed-off-by: Mike Christie
---
drivers/vhost/scsi.c | 174 +--
1 file changed, 151 insertions(+), 23 deletions(-)
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index bb10fa4bb4f6..dbad8fb579eb 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -25,6 +25,8 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
@@ -75,6 +77,9 @@ struct vhost_scsi_cmd {
u32 tvc_prot_sgl_count;
/* Saved unpacked SCSI LUN for vhost_scsi_target_queue_cmd() */
u32 tvc_lun;
+ u32 copied_iov:1;
+ const void *saved_iter_addr;
+ struct iov_iter saved_iter;
/* Pointer to the SGL formatted memory from virtio-scsi */
struct scatterlist *tvc_sgl;
struct scatterlist *tvc_prot_sgl;
@@ -107,6 +112,7 @@ struct vhost_scsi_nexus {
struct vhost_scsi_tpg {
/* Vhost port target portal group tag for TCM */
u16 tport_tpgt;
+ bool check_io_alignment;
/* Used to track number of TPG Port/Lun Links wrt to explict I_T Nexus
shutdown */
int tv_tpg_port_count;
/* Used for vhost_scsi device reference to tpg_nexus, protected by
tv_tpg_mutex */
@@ -328,8 +334,13 @@ static void vhost_scsi_release_cmd_res(struct se_cmd
*se_cmd)
int i;
if (tv_cmd->tvc_sgl_count) {
- for (i = 0; i < tv_cmd->tvc_sgl_count; i++)
- put_page(sg_page(_cmd->tvc_sgl[i]));
+ for (i = 0; i < tv_cmd->tvc_sgl_count; i++) {
+ if (tv_cmd->copied_iov)
+ __free_page(sg_page(_cmd->tvc_sgl[i]));
+ else
+ put_page(sg_page(_cmd->tvc_sgl[i]));
+ }
+ kfree(tv_cmd->saved_iter_addr);
}
if (tv_cmd->tvc_prot_sgl_count) {
for (i = 0; i < tv_cmd->tvc_prot_sgl_count; i++)
@@ -502,6 +513,27 @@ static void vhost_scsi_evt_work(struct vhost_work *work)
mutex_unlock(>mutex);
}
+static int vhost_scsi_copy_sgl_to_iov(struct vhost_scsi_cmd *cmd)
+{
+ struct iov_iter *iter = >saved_iter;
+ struct scatterlist *sg = cmd->tvc_sgl;
+ struct page *page;
+ size_t len;
+ int i;
+
+ for (i = 0; i < cmd->tvc_sgl_count; i++) {
+ page = sg_page([i]);
+ len = sg[i].length;
+
+ if (copy_page_to_iter(page, 0, len, iter) != len) {
+ pr_err("Could not copy data. Error %lu\n", len);
+ return -1;
+ }
+ }
+
+ return 0;
+}
+
/* Fill in status and signal that we are done processing this command
*
* This is scheduled in the vhost work queue so we are called with the owner
@@ -525,15 +557,20 @@ static void vhost_scsi_complete_cmd_work(struct
vhost_work *work)
pr_debug("%s tv_cmd %p resid %u status %#02x\n", __func__,
cmd, se_cmd->residual_count, se_cmd->scsi_status);
-
memset(_rsp, 0, sizeof(v_rsp));
- v_rsp.resid = cpu_to_vhost32(cmd->tvc_vq,
se_cmd->residual_count);
- /* TODO is status_qualifier field needed? */
- v_rsp.status = se_cmd->scsi_status;
- v_rsp.sense_len = cpu_to_vhost32(cmd->tvc_vq,
-se_cmd->scsi_sense_length);
- memcpy(v_rsp.sense, cmd->tvc_sense_buf,
-
[PATCH 2/3] vhost-scsi: Remove unused write argument
The write arg that's passed to the mapping functions is not used so
remove it.
Signed-off-by: Mike Christie
---
drivers/vhost/scsi.c | 19 +++
1 file changed, 7 insertions(+), 12 deletions(-)
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index dbad8fb579eb..382158b39740 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -649,10 +649,8 @@ vhost_scsi_get_cmd(struct vhost_virtqueue *vq, struct
vhost_scsi_tpg *tpg,
* Returns the number of scatterlist entries used or -errno on error.
*/
static int
-vhost_scsi_map_to_sgl(struct vhost_scsi_cmd *cmd,
- struct iov_iter *iter,
- struct scatterlist *sgl,
- bool write)
+vhost_scsi_map_to_sgl(struct vhost_scsi_cmd *cmd, struct iov_iter *iter,
+ struct scatterlist *sgl)
{
struct page **pages = cmd->tvc_upages;
struct scatterlist *sg = sgl;
@@ -696,15 +694,14 @@ vhost_scsi_calc_sgls(struct iov_iter *iter, size_t bytes,
int max_sgls)
}
static int
-vhost_scsi_iov_to_sgl(struct vhost_scsi_cmd *cmd, bool write,
- struct iov_iter *iter,
+vhost_scsi_iov_to_sgl(struct vhost_scsi_cmd *cmd, struct iov_iter *iter,
struct scatterlist *sg, int sg_count)
{
struct scatterlist *p = sg;
int ret;
while (iov_iter_count(iter)) {
- ret = vhost_scsi_map_to_sgl(cmd, iter, sg, write);
+ ret = vhost_scsi_map_to_sgl(cmd, iter, sg);
if (ret < 0) {
while (p < sg) {
struct page *page = sg_page(p++);
@@ -769,7 +766,6 @@ vhost_scsi_mapal(struct vhost_scsi_tpg *tpg, struct
vhost_scsi_cmd *cmd,
size_t data_bytes, struct iov_iter *data_iter)
{
int sgl_count, ret;
- bool write = (cmd->tvc_data_direction == DMA_FROM_DEVICE);
if (prot_bytes) {
sgl_count = vhost_scsi_calc_sgls(prot_iter, prot_bytes,
@@ -782,8 +778,7 @@ vhost_scsi_mapal(struct vhost_scsi_tpg *tpg, struct
vhost_scsi_cmd *cmd,
pr_debug("%s prot_sg %p prot_sgl_count %u\n", __func__,
cmd->tvc_prot_sgl, cmd->tvc_prot_sgl_count);
- ret = vhost_scsi_iov_to_sgl(cmd, write, prot_iter,
- cmd->tvc_prot_sgl,
+ ret = vhost_scsi_iov_to_sgl(cmd, prot_iter, cmd->tvc_prot_sgl,
cmd->tvc_prot_sgl_count);
if (ret < 0)
goto map_fail;
@@ -813,8 +808,8 @@ vhost_scsi_mapal(struct vhost_scsi_tpg *tpg, struct
vhost_scsi_cmd *cmd,
ret = vhost_scsi_copy_iov_to_sgl(cmd, data_iter, cmd->tvc_sgl,
cmd->tvc_sgl_count);
else
- ret = vhost_scsi_iov_to_sgl(cmd, write, data_iter,
- cmd->tvc_sgl, cmd->tvc_sgl_count);
+ ret = vhost_scsi_iov_to_sgl(cmd, data_iter, cmd->tvc_sgl,
+ cmd->tvc_sgl_count);
if (ret)
goto map_fail;
--
2.25.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 0/3] vhost-scsi: Fix IO hangs when using windows
The following patches were made over Linus's tree and fix an issue where windows guests will send iovecs with offset/lengths that result in IOs that are not aligned to 512. The LIO layer will then send them to Linux's block layer but it requires 512 byte alignment, so depending on the block driver being used we will get IO errors or hung IO. The following patches have vhost-scsi detect when windows sends these IOs and copy them to a bounce buffer. It then does some cleanup in the related code. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 3/3] fork, vhost: Use CLONE_THREAD to fix freezer/ps regression
Oleg Nesterov writes: > On 05/23, Eric W. Biederman wrote: >> >> I want to point out that we need to consider not just SIGKILL, but >> SIGABRT that causes a coredump, as well as the process peforming >> an ordinary exit(2). All of which will cause get_signal to return >> SIGKILL in this context. > > Yes, but probably SIGABRT/exit doesn't really differ from SIGKILL wrt > vhost_worker(). Actually I think it reveals that exiting with SIGABRT will cause a deadlock. coredump_wait will wait for all of the threads to reach coredump_task_exit. Meanwhile vhost_worker is waiting for all of the other threads to reach exit_files to close their file descriptors. So it looks like the final pieces of work will actually need to be moved into to vhost_xxx_flush or vhost_xxx_release to avoid the exiting threads from waiting on each other, instead of depending upon the vhost_worker to do the work. Which gets back to most of your other questions. >> It is probably not the worst thing in the world, but what this means >> is now if you pass a copy of the vhost file descriptor to another >> process the vhost_worker will persist, and thus the process will persist >> until that copy of the file descriptor is closed. > > Hadn't thought about it. > > I am fighting with internal bugzillas today, will try to write another > email tomorrow. > > But before that, I would like to have an answer to my "main" question in > my previois email. Otherwise I am still not sure I understand what exactly > we need to fix. Let me repeat your "main" question just for clarity here. If a signal comes in after the signal_pending check but before the "work->fn(work)" call is "work->fn(work)" expected to run correctly with signal_pending() or fatal_signal_pending returning true? Eric ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 3/3] fork, vhost: Use CLONE_THREAD to fix freezer/ps regression
On 05/23, Eric W. Biederman wrote: > > I want to point out that we need to consider not just SIGKILL, but > SIGABRT that causes a coredump, as well as the process peforming > an ordinary exit(2). All of which will cause get_signal to return > SIGKILL in this context. Yes, but probably SIGABRT/exit doesn't really differ from SIGKILL wrt vhost_worker(). > It is probably not the worst thing in the world, but what this means > is now if you pass a copy of the vhost file descriptor to another > process the vhost_worker will persist, and thus the process will persist > until that copy of the file descriptor is closed. Hadn't thought about it. I am fighting with internal bugzillas today, will try to write another email tomorrow. But before that, I would like to have an answer to my "main" question in my previois email. Otherwise I am still not sure I understand what exactly we need to fix. Oleg. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH V3 net-next 1/2] virtio-net: convert rx mode setting to use workqueue
On Wed, May 24, 2023 at 04:18:41PM +0800, Jason Wang wrote:
> This patch convert rx mode setting to be done in a workqueue, this is
> a must for allow to sleep when waiting for the cvq command to
> response since current code is executed under addr spin lock.
>
> Signed-off-by: Jason Wang
> ---
> Changes since V1:
> - use RTNL to synchronize rx mode worker
> ---
> drivers/net/virtio_net.c | 55 +---
> 1 file changed, 52 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 56ca1d270304..5d2f1da4eaa0 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -265,6 +265,12 @@ struct virtnet_info {
> /* Work struct for config space updates */
> struct work_struct config_work;
>
> + /* Work struct for config rx mode */
With a bit less abbreviation maybe? setting rx mode?
> + struct work_struct rx_mode_work;
> +
> + /* Is rx mode work enabled? */
Ugh not a great comment.
> + bool rx_mode_work_enabled;
> +
> /* Does the affinity hint is set for virtqueues? */
> bool affinity_hint_set;
>
> @@ -388,6 +394,20 @@ static void disable_delayed_refill(struct virtnet_info
> *vi)
> spin_unlock_bh(>refill_lock);
> }
>
> +static void enable_rx_mode_work(struct virtnet_info *vi)
> +{
> + rtnl_lock();
> + vi->rx_mode_work_enabled = true;
> + rtnl_unlock();
> +}
> +
> +static void disable_rx_mode_work(struct virtnet_info *vi)
> +{
> + rtnl_lock();
> + vi->rx_mode_work_enabled = false;
> + rtnl_unlock();
> +}
> +
> static void virtqueue_napi_schedule(struct napi_struct *napi,
> struct virtqueue *vq)
> {
> @@ -2341,9 +2361,11 @@ static int virtnet_close(struct net_device *dev)
> return 0;
> }
>
> -static void virtnet_set_rx_mode(struct net_device *dev)
> +static void virtnet_rx_mode_work(struct work_struct *work)
> {
> - struct virtnet_info *vi = netdev_priv(dev);
> + struct virtnet_info *vi =
> + container_of(work, struct virtnet_info, rx_mode_work);
> + struct net_device *dev = vi->dev;
> struct scatterlist sg[2];
> struct virtio_net_ctrl_mac *mac_data;
> struct netdev_hw_addr *ha;
> @@ -2356,6 +2378,8 @@ static void virtnet_set_rx_mode(struct net_device *dev)
> if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_RX))
> return;
>
> + rtnl_lock();
> +
> vi->ctrl->promisc = ((dev->flags & IFF_PROMISC) != 0);
> vi->ctrl->allmulti = ((dev->flags & IFF_ALLMULTI) != 0);
>
> @@ -2373,14 +2397,19 @@ static void virtnet_set_rx_mode(struct net_device
> *dev)
> dev_warn(>dev, "Failed to %sable allmulti mode.\n",
>vi->ctrl->allmulti ? "en" : "dis");
>
> + netif_addr_lock_bh(dev);
> +
> uc_count = netdev_uc_count(dev);
> mc_count = netdev_mc_count(dev);
> /* MAC filter - use one buffer for both lists */
> buf = kzalloc(((uc_count + mc_count) * ETH_ALEN) +
> (2 * sizeof(mac_data->entries)), GFP_ATOMIC);
> mac_data = buf;
> - if (!buf)
> + if (!buf) {
> + netif_addr_unlock_bh(dev);
> + rtnl_unlock();
> return;
> + }
>
> sg_init_table(sg, 2);
>
> @@ -2401,6 +2430,8 @@ static void virtnet_set_rx_mode(struct net_device *dev)
> netdev_for_each_mc_addr(ha, dev)
> memcpy(_data->macs[i++][0], ha->addr, ETH_ALEN);
>
> + netif_addr_unlock_bh(dev);
> +
> sg_set_buf([1], mac_data,
> sizeof(mac_data->entries) + (mc_count * ETH_ALEN));
>
> @@ -2408,9 +2439,19 @@ static void virtnet_set_rx_mode(struct net_device *dev)
> VIRTIO_NET_CTRL_MAC_TABLE_SET, sg))
> dev_warn(>dev, "Failed to set MAC filter table.\n");
>
> + rtnl_unlock();
> +
> kfree(buf);
> }
>
> +static void virtnet_set_rx_mode(struct net_device *dev)
> +{
> + struct virtnet_info *vi = netdev_priv(dev);
> +
> + if (vi->rx_mode_work_enabled)
> + schedule_work(>rx_mode_work);
> +}
> +
> static int virtnet_vlan_rx_add_vid(struct net_device *dev,
> __be16 proto, u16 vid)
> {
> @@ -3181,6 +3222,8 @@ static void virtnet_freeze_down(struct virtio_device
> *vdev)
>
> /* Make sure no work handler is accessing the device */
> flush_work(>config_work);
> + disable_rx_mode_work(vi);
> + flush_work(>rx_mode_work);
>
> netif_tx_lock_bh(vi->dev);
> netif_device_detach(vi->dev);
Hmm so queued rx mode work will just get skipped
and on restore we get a wrong rx mode.
Any way to make this more robust?
> @@ -3203,6 +3246,7 @@ static int virtnet_restore_up(struct virtio_device
> *vdev)
> virtio_device_ready(vdev);
>
> enable_delayed_refill(vi);
> + enable_rx_mode_work(vi);
>
> if (netif_running(vi->dev)) {
>
[PATCH V3 net-next 2/2] virtio-net: add cond_resched() to the command waiting loop
Adding cond_resched() to the command waiting loop for a better
co-operation with the scheduler. This allows to give CPU a breath to
run other task(workqueue) instead of busy looping when preemption is
not allowed on a device whose CVQ might be slow.
Signed-off-by: Jason Wang
---
drivers/net/virtio_net.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 5d2f1da4eaa0..de498dbbf0d4 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2207,8 +2207,10 @@ static bool virtnet_send_command(struct virtnet_info
*vi, u8 class, u8 cmd,
* into the hypervisor, so the request should be handled immediately.
*/
while (!virtqueue_get_buf(vi->cvq, ) &&
- !virtqueue_is_broken(vi->cvq))
+ !virtqueue_is_broken(vi->cvq)) {
+ cond_resched();
cpu_relax();
+ }
return vi->ctrl->status == VIRTIO_NET_OK;
}
--
2.25.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH V3 net-next 1/2] virtio-net: convert rx mode setting to use workqueue
This patch convert rx mode setting to be done in a workqueue, this is
a must for allow to sleep when waiting for the cvq command to
response since current code is executed under addr spin lock.
Signed-off-by: Jason Wang
---
Changes since V1:
- use RTNL to synchronize rx mode worker
---
drivers/net/virtio_net.c | 55 +---
1 file changed, 52 insertions(+), 3 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 56ca1d270304..5d2f1da4eaa0 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -265,6 +265,12 @@ struct virtnet_info {
/* Work struct for config space updates */
struct work_struct config_work;
+ /* Work struct for config rx mode */
+ struct work_struct rx_mode_work;
+
+ /* Is rx mode work enabled? */
+ bool rx_mode_work_enabled;
+
/* Does the affinity hint is set for virtqueues? */
bool affinity_hint_set;
@@ -388,6 +394,20 @@ static void disable_delayed_refill(struct virtnet_info *vi)
spin_unlock_bh(>refill_lock);
}
+static void enable_rx_mode_work(struct virtnet_info *vi)
+{
+ rtnl_lock();
+ vi->rx_mode_work_enabled = true;
+ rtnl_unlock();
+}
+
+static void disable_rx_mode_work(struct virtnet_info *vi)
+{
+ rtnl_lock();
+ vi->rx_mode_work_enabled = false;
+ rtnl_unlock();
+}
+
static void virtqueue_napi_schedule(struct napi_struct *napi,
struct virtqueue *vq)
{
@@ -2341,9 +2361,11 @@ static int virtnet_close(struct net_device *dev)
return 0;
}
-static void virtnet_set_rx_mode(struct net_device *dev)
+static void virtnet_rx_mode_work(struct work_struct *work)
{
- struct virtnet_info *vi = netdev_priv(dev);
+ struct virtnet_info *vi =
+ container_of(work, struct virtnet_info, rx_mode_work);
+ struct net_device *dev = vi->dev;
struct scatterlist sg[2];
struct virtio_net_ctrl_mac *mac_data;
struct netdev_hw_addr *ha;
@@ -2356,6 +2378,8 @@ static void virtnet_set_rx_mode(struct net_device *dev)
if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_RX))
return;
+ rtnl_lock();
+
vi->ctrl->promisc = ((dev->flags & IFF_PROMISC) != 0);
vi->ctrl->allmulti = ((dev->flags & IFF_ALLMULTI) != 0);
@@ -2373,14 +2397,19 @@ static void virtnet_set_rx_mode(struct net_device *dev)
dev_warn(>dev, "Failed to %sable allmulti mode.\n",
vi->ctrl->allmulti ? "en" : "dis");
+ netif_addr_lock_bh(dev);
+
uc_count = netdev_uc_count(dev);
mc_count = netdev_mc_count(dev);
/* MAC filter - use one buffer for both lists */
buf = kzalloc(((uc_count + mc_count) * ETH_ALEN) +
(2 * sizeof(mac_data->entries)), GFP_ATOMIC);
mac_data = buf;
- if (!buf)
+ if (!buf) {
+ netif_addr_unlock_bh(dev);
+ rtnl_unlock();
return;
+ }
sg_init_table(sg, 2);
@@ -2401,6 +2430,8 @@ static void virtnet_set_rx_mode(struct net_device *dev)
netdev_for_each_mc_addr(ha, dev)
memcpy(_data->macs[i++][0], ha->addr, ETH_ALEN);
+ netif_addr_unlock_bh(dev);
+
sg_set_buf([1], mac_data,
sizeof(mac_data->entries) + (mc_count * ETH_ALEN));
@@ -2408,9 +2439,19 @@ static void virtnet_set_rx_mode(struct net_device *dev)
VIRTIO_NET_CTRL_MAC_TABLE_SET, sg))
dev_warn(>dev, "Failed to set MAC filter table.\n");
+ rtnl_unlock();
+
kfree(buf);
}
+static void virtnet_set_rx_mode(struct net_device *dev)
+{
+ struct virtnet_info *vi = netdev_priv(dev);
+
+ if (vi->rx_mode_work_enabled)
+ schedule_work(>rx_mode_work);
+}
+
static int virtnet_vlan_rx_add_vid(struct net_device *dev,
__be16 proto, u16 vid)
{
@@ -3181,6 +3222,8 @@ static void virtnet_freeze_down(struct virtio_device
*vdev)
/* Make sure no work handler is accessing the device */
flush_work(>config_work);
+ disable_rx_mode_work(vi);
+ flush_work(>rx_mode_work);
netif_tx_lock_bh(vi->dev);
netif_device_detach(vi->dev);
@@ -3203,6 +3246,7 @@ static int virtnet_restore_up(struct virtio_device *vdev)
virtio_device_ready(vdev);
enable_delayed_refill(vi);
+ enable_rx_mode_work(vi);
if (netif_running(vi->dev)) {
err = virtnet_open(vi->dev);
@@ -4002,6 +4046,7 @@ static int virtnet_probe(struct virtio_device *vdev)
vdev->priv = vi;
INIT_WORK(>config_work, virtnet_config_changed_work);
+ INIT_WORK(>rx_mode_work, virtnet_rx_mode_work);
spin_lock_init(>refill_lock);
if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF)) {
@@ -4110,6 +4155,8 @@ static int virtnet_probe(struct
[PATCH V3 net-next 0/2] virtio-net: don't busy poll for cvq command
Hi all: The code used to busy poll for cvq command which turns out to have several side effects: 1) infinite poll for buggy devices 2) bad interaction with scheduler So this series tries to use cond_resched() in the waiting loop. Before doing this we need first make sure the cvq command is not executed in atomic environment, so we need first convert rx mode handling to a workqueue. Please review. Thanks Changes since V2: - Don't use interrupt but cond_resched() Changes since V1: - use RTNL to synchronize rx mode worker - use completion for simplicity - don't try to harden CVQ command Changes since RFC: - switch to use BAD_RING in virtio_break_device() - check virtqueue_is_broken() after being woken up - use more_used() instead of virtqueue_get_buf() to allow caller to get buffers afterwards - break the virtio-net device when timeout - get buffer manually since the virtio core check more_used() instead Jason Wang (2): virtio-net: convert rx mode setting to use workqueue virtio-net: add cond_resched() to the command waiting loop drivers/net/virtio_net.c | 59 +--- 1 file changed, 55 insertions(+), 4 deletions(-) -- 2.25.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH V2 4/5] vDPA/ifcvf: synchronize irqs in the reset routine
On Mon, May 8, 2023 at 6:05 PM Zhu Lingshan wrote:
>
> This commit synchronize irqs of the virtqueues
> and config space in the reset routine.
> Thus ifcvf_stop_hw() and reset() are refactored as well.
>
> Signed-off-by: Zhu Lingshan
> ---
> drivers/vdpa/ifcvf/ifcvf_base.c | 41 +
> drivers/vdpa/ifcvf/ifcvf_base.h | 1 +
> drivers/vdpa/ifcvf/ifcvf_main.c | 46 +
> 3 files changed, 38 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.c b/drivers/vdpa/ifcvf/ifcvf_base.c
> index 79e313c5e10e..1f39290baa38 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_base.c
> +++ b/drivers/vdpa/ifcvf/ifcvf_base.c
> @@ -170,12 +170,9 @@ void ifcvf_set_status(struct ifcvf_hw *hw, u8 status)
>
> void ifcvf_reset(struct ifcvf_hw *hw)
> {
> - hw->config_cb.callback = NULL;
> - hw->config_cb.private = NULL;
> -
> ifcvf_set_status(hw, 0);
> - /* flush set_status, make sure VF is stopped, reset */
> - ifcvf_get_status(hw);
> + while (ifcvf_get_status(hw))
> + msleep(1);
> }
>
> u64 ifcvf_get_hw_features(struct ifcvf_hw *hw)
> @@ -368,20 +365,42 @@ void ifcvf_set_vq_ready(struct ifcvf_hw *hw, u16 qid,
> bool ready)
> vp_iowrite16(ready, >queue_enable);
> }
>
> -static void ifcvf_hw_disable(struct ifcvf_hw *hw)
> +static void ifcvf_reset_vring(struct ifcvf_hw *hw)
> {
> - u32 i;
> + u16 qid;
> +
> + for (qid = 0; qid < hw->nr_vring; qid++) {
> + hw->vring[qid].cb.callback = NULL;
> + hw->vring[qid].cb.private = NULL;
> + ifcvf_set_vq_vector(hw, qid, VIRTIO_MSI_NO_VECTOR);
> + }
> +}
>
> +static void ifcvf_reset_config_handler(struct ifcvf_hw *hw)
> +{
> + hw->config_cb.callback = NULL;
> + hw->config_cb.private = NULL;
> ifcvf_set_config_vector(hw, VIRTIO_MSI_NO_VECTOR);
> - for (i = 0; i < hw->nr_vring; i++) {
> - ifcvf_set_vq_vector(hw, i, VIRTIO_MSI_NO_VECTOR);
> +}
> +
> +static void ifcvf_synchronize_irq(struct ifcvf_hw *hw)
> +{
> + u32 nvectors = hw->num_msix_vectors;
> + struct pci_dev *pdev = hw->pdev;
> + int i, irq;
> +
> + for (i = 0; i < nvectors; i++) {
> + irq = pci_irq_vector(pdev, i);
> + if (irq >= 0)
> + synchronize_irq(irq);
> }
> }
>
> void ifcvf_stop_hw(struct ifcvf_hw *hw)
> {
> - ifcvf_hw_disable(hw);
> - ifcvf_reset(hw);
> + ifcvf_synchronize_irq(hw);
> + ifcvf_reset_vring(hw);
> + ifcvf_reset_config_handler(hw);
Nit:
So the name of this function is kind of misleading since irq
synchronization and virtqueue/config handler are not belong to
hardware?
Maybe it would be better to call it ifcvf_stop().
Thanks
> }
>
> void ifcvf_notify_queue(struct ifcvf_hw *hw, u16 qid)
> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h
> index d34d3bc0dbf4..7430f80779be 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_base.h
> +++ b/drivers/vdpa/ifcvf/ifcvf_base.h
> @@ -82,6 +82,7 @@ struct ifcvf_hw {
> int vqs_reused_irq;
> u16 nr_vring;
> /* VIRTIO_PCI_CAP_DEVICE_CFG size */
> + u32 num_msix_vectors;
> u32 cap_dev_config_size;
> struct pci_dev *pdev;
> };
> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
> index 968687159e44..3401b9901dd2 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_main.c
> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c
> @@ -125,6 +125,7 @@ static void ifcvf_free_irq(struct ifcvf_hw *vf)
> ifcvf_free_vq_irq(vf);
> ifcvf_free_config_irq(vf);
> ifcvf_free_irq_vectors(pdev);
> + vf->num_msix_vectors = 0;
> }
>
> /* ifcvf MSIX vectors allocator, this helper tries to allocate
> @@ -343,36 +344,11 @@ static int ifcvf_request_irq(struct ifcvf_hw *vf)
> if (ret)
> return ret;
>
> - return 0;
> -}
> -
> -static int ifcvf_stop_datapath(struct ifcvf_adapter *adapter)
> -{
> - struct ifcvf_hw *vf = adapter->vf;
> - int i;
> -
> - for (i = 0; i < vf->nr_vring; i++)
> - vf->vring[i].cb.callback = NULL;
> -
> - ifcvf_stop_hw(vf);
> + vf->num_msix_vectors = nvectors;
>
> return 0;
> }
>
> -static void ifcvf_reset_vring(struct ifcvf_adapter *adapter)
> -{
> - struct ifcvf_hw *vf = adapter->vf;
> - int i;
> -
> - for (i = 0; i < vf->nr_vring; i++) {
> - vf->vring[i].last_avail_idx = 0;
> - vf->vring[i].cb.callback = NULL;
> - vf->vring[i].cb.private = NULL;
> - }
> -
> - ifcvf_reset(vf);
> -}
> -
> static struct ifcvf_adapter *vdpa_to_adapter(struct vdpa_device *vdpa_dev)
> {
> return container_of(vdpa_dev, struct ifcvf_adapter, vdpa);
> @@ -462,23 +438,15 @@ static void ifcvf_vdpa_set_status(struct vdpa_device
> *vdpa_dev, u8 status)
>
> static int ifcvf_vdpa_reset(struct
Re: [PATCH V2 2/5] vDPA/ifcvf: get_driver_features from virtio registers
On Mon, May 8, 2023 at 6:05 PM Zhu Lingshan wrote:
>
> This commit implements a new function ifcvf_get_driver_feature()
> which read driver_features from virtio registers.
>
> To be less ambiguous, ifcvf_set_features() is renamed to
> ifcvf_set_driver_features(), and ifcvf_get_features()
> is renamed to ifcvf_get_dev_features() which returns
> the provisioned vDPA device features.
>
> Signed-off-by: Zhu Lingshan
Acked-by: Jason Wang
Thanks
> ---
> drivers/vdpa/ifcvf/ifcvf_base.c | 38 +
> drivers/vdpa/ifcvf/ifcvf_base.h | 5 +++--
> drivers/vdpa/ifcvf/ifcvf_main.c | 9 +---
> 3 files changed, 29 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.c b/drivers/vdpa/ifcvf/ifcvf_base.c
> index 6c5650f73007..546e923bcd16 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_base.c
> +++ b/drivers/vdpa/ifcvf/ifcvf_base.c
> @@ -204,11 +204,29 @@ u64 ifcvf_get_hw_features(struct ifcvf_hw *hw)
> return features;
> }
>
> -u64 ifcvf_get_features(struct ifcvf_hw *hw)
> +/* return provisioned vDPA dev features */
> +u64 ifcvf_get_dev_features(struct ifcvf_hw *hw)
> {
> return hw->dev_features;
> }
>
> +u64 ifcvf_get_driver_features(struct ifcvf_hw *hw)
> +{
> + struct virtio_pci_common_cfg __iomem *cfg = hw->common_cfg;
> + u32 features_lo, features_hi;
> + u64 features;
> +
> + vp_iowrite32(0, >device_feature_select);
> + features_lo = vp_ioread32(>guest_feature);
> +
> + vp_iowrite32(1, >device_feature_select);
> + features_hi = vp_ioread32(>guest_feature);
> +
> + features = ((u64)features_hi << 32) | features_lo;
> +
> + return features;
> +}
> +
> int ifcvf_verify_min_features(struct ifcvf_hw *hw, u64 features)
> {
> if (!(features & BIT_ULL(VIRTIO_F_ACCESS_PLATFORM)) && features) {
> @@ -275,7 +293,7 @@ void ifcvf_write_dev_config(struct ifcvf_hw *hw, u64
> offset,
> vp_iowrite8(*p++, hw->dev_cfg + offset + i);
> }
>
> -static void ifcvf_set_features(struct ifcvf_hw *hw, u64 features)
> +void ifcvf_set_driver_features(struct ifcvf_hw *hw, u64 features)
> {
> struct virtio_pci_common_cfg __iomem *cfg = hw->common_cfg;
>
> @@ -286,19 +304,6 @@ static void ifcvf_set_features(struct ifcvf_hw *hw, u64
> features)
> vp_iowrite32(features >> 32, >guest_feature);
> }
>
> -static int ifcvf_config_features(struct ifcvf_hw *hw)
> -{
> - ifcvf_set_features(hw, hw->req_features);
> - ifcvf_add_status(hw, VIRTIO_CONFIG_S_FEATURES_OK);
> -
> - if (!(ifcvf_get_status(hw) & VIRTIO_CONFIG_S_FEATURES_OK)) {
> - IFCVF_ERR(hw->pdev, "Failed to set FEATURES_OK status\n");
> - return -EIO;
> - }
> -
> - return 0;
> -}
> -
> u16 ifcvf_get_vq_state(struct ifcvf_hw *hw, u16 qid)
> {
> struct ifcvf_lm_cfg __iomem *ifcvf_lm;
> @@ -387,9 +392,6 @@ int ifcvf_start_hw(struct ifcvf_hw *hw)
> ifcvf_add_status(hw, VIRTIO_CONFIG_S_ACKNOWLEDGE);
> ifcvf_add_status(hw, VIRTIO_CONFIG_S_DRIVER);
>
> - if (ifcvf_config_features(hw) < 0)
> - return -EINVAL;
> -
> ifcvf_add_status(hw, VIRTIO_CONFIG_S_DRIVER_OK);
>
> return 0;
> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h
> index d545a9411143..cb19196c3ece 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_base.h
> +++ b/drivers/vdpa/ifcvf/ifcvf_base.h
> @@ -69,7 +69,6 @@ struct ifcvf_hw {
> phys_addr_t notify_base_pa;
> u32 notify_off_multiplier;
> u32 dev_type;
> - u64 req_features;
> u64 hw_features;
> /* provisioned device features */
> u64 dev_features;
> @@ -122,7 +121,7 @@ u8 ifcvf_get_status(struct ifcvf_hw *hw);
> void ifcvf_set_status(struct ifcvf_hw *hw, u8 status);
> void io_write64_twopart(u64 val, u32 *lo, u32 *hi);
> void ifcvf_reset(struct ifcvf_hw *hw);
> -u64 ifcvf_get_features(struct ifcvf_hw *hw);
> +u64 ifcvf_get_dev_features(struct ifcvf_hw *hw);
> u64 ifcvf_get_hw_features(struct ifcvf_hw *hw);
> int ifcvf_verify_min_features(struct ifcvf_hw *hw, u64 features);
> u16 ifcvf_get_vq_state(struct ifcvf_hw *hw, u16 qid);
> @@ -137,4 +136,6 @@ int ifcvf_set_vq_address(struct ifcvf_hw *hw, u16 qid,
> u64 desc_area,
> u64 driver_area, u64 device_area);
> bool ifcvf_get_vq_ready(struct ifcvf_hw *hw, u16 qid);
> void ifcvf_set_vq_ready(struct ifcvf_hw *hw, u16 qid, bool ready);
> +void ifcvf_set_driver_features(struct ifcvf_hw *hw, u64 features);
> +u64 ifcvf_get_driver_features(struct ifcvf_hw *hw);
> #endif /* _IFCVF_H_ */
> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
> index 1357c67014ab..4588484bd53d 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_main.c
> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c
> @@ -410,7 +410,7 @@ static u64 ifcvf_vdpa_get_device_features(struct
> vdpa_device *vdpa_dev)
> u64 features;
>
> if (type == VIRTIO_ID_NET || type
