[07/40] net/virtio: move MSIX detection to PCI ethdev
Checks
Commit Message
There is no point it detecting whether we can use MSIX
every time the interrupt is enabled/disabled/masked.
Let's do it once for all at PCI device init time.
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
drivers/net/virtio/virtio_ethdev.c | 15 ---------------
drivers/net/virtio/virtio_pci.c | 5 ++++-
2 files changed, 4 insertions(+), 16 deletions(-)
Comments
> -----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 07/40] net/virtio: move MSIX detection to PCI ethdev
>
> There is no point it detecting whether we can use MSIX
> every time the interrupt is enabled/disabled/masked.
>
> Let's do it once for all at PCI device init time.
>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
> drivers/net/virtio/virtio_ethdev.c | 15 ---------------
> drivers/net/virtio/virtio_pci.c | 5 ++++-
> 2 files changed, 4 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/net/virtio/virtio_ethdev.c
> b/drivers/net/virtio/virtio_ethdev.c
> index 4032a89396..67f6be3fa8 100644
> --- a/drivers/net/virtio/virtio_ethdev.c
> +++ b/drivers/net/virtio/virtio_ethdev.c
> @@ -1282,42 +1282,27 @@ virtio_vlan_filter_set(struct rte_eth_dev *dev,
> uint16_t vlan_id, int on)
> static int
> virtio_intr_unmask(struct rte_eth_dev *dev)
> {
> - struct virtio_hw *hw = dev->data->dev_private;
> -
> if (rte_intr_ack(dev->intr_handle) < 0)
> return -1;
>
> - if (hw->bus_type == VIRTIO_BUS_PCI_LEGACY || hw->bus_type ==
> VIRTIO_BUS_PCI_MODERN)
> - hw->use_msix = vtpci_msix_detect(RTE_ETH_DEV_TO_PCI(dev));
> -
> return 0;
> }
>
> static int
> virtio_intr_enable(struct rte_eth_dev *dev)
> {
> - struct virtio_hw *hw = dev->data->dev_private;
> -
> if (rte_intr_enable(dev->intr_handle) < 0)
> return -1;
>
> - if (hw->bus_type == VIRTIO_BUS_PCI_LEGACY || hw->bus_type ==
> VIRTIO_BUS_PCI_MODERN)
> - hw->use_msix = vtpci_msix_detect(RTE_ETH_DEV_TO_PCI(dev));
> -
> return 0;
> }
>
> static int
> virtio_intr_disable(struct rte_eth_dev *dev)
> {
> - struct virtio_hw *hw = dev->data->dev_private;
> -
> if (rte_intr_disable(dev->intr_handle) < 0)
> return -1;
>
> - if (hw->bus_type == VIRTIO_BUS_PCI_LEGACY || hw->bus_type ==
> VIRTIO_BUS_PCI_MODERN)
> - hw->use_msix = vtpci_msix_detect(RTE_ETH_DEV_TO_PCI(dev));
> -
> return 0;
> }
>
> diff --git a/drivers/net/virtio/virtio_pci.c b/drivers/net/virtio/virtio_pci.c
> index 1692268f30..8605254e53 100644
> --- a/drivers/net/virtio/virtio_pci.c
> +++ b/drivers/net/virtio/virtio_pci.c
> @@ -698,7 +698,7 @@ vtpci_init(struct rte_pci_device *dev, struct virtio_hw
> *hw)
> 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;
> - return 0;
> + goto msix_detect;
> }
>
> PMD_INIT_LOG(INFO, "trying with legacy virtio pci.");
> @@ -718,6 +718,9 @@ 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;
>
> +msix_detect:
> + hw->use_msix = vtpci_msix_detect(dev);
> +
> return 0;
> }
>
> --
> 2.29.2
Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>
On Sun, Dec 20, 2020 at 10:14 PM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
>
> There is no point it detecting whether we can use MSIX
> every time the interrupt is enabled/disabled/masked.
>
> Let's do it once for all at PCI device init time.
>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Is this a rework/fix of fe19d49cb525 ("net/virtio: fix Rx interrupt
with VFIO") ?
On 1/6/21 9:22 AM, David Marchand wrote:
> On Sun, Dec 20, 2020 at 10:14 PM Maxime Coquelin
> <maxime.coquelin@redhat.com> wrote:
>>
>> There is no point it detecting whether we can use MSIX
>> every time the interrupt is enabled/disabled/masked.
>>
>> Let's do it once for all at PCI device init time.
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>
> Is this a rework/fix of fe19d49cb525 ("net/virtio: fix Rx interrupt
> with VFIO") ?
Hmm, that was only planned as a rework, but it is likely to introduce a
regression by reverting the fix you mention.
I think I'll have to introduce a new callback so that drivers can update
their status.
Good catch, thanks!
Maxime
@@ -1282,42 +1282,27 @@ virtio_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on)
static int
virtio_intr_unmask(struct rte_eth_dev *dev)
{
- struct virtio_hw *hw = dev->data->dev_private;
-
if (rte_intr_ack(dev->intr_handle) < 0)
return -1;
- if (hw->bus_type == VIRTIO_BUS_PCI_LEGACY || hw->bus_type == VIRTIO_BUS_PCI_MODERN)
- hw->use_msix = vtpci_msix_detect(RTE_ETH_DEV_TO_PCI(dev));
-
return 0;
}
static int
virtio_intr_enable(struct rte_eth_dev *dev)
{
- struct virtio_hw *hw = dev->data->dev_private;
-
if (rte_intr_enable(dev->intr_handle) < 0)
return -1;
- if (hw->bus_type == VIRTIO_BUS_PCI_LEGACY || hw->bus_type == VIRTIO_BUS_PCI_MODERN)
- hw->use_msix = vtpci_msix_detect(RTE_ETH_DEV_TO_PCI(dev));
-
return 0;
}
static int
virtio_intr_disable(struct rte_eth_dev *dev)
{
- struct virtio_hw *hw = dev->data->dev_private;
-
if (rte_intr_disable(dev->intr_handle) < 0)
return -1;
- if (hw->bus_type == VIRTIO_BUS_PCI_LEGACY || hw->bus_type == VIRTIO_BUS_PCI_MODERN)
- hw->use_msix = vtpci_msix_detect(RTE_ETH_DEV_TO_PCI(dev));
-
return 0;
}
@@ -698,7 +698,7 @@ vtpci_init(struct rte_pci_device *dev, struct virtio_hw *hw)
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;
- return 0;
+ goto msix_detect;
}
PMD_INIT_LOG(INFO, "trying with legacy virtio pci.");
@@ -718,6 +718,9 @@ 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;
+msix_detect:
+ hw->use_msix = vtpci_msix_detect(dev);
+
return 0;
}