On Fri, Jan 15, 2021 at 08:32:19PM -0800, Alexander Duyck wrote: > On Fri, Jan 15, 2021 at 6:06 AM Jason Gunthorpe <jgg@xxxxxxxxxx> wrote: > > > > On Thu, Jan 14, 2021 at 05:56:20PM -0800, Alexander Duyck wrote: > > > > > That said, it only works at the driver level. So if the firmware is > > > the one that is having to do this it also occured to me that if this > > > update happened on FLR that would probably be preferred. > > > > FLR is not free, I'd prefer not to require it just for some > > philosophical reason. > > It wasn't so much a philosophical thing as the fact that it can sort > of take the place as a reload. Asserting no driver is present and doing some SW-only "FLR" is pretty much the same thing. We can't issue FLR unless no driver is present anyhow, so really all this does is add a useless step. If some HW needs FLR then it can do it in here, but I don't see a value to inject it when not needed. Yes, if we were PCI-SIG we'd probably insist that a FLR be done, but we are not PCI-SIG, this is just Linux, and asserting there are no users of the MSI is sufficient. > However looking over the mlx5 code I don't see any handling of FLR > in there so I am assuming that is handled by the firmware. The device does the device side of the FLR, the mlx5 driver should trigger FLR during error recovery flows. > It is about the setup of things. The sysfs existing in the VF is kind > of ugly since it is a child device calling up to the parent and > telling it how it is supposed to be configured. Well, the logical place to put that sysfs file is under the VF, otherwise it becomes ugly in a different way. I agree it would be nicer if the file only existed when the right driver is loaded, and there was a better way to get from the PF to VF. > I'm sure in theory we could probably even have the VF request > something like that itself through some sort of mailbox and cut out > the middle-man but that would be even uglier. No, not ever. The VF is in a security domain that can't make those kinds of changes to itself. > In my mind it was the PF driver providing a devlink instance for the > VF if a driver isn't loaded. I think hacking up devlink to provide dummy devlink objects for VFs that otherwise wouldn't exist and then ensuring handover to/from real drivers that might want those objects natively, just for the sake of using devlink to instead of the existing PCI sysfs is major overkill. If we are even thinking of moving PCI to devlink I'd want to see devlink taken out of net and a whole devlink PCI subsystem infrastructure created to manage all this sanely. Hacking a subystem into devlink on the side with some small niche feature is not the way to approach such fundamental things. I also don't know if PCI will get much value from netlinkification, or if devlink is even the right netlink representation for PCI in the first place. Jason