On 1/5/21 10:15 PM, David Marchand wrote:
> On Sun, Dec 20, 2020 at 10:14 PM Maxime Coquelin
> <maxime.coque...@redhat.com> wrote:
>>
>> 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.
>
>
>> @@ -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.
>
>> - 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.
>
>> 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.
>
>
>> 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:
>
>
