On 1/5/21 10:15 PM, David Marchand wrote:

On Sun, Dec 20, 2020 at 10:14 PM Maxime Coquelin
[off-list ref] wrote:

quoted

This patch is preliminary work for introducing a bus layer
in Virtio PMD, in order to improve Virtio-user integration.

A new bus type is added to provide a unified way to distinguish
which bus type is used (PCI modern, PCI legacy or Virtio-user).

In dpdk, we don't use a capital letter for starting a title.

quoted

@@ -1883,15 +1883,14 @@ virtio_remap_pci(struct rte_pci_device *pci_dev, struct virtio_hw *hw)
 static void
 virtio_set_vtpci_ops(struct virtio_hw *hw)
 {
-#ifdef RTE_VIRTIO_USER

Too soon to remove this check, since virtio_user_ops comes from
virtio_user_ethdev.c.
This will break compilation on FreeBSD.

Agree, I restored it back in v2.

quoted

-       if (hw->virtio_user_dev)
+       if (hw->bus_type == VIRTIO_BUS_USER)
                VTPCI_OPS(hw) = &virtio_user_ops;
-       else
-#endif
-       if (hw->modern)
+       else if (hw->bus_type == VIRTIO_BUS_PCI_MODERN)
                VTPCI_OPS(hw) = &modern_ops;
-       else
+       else if (hw->bus_type == VIRTIO_BUS_PCI_LEGACY)
                VTPCI_OPS(hw) = &legacy_ops;
+
+       return;
 }

 /*

@@ -1919,7 +1918,7 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
        eth_dev->rx_descriptor_done = virtio_dev_rx_queue_done;

        if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
-               if (!hw->virtio_user_dev) {
+               if (hw->bus_type != VIRTIO_BUS_USER) {

In the rest of the patch, we check for PCI types when dealing with PCI
code, so I'd rather be consistent and check for modern and legacy pci
types here too.

Agree, it is not consistent. I fixed it here and below for v2.

quoted

                        ret = virtio_remap_pci(RTE_ETH_DEV_TO_PCI(eth_dev), hw);
                        if (ret)
                                return ret;

@@ -1950,7 +1949,7 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
        /* For virtio_user case the hw->virtio_user_dev is populated by
         * virtio_user_eth_dev_alloc() before eth_virtio_dev_init() is called.
         */
-       if (!hw->virtio_user_dev) {
+       if (hw->bus_type != VIRTIO_BUS_USER) {

Idem.

quoted

                ret = vtpci_init(RTE_ETH_DEV_TO_PCI(eth_dev), hw);
                if (ret)
                        goto err_vtpci_init;

@@ -1982,9 +1981,9 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
        return 0;

 err_virtio_init:
-       if (!hw->virtio_user_dev) {
+       if (hw->bus_type == VIRTIO_BUS_PCI_MODERN || hw->bus_type == VIRTIO_BUS_PCI_LEGACY) {
                rte_pci_unmap_device(RTE_ETH_DEV_TO_PCI(eth_dev));
-               if (!hw->modern)
+               if (hw->bus_type == VIRTIO_BUS_PCI_LEGACY)
                        rte_pci_ioport_unmap(VTPCI_IO(hw));
        }
 err_vtpci_init: