diff mbox series

[22/40] net/virtio: remove last PCI refs in non-PCI code

Message ID	20201220211405.313012-23-maxime.coquelin@redhat.com (mailing list archive)
State	Superseded, archived
Delegated to:	Maxime Coquelin
Headers	From: Maxime Coquelin <maxime.coquelin@redhat.com> To: dev@dpdk.org, chenbo.xia@intel.com, olivier.matz@6wind.com, amorenoz@redhat.com, david.marchand@redhat.com Cc: Maxime Coquelin <maxime.coquelin@redhat.com> Date: Sun, 20 Dec 2020 22:13:47 +0100 Message-Id: <20201220211405.313012-23-maxime.coquelin@redhat.com> In-Reply-To: <20201220211405.313012-1-maxime.coquelin@redhat.com> References: <20201220211405.313012-1-maxime.coquelin@redhat.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII" Subject: [dpdk-dev] [PATCH 22/40] net/virtio: remove last PCI refs in non-PCI code Precedence: list Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org>
Series	net/virtio: Virtio PMD rework \| [00/40] net/virtio: Virtio PMD rework [01/40] bus/vdev: add helper to get vdev from eth dev [02/40] net/virtio: Introduce Virtio bus type [03/40] net/virtio: refactor virtio-user device [04/40] net/virtio: introduce PCI device metadata [05/40] net/virtio: move PCI device init in dedicated file [06/40] net/virtio: move PCI specific dev init to PCI ethdev init [07/40] net/virtio: move MSIX detection to PCI ethdev [08/40] net/virtio: force IOVA as VA mode for Virtio-user [09/40] net/virtio: store PCI type in Virtio device metadata [10/40] net/virtio: add callback for device closing [11/40] net/virtio: validate features at bus level [12/40] net/virtio: remove bus type enum [13/40] net/virtio: move PCI-specific fields to PCI device [14/40] net/virtio: pack virtio HW struct [15/40] net/virtio: move legacy IO to Virtio PCI [16/40] net/virtio: introduce generic virtio header [17/40] net/virtio: move features definition to generic header [18/40] net/virtio: move virtqueue defines in generic header [19/40] net/virtio: move config definitions to generic header [20/40] net/virtio: make interrupt handling more generic [21/40] net/virtio: move vring alignment to generic header [22/40] net/virtio: remove last PCI refs in non-PCI code [23/40] net/virtio: make Vhost-user req sender consistent [24/40] net/virtio: add Virtio-user ops to set owner [25/40] net/virtio: add Virtio-user features ops [26/40] net/virtio: add Virtio-user protocol features ops [27/40] net/virtio: add Virtio-user memory tables ops [28/40] net/virtio: add Virtio-user vring setting ops [29/40] net/virtio: add Virtio-user vring file ops [30/40] net/virtio: add Virtio-user vring address ops [31/40] net/virtio: add Virtio-user status ops [32/40] net/virtio: remove useless request ops [33/40] net/virtio: improve Virtio-user errors handling [34/40] net/virtio: move Vhost-user reqs to Vhost-user backend [35/40] net/virtio: make server mode blocking [36/40] net/virtio: move protocol features to Vhost-user [37/40] net/virtio: introduce backend data [38/40] net/virtio: move Vhost-user specifics to its backend [39/40] net/virtio: move Vhost-kernel data to its backend [40/40] net/virtio: move Vhost-vDPA data to its backend

Checks

Context	Check	Description
ci/checkpatch	success	coding style OK

Commit Message

Maxime Coquelin Dec. 20, 2020, 9:13 p.m. UTC

  This patch finalizes the bus isolation part of this
refactoring.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/net/virtio/virtio_ethdev.c          | 21 +++++++++------------
 drivers/net/virtio/virtio_rxtx.c            | 18 +++++++++---------
 drivers/net/virtio/virtio_rxtx_packed_avx.c |  2 +-
 drivers/net/virtio/virtio_user/vhost.h      |  4 +++-
 drivers/net/virtio/virtqueue.c              |  2 +-
 drivers/net/virtio/virtqueue.h              |  6 +++---
 6 files changed, 26 insertions(+), 27 deletions(-)

Comments

Chenbo Xia Dec. 30, 2020, 3:25 a.m. UTC | #1

Hi Maxime,

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Monday, December 21, 2020 5:14 AM
> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; olivier.matz@6wind.com;
> amorenoz@redhat.com; david.marchand@redhat.com
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: [PATCH 22/40] net/virtio: remove last PCI refs in non-PCI code
> 
> This patch finalizes the bus isolation part of this
> refactoring.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>

I think this one is also the remove-dependency part? Because you said first 21 patches
are the first part :P

I think the first part is very great clean-up. It does make our code cleaner!

For this patch:

Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>

> ---
>  drivers/net/virtio/virtio_ethdev.c          | 21 +++++++++------------
>  drivers/net/virtio/virtio_rxtx.c            | 18 +++++++++---------
>  drivers/net/virtio/virtio_rxtx_packed_avx.c |  2 +-
>  drivers/net/virtio/virtio_user/vhost.h      |  4 +++-
>  drivers/net/virtio/virtqueue.c              |  2 +-
>  drivers/net/virtio/virtqueue.h              |  6 +++---
>  6 files changed, 26 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_ethdev.c
> b/drivers/net/virtio/virtio_ethdev.c
> index 1ca8715832..96871b7b70 100644
> --- a/drivers/net/virtio/virtio_ethdev.c
> +++ b/drivers/net/virtio/virtio_ethdev.c
> @@ -9,14 +9,11 @@
>  #include <unistd.h>
> 
>  #include <rte_ethdev_driver.h>
> -#include <rte_ethdev_pci.h>
>  #include <rte_memcpy.h>
>  #include <rte_string_fns.h>
>  #include <rte_memzone.h>
>  #include <rte_malloc.h>
>  #include <rte_branch_prediction.h>
> -#include <rte_pci.h>
> -#include <rte_bus_pci.h>
>  #include <rte_ether.h>
>  #include <rte_ip.h>
>  #include <rte_arp.h>
> @@ -32,7 +29,7 @@
>  #include <rte_kvargs.h>
> 
>  #include "virtio_ethdev.h"
> -#include "virtio_pci.h"
> +#include "virtio.h"
>  #include "virtio_logs.h"
>  #include "virtqueue.h"
>  #include "virtio_rxtx.h"
> @@ -422,7 +419,7 @@ virtio_init_vring(struct virtqueue *vq)
>  }
> 
>  static int
> -virtio_init_queue(struct rte_eth_dev *dev, uint16_t vtpci_queue_idx)
> +virtio_init_queue(struct rte_eth_dev *dev, uint16_t queue_idx)
>  {
>  	char vq_name[VIRTQUEUE_MAX_NAME_SZ];
>  	char vq_hdr_name[VIRTQUEUE_MAX_NAME_SZ];
> @@ -435,18 +432,18 @@ virtio_init_queue(struct rte_eth_dev *dev, uint16_t
> vtpci_queue_idx)
>  	struct virtqueue *vq;
>  	size_t sz_hdr_mz = 0;
>  	void *sw_ring = NULL;
> -	int queue_type = virtio_get_queue_type(hw, vtpci_queue_idx);
> +	int queue_type = virtio_get_queue_type(hw, queue_idx);
>  	int ret;
>  	int numa_node = dev->device->numa_node;
> 
>  	PMD_INIT_LOG(INFO, "setting up queue: %u on NUMA node %d",
> -			vtpci_queue_idx, numa_node);
> +			queue_idx, numa_node);
> 
>  	/*
>  	 * Read the virtqueue size from the Queue Size field
>  	 * Always power of 2 and if 0 virtqueue does not exist
>  	 */
> -	vq_size = VIRTIO_OPS(hw)->get_queue_num(hw, vtpci_queue_idx);
> +	vq_size = VIRTIO_OPS(hw)->get_queue_num(hw, queue_idx);
>  	PMD_INIT_LOG(DEBUG, "vq_size: %u", vq_size);
>  	if (vq_size == 0) {
>  		PMD_INIT_LOG(ERR, "virtqueue does not exist");
> @@ -459,7 +456,7 @@ virtio_init_queue(struct rte_eth_dev *dev, uint16_t
> vtpci_queue_idx)
>  	}
> 
>  	snprintf(vq_name, sizeof(vq_name), "port%d_vq%d",
> -		 dev->data->port_id, vtpci_queue_idx);
> +		 dev->data->port_id, queue_idx);
> 
>  	size = RTE_ALIGN_CEIL(sizeof(*vq) +
>  				vq_size * sizeof(struct vq_desc_extra),
> @@ -481,10 +478,10 @@ virtio_init_queue(struct rte_eth_dev *dev, uint16_t
> vtpci_queue_idx)
>  		PMD_INIT_LOG(ERR, "can not allocate vq");
>  		return -ENOMEM;
>  	}
> -	hw->vqs[vtpci_queue_idx] = vq;
> +	hw->vqs[queue_idx] = vq;
> 
>  	vq->hw = hw;
> -	vq->vq_queue_index = vtpci_queue_idx;
> +	vq->vq_queue_index = queue_idx;
>  	vq->vq_nentries = vq_size;
>  	if (virtio_with_packed_queue(hw)) {
>  		vq->vq_packed.used_wrap_counter = 1;
> @@ -527,7 +524,7 @@ virtio_init_queue(struct rte_eth_dev *dev, uint16_t
> vtpci_queue_idx)
> 
>  	if (sz_hdr_mz) {
>  		snprintf(vq_hdr_name, sizeof(vq_hdr_name), "port%d_vq%d_hdr",
> -			 dev->data->port_id, vtpci_queue_idx);
> +			 dev->data->port_id, queue_idx);
>  		hdr_mz = rte_memzone_reserve_aligned(vq_hdr_name, sz_hdr_mz,
>  				numa_node, RTE_MEMZONE_IOVA_CONTIG,
>  				RTE_CACHE_LINE_SIZE);
> diff --git a/drivers/net/virtio/virtio_rxtx.c
> b/drivers/net/virtio/virtio_rxtx.c
> index 10989118b0..aca6eb9cd0 100644
> --- a/drivers/net/virtio/virtio_rxtx.c
> +++ b/drivers/net/virtio/virtio_rxtx.c
> @@ -27,7 +27,7 @@
> 
>  #include "virtio_logs.h"
>  #include "virtio_ethdev.h"
> -#include "virtio_pci.h"
> +#include "virtio.h"
>  #include "virtqueue.h"
>  #include "virtio_rxtx.h"
>  #include "virtio_rxtx_simple.h"
> @@ -628,9 +628,9 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,
>  			const struct rte_eth_rxconf *rx_conf,
>  			struct rte_mempool *mp)
>  {
> -	uint16_t vtpci_queue_idx = 2 * queue_idx + VTNET_SQ_RQ_QUEUE_IDX;
> +	uint16_t vq_idx = 2 * queue_idx + VTNET_SQ_RQ_QUEUE_IDX;
>  	struct virtio_hw *hw = dev->data->dev_private;
> -	struct virtqueue *vq = hw->vqs[vtpci_queue_idx];
> +	struct virtqueue *vq = hw->vqs[vq_idx];
>  	struct virtnet_rx *rxvq;
>  	uint16_t rx_free_thresh;
> 
> @@ -678,9 +678,9 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,
>  int
>  virtio_dev_rx_queue_setup_finish(struct rte_eth_dev *dev, uint16_t queue_idx)
>  {
> -	uint16_t vtpci_queue_idx = 2 * queue_idx + VTNET_SQ_RQ_QUEUE_IDX;
> +	uint16_t vq_idx = 2 * queue_idx + VTNET_SQ_RQ_QUEUE_IDX;
>  	struct virtio_hw *hw = dev->data->dev_private;
> -	struct virtqueue *vq = hw->vqs[vtpci_queue_idx];
> +	struct virtqueue *vq = hw->vqs[vq_idx];
>  	struct virtnet_rx *rxvq = &vq->rxq;
>  	struct rte_mbuf *m;
>  	uint16_t desc_idx;
> @@ -779,9 +779,9 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
>  			unsigned int socket_id __rte_unused,
>  			const struct rte_eth_txconf *tx_conf)
>  {
> -	uint8_t vtpci_queue_idx = 2 * queue_idx + VTNET_SQ_TQ_QUEUE_IDX;
> +	uint8_t vq_idx = 2 * queue_idx + VTNET_SQ_TQ_QUEUE_IDX;
>  	struct virtio_hw *hw = dev->data->dev_private;
> -	struct virtqueue *vq = hw->vqs[vtpci_queue_idx];
> +	struct virtqueue *vq = hw->vqs[vq_idx];
>  	struct virtnet_tx *txvq;
>  	uint16_t tx_free_thresh;
> 
> @@ -823,9 +823,9 @@ int
>  virtio_dev_tx_queue_setup_finish(struct rte_eth_dev *dev,
>  				uint16_t queue_idx)
>  {
> -	uint8_t vtpci_queue_idx = 2 * queue_idx + VTNET_SQ_TQ_QUEUE_IDX;
> +	uint8_t vq_idx = 2 * queue_idx + VTNET_SQ_TQ_QUEUE_IDX;
>  	struct virtio_hw *hw = dev->data->dev_private;
> -	struct virtqueue *vq = hw->vqs[vtpci_queue_idx];
> +	struct virtqueue *vq = hw->vqs[vq_idx];
> 
>  	PMD_INIT_FUNC_TRACE();
> 
> diff --git a/drivers/net/virtio/virtio_rxtx_packed_avx.c
> b/drivers/net/virtio/virtio_rxtx_packed_avx.c
> index c272766a9f..a3dcc01a43 100644
> --- a/drivers/net/virtio/virtio_rxtx_packed_avx.c
> +++ b/drivers/net/virtio/virtio_rxtx_packed_avx.c
> @@ -12,7 +12,7 @@
> 
>  #include "virtio_logs.h"
>  #include "virtio_ethdev.h"
> -#include "virtio_pci.h"
> +#include "virtio.h"
>  #include "virtqueue.h"
> 
>  #define BYTE_SIZE 8
> diff --git a/drivers/net/virtio/virtio_user/vhost.h
> b/drivers/net/virtio/virtio_user/vhost.h
> index 210a3704e7..9d2a8505b3 100644
> --- a/drivers/net/virtio/virtio_user/vhost.h
> +++ b/drivers/net/virtio/virtio_user/vhost.h
> @@ -9,7 +9,9 @@
>  #include <linux/types.h>
>  #include <linux/ioctl.h>
> 
> -#include "../virtio_pci.h"
> +#include <rte_errno.h>
> +
> +#include "../virtio.h"
>  #include "../virtio_logs.h"
>  #include "../virtqueue.h"
> 
> diff --git a/drivers/net/virtio/virtqueue.c b/drivers/net/virtio/virtqueue.c
> index 59a2cb6599..1f9af3c31b 100644
> --- a/drivers/net/virtio/virtqueue.c
> +++ b/drivers/net/virtio/virtqueue.c
> @@ -7,7 +7,7 @@
> 
>  #include "virtqueue.h"
>  #include "virtio_logs.h"
> -#include "virtio_pci.h"
> +#include "virtio.h"
>  #include "virtio_rxtx_simple.h"
> 
>  /*
> diff --git a/drivers/net/virtio/virtqueue.h b/drivers/net/virtio/virtqueue.h
> index 6c1df6f8e5..9274c48080 100644
> --- a/drivers/net/virtio/virtqueue.h
> +++ b/drivers/net/virtio/virtqueue.h
> @@ -449,11 +449,11 @@ virtqueue_full(const struct virtqueue *vq)
>  }
> 
>  static inline int
> -virtio_get_queue_type(struct virtio_hw *hw, uint16_t vtpci_queue_idx)
> +virtio_get_queue_type(struct virtio_hw *hw, uint16_t vq_idx)
>  {
> -	if (vtpci_queue_idx == hw->max_queue_pairs * 2)
> +	if (vq_idx == hw->max_queue_pairs * 2)
>  		return VTNET_CQ;
> -	else if (vtpci_queue_idx % 2 == 0)
> +	else if (vq_idx % 2 == 0)
>  		return VTNET_RQ;
>  	else
>  		return VTNET_TQ;
> --
> 2.29.2

David Marchand Jan. 6, 2021, 4:18 p.m. UTC | #2

On Sun, Dec 20, 2020 at 10:15 PM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
>
> This patch finalizes the bus isolation part of this
> refactoring.
>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---

[snip]

> diff --git a/drivers/net/virtio/virtqueue.h b/drivers/net/virtio/virtqueue.h
> index 6c1df6f8e5..9274c48080 100644
> --- a/drivers/net/virtio/virtqueue.h
> +++ b/drivers/net/virtio/virtqueue.h
> @@ -449,11 +449,11 @@ virtqueue_full(const struct virtqueue *vq)
>  }
>
>  static inline int
> -virtio_get_queue_type(struct virtio_hw *hw, uint16_t vtpci_queue_idx)
> +virtio_get_queue_type(struct virtio_hw *hw, uint16_t vq_idx)
>  {
> -       if (vtpci_queue_idx == hw->max_queue_pairs * 2)
> +       if (vq_idx == hw->max_queue_pairs * 2)
>                 return VTNET_CQ;
> -       else if (vtpci_queue_idx % 2 == 0)
> +       else if (vq_idx % 2 == 0)
>                 return VTNET_RQ;
>         else
>                 return VTNET_TQ;
> --
> 2.29.2
>

I noticed:
drivers/net/virtio/virtqueue.h: uint16_t  vq_queue_index;   /**< PCI
queue index */

Worth cleaning while at it?

Reviewed-by: David Marchand <david.marchand@redhat.com>



Same comment as Chenbo, this is a great cleanup so far.
I'll look at the rest of the series probably tomorrow.

Maxime Coquelin Jan. 14, 2021, 8:46 a.m. UTC | #3

On 12/30/20 4:25 AM, Xia, Chenbo wrote:
> Hi Maxime,
> 
>> -----Original Message-----
>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Sent: Monday, December 21, 2020 5:14 AM
>> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; olivier.matz@6wind.com;
>> amorenoz@redhat.com; david.marchand@redhat.com
>> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Subject: [PATCH 22/40] net/virtio: remove last PCI refs in non-PCI code
>>
>> This patch finalizes the bus isolation part of this
>> refactoring.
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> 
> I think this one is also the remove-dependency part? Because you said first 21 patches
> are the first part :P

Yes, that's what I mean by bus isolation.

> I think the first part is very great clean-up. It does make our code cleaner!
>
> For this patch:
> 
> Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>


Thanks, your review is much apopreciated.

Maxime

> 
>> ---
>>  drivers/net/virtio/virtio_ethdev.c          | 21 +++++++++------------
>>  drivers/net/virtio/virtio_rxtx.c            | 18 +++++++++---------
>>  drivers/net/virtio/virtio_rxtx_packed_avx.c |  2 +-
>>  drivers/net/virtio/virtio_user/vhost.h      |  4 +++-
>>  drivers/net/virtio/virtqueue.c              |  2 +-
>>  drivers/net/virtio/virtqueue.h              |  6 +++---
>>  6 files changed, 26 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/net/virtio/virtio_ethdev.c
>> b/drivers/net/virtio/virtio_ethdev.c
>> index 1ca8715832..96871b7b70 100644
>> --- a/drivers/net/virtio/virtio_ethdev.c
>> +++ b/drivers/net/virtio/virtio_ethdev.c
>> @@ -9,14 +9,11 @@
>>  #include <unistd.h>
>>
>>  #include <rte_ethdev_driver.h>
>> -#include <rte_ethdev_pci.h>
>>  #include <rte_memcpy.h>
>>  #include <rte_string_fns.h>
>>  #include <rte_memzone.h>
>>  #include <rte_malloc.h>
>>  #include <rte_branch_prediction.h>
>> -#include <rte_pci.h>
>> -#include <rte_bus_pci.h>
>>  #include <rte_ether.h>
>>  #include <rte_ip.h>
>>  #include <rte_arp.h>
>> @@ -32,7 +29,7 @@
>>  #include <rte_kvargs.h>
>>
>>  #include "virtio_ethdev.h"
>> -#include "virtio_pci.h"
>> +#include "virtio.h"
>>  #include "virtio_logs.h"
>>  #include "virtqueue.h"
>>  #include "virtio_rxtx.h"
>> @@ -422,7 +419,7 @@ virtio_init_vring(struct virtqueue *vq)
>>  }
>>
>>  static int
>> -virtio_init_queue(struct rte_eth_dev *dev, uint16_t vtpci_queue_idx)
>> +virtio_init_queue(struct rte_eth_dev *dev, uint16_t queue_idx)
>>  {
>>  	char vq_name[VIRTQUEUE_MAX_NAME_SZ];
>>  	char vq_hdr_name[VIRTQUEUE_MAX_NAME_SZ];
>> @@ -435,18 +432,18 @@ virtio_init_queue(struct rte_eth_dev *dev, uint16_t
>> vtpci_queue_idx)
>>  	struct virtqueue *vq;
>>  	size_t sz_hdr_mz = 0;
>>  	void *sw_ring = NULL;
>> -	int queue_type = virtio_get_queue_type(hw, vtpci_queue_idx);
>> +	int queue_type = virtio_get_queue_type(hw, queue_idx);
>>  	int ret;
>>  	int numa_node = dev->device->numa_node;
>>
>>  	PMD_INIT_LOG(INFO, "setting up queue: %u on NUMA node %d",
>> -			vtpci_queue_idx, numa_node);
>> +			queue_idx, numa_node);
>>
>>  	/*
>>  	 * Read the virtqueue size from the Queue Size field
>>  	 * Always power of 2 and if 0 virtqueue does not exist
>>  	 */
>> -	vq_size = VIRTIO_OPS(hw)->get_queue_num(hw, vtpci_queue_idx);
>> +	vq_size = VIRTIO_OPS(hw)->get_queue_num(hw, queue_idx);
>>  	PMD_INIT_LOG(DEBUG, "vq_size: %u", vq_size);
>>  	if (vq_size == 0) {
>>  		PMD_INIT_LOG(ERR, "virtqueue does not exist");
>> @@ -459,7 +456,7 @@ virtio_init_queue(struct rte_eth_dev *dev, uint16_t
>> vtpci_queue_idx)
>>  	}
>>
>>  	snprintf(vq_name, sizeof(vq_name), "port%d_vq%d",
>> -		 dev->data->port_id, vtpci_queue_idx);
>> +		 dev->data->port_id, queue_idx);
>>
>>  	size = RTE_ALIGN_CEIL(sizeof(*vq) +
>>  				vq_size * sizeof(struct vq_desc_extra),
>> @@ -481,10 +478,10 @@ virtio_init_queue(struct rte_eth_dev *dev, uint16_t
>> vtpci_queue_idx)
>>  		PMD_INIT_LOG(ERR, "can not allocate vq");
>>  		return -ENOMEM;
>>  	}
>> -	hw->vqs[vtpci_queue_idx] = vq;
>> +	hw->vqs[queue_idx] = vq;
>>
>>  	vq->hw = hw;
>> -	vq->vq_queue_index = vtpci_queue_idx;
>> +	vq->vq_queue_index = queue_idx;
>>  	vq->vq_nentries = vq_size;
>>  	if (virtio_with_packed_queue(hw)) {
>>  		vq->vq_packed.used_wrap_counter = 1;
>> @@ -527,7 +524,7 @@ virtio_init_queue(struct rte_eth_dev *dev, uint16_t
>> vtpci_queue_idx)
>>
>>  	if (sz_hdr_mz) {
>>  		snprintf(vq_hdr_name, sizeof(vq_hdr_name), "port%d_vq%d_hdr",
>> -			 dev->data->port_id, vtpci_queue_idx);
>> +			 dev->data->port_id, queue_idx);
>>  		hdr_mz = rte_memzone_reserve_aligned(vq_hdr_name, sz_hdr_mz,
>>  				numa_node, RTE_MEMZONE_IOVA_CONTIG,
>>  				RTE_CACHE_LINE_SIZE);
>> diff --git a/drivers/net/virtio/virtio_rxtx.c
>> b/drivers/net/virtio/virtio_rxtx.c
>> index 10989118b0..aca6eb9cd0 100644
>> --- a/drivers/net/virtio/virtio_rxtx.c
>> +++ b/drivers/net/virtio/virtio_rxtx.c
>> @@ -27,7 +27,7 @@
>>
>>  #include "virtio_logs.h"
>>  #include "virtio_ethdev.h"
>> -#include "virtio_pci.h"
>> +#include "virtio.h"
>>  #include "virtqueue.h"
>>  #include "virtio_rxtx.h"
>>  #include "virtio_rxtx_simple.h"
>> @@ -628,9 +628,9 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,
>>  			const struct rte_eth_rxconf *rx_conf,
>>  			struct rte_mempool *mp)
>>  {
>> -	uint16_t vtpci_queue_idx = 2 * queue_idx + VTNET_SQ_RQ_QUEUE_IDX;
>> +	uint16_t vq_idx = 2 * queue_idx + VTNET_SQ_RQ_QUEUE_IDX;
>>  	struct virtio_hw *hw = dev->data->dev_private;
>> -	struct virtqueue *vq = hw->vqs[vtpci_queue_idx];
>> +	struct virtqueue *vq = hw->vqs[vq_idx];
>>  	struct virtnet_rx *rxvq;
>>  	uint16_t rx_free_thresh;
>>
>> @@ -678,9 +678,9 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,
>>  int
>>  virtio_dev_rx_queue_setup_finish(struct rte_eth_dev *dev, uint16_t queue_idx)
>>  {
>> -	uint16_t vtpci_queue_idx = 2 * queue_idx + VTNET_SQ_RQ_QUEUE_IDX;
>> +	uint16_t vq_idx = 2 * queue_idx + VTNET_SQ_RQ_QUEUE_IDX;
>>  	struct virtio_hw *hw = dev->data->dev_private;
>> -	struct virtqueue *vq = hw->vqs[vtpci_queue_idx];
>> +	struct virtqueue *vq = hw->vqs[vq_idx];
>>  	struct virtnet_rx *rxvq = &vq->rxq;
>>  	struct rte_mbuf *m;
>>  	uint16_t desc_idx;
>> @@ -779,9 +779,9 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
>>  			unsigned int socket_id __rte_unused,
>>  			const struct rte_eth_txconf *tx_conf)
>>  {
>> -	uint8_t vtpci_queue_idx = 2 * queue_idx + VTNET_SQ_TQ_QUEUE_IDX;
>> +	uint8_t vq_idx = 2 * queue_idx + VTNET_SQ_TQ_QUEUE_IDX;
>>  	struct virtio_hw *hw = dev->data->dev_private;
>> -	struct virtqueue *vq = hw->vqs[vtpci_queue_idx];
>> +	struct virtqueue *vq = hw->vqs[vq_idx];
>>  	struct virtnet_tx *txvq;
>>  	uint16_t tx_free_thresh;
>>
>> @@ -823,9 +823,9 @@ int
>>  virtio_dev_tx_queue_setup_finish(struct rte_eth_dev *dev,
>>  				uint16_t queue_idx)
>>  {
>> -	uint8_t vtpci_queue_idx = 2 * queue_idx + VTNET_SQ_TQ_QUEUE_IDX;
>> +	uint8_t vq_idx = 2 * queue_idx + VTNET_SQ_TQ_QUEUE_IDX;
>>  	struct virtio_hw *hw = dev->data->dev_private;
>> -	struct virtqueue *vq = hw->vqs[vtpci_queue_idx];
>> +	struct virtqueue *vq = hw->vqs[vq_idx];
>>
>>  	PMD_INIT_FUNC_TRACE();
>>
>> diff --git a/drivers/net/virtio/virtio_rxtx_packed_avx.c
>> b/drivers/net/virtio/virtio_rxtx_packed_avx.c
>> index c272766a9f..a3dcc01a43 100644
>> --- a/drivers/net/virtio/virtio_rxtx_packed_avx.c
>> +++ b/drivers/net/virtio/virtio_rxtx_packed_avx.c
>> @@ -12,7 +12,7 @@
>>
>>  #include "virtio_logs.h"
>>  #include "virtio_ethdev.h"
>> -#include "virtio_pci.h"
>> +#include "virtio.h"
>>  #include "virtqueue.h"
>>
>>  #define BYTE_SIZE 8
>> diff --git a/drivers/net/virtio/virtio_user/vhost.h
>> b/drivers/net/virtio/virtio_user/vhost.h
>> index 210a3704e7..9d2a8505b3 100644
>> --- a/drivers/net/virtio/virtio_user/vhost.h
>> +++ b/drivers/net/virtio/virtio_user/vhost.h
>> @@ -9,7 +9,9 @@
>>  #include <linux/types.h>
>>  #include <linux/ioctl.h>
>>
>> -#include "../virtio_pci.h"
>> +#include <rte_errno.h>
>> +
>> +#include "../virtio.h"
>>  #include "../virtio_logs.h"
>>  #include "../virtqueue.h"
>>
>> diff --git a/drivers/net/virtio/virtqueue.c b/drivers/net/virtio/virtqueue.c
>> index 59a2cb6599..1f9af3c31b 100644
>> --- a/drivers/net/virtio/virtqueue.c
>> +++ b/drivers/net/virtio/virtqueue.c
>> @@ -7,7 +7,7 @@
>>
>>  #include "virtqueue.h"
>>  #include "virtio_logs.h"
>> -#include "virtio_pci.h"
>> +#include "virtio.h"
>>  #include "virtio_rxtx_simple.h"
>>
>>  /*
>> diff --git a/drivers/net/virtio/virtqueue.h b/drivers/net/virtio/virtqueue.h
>> index 6c1df6f8e5..9274c48080 100644
>> --- a/drivers/net/virtio/virtqueue.h
>> +++ b/drivers/net/virtio/virtqueue.h
>> @@ -449,11 +449,11 @@ virtqueue_full(const struct virtqueue *vq)
>>  }
>>
>>  static inline int
>> -virtio_get_queue_type(struct virtio_hw *hw, uint16_t vtpci_queue_idx)
>> +virtio_get_queue_type(struct virtio_hw *hw, uint16_t vq_idx)
>>  {
>> -	if (vtpci_queue_idx == hw->max_queue_pairs * 2)
>> +	if (vq_idx == hw->max_queue_pairs * 2)
>>  		return VTNET_CQ;
>> -	else if (vtpci_queue_idx % 2 == 0)
>> +	else if (vq_idx % 2 == 0)
>>  		return VTNET_RQ;
>>  	else
>>  		return VTNET_TQ;
>> --
>> 2.29.2
>

Maxime Coquelin Jan. 15, 2021, 11:10 a.m. UTC | #4

On 1/6/21 5:18 PM, David Marchand wrote:
> On Sun, Dec 20, 2020 at 10:15 PM Maxime Coquelin
> <maxime.coquelin@redhat.com> wrote:
>>
>> This patch finalizes the bus isolation part of this
>> refactoring.
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> ---
> 
> [snip]
> 
>> diff --git a/drivers/net/virtio/virtqueue.h b/drivers/net/virtio/virtqueue.h
>> index 6c1df6f8e5..9274c48080 100644
>> --- a/drivers/net/virtio/virtqueue.h
>> +++ b/drivers/net/virtio/virtqueue.h
>> @@ -449,11 +449,11 @@ virtqueue_full(const struct virtqueue *vq)
>>  }
>>
>>  static inline int
>> -virtio_get_queue_type(struct virtio_hw *hw, uint16_t vtpci_queue_idx)
>> +virtio_get_queue_type(struct virtio_hw *hw, uint16_t vq_idx)
>>  {
>> -       if (vtpci_queue_idx == hw->max_queue_pairs * 2)
>> +       if (vq_idx == hw->max_queue_pairs * 2)
>>                 return VTNET_CQ;
>> -       else if (vtpci_queue_idx % 2 == 0)
>> +       else if (vq_idx % 2 == 0)
>>                 return VTNET_RQ;
>>         else
>>                 return VTNET_TQ;
>> --
>> 2.29.2
>>
> 
> I noticed:
> drivers/net/virtio/virtqueue.h: uint16_t  vq_queue_index;   /**< PCI
> queue index */
> 
> Worth cleaning while at it?

I removed the comment, it is self-explanatory.

Thanks,
Maxime

> Reviewed-by: David Marchand <david.marchand@redhat.com>
> 
> 
> 
> Same comment as Chenbo, this is a great cleanup so far.
> I'll look at the rest of the series probably tomorrow.
>

diff mbox series

Patch

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 1ca8715832..96871b7b70 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -9,14 +9,11 @@ 
 #include <unistd.h>
 
 #include <rte_ethdev_driver.h>
-#include <rte_ethdev_pci.h>
 #include <rte_memcpy.h>
 #include <rte_string_fns.h>
 #include <rte_memzone.h>
 #include <rte_malloc.h>
 #include <rte_branch_prediction.h>
-#include <rte_pci.h>
-#include <rte_bus_pci.h>
 #include <rte_ether.h>
 #include <rte_ip.h>
 #include <rte_arp.h>
@@ -32,7 +29,7 @@ 
 #include <rte_kvargs.h>
 
 #include "virtio_ethdev.h"
-#include "virtio_pci.h"
+#include "virtio.h"
 #include "virtio_logs.h"
 #include "virtqueue.h"
 #include "virtio_rxtx.h"
@@ -422,7 +419,7 @@  virtio_init_vring(struct virtqueue *vq)
 }
 
 static int
-virtio_init_queue(struct rte_eth_dev *dev, uint16_t vtpci_queue_idx)
+virtio_init_queue(struct rte_eth_dev *dev, uint16_t queue_idx)
 {
 	char vq_name[VIRTQUEUE_MAX_NAME_SZ];
 	char vq_hdr_name[VIRTQUEUE_MAX_NAME_SZ];
@@ -435,18 +432,18 @@  virtio_init_queue(struct rte_eth_dev *dev, uint16_t vtpci_queue_idx)
 	struct virtqueue *vq;
 	size_t sz_hdr_mz = 0;
 	void *sw_ring = NULL;
-	int queue_type = virtio_get_queue_type(hw, vtpci_queue_idx);
+	int queue_type = virtio_get_queue_type(hw, queue_idx);
 	int ret;
 	int numa_node = dev->device->numa_node;
 
 	PMD_INIT_LOG(INFO, "setting up queue: %u on NUMA node %d",
-			vtpci_queue_idx, numa_node);
+			queue_idx, numa_node);
 
 	/*
 	 * Read the virtqueue size from the Queue Size field
 	 * Always power of 2 and if 0 virtqueue does not exist
 	 */
-	vq_size = VIRTIO_OPS(hw)->get_queue_num(hw, vtpci_queue_idx);
+	vq_size = VIRTIO_OPS(hw)->get_queue_num(hw, queue_idx);
 	PMD_INIT_LOG(DEBUG, "vq_size: %u", vq_size);
 	if (vq_size == 0) {
 		PMD_INIT_LOG(ERR, "virtqueue does not exist");
@@ -459,7 +456,7 @@  virtio_init_queue(struct rte_eth_dev *dev, uint16_t vtpci_queue_idx)
 	}
 
 	snprintf(vq_name, sizeof(vq_name), "port%d_vq%d",
-		 dev->data->port_id, vtpci_queue_idx);
+		 dev->data->port_id, queue_idx);
 
 	size = RTE_ALIGN_CEIL(sizeof(*vq) +
 				vq_size * sizeof(struct vq_desc_extra),
@@ -481,10 +478,10 @@  virtio_init_queue(struct rte_eth_dev *dev, uint16_t vtpci_queue_idx)
 		PMD_INIT_LOG(ERR, "can not allocate vq");
 		return -ENOMEM;
 	}
-	hw->vqs[vtpci_queue_idx] = vq;
+	hw->vqs[queue_idx] = vq;
 
 	vq->hw = hw;
-	vq->vq_queue_index = vtpci_queue_idx;
+	vq->vq_queue_index = queue_idx;
 	vq->vq_nentries = vq_size;
 	if (virtio_with_packed_queue(hw)) {
 		vq->vq_packed.used_wrap_counter = 1;
@@ -527,7 +524,7 @@  virtio_init_queue(struct rte_eth_dev *dev, uint16_t vtpci_queue_idx)
 
 	if (sz_hdr_mz) {
 		snprintf(vq_hdr_name, sizeof(vq_hdr_name), "port%d_vq%d_hdr",
-			 dev->data->port_id, vtpci_queue_idx);
+			 dev->data->port_id, queue_idx);
 		hdr_mz = rte_memzone_reserve_aligned(vq_hdr_name, sz_hdr_mz,
 				numa_node, RTE_MEMZONE_IOVA_CONTIG,
 				RTE_CACHE_LINE_SIZE);
diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index 10989118b0..aca6eb9cd0 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -27,7 +27,7 @@ 
 
 #include "virtio_logs.h"
 #include "virtio_ethdev.h"
-#include "virtio_pci.h"
+#include "virtio.h"
 #include "virtqueue.h"
 #include "virtio_rxtx.h"
 #include "virtio_rxtx_simple.h"
@@ -628,9 +628,9 @@  virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,
 			const struct rte_eth_rxconf *rx_conf,
 			struct rte_mempool *mp)
 {
-	uint16_t vtpci_queue_idx = 2 * queue_idx + VTNET_SQ_RQ_QUEUE_IDX;
+	uint16_t vq_idx = 2 * queue_idx + VTNET_SQ_RQ_QUEUE_IDX;
 	struct virtio_hw *hw = dev->data->dev_private;
-	struct virtqueue *vq = hw->vqs[vtpci_queue_idx];
+	struct virtqueue *vq = hw->vqs[vq_idx];
 	struct virtnet_rx *rxvq;
 	uint16_t rx_free_thresh;
 
@@ -678,9 +678,9 @@  virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,
 int
 virtio_dev_rx_queue_setup_finish(struct rte_eth_dev *dev, uint16_t queue_idx)
 {
-	uint16_t vtpci_queue_idx = 2 * queue_idx + VTNET_SQ_RQ_QUEUE_IDX;
+	uint16_t vq_idx = 2 * queue_idx + VTNET_SQ_RQ_QUEUE_IDX;
 	struct virtio_hw *hw = dev->data->dev_private;
-	struct virtqueue *vq = hw->vqs[vtpci_queue_idx];
+	struct virtqueue *vq = hw->vqs[vq_idx];
 	struct virtnet_rx *rxvq = &vq->rxq;
 	struct rte_mbuf *m;
 	uint16_t desc_idx;
@@ -779,9 +779,9 @@  virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
 			unsigned int socket_id __rte_unused,
 			const struct rte_eth_txconf *tx_conf)
 {
-	uint8_t vtpci_queue_idx = 2 * queue_idx + VTNET_SQ_TQ_QUEUE_IDX;
+	uint8_t vq_idx = 2 * queue_idx + VTNET_SQ_TQ_QUEUE_IDX;
 	struct virtio_hw *hw = dev->data->dev_private;
-	struct virtqueue *vq = hw->vqs[vtpci_queue_idx];
+	struct virtqueue *vq = hw->vqs[vq_idx];
 	struct virtnet_tx *txvq;
 	uint16_t tx_free_thresh;
 
@@ -823,9 +823,9 @@  int
 virtio_dev_tx_queue_setup_finish(struct rte_eth_dev *dev,
 				uint16_t queue_idx)
 {
-	uint8_t vtpci_queue_idx = 2 * queue_idx + VTNET_SQ_TQ_QUEUE_IDX;
+	uint8_t vq_idx = 2 * queue_idx + VTNET_SQ_TQ_QUEUE_IDX;
 	struct virtio_hw *hw = dev->data->dev_private;
-	struct virtqueue *vq = hw->vqs[vtpci_queue_idx];
+	struct virtqueue *vq = hw->vqs[vq_idx];
 
 	PMD_INIT_FUNC_TRACE();
 
diff --git a/drivers/net/virtio/virtio_rxtx_packed_avx.c b/drivers/net/virtio/virtio_rxtx_packed_avx.c
index c272766a9f..a3dcc01a43 100644
--- a/drivers/net/virtio/virtio_rxtx_packed_avx.c
+++ b/drivers/net/virtio/virtio_rxtx_packed_avx.c
@@ -12,7 +12,7 @@ 
 
 #include "virtio_logs.h"
 #include "virtio_ethdev.h"
-#include "virtio_pci.h"
+#include "virtio.h"
 #include "virtqueue.h"
 
 #define BYTE_SIZE 8
diff --git a/drivers/net/virtio/virtio_user/vhost.h b/drivers/net/virtio/virtio_user/vhost.h
index 210a3704e7..9d2a8505b3 100644
--- a/drivers/net/virtio/virtio_user/vhost.h
+++ b/drivers/net/virtio/virtio_user/vhost.h
@@ -9,7 +9,9 @@ 
 #include <linux/types.h>
 #include <linux/ioctl.h>
 
-#include "../virtio_pci.h"
+#include <rte_errno.h>
+
+#include "../virtio.h"
 #include "../virtio_logs.h"
 #include "../virtqueue.h"
 
diff --git a/drivers/net/virtio/virtqueue.c b/drivers/net/virtio/virtqueue.c
index 59a2cb6599..1f9af3c31b 100644
--- a/drivers/net/virtio/virtqueue.c
+++ b/drivers/net/virtio/virtqueue.c
@@ -7,7 +7,7 @@ 
 
 #include "virtqueue.h"
 #include "virtio_logs.h"
-#include "virtio_pci.h"
+#include "virtio.h"
 #include "virtio_rxtx_simple.h"
 
 /*
diff --git a/drivers/net/virtio/virtqueue.h b/drivers/net/virtio/virtqueue.h
index 6c1df6f8e5..9274c48080 100644
--- a/drivers/net/virtio/virtqueue.h
+++ b/drivers/net/virtio/virtqueue.h
@@ -449,11 +449,11 @@  virtqueue_full(const struct virtqueue *vq)
 }
 
 static inline int
-virtio_get_queue_type(struct virtio_hw *hw, uint16_t vtpci_queue_idx)
+virtio_get_queue_type(struct virtio_hw *hw, uint16_t vq_idx)
 {
-	if (vtpci_queue_idx == hw->max_queue_pairs * 2)
+	if (vq_idx == hw->max_queue_pairs * 2)
 		return VTNET_CQ;
-	else if (vtpci_queue_idx % 2 == 0)
+	else if (vq_idx % 2 == 0)
 		return VTNET_RQ;
 	else
 		return VTNET_TQ;