> > 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.
No. It's more than that.
It's a given platform type on which SR-IOV will -never- work. Requiring
users to always pass sriov=false for those platforms to be usable is
beyond stupid.
Ben.
>
> >> -----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
