On 1/14/21 10:24 AM, Maxime Coquelin wrote:

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

quoted

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

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.

Wait, as we discussed off-list, we need hw->bus_type != VIRTIO_BUS_USER,
as the bus_type is still unknown for PCI bus at that stage.

quoted

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.

Same here.

quoted

quoted

                ret = vtpci_init(RTE_ETH_DEV_TO_PCI(eth_dev), hw);

bus_type is set to VIRTIO_BUS_PCI_MODERN or VIRTIO_BUS_PCI_LEGACY in
vtcpi_init().

quoted

quoted

                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: