[dpdk-dev,2/3] net/virtio: clean up LSC setting
Checks
Commit Message
Clean up LSC setting:
- LSC flag is set in several places, but only the last one takes
effect; so we just keep the last one.
- As we already change to use capability list to detect MSI-X, remove
the redundant MSI-X detection in legacy devices.
Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
drivers/net/virtio/virtio_ethdev.c | 7 ++----
drivers/net/virtio/virtio_pci.c | 48 ++------------------------------------
drivers/net/virtio/virtio_pci.h | 3 +--
3 files changed, 5 insertions(+), 53 deletions(-)
Comments
On Wed, Apr 26, 2017 at 04:52:50AM +0000, Jianfeng Tan wrote:
> Clean up LSC setting:
Firstly, this patch does two things. There should be two patches.
> - LSC flag is set in several places, but only the last one takes
> effect; so we just keep the last one.
> - As we already change to use capability list to detect MSI-X, remove
> the redundant MSI-X detection in legacy devices.
However, there is no capability list for legacy device.
--yliu
> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> Sent: Wednesday, April 26, 2017 1:33 PM
> To: Tan, Jianfeng
> Cc: dev@dpdk.org; maxime.coquelin@redhat.com; thomas@monjalon.net
> Subject: Re: [PATCH 2/3] net/virtio: clean up LSC setting
>
> On Wed, Apr 26, 2017 at 04:52:50AM +0000, Jianfeng Tan wrote:
> > Clean up LSC setting:
>
> Firstly, this patch does two things. There should be two patches.
I just merged them into one to adapt to the name of "clean up" :-)
>
> > - LSC flag is set in several places, but only the last one takes
> > effect; so we just keep the last one.
> > - As we already change to use capability list to detect MSI-X, remove
> > the redundant MSI-X detection in legacy devices.
>
> However, there is no capability list for legacy device.
When I try legacy device on QEMU (disable-modern=true), I did detect MSI-X capability. So does virtio spec mean legacy devices do not have vendor-specific capability list?
Thanks,
Jianfeng
On Wed, Apr 26, 2017 at 05:44:05AM +0000, Tan, Jianfeng wrote:
>
>
> > -----Original Message-----
> > From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> > Sent: Wednesday, April 26, 2017 1:33 PM
> > To: Tan, Jianfeng
> > Cc: dev@dpdk.org; maxime.coquelin@redhat.com; thomas@monjalon.net
> > Subject: Re: [PATCH 2/3] net/virtio: clean up LSC setting
> >
> > On Wed, Apr 26, 2017 at 04:52:50AM +0000, Jianfeng Tan wrote:
> > > Clean up LSC setting:
> >
> > Firstly, this patch does two things. There should be two patches.
>
> I just merged them into one to adapt to the name of "clean up" :-)
I then could merge all bug fix patches into one and name it "fix buges"?
No, please don't do that. It hurts review.
> >
> > > - LSC flag is set in several places, but only the last one takes
> > > effect; so we just keep the last one.
> > > - As we already change to use capability list to detect MSI-X, remove
> > > the redundant MSI-X detection in legacy devices.
> >
> > However, there is no capability list for legacy device.
>
> When I try legacy device on QEMU (disable-modern=true), I did detect MSI-X capability. So does virtio spec mean legacy devices do not have vendor-specific capability list?
Oh, right. I mixed that up.
--yliu
> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> Sent: Wednesday, April 26, 2017 1:52 PM
> To: Tan, Jianfeng
> Cc: dev@dpdk.org; maxime.coquelin@redhat.com; thomas@monjalon.net;
> Michael S. Tsirkin
> Subject: Re: [PATCH 2/3] net/virtio: clean up LSC setting
>
> On Wed, Apr 26, 2017 at 05:44:05AM +0000, Tan, Jianfeng wrote:
> >
> >
> > > -----Original Message-----
> > > From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> > > Sent: Wednesday, April 26, 2017 1:33 PM
> > > To: Tan, Jianfeng
> > > Cc: dev@dpdk.org; maxime.coquelin@redhat.com;
> thomas@monjalon.net
> > > Subject: Re: [PATCH 2/3] net/virtio: clean up LSC setting
> > >
> > > On Wed, Apr 26, 2017 at 04:52:50AM +0000, Jianfeng Tan wrote:
> > > > Clean up LSC setting:
> > >
> > > Firstly, this patch does two things. There should be two patches.
> >
> > I just merged them into one to adapt to the name of "clean up" :-)
>
> I then could merge all bug fix patches into one and name it "fix buges"?
> No, please don't do that. It hurts review.
OK, will fix this in next version.
Thanks,
Jianfeng
On Wed, Apr 26, 2017 at 05:44:05AM +0000, Tan, Jianfeng wrote:
>
>
> > -----Original Message-----
> > From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> > Sent: Wednesday, April 26, 2017 1:33 PM
> > To: Tan, Jianfeng
> > Cc: dev@dpdk.org; maxime.coquelin@redhat.com; thomas@monjalon.net
> > Subject: Re: [PATCH 2/3] net/virtio: clean up LSC setting
> >
> > On Wed, Apr 26, 2017 at 04:52:50AM +0000, Jianfeng Tan wrote:
> > > Clean up LSC setting:
> >
> > Firstly, this patch does two things. There should be two patches.
>
> I just merged them into one to adapt to the name of "clean up" :-)
>
> >
> > > - LSC flag is set in several places, but only the last one takes
> > > effect; so we just keep the last one.
> > > - As we already change to use capability list to detect MSI-X, remove
> > > the redundant MSI-X detection in legacy devices.
> >
> > However, there is no capability list for legacy device.
>
> When I try legacy device on QEMU (disable-modern=true), I did detect MSI-X capability. So does virtio spec mean legacy devices do not have vendor-specific capability list?
>
> Thanks,
> Jianfeng
Yes, as far as I know legacy devices don't have vendor-specific
capability lists.
> -----Original Message-----
> From: Michael S. Tsirkin [mailto:mst@redhat.com]
> Sent: Wednesday, April 26, 2017 11:11 PM
> To: Tan, Jianfeng
> Cc: Yuanhan Liu; dev@dpdk.org; maxime.coquelin@redhat.com;
> thomas@monjalon.net
> Subject: Re: [PATCH 2/3] net/virtio: clean up LSC setting
>
> On Wed, Apr 26, 2017 at 05:44:05AM +0000, Tan, Jianfeng wrote:
> >
> >
> > > -----Original Message-----
> > > From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> > > Sent: Wednesday, April 26, 2017 1:33 PM
> > > To: Tan, Jianfeng
> > > Cc: dev@dpdk.org; maxime.coquelin@redhat.com;
> thomas@monjalon.net
> > > Subject: Re: [PATCH 2/3] net/virtio: clean up LSC setting
> > >
> > > On Wed, Apr 26, 2017 at 04:52:50AM +0000, Jianfeng Tan wrote:
> > > > Clean up LSC setting:
> > >
> > > Firstly, this patch does two things. There should be two patches.
> >
> > I just merged them into one to adapt to the name of "clean up" :-)
> >
> > >
> > > > - LSC flag is set in several places, but only the last one takes
> > > > effect; so we just keep the last one.
> > > > - As we already change to use capability list to detect MSI-X, remove
> > > > the redundant MSI-X detection in legacy devices.
> > >
> > > However, there is no capability list for legacy device.
> >
> > When I try legacy device on QEMU (disable-modern=true), I did detect
> MSI-X capability. So does virtio spec mean legacy devices do not have
> vendor-specific capability list?
> >
> > Thanks,
> > Jianfeng
>
> Yes, as far as I know legacy devices don't have vendor-specific
> capability lists.
>
> --
> MST
Thank you to confirm this, Michael.
Thanks,
Jianfeng
@@ -1353,6 +1353,7 @@ virtio_init_device(struct rte_eth_dev *eth_dev, uint64_t req_features)
rte_eth_copy_pci_info(eth_dev, pci_dev);
}
+ eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
/* If host does not support both status and MSI-X then disable LSC */
if (vtpci_with_feature(hw, VIRTIO_NET_F_STATUS) && hw->use_msix)
eth_dev->data->dev_flags |= RTE_ETH_DEV_INTR_LSC;
@@ -1521,7 +1522,6 @@ int
eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
{
struct virtio_hw *hw = eth_dev->data->dev_private;
- uint32_t dev_flags = RTE_ETH_DEV_DETACHABLE;
int ret;
RTE_BUILD_BUG_ON(RTE_PKTMBUF_HEADROOM < sizeof(struct virtio_net_hdr_mrg_rxbuf));
@@ -1561,14 +1561,11 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
* virtio_user_eth_dev_alloc() before eth_virtio_dev_init() is called.
*/
if (!hw->virtio_user_dev) {
- ret = vtpci_init(RTE_DEV_TO_PCI(eth_dev->device), hw,
- &dev_flags);
+ ret = vtpci_init(RTE_DEV_TO_PCI(eth_dev->device), hw);
if (ret)
return ret;
}
- eth_dev->data->dev_flags = dev_flags;
-
/* reset device and negotiate default features */
ret = virtio_init_device(eth_dev, VIRTIO_PMD_DEFAULT_GUEST_FEATURES);
if (ret < 0)
@@ -279,47 +279,6 @@ legacy_notify_queue(struct virtio_hw *hw, struct virtqueue *vq)
VIRTIO_PCI_QUEUE_NOTIFY);
}
-#ifdef RTE_EXEC_ENV_LINUXAPP
-static int
-legacy_virtio_has_msix(const struct rte_pci_addr *loc)
-{
- DIR *d;
- char dirname[PATH_MAX];
-
- snprintf(dirname, sizeof(dirname),
- "%s/" PCI_PRI_FMT "/msi_irqs", pci_get_sysfs_path(),
- loc->domain, loc->bus, loc->devid, loc->function);
-
- d = opendir(dirname);
- if (d)
- closedir(d);
-
- return d != NULL;
-}
-#else
-static int
-legacy_virtio_has_msix(const struct rte_pci_addr *loc __rte_unused)
-{
- /* nic_uio does not enable interrupts, return 0 (false). */
- return 0;
-}
-#endif
-
-static int
-legacy_virtio_resource_init(struct rte_pci_device *pci_dev,
- struct virtio_hw *hw, uint32_t *dev_flags)
-{
- if (rte_eal_pci_ioport_map(pci_dev, 0, VTPCI_IO(hw)) < 0)
- return -1;
-
- if (pci_dev->intr_handle.type != RTE_INTR_HANDLE_UNKNOWN)
- *dev_flags |= RTE_ETH_DEV_INTR_LSC;
- else
- *dev_flags &= ~RTE_ETH_DEV_INTR_LSC;
-
- return 0;
-}
-
const struct virtio_pci_ops legacy_ops = {
.read_dev_cfg = legacy_read_dev_config,
.write_dev_cfg = legacy_write_dev_config,
@@ -712,8 +671,7 @@ virtio_read_caps(struct rte_pci_device *dev, struct virtio_hw *hw)
* Return 0 on success.
*/
int
-vtpci_init(struct rte_pci_device *dev, struct virtio_hw *hw,
- uint32_t *dev_flags)
+vtpci_init(struct rte_pci_device *dev, struct virtio_hw *hw)
{
/*
* Try if we can succeed reading virtio pci caps, which exists
@@ -724,12 +682,11 @@ 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->modern = 1;
- *dev_flags |= RTE_ETH_DEV_INTR_LSC;
return 0;
}
PMD_INIT_LOG(INFO, "trying with legacy virtio pci.");
- if (legacy_virtio_resource_init(dev, hw, dev_flags) < 0) {
+ if (rte_eal_pci_ioport_map(dev, 0, VTPCI_IO(hw)) < 0) {
if (dev->kdrv == RTE_KDRV_UNKNOWN &&
(!dev->device.devargs ||
dev->device.devargs->type !=
@@ -742,7 +699,6 @@ vtpci_init(struct rte_pci_device *dev, struct virtio_hw *hw,
}
virtio_hw_internal[hw->port_id].vtpci_ops = &legacy_ops;
- hw->use_msix = legacy_virtio_has_msix(&dev->addr);
hw->modern = 0;
return 0;
@@ -321,8 +321,7 @@ vtpci_with_feature(struct virtio_hw *hw, uint64_t bit)
/*
* Function declaration from virtio_pci.c
*/
-int vtpci_init(struct rte_pci_device *, struct virtio_hw *,
- uint32_t *dev_flags);
+int vtpci_init(struct rte_pci_device *, struct virtio_hw *);
void vtpci_reset(struct virtio_hw *);
void vtpci_reinit_complete(struct virtio_hw *);