Re: [PATCH] PCI: handle pci_sriov_set_totalvfs(dev, 0)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 07/31/2014 12:57 PM, Edward Cree wrote:
On 31/07/14 17:40, Alexander Duyck wrote:
If this PF-IOV mode is enabled what would be the layout of the PF
devices?  It seems like you should be able to scan for multiple PFs all
showing up on the same bus with a certain stride if you wanted to detect it.
Alas, it's (once again) more complicated than that.  There's another
mode, NIC partitioning, which has multiple PFs per port but connected
with a VLAN aggregator rather than a v-switch, which means that each PF
then can support VFs (the firmware limitation is that v-switches can't
be stacked).
So having multiple PFs on the port doesn't necessarily mean we can't do
SR-IOV.

Ed,
It is fairly obvious that your PCIe device operates in very unexpected,
non-std modes.
Ignore twiddling total-vfs, and just have the driver fail sriov
configuration when they can operate, print a warning why, and let's not create
complicated/convoluted hacks dependent on the use of various assumptions/uses
of pdev flags.
Ideally, when the device is configured in different modes, the SRIOV cap structure
should be modified so the pci sriov code doesn't try to act or reflect the
non-reality the device is in.

Thanks for pointing that out.  It seems like that is a bug.  We should
probably be checking for dev->sriov, not dev->is_physfn before calling
sriov_release.

I might see about submitting a patch to address that.
Unfortunately, that won't work because dev->sriov is in a union with
dev->physfn (presumably on the grounds that if you have an associated
PF, you're a VF, so you can't have VFs of your own).
So I think the test in pci_iov_release is correct, and anyone changing
dev->is_physfn after the PCI device has been set up is in the wrong.

I'm also unconvinced that having !(dev->is_physfn || dev->is_virtfn) is
a valid state; but maybe I'm misunderstanding and "is_physfn" doesn't
actually mean "is a physical function", but rather "has SR-IOV capability".
is_physfn is typically used to indicate the device is capable of acting
as a physical function.  So a non-IOV device will not set either
is_physfn or is_virtfn.
Ok, well I think a comment to that effect in struct pci_dev might be a
good idea.
In addition this solution would
also resolve the fact that the driver wouldn't actually have to be
loaded for it to work so if someone were to load a driver that didn't
contain the fix they would be blocked from enabling SR-IOV as well.
The current driver fails to probe the PF because it assumes the vswitch
creation failure is fatal.  There should never be a driver that knows it
can live without the vswitch but doesn't know that that breaks SR-IOV.

-Edward
Is the vswitch a hard requirement for something other than SR-IOV?  If
not then maybe you should consider modifying the driver so it simply
disabled SR-IOV if you cannot allocate the vswitch instead of blocking
probe.

That's exactly what I'm trying to do with pci_sriov_set_totalvfs(dev,
0); that's the whole point.
P.S.  You should really consider changing your email signature.  The bit
about "you may not use, copy or disclose to anyone this message" kind of
goes against the whole point of submitting patches to an open source
project.

I know; it's automatically appended by our mail server to all outgoing
mail.  I've already raised this with our IT department but they haven't
answered yet :(

-Edward
The information contained in this message is confidential and is intended for the addressee(s) only. If you have received this message in error, please notify the sender immediately and delete the message. Unless you are an addressee (or authorized to receive for an addressee), you may not use, copy or disclose to anyone this message or any information contained in this message. The unauthorized use, disclosure, copying or alteration of this message is strictly prohibited.
--
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


--
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




[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux