On 10/06/2011 06:18 PM, Nishanth Aravamudan wrote:
On 06.10.2011 [18:05:37 -0400], Prarit Bhargava wrote:
On 10/06/2011 05:03 PM, Nishanth Aravamudan wrote:
We have observed the following on Power systems with SR-IOV capable
adapters:
lpfc 0002:01:00.0: device not available because of BAR 7 [0x000000-0x00ffff] collisions
The issue is that on Power systems, PCI BARs cannot be remapped and VF
BARs might have values that collide. As far as I can tell, the current
SR-IOV code cannot be supported on Power and so it seems like we could
provide a hook for an architecture that might set CONFIG_PCI_IOV to
disable SR-IOV support (potentially at run-time).
I'd rather see an off/on switch because in a short while when powerpc
does support SRIOV there maybe no users of the code you've suggested.
Well, I think this is orthogonal to my patch and seems useful
independent of it.
There are two use cases being discussed here, I think.
1) Mis-behaving implementations.
Your patch does resolve this issue by allowing the *user* (not
the kernel itself at run-time) to determine if SR-IOV is the
root cause by trying to disable it.
2) Platforms that are known not to support SR-IOV as currently
implemented.
My patch resolves this issue by not attemping SR-IOV support on
those platforms.
I suppose your hammer is bigger than mine :) But I don't think it's good
to have a flag to workaround a code issue that the user must pass to
ensure functioning adapters.
It's a BIOS (or PPC BIOS-equiv) issue, and the kernel has a number of boot-time parameters
like the one proposed to resolve such nuances relative to known, working,
architected technology that works for the majority of cases it is employed.
-----8<------
[PATCH] Add a kernel parameter that disables SRIOV
Add sriov=[0|1] as a kernel parameter to disable or enable SRIOV. This is
useful for debugging SRIOV issues vs. bare-metal issues.
Signed-off-by: Prarit Bhargava<prarit@xxxxxxxxxx>
---
Documentation/kernel-parameters.txt | 4 ++++
drivers/pci/iov.c | 30 +++++++++++++++++++++++++++++-
2 files changed, 33 insertions(+), 1 deletions(-)
diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 854ed5c..235b5c5 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -2385,6 +2385,10 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
spia_pedr=
spia_peddr=
+ sriov= [HW]
+ Format: [0|1]
+ Enable or disable SRIOV support in kernel.
+
stacktrace [FTRACE]
Enabled the stack tracer on boot up.
diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 42fae47..da96e3f 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -408,6 +408,19 @@ static void sriov_disable(struct pci_dev *dev)
iov->nr_virtfn = 0;
}
+static unsigned int sriov_enabled = 1;
+static int __init do_sriov_enable(char *str)
+{
+ get_option(&str,&sriov_enabled);
+ if (!sriov_enabled)
+ printk(KERN_INFO "SRIOV: Disabled\n");
+ else
+ printk(KERN_INFO "SRIOV: Enabled\n");
+
+ return 1;
+}
+__setup("sriov=", do_sriov_enable);
Hrm, we only will print the above is sriov= is passed on the kernel
command-line, right? It seems like maybe we should print it always. And
perhaps the prefix should be "SR-IOV"?
static int sriov_init(struct pci_dev *dev, int pos)
{
int i;
@@ -419,6 +432,12 @@ static int sriov_init(struct pci_dev *dev, int pos)
struct resource *res;
struct pci_dev *pdev;
+ if (!sriov_enabled) {
+ rc = -EPERM;
+ /* in theory no flags should have been set ... */
+ goto failed;
+ }
+
Hrm, is this needed if pci_enable_sriov() checks it already? Isn't that
the only caller?
if (dev->pcie_type != PCI_EXP_TYPE_RC_END&&
dev->pcie_type != PCI_EXP_TYPE_ENDPOINT)
return -ENODEV;
@@ -614,7 +633,10 @@ resource_size_t pci_sriov_resource_alignment(struct pci_dev *dev, int resno)
struct resource tmp;
enum pci_bar_type type;
int reg = pci_iov_resource_bar(dev, resno,&type);
-
+
+ if (!sriov_enabled)
+ return 0;
+
if (!reg)
return 0;
@@ -667,6 +689,9 @@ int pci_enable_sriov(struct pci_dev *dev, int nr_virtfn)
{
might_sleep();
+ if (!sriov_enabled)
+ return -EPERM;
+
if (!dev->is_physfn)
return -ENODEV;
@@ -682,6 +707,9 @@ void pci_disable_sriov(struct pci_dev *dev)
{
might_sleep();
+ if (!sriov_enabled)
+ return;
+
if (!dev->is_physfn)
return;
Ah my patch needs these three checks as well, FWIW.
Thanks,
Nish
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
