[09/40] net/virtio: store PCI type in Virtio device metadata

Message ID 20201220211405.313012-10-maxime.coquelin@redhat.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers
Series net/virtio: Virtio PMD rework |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Maxime Coquelin Dec. 20, 2020, 9:13 p.m. UTC
  Going further in making the Virtio ethdev layer bus agnostic,
this patch adds a boolean in the Virtio PCI device metadata.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/net/virtio/virtio_pci.c        | 20 ++++++++++++--------
 drivers/net/virtio/virtio_pci.h        |  3 ++-
 drivers/net/virtio/virtio_pci_ethdev.c | 12 +++++++-----
 3 files changed, 21 insertions(+), 14 deletions(-)
  

Comments

Chenbo Xia Dec. 30, 2020, 3:07 a.m. UTC | #1
> -----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 09/40] net/virtio: store PCI type in Virtio device metadata
> 
> Going further in making the Virtio ethdev layer bus agnostic,
> this patch adds a boolean in the Virtio PCI device metadata.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  drivers/net/virtio/virtio_pci.c        | 20 ++++++++++++--------
>  drivers/net/virtio/virtio_pci.h        |  3 ++-
>  drivers/net/virtio/virtio_pci_ethdev.c | 12 +++++++-----
>  3 files changed, 21 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_pci.c b/drivers/net/virtio/virtio_pci.c
> index 8605254e53..7f0c066968 100644
> --- a/drivers/net/virtio/virtio_pci.c
> +++ b/drivers/net/virtio/virtio_pci.c
> @@ -687,26 +687,29 @@ virtio_read_caps(struct rte_pci_device *dev, struct
> virtio_hw *hw)
>   * Return 0 on success.
>   */
>  int
> -vtpci_init(struct rte_pci_device *dev, struct virtio_hw *hw)
> +vtpci_init(struct rte_pci_device *pci_dev, struct virtio_pci_dev *dev)
>  {
> +	struct virtio_hw *hw = &dev->hw;
> +
>  	/*
>  	 * Try if we can succeed reading virtio pci caps, which exists
>  	 * only on modern pci device. If failed, we fallback to legacy
>  	 * virtio handling.
>  	 */
> -	if (virtio_read_caps(dev, hw) == 0) {
> +	if (virtio_read_caps(pci_dev, hw) == 0) {
>  		PMD_INIT_LOG(INFO, "modern virtio pci detected.");
>  		virtio_hw_internal[hw->port_id].vtpci_ops = &modern_ops;
>  		hw->bus_type = VIRTIO_BUS_PCI_MODERN;
> +		dev->modern = true;
>  		goto msix_detect;
>  	}
> 
>  	PMD_INIT_LOG(INFO, "trying with legacy virtio pci.");
> -	if (rte_pci_ioport_map(dev, 0, VTPCI_IO(hw)) < 0) {
> -		rte_pci_unmap_device(dev);
> -		if (dev->kdrv == RTE_PCI_KDRV_UNKNOWN &&
> -		    (!dev->device.devargs ||
> -		     dev->device.devargs->bus !=
> +	if (rte_pci_ioport_map(pci_dev, 0, VTPCI_IO(hw)) < 0) {
> +		rte_pci_unmap_device(pci_dev);
> +		if (pci_dev->kdrv == RTE_PCI_KDRV_UNKNOWN &&
> +		    (!pci_dev->device.devargs ||
> +		     pci_dev->device.devargs->bus !=
>  		     rte_bus_find_by_name("pci"))) {
>  			PMD_INIT_LOG(INFO,
>  				"skip kernel managed virtio device.");
> @@ -717,9 +720,10 @@ vtpci_init(struct rte_pci_device *dev, struct virtio_hw
> *hw)
> 
>  	virtio_hw_internal[hw->port_id].vtpci_ops = &legacy_ops;
>  	hw->bus_type = VIRTIO_BUS_PCI_LEGACY;
> +	dev->modern = false;
> 
>  msix_detect:
> -	hw->use_msix = vtpci_msix_detect(dev);
> +	hw->use_msix = vtpci_msix_detect(pci_dev);
> 
>  	return 0;
>  }
> diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
> index 8d3dc0e22e..3e245ed630 100644
> --- a/drivers/net/virtio/virtio_pci.h
> +++ b/drivers/net/virtio/virtio_pci.h
> @@ -291,6 +291,7 @@ struct virtio_hw {
> 
>  struct virtio_pci_dev {
>  	struct virtio_hw hw;
> +	bool modern;
>  };
> 
>  #define virtio_pci_get_dev(hw) container_of(hw, struct virtio_pci_dev, hw)
> @@ -367,7 +368,7 @@ vtpci_packed_queue(struct virtio_hw *hw)
>  /*
>   * Function declaration from virtio_pci.c
>   */
> -int vtpci_init(struct rte_pci_device *dev, struct virtio_hw *hw);
> +int vtpci_init(struct rte_pci_device *pci_dev, struct virtio_pci_dev *dev);
>  void vtpci_reset(struct virtio_hw *);
> 
>  void vtpci_reinit_complete(struct virtio_hw *);
> diff --git a/drivers/net/virtio/virtio_pci_ethdev.c
> b/drivers/net/virtio/virtio_pci_ethdev.c
> index d6cbe582d2..f513381707 100644
> --- a/drivers/net/virtio/virtio_pci_ethdev.c
> +++ b/drivers/net/virtio/virtio_pci_ethdev.c
> @@ -39,9 +39,11 @@ static const struct rte_pci_id pci_id_virtio_map[] = {
>   * could have the PCI initiated correctly.
>   */
>  static int
> -virtio_remap_pci(struct rte_pci_device *pci_dev, struct virtio_hw *hw)
> +virtio_remap_pci(struct rte_pci_device *pci_dev, struct virtio_pci_dev *dev)
>  {
> -	if (hw->bus_type == VIRTIO_BUS_PCI_MODERN) {
> +	struct virtio_hw *hw = &dev->hw;
> +
> +	if (dev->modern) {
>  		/*
>  		 * We don't have to re-parse the PCI config space, since
>  		 * rte_pci_map_device() makes sure the mapped address
> @@ -57,7 +59,7 @@ virtio_remap_pci(struct rte_pci_device *pci_dev, struct
> virtio_hw *hw)
>  			PMD_INIT_LOG(DEBUG, "failed to map pci device!");
>  			return -1;
>  		}
> -	} else if (hw->bus_type == VIRTIO_BUS_PCI_LEGACY) {
> +	} else {
>  		if (rte_pci_ioport_map(pci_dev, 0, VTPCI_IO(hw)) < 0)
>  			return -1;
>  	}
> @@ -74,13 +76,13 @@ eth_virtio_pci_init(struct rte_eth_dev *eth_dev)
>  	int ret;
> 
>  	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
> -		ret = vtpci_init(RTE_ETH_DEV_TO_PCI(eth_dev), hw);
> +		ret = vtpci_init(RTE_ETH_DEV_TO_PCI(eth_dev), dev);
>  		if (ret) {
>  			PMD_INIT_LOG(ERR, "Failed to init PCI device\n");
>  			return -1;
>  		}
>  	} else {
> -		ret = virtio_remap_pci(RTE_ETH_DEV_TO_PCI(eth_dev), hw);
> +		ret = virtio_remap_pci(RTE_ETH_DEV_TO_PCI(eth_dev), dev);
>  		if (ret < 0) {
>  			PMD_INIT_LOG(ERR, "Failed to remap PCI device\n");
>  			return -1;
> --
> 2.29.2

Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>
  
David Marchand Jan. 6, 2021, 9:14 a.m. UTC | #2
On Sun, Dec 20, 2020 at 10:14 PM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
>
> Going further in making the Virtio ethdev layer bus agnostic,
> this patch adds a boolean in the Virtio PCI device metadata.
>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>

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

Patch

diff --git a/drivers/net/virtio/virtio_pci.c b/drivers/net/virtio/virtio_pci.c
index 8605254e53..7f0c066968 100644
--- a/drivers/net/virtio/virtio_pci.c
+++ b/drivers/net/virtio/virtio_pci.c
@@ -687,26 +687,29 @@  virtio_read_caps(struct rte_pci_device *dev, struct virtio_hw *hw)
  * Return 0 on success.
  */
 int
-vtpci_init(struct rte_pci_device *dev, struct virtio_hw *hw)
+vtpci_init(struct rte_pci_device *pci_dev, struct virtio_pci_dev *dev)
 {
+	struct virtio_hw *hw = &dev->hw;
+
 	/*
 	 * Try if we can succeed reading virtio pci caps, which exists
 	 * only on modern pci device. If failed, we fallback to legacy
 	 * virtio handling.
 	 */
-	if (virtio_read_caps(dev, hw) == 0) {
+	if (virtio_read_caps(pci_dev, hw) == 0) {
 		PMD_INIT_LOG(INFO, "modern virtio pci detected.");
 		virtio_hw_internal[hw->port_id].vtpci_ops = &modern_ops;
 		hw->bus_type = VIRTIO_BUS_PCI_MODERN;
+		dev->modern = true;
 		goto msix_detect;
 	}
 
 	PMD_INIT_LOG(INFO, "trying with legacy virtio pci.");
-	if (rte_pci_ioport_map(dev, 0, VTPCI_IO(hw)) < 0) {
-		rte_pci_unmap_device(dev);
-		if (dev->kdrv == RTE_PCI_KDRV_UNKNOWN &&
-		    (!dev->device.devargs ||
-		     dev->device.devargs->bus !=
+	if (rte_pci_ioport_map(pci_dev, 0, VTPCI_IO(hw)) < 0) {
+		rte_pci_unmap_device(pci_dev);
+		if (pci_dev->kdrv == RTE_PCI_KDRV_UNKNOWN &&
+		    (!pci_dev->device.devargs ||
+		     pci_dev->device.devargs->bus !=
 		     rte_bus_find_by_name("pci"))) {
 			PMD_INIT_LOG(INFO,
 				"skip kernel managed virtio device.");
@@ -717,9 +720,10 @@  vtpci_init(struct rte_pci_device *dev, struct virtio_hw *hw)
 
 	virtio_hw_internal[hw->port_id].vtpci_ops = &legacy_ops;
 	hw->bus_type = VIRTIO_BUS_PCI_LEGACY;
+	dev->modern = false;
 
 msix_detect:
-	hw->use_msix = vtpci_msix_detect(dev);
+	hw->use_msix = vtpci_msix_detect(pci_dev);
 
 	return 0;
 }
diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
index 8d3dc0e22e..3e245ed630 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -291,6 +291,7 @@  struct virtio_hw {
 
 struct virtio_pci_dev {
 	struct virtio_hw hw;
+	bool modern;
 };
 
 #define virtio_pci_get_dev(hw) container_of(hw, struct virtio_pci_dev, hw)
@@ -367,7 +368,7 @@  vtpci_packed_queue(struct virtio_hw *hw)
 /*
  * Function declaration from virtio_pci.c
  */
-int vtpci_init(struct rte_pci_device *dev, struct virtio_hw *hw);
+int vtpci_init(struct rte_pci_device *pci_dev, struct virtio_pci_dev *dev);
 void vtpci_reset(struct virtio_hw *);
 
 void vtpci_reinit_complete(struct virtio_hw *);
diff --git a/drivers/net/virtio/virtio_pci_ethdev.c b/drivers/net/virtio/virtio_pci_ethdev.c
index d6cbe582d2..f513381707 100644
--- a/drivers/net/virtio/virtio_pci_ethdev.c
+++ b/drivers/net/virtio/virtio_pci_ethdev.c
@@ -39,9 +39,11 @@  static const struct rte_pci_id pci_id_virtio_map[] = {
  * could have the PCI initiated correctly.
  */
 static int
-virtio_remap_pci(struct rte_pci_device *pci_dev, struct virtio_hw *hw)
+virtio_remap_pci(struct rte_pci_device *pci_dev, struct virtio_pci_dev *dev)
 {
-	if (hw->bus_type == VIRTIO_BUS_PCI_MODERN) {
+	struct virtio_hw *hw = &dev->hw;
+
+	if (dev->modern) {
 		/*
 		 * We don't have to re-parse the PCI config space, since
 		 * rte_pci_map_device() makes sure the mapped address
@@ -57,7 +59,7 @@  virtio_remap_pci(struct rte_pci_device *pci_dev, struct virtio_hw *hw)
 			PMD_INIT_LOG(DEBUG, "failed to map pci device!");
 			return -1;
 		}
-	} else if (hw->bus_type == VIRTIO_BUS_PCI_LEGACY) {
+	} else {
 		if (rte_pci_ioport_map(pci_dev, 0, VTPCI_IO(hw)) < 0)
 			return -1;
 	}
@@ -74,13 +76,13 @@  eth_virtio_pci_init(struct rte_eth_dev *eth_dev)
 	int ret;
 
 	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
-		ret = vtpci_init(RTE_ETH_DEV_TO_PCI(eth_dev), hw);
+		ret = vtpci_init(RTE_ETH_DEV_TO_PCI(eth_dev), dev);
 		if (ret) {
 			PMD_INIT_LOG(ERR, "Failed to init PCI device\n");
 			return -1;
 		}
 	} else {
-		ret = virtio_remap_pci(RTE_ETH_DEV_TO_PCI(eth_dev), hw);
+		ret = virtio_remap_pci(RTE_ETH_DEV_TO_PCI(eth_dev), dev);
 		if (ret < 0) {
 			PMD_INIT_LOG(ERR, "Failed to remap PCI device\n");
 			return -1;