Found kernel try to load mlx4 drivers for VFs before
PF's is loaded when the drivers are built-in, and kernel
command line include probe_vfs=63, num_vfs=63.
[ 169.581682] calling mlx4_init+0x0/0x119 @ 1
[ 169.595681] mlx4_core: Mellanox ConnectX core driver v1.1 (Dec, 2011)
[ 169.600194] mlx4_core: Initializing 0000:02:00.0
[ 169.616322] mlx4_core 0000:02:00.0: Enabling SR-IOV with 63 VFs
[ 169.724084] pci 0000:02:00.1: [15b3:1002] type 00 class 0x0c0600
[ 169.732442] mlx4_core: Initializing 0000:02:00.1
[ 169.734345] mlx4_core 0000:02:00.1: enabling device (0000 -> 0002)
[ 169.747060] mlx4_core 0000:02:00.1: enabling bus mastering
[ 169.764283] mlx4_core 0000:02:00.1: Detected virtual function - running in slave mode
[ 169.767409] mlx4_core 0000:02:00.1: with iommu 3 : domain 11
[ 169.785589] mlx4_core 0000:02:00.1: Sending reset
[ 179.790131] mlx4_core 0000:02:00.1: Got slave FLRed from Communication channel (ret:0x1)
[ 181.798661] mlx4_core 0000:02:00.1: slave is currently in themiddle of FLR. retrying...(try num:1)
[ 181.803336] mlx4_core 0000:02:00.1: Communication channel is not idle.my toggle is 1 (cmd:0x0)
...
[ 182.078710] mlx4_core 0000:02:00.1: slave is currently in themiddle of FLR. retrying...(try num:10)
[ 182.096657] mlx4_core 0000:02:00.1: Communication channel is not idle.my toggle is 1 (cmd:0x0)
[ 182.104935] mlx4_core 0000:02:00.1: slave driver version is not supported by the master
[ 182.118570] mlx4_core 0000:02:00.1: Communication channel is not idle.my toggle is 1 (cmd:0x0)
[ 182.138190] mlx4_core 0000:02:00.1: Failed to initialize slave
[ 182.141728] mlx4_core: probe of 0000:02:00.1 failed with error -5
It turns that this also happen for hotadd path even drivers are
compiled as modules and if they are loaded. Esp some VF share the
same driver with PF.
calling path:
device driver probe
==> pci_enable_sriov
==> virtfn_add
==> pci_dev_add
==> pci_bus_device_add
when pci_bus_device_add is called, the VF's driver will be attached.
and at that time PF's driver does not finish yet.
Need to move out pci_bus_device_add from virtfn_add and call it
later.
bnx2x and qlcnic are ok, because it does not modules command line
to enable sriov. They must use sysfs to enable it.
be2net is ok, according to Sathya Perla,
he fixed this issue in be2net with the following patch (commit b4c1df93)
http://marc.info/?l=linux-netdev&m=136801459808765&w=2
For igb and ixgbe is ok, as Alex Duyck said:
| The VF driver should be able to be loaded when the PF driver is not
| present. We handle it in igb and ixgbe last I checked, and I don't see
| any reason why it cannot be handled in all other VF drivers. I'm not
| saying the VF has to be able to fully functional, but it should be able
| to detect the PF becoming enabled and then bring itself to a fully
| functional state. To not handle that case is a bug.
Looks like the patch will help enic, mlx4, efx, vxge and lpfc now.
-v2: don't use schedule_callback, and initcall after Alex's patch.
pci: Avoid reentrant calls to work_on_cpu
Signed-off-by: Yinghai Lu <[email protected]>
Cc: Alexander Duyck <[email protected]>
Cc: Yan Burman <[email protected]>
Cc: Sathya Perla <[email protected]>
Cc: [email protected]
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c | 7 ++
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 4 +
drivers/net/ethernet/cisco/enic/enic_main.c | 2
drivers/net/ethernet/emulex/benet/be_main.c | 4 +
drivers/net/ethernet/intel/igb/igb_main.c | 11 +++-
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2
drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 9 ++-
drivers/net/ethernet/mellanox/mlx4/main.c | 3 +
drivers/net/ethernet/neterion/vxge/vxge-main.c | 3 +
drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_pf.c | 5 +-
drivers/net/ethernet/sfc/efx.c | 1
drivers/pci/iov.c | 47 ++++++++++++++++---
drivers/scsi/lpfc/lpfc_init.c | 2
include/linux/pci.h | 4 +
14 files changed, 90 insertions(+), 14 deletions(-)
Index: linux-2.6/drivers/net/ethernet/mellanox/mlx4/main.c
===================================================================
--- linux-2.6.orig/drivers/net/ethernet/mellanox/mlx4/main.c
+++ linux-2.6/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -2308,6 +2308,9 @@ slave_start:
priv->pci_dev_data = pci_dev_data;
pci_set_drvdata(pdev, dev);
+ if (dev->flags & MLX4_FLAG_SRIOV)
+ pci_bus_add_device_vfs(pdev);
+
return 0;
err_port:
Index: linux-2.6/drivers/pci/iov.c
===================================================================
--- linux-2.6.orig/drivers/pci/iov.c
+++ linux-2.6/drivers/pci/iov.c
@@ -66,7 +66,8 @@ static void virtfn_remove_bus(struct pci
pci_remove_bus(child);
}
-static int virtfn_add(struct pci_dev *dev, int id, int reset)
+static int virtfn_add(struct pci_dev *dev, int id, int reset,
+ struct pci_dev **ret)
{
int i;
int rc;
@@ -116,7 +117,6 @@ static int virtfn_add(struct pci_dev *de
pci_device_add(virtfn, virtfn->bus);
mutex_unlock(&iov->dev->sriov->lock);
- rc = pci_bus_add_device(virtfn);
sprintf(buf, "virtfn%u", id);
rc = sysfs_create_link(&dev->dev.kobj, &virtfn->dev.kobj, buf);
if (rc)
@@ -127,6 +127,8 @@ static int virtfn_add(struct pci_dev *de
kobject_uevent(&virtfn->dev.kobj, KOBJ_CHANGE);
+ if (ret)
+ *ret = virtfn;
return 0;
failed2:
@@ -141,6 +143,26 @@ failed1:
return rc;
}
+void pci_bus_add_device_vfs(struct pci_dev *pdev)
+{
+ int rc;
+ struct pci_dev *dev;
+
+ /* only search if it is a PF */
+ if (!pdev->is_physfn)
+ return;
+
+ /* loop through all the VFs to see if we own and is not added yet*/
+ dev = pci_get_device(pdev->vendor, PCI_ANY_ID, NULL);
+ while (dev) {
+ if (dev->is_virtfn && dev->physfn == pdev && !dev->is_added)
+ rc = pci_bus_add_device(dev);
+
+ dev = pci_get_device(pdev->vendor, PCI_ANY_ID, dev);
+ }
+}
+EXPORT_SYMBOL_GPL(pci_bus_add_device_vfs);
+
static void virtfn_remove(struct pci_dev *dev, int id, int reset)
{
char buf[VIRTFN_ID_LEN];
@@ -204,6 +226,7 @@ static int sriov_migration(struct pci_de
static void sriov_migration_task(struct work_struct *work)
{
int i;
+ int rc;
u8 state;
u16 status;
struct pci_sriov *iov = container_of(work, struct pci_sriov, mtask);
@@ -213,14 +236,24 @@ static void sriov_migration_task(struct
if (state == PCI_SRIOV_VFM_MI) {
writeb(PCI_SRIOV_VFM_AV, iov->mstate + i);
state = readb(iov->mstate + i);
- if (state == PCI_SRIOV_VFM_AV)
- virtfn_add(iov->self, i, 1);
+ if (state == PCI_SRIOV_VFM_AV) {
+ struct pci_dev *virtfn = NULL;
+
+ virtfn_add(iov->self, i, 1, &virtfn);
+ if (virtfn)
+ rc = pci_bus_add_device(virtfn);
+ }
} else if (state == PCI_SRIOV_VFM_MO) {
virtfn_remove(iov->self, i, 1);
writeb(PCI_SRIOV_VFM_UA, iov->mstate + i);
state = readb(iov->mstate + i);
- if (state == PCI_SRIOV_VFM_AV)
- virtfn_add(iov->self, i, 0);
+ if (state == PCI_SRIOV_VFM_AV) {
+ struct pci_dev *virtfn = NULL;
+
+ virtfn_add(iov->self, i, 0, &virtfn);
+ if (virtfn)
+ rc = pci_bus_add_device(virtfn);
+ }
}
}
@@ -356,7 +389,7 @@ static int sriov_enable(struct pci_dev *
initial = nr_virtfn;
for (i = 0; i < initial; i++) {
- rc = virtfn_add(dev, i, 0);
+ rc = virtfn_add(dev, i, 0, NULL);
if (rc)
goto failed;
}
Index: linux-2.6/include/linux/pci.h
===================================================================
--- linux-2.6.orig/include/linux/pci.h
+++ linux-2.6/include/linux/pci.h
@@ -1659,6 +1659,7 @@ void __iomem *pci_ioremap_bar(struct pci
#ifdef CONFIG_PCI_IOV
int pci_enable_sriov(struct pci_dev *dev, int nr_virtfn);
+void pci_bus_add_device_vfs(struct pci_dev *dev);
void pci_disable_sriov(struct pci_dev *dev);
irqreturn_t pci_sriov_migration(struct pci_dev *dev);
int pci_num_vf(struct pci_dev *dev);
@@ -1670,6 +1671,9 @@ static inline int pci_enable_sriov(struc
{
return -ENODEV;
}
+static inline void pci_bus_add_device_vfs(struct pci_dev *dev)
+{
+}
static inline void pci_disable_sriov(struct pci_dev *dev)
{
}
Index: linux-2.6/drivers/net/ethernet/intel/igb/igb_main.c
===================================================================
--- linux-2.6.orig/drivers/net/ethernet/intel/igb/igb_main.c
+++ linux-2.6/drivers/net/ethernet/intel/igb/igb_main.c
@@ -2366,6 +2366,9 @@ static int igb_probe(struct pci_dev *pde
}
pm_runtime_put_noidle(&pdev->dev);
+
+ pci_bus_add_device_vfs(pdev);
+
return 0;
err_register:
@@ -7278,8 +7281,12 @@ static int igb_pci_sriov_configure(struc
#ifdef CONFIG_PCI_IOV
if (num_vfs == 0)
return igb_pci_disable_sriov(dev);
- else
- return igb_pci_enable_sriov(dev, num_vfs);
+ else {
+ int ret = igb_pci_enable_sriov(dev, num_vfs);
+ if (ret > 0)
+ pci_bus_add_device_vfs(dev);
+ return ret;
+ }
#endif
return 0;
}
Index: linux-2.6/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
===================================================================
--- linux-2.6.orig/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ linux-2.6/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -346,8 +346,13 @@ int ixgbe_pci_sriov_configure(struct pci
{
if (num_vfs == 0)
return ixgbe_pci_sriov_disable(dev);
- else
- return ixgbe_pci_sriov_enable(dev, num_vfs);
+ else {
+ int ret = ixgbe_pci_sriov_enable(dev, num_vfs);
+
+ if (ret > 0)
+ pci_bus_add_device_vfs(dev);
+ return ret;
+ }
}
static int ixgbe_set_vf_multicasts(struct ixgbe_adapter *adapter,
Index: linux-2.6/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
===================================================================
--- linux-2.6.orig/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ linux-2.6/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -7658,6 +7658,8 @@ skip_sriov:
IXGBE_LINK_SPEED_10GB_FULL | IXGBE_LINK_SPEED_1GB_FULL,
true);
+ pci_bus_add_device_vfs(pdev);
+
return 0;
err_register:
Index: linux-2.6/drivers/scsi/lpfc/lpfc_init.c
===================================================================
--- linux-2.6.orig/drivers/scsi/lpfc/lpfc_init.c
+++ linux-2.6/drivers/scsi/lpfc/lpfc_init.c
@@ -10582,6 +10582,8 @@ lpfc_pci_probe_one(struct pci_dev *pdev,
else
rc = lpfc_pci_probe_one_s3(pdev, pid);
+ pci_bus_add_device_vfs(pdev);
+
return rc;
}
Index: linux-2.6/drivers/net/ethernet/emulex/benet/be_main.c
===================================================================
--- linux-2.6.orig/drivers/net/ethernet/emulex/benet/be_main.c
+++ linux-2.6/drivers/net/ethernet/emulex/benet/be_main.c
@@ -4111,6 +4111,7 @@ static int lancer_recover_func(struct be
if (status)
goto err;
+ pci_bus_add_device_vfs(adapter->pdev);
if (netif_running(adapter->netdev)) {
status = be_open(adapter->netdev);
if (status)
@@ -4327,6 +4328,8 @@ static int be_probe(struct pci_dev *pdev
dev_info(&pdev->dev, "%s: %s %s port %c\n", nic_name(pdev),
func_name(adapter), mc_name(adapter), port_name);
+ pci_bus_add_device_vfs(pdev);
+
return 0;
unsetup:
@@ -4398,6 +4401,7 @@ static int be_resume(struct pci_dev *pde
rtnl_unlock();
}
+ pci_bus_add_device_vfs(adapter->pdev);
schedule_delayed_work(&adapter->func_recovery_work,
msecs_to_jiffies(1000));
netif_device_attach(netdev);
Index: linux-2.6/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_pf.c
===================================================================
--- linux-2.6.orig/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_pf.c
+++ linux-2.6/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_pf.c
@@ -568,8 +568,11 @@ int qlcnic_pci_sriov_configure(struct pc
if (num_vfs == 0)
err = qlcnic_pci_sriov_disable(adapter);
- else
+ else {
err = qlcnic_pci_sriov_enable(adapter, num_vfs);
+ if (err > 0)
+ pci_bus_add_device_vfs(dev);
+ }
clear_bit(__QLCNIC_RESETTING, &adapter->state);
return err;
Index: linux-2.6/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c
===================================================================
--- linux-2.6.orig/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c
+++ linux-2.6/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c
@@ -3048,7 +3048,12 @@ int bnx2x_sriov_configure(struct pci_dev
pci_disable_sriov(dev);
return 0;
} else {
- return bnx2x_enable_sriov(bp);
+ int ret = bnx2x_enable_sriov(bp);
+
+ if (ret > 0)
+ pci_bus_add_device_vfs(dev);
+
+ return 0;
}
}
Index: linux-2.6/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
===================================================================
--- linux-2.6.orig/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ linux-2.6/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -5749,10 +5749,12 @@ static int init_one(struct pci_dev *pdev
sriov:
#ifdef CONFIG_PCI_IOV
if (func < ARRAY_SIZE(num_vf) && num_vf[func] > 0)
- if (pci_enable_sriov(pdev, num_vf[func]) == 0)
+ if (pci_enable_sriov(pdev, num_vf[func]) == 0) {
dev_info(&pdev->dev,
"instantiated %u virtual functions\n",
num_vf[func]);
+ pci_bus_add_device_vfs(pdev);
+ }
#endif
return 0;
Index: linux-2.6/drivers/net/ethernet/cisco/enic/enic_main.c
===================================================================
--- linux-2.6.orig/drivers/net/ethernet/cisco/enic/enic_main.c
+++ linux-2.6/drivers/net/ethernet/cisco/enic/enic_main.c
@@ -2524,6 +2524,8 @@ static int enic_probe(struct pci_dev *pd
goto err_out_dev_deinit;
}
+ pci_bus_add_device_vfs(pdev);
+
return 0;
err_out_dev_deinit:
Index: linux-2.6/drivers/net/ethernet/neterion/vxge/vxge-main.c
===================================================================
--- linux-2.6.orig/drivers/net/ethernet/neterion/vxge/vxge-main.c
+++ linux-2.6/drivers/net/ethernet/neterion/vxge/vxge-main.c
@@ -4731,6 +4731,9 @@ vxge_probe(struct pci_dev *pdev, const s
vxge_hw_device_trace_level_get(hldev));
kfree(ll_config);
+
+ pci_bus_add_device_vfs(pdev);
+
return 0;
_exit6:
Index: linux-2.6/drivers/net/ethernet/sfc/efx.c
===================================================================
--- linux-2.6.orig/drivers/net/ethernet/sfc/efx.c
+++ linux-2.6/drivers/net/ethernet/sfc/efx.c
@@ -2824,6 +2824,7 @@ static int efx_pci_probe(struct pci_dev
netif_warn(efx, probe, efx->net_dev,
"pci_enable_pcie_error_reporting failed (%d)\n", rc);
+ pci_bus_add_device_vfs(pci_dev);
return 0;
fail4:
On 05/14/2013 07:48 PM, Yinghai Lu wrote:
> Found kernel try to load mlx4 drivers for VFs before
> PF's is loaded when the drivers are built-in, and kernel
> command line include probe_vfs=63, num_vfs=63.
>
> [ 169.581682] calling mlx4_init+0x0/0x119 @ 1
> [ 169.595681] mlx4_core: Mellanox ConnectX core driver v1.1 (Dec, 2011)
> [ 169.600194] mlx4_core: Initializing 0000:02:00.0
> [ 169.616322] mlx4_core 0000:02:00.0: Enabling SR-IOV with 63 VFs
> [ 169.724084] pci 0000:02:00.1: [15b3:1002] type 00 class 0x0c0600
> [ 169.732442] mlx4_core: Initializing 0000:02:00.1
> [ 169.734345] mlx4_core 0000:02:00.1: enabling device (0000 -> 0002)
> [ 169.747060] mlx4_core 0000:02:00.1: enabling bus mastering
> [ 169.764283] mlx4_core 0000:02:00.1: Detected virtual function - running in slave mode
> [ 169.767409] mlx4_core 0000:02:00.1: with iommu 3 : domain 11
> [ 169.785589] mlx4_core 0000:02:00.1: Sending reset
> [ 179.790131] mlx4_core 0000:02:00.1: Got slave FLRed from Communication channel (ret:0x1)
> [ 181.798661] mlx4_core 0000:02:00.1: slave is currently in themiddle of FLR. retrying...(try num:1)
> [ 181.803336] mlx4_core 0000:02:00.1: Communication channel is not idle.my toggle is 1 (cmd:0x0)
> ...
> [ 182.078710] mlx4_core 0000:02:00.1: slave is currently in themiddle of FLR. retrying...(try num:10)
> [ 182.096657] mlx4_core 0000:02:00.1: Communication channel is not idle.my toggle is 1 (cmd:0x0)
> [ 182.104935] mlx4_core 0000:02:00.1: slave driver version is not supported by the master
> [ 182.118570] mlx4_core 0000:02:00.1: Communication channel is not idle.my toggle is 1 (cmd:0x0)
> [ 182.138190] mlx4_core 0000:02:00.1: Failed to initialize slave
> [ 182.141728] mlx4_core: probe of 0000:02:00.1 failed with error -5
>
> It turns that this also happen for hotadd path even drivers are
> compiled as modules and if they are loaded. Esp some VF share the
> same driver with PF.
>
> calling path:
> device driver probe
> ==> pci_enable_sriov
> ==> virtfn_add
> ==> pci_dev_add
> ==> pci_bus_device_add
> when pci_bus_device_add is called, the VF's driver will be attached.
> and at that time PF's driver does not finish yet.
>
> Need to move out pci_bus_device_add from virtfn_add and call it
> later.
>
> bnx2x and qlcnic are ok, because it does not modules command line
> to enable sriov. They must use sysfs to enable it.
>
> be2net is ok, according to Sathya Perla,
> he fixed this issue in be2net with the following patch (commit b4c1df93)
> http://marc.info/?l=linux-netdev&m=136801459808765&w=2
>
> For igb and ixgbe is ok, as Alex Duyck said:
> | The VF driver should be able to be loaded when the PF driver is not
> | present. We handle it in igb and ixgbe last I checked, and I don't see
> | any reason why it cannot be handled in all other VF drivers. I'm not
> | saying the VF has to be able to fully functional, but it should be able
> | to detect the PF becoming enabled and then bring itself to a fully
> | functional state. To not handle that case is a bug.
>
> Looks like the patch will help enic, mlx4, efx, vxge and lpfc now.
>
> -v2: don't use schedule_callback, and initcall after Alex's patch.
> pci: Avoid reentrant calls to work_on_cpu
>
> Signed-off-by: Yinghai Lu <[email protected]>
> Cc: Alexander Duyck <[email protected]>
> Cc: Yan Burman <[email protected]>
> Cc: Sathya Perla <[email protected]>
> Cc: [email protected]
>
This is a driver bug in mlx4 and possibly a few others, not a bug in the
SR-IOV code. My concern is your patch may introduce issues in all of
the drivers, especially the ones that don't need this workaround.
Fixing the kernel to make this work is just encouraging a poor design model.
The problem is the mlx4 driver is enabling SR-IOV before it is ready to
support VFs. The mlx4 driver should probably be fixed by either,
changing over to sysfs, provisioning the resources before enabling
SR-IOV like be2net, or via the igb/ixgbe approach where the VF
gracefully handles the PF not being present.
Thanks,
Alex
On Tue, 14 May 2013 19:48:22 -0700
Yinghai Lu <[email protected]> wrote:
> Found kernel try to load mlx4 drivers for VFs before
> PF's is loaded when the drivers are built-in, and kernel
> command line include probe_vfs=63, num_vfs=63.
>
> [ 169.581682] calling mlx4_init+0x0/0x119 @ 1
> [ 169.595681] mlx4_core: Mellanox ConnectX core driver v1.1 (Dec,
> 2011) [ 169.600194] mlx4_core: Initializing 0000:02:00.0
> [ 169.616322] mlx4_core 0000:02:00.0: Enabling SR-IOV with 63 VFs
> [ 169.724084] pci 0000:02:00.1: [15b3:1002] type 00 class 0x0c0600
> [ 169.732442] mlx4_core: Initializing 0000:02:00.1
> [ 169.734345] mlx4_core 0000:02:00.1: enabling device (0000 -> 0002)
> [ 169.747060] mlx4_core 0000:02:00.1: enabling bus mastering
> [ 169.764283] mlx4_core 0000:02:00.1: Detected virtual function -
> running in slave mode [ 169.767409] mlx4_core 0000:02:00.1: with
> iommu 3 : domain 11 [ 169.785589] mlx4_core 0000:02:00.1: Sending
> reset [ 179.790131] mlx4_core 0000:02:00.1: Got slave FLRed from
> Communication channel (ret:0x1) [ 181.798661] mlx4_core
> 0000:02:00.1: slave is currently in themiddle of FLR. retrying...(try
> num:1) [ 181.803336] mlx4_core 0000:02:00.1: Communication channel
> is not idle.my toggle is 1 (cmd:0x0) ... [ 182.078710] mlx4_core
> 0000:02:00.1: slave is currently in themiddle of FLR. retrying...(try
> num:10) [ 182.096657] mlx4_core 0000:02:00.1: Communication channel
> is not idle.my toggle is 1 (cmd:0x0) [ 182.104935] mlx4_core
> 0000:02:00.1: slave driver version is not supported by the master
> [ 182.118570] mlx4_core 0000:02:00.1: Communication channel is not
> idle.my toggle is 1 (cmd:0x0) [ 182.138190] mlx4_core 0000:02:00.1:
> Failed to initialize slave [ 182.141728] mlx4_core: probe of
> 0000:02:00.1 failed with error -5
>
> It turns that this also happen for hotadd path even drivers are
> compiled as modules and if they are loaded. Esp some VF share the
> same driver with PF.
>
> calling path:
> device driver probe
> ==> pci_enable_sriov
> ==> virtfn_add
> ==> pci_dev_add
> ==> pci_bus_device_add
> when pci_bus_device_add is called, the VF's driver will be attached.
> and at that time PF's driver does not finish yet.
>
> Need to move out pci_bus_device_add from virtfn_add and call it
> later.
>
> bnx2x and qlcnic are ok, because it does not modules command line
> to enable sriov. They must use sysfs to enable it.
>
> be2net is ok, according to Sathya Perla,
> he fixed this issue in be2net with the following patch (commit
> b4c1df93) http://marc.info/?l=linux-netdev&m=136801459808765&w=2
>
> For igb and ixgbe is ok, as Alex Duyck said:
> | The VF driver should be able to be loaded when the PF driver is not
> | present. We handle it in igb and ixgbe last I checked, and I don't
> see | any reason why it cannot be handled in all other VF drivers.
> I'm not | saying the VF has to be able to fully functional, but it
> should be able | to detect the PF becoming enabled and then bring
> itself to a fully | functional state. To not handle that case is a
> bug.
>
> Looks like the patch will help enic, mlx4, efx, vxge and lpfc now.
>
> -v2: don't use schedule_callback, and initcall after Alex's patch.
> pci: Avoid reentrant calls to work_on_cpu
>
> Signed-off-by: Yinghai Lu <[email protected]>
> Cc: Alexander Duyck <[email protected]>
> Cc: Yan Burman <[email protected]>
> Cc: Sathya Perla <[email protected]>
> Cc: [email protected]
I'm really not a fan of this. Seems to me the tail is wagging the dog
here. Fix the driver to work without a PF driver being present.
- Greg
>
> ---
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c | 7 ++
> drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 4 +
> drivers/net/ethernet/cisco/enic/enic_main.c | 2
> drivers/net/ethernet/emulex/benet/be_main.c | 4 +
> drivers/net/ethernet/intel/igb/igb_main.c | 11 +++-
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2
> drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 9 ++-
> drivers/net/ethernet/mellanox/mlx4/main.c | 3 +
> drivers/net/ethernet/neterion/vxge/vxge-main.c | 3 +
> drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_pf.c | 5 +-
> drivers/net/ethernet/sfc/efx.c | 1
> drivers/pci/iov.c | 47
> ++++++++++++++++---
> drivers/scsi/lpfc/lpfc_init.c | 2
> include/linux/pci.h | 4 + 14
> files changed, 90 insertions(+), 14 deletions(-)
>
> Index: linux-2.6/drivers/net/ethernet/mellanox/mlx4/main.c
> ===================================================================
> --- linux-2.6.orig/drivers/net/ethernet/mellanox/mlx4/main.c
> +++ linux-2.6/drivers/net/ethernet/mellanox/mlx4/main.c
> @@ -2308,6 +2308,9 @@ slave_start:
> priv->pci_dev_data = pci_dev_data;
> pci_set_drvdata(pdev, dev);
>
> + if (dev->flags & MLX4_FLAG_SRIOV)
> + pci_bus_add_device_vfs(pdev);
> +
> return 0;
>
> err_port:
> Index: linux-2.6/drivers/pci/iov.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/iov.c
> +++ linux-2.6/drivers/pci/iov.c
> @@ -66,7 +66,8 @@ static void virtfn_remove_bus(struct pci
> pci_remove_bus(child);
> }
>
> -static int virtfn_add(struct pci_dev *dev, int id, int reset)
> +static int virtfn_add(struct pci_dev *dev, int id, int reset,
> + struct pci_dev **ret)
> {
> int i;
> int rc;
> @@ -116,7 +117,6 @@ static int virtfn_add(struct pci_dev *de
> pci_device_add(virtfn, virtfn->bus);
> mutex_unlock(&iov->dev->sriov->lock);
>
> - rc = pci_bus_add_device(virtfn);
> sprintf(buf, "virtfn%u", id);
> rc = sysfs_create_link(&dev->dev.kobj, &virtfn->dev.kobj,
> buf); if (rc)
> @@ -127,6 +127,8 @@ static int virtfn_add(struct pci_dev *de
>
> kobject_uevent(&virtfn->dev.kobj, KOBJ_CHANGE);
>
> + if (ret)
> + *ret = virtfn;
> return 0;
>
> failed2:
> @@ -141,6 +143,26 @@ failed1:
> return rc;
> }
>
> +void pci_bus_add_device_vfs(struct pci_dev *pdev)
> +{
> + int rc;
> + struct pci_dev *dev;
> +
> + /* only search if it is a PF */
> + if (!pdev->is_physfn)
> + return;
> +
> + /* loop through all the VFs to see if we own and is not
> added yet*/
> + dev = pci_get_device(pdev->vendor, PCI_ANY_ID, NULL);
> + while (dev) {
> + if (dev->is_virtfn && dev->physfn == pdev
> && !dev->is_added)
> + rc = pci_bus_add_device(dev);
> +
> + dev = pci_get_device(pdev->vendor, PCI_ANY_ID, dev);
> + }
> +}
> +EXPORT_SYMBOL_GPL(pci_bus_add_device_vfs);
> +
> static void virtfn_remove(struct pci_dev *dev, int id, int reset)
> {
> char buf[VIRTFN_ID_LEN];
> @@ -204,6 +226,7 @@ static int sriov_migration(struct pci_de
> static void sriov_migration_task(struct work_struct *work)
> {
> int i;
> + int rc;
> u8 state;
> u16 status;
> struct pci_sriov *iov = container_of(work, struct pci_sriov,
> mtask); @@ -213,14 +236,24 @@ static void sriov_migration_task(struct
> if (state == PCI_SRIOV_VFM_MI) {
> writeb(PCI_SRIOV_VFM_AV, iov->mstate + i);
> state = readb(iov->mstate + i);
> - if (state == PCI_SRIOV_VFM_AV)
> - virtfn_add(iov->self, i, 1);
> + if (state == PCI_SRIOV_VFM_AV) {
> + struct pci_dev *virtfn = NULL;
> +
> + virtfn_add(iov->self, i, 1, &virtfn);
> + if (virtfn)
> + rc =
> pci_bus_add_device(virtfn);
> + }
> } else if (state == PCI_SRIOV_VFM_MO) {
> virtfn_remove(iov->self, i, 1);
> writeb(PCI_SRIOV_VFM_UA, iov->mstate + i);
> state = readb(iov->mstate + i);
> - if (state == PCI_SRIOV_VFM_AV)
> - virtfn_add(iov->self, i, 0);
> + if (state == PCI_SRIOV_VFM_AV) {
> + struct pci_dev *virtfn = NULL;
> +
> + virtfn_add(iov->self, i, 0, &virtfn);
> + if (virtfn)
> + rc =
> pci_bus_add_device(virtfn);
> + }
> }
> }
>
> @@ -356,7 +389,7 @@ static int sriov_enable(struct pci_dev *
> initial = nr_virtfn;
>
> for (i = 0; i < initial; i++) {
> - rc = virtfn_add(dev, i, 0);
> + rc = virtfn_add(dev, i, 0, NULL);
> if (rc)
> goto failed;
> }
> Index: linux-2.6/include/linux/pci.h
> ===================================================================
> --- linux-2.6.orig/include/linux/pci.h
> +++ linux-2.6/include/linux/pci.h
> @@ -1659,6 +1659,7 @@ void __iomem *pci_ioremap_bar(struct pci
>
> #ifdef CONFIG_PCI_IOV
> int pci_enable_sriov(struct pci_dev *dev, int nr_virtfn);
> +void pci_bus_add_device_vfs(struct pci_dev *dev);
> void pci_disable_sriov(struct pci_dev *dev);
> irqreturn_t pci_sriov_migration(struct pci_dev *dev);
> int pci_num_vf(struct pci_dev *dev);
> @@ -1670,6 +1671,9 @@ static inline int pci_enable_sriov(struc
> {
> return -ENODEV;
> }
> +static inline void pci_bus_add_device_vfs(struct pci_dev *dev)
> +{
> +}
> static inline void pci_disable_sriov(struct pci_dev *dev)
> {
> }
> Index: linux-2.6/drivers/net/ethernet/intel/igb/igb_main.c
> ===================================================================
> --- linux-2.6.orig/drivers/net/ethernet/intel/igb/igb_main.c
> +++ linux-2.6/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -2366,6 +2366,9 @@ static int igb_probe(struct pci_dev *pde
> }
>
> pm_runtime_put_noidle(&pdev->dev);
> +
> + pci_bus_add_device_vfs(pdev);
> +
> return 0;
>
> err_register:
> @@ -7278,8 +7281,12 @@ static int igb_pci_sriov_configure(struc
> #ifdef CONFIG_PCI_IOV
> if (num_vfs == 0)
> return igb_pci_disable_sriov(dev);
> - else
> - return igb_pci_enable_sriov(dev, num_vfs);
> + else {
> + int ret = igb_pci_enable_sriov(dev, num_vfs);
> + if (ret > 0)
> + pci_bus_add_device_vfs(dev);
> + return ret;
> + }
> #endif
> return 0;
> }
> Index: linux-2.6/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> ===================================================================
> --- linux-2.6.orig/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> +++ linux-2.6/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> @@ -346,8 +346,13 @@ int ixgbe_pci_sriov_configure(struct pci
> {
> if (num_vfs == 0)
> return ixgbe_pci_sriov_disable(dev);
> - else
> - return ixgbe_pci_sriov_enable(dev, num_vfs);
> + else {
> + int ret = ixgbe_pci_sriov_enable(dev, num_vfs);
> +
> + if (ret > 0)
> + pci_bus_add_device_vfs(dev);
> + return ret;
> + }
> }
>
> static int ixgbe_set_vf_multicasts(struct ixgbe_adapter *adapter,
> Index: linux-2.6/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> ===================================================================
> --- linux-2.6.orig/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ linux-2.6/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -7658,6 +7658,8 @@ skip_sriov:
> IXGBE_LINK_SPEED_10GB_FULL |
> IXGBE_LINK_SPEED_1GB_FULL, true);
>
> + pci_bus_add_device_vfs(pdev);
> +
> return 0;
>
> err_register:
> Index: linux-2.6/drivers/scsi/lpfc/lpfc_init.c
> ===================================================================
> --- linux-2.6.orig/drivers/scsi/lpfc/lpfc_init.c
> +++ linux-2.6/drivers/scsi/lpfc/lpfc_init.c
> @@ -10582,6 +10582,8 @@ lpfc_pci_probe_one(struct pci_dev *pdev,
> else
> rc = lpfc_pci_probe_one_s3(pdev, pid);
>
> + pci_bus_add_device_vfs(pdev);
> +
> return rc;
> }
>
> Index: linux-2.6/drivers/net/ethernet/emulex/benet/be_main.c
> ===================================================================
> --- linux-2.6.orig/drivers/net/ethernet/emulex/benet/be_main.c
> +++ linux-2.6/drivers/net/ethernet/emulex/benet/be_main.c
> @@ -4111,6 +4111,7 @@ static int lancer_recover_func(struct be
> if (status)
> goto err;
>
> + pci_bus_add_device_vfs(adapter->pdev);
> if (netif_running(adapter->netdev)) {
> status = be_open(adapter->netdev);
> if (status)
> @@ -4327,6 +4328,8 @@ static int be_probe(struct pci_dev *pdev
> dev_info(&pdev->dev, "%s: %s %s port %c\n", nic_name(pdev),
> func_name(adapter), mc_name(adapter), port_name);
>
> + pci_bus_add_device_vfs(pdev);
> +
> return 0;
>
> unsetup:
> @@ -4398,6 +4401,7 @@ static int be_resume(struct pci_dev *pde
> rtnl_unlock();
> }
>
> + pci_bus_add_device_vfs(adapter->pdev);
> schedule_delayed_work(&adapter->func_recovery_work,
> msecs_to_jiffies(1000));
> netif_device_attach(netdev);
> Index: linux-2.6/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_pf.c
> ===================================================================
> ---
> linux-2.6.orig/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_pf.c
> +++ linux-2.6/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_pf.c @@
> -568,8 +568,11 @@ int qlcnic_pci_sriov_configure(struct pc
> if (num_vfs == 0)
> err = qlcnic_pci_sriov_disable(adapter);
> - else
> + else {
> err = qlcnic_pci_sriov_enable(adapter, num_vfs);
> + if (err > 0)
> + pci_bus_add_device_vfs(dev);
> + }
>
> clear_bit(__QLCNIC_RESETTING, &adapter->state);
> return err;
> Index: linux-2.6/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c
> ===================================================================
> --- linux-2.6.orig/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c
> +++ linux-2.6/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c
> @@ -3048,7 +3048,12 @@ int bnx2x_sriov_configure(struct pci_dev
> pci_disable_sriov(dev);
> return 0;
> } else {
> - return bnx2x_enable_sriov(bp);
> + int ret = bnx2x_enable_sriov(bp);
> +
> + if (ret > 0)
> + pci_bus_add_device_vfs(dev);
> +
> + return 0;
> }
> }
>
> Index: linux-2.6/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
> ===================================================================
> --- linux-2.6.orig/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
> +++ linux-2.6/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
> @@ -5749,10 +5749,12 @@ static int init_one(struct pci_dev *pdev
> sriov:
> #ifdef CONFIG_PCI_IOV
> if (func < ARRAY_SIZE(num_vf) && num_vf[func] > 0)
> - if (pci_enable_sriov(pdev, num_vf[func]) == 0)
> + if (pci_enable_sriov(pdev, num_vf[func]) == 0) {
> dev_info(&pdev->dev,
> "instantiated %u virtual
> functions\n", num_vf[func]);
> + pci_bus_add_device_vfs(pdev);
> + }
> #endif
> return 0;
>
> Index: linux-2.6/drivers/net/ethernet/cisco/enic/enic_main.c
> ===================================================================
> --- linux-2.6.orig/drivers/net/ethernet/cisco/enic/enic_main.c
> +++ linux-2.6/drivers/net/ethernet/cisco/enic/enic_main.c
> @@ -2524,6 +2524,8 @@ static int enic_probe(struct pci_dev *pd
> goto err_out_dev_deinit;
> }
>
> + pci_bus_add_device_vfs(pdev);
> +
> return 0;
>
> err_out_dev_deinit:
> Index: linux-2.6/drivers/net/ethernet/neterion/vxge/vxge-main.c
> ===================================================================
> --- linux-2.6.orig/drivers/net/ethernet/neterion/vxge/vxge-main.c
> +++ linux-2.6/drivers/net/ethernet/neterion/vxge/vxge-main.c
> @@ -4731,6 +4731,9 @@ vxge_probe(struct pci_dev *pdev, const s
> vxge_hw_device_trace_level_get(hldev));
>
> kfree(ll_config);
> +
> + pci_bus_add_device_vfs(pdev);
> +
> return 0;
>
> _exit6:
> Index: linux-2.6/drivers/net/ethernet/sfc/efx.c
> ===================================================================
> --- linux-2.6.orig/drivers/net/ethernet/sfc/efx.c
> +++ linux-2.6/drivers/net/ethernet/sfc/efx.c
> @@ -2824,6 +2824,7 @@ static int efx_pci_probe(struct pci_dev
> netif_warn(efx, probe, efx->net_dev,
> "pci_enable_pcie_error_reporting failed
> (%d)\n", rc);
> + pci_bus_add_device_vfs(pci_dev);
> return 0;
>
> fail4:
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, May 15, 2013 at 7:12 PM, Greg Rose <[email protected]> wrote:
> I'm really not a fan of this. Seems to me the tail is wagging the dog
> here. Fix the driver to work without a PF driver being present.
Greg, Alex,
As I wrote over the V1 thread, currently we can't go and patch mlx4 to
use the sysfs API nor defer the call from within our probe function to
enable sriov since this requires some firmware change to allow
enabling SRIOV after some resources are initialized/provisioned.
Hence the patch suggested here or any other patch we can agree on
which will make sure that VF probing is done only once the PF is ready
is preferred, I think.
I wasn't sure to totally follow on the argument that things need to
work when the PF is absent in the sense there's no driver instance
around over which the PF is probed, if you can explain little better,
that would help.
Or.
On 20/05/2013 15:28, Or Gerlitz wrote:
> On Wed, May 15, 2013 at 7:12 PM, Greg Rose <[email protected]> wrote:
>
>
>> I'm really not a fan of this. Seems to me the tail is wagging the dog
>> here. Fix the driver to work without a PF driver being present.
>
> Greg, Alex,
>
> As I wrote over the V1 thread, currently we can't go and patch mlx4 to
> use the sysfs API nor defer the call from within our probe function to
> enable sriov since this requires some firmware change to allow
> enabling SRIOV after some resources are initialized/provisioned.
> Hence the patch suggested here or any other patch we can agree on
> which will make sure that VF probing is done only once the PF is ready
> is preferred, I think.
>
Maybe do the following:
1. split the VF PCI IDs into a separate PCI_ID device table.
2. have two pci_driver structs, one for all of the PF IDS and one with
the VF IDS.
3. only register the first pci_driver struct from mlx4_init().
4. register the one for the VFs once your PFs are all up and ready.
Eliezer
On Mon, May 20, 2013 at 5:58 AM, Eliezer Tamir
<[email protected]> wrote:
>>
> Maybe do the following:
> 1. split the VF PCI IDs into a separate PCI_ID device table.
> 2. have two pci_driver structs, one for all of the PF IDS and one with the
> VF IDS.
> 3. only register the first pci_driver struct from mlx4_init().
> 4. register the one for the VFs once your PFs are all up and ready.
that may help booting path that have driver built-in, or first time
load driver path.
For driver already loaded, and hotadd path, the problem is still there.
Yinghai
On Mon, 2013-05-20 at 15:58 +0300, Eliezer Tamir wrote:
> On 20/05/2013 15:28, Or Gerlitz wrote:
> > On Wed, May 15, 2013 at 7:12 PM, Greg Rose <[email protected]> wrote:
> >
> >
> >> I'm really not a fan of this. Seems to me the tail is wagging the dog
> >> here. Fix the driver to work without a PF driver being present.
> >
> > Greg, Alex,
> >
> > As I wrote over the V1 thread, currently we can't go and patch mlx4 to
> > use the sysfs API nor defer the call from within our probe function to
> > enable sriov since this requires some firmware change to allow
> > enabling SRIOV after some resources are initialized/provisioned.
> > Hence the patch suggested here or any other patch we can agree on
> > which will make sure that VF probing is done only once the PF is ready
> > is preferred, I think.
> >
> Maybe do the following:
> 1. split the VF PCI IDs into a separate PCI_ID device table.
> 2. have two pci_driver structs, one for all of the PF IDS and one with
> the VF IDS.
> 3. only register the first pci_driver struct from mlx4_init().
> 4. register the one for the VFs once your PFs are all up and ready.
But hotplug.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
On Mon, 2013-05-20 at 15:28 +0300, Or Gerlitz wrote:
> On Wed, May 15, 2013 at 7:12 PM, Greg Rose <[email protected]> wrote:
>
>
> > I'm really not a fan of this. Seems to me the tail is wagging the dog
> > here. Fix the driver to work without a PF driver being present.
>
> Greg, Alex,
>
> As I wrote over the V1 thread, currently we can't go and patch mlx4 to
> use the sysfs API nor defer the call from within our probe function to
> enable sriov since this requires some firmware change to allow
> enabling SRIOV after some resources are initialized/provisioned.
> Hence the patch suggested here or any other patch we can agree on
> which will make sure that VF probing is done only once the PF is ready
> is preferred, I think.
>
> I wasn't sure to totally follow on the argument that things need to
> work when the PF is absent in the sense there's no driver instance
> around over which the PF is probed, if you can explain little better,
> that would help.
It might be sufficient to return -EPROBE_DEFER from the VF probe
function when the PF is not yet ready, if you can easily detect that.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
On 05/20/2013 05:28 AM, Or Gerlitz wrote:
> On Wed, May 15, 2013 at 7:12 PM, Greg Rose <[email protected]> wrote:
>
>
>> I'm really not a fan of this. Seems to me the tail is wagging the dog
>> here. Fix the driver to work without a PF driver being present.
> Greg, Alex,
>
> As I wrote over the V1 thread, currently we can't go and patch mlx4 to
> use the sysfs API nor defer the call from within our probe function to
> enable sriov since this requires some firmware change to allow
> enabling SRIOV after some resources are initialized/provisioned.
> Hence the patch suggested here or any other patch we can agree on
> which will make sure that VF probing is done only once the PF is ready
> is preferred, I think.
I guess I am not understanding. Are you saying you have to enable
SR-IOV, then allocate some resources, and then wait for firmware to
complete, and then load VFs? Is it not possible to do whatever it is
you need to do in firmware first, and then enable SR-IOV?
Would it be possible for the VFs to detect this state? If so you could
probably work around it by either delaying probe as Ben suggested with
EPROBE_DEFER, or by using something such as the igbvf/ixgbevf approach
which will treat the lack of a PF and resources as a link down condition
until the PF and resources become available.
> I wasn't sure to totally follow on the argument that things need to
> work when the PF is absent in the sense there's no driver instance
> around over which the PF is probed, if you can explain little better,
> that would help.
>
> Or.
The problem I was referring to was the case where the PF is loaded, the
VFs are then assigned to guests, and then someone attempts to unload the
PF driver. The problem in that case is that disabling SR-IOV will cause
all of the guests with assigned VFs to crash so the solution is to leave
the VFs loaded when the PF is unloaded or we would have to block PF
driver unload. As such the Intel VFs have to deal with a PF that can be
unloaded while they are present.
If you take a look at the code for the igb/igbvf drivers it is a bit
easier to tell what is going on in terms of how we handle the unloaded
PF state. Basically what happens is that the mailbox we use goes dead
so we just report link down until we can get the PF to come back on the
other end of the mailbox.
Thanks,
Alex