Re: [PATCH 3/5] retire ifcvf_start_datapath and ifcvf_add_status

From: Jason Wang <jasowang@redhat.com>
To: Zhu Lingshan <lingshan.zhu@intel.com>, mst@redhat.com
Cc: virtualization@lists.linux-foundation.org
Subject: Re: [PATCH 3/5] retire ifcvf_start_datapath and ifcvf_add_status
Date: Wed, 26 Apr 2023 12:04:56 +0800	[thread overview]
Message-ID: <acc427b5-6f22-9792-b3d8-cc8fb10fe30e@redhat.com> (raw)
In-Reply-To: <20230331204854.20082-4-lingshan.zhu@intel.com>


在 2023/4/1 04:48, Zhu Lingshan 写道:
> Rather than former lazy-initialization mechanism,
> now the virtqueue operations and driver_features related
> ops access the virtio registers directly to take
> immediate actions. So ifcvf_start_datapath() should
> retire.
>
> ifcvf_add_status() is retierd because we should not change
> device status by a vendor driver's decision, this driver should
> only set device status which is from virito drivers
> upon vdpa_ops.set_status()
>
> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>


Acked-by: Jason Wang <jasowang@redhat.com>

Thanks


> ---
>   drivers/vdpa/ifcvf/ifcvf_base.c | 19 -------------------
>   drivers/vdpa/ifcvf/ifcvf_base.h |  1 -
>   drivers/vdpa/ifcvf/ifcvf_main.c | 23 -----------------------
>   3 files changed, 43 deletions(-)
>
> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.c b/drivers/vdpa/ifcvf/ifcvf_base.c
> index 546e923bcd16..79e313c5e10e 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_base.c
> +++ b/drivers/vdpa/ifcvf/ifcvf_base.c
> @@ -178,15 +178,6 @@ void ifcvf_reset(struct ifcvf_hw *hw)
>   	ifcvf_get_status(hw);
>   }
>   
> -static void ifcvf_add_status(struct ifcvf_hw *hw, u8 status)
> -{
> -	if (status != 0)
> -		status |= ifcvf_get_status(hw);
> -
> -	ifcvf_set_status(hw, status);
> -	ifcvf_get_status(hw);
> -}
> -
>   u64 ifcvf_get_hw_features(struct ifcvf_hw *hw)
>   {
>   	struct virtio_pci_common_cfg __iomem *cfg = hw->common_cfg;
> @@ -387,16 +378,6 @@ static void ifcvf_hw_disable(struct ifcvf_hw *hw)
>   	}
>   }
>   
> -int ifcvf_start_hw(struct ifcvf_hw *hw)
> -{
> -	ifcvf_add_status(hw, VIRTIO_CONFIG_S_ACKNOWLEDGE);
> -	ifcvf_add_status(hw, VIRTIO_CONFIG_S_DRIVER);
> -
> -	ifcvf_add_status(hw, VIRTIO_CONFIG_S_DRIVER_OK);
> -
> -	return 0;
> -}
> -
>   void ifcvf_stop_hw(struct ifcvf_hw *hw)
>   {
>   	ifcvf_hw_disable(hw);
> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h
> index cb19196c3ece..d34d3bc0dbf4 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_base.h
> +++ b/drivers/vdpa/ifcvf/ifcvf_base.h
> @@ -110,7 +110,6 @@ struct ifcvf_vdpa_mgmt_dev {
>   };
>   
>   int ifcvf_init_hw(struct ifcvf_hw *hw, struct pci_dev *dev);
> -int ifcvf_start_hw(struct ifcvf_hw *hw);
>   void ifcvf_stop_hw(struct ifcvf_hw *hw);
>   void ifcvf_notify_queue(struct ifcvf_hw *hw, u16 qid);
>   void ifcvf_read_dev_config(struct ifcvf_hw *hw, u64 offset,
> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
> index 4588484bd53d..968687159e44 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_main.c
> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c
> @@ -346,22 +346,6 @@ static int ifcvf_request_irq(struct ifcvf_hw *vf)
>   	return 0;
>   }
>   
> -static int ifcvf_start_datapath(struct ifcvf_adapter *adapter)
> -{
> -	struct ifcvf_hw *vf = adapter->vf;
> -	u8 status;
> -	int ret;
> -
> -	ret = ifcvf_start_hw(vf);
> -	if (ret < 0) {
> -		status = ifcvf_get_status(vf);
> -		status |= VIRTIO_CONFIG_S_FAILED;
> -		ifcvf_set_status(vf, status);
> -	}
> -
> -	return ret;
> -}
> -
>   static int ifcvf_stop_datapath(struct ifcvf_adapter *adapter)
>   {
>   	struct ifcvf_hw *vf = adapter->vf;
> @@ -452,13 +436,11 @@ static u8 ifcvf_vdpa_get_status(struct vdpa_device *vdpa_dev)
>   
>   static void ifcvf_vdpa_set_status(struct vdpa_device *vdpa_dev, u8 status)
>   {
> -	struct ifcvf_adapter *adapter;
>   	struct ifcvf_hw *vf;
>   	u8 status_old;
>   	int ret;
>   
>   	vf  = vdpa_to_vf(vdpa_dev);
> -	adapter = vdpa_to_adapter(vdpa_dev);
>   	status_old = ifcvf_get_status(vf);
>   
>   	if (status_old == status)
> @@ -473,11 +455,6 @@ static void ifcvf_vdpa_set_status(struct vdpa_device *vdpa_dev, u8 status)
>   			ifcvf_set_status(vf, status);
>   			return;
>   		}
> -
> -		if (ifcvf_start_datapath(adapter) < 0)
> -			IFCVF_ERR(adapter->pdev,
> -				  "Failed to set ifcvf vdpa  status %u\n",
> -				  status);
>   	}
>   
>   	ifcvf_set_status(vf, status);

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization