On Tue, Oct 14, 2008 at 08:23:34AM +0800, Dong, Eddie wrote: > Matthew Wilcox wrote: > > Wouldn't it be more useful to have the iov/N directories > > be a symlink to the actual pci_dev used by the virtual > > function? > > The main concern here is that a VF may be disabed such as when PF enter > D3 state or undergo an reset and thus be plug-off, but user won't > re-configure the VF in case the PF return back to working state. If we're relying on the user to reconfigure virtual functions on return to D0 from D3, that's a very fragile system. > >> +For network device, there are: > >> + - /sys/bus/pci/devices/BB:DD.F/iov/N/mac > >> + - /sys/bus/pci/devices/BB:DD.F/iov/N/vlan > >> + (value update will notify PF driver) > > > > We already have tools to set the MAC and VLAN parameters > > for network devices. > > Do you mean Ethtool? If yes, it is impossible for SR-IOV since the > configuration has to be done in PF side, rather than VF. I don't think ethtool has that ability; ip(8) can set mac addresses and vconfig(8) sets vlan parameters. The device driver already has to be aware of SR-IOV. If it's going to support the standard tools (and it damn well ought to), then it should call the PF driver to set these kinds of parameters. > > I'm not 100% convinced about this API. The assumption > > here is that the driver will do it, but I think it should > > probably be in the core. The driver probably wants to be > > Our concern is that the PF driver may put an default state when it is > loaded so that SR-IOV can work without any user level configuration, but > of course the driver won't dynamically change it. > Do u mean we remove this ability? > > > notified that the PCI core is going to create a virtual > > function, and would it please prepare to do so, but I'm > > not convinced this should be triggered by the driver. > > How would the driver decide to create a new virtual > > function? Let me try to explain this a bit better. The user decides they want a new ethernet virtual function. In the scheme as you have set up: 1. User communicates to ethernet driver "I want a new VF" 2. Ethernet driver tells PCI core "create new VF". I propose: 1. User tells PCI core "I want a new VF on PCI device 0000:01:03.0" 2. PCI core tells driver "User wants a new VF" My scheme gives us a unified way of creating new VFs, yours requires each driver to invent a way for the user to tell them to create a new VF. Unless I've misunderstood your code and docs. > > From my reading of the SR-IOV spec, this isn't how it's > > supposed to work. The device is supposed to be a fully > > functional PCI device that on demand can start peeling > > off virtual functions; it's not supposed to boot up and > > initialise all its virtual functions at once. > > The spec defines either we enable all VFs or Disable. Per VF enabling is > not supported. > Is this what you concern? I don't think that's true. The spec requires you to enable all the VFs from 0 to NumVFs, but NumVFs can be lower than TotalVFs. At least, that's how I read it. But no, that isn't my concern. My concern is that you've written a driver here that seems to be a stub driver. That doesn't seem to be how SR-IOV is supposed to work; it's supposed to be a fully-functional driver that has SR-IOV knowledge added to it. -- Matthew Wilcox Intel Open Source Technology Centre "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step." -- 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
