| Message ID | 20201220211405.313012-8-maxime.coquelin@redhat.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Maxime Coquelin |
| Headers | show |
| Series | net/virtio: Virtio PMD rework | expand |
| Context | Check | Description |
|---|---|---|
| ci/checkpatch | success | coding style OK |
> -----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
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; }
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(-)